TmxObjectMapCollider should not redefine collide_map

[dangillet]

After fixing issue #248 it was discovered that TmxObjectMapCollider is inheriting from RectMapCollider but it redefines its collide_map method. This is now unnecessary and TmxObjectMapCollider could now just inherit from RectMapCollider without any additional changes.

[dangillet]

Hi Claudio,

Fix is done in branch TmxObjectMapCollider a0495523.

Let me know if it's ok to merge to master.

[ccanepa]

The parent class RectMapCollider has the docstring """This class implements collisions between a moving rect object and a tilemap.""" and the core functionallity is in method collide_map(map, ...), which restricts a rect movement to honor whatever collision restrictions imposed by the tilemap 'map'

What would be the semantics for TmxObjectMapCollider(RectMapCollider) ? Seems natural that the docstringd become """This class implements collisions between a moving rect object and a the objects in a TmxObjectLayer.""" and collide_map(map,...) becoming "restricts a rect movement to honor whatever collision restrictions imposed by the TmxObjectMap 'map'"

But the code in RectMapCollider (and he one in the old TmxObjectMapCollider) only considers collision with tiles. To allign the code with the stated semantic, the lines

            if cell is None or cell.tile is None or not cell.intersects(new):
                continue

should be removed, and hence we need to write explicitly TmxObjectMapCollider.collide_map

What do you think ?

[ccanepa]

correction: the lines should be changed to

            if not cell.intersects(new):
                continue

[dangillet]

Hi Claudio,

Well I'm not too familiar with TMX maps. But I think I understand what you mean. Correct me if I'm wrong, but it could be that the TmxObject does not have a tile, but you would like to know when you collide with it, like an invisible trigger zone used to start the fight with the final boss.

The problem is that the collide_map method does not allow our moving rect to move into the zones of the map. It pushes it back so that there is no overlapping. The real question is : What is the desired behaviour for collisions with TmxObjectLayer ?

Maybe if there is a tile attached to the cell, then the rect movements should be restricted, and if not, the rect can move freely, but in both cases, the collide_bottom, collide_top, etc should be called. This could be implemented rather easily.

Daniel.

[ccanepa]

Lots of different desired behavior are possible, it really depends on the game mechanics and the specific objects in the interaction.

Some concrete examples when the desired behavior is to forbid some actor to step into the boundaries of other objects.

• moving platforms in a platform game, it is desired that the rect (player) can land in the platform if it is coming from above, bounce if coming in other directions
• in very simply cavern rooms, colliding boundaries can be defined by a few rectangles, and here it it desired to block player movement outside the free area.

We can build TmxObjectMapCollider.collide_map to that case (more later)

Examples when only detect superposition is necessary, and not blocking player

• enemies touching player ?
• player touching a power up ?
• rectangular mouse selection

Those are handled by TmxObjectLayer methods .get_in_region(...) and .collide(...)

Going back to TmxObjectMapCollider.collide_map (map,...)

• if we desired only collision with tiles in the TmxObjectLayer passed in map, then that's exactly what RectMapCollider does, so no need for a separate TmxObjectMapCollider
• To do collision with any TmxObject into the map, and writing the less additional code possible over the one in RectMapCollider, we can simply override collide_map to eliminate the filtering by 'is cell'., like mentioned in my above comments. The user will be forced to add properties collide_bottom, etc to any object
• To simplify the case for the user, we can assume collision in all sides, then no collide_bottom and etc are needed but we need to also override do_collision (same code as in superclass but deleting the line g = cell.get and deleting the four instances of "g('zzz') and" in the ifs)

What do you think ?

[dangillet]

The user will be forced to add properties collide_bottom, etc to any object

Maybe I'm misunderstanding you. These methods have to be defined for a RectMapCollider or TmxMapCollider. This is normally for the player.

Thinking more about this, my understanding is that you design a map in Tiled Map Editor. You will place tiles, some of which are non-blocking (say the grass) and others are (say walls). For those you can set a top, bottom, left and right property to define that all sides should be blocking. If you want to allow one side to be non-blocking (say the bottom of a platform in a platform game, where you can jump through platforms from below) you just don't set the bottom property.

Our player can have as an instance attribute a derived class of RectMapCollider where the methods collide_xxx can be re-defined to have additional behaviour when that side hits an obstacle. collide_bottom could be used to set a variable player.on_ground = True, for instance.And maybe the player is hurt if his heads hits something. test_platformer.py shows an implementation where the RectMapCollider is actually an Action. So it can access the player attributes through self.target.

