Summary
composeWithParent in src/common/systems/transform-system.ts inherits rotation and scale correctly, but composes a child's world position by plain addition. The child's local offset is never rotated or scaled by the parent's world transform, so a child of a rotating parent spins in place instead of orbiting it.
Raised by @stormmuller while reviewing #580.
Current behaviour
src/common/systems/transform-system.ts:
world.rotation = parent.world.rotation + local.rotation // correct
world.scale = parent.world.scale * local.scale // correct
world.position = parent.world.position + local.position // missing two terms
A correct 2D TRS composition transforms the child's local offset by the parent before adding it:
world.position = parent.world.position
+ rotate(local.position * parent.world.scale, parent.world.rotation)
Reproduction
Parent at the origin with rotation.local = Math.PI / 2, child parented to it with position.local = { x: 10, y: 0 }:
|
Expected |
Actual |
child position.world |
(0, 10) |
(10, 0) |
child rotation.world |
π/2 |
π/2 ✓ |
The child's rotation is right, so the sprite visibly spins — it just doesn't orbit. The scale case fails the same way: parent scale.local = (2, 2) and child position.local = (10, 0) gives (10, 0) where it should give (20, 0).
Symptom in a real scene: a turret parented to a rotating tank rotates correctly but stays glued at an un-rotated offset, so it detaches from its mount as the tank turns.
Why it wasn't caught
transform-system.test.ts has seven tests and none of them attach a RotationEcsComponent or ScaleEcsComponent to a parent. Nothing currently asserts this behaviour in either direction.
Related dead code
parent-position-system.ts, parent-rotation-system.ts, and parent-scale-system.ts (all in src/common/systems/) carry the same additive position composition. They are not exported from src/common/systems/index.ts and are referenced only by their own test files — they appear to be superseded by createTransformEcsSystem. Worth deleting alongside the fix rather than fixing in three places, unless they're being kept deliberately.
Suggested scope
- Fix
composeWithParent to apply the parent's world rotation and scale to the child's local offset.
- Add tests covering: rotated parent, scaled parent, rotated + scaled parent, and a three-deep chain.
- Decide whether to delete the three superseded
parent-*-system.ts files and their tests.
Note this is a behaviour change for any existing content that parents a positioned entity to a rotated or scaled one, and that has been authored against the current (incorrect) composition. Given the engine is pre-1.0, correcting it seems right, but it likely wants a changelog entry flagging the visual change.
Summary
composeWithParentinsrc/common/systems/transform-system.tsinherits rotation and scale correctly, but composes a child's world position by plain addition. The child's local offset is never rotated or scaled by the parent's world transform, so a child of a rotating parent spins in place instead of orbiting it.Raised by @stormmuller while reviewing #580.
Current behaviour
src/common/systems/transform-system.ts:A correct 2D TRS composition transforms the child's local offset by the parent before adding it:
Reproduction
Parent at the origin with
rotation.local = Math.PI / 2, child parented to it withposition.local = { x: 10, y: 0 }:position.world(0, 10)(10, 0)rotation.worldπ/2π/2✓The child's rotation is right, so the sprite visibly spins — it just doesn't orbit. The scale case fails the same way: parent
scale.local = (2, 2)and childposition.local = (10, 0)gives(10, 0)where it should give(20, 0).Symptom in a real scene: a turret parented to a rotating tank rotates correctly but stays glued at an un-rotated offset, so it detaches from its mount as the tank turns.
Why it wasn't caught
transform-system.test.tshas seven tests and none of them attach aRotationEcsComponentorScaleEcsComponentto a parent. Nothing currently asserts this behaviour in either direction.Related dead code
parent-position-system.ts,parent-rotation-system.ts, andparent-scale-system.ts(all insrc/common/systems/) carry the same additive position composition. They are not exported fromsrc/common/systems/index.tsand are referenced only by their own test files — they appear to be superseded bycreateTransformEcsSystem. Worth deleting alongside the fix rather than fixing in three places, unless they're being kept deliberately.Suggested scope
composeWithParentto apply the parent's world rotation and scale to the child's local offset.parent-*-system.tsfiles and their tests.Note this is a behaviour change for any existing content that parents a positioned entity to a rotated or scaled one, and that has been authored against the current (incorrect) composition. Given the engine is pre-1.0, correcting it seems right, but it likely wants a changelog entry flagging the visual change.