Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compound colliders #6979

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Fix compound colliders #6979

merged 3 commits into from
Sep 26, 2024

Conversation

LeXXik
Copy link
Contributor

@LeXXik LeXXik commented Sep 20, 2024

Fixes #6976

The issue was that when we loop over compounds during physics update, we go over all nodes of the compound entity:

entity.forEach(this.system.implementations.compound._updateEachDescendantTransform, entity);

Since we use entity.forEach, that includes the parent itself, which adds parent compound shape as a child to itself. When 2 such compounds collide, the bullet physics locks in a loop, causing max stack exception.

While at it, also changed the frequency of the children shapes transform updates. Instead of removing and adding them every frame if parent moved, they are now updated only when their own transforms changed or they are recreated.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott
Copy link
Contributor

Hey @Maksims, this looks ok to me. But I think you wrote the original support for compound collision shapes, right? Would you be able to cast your eye over this too please?

@Maksims
Copy link
Contributor

Maksims commented Sep 26, 2024

I think it was caused by my assumption that forEach would not include self into the loop. It has been 5 years since.
If tested and works well, good fix!

@willeastcott willeastcott merged commit e5bf252 into playcanvas:main Sep 26, 2024
8 checks passed
@LeXXik LeXXik deleted the lxk-compounds branch September 26, 2024 12:41
@LeXXik
Copy link
Contributor Author

LeXXik commented Sep 26, 2024

@willeastcott this should be good for v1 as well.

willeastcott pushed a commit that referenced this pull request Sep 26, 2024
* fix compound colliders

* minor
@willeastcott
Copy link
Contributor

@LeXXik I have just cherry-picked to main_v1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: physics Physics related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Uncaught RangeError: Maximum call stack size exceeded
4 participants