Now we can also have invisible cells, which don't have a tile. They are just zones which could be used to know when to trigger some actions. Say when the player moves through that zone, some ennemies appear. But I don't think this should be checked in the TmxMapCollider. As you said 3 messages ago, its purpose is only to restrict the Rect (player) movements to honnor whatever collision restrictions are imposed by the map. For such trigger zones, we agree that TmxObjectLayer methods .get_in_region(...) and .collide(...) must be used.

Now sure, we could imagine that in a map, the level designer wants a blocking invisible zone. Then we would have a cell without a tile which could have top, bottom, left and right properties. To account for this, as you correctly said, we should redefine the collide_map method to

if cell is None or not cell.intersects(new):
    continue

But besides that, I don't see how TmxMapCollider could be different from RectMapCollider.

Any suggestions ?

[ccanepa]

On 4/4/16, Daniel Gillet notifications@github.com wrote:

The user will be forced to add properties collide_bottom, etc to any object

Maybe I'm misunderstanding you. These methods have to be defined for a RectMapCollider or TmxMapCollider. This is normally for the player.

My bad, I was thinking of the properties 'up', 'down', ...a tile (in RectMapLayer) must have to be collidable

Thinking more about this, my understanding is that you design a map in Tiled Map Editor. You will place tiles, some of which are non-blocking (say the grass) and others are (say walls). For those you can set a top, bottom, left and right property to define that all sides should be blocking. If you want to allow one side to be non-blocking (say the bottom of a platform in a platform game, where you can jump through platforms from below) you just don't set the bottom property.

Thats true, but it is not all the truth :-)

A map done in Tiled Map Editor can also contain rectangles, ellipses, polygonals, tiles not anchored to the grid and more.

Those objects don't care for the grid and are loaded in cocos in a TmxObjectLayer.

A Tiled map does not need to have tiles at all, and the objects mentioned, except the tile object, are not associated with any tile.

Hence, if the game logic needs to collide player with those objects, we need a TmxCollider collide_map that ignores the tile attribute.

If a particular game wants to collide with any side of any object being obliged to set 'top', etc in each object is a nuance, hence the posibility to override do_collision to not check those propereties.

Our player can have as an instance attribute a derived class of RectMapCollider where the methods collide_xxx can be re-defined to have additional behaviour when that side hits an obstacle. collide_bottom could be used to set a variable player.on_ground = True, for instance.And maybe the player is hurt if his heads hits something. test_platformer.py shows an implementation where the RectMapCollider is actually an Action. So it can access the player attributes through self.target.

I think this was the original intent.

Now we can also have invisible cells, which don't have a tile. They are just zones which could be used to know when to trigger some actions. Say when the player moves through that zone, some ennemies appear.But I don't think this should be checked in the TmxMapCollider. As you said 3 messages ago, its purpose is only to restrict the Rect (player) movements to honnor whatever collision restrictions are imposed by the map. For such trigger zones, we agree that TmxObjectLayer methods .get_in_region(...) and .collide(...) must be used.

Now sure, we could imagine that in a map, the level designer wants a blocking invisible zone. Then we would have a cell without a tile which could have top, bottom, left and right properties. To account for this, as you correctly said, we should redefine the collide_map method to

if cell is None or not cell.intersects(new):
    continue

But besides that, I don't see how TmxMapCollider could be different from RectMapCollider.

keep in mind that in the TmxMapCollider collide_map, when you see

    cells = map.get_in_region(*(new.bottomleft + new.topright))
    for cell in cells:

it is really objs = map.get_in_region(*(new.nottomleft + new.topright)) for obj in objs:

obj can can be a rect, and ellipse, etc

if you let the next line as in RectMapCollider, that is if cell is None or cell.tile is None or not cell.intersects(new):

you are throwing out non cell objects, because they have obj.tile = None

Any suggestions ?

think a moving platform in a level. think you don't want to depict with tiles, that you want more freedom for your graphic artist. Then you use a tmx rect object. And you can use a TmxMapCollider to do the collision.

[dangillet]

Ok, I see !

So I started by reimplementing the collide_map in TmxMapCollider to account for objects with a None tile attribute.

I tried to factor out the difference, but it didn't work as you can see in 39961cb. The problem was that we select the cells while we resolve collisions, which can mutate the new rect and so change the criteria (ie. cell.intersects(new)) when considering the new cell in region.

I could have made a method to be called in the for loop. But function calls are expensive, so I thought it's best to just copy everything in TmxMapCollider and make the necessary changes.

