fix(common): apply parent rotation and scale to child position offsets - #587
Merged
Conversation
composeWithParent inherited rotation and scale correctly but composed a child's world position by plain addition, so a child's local offset was never rotated or scaled by the parent's world transform - a child of a rotating parent spun in place instead of orbiting it. Rotate and scale the local offset by the parent's world rotation/scale before adding it to the parent's world position. Also deletes parent-position-system.ts, parent-rotation-system.ts, and parent-scale-system.ts, which carried the same additive-position bug and were superseded by createTransformEcsSystem but never removed - they were not exported from src/common/systems/index.ts and referenced only by their own test files. Fixes #581 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42FeAcwfsHYR3btuySY5U
stormmuller
enabled auto-merge (squash)
August 6, 2026 21:58
Member
Author
|
CodeQL analysis passed. However, the Generated by Claude Code |
Push/pull_request webhook events were dropped during a GitHub Actions incident (stuck ARC runner pods, resolved 2026-08-07 02:04 UTC), so CI never ran for this PR. Empty commit to re-fire the trigger per GitHub's incident notice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42FeAcwfsHYR3btuySY5U
"Behaviour" isn't in the project's dictionary; the repo consistently
uses American spelling ("behavior") elsewhere.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M42FeAcwfsHYR3btuySY5U
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…stem The refactor split into composePositionWithParent/composeRotationWithParent/ composeScaleWithParent added branches for a component present on an entity but absent on its parent (or vice versa) that weren't exercised by the existing rotated/scaled/chain tests, dropping patch coverage below the project's Codecov target. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42FeAcwfsHYR3btuySY5U
stormmuller
pushed a commit
that referenced
this pull request
Aug 7, 2026
…ng on dev Merges dev, which brings in #587 (the parent-transform fix filed as #581) and #588 (the AGENTS.md input capability correction filed as #582). Both were surfaced by the review of this document, and both are now fixed, so the document's claims about them were stale. The transform gap row and its explanatory note move to past tense. #587 landed exactly the composition the note proposed - scale the child's local offset by the parent's world scale, rotate it by the parent's world rotation, then add - and deleted the three superseded parent-*-system.ts files along with it. The note is retained rather than removed, since the design's assumptions were formed against the broken behavior and it is worth knowing they no longer are. DL-03's "does this survive the fix" subsection moves to past tense too, and now records that option (b) is no longer broken while still losing on the load-bearing arguments. Open question 2 loses its blocker: world-space canvas mode depended on the transform fix, since a health bar parented to a rotating ship was exactly the broken case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WmVKWzRGU5FPVEwAa2oqVw
9 tasks
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
2 participants
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.
Summary
composeWithParentinsrc/common/systems/transform-system.tsinherited a parent's world rotation and scale correctly, but composed a child's world position by plain addition of the parent's world position. The child's local offset was never rotated or scaled by the parent's world transform, so a child of a rotating parent spun in place instead of orbiting it (and a child of a scaled parent kept an un-scaled offset).composeWithParentintocomposePositionWithParent,composeRotationWithParent, andcomposeScaleWithParent(also fixes a SonarJS cognitive-complexity lint failure the fix would otherwise trigger).composePositionWithParentnow rotates and scales the child's local offset by the parent's world rotation/scale before adding it to the parent's world position:world.position = parent.world.position + rotate(local.position * parent.world.scale, parent.world.rotation)parent-position-system.ts,parent-rotation-system.ts, andparent-scale-system.ts(and their tests) - these carried the same additive-position bug, were superseded bycreateTransformEcsSystem, and were not exported fromsrc/common/systems/index.tsor referenced anywhere outside their own tests.No demo under
/documentation-site/src/pages/demosusesaddParentComponent/ParentEcsComponent, so none are affected by this behavior change.Behaviour change: any existing content that parents a positioned entity to a rotated or scaled one, authored against the previous (incorrect) additive composition, will now render differently (correctly). Flagged in the changelog per the issue's suggestion.
Related issue(s)
Closes #581
Verification checklist
npm run check-typespasses with 0 errorsnpm testpassesnpm run lintpasses with 0 errorsnpm run cspellpasses with 0 errorsnpm run check-exportspassesindex.ts(and/src/index.ts/package.jsonexportsif it's a new module) — n/a, no public API changed/documentation-site/docs/docsis updated if this change affects documented behavior — n/a, no doc describes this composition behavior/documentation-site/src/pages/demos, the demo has been updated and verified — n/a, no demo uses entity parentingChangelog
## [Unreleased]inCHANGELOG.mdGenerated by Claude Code