search

schteppe / p2.js

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

ContactEquation can point to temp-shape that has body always set to null #308

Open ghost opened 7 years ago

ghost commented 7 years ago

In narrowphase you have all functions to test collision between shapes in bodies in world.

Function to test collision between convex and capsule has a bug due to temporary shape object that is used to test collision between convex and middle box of capsule

var r = convexCapsule_tempRect;
setConvexToCapsuleShapeMiddle(r,capsuleShape);

var result = this.convexConvex(convexBody,convexShape,convexPosition,convexAngle, capsuleBody,r,capsulePosition,capsuleAngle, justTest);

Inner convexConvex-call produces contactEquations for outer-convexCapsule call, but it do it with fake-shape (r) and assign that shape to that contactEquations.

This is serious bug, because that fake-shape for example hasn't any body (shape.body), it's just null.

Any idea for quick fix?

EDIT:

I've made hot fix for that :D works great. Don't know how to pull request only for one commit, so maybe someone can help?

https://github.com/schteppe/p2.js/commit/6d213c3422fd4300eb96ede23b4c2449d4fa5703?diff=unified

It's very important fixed bug I think. ContactEquations should never point to temporary shape variable that besides (or most important) has body set to null!

schteppe commented 6 years ago

Nice find! In Cannon.js I made a somewhat clean solution for this: passing two "override shape" arguments to createContactEquation() and each narrowphase method. This way you can pass the shapes used for collision as well as the one you want to set on the contact. See the solution here: https://github.com/schteppe/cannon.js/blob/569730f94a1d9da47967a24fad0323ef7d5b4119/src/world/Narrowphase.js#L63 Example in one of the collision methods: https://github.com/schteppe/cannon.js/blob/569730f94a1d9da47967a24fad0323ef7d5b4119/src/world/Narrowphase.js#L330

Now that I think of it, maybe the createContactEquation() there is wrong. It uses the override shapes correctly when creating the ContactEquation, but it seems like the material is still used from the shapes used for collision... I think it should use the override shape for everything.