Fixed old way of finding types of objects to draw in Overlay.py, changed icon drawing in Systems.py.

Fixed old way of finding types of objects to draw in Overlay.py, changed icon drawing in Systems.py.

[mithro, 2009-05-27 04:31:27]

Just some small fixes.

http://codereview.appspot.com/67114/diff/1/2
File windows/main/overlays/Overlay.py (right):

http://codereview.appspot.com/67114/diff/1/2#newcode280
Line 280: if not parent.id == 0 and (not hasattr(obj, 'parent') or not
self.cache.objects[parent.parent].id == 0):
Can you please add a comment about this code explaining what is excluding and
why.

Maybe a comment about the troubles with Galaxy and stuff..

http://codereview.appspot.com/67114/diff/1/3
File windows/main/overlays/Systems.py (right):

http://codereview.appspot.com/67114/diff/1/3#newcode25
Line 25: from extra	   import objectutils
The indent here doesn't work. Just use: 
  from extra import objectutils

http://codereview.appspot.com/67114/diff/1/3#newcode111
Line 111: #	raise TypeError('SystemIcon must be given a StarSystem, %r' %
system)
Please just remove this code rather then comment it out.

[mithro, 2009-05-27 05:07:36]

http://codereview.appspot.com/67114/diff/7/8
File windows/main/overlays/Overlay.py (right):

http://codereview.appspot.com/67114/diff/7/8#newcode286
Line 286: # differentiate galaxies and fleets, here.
This doesn't make sense.

The hierarchy looks like this:
 Universe
 +-Galaxy
 |  +-Star System
 |  |  +-Planet
 |  |  |  +- Ship 0
 |  |  +-Ship 1
 |  +-Ship 2
 +-Ship 3

It the ID of this object is zero then it is the Universe.
If the object's parent ID of this object is zero then it could either be a
Galaxy or a Ship.

[mithro, 2009-05-27 08:59:03]

http://codereview.appspot.com/67114/diff/12/1004
File windows/main/overlays/Overlay.py (right):

http://codereview.appspot.com/67114/diff/12/1004#newcode281
Line 281: 
I recommend create a function called "IsTopLevel" which takes the id and cache.

The function can then be something like:

def IsTopLevel(cache, id):
  """If they are not galaxy or universe."""

  #FIXME: This is a hack, currently we determine this by assuming any object
which has no useful information and is parented to Universe is "Top Level".

  # Is it the Universe?
  if oid == 0:
     return True

  # Is the object's parent the Universe?
  if self.cache.objects[obj.parent].id != 0:
     return False

  # Does the object have any interesting properties?
  # Yes -> Not Top-level.
  return len(obj.properties) > 0

Then the code becomes
  if TopLevel(self.cache, obj):
    self[oid] = []
    return

  if not TopLevel(self.cache, self.cache[obj.parent]):
    self[oid] = []
    return

  <do something else here>

http://codereview.appspot.com/67114/diff/12/1004#newcode302
Line 302: if not parent.id == 0:
if partner.id != 0:

http://codereview.appspot.com/67114/diff/12/1004#newcode306
Line 306: return
Everything should have a parent except the Universe. Since you are already
excluding the Universe by "if not parent.id == 0" line.

http://codereview.appspot.com/67114/diff/12/1004#newcode310
Line 310: if not self.cache.objects[parent.parent].id == 0:
if self.cache.objects[parent.parent].id != 0:

http://codereview.appspot.com/67114/diff/12/1004#newcode319
Line 319: return
See above....

[Greywhind, 2009-05-27 22:56:29]

I've made some changes in response to this, but it seems like git-cl has trouble
dealing with changed submodules. It gives me:

greywhind (03:28 PM):~/Programming/tp04/tpclient-pywx > git cl upload
origin/tp04 extra/objectutils.py             |   32 ++++++++++++++++++++++-
 libtpproto-py                    |    2 +-
 windows/main/overlays/Overlay.py |   13 ++++++---
 windows/main/overlays/Systems.py |   52 ++++++++++++++++++++++---------------
 4 files changed, 71 insertions(+), 28 deletions(-)
This branch is associated with issue 67114. Adding patch to that issue.
Got error status from 'git show aff2b4cf79ae082b797c8b8d19d25aff4a23c5f9'

[mithro, 2009-05-28 01:10:30]

http://codereview.appspot.com/67114/diff/1009/14
File extra/objectutils.py (right):

http://codereview.appspot.com/67114/diff/1009/14#newcode38
Line 38: def is_top_level(cache, oid):
This needs a docstring.

This function name would be better as isTopLevel.

http://codereview.appspot.com/67114/diff/1009/15
File windows/main/overlays/Overlay.py (right):

http://codereview.appspot.com/67114/diff/1009/15#newcode285
Line 285: parent = self.cache.objects[obj.parent]
Why do you get the parent object here? It doesn't appear to be used.

[mithro, 2009-05-28 01:43:24]

LGTM

http://codereview.appspot.com/67114/diff/18/19
File extra/objectutils.py (right):

http://codereview.appspot.com/67114/diff/18/19#newcode8
Line 8: # Each tuple has the form: (x, y, z, coord name)
These comments should be part of the doc string.

http://codereview.appspot.com/67114/diff/18/19#newcode40
Line 40: # Attempt to determine whether an object is the universe or a galaxy.
This comment is not needed.