[Examples/Vehicle] Fix Ammo null function error on restart #6902
Conversation
|
I wanted to keep track of every object and then it turned out that it's not even possible to destroy So I just deleted the |
Hmm, I might be not correct here, but I don't think Emscripten actually destroys it. WebIDL "nodelete" keyword means that this class cannot be deleted, since it has no public destructor (e.g. it is private). I think ems simply suppresses the error, when you try to destroy it. Similar to how you won't get an error, if you try to destroy an object a second time: const vec = new Ammo.btVector3(0, 0, 0);
Ammo.destroy(vec);
Ammo.destroy(vec); // no error |
Using arrow function Co-authored-by: Will Eastcott <[email protected]>
|
@LeXXik Yea, that description about no-destructor I read aswell and it makes absolutely no sense to me. You don't need a destructor to free memory, it just feels like an arbitrary and even harmful limitation. Historically classes were basically "structs with functions" and in order to have a struct, you have to allocate its memory... and then obviously free that again for good housekeeping - which that keyword makes impossible. We can discuss this out further in a new issue/PR, in this one I'm just happy to have the actual bug fixed. |
So folks are happy for me to go ahead and merge this now? |
Yes, this is a good PR, regardless of the bug. |
|
@willeastcott suggesting this for v1 |
* Just example simplification (forEach -> map) * Don't call `Ammo.destroy` on a internal transform pointer * Clean up temporary Ammo objects on restart * Update examples/src/examples/physics/vehicle.example.mjs Using arrow function Co-authored-by: Will Eastcott <[email protected]> --------- Co-authored-by: Will Eastcott <[email protected]>
|
@LeXXik Done. I've just cherry picked it to |
Fixes #6724
These are just my current findings, the engine still seems to leak rigidbody Ammo objects/pointers... but at least this makes an Example refresh work in the first place. This allows for further fixes without just crashing like it currently does here: https://playcanvas.vercel.app/#/physics/vehicle
About 7b0c745 / Removing
Ammo.destroy(origin);:The origin is part of
btTransform:https://github.com/kripken/ammo.js/blob/1ed8b58c7058a5f697f2642ceef8ee20fdd55e10/bullet/src/LinearMath/btTransform.h#L32-L41
You don't just get that pointer and call
freeon it - at least it doesn't make sense to me. Probably at best nothing happens and worst case is messing up internal allocation structures while causing other subtle bugs. Any other opinions on this?Feedback about all this highly welcome and some testing.
I can't even logically explain why the
null function erroroccurs, since why should it matter if we reuse some old allocated pointers or free them and recreate those anew - it feels like a black box. 🙈Vercel quick link: https://engine-r0b3y912i-playcanvas.vercel.app/#/physics/vehicle
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.