schteppe / p2.js

JavaScript 2D physics library
Other
2.64k stars 330 forks source link

nj.neighbors.push(ni); #207

Closed Weedshaker closed 8 years ago

Weedshaker commented 8 years ago

Hi,

I get the following error: "Uncaught TypeError: Cannot read property 'neighbors' of undefined" when I remove a body, which has a constraint. Error is at: src/world/IslandManager.js Line: 168

I am using phaser with p2.js and set the body to "exists = false". Hope this could be fixed at your side, since it can be tiring to remove all constraints before setting the above value.

Anyway, THANKS a lot!

schteppe commented 8 years ago

I guess this boils down to the question if the following should be allowed, when using the p2 API:

var c = new p2.Constraint(bodyA, bodyB);
world.addConstraint(c);
world.removeBody(bodyA);

It feels a bit dangerous to have an implicit removal of the constraint there, no? I'd rather have a crash and bang than a silent and sneaky bug in my game.

However it looks like box2d implements this dangerous removal. http://www.box2d.org/manual.html#_Toc258082974

However, it looks like Bullet assumes the user removes the constraints from the world first. http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?f=9&t=1314

Shouldn't Phaser take care of the constraint removal?

Weedshaker commented 8 years ago

Thanks for your quick answer!

I guess you're right, although having Phaser loop once more through the worlds constraints when removing a body from world would just be extra overhead, while p2.js already loops through it. Maybe it would be more efficient solving this at your side. But properly it should be fixed at Phasers side, since this is rather in their scope. They may could reference the constraints to the body instead of looping through all constraints in the world. You can close this issue and by occasion let me open a Pull request at Phaser, if you want. Although, in the meantime I do solve it within my code being tidy!

Cheers

schteppe commented 8 years ago

The complexity would be the same, no matter who runs the loop. But its complexity can in any case be disregarded, because I don't see any case where you would remove a ton of bodies every frame.

Let's leave the issue open for a while. I'll keep it as a reminder to add more docs about the subject, and to implement one of the two solutions (Bullet/box2d).

Weedshaker commented 8 years ago

cool!

Have a nice day

schteppe commented 8 years ago

Settled for the Bullet solution: assuming removal of constraints from the world before bodies. You should now get a little more helpful error message instead of the TypeError.