diff --git a/docs/superpowers/specs/2026-05-02-l3-remote-entity-motion-design.md b/docs/superpowers/specs/2026-05-02-l3-remote-entity-motion-design.md index 29051cd..21ce779 100644 --- a/docs/superpowers/specs/2026-05-02-l3-remote-entity-motion-design.md +++ b/docs/superpowers/specs/2026-05-02-l3-remote-entity-motion-design.md @@ -90,17 +90,39 @@ addresses — not guesses): **Phase L.3 — Remote Entity Motion Conformance.** Slots into the L = movement category alongside L.1 (animation) and L.2 (collision). -Three sub-lanes, each independently shippable + visually verifiable: +**Scope revision 2026-05-02 (after Task 7 visual verification):** +L.3.1 was originally scoped as "InterpolationManager only", with L.3.2 +("PositionManager") deferred. Visual verification proved L.3.1 alone +**cannot produce smooth motion** — retail combines animation root motion ++ InterpolationManager corrections, and only the second half ships in +L.3.1-as-originally-scoped. The two halves are functionally inseparable. + +**L.3.1 and L.3.2 are now combined into a single sub-lane** ("L.3.1+L.3.2 +combined"). L.3.3 remains a separate sub-lane. | Sub-lane | Title | Ships | |---|---|---| -| **L.3.1** | InterpolationManager core + routing | New `InterpolationManager` class, `MoveOrTeleport` routing replacing the hard-snap in `OnLivePositionUpdated`, `VectorUpdate.Omega` application, deletion of `RemoteMotion` soft-snap residual | -| **L.3.2** | PositionManager (root-motion + interpolation-offset combiner) | New `PositionManager` class that combines per-frame animation root-motion offset with the InterpolationManager's catch-up offset before writing the body's frame | +| **L.3.1 + L.3.2 combined** | InterpolationManager + PositionManager + retail-faithful jump | (1) `InterpolationManager` (FIFO queue + AdjustOffset), (2) `MotionInterpreter.GetMaxSpeed`, (3) `PositionManager` class combining animation root motion + Interp correction per frame, (4) `IsGrounded` plumbed through `EntityPositionUpdate`, (5) `OnLivePositionUpdated` retail-faithful routing (airborne no-op + landing transition + grounded routing), (6) per-frame `TickAnimations` calls `PositionManager.ComputeOffset` + `UpdatePhysicsInternal`, (7) `VectorUpdate.Omega` application | | **L.3.3** | MoveToManager (server-controlled creature MoveTo) | Replaces `RemoteMoveToDriver` MVP with a faithful port: retracking, sticky-to-target, fail-distance progress checks, sphere-cylinder distance variants | -L.3.2 and L.3.3 get their own brainstorm + spec when L.3.1 lands. -**This document specifies L.3.1 in detail; L.3.2 and L.3.3 are -sketches** (above) so the phase shape is on record. +L.3.3 gets its own brainstorm + spec when L.3.1+L.3.2 ships. +**This document specifies L.3.1+L.3.2 in detail; L.3.3 is a sketch** +(above) so the phase shape is on record. + +### What changed since original spec + +- **L.3.2 PositionManager** is now part of L.3.1, not a separate phase. +- **`IsGrounded` plumbing** added — verified to already exist as + `PositionFlags.IsGrounded = 0x04` in `UpdatePosition.cs:48`, parsed but + not exposed through `EntityPositionUpdate`. Now exposed. +- **Jump pipeline** rewritten to match retail's `MoveOrTeleport` + has_contact=false → no-op semantics. Local arc prediction (the source + of the "endless jump" bug) eliminated. Server is authoritative; landing + detected via the first `IsGrounded=true` UP after airborne. +- **Stall-blip → TAIL** (resolved Task 0 via decomp dive of + `acclient!InterpolationManager::UseTime` @ 0x00555F20). +- **Reverted band-aid commits** `5154a3e` + `f199a6a` (commit `1641d6e`) + before re-implementing properly. --- @@ -163,13 +185,14 @@ internal sealed class InterpolationNode if (progress < StallProgressMinFraction * (catchUpSpeed * dt * StallCheckFrameInterval)): _failCount++ if _failCount > StallFailCountForBlip: - // blip: hard-snap and clear queue. - // OPEN PRECISION ITEM: retail's UseTime (acclient!00555f20) decides - // head-vs-tail snap; the agent reports disagreed (R1 implies head, R2 says - // tail). Verify by reading the UseTime disasm before implementing this - // branch. Default for the initial port: snap to HEAD (next intended - // waypoint) which matches the more common pattern. - body.Position = headTarget.Position + // blip: snap to TAIL (most recent server-sent waypoint) and clear queue. + // RESOLVED 2026-05-02 via decomp dive of acclient!InterpolationManager:: + // UseTime @ 0x00555F20: lines 353273-353333 read this->position_queue.tail_, + // copy tail.Position into local var, call CPhysicsObj::SetPositionSimple + // on it, then StopInterpolating. Semantic: "warp to where the server + // LAST SAID you are", not "where you were trying to get to next." + tail = queue.Last + body.Position = tail.TargetPosition // SetPositionSimple equivalent Clear() return Vector3.Zero else: @@ -292,13 +315,155 @@ Public method, ~10 lines, no new file. One commit titled `chore(motion): remove ACDREAM_INTERP_MANAGER flag + dead soft-snap path`: - Delete the `if/else` env-var gate in `OnLivePositionUpdated` and - `OnLiveRemoteTick`. Keep only the new path. + `TickAnimations` per-frame remote tick. Keep only the new path. - Delete `RemoteMotion.SnapResidualDecayRate` field + soft-snap residual fields. - Delete the apply_current_movement + Euler dead-reckoning code in the per-frame remote tick (the OLD branch). -Net diff after cleanup: ~50 lines deletion, code shrinks. +Net diff after cleanup: ~80 lines deletion, code shrinks. + +--- + +## L.3.2 architecture (PositionManager — combined into L.3.1) + +### New file — `src/AcDream.Core/Physics/PositionManager.cs` + +Pure-data class, no game/window deps. Pure function: takes (animation +root motion + body orientation + InterpolationManager + maxSpeed) and +returns the per-frame world-space delta to add to `body.Position`. +Composed into `RemoteMotion` alongside the `Interp` field. + +**API:** + +```csharp +public sealed class PositionManager +{ + /// + /// Per-frame combiner: animation root motion + InterpolationManager + /// correction. Mirrors retail CPhysicsObj::UpdateObjectInternal + /// (acclient @ 0x00513730): + /// rootOffset = CPartArray::Update(dt) // animation + /// PositionManager::adjust_offset(rootOffset) // adds correction + /// frame.origin += rootOffset + /// + public Vector3 ComputeOffset( + double dt, + Vector3 currentBodyPosition, + Vector3 seqVel, // body-local velocity from active animation cycle + Quaternion ori, // body orientation (for local→world rotation) + InterpolationManager interp, + float maxSpeed) + { + // Step 1: animation root motion (body-local → world). + Vector3 rootMotionLocal = seqVel * (float)dt; + Vector3 rootMotionWorld = Vector3.Transform(rootMotionLocal, ori); + + // Step 2: interpolation correction (world-space already). + Vector3 correction = interp.AdjustOffset(dt, currentBodyPosition, maxSpeed); + + // Step 3: combined. + return rootMotionWorld + correction; + } +} +``` + +### Composition + +`RemoteMotion` (in `GameWindow.cs:224`) gains a second field: + +```csharp +public AcDream.Core.Physics.PositionManager Position { get; } = + new AcDream.Core.Physics.PositionManager(); +``` + +(Already has `public InterpolationManager Interp` from Task 3.) + +### Per-frame `TickAnimations` (env-var-on branch) + +```csharp +if (Environment.GetEnvironmentVariable("ACDREAM_INTERP_MANAGER") == "1") +{ + // Always-run-all-steps per retail UpdateObjectInternal (0x00513730). + Vector3 seqVel = ae.Sequencer?.CurrentVelocity ?? Vector3.Zero; + float maxSpeed = rm.Motion.GetMaxSpeed(); + Vector3 offset = rm.Position.ComputeOffset( + dt, rm.Body.Position, seqVel, rm.Body.Orientation, rm.Interp, maxSpeed); + rm.Body.Position += offset; + rm.Body.UpdatePhysicsInternal(dt); // gravity for airborne; no-op for grounded +} +else { /* legacy path (kept until cleanup commit) */ } +``` + +Replaces the Task 5 commit's `if (rm.Interp.IsActive) { ... AdjustOffset ... }` +block. PositionManager calls AdjustOffset internally. + +### IsGrounded plumbing — `EntityPositionUpdate` + +`PositionFlags.IsGrounded = 0x04` is already parsed in +`UpdatePosition.cs:48`. Add a `bool IsGrounded` field to +`EntityPositionUpdate` record, populate at the parse site, consume in +`OnLivePositionUpdated`. ~3 lines. + +### Retail-faithful jump pipeline + +Rewrites the `OnLivePositionUpdated` env-var-on branch to match retail +`MoveOrTeleport` (acclient @ 0x00516330) — `has_contact=false → return`: + +```csharp +if (Environment.GetEnvironmentVariable("ACDREAM_INTERP_MANAGER") == "1") +{ + rmState.Body.Orientation = rot; // orientation always snaps + + if (!update.IsGrounded) // airborne: no-op + return; + + if (rmState.Airborne) // landing transition + { + rmState.Airborne = false; + rmState.Body.Velocity = Vector3.Zero; + rmState.Body.State &= ~PhysicsStateFlags.Gravity; + rmState.Body.TransientState |= TransientStateFlags.Contact | TransientStateFlags.OnWalkable; + rmState.Interp.Clear(); + rmState.Body.Position = worldPos; // hard-snap to landing + return; + } + + // Grounded routing (CPhysicsObj::MoveOrTeleport): + const float MaxPhysicsDistance = 96f; + var localPlayerPos = _playerController?.Position ?? Vector3.Zero; + float dist = Vector3.Distance(worldPos, localPlayerPos); + + if (dist > MaxPhysicsDistance) + { + rmState.Interp.Clear(); + rmState.Body.Position = worldPos; + } + else + { + float headingFromQuat = ExtractYawFromQuaternion(rot); + rmState.Interp.Enqueue(worldPos, headingFromQuat, isMovingTo: false); + } + return; +} +``` + +`OnLiveVectorUpdated` is **unchanged** — already sets velocity, marks +airborne, enables Gravity, applies Omega (Task 6). + +### L.3.2 unit tests + +New test file `tests/AcDream.Core.Tests/Physics/PositionManagerTests.cs`, +~6 tests against the pure `ComputeOffset` function: + +| Test | Verifies | +|---|---| +| `ComputeOffset_StationaryRemote_BothSourcesZero_NoMotion` | seqVel=0, queue empty → returns Vector3.Zero | +| `ComputeOffset_AnimationOnly_Forward_BodyAdvances` | seqVel=(0,4,0), identity orientation → returns (0, 4*dt, 0) | +| `ComputeOffset_AnimationOnly_OrientedSouth_BodyMovesSouth` | seqVel=(0,4,0), orientation faces -Y → returns (0,-4*dt,0) | +| `ComputeOffset_InterpOnly_NoAnimation_BodyChasesQueue` | seqVel=0, queue active → returns Interp's delta | +| `ComputeOffset_BothActive_Combined` | both nonzero → returns sum | +| `ComputeOffset_LocalToWorldRotation_Yaw90` | seqVel=(0,1,0), yaw=π/2 → returns (sin(π/2), cos(π/2)·1, 0) verifying rotation | --- @@ -320,13 +485,13 @@ no game/window/loader needed. ## Acceptance criteria -L.3.1 is shippable when: +L.3.1+L.3.2 (combined) is shippable when: -1. `dotnet build` green; existing 91 unit tests + new ~13 InterpolationManager + ~5 routing tests all pass. -2. **Visual primary:** parallel retail observer of `+Acdream` standing still, walking, running, strafing, jumping, turning — **all motion glides smoothly**, no 1-Hz popping. -3. **Visual regression check:** `+Acdream`-from-retail-observer behaviors fixed in commit `17a9ff1` (backward jump direction, strafe-run animation, walk-back broadcast direction) all still work. -4. **Visual jump arc:** remote retail toon jumping shows a curved arc as observed from acdream (`Omega` applied), not a flat path. -5. After visual confirmation: cleanup commit lands removing `ACDREAM_INTERP_MANAGER` flag + old hard-snap path + dead `RemoteMotion` fields. +1. `dotnet build` green; existing 105 unit tests + 16 InterpolationManager + 5 GetMaxSpeed + ~6 PositionManager tests all pass. +2. **Visual primary:** parallel retail observer of `+Acdream` standing still, walking, running, strafing, turning — **all motion glides smoothly**, no 1-Hz popping. (PositionManager's animation-root-motion is what eliminates the chop.) +3. **Visual jump:** retail toon jumping shows a curved arc that LANDS correctly (no endless rise). Server-authoritative airborne (`IsGrounded=false → no-op`). +4. **Visual regression check:** behaviors fixed in commit `17a9ff1` (backward jump direction, strafe-run animation, walk-back broadcast direction) all still work. +5. After visual confirmation: cleanup commit lands removing `ACDREAM_INTERP_MANAGER` flag + old hard-snap path + dead `RemoteMotion` soft-snap fields. --- @@ -344,53 +509,58 @@ L.3.1 is shippable when: ## Files -### New +### Already shipped (L.3.1 original scope) -- `src/AcDream.Core/Physics/InterpolationManager.cs` — the manager class -- `tests/AcDream.Core.Tests/Physics/InterpolationManagerTests.cs` — manager tests -- `tests/AcDream.Core.Tests/Physics/MoveOrTeleportRoutingTests.cs` — routing tests (or merged into above) +- `src/AcDream.Core/Physics/InterpolationManager.cs` (commits `f43f168` + `927636e`) +- `tests/AcDream.Core.Tests/Physics/InterpolationManagerTests.cs` (16 tests) +- `src/AcDream.Core/Physics/MotionInterpreter.cs` `GetMaxSpeed()` (commits `9c5634a` + `5b26d28`) +- `tests/AcDream.Core.Tests/Physics/MotionInterpreterTests.cs` (5 GetMaxSpeed tests added) +- `RemoteMotion.Interp` field (commit `517a3ce`) +- `OnLivePositionUpdated` env-var routing v1 (commit `062e19f`) +- Per-frame `Interp.AdjustOffset` v1 (commit `ae79e34`) +- `OnLiveVectorUpdated.Omega` application (commit `e08accf`) +- Reverted band-aid commits (commit `1641d6e`) -### Modified +### To ship (L.3.2 added scope) +**New:** +- `src/AcDream.Core/Physics/PositionManager.cs` — pure-data combiner class +- `tests/AcDream.Core.Tests/Physics/PositionManagerTests.cs` — ~6 tests + +**Modified:** +- `src/AcDream.Core.Net/WorldSession.cs` — add `IsGrounded` field to `EntityPositionUpdate` record + populate at parse site (~3 lines) - `src/AcDream.App/Rendering/GameWindow.cs`: - - `RemoteMotion` (~line 224): add `Interp` field - - `OnLivePositionUpdated` (~line 3151): new routing behind env-var - - `OnLiveVectorUpdated` (~line 3064): apply Omega - - `OnLiveRemoteTick` (per-frame): new offset-add behind env-var -- `src/AcDream.Core/Physics/MotionInterpreter.cs`: add `GetMaxSpeed()` -- `docs/plans/2026-04-11-roadmap.md`: insert Phase L.3 entry between L.2 and M -- `docs/ISSUES.md`: close any motion-related open issues this fixes (none currently filed) + - `RemoteMotion` (~line 224): add `Position` field (alongside existing `Interp`) + - `OnLivePositionUpdated` env-var branch: rewritten — airborne no-op + landing transition + grounded routing (replaces the existing v1 routing) + - `TickAnimations` env-var branch: rewritten — `PositionManager.ComputeOffset` + `UpdatePhysicsInternal` (replaces the existing v1 Interp.AdjustOffset call) ### Cleanup commit (after verification) -Same files as Modified above, with the env-var dual paths collapsed -to single retail-faithful path, and `RemoteMotion` soft-snap fields -deleted. +Single commit: collapses env-var dual paths to retail-faithful path, +deletes `RemoteMotion` soft-snap residual fields. ~80 lines deletion. --- -## Out of scope (deferred to L.3.2 / L.3.3) +## Out of scope (deferred to L.3.3) -- `PositionManager` (combines anim root-motion + interpolation offset before writing body.Frame) — L.3.2 - Server-controlled MoveTo creature behavior (retracking, sticky, fail-distance) — L.3.3 - Replacing `RemoteMoveToDriver.cs` — L.3.3 - VectorUpdate.Omega for other entity types (projectiles, dropped items) — defer; current spec applies only to player/creature/NPC paths --- -## Implementation order (L.3.1) +## Implementation order (L.3.1+L.3.2 combined — remaining work) -1. Add `InterpolationManager.cs` + unit tests. Build green. -2. Add `MotionInterpreter.GetMaxSpeed()`. Build green. -3. Modify `RemoteMotion` to compose `Interp`. Build green. -4. Add env-var gated routing in `OnLivePositionUpdated`. Build green; flag off → existing behavior. -5. Add env-var gated tick in `OnLiveRemoteTick`. Build green; flag off → existing behavior. -6. Apply `OnLiveVectorUpdated.Omega`. Build green. -7. Visual verification (flag on) — confirm acceptance criteria. -8. Cleanup commit: delete env-var, dead paths, dead RemoteMotion fields. -9. Update roadmap. +Original L.3.1 commits 1-6 already shipped. The two band-aid commits (`5154a3e`, `f199a6a`) reverted in `1641d6e`. Remaining: -Each step is a single commit. Direct-to-main per CLAUDE.md. +1. **`feat(physics): PositionManager class + 6 unit tests`** — subagent-implemented. Pure-data class + tests against stub Interp. +2. **`feat(net): plumb IsGrounded through EntityPositionUpdate`** — parent edit, 3 lines. +3. **`feat(motion): retail-faithful per-frame remote tick (PositionManager + IsGrounded routing)`** — subagent. Adds `RemoteMotion.Position` field + rewrites both env-var-on branches (`OnLivePositionUpdated` and the per-frame tick). Single commit because changes are tightly coupled. +4. **USER GATE — visual verification** with retail observer of `+Acdream` performing the test matrix. +5. **`chore(motion): remove ACDREAM_INTERP_MANAGER flag + dead legacy paths`** — subagent. Cleanup commit. +6. **`docs(roadmap+spec): L.3.1+L.3.2 combined; L.3.3 still separate`** — parent. Roadmap entry update + spec status mark. + +Each step is one commit. Direct-to-main per CLAUDE.md. ---