What do you think ?

[ccanepa]

There are multiple small issues with current code related to tmx collider. I created some basic examples at the repo https://github.com/los-cocos/etc_code/tree/master/tmx_collision and in parallel refactored code in cocos.tiles in the branch https://github.com/los-cocos/cocos/tree/tmx_colision

I think now the api is cleaner and easier to explain and use. I limited changes to tmx object related things, later some decisions can be ported to RectMapCollider.

I annotated the whats and the whys about the changes, and I will paste in next comment. Probably they are too long, but nobody is forced to read them.

So, @dangillet , and @r1chardj0n3s if he has time, care to review ?

[ccanepa]

• copy methods collide_map and do_collision from RectMapCollider to TmxObjectMapCollider to allow modification
• do not override TmxObject.intersects : it has no sense to change the base class signature for near same functionality.
• change TmxObjectLayer.get_in_region implementation to not use TmxObjects intersects: less calls, no signature adaption needed
• change the keys used in TmxObjectLayer._update_sprite_set from obj position to obj: unique key, easier access
• External script to exercise / test do_collision is tmx_get_in_region.py, to be found at repo https://github.com/los-cocos/etc_code/tree/master/tmx_collision
• Created a test_platformer.py translation from tiles to tmx objects to exercise tmx collision, called tmx_player_collision.py to be found at repo https://github.com/los-cocos/etc_code/tree/master/tmx_collision
• In collide_map eliminate the filtering that discarded objects that weren't tiles : we want to consider all types of TmxObjects, not only tiles
• in do_collision assume we want to collide with all objects in the layer, thus eliminating the need for props 'top', 'left',... to signal an object side should be considered for collision. So prop checks eliminated : easier to prepare a working .tmx file, less code, by subclassing and modify the more detailed behavoir can be recovered, so not loss of posibilities
• Now it is time to look at the (dx, dy) params and return value in collide_map and do_collide, that always felt a bit off focus. The crucial observation is thar the core task "mutate 'new' to not overlap any object", after the @dangillet refactoriong, does not uses at all (dx, dy). So theres the oportunity to change the semantics and code behind to something more clear cut
• looking at RectMapCollider.collide_map usage in test_platformer.py we can see the posible semantics
  • the two last collide_map parameters as the displacement dx, dy , and the return value can be one of:
  • the effective displacement dx, dy = new.position - last.position with 'new' in the state at end of collide_map
  • the "unused" displacement dx, dy = new.position at enter - new.position at end of collide_map
  • last two collide_map parameters to be the velocity, vx, vy, and return to be vx, vy the velocity after the interaction with obstacle(s)
• both displacement semantics are hard to use by the caller in a sensible way

• the velocity semantic is clear cut and easy to use by the caller

  vx, vy = actor.velocity
  # update velocity by any acceletation needed, by example
  vy += gravity * dt
  
  last = actor.get_rect()
  # get tentative new position
  new = last.copy()
  (dx, dy) += (vx, vy) * dt
  new.position += (dx, dy)
  
  # account for collision and update actor's pos and vel
  actor.velocity = collide_map(maplayer, last, new, vx, vy)
  actor.center = new.center

• so, what sensible 'velocity after consider interaction with obstacles' can we offer with the info that already handles collide_map ? Three immediate candidates come to mind
  • 'bouncy' interaction. Think the ball in a arkanoid / blockout game interacting with walls and blocks. Reflects velocity in any axis that the actor collides (if collision in z, vz = -vz else vz)
  • 'sticky' interaction. Sticky grenade, projectile in a hook weapon. Stop in both axis at any collision (if collision vx,vy=0,0)
  • 'slide' interaction. Common player behavior, think player pushing into a wall or jumping; it stops only on the axis that has a collision. (if collision in z, vz = 0)
• They are all sensible, can we support all ? Yes. interaction type by object would be cumbersome, needing props and code. Lets do a per TmxObjectMapCollider instance, so the interaction type is specified in __init__
• introduce changes in collide_map to implement this functionality
  • remove dx, dy in the call to do_collision; they will not be needed
  • at the beginning, reset bumped_z flags that tells if a hit was detected on the direction z
  • let do_collision update those flags when it detect a collision
  • calculate the return value (vx, vy) from the bumped_z flags, using
  • the correct helper method on_bump_<interaction mode> as set by __init__
• Remove parameters dx, dy in do_collision, return None instead of dx, dy
• remove now unused code to update dx, dy
• add code to set bumped_z when a collision in the z axis was detected
• The last remaining uses for dx, dy in docollision are in the calls to the .collide[top | bottom | left | right]
• What if we pass the object collided instead of dx or dy ? Seems useful to implement
  • player explodes if touches some specific object
  • a bonus spawns from an object when bumped from the bottom
  • object explodes if touched by the actor
• So changed that.
• Add an utility function make_collision_handler(collider, maplayer)
• example usages for the final code at repo https://github.com/los-cocos/etc_code/tree/master/tmx_collision

[dangillet]

Hi Claudio,

Lots of good things here. I'm still digesting all that information.

All in all, I think this is a very good addition, and as you said, we might then go back to RectMapCollider to have similar semantics.

Two minor things:

• change TmxObjectLayer.get_in_region implementation to not use TmxObjects intersects: less calls, no signature adaption needed.

  Good thing not to use TmxObjects.intersects, but I'm wondering why not using then Rect.intersects instead ? Sure in the current form, it's a bit more efficient as we don't have to call a method. But it also means that if in the future we find out that Rect.intersects should be changed, we will forget that there is also something in TmxObjects.get_in_region.

• TmxObjectMapCollider.__init__ method uses getattr to connect self.on_bump_handler to the correct method. I don't know if this is a possible scenario, but if the client want to change the way the collider behaves, this method does not allow it. A simple way to do it would be to make on_bump_handler a property which would point to the correct method using getattr. Something like this:

class TmxObjectMapCollider(RectMapCollider):
    """Helper to handle collisions between an actor and objects in a TmxObjectLayer

    Arguments:
        velocity_on_bump (str) : one of 'bounce', 'stick', 'slide'

    The argument 'velocity_on_bump' selects how the velocity will be modified
    when the actor rectangle bumps into an object.
    """

    def __init__(self, velocity_on_bump='slide'):
        self.on_bump_handler = velocity_on_bump

    @property
    def on_bump_handler(self):
        return self._on_bump_handler

    @on_bump_handler.setter
    def on_bump_handler(self, velocity_on_bump):
        self._on_bump_handler = getattr(self, 'on_bump_' + velocity_on_bump)

I'll try to give you a better review later on, once I've digested everything.

In any case, well done ! This is a nice achievement.

Daniel.

[dangillet]

Hello,

I noticed the changes you made in the last commit cafbce37d.

One additional note that came to my mind while reading the docstring for do_collision. As we removed the functionality to test if the TmxObject has a property to know which side is collidable, would it make sense to add the old code as an example how one could subclass TmxObjectMapCollider.do_collision to provide this behaviour ?

Imagining how to do this subclassing, I realize that the user would have to copy an awful lot of code to only change the first part. Maybe we should split do_collision in 2 different methods. One is responsible to calculate the dx_correction and dy_correction and the other is responsible to react to that.

Here is a quick example of what I mean. Note that I didn't put any docstrings. It's just to show the functionality.

class TmxObjectMapCollider(RectMapCollider):
    def collide_map(self, maplayer, last, new, vx, vy):
        self.bumped_x = False
        self.bumped_y = False
        tested = set()
        cells = maplayer.get_in_region(*(new.bottomleft + new.topright))
        do_collision = self.do_collision
        resolve_collision = self.resolve_collision
        for cell in cells:
            # the if is not superfluous in the loop because 'new' can change
            if not cell.intersects(new):
                continue
            tested.add(cell)
            dx_correction, dy_correction = do_collision(cell, last, new)
            resolve_collision(cell, new, dx_correction, dy_correction)
        cells_collide_later = [cell for cell in tested
                               if hasattr(cell, 'collide_later')]
        for cell in cells_collide_later:
            if cell.intersects(new):
                dx_correction, dy_correction = do_collision(cell, last, new)
                resolve_collision(cell, new, dx_correction, dy_correction)
            del cell.collide_later

        vx, vy = self.on_bump_handler(vx, vy)

        return vx, vy

        return vx, vy

    def do_collision(self, cell, last, new):
        dx_correction = dy_correction = 0.0
        if last.bottom >= cell.top and new.bottom < cell.top:
            dy_correction = cell.top - new.bottom
        elif last.top <= cell.bottom and new.top > cell.bottom:
            dy_correction = cell.bottom - new.top
        if last.right <= cell.left and new.right > cell.left:
            dx_correction = cell.left - new.right
        elif last.left >= cell.right and new.left < cell.right:
            dx_correction = cell.right - new.left

        return dx_correction, dy_correction

    def resolve_collision(self, cell, new, dx_correction, dy_correction):
        if dx_correction != 0.0 and dy_correction != 0.0:
            # Correction on both axis
            if hasattr(cell, 'collide_later'):
                if abs(dx_correction) < abs(dy_correction):
                    # do correction only on X (below)
                    dy_correction = 0.0
                elif abs(dy_correction) < abs(dx_correction):
                    # do correction only on Y (below)
                    dx_correction = 0.0
                else:
                    # let both corrections happen below
                    pass
            else:
                cell.collide_later = True
                return

        if dx_correction != 0.0:
            self.bumped_x = True
            # Correction on X axis
            new.left += dx_correction
            if dx_correction > 0.0:
                self.collide_left(cell)
            else:
                self.collide_right(cell)
        if dy_correction != 0.0:
            # Correction on Y axis
            self.bumped_y = True
            new.top += dy_correction
            if dy_correction > 0.0:
                self.collide_bottom(cell)
            else:
                self.collide_top(cell)
        return

This comes at the cost of an extra function call in the loop. So less efficient. But the user would only need to override do_collision which is then a smaller amount of code to change. Not sure it's worth it though...

[ccanepa]

I like it. Performance would not be too different. Added a commit with this.

While looking to do docstrings (old) do_collision was a bit blurry. So tentatively I moved the conditional from do_collision to collide_map, now do_collision is clear cut. Now it was easy to eliminate the transitory flag collide_later, the 'tested' set and the 'del' associated. : For what you called do_collision I considered alternatives 'detect_collision', 'measure_overlap'; tentatively set to 'detect_collision'

I think both the overall code, docstrings and api is clearer. Added a commit with this.

I'm open to change in this feature, if you have more suggestions.

Otherwise, I will be thinking to refactor RectMapCollider as:

• make signatures in RectMapCollider compatible with TmxObjectMapCollider
• adjust code in each method adapting the tmx one to rect
• collision in rect map will continue using props, both for compatibility and to show alternative collision detection

About this, it would be better to use same variable / parameter names in both classes: easier to spot the significant differences, and mix and match snippets of code. 'cell' sounded awkward while working in Tmx. Would replacing it by 'obj' sound too bad in RectMapCollider ? 'collidable' ? another ?

edit: commit added, typo.

[dangillet]

Hello,

Indeed the code is WAY better this way. It's really a joy to see the difference with the old code. It does not surprise me so much that by cutting the code into more functions, you were able to clean up the way we detected collisions in two passes. Well done ! I really like the new function names. I think it makes sense to have a detect_collision followed by a resolve_collision.

I took the liberty to adjust slightly the docstrings. It's generally only some cosmetic changes to better use sphinx autodoc utils. I pushed a new branch on top of yours, https://github.com/los-cocos/cocos/tree/tmx_collision_dan.

One question you didn't really answer : what about on_bump_handler as a property ? I don't think it's that important. But it doesn't cost much to have it.

Regarding the RectMapCollider refactoring, I agree with everything except I wouldn't use props as the default. What I would do is offer in the code RectMapCollider and RectMapWithPropsCollider which derives from RectMapCollider and shows how to use this feature. Of course we might need to amend some files which maybe used the old behaviour:

• cocos\test\test_platformer.py
• cocos\utest\aux_RectMapCollider__no_stuck.py
• cocos\utest\test_RectMapCollider__no_stuck.py
• cocos\utest\viewer_RectMapCollider__no_stuck.py

Daniel.

[ccanepa]

Thanks for the docstrings changes, adopted.

Also adopted RectMapCollider and RectMapWithPropsCollider.

Moved RectMapCollider to be near the other colliders

After that I did multiple passes to

• flesh the docstrings to see if something was difficult to document
• adapt so base methods docs are consistent when reading from derived class
• sphinx formatting in general
• eliminated sphinx markup in first line of docstrings, which are displayed by autocompletion and looked funny there.
• when references to a class, etc where too near, eliminated sphinx markup in one for fancier view

No propagation of changes outside cocos/ was done

Committed at 3aee9eaebaa after rebasing on master

One question you didn't really answer : what about on_bump_handler as a property ? I don't think it's that important. But it doesn't cost much to have it.

I'm -0.1 about that; being that functions are first class entities in python it feels like bloat. If you think it is > 0.1 worthy i will include.

tiles.py is growing big, and the Collider related parts in tiles doc page are scattered around. To me it makes sense to move collider code to a new mapcolliders module (with some compatibility shims left on tiles).

