schteppe / p2.js

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

Don't allow body removal during step() #212

Closed schteppe closed 8 years ago

schteppe commented 8 years ago

From previous discussions (https://github.com/schteppe/p2.js/pull/207, https://github.com/schteppe/p2.js/pull/208), I've realized that it's too complicated to try to "fix" the problem of removing bodies during step(). Logically, one would need to fix adding of bodies as well, and applying the same solution for constraints as well. And all other things that can be changed in the World which aren't really compatible with step().

This is simply too much work and can simply be avoided by not doing those things during step() - just postpone it. Read more here: http://www.iforce2d.net/b2dtut/removing-bodies

I suggest throwing an error in the following methods, if they are used during the step.

(just keeping this issue here to remind myself to implement this, and to get potential feedback)

jtenner commented 8 years ago

Please do now "throw" inside the function. This will cause a v8 deoptimization and really hinder performance.

Do this instead:

function privateFunctionThrower(msg) {
   throw new Error(msg);
}
//later in the code ...
if (errorState) {
   privateFunctionThrower('Message');
}

V8 doesn't like compiling functions that throw.

This is also a very good idea.

schteppe commented 8 years ago

Thanks!

I thought only try/catch statements were unoptimized in v8, not actually the throwing code? See https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#2-unsupported-syntax

If "code that might throw" was unoptimized, then all JS code would be unoptimized (for example, TypeError). Maybe I'm wrong.

jtenner commented 8 years ago

Why am I so quick when I'm wrong? Hah.

I remembered that try-catch statements were bad. My mistake.

Throw as you like.

schteppe commented 8 years ago

I don't blame you, I do exactly the same thing sometimes too.

Gonna throw my errors in the air, like I just don't care.