Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Hitting crash in A-Frame Physics System (maybe when switching between kinematic & dynamic?) #197

Open
diarmidmackenzie opened this issue Dec 11, 2021 · 4 comments

Comments

@diarmidmackenzie
Copy link
Collaborator

diarmidmackenzie commented Dec 11, 2021

Using Ammo.js in my app here:
https://diarmidmackenzie.github.io/christmas-scene/research/physics-ammo/full-test.html

Sometimes when using this, I hit the following crash (note, these are separate instances, 17 seconds apart).

image

No clear idea what the repro scenario is. Unpredictable when I hit this, but usually after a few minutes.

aframe-physics-system.js:16758 Uncaught TypeError: Cannot read properties of null (reading 'added')
at i.AmmoBody._updateShapes (aframe-physics-system.js:16758)
at i.beforeStep (aframe-physics-system.js:16915)
at o.tick (aframe-physics-system.js:19019)
at HTMLElement.value (a-scene.js:746)
at HTMLElement.value (a-scene.js:790)
at bind.js:12
at u (three.js:19157)
at t (three.js:10460)

The code we are hitting a bug in is here:

for (let j = 0; j < collisionShapes.length; j++) {
            const collisionShape = collisionShapes[j];
            if (!collisionShape.added) {
              this.compoundShape.addChildShape(collisionShape.localTransform, collisionShape);
              collisionShape.added = true;
            }
          }

Why would we be calling "_updateShapes" mid-simulation?

My guess is it is related to the fact that I frequently switch objects between kinetic & dynamic, as described here:
https://diarmidmackenzie.github.io/christmas-scene/research/README.md

("phase 2" is the bit that explains switching between kinetic & dynamic).

Can't see any other reason why shapes would be updated, as I am not modifying object geometry, or creating new objects.

So my guess is there is some sort of race condition when switching an object between dynamic & kinetic, where collision data is ot in the state that this function assumes.

I can imagine it being difficult to understand the exact repro scenario, but might a defensive fix be in order?

for (let j = 0; j < collisionShapes.length; j++) {
            const collisionShape = collisionShapes[j];
            if (collisionShape && 
                !collisionShape.added) {
              this.compoundShape.addChildShape(collisionShape.localTransform, collisionShape);
              collisionShape.added = true;
            }
          }

I'm going to try making this fix to a forked copy, and would be happy to submit a PR if it's wanted.

Any insights as to what might be a better fix would be appreciated.

@diarmidmackenzie
Copy link
Collaborator Author

I have come across a simple reliable desktop repro for this problem.

Not yet looked into what the actual cause is, but I suspect it's related to changing body type after initialization.

https://diarmidmackenzie.github.io/christmas-scene/research/physics-ammo/repro-for-issue-197.html

@dirkk0
Copy link

dirkk0 commented Dec 12, 2021

The link doesn't work for me.

@diarmidmackenzie
Copy link
Collaborator Author

Which link doesn't work?

In what way does it not work?

@dirkk0
Copy link

dirkk0 commented Dec 12, 2021

The link in your latest comment gave a 404 for me.
Now it works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants