add TMX object layer support

[r1chardj0n3s]

No description provided.

[ccanepa]

• +1 to the refactor x1, x2,... -> left, right, ...
• would you mind to rename local variables that shadows common python names ? Besides being more clear, a bug in pyglet under python 3.3 was tracked to one such shadowing ( http://code.google.com/p/pyglet/issues/detail?id=704 ). Actually I noticed 'object' and 'map'
• I wanted to exercise the new code with some example, so I modified test_tmx.py into a test_tmx_objects.py with the following changes:
  • in the code simply change the file to load 'road-map.tmx' -> 'road-map-objects.tmx'
  • the new map was obtained by loading 'road-map.tmx' into Tile Map editor v 0.9.1, adding an object layer, adding a few objects, saving with the new name. (see the .tmx in https://gist.github.com/ccanepa/29d78eead5ae48a070ef#file-road-map-objects-tmx )
• When running the test_tmx_object.py with python 2.6 I got a traceback:

D:\cocos_pristine\cocos\test>py -2.6 test_tmx_objects.py
Traceback (most recent call last):
  File "test_tmx_objects.py", line 66, in <module>
    main()
  File "test_tmx_objects.py", line 44, in main
    test_layer = tiles.load('road-map-objects.tmx')['map0']
  File "..\cocos\tiles.py", line 202, in load
    obj = load_tmx(filename)
  File "..\cocos\tiles.py", line 347, in load_tmx
    layer = TmxObjectLayer.fromxml(tag, tilesets, tile_width, tile_height)
  File "..\cocos\tiles.py", line 1641, in fromxml
    height))
  File "..\cocos\tiles.py", line 1566, in fromxml
    w = int(tag.attrib['width'])
KeyError: u'width'

Do you see any reason for this ?
You have a simple example to compare with this ?

[ccanepa]

Re the traceback :
The current code expects width and height attribs for objects.
Not all objects have those, hence the traceback.
You meant to only support rectangles with width > 0 and height >0 ?

I see some possibilities here, listed from (less features, less work) to (more features, more work)

• support only rectangles with area > 0 :
  • set a try / except block to discard objects with no width or height,
  • maintain a count of objects discarded
  • add a member in TmxObjectLayer to report the number of discarded objects (so caller can get minimal feedback)
• support rectangles and points (people will be tempted to use them, and they are created by the same tool as rectangles)
  • if it has width and height, it is rectangle with area > 0
  • else if (does not have 'polygon points' and does not have 'polyline points' and not eclipse) it is point
  • set width = height = 0 for points
  • discard all other objets
• think the rect as the object bounding box.
  • It is trivial to calculate BB except maybe for ellipses.
  • Declare ellipses with be silently discarded.
  • All the others we calculate the BB and use as the rect.
  • TmxObject gains a new member, maybe 'tmx_type' with values in ['point', 'rect', 'polygon', 'polyline']
  • if type 'polygon' or 'polyline' , add a member 'points' to TmxObject storing the points coords
  • The user of the cocos library can access the object info and do whatever it feels.

In any case, whatever will be supported should be clearly stated in the docs.

What do you think ?

[r1chardj0n3s]

Great feedback, thanks!

1. I will look at renaming those names that shadow builtins, yep.
2. Indeed I didn't attempt to implement any shapes other than rects. They were just out of scope for the amount of effort I could put in. I guess raising an exception upon encountering them would be a good idea... Explicitly fail in the face of unsupported shapes.

[ccanepa]

On Tue, May 20, 2014 at 1:56 AM, Richard Jones notifications@github.comwrote:

Great feedback, thanks!

1. I will look at renaming those names that shadow builtins, yep.

Great,

1. Indeed I didn't attempt to implement any shapes other than rects.
   They were just out of scope for the amount of effort I could put in. I
   guess raising an exception upon encountering them would be a good idea...
   Explicitly fail in the face of unsupported shapes.

If you don't have time to support more types, let me try it, just ring the
bell when you are done.

[ccanepa]

@r1chardj0n3s : do you want I do the rename so I can merge to master ?

[r1chardj0n3s]

Sorry, I'm kinda swamped at the moment. If you have spare moments please do the rename, thanks!

[ccanepa]

this PR was included into PR #215 , which was merged into master. Closing.