Selecting bump style at __init__ plays a bit dirty with mixin style usage and custom on_bump_handler.

I will do a branch with both changes to see how it look

[ccanepa]

committed 589248c2c2 "moving colliders to cocos.mapcolliders"

and later

ad6e3ccfb eliminated string names for onbump*, changes propagated outside cocos/

The last was to have a more clean mixin style colliders.

also adapted exploratory scripts in repo los-cocos/etc_code to those changes, commit c89b2eed7ae

I like how it looks, but I'm open to suggestions.

After handling suggestions, if any, I will add to test/ one of the tmx collider examples, then maybe edit dosctrings have shorter line lenghts, then squash and merge.

[dangillet]

Hi Claudio,

You've been quite busy. Well done ! Sorry I couldn't review all this earlier.

I like a lot all the changes. I think this is a neat improvement on the older code. I only have small suggestions / remarks.

For docstrings, when you describe Arguments I found out that if you reference a type which is described somewhere in cocos, appending a dot before the type will make it automatically a link in the doc. So for instance:

Arguments:
    maplayer (.RectMapLayer) : layer with solid objects to collide with.

will make RectMapLayer a link to the class description.

eliminated string names for onbump*, changes propagated outside cocos/ The last was to have a more clean mixin style colliders.

I understand the reason. I'm just thinking that for casual coders not familiar with this module it's kind of tricky to remember that you need to instantiate a class and then you need to assign to on_bump_handler a method. But as you said, for mixin style, this is probably the preferred method. Maybe a solution would be to leave a __init__ method with an optional keyword. Something like this:

def __init__(self, velocity_on_bump=None):
    if velocity_on_bump:
        self.on_bump_handler = getattr(self, 'on_bump_' + velocity_on_bump)

So the client code is not forced to pass any parameters to the __init__ method, but it's still possible if that's how the client wants to define its on_bump_handler method.

With this in place, test_platformer.py which does not use mixin classes could be re-written like this:

    # give a collision handler to the player
    mapcollider = mapcolliders.RectMapWithPropsCollider(velocity_on_bump='slide')
    player.collision_handler = mapcolliders.make_collision_handler(mapcollider, tilemap)

Regarding my question above I was more like -0.1. And anyway this is now irrelevant as client code can redefine on_bump_handler as you show for the mixin class case.

One last tiny suggestion. In test_platformer.py, it's kind of frustrating that the character cannot jump over the obstacles. Changing the JUMP_SPEED to 1000 would allow jumping over the obstacles. We can then go to the end of the map and see the cauldron. :)

Dan.

[ccanepa]

For docstrings, when you describe Arguments I found out that if you reference a type which is described somewhere in cocos, appending a dot before the type will make it automatically a link in the doc.

Good to know. Things like this should be included in docgen/dev/documentation.txt

for mixin style, this is probably the preferred method. Maybe a solution would be to leave a init method with an optional keyword.

Good compromise, will do.

One last tiny suggestion. In test_platformer.py, it's kind of frustrating that the character cannot jump over the obstacles. Changing the JUMP_SPEED to 1000 would allow jumping over the obstacles. We can then go to the end of the map and see the cauldron. :)

Sure.

[ccanepa]

commit eb8ee8064ff reintroduces velocity_on_bump, as talked commit 64e43acb3a does a pass on docstrings (adjusted some content, shortened lines to be more near the 80 columns recommended by pep8)

Not changed the jump velocity, I prefer to change the map to have some ceiling that the player could hit and some more platform to allow reach the cauldron. This will wait after the merge, because tools/editor.py misbehaved when saving the edited map, and fixing that will probably need to touch tiles.py

rebased on master, squashed locally, merged to master. pushed as commit 058a359b8c79

The remote branch is unsquashed and I will let alive a few days, but obviously any further desired change must go on master or a new branch.

[ccanepa]

Changed test_platformer.py to use the RectMapCollider, props collision will not work well with the tileset it uses now. Player now reach the cauldron, interaction with walls are as expected.

I will wait a few days just in case some more comment, then will close.

[dangillet]

I saw your changes. Nice. I noticed indeed that the editor cannot save properly a map as there are not functions which would allow an atlas, image or tileset to be converted to XML. Looking at how the atlas is loaded, too much information is lost to be able to easily save the data back as it was in the file. So I guess this should be a separate issue which will involve some efforts.

For the rest, it was good fun to be able to run around in the platform. Kudos !