diff --git a/CLAUDE.md b/CLAUDE.md index 75ff1fe..0e02b6d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -548,6 +548,41 @@ via `PlayerMovementController.ApplyServerRunRate`) or from (default 2 = 5×5). - `ACDREAM_NO_AUDIO=1` — suppress OpenAL init for headless / driver- broken setups. +- `ACDREAM_REMOTE_VEL_DIAG=1` — dump per-tick / per-UM remote motion + diagnostics (`[UM_RAW]`, `[SCFAST]`, `[SCFULL]`, `[SETCYCLE]`, + `[FWD_WIRE]`, `[OMEGA_DIAG]`, `[SEQSTATE]`, `[PARTSDIAG]`, + `[VEL_DIAG]`, `[UPCYCLE]`). Heavy. +- ⚠️ `ACDREAM_INTERP_MANAGER=1` — **DO NOT ENABLE.** This was an + experimental rewrite (e94e791) of the per-tick remote motion path. + It's regressed: the env-var path drops the per-tick collision sweep + (`ResolveWithTransition`) that the default path retains, causing a + visible "staircase" pattern when remotes run up/down slopes (body + Z stays flat between UPs, snaps at each one) plus position blips + during steady-state motion. Default (env-var unset) uses the + working retail-port chain. The PositionManager class itself is + fine and retail-faithful; only the integration into per-tick was + wrong. To be re-done in a future L.3 follow-up phase as additive + refinement on top of the working chain. + +### Outbound motion wire format (acdream → ACE) + +Important quirk for cross-checking observed remote behavior. acdream's +`PlayerMovementController` + `MoveToState` builder encode motion as: + +| Local input | Wire `ForwardCommand` | Wire `HoldKey` | Wire `ForwardSpeed` | +|---|---|---|---| +| W (run) | `WalkForward` (0x05) | `Run` (2) | server runRate (~2.4–2.94) | +| W + Shift (walk) | `WalkForward` (0x05) | `None` (1) | 1.0 | + +ACE auto-upgrades `WalkForward + HoldKey.Run` → `RunForward (0x07)` +when relaying to remote observers. So our INBOUND parser sees +`fwd=0x07` for "remote is running." This matches retail's encoding. + +When the local player toggles Shift while keeping W held (Run↔Walk +demote/promote), acdream sends a fresh `MoveToState` with the new +HoldKey + ForwardSpeed. Retail's outbound likely does the same, but +ACE's behavior on relay is uncertain — see `#L.X` in ISSUES.md for +the open Run↔Walk cycle bug on observed retail-driven remotes. ### Visual verification workflow diff --git a/docs/ISSUES.md b/docs/ISSUES.md index c95ec3b..794a37b 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,154 @@ Copy this block when adding a new issue: # Active issues +## #39 — Run↔Walk cycle transition not visible on observed player remotes (acdream-as-observer) + +**Status:** OPEN +**Severity:** MEDIUM (visible animation desync; not a correctness/wire bug) +**Filed:** 2026-05-03 +**Component:** physics / motion / animation + +**Description:** When observing a remote-driven player character through +acdream and the actor toggles Shift while keeping a direction key held +(Run↔Walk demote/promote), the visible leg cycle does NOT update on the +observer side. Body position eventually corrects via UpdatePosition +hard-snaps (causing visible position blips), but the animation cycle +stays at whatever it was last set to (Run sticks; Walk sticks). + +Observation matrix: + +| Observer | Actor | Cycle Run↔Walk | Z on slopes | +|---|---|---|---| +| Retail | Retail | ✓ | ✓ | +| Retail | Acdream | ✓ | ✓ | +| Acdream | Acdream | ✓ | ✗ (only with env-var path) | +| Acdream | Retail | ✗ | ✗ | + +**Root cause / status:** + +ACE only broadcasts a fresh `UpdateMotion` (UM) when the wire's +`ForwardCommand` byte changes — i.e. on direction-key state changes +(W press, W release). Toggling Shift while W is held changes +`ForwardSpeed` and `HoldKey` but NOT `ForwardCommand`, so ACE does +NOT broadcast a UM for the demote/promote. The speed change DOES +propagate via `UpdatePosition` (position-delta velocity changes +between Run-pace and Walk-pace), confirmed via `[VEL_DIAG]` +serverSpeed varying ~2.5 m/s (walk) ↔ ~9 m/s (run). + +Retail's inbound code uses UP-derived velocity to refine the visible +cycle when no UM tells it. Acdream has the equivalent function — +`ApplyServerControlledVelocityCycle` in `GameWindow.cs:3274` — but +it's gated `if (IsPlayerGuid(serverGuid)) return;` for player +remotes, exactly the case where the gap matters. + +(Earlier hypothesized as H2 in the 2026-05-03 four-agent investigation +but marked refuted because the [UPCYCLE] diag never fired — that +was BECAUSE of the gate; un-gating reveals it firing per UP, which +is the correct behavior.) + +**Fix sketch (~10 lines):** un-gate `ApplyServerControlledVelocityCycle` +for player remotes when `currentMotion` is a locomotion cycle +(Run/Walk/Sidestep/Backward). UMs still drive direction-key changes +authoritatively; UP-derived velocity refines the speed bucket within +the same direction. Add a `LastUMUpdateTime` grace window (e.g. +500ms) so UMs win when fresh. + +**Files:** + +- `src/AcDream.App/Rendering/GameWindow.cs:3274` — `ApplyServerControlledVelocityCycle` + (the gate `if (IsPlayerGuid(serverGuid)) return;` to remove with conditions) +- `src/AcDream.App/Rendering/GameWindow.cs:3640-3660` — call site (already + passes through with HasServerVelocity from synthesized UP-deltas) +- `src/AcDream.Core/Physics/ServerControlledLocomotion.cs:54-76` — + `PlanFromVelocity` thresholds (may need re-tuning if banding is observed) + +**Research:** + +- `docs/research/2026-05-03-remote-anim-cycle/investigation-prompt.md` — + full background of the four-agent investigation +- This session's diagnostic logs at `tools/diag-logs/walkrun-A1b-*.log` + (UM_RAW, FWD_WIRE, SETCYCLE traces) confirming ACE's wire pattern + +**Acceptance:** + +- Observer in acdream watching a retail-driven character toggle Shift + while holding W: visible leg cycle switches Run↔Walk within ~200ms + of the wire change. +- No regression on the working cases (acdream-on-acdream, retail + observers, idle↔Run, idle↔Walk). +- No spurious cycle thrashing during turning while running (ObservedOmega + doesn't trigger velocity-bucket changes). + +## #40 — ACDREAM_INTERP_MANAGER=1 env-var path regressed (staircase + blips) + +**Status:** OPEN (do-not-enable; pending L.3 follow-up rebuild) +**Severity:** N/A (gated; default behavior unaffected) +**Filed:** 2026-05-03 +**Component:** physics / motion (per-tick remote prediction) + +**Description:** The `ACDREAM_INTERP_MANAGER=1` per-frame remote tick +introduced by commit `e94e791` (L.3.1+L.3.2 Task 3) is a regression and +should not be enabled. Two visible symptoms: + +1. **Z staircase on slopes:** observed remotes running up/down hills + sink into rising terrain or float over receding terrain, then snap + to correct Z at each `UpdatePosition` arrival. Body never follows + the terrain mesh between UPs. + +2. **Position blips during steady-state motion:** XY drifts + unconstrained between UPs, then UP hard-snaps cause visible jumps. + +Both symptoms ABSENT when env-var unset (default legacy path). + +**Root cause:** the env-var path was designed to mirror retail +`CPhysicsObj::MoveOrTeleport` (acclient @ 0x00516330). MoveOrTeleport +is retail's network-packet entry point — minimal work. The per-frame +physics tick is retail's `update_object` (FUN_00515020) — full chain +including `apply_current_movement` → `UpdatePhysicsInternal` → +`Transition::FindTransitionalPosition` (collision sweep). The legacy +path mirrors `update_object` correctly. The env-var path stripped the +collision sweep on a wrong assumption that this was "more retail- +faithful" — it was the opposite. + +Commit B (039149a, 2026-05-03) ported `ResolveWithTransition` into the +env-var path, but the symptom persisted because the env-var path also +clears `body.Velocity` for grounded remotes (no Euler integration of +horizontal motion → sweep input is the catch-up offset only, which +itself stair-steps because UPs are sampled at ~1 Hz). + +**Files:** + +- `src/AcDream.App/Rendering/GameWindow.cs:6042-6260` — env-var per-frame branch +- `src/AcDream.App/Rendering/GameWindow.cs:6260+` — legacy per-frame branch (works) +- `src/AcDream.Core/Physics/PositionManager.cs` — class itself is retail-faithful + (port of CPositionManager::adjust_offset), only the integration was wrong + +**Research:** + +- This session's `2026-05-03` chronological commit log + visual verification +- `docs/research/2026-05-03-remote-anim-cycle/investigation-prompt.md` + for the four-agent investigation that traced this + +**Fix path (separate L.3 follow-up phase, NOT this session):** + +The PositionManager class is correct retail-port. Re-integrate it as +ADDITIVE refinement on top of the working legacy chain (small +correction toward queued server positions, applied AFTER +`apply_current_movement` + `UpdatePhysicsInternal` + collision sweep) +— not as a REPLACEMENT for them. Match retail's actual `update_object` +chain ordering: `position_manager::adjust_offset` runs after the +primary motion + collision resolution. + +**Acceptance:** + +- New per-tick path enabled via env-var (or default after stabilization) + produces the same smooth slope motion + zero blips as the legacy path. +- Inbound `UpdatePosition` queue catch-up nudges body toward server + authoritative position without overriding terrain Z snap or causing + position blips. +- Verification: side-by-side vs legacy default in 2-client setup, + identical visible behavior. + ## #38 — Chase camera + player feel "30 fps" since L.5 physics-tick gate **Status:** OPEN diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index e44a975..fdb71a9 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -6041,6 +6041,28 @@ public sealed class GameWindow : IDisposable { if (System.Environment.GetEnvironmentVariable("ACDREAM_INTERP_MANAGER") == "1") { + // ⚠️ REGRESSED 2026-05-03 — DO NOT ENABLE — see docs/ISSUES.md #40 ⚠️ + // + // Introduced by e94e791 (L.3.1+L.3.2 Task 3) intending to + // mirror retail CPhysicsObj::MoveOrTeleport (network-packet + // entry point — minimal work). Wrong retail function for the + // per-frame tick — the actual per-frame chain is retail's + // update_object (FUN_00515020), which the LEGACY path below + // correctly mirrors (apply_current_movement → + // UpdatePhysicsInternal → ResolveWithTransition collision + // sweep). This env-var path strips the collision sweep AND + // clears body.Velocity, leaving only PositionManager queue + // catch-up — which stair-steps with the 1 Hz UP cadence on + // slopes and produces visible position blips on flat ground. + // + // Commit B (039149a, 2026-05-03) ported ResolveWithTransition + // here but symptom persists because body.Velocity=0 means + // pre/postIntegrate sweep input is just the queue catch-up, + // which itself snaps in steps. Fix requires re-integrating + // PositionManager as ADDITIVE adjust_offset on top of the + // legacy chain — separate L.3 follow-up phase. + // + // Until that lands, stay on the legacy path (env-var unset). // ── NEW PATH: retail-faithful per-frame remote tick ── // (L.3.1+L.3.2 Task 3/follow-up — ACDREAM_INTERP_MANAGER=1 gates this path) // diff --git a/src/AcDream.Core/Physics/AnimationSequencer.cs b/src/AcDream.Core/Physics/AnimationSequencer.cs index fa7b23d..9687fea 100644 --- a/src/AcDream.Core/Physics/AnimationSequencer.cs +++ b/src/AcDream.Core/Physics/AnimationSequencer.cs @@ -283,9 +283,12 @@ public sealed class AnimationSequencer private const double RateEpsilon = 1e-6; // ── Diagnostics (Commit A 2026-05-03) ─────────────────────────────────── - // Removed throttle in A.1 (2026-05-03) — every SCFAST/SCFULL call logs - // unthrottled (still gated on ACDREAM_REMOTE_VEL_DIAG=1) so we can read - // exact call rate and Run→Ready transitions one tick at a time. + // Throttle clock for the [SCFAST] / [SCFULL] / [SCNULLFALLBACK] log lines + // emitted from SetCycle. Gated on env var ACDREAM_REMOTE_VEL_DIAG=1; reads + // the env var inline rather than caching so a launch can be re-toggled + // without restarting. 0.5s per sequencer instance keeps logs readable + // while still capturing meaningful state changes. + private double _lastSetCycleDiagTime; // ── Constructor ────────────────────────────────────────────────────────── @@ -413,17 +416,20 @@ public sealed class AnimationSequencer } // D3 (Commit A 2026-05-03): SCFAST — proves whether the fast-path - // is firing instead of the full rebuild. - // A.1 (2026-05-03): unthrottled — we need actual call rate, not - // 0.5s-bucketed sample. Keeps cost low (just a Console.WriteLine - // per SetCycle call, all gated on env var). + // is firing instead of the full rebuild. Throttled to 0.5s per + // instance (re-throttled after A.1 unthrottled experiment). if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") { - System.Console.WriteLine( - $"[SCFAST] motion=0x{motion:X8} speedMod={speedMod:F3} " - + $"oldSpeedMod={CurrentSpeedMod:F3} " - + $"qCount={_queue.Count} " - + $"currNodeIsCyclic={(_currNode == _firstCyclic)}"); + double nowSec = (System.DateTime.UtcNow - System.DateTime.UnixEpoch).TotalSeconds; + if (nowSec - _lastSetCycleDiagTime > 0.5) + { + System.Console.WriteLine( + $"[SCFAST] motion=0x{motion:X8} speedMod={speedMod:F3} " + + $"oldSpeedMod={CurrentSpeedMod:F3} " + + $"qCount={_queue.Count} " + + $"currNodeIsCyclic={(_currNode == _firstCyclic)}"); + _lastSetCycleDiagTime = nowSec; + } } return; } @@ -546,20 +552,23 @@ public sealed class AnimationSequencer } // D3 (Commit A 2026-05-03): SCFULL — counterpart to SCFAST. Fires on - // the full-rebuild SetCycle path. - // A.1 (2026-05-03): unthrottled — see SCFAST comment. We also print - // the previous CurrentMotion so the log directly shows the cycle - // transition (e.g. "Run → Ready" indicates the visible cycle just - // got reset back to Ready, mid-Run). + // the full-rebuild SetCycle path. Throttled to 0.5s per instance. + // Logs prev CurrentMotion so the line shows the transition directly + // (e.g. "Run → Ready" = cycle just got reset). if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") { - System.Console.WriteLine( - $"[SCFULL] prev=0x{CurrentMotion:X8} -> motion=0x{motion:X8} adjustedMotion=0x{adjustedMotion:X8} " - + $"speedMod={speedMod:F3} " - + $"qCount={_queue.Count} " - + $"firstNewNull={(firstNew is null)} " - + $"currNodeIsCyclic={(_currNode == _firstCyclic)} " - + $"firstCyclicNull={(_firstCyclic is null)}"); + double nowSec = (System.DateTime.UtcNow - System.DateTime.UnixEpoch).TotalSeconds; + if (nowSec - _lastSetCycleDiagTime > 0.5) + { + System.Console.WriteLine( + $"[SCFULL] prev=0x{CurrentMotion:X8} -> motion=0x{motion:X8} adjustedMotion=0x{adjustedMotion:X8} " + + $"speedMod={speedMod:F3} " + + $"qCount={_queue.Count} " + + $"firstNewNull={(firstNew is null)} " + + $"currNodeIsCyclic={(_currNode == _firstCyclic)} " + + $"firstCyclicNull={(_firstCyclic is null)}"); + _lastSetCycleDiagTime = nowSec; + } } CurrentStyle = style;