schteppe / p2.js

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

Fix for issue 193 #194

Closed psalaets closed 8 years ago

psalaets commented 8 years ago

Changed so when a body is removed from a World any related "disabled body collision" pairs are also removed.

Fixes #193

psalaets commented 8 years ago

With this change, there are at least 3 places (internalStep(), enableBodyCollision(), removeBody()) in the World code that have to know disabledBodyCollisionPairs is a flat array, where every 2 elements make up a pair.

Is the flat array used for performance reasons?

Do you think it's worth changing disabledBodyCollisionPairs to be an array of "BodyPairs" where a BodyPair looks like

function BodyPair(bodyA, bodyB) {
  this.bodyA = bodyA;
  this.bodyB = bodyB;
}

BodyPair.prototype = {
  involves: function(body) {
    return this.bodyA === body || this.bodyB === body;
  }
};
schteppe commented 8 years ago

Awesome, thanks! Just a minor thing; the body ID should be an integer, so it can be used in integer ops like in the TupleDictionary https://github.com/schteppe/p2.js/blob/master/src/utils/TupleDictionary.js

The reason for the flat pair arrays is performance. Filling and emptying a flat array don't need object allocation or garbage collection. One could use a class solution, like yours, together with an object pool to solve the GC problem. But my gut feeling is that it wouldn't increase performance... The API would be really nice on the other hand.

psalaets commented 8 years ago

Thanks for the heads up.

I updated the test so body ids are integers and renamed bodies to match their id.

schteppe commented 8 years ago

Ok, thanks a bunch!