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>
277 lines
15 KiB
Markdown
277 lines
15 KiB
Markdown
# 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.
|