Wake the constrained bodies when a joint constraint is created or destroyed - #9177
Merged
Merged
Conversation
…troyed `_activateBodies` was only called from `_updateLimits`, `_updateMotor` and `_updateSpring`, so adjusting a joint's limits, motor or spring took effect immediately on a sleeping simulation island, but creating or destroying the constraint itself did not. Every structural change goes through `_createConstraint`/`_destroyConstraint`, and neither woke anything, so a change that should visibly alter the simulation silently did nothing until something else disturbed the bodies. Destroying a constraint was the worst case: the bodies kept the pose the constraint had been holding them in, so a released assembly hung in mid-air indefinitely. Call `_activateBodies` from both, which covers every path that creates or tears down a constraint: `type`, `enableCollision`, `refreshFrames`, `entityA`/`entityB` assignment, a body leaving the simulation, exceeding `breakImpulse`, `onDisable` and `onBeforeRemove`. The destroy-side wake sits inside the `if (joint)` guard so nothing is activated when there was no constraint to tear down. Re-arming a broken joint with `refreshFrames` is still covered, since a broken joint has no constraint to destroy and the subsequent creation does the waking. Ordering works out on every teardown path: `_setJointEntity` destroys before reassigning the backing field and `onBeforeRemove` never clears the entity references, so both still have the outgoing bodies in hand. The exception is the entity `destroy` handler, which nulls its own reference first - that body is being destroyed so waking it is moot, and the surviving body is still woken. Tests run against `NullPhysicsWorld`, which takes joints through their full lifecycle without Ammo, making constraint creation and teardown unit-testable where it previously was not. Fixes #9176 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Build size reportThis PR changes the size of the minified bundles.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures joint constraint lifecycle changes (creation/teardown) immediately affect sleeping physics islands by waking the constrained bodies whenever a constraint is created or destroyed, matching the behavior already present for limit/motor/spring updates.
Changes:
- Call
JointComponent._activateBodies()after creating a constraint and after destroying an existing constraint. - Expand
_activateBodies()docstring to clarify that structural constraint changes also wake bodies. - Add unit tests (using
NullPhysicsWorld) that assert body activation on constraint create/destroy paths and verify the no-constraint teardown guard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/framework/components/joint/component.js | Wakes constrained bodies on joint constraint creation and destruction to ensure immediate effect on sleeping islands. |
| test/framework/components/joint/component.test.mjs | Adds targeted activation-count tests for constraint lifecycle events using NullPhysicsWorld. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
"Creating and destroying the constraint wake the bodies too" put a plural verb straight after a singular noun, so it read as a garden path, and "too" pointed at a set of callers the preceding sentence never named. Make the method itself the subject and state its call sites explicitly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
JointComponent._activateBodieswas only called from_updateLimits,_updateMotorand_updateSpring. Adjusting a joint's limits, motor or spring therefore took effect immediately on a settled simulation island, but creating or destroying the constraint itself did not — and every structural change goes through_createConstraint/_destroyConstraint, neither of which woke anything.The result was that a change which should visibly alter the simulation silently did nothing until something else disturbed the bodies. Destroying a constraint was the worst case: the bodies kept the pose the constraint had been holding them in, so a released assembly hung in mid-air indefinitely.
This calls
_activateBodiesfrom both, covering every path that creates or tears down a constraint:type,enableCollision,refreshFrames,entityA/entityBassignment, a body leaving the simulation, exceedingbreakImpulse,onDisableandonBeforeRemove.Fixes #9176
Notes for reviewers
The destroy-side wake is inside the
if (joint)guard, so nothing is activated when there was no constraint to tear down. Re-arming a broken joint viarefreshFramesis still covered — a broken joint has no constraint to destroy, and the subsequent creation does the waking.Ordering works out on every teardown path.
_setJointEntitydestroys the constraint before reassigning the backing field, andonBeforeRemovenever clears the entity references, so both still have the outgoing bodies in hand. The one exception is the entitydestroyhandler, which nulls its own reference first — that body is being destroyed so waking it is moot, and the surviving body is still woken (in practice the rigid body'sbeforeremove→_onBodyLosthas already woken both by then).Tests add a
body activationblock covering creation,enabled = false, component removal, clearingentityA(the ordering case),refreshFrames(2 activations — teardown then rebuild), plus one asserting no wake when there is no constraint. These run againstNullPhysicsWorld, which takes joints through their full lifecycle without Ammo, so constraint creation and teardown are now unit-testable where they previously were not.All 5 activation tests fail on the unpatched source and pass with the fix; the no-constraint guard test passes either way. 86 tests pass across
joint/rigid-body/collision/physics, and ESLint is clean.Left out of scope
The issue also notes that
RigidBodyComponent#teleportdoes not activate either. That gap is real (teleport→syncEntityToBody→setTransformwith noactivate), but unlike the alpha joint componentteleportis a long-standing public API, and "reposition a settled prop and have it stay settled" is plausibly relied upon today. That seems worth its own issue and decision rather than folding it in here.Checklist
🤖 Generated with Claude Code