los-cocos / cocos

graphic library for games and multimedia, for python language on PC-like hardware
http://los-cocos.github.io/cocos-site/
Other
632 stars 147 forks source link

Bug in RectMapCollider #294

Closed dangillet closed 7 years ago

dangillet commented 7 years ago

Hello,

Just noticed by pure coincidence that in the method detect_collision for both RectMapCollider and RectMapWithPropsCollider there is a small bug.

There is one condition

if last.bottom >= obj.top > obj.bottom:

which should read instead

if last.bottom >= obj.top > new.bottom:

I'm actually surprised that this never came up in the tests. Can I make the change and commit to master?

Dan

ccanepa commented 7 years ago

Hi Dan, yes, go ahead

On Sun, Apr 16, 2017 at 4:52 AM, Daniel Gillet notifications@github.com wrote:

Hello,

Just noticed by pure coincidence that in the method detect_collision for both RectMapCollider and RectMapWithPropsCollider there is a small bug.

There is one condition

if last.bottom >= obj.top > obj.bottom:

which should read instead

if last.bottom >= obj.top > new.bottom:

I'm actually surprised that this never came up in the tests. Can I make the change and commit to master?

Dan

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/los-cocos/cocos/issues/294, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWG7PYgZcsCwxsV1bL8DhXlgdj2ssKAks5rwcilgaJpZM4M-kV7 .

dangillet commented 7 years ago

Thanks.

Bug fixed with commit 4ee49037

mryellow commented 7 years ago

I'm actually surprised that this never came up in the tests.

To avoid regression probably need a test which randomly throws objects at a wall and sees if any of them make it past.

I started one, but building the tiles by hand is a pain.

from __future__ import division, print_function, unicode_literals

# This code is so you can run the samples without installing the package
import sys
import os
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
#

import cocos
from cocos.mapcolliders import RectMapCollider
from cocos.tiles import RectCell, RectMapLayer
from cocos.director import director

class TestLayer(cocos.layer.Layer, RectMapCollider):
    def __init__(self):
        super(TestLayer, self).__init__()

        self.width = 20
        self.height = 20
        self.tw = 10
        self.th = 10
        self.px_width = self.width * self.tw
        self.px_height = self.height * self.th

        self.schedule(self.update)
        self.spawn()

    def spawn(self):
        tile = # TODO
        for x in xrange(self.width):
            y = int(self.height/2)
            cells = [x][y] = RectCell(x, y, self.tw, self.th, {}, tile)
        print(cells)
        self.map_layer = RectMapLayer(self.tw, self.th, cells)
        self.map_layer.set_view(0, 0, self.map_layer.px_width, self.map_layer.px_height)

    def update(self, dt):
        #newVel.x, newVel.y = self.collide_map(self.map_layer, oldRect, newRect, newVel.x, newVel.y)
        #newPos = self.player.cshape.center
        #newPos.x, newPos.y = newRect.center
        #self.player.velocity = newVel
        #self.player.update_center(newPos)
        pass

def main():
    director.init()
    test_layer = TestLayer ()
    main_scene = cocos.scene.Scene (test_layer)
    director.run (main_scene)

if __name__ == '__main__':
    main()
dangillet commented 7 years ago

Hello,

Well there are some existing tests. These can be found in utest/test_RectMapCollider__no_stuck.py which relies on utest/aux_RectMapCollider__no_stuck.py. In the latter file you will find some cases where a character moves along a wall and tries to push through it.

The number of cases is quite extensive, but obviously it does not cover all possible cases.

mryellow commented 7 years ago

I've been training a DQN agent, E-Greedy exploration means it executes random actions. It was finding ways to slip past the first collision and hit the inside wall. Can't confirm but think it was only slipping past bottom edges.

dangillet commented 7 years ago

I would happily investigate any concrete examples you provide where the collision does not respond correctly.

mryellow commented 7 years ago

Had assumed this patched it but did observe the behaviour again recently. Arghh I've re-run with the same log folder and deleted a good example.

I was running the agent with a different reward function. The one which very quickly preferenced forward movement never seemed to display the issue, however when I changed to a reward function without this forward motion bonus the actions were a lot more erratic and the issue popped up again.

This shows a very short occurrence, where once in a corner and wriggling randomly it was able to go within a wall:

https://www.dropbox.com/s/ckjettmrw1u8iwv/openaigym.video.0.2307.video000000.mp4?dl=0

This is the environment:

https://github.com/mryellow/maze_explorer

I'll reconfigure the reward function and log every episode to see if I can reproduce it reliably.

mryellow commented 7 years ago

Some more examples.

With the speeding up and erratic behaviour there is potentially also a bug in my environment allowing velocities to make it from one episode into the next (although I do create a new instance each episode). Perhaps it's a matter of speed and the collision detection making it through the wall and into the empty space beyond.

https://www.dropbox.com/s/thncpfnzefnff8y/openaigym.video.0.4854.video000014.mp4?dl=0 https://www.dropbox.com/s/9m7baxu1g1c4y7k/openaigym.video.0.4854.video000016.mp4?dl=0 https://www.dropbox.com/s/xm216qklq70yz8k/openaigym.video.0.4854.video000018.mp4?dl=0 https://www.dropbox.com/s/lndzam08dsobowm/openaigym.video.0.4854.video000019.mp4?dl=0 https://www.dropbox.com/s/v277i4kjyu30npx/openaigym.video.0.4854.video000021.mp4?dl=0 https://www.dropbox.com/s/3obr60njxegaots/openaigym.video.0.4854.video000024.mp4?dl=0 https://www.dropbox.com/s/ofro4wbkk4fq71y/openaigym.video.0.4854.video000027.mp4?dl=0 https://www.dropbox.com/s/kal43qfhmii93ee/openaigym.video.0.4854.video000028.mp4?dl=0

mryellow commented 7 years ago

Is it simply not recursive and re-checking for collisions after the initial velocity restriction... Allowing it to slide into a wall. Would appear not on the surface of things but perhaps there is an edge case where it can ignore the wall it's sliding towards.

mryellow commented 7 years ago

Perhaps it's a matter of speed and the collision detection making it through the wall and into the empty space beyond.

This is a real possibility. With the right reward function the agent was learning to exploit a bug in environment code which allowed speed to increase above max. (Speed was only limited on forward rather than more globally).

https://github.com/mryellow/maze_explorer/blob/b3ca0de27259a66ac370f347f8fc5934d59b60ca/mazeexp/engine/player.py#L114-L116

dangillet commented 7 years ago

Indeed the current implementation does not check for tunnelling effects. So if your agent moves from one frame to the next frame with a velocity which would make it go through the wall, then you will see this behaviour.

If you need a more robust physics engine, I would suggest you check Box2d or Chipmunks. Cocos2D only provides basic collision detections and resolutions.

mryellow commented 7 years ago

All sorts of bug in my velocity code, running some more agents with better handling of all that, will let you know if they work out any holes.

On 19/08/2017 10:30 PM, "Daniel Gillet" notifications@github.com wrote:

Indeed the current implementation does not check for tunnelling effects. So if your agent moves from one frame to the next frame with a velocity which would make it go through the wall, then you will see this behaviour.

If you need a more robust physics engine, I would suggest you check Box2d or Chipmunks. Cocos2D only provides basic collision detections and resolutions.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/los-cocos/cocos/issues/294#issuecomment-323520357, or mute the thread https://github.com/notifications/unsubscribe-auth/AANCRzTwWXSLkiuzU-q2trUrYyqQYtCGks5sZtVvgaJpZM4M-kV7 .