Bodies still collide after their removal

[RobertHerhold]

This was originally reported in #206, but the provided code was said to be unable to be reproduced. I have encountered this again, and have created a Pen to demonstrate it here. To reproduce, simply press the up arrow key and land the block on one of the moving rectangles. You should see that the body on bottom will be removed, but it will still report a collision (see the console logs). In certain cases, I have gotten it to report around 80 collisions after the removal of the body while the block is still suspended in the air.

[iamjoshua]

I just ran into this issue. It will be a major blocker in my current project. Any hope of a fix soon? If not, I'll probably dig in later in the week and see if I can help.

[liabru]

Super strange. Also, why do the obstacles slow down? As if it is frozen, but it's not?

[RobertHerhold]

It does seem to freeze for around 80 ticks before it catches up... I'm not sure what causes it to check that entity is actually there after that time

[plurry]

Another demo:

var e = Matter.Engine.create(document.body);

var a = Matter.Bodies.rectangle(400, 400, 100, 60);
var b = Matter.Bodies.rectangle(450, 100, 100, 60);
var c = Matter.Bodies.rectangle(400, 610, 810, 60, {isStatic: true});

Matter.Events.on(e, 'collisionStart', _ => {
    _.pairs.forEach(_ => {
        if(_.bodyA === a || _.bodyB === a)
            Matter.World.remove(e.world, a);
    });
});

Matter.World.add(e.world, [a, b, c]);
Matter.Engine.run(e);

b bounces off a even though a isn't visible and should have been removed. This didn't happen until 0.9.0.

But, even in 0.8.0, if you remove b instead of a, it will still cause a to bounce. Shouldn't I be able to use collisionStart to remove a body before collisions have been resolved?

[liabru]

Thanks for the additional test case @plurry. This should now be fixed in the edge build and should be in a release soon. Tell me if it works for you guys.

[plurry]

Cool, bodies now get removed like they should. So yes, the original bug is fixed for me.

But, a body that's not removed until collisionStart still knocks back the body it collided with. For what I needed I handled the collision myself outside of Matter, but it would be nice to have an event like Box2D's PreSolve.

[iamjoshua]

I've created a demo where the issue still exists:
https://jsfiddle.net/460sc0ff/3/

[liabru]

Thanks for the new example @iamjoshua. I'm having a bit of trouble with this one as when I add your code into my dev build the issue does not seem to occur. I checked that you're using the correct version of matter.js in your demo and it seems to be up to date... weird.

[iamjoshua]

Very weird. I've tried several variations and have a new clue for you:

https://jsfiddle.net/bkr16m07/4/

Removing a body on load, as in some of the previous demos, is no longer an issue with the edge build. However, removing a body after any delay, results in the bug. So my guess would be there is some reference array —perhaps in the rederer?— not getting updated.

[iamjoshua]

Alright, I've been digging around a bit and I'm still getting familiar with Matter so forgive me if I'm way off.

It looks like within Engine.update you're taking Composite.allBodies as the source of bodies for collision detection which is why this is not an issue if the removal of a body occurs immediately. For subsequent updates, broadphase.pairsList is used:

matter-js/src/core/Engine.js

Line 148 in 7eb9ac7

if (broadphase.controller) {

However, from what I can tell so far, the pairs list does not get updated upon body removal:
https://github.com/liabru/matter-js/blob/master/src/collision/Grid.js#L138

The pairsList is only updated if the grid has changed, but to determine if the grid has changed, you're taking the current list of bodies which does not include the removed bodies.

[iamjoshua]

It definitely seems to be a reference problem.

If you update the same demo above (https://jsfiddle.net/bkr16m07/4/) as follows, the issue is resolved:

//Matter.World.remove(world, prey);
Matter.World.remove(engine.world, prey);

[iamjoshua]

The absolute heart of the issue is within Common.extend. Some function reference isn't getting merged when extending. I'm afraid to break things so I'll stop digging here. An easy fix is to just have Engine use the world provided instead of creating a new one:

https://github.com/liabru/matter-js/blob/master/src/core/Engine.js#L75

like so:

// use the provided world or create a new one
engine.world = options.world || World.create(engine.world);

I'd submit a pull request but I'm guessing you'll want to resolve the reference issue itself some how.

Anyways, I hope that helps.

[liabru]

Ah, good work! I guess I've never passed a world before, I didn't spot that in your example either.
Extend will be failing due to reference conflicts as it tries to do a deep copy.
Your solution is exactly what is done else where for option objects that need to be by reference anyway.

So can you please submit a pull request with it (the comment isn't necessary though)?
Thanks!

[liabru]

This is now in the latest release, thanks guys!

[ManfredHu]

ccb version still has this problem.
I set the collisionFilter.category before remove the body to solve this problem