feat(motion): L.3 M1 — fresh InterpolationManager port + retail spec

Rewrites src/AcDream.Core/Physics/InterpolationManager.cs from the spec in docs/research/2026-05-04-l3-port/04-interp-manager.md. Public API preserved (Vector3-returning AdjustOffset, Enqueue, Clear, IsActive, Count) so PositionManager + GameWindow callers continue to compile; internals are full retail spec. Bug fixes vs prior port (audit 04-interp-manager.md § 7): #1 progress_quantum accumulates dt (sum of frame deltas), not step magnitude. Retail line 353140; the prior port's `+= step` made the secondary stall ratio meaningless. #3 Far-branch Enqueue (dist > AutonomyBlipDistance = 100m) sets _failCount = StallFailCountThreshold + 1 = 4, so the next AdjustOffset call's post-stall check fires an immediate blip-to- tail snap. Retail line 352944. Prior port silently drifted toward far targets at catch-up speed instead of teleporting. #4 Secondary stall test ports the retail formula verbatim: cumulative / progress_quantum / dt < CREATURE_FAILED_INTERPOLATION_PERCENTAGE. Audit notes the units are 1/sec (likely Turbine bug or x87 FPU misread by Binary Ninja) — mirrored byte-for-byte regardless. #5 Tail-prune is a tail-walking loop, not a single-tail compare. Multiple consecutive stale tail entries within DesiredDistance (0.05 m) of the new target collapse together. Retail line 352977. #6 Cap-eviction at the HEAD when count reaches 20 (already correct in the prior port; verified). New API: Enqueue gains an optional `currentBodyPosition` parameter so the far-branch detection can reference the body when the queue is empty. Backward-compatible (default null = pre-far-branch behavior). UseTime collapsed into AdjustOffset's tail (post-stall blip check) since acdream has no per-tick UseTime call separate from adjust_offset; identical semantic outcome. State fields renamed to retail names with sentinel values: _frameCounter, _progressQuantum, _originalDistance (init = 999999f sentinel per retail line 0x00555D30 ctor), _failCount. Tests: - 17/17 InterpolationManagerTests green. - New test Enqueue_FarBranch_PrearmsImmediateBlipOnNextAdjustOffset pins the bug #3 fix: enqueueing 150 m away triggers a same-tick blip (delta length ≈ 150 m), and the queue clears. Spec tree: 17 research docs (00–14) under docs/research/2026-05-04-l3-port/. 00-master-plan + 00-port-plan describe the 8-phase rollout. 01-per-tick, 03-up-routing, 04-interp-manager, 05-position-manager-and-partarray, 06-acdream-audit, 14-local-player-audit are the L.3 spec used by this commit and the M2 follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 14:56:42 +02:00 · 2026-05-05 14:56:42 +02:00 · de129bc164
commit de129bc164
parent a3f53c2644
18 changed files with 10721 additions and 190 deletions
--- a/docs/research/2026-05-04-l3-port/00-port-plan.md
+++ b/docs/research/2026-05-04-l3-port/00-port-plan.md
@ -0,0 +1,277 @@
+# L.3 — Player remote motion: retail-faithful port plan
+
+**Date:** 2026-05-04
+**Source research:** [01-per-tick.md](01-per-tick.md), [02-um-handling.md](02-um-handling.md), [03-up-routing.md](03-up-routing.md), [04-interp-manager.md](04-interp-manager.md), [05-position-manager-and-partarray.md](05-position-manager-and-partarray.md), [06-acdream-audit.md](06-acdream-audit.md)
+**Goal:** Reported user issues — body keeps walking after actor stops, backward walk regression flips to forward-run animation, position drift over time during co-running. Fix all three by porting retail's motion pipeline faithfully.
+
+---
+
+## What retail actually does (synthesized)
+
+**Per-tick body advancement** (`CPhysicsObj::UpdateObjectInternal` @ acclient 0x005156b0 → `UpdatePositionInternal` @ 0x00512c30):
+
+```
+1. Frame local = identity                           // var_40 cached
+2. CPartArray::Update writes animation root motion into local
+   - For locomotion cycles (Walk/Run/Sidestep), translation += seqVel × dt
+   - For Ready/idle cycles, no translation
+3. PositionManager::adjust_offset(local, dt) modifies local in-place:
+   - InterpolationManager::adjust_offset:
+     * If queue empty OR body within DesiredDistance(0.05m) of head → no-op
+       (animation root motion drives)
+     * Else → OVERWRITE local.translation with direction × min(catchUpSpeed × dt, distance)
+       (catch-up REPLACES root motion this frame)
+   - StickyManager / ConstraintManager (deferred — niche features)
+4. Frame::combine(out, m_position.frame, local) — compose with current world frame
+5. UpdatePhysicsInternal Euler-integrates m_velocityVector × dt INTO out.origin
+   (gated on velocity² > 0; for walking remotes m_velocityVector=0 so no-op)
+6. transition() — sphere sweep from m_position to out
+7. set_frame(resolved) or SetPositionInternal(transition result)
+```
+
+**Critical retail invariants:**
+- `m_velocityVector` is 0 for walking remotes. Only set by outbound jump (LeaveGround) + inbound 0xF74E VectorUpdate.
+- ALL visible motion comes from animation root motion + InterpolationManager catch-up.
+- Catch-up speed = `2 × motion_max_speed × dt` where `motion_max_speed` = current cycle's actual velocity magnitude.
+- Same pipeline for every entity — no player-vs-NPC special-casing at per-tick layer.
+
+**UM (UpdateMotion) handling** (`CMotionInterp::move_to_interpreted_state` @ 0x00528a90):
+- Inbound 0xF74C → bulk `copy_movement_from` of all 7 InterpretedState fields (acdream already does this).
+- **Stop signal is implicit**: flag 0x02 (forward) cleared → `ForwardCommand` defaults to `Ready`, `ForwardSpeed = 1.0`. No explicit "stop motion" packet.
+- Backward-walk arrives pre-adjusted: sender's `adjust_motion` flips `WalkBackward → WalkForward + speed=-0.65×s`; receiver bulk-copies.
+- Side-axis and turn-axis fire `DoInterpretedMotion` per axis (acdream already does this).
+
+**UP (UpdatePosition) routing** (`CPhysicsObj::MoveOrTeleport` @ 0x00516330):
+- Tri-state decision tree:
+  - **Hard teleport**: teleport-seq advanced OR cell == 0 → `SetPosition` with flags 0x1012 (Slide+Placement+SendPositionEvent). Body.Position changes immediately.
+  - **InterpolateTo (queue)**: grounded AND distance < 96m → `position_queue` mutated; **Body.Position does NOT change**.
+  - **Slide-snap**: grounded AND distance ≥ 96m → `StopInterpolating` + `SetPositionSimple`.
+  - **Airborne**: no-op (gravity arc continues from launch velocity).
+- Orientation rides the Position struct's Frame — never queued separately. Hard-snapped on UP.
+
+**Constants verified from named binary:**
+- `MAX_PHYSICS_DISTANCE = 96` m
+- `CREATURE_OUTSIDE_BLIP_DISTANCE = 100` m
+- `CREATURE_INSIDE_BLIP_DISTANCE = 20` m
+- `MAX_INTERPOLATED_VELOCITY_MOD = 2.0`
+- `MAX_INTERPOLATED_VELOCITY = 7.5` m/s (fallback when motion_max unavailable)
+- `MIN_DISTANCE_TO_REACH_POSITION = 0.20` m
+- `DESIRED_DISTANCE = 0.05` m
+- `CREATURE_FAILED_INTERPOLATION_PERCENTAGE = 0.30`
+- `StallCheckFrameInterval = 5` frames
+- `StallFailCountThreshold = 3` fails
+- Queue cap = 20
+
+---
+
+## Where acdream diverges (top issues from audit)
+
+1. **Per-tick `apply_current_movement` on player remotes** (GameWindow.cs:6599) writes `body.Velocity = RunAnimSpeed × ForwardSpeed × orientation`. Retail spec: `body.Velocity` must be 0 for walking remotes. **This is the central regression.**
+
+2. **Two parallel per-tick paths.** Env-var path (L6118-6445) is the L.3 architecture but regressed (issue #40). Legacy path (L6446-6764) is production default and fundamentally wrong vs L.3. Need to collapse into one correct path.
+
+3. **`IsPlayerGuid` gates at 5 sites** route player remotes through the broken `apply_current_movement` else-branch. Retail uses one pipeline for all entities.
+
+4. **InterpolationManager bugs** (per research 04):
+   - `progress_quantum` accumulates `step` (distance) instead of `dt` (time)
+   - Secondary stall check missing `/dt` factor
+   - Missing `NodeCompleted(0)` head-pop on stall (one bad waypoint stalls indefinitely)
+   - Missing `transient_state & 1` gate
+   - Missing far-distance force-blip via `_failCount = 4` on enqueue
+
+5. **`CPartArray::Update` collapsed into single `seqVel × dt` per tick.** For locomotion cycles, both retail and acdream synthesize velocity from formula (Humanoid dat ships zero), so this is OK for the user's reported issues. The per-keyframe loop matters for non-locomotion (emotes etc) — defer.
+
+6. **Cycle picker in OnLiveMotionUpdated is acdream-original** (forward → sidestep → turn → Ready priority). Retail just plays the cycle the wire told it to play. Defer; not the immediate cause of reported bugs.
+
+---
+
+## Port plan — concrete changes
+
+Targeted at fixing all three reported user symptoms. Defers cosmetic divergences (cycle picker, full per-keyframe loop, sticky/constraint managers) to follow-up phases.
+
+### Step 1: Fix `InterpolationManager.cs` bugs
+
+File: `src/AcDream.Core/Physics/InterpolationManager.cs`
+
+Changes (all from research doc 04, sections 3 + 7):
+
+1. **`AdjustOffset`**: change `_progressQuantum += step;` → `_progressQuantum += (float)dt;`. Accumulate time, not distance.
+2. **Secondary stall check**: change `cumulative / progressQuantum < 0.30` → `(cumulative / progressQuantum) / dt < 0.30`. Match retail formula.
+3. **Stall handling**: when stall threshold exceeded, pop head node into a `_blipToPosition` field. Don't return snap delta inside AdjustOffset.
+4. **Add `UseTime()` method**: separately performs the blip via `body.SetPositionSimple` when `_failCount > StallFailCountThreshold`. Called once per tick from per-tick path.
+5. **`Enqueue`**: when distance from current body to enqueued position exceeds `MAX_PHYSICS_DISTANCE` (96m), pre-arm `_failCount = StallFailCountThreshold + 1` so next tick's `UseTime` blips immediately.
+6. **Add `IsLive` parameter to AdjustOffset** corresponding to `transient_state & 1`. Default true; pass through.
+
+Tests to add:
+- `progress_quantum` accumulates dt, not step
+- Stall after 3 windows pops head and arms blip
+- `UseTime` calls `SetPositionSimple` when armed and clears state
+- Far enqueue arms immediate blip
+
+### Step 2: Unify the per-tick path in `GameWindow.TickAnimations`
+
+File: `src/AcDream.App/Rendering/GameWindow.cs`
+
+Delete the env-var fork. Single per-tick path for all remote entities (player or NPC). This is the bulk of the work — replace lines 6118-6764 (~640 LOC) with a single ~150 LOC retail-faithful port.
+
+Per-tick algorithm (matching retail `UpdatePositionInternal`):
+
+```csharp
+// Step 0: Force grounded transient flags for non-airborne (no change)
+if (!rm.Airborne) {
+    rm.Body.TransientState |= Contact | OnWalkable | Active;
+    rm.Body.Velocity = Vector3.Zero;  // RETAIL INVARIANT: walking remotes have zero velocity
+}
+
+// Step 1: NPC MoveTo branch (existing — unchanged)
+if (!IsPlayerGuid(serverGuid) && rm.ServerMoveToActive && rm.HasMoveToDestination) {
+    /* RemoteMoveToDriver.Drive (existing — keep as-is) */
+}
+
+// Step 2: PositionManager.ComputeOffset returns either:
+//   - queue catch-up correction (when body drifted from head)
+//   - animation root motion (when at head OR queue empty)
+//   - zero (when queue empty AND seqVel zero — Ready cycle, idle observer)
+var seqVel = ae.Sequencer?.CurrentVelocity ?? Vector3.Zero;
+float maxSpeed = seqVel.Length();  // motion_max_speed = cycle's actual velocity
+if (maxSpeed <= 0f) maxSpeed = MotionInterpreter.RunAnimSpeed; // 4.0 fallback
+var preIntegratePos = rm.Body.Position;
+var offset = rm.Position.ComputeOffset(dt, preIntegratePos, seqVel, rm.Body.Orientation, rm.Interp, maxSpeed);
+var postIntegratePos = preIntegratePos + offset;
+
+// Step 3: Manual omega integration (preserve existing — bypasses MinQuantum gate)
+ApplyObservedOmega(rm, dt);
+
+// Step 4: Physics integration. With body.Velocity=0 (set in step 0), this is a
+// no-op for grounded remotes. For airborne remotes, gravity drives body.Position.
+rm.Body.calc_acceleration();
+rm.Body.UpdatePhysicsInternal(dt);
+postIntegratePos = rm.Body.Position;  // re-read in case airborne integration moved it
+
+// Step 5: Collision sweep (preserve existing — Commit B added this for slope tracking)
+if (rm.CellId != 0 && _physicsEngine.LandblockCount > 0) {
+    var resolveResult = _physicsEngine.ResolveWithTransition(
+        preIntegratePos, postIntegratePos, rm.CellId,
+        sphereRadius: 0.48f, sphereHeight: 1.2f,
+        stepUpHeight: 0.4f, stepDownHeight: 0.4f,
+        isOnGround: !rm.Airborne, body: rm.Body,
+        moverFlags: ObjectInfoState.EdgeSlide);
+    rm.Body.Position = resolveResult.Position;
+    if (resolveResult.CellId != 0) rm.CellId = resolveResult.CellId;
+    /* existing post-resolve landing detection — unchanged */
+}
+
+// Step 6: UseTime — fire stall blips for non-airborne entities with armed fail counter
+if (!rm.Airborne) {
+    rm.Interp.UseTime(rm.Body);
+}
+
+// Step 7: Sync renderable
+ae.Entity.Position = rm.Body.Position;
+ae.Entity.Rotation = rm.Body.Orientation;
+```
+
+**Removed:** the entire `apply_current_movement` else-branch (current L6599) for player remotes. The NPC `HasServerVelocity` synth-velocity branch (current L6493-6511) — NPCs don't need this either, they should also use queue-based motion. Defer NPC migration to a follow-up if it risks regression; for this phase, keep the NPC HasServerVelocity branch but remove the player path.
+
+**Conservative scope:** Player remotes get the L.3 path. NPCs keep their existing `HasServerVelocity` branch + `RemoteMoveToDriver`. Both can converge later.
+
+### Step 3: Update `OnLivePositionUpdated` UP routing
+
+File: `src/AcDream.App/Rendering/GameWindow.cs:3425-3824`
+
+Replace the legacy default branch (L3628-3761) with the env-var branch's logic — but keep the synth-velocity computation for NPCs (which still uses it via `HasServerVelocity`).
+
+For player remotes within 96m grounded: `Interp.Enqueue(worldPos, heading, isMovingTo:false)`. **No hard-snap** of `body.Position`.
+
+For player remotes outside 96m or first UP: hard-snap + `Interp.Clear()`.
+
+For airborne player remotes: existing landing-transition logic.
+
+For NPCs: existing path (synth velocity, hard-snap, etc.) — preserve.
+
+### Step 4: Drop `ApplyServerControlledVelocityCycle`
+
+File: `src/AcDream.App/Rendering/GameWindow.cs:3325-3423`
+
+This whole function exists because of issue #39 — Shift-toggle Run↔Walk doesn't fire a fresh UM. Per research doc 02, **retail's wire actually does fire fresh UMs on Shift-toggle** (because retail's outbound `apply_run_to_command` re-runs and produces a different `ForwardSpeed`). If our observed acdream-on-retail behavior shows UMs missing on Shift-toggle, that's an ACE bug — not something we should compensate for client-side.
+
+Drop the function. Drop the call site at line 3791. Drop `RemoteMotion.LastUmUpdateTime`. Drop the `IsPlayerGuid` gates the function relies on.
+
+If issue #39 reappears after this, file an ACE bug rather than re-adding client-side hysteresis logic.
+
+### Step 5: Drop `IsPlayerGuid` per-tick gates
+
+Five sites identified in audit:
+- L706 (definition — keep, used elsewhere)
+- L3349 (`ApplyServerControlledVelocityCycle` — dropped in Step 4)
+- L3727 (UP velocity-adoption fallback — review, may stay for NPCs)
+- L6493 (NPC HasServerVelocity branch — keep for now, NPCs)
+- L6512 (NPC ServerMoveToActive branch — keep, NPCs)
+- L6588 (NPC ServerMoveToActive without dest — keep, NPCs)
+
+Effectively: drop the L3349 gate via Step 4. The remaining gates correctly route NPC paths.
+
+### Step 6: Verify CPartArray velocity for locomotion cycles
+
+File: `src/AcDream.Core/Physics/AnimationSequencer.cs`
+
+`CurrentVelocity` synthesis at lines 614-646 already matches retail constants (RunAnimSpeed=4.0, WalkAnimSpeed=3.12, SidestepAnimSpeed=1.25). Per research doc 05, this is the right approximation for Humanoid (dat ships zero velocity). **No changes needed.**
+
+For sign-flipped backward walk (`WalkForward + speed=-1`), `adjustedSpeed = speedMod` directly preserves the negative sign. `CurrentVelocity.Y = WalkAnimSpeed × -1 = -3.12`. Body root-motions backward in body-local frame. Rotated by orientation = backward in world frame. Correct.
+
+### Step 7: Test, code review, visual verify
+
+- Build: `dotnet build`
+- Tests: existing `ServerControlledLocomotionTests` (7) should still pass; new `InterpolationManagerStallTests` for the bug fixes
+- Code review subagent on the unified per-tick path
+- Visual verify with user — full motion test matrix:
+  1. Steady run forward
+  2. Steady walk forward
+  3. Steady walk backward
+  4. Steady strafe right
+  5. Steady strafe left
+  6. Run + turn
+  7. Walk + turn
+  8. Run → Stop (release W)
+  9. Walk → Stop
+  10. Run → Shift toggle to walk
+  11. Walk → Shift release to run
+  12. Jump + land
+
+---
+
+## What this fix does NOT address (deferred)
+
+- **Full per-keyframe `CPartArray::Update` loop** — for non-locomotion cycles (emotes, idle subtleties). Defer until visible bug.
+- **StickyManager / ConstraintManager** — niche retail features (locked targets, etc).
+- **Branch A (Hard teleport)** in MoveOrTeleport — needs `teleport_timestamp` plumbing through the protocol.
+- **NPC migration to L.3 path** — keeps existing `HasServerVelocity` synth path; will converge later.
+- **OnLiveMotionUpdated cycle picker** — current acdream-original logic. Retail just plays the wire's cycle directly. Defer if user-visible bugs don't depend on it.
+
+---
+
+## Acceptance
+
+- All three reported user issues resolved:
+  1. Stop after running: body settles within ≤1 UP cycle (200ms) of UM(Ready) arrival.
+  2. Backward walk: body moves backward, animation plays backward (no flip to forward-run).
+  3. Long co-run: positional sync holds — drift bounded by `DesiredDistance` (0.05m).
+- `dotnet build` green.
+- `dotnet test` green (existing tests pass + new tests for InterpolationManager bug fixes).
+- Code review pass on the unified per-tick path.
+- Visual verify by user.
+
+---
+
+## Implementation order
+
+Strict serial — each step must build green before next:
+
+1. **InterpolationManager bug fixes** (Step 1) — small, isolated, testable in unit tests.
+2. **Drop `ApplyServerControlledVelocityCycle`** (Step 4) — surgical removal.
+3. **Unify per-tick path** (Step 2) — large change. Will need a code review after.
+4. **Update UP routing** (Step 3) — surgical replacement of OnLivePositionUpdated default branch.
+5. **Build + run tests** (Step 7).
+6. **Visual verify with user** (Step 7).
+
+Do NOT proceed past step 5 to user testing if any earlier step is incomplete or broken.