diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 11173ad..449b129 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -111,6 +111,12 @@ the same direction. Add a `LastUMUpdateTime` grace window (e.g. - `docs/research/2026-05-03-remote-anim-cycle/investigation-prompt.md` — full background of the four-agent investigation +- `docs/research/2026-05-06-locomotion-cycle-transitions/investigation-prompt.md` — + expansion to the full 7-transition matrix (Run↔Walk forward + backward, + Fast↔Slow strafe L+R, direction-flip cases) with TTD-driven workflow +- `docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md` — + static-analysis findings + scope of the 2026-05-06 candidate fix + (case #1, Run↔Walk forward only) - This session's diagnostic logs at `tools/diag-logs/walkrun-A1b-*.log` (UM_RAW, FWD_WIRE, SETCYCLE traces) confirming ACE's wire pattern @@ -124,6 +130,23 @@ the same direction. Add a `LastUMUpdateTime` grace window (e.g. - No spurious cycle thrashing during turning while running (ObservedOmega doesn't trigger velocity-bucket changes). +**Progress 2026-05-06 (candidate fix shipped, awaiting visual verify):** + +- `RemoteMotion.LastUMTime` added, stamped in `OnLiveMotionUpdated`. +- `ApplyServerControlledVelocityCycle` un-gated for player remotes; + player path routed through new `ApplyPlayerLocomotionRefinement`: + - 500 ms UM grace window + - Forward-direction-only refinement (low byte 0x05 / 0x07) + - Hysteresis: Run → Walk demote < 4.5 m/s; Walk → Run promote > 5.5 m/s + - Skip SetCycle when neither motion ID nor speedMod changed meaningfully +- Outer call site at `OnLivePositionUpdated` un-gated (`!IsPlayerGuid` + removed); function-internal route handles player vs NPC. +- Diagnostic `[UPCYCLE_PLAYER]` line gated on `ACDREAM_REMOTE_VEL_DIAG=1`. + +Scope of this fix is **case #1 (Run↔Walk forward) only**. Cases #2–#7 +(backward, sidestep, direction-flips) remain deferred until TTD trace +grounds retail's exact algorithm — see findings-static.md §"Does NOT". + ## #42 — [DONE 2026-05-05 · ec59a08] Airborne XY drift on observed player remote jumps (~1 m horizontal offset over arc) **Status:** DONE diff --git a/docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md b/docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md new file mode 100644 index 0000000..c75e24f --- /dev/null +++ b/docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md @@ -0,0 +1,167 @@ +# Locomotion-cycle transitions — static-analysis findings + +**Date:** 2026-05-06 (follow-up to `investigation-prompt.md`) +**Status:** static analysis complete; candidate fix shipped for case #1; cases #2–#7 deferred until TTD validation. + +This is what code reading + ACE cross-reference + named-retail grep +established before any TTD/cdb trace was run. It scopes the candidate +fix and identifies which questions still need the trace. + +--- + +## What's confirmed by static analysis + +### 1. ACE broadcasts unconditionally on inbound MoveToState + +`references/ACE/Source/ACE.Server/Network/GameAction/Actions/GameActionMoveToState.cs:36` +calls `session.Player.BroadcastMovement(moveToState)` for every received +MoveToState, no diff-check. So **whether ACE broadcasts a UM is determined +purely by whether the client sent a MoveToState**. + +### 2. ACE auto-upgrades `WalkForward + HoldKey.Run → RunForward` on broadcast + +`references/ACE/Source/ACE.Server/Network/Motion/MovementData.cs:110-113` +shows the conversion: when client sends `WalkForward + HoldKey.Run`, the +broadcast `interpState.ForwardCommand` is upgraded to `RunForward`. So +the broadcast UM byte differs between Run and Walk even though the +client's wire byte is the same `WalkForward`. + +### 3. Therefore the bug is one of these two + +Either: +- **A.** Retail (the actor) does NOT send a fresh MoveToState when only + HoldKey changes (Shift toggle while a direction key is held). ACE has + nothing to broadcast → no UM arrives at the observer. UPs continue + with new velocity (run-pace ↔ walk-pace), but observer's UM-driven + cycle never updates. +- **B.** Retail DOES send a fresh MoveToState. ACE broadcasts a UM. Our + parser receives it but fails to apply the cycle change for some + reason. + +The diagnostic data captured in 2026-05-03's investigation +(`[FWD_WIRE]`, `[UM_RAW]`, `[SETCYCLE]`) is more consistent with **A**: +no `[FWD_WIRE]` line was logged for Shift toggles, suggesting no UM +arrived. But that data was for a different scenario, so a fresh trace +is needed. + +**This question is the primary gap the TTD/cdb trace must close.** +A 2-minute cdb session with a breakpoint counter on +`acclient!CommandInterpreter::SendMovementEvent` answers it +definitively. + +### 4. `ApplyServerControlledVelocityCycle` was gated against player remotes at three sites + +In `src/AcDream.App/Rendering/GameWindow.cs` before this change: + +- **Inner gate**, function entry (~line 3326): + `if (IsPlayerGuid(serverGuid)) return;` +- **Outer gate**, call site in `OnLivePositionUpdated` (~line 3683): + `if (!IsPlayerGuid(update.Guid) && rmState.HasServerVelocity ...)` +- **TickAnimations gate** (~line 6325): + `if (!IsPlayerGuid(serverGuid) && rm.HasServerVelocity)` — + this one is for the "stale velocity → reset to zero" path, used by + NPCs whose MoveTo has expired. Leave as-is for now; player remotes + don't go through this branch (no MoveTo path). + +### 5. `ServerControlledLocomotion.PlanFromVelocity` thresholds are wrong for player speeds + +In `src/AcDream.Core/Physics/ServerControlledLocomotion.cs:30-33`: + +```csharp +public const float StopSpeed = 0.20f; +public const float RunThreshold = 1.25f; +``` + +For player speeds (Walk ≈ 2.5 m/s, Run ≈ 9 m/s — both observed in +prior `[VEL_DIAG]` traces), `RunThreshold = 1.25` is below both +buckets. Both speeds resolve to `RunForward`. So even if we just +un-gated the existing function for player remotes, the function would +*always* return `RunForward` and the bug would persist. + +The function works correctly for NPCs (typical NPC speed range +1–6 m/s falls on the right side of the 1.25 threshold for the +Idle/Walk/Run distinction). + +**Implication:** the player path must use different thresholds. The +candidate fix introduces hysteresis tuned for player speeds (4.5 demote, +5.5 promote) and routes player remotes through a dedicated +`ApplyPlayerLocomotionRefinement` helper instead of `PlanFromVelocity`. + +### 6. `PlanFromVelocity` returns Forward-only motions + +It returns `Ready`, `WalkForward`, or `RunForward` — never sidestep, +never backward. Even with thresholds fixed, calling it for a sidestepping +player remote would clobber `SideStepLeft`/`Right` with `RunForward`. + +**Implication:** the player path must scope itself to forward direction +only; sidestep/backward HoldKey toggles are deferred until TTD confirms +retail's per-direction algorithm. + +--- + +## What the candidate fix does (and doesn't) + +**Does:** + +- Adds `RemoteMotion.LastUMTime` (timestamp of most recent UM for a remote). +- Stamps `LastUMTime` at the top of `OnLiveMotionUpdated`. +- Removes the `IsPlayerGuid` early return inside + `ApplyServerControlledVelocityCycle`. +- Removes the `!IsPlayerGuid` gate at the outer call site in + `OnLivePositionUpdated` (so player remotes get the function called). +- Routes player remotes through new `ApplyPlayerLocomotionRefinement`: + - 500 ms UM grace window (UMs are authoritative). + - Forward-direction-only refinement (low byte 0x05 / 0x07). + - Hysteresis: Run → Walk demote at < 4.5 m/s; Walk → Run promote at + > 5.5 m/s. + - SetCycle skipped if neither motion ID nor speedMod changed + meaningfully. + - Diagnostic `[UPCYCLE_PLAYER]` line gated on `ACDREAM_REMOTE_VEL_DIAG=1`. + +**Does NOT (deferred to TTD-validated follow-up):** + +- Backward (case #2) HoldKey toggle. +- Sidestep speed-bucket refinement (cases #4, #5). +- Direction-flip transitions (cases #3, #6, #7) — these are believed + to work today via UM-driven SetCycle, but the prompt's acceptance + criteria explicitly call for visual confirmation of all 7 cases. +- Tuning the grace window or the hysteresis thresholds against retail's + exact behavior. 500 ms / 4.5 / 5.5 are defensible defaults but TTD + may show retail uses different values. + +--- + +## Acceptance for the candidate fix (case #1 only) + +When acdream observes a retail-driven character: + +- Hold W (run) for 2 s, then press Shift, then release Shift, then + release W. The visible leg cycle should switch Run → Walk → Run + → Idle within ~200–500 ms of each transition. +- All other working cases (acdream-on-acdream, retail-on-acdream, + Idle ↔ Run, Idle ↔ Walk) must still work — no regressions. + +If the candidate fix produces visible Run↔Walk transitions on retail +actors, ship it (close part of #39, file the remaining 6 cases as a +follow-up issue) and run TTD on cases #2–#7 next. + +If it doesn't switch the cycle (or thrashes / regresses something), +the next investigation step is the cdb breakpoint count on +`CommandInterpreter::SendMovementEvent` to settle question A vs. B +above before any further fix attempt. + +--- + +## Files touched by the candidate fix + +- `src/AcDream.App/Rendering/GameWindow.cs` + - Added `RemoteMotion.LastUMTime` field + - Stamped `LastUMTime` at top of `OnLiveMotionUpdated` + - Removed inner `IsPlayerGuid` gate in `ApplyServerControlledVelocityCycle` + - Routed player remotes to new `ApplyPlayerLocomotionRefinement` + - Added constants `UmGraceSeconds`, `PlayerRunPromoteSpeed`, + `PlayerRunDemoteSpeed` + - Removed `!IsPlayerGuid` gate at outer call site in `OnLivePositionUpdated` + +No changes to `ServerControlledLocomotion.cs`, `MotionInterpreter.cs`, +or `AnimationSequencer.cs`. diff --git a/docs/research/2026-05-06-locomotion-cycle-transitions/investigation-prompt.md b/docs/research/2026-05-06-locomotion-cycle-transitions/investigation-prompt.md new file mode 100644 index 0000000..84b2c09 --- /dev/null +++ b/docs/research/2026-05-06-locomotion-cycle-transitions/investigation-prompt.md @@ -0,0 +1,305 @@ +# Locomotion-cycle transitions on observed remotes — investigation prompt + +**Hand-off date:** 2026-05-06 +**Status:** open. ISSUES.md #39 captured the Run↔Walk-forward variant; this +prompt expands to the full locomotion transition matrix and lays out a +TTD-driven investigation against retail to ground the fix. + +This document is a self-contained briefing for an agent (or fresh session) +picking this up. The TTD toolchain landed in commit `e3e5bf5` (#44) and +is the primary investigative tool here. + +--- + +## What problem are we trying to solve? + +When acdream observes a remote-driven player character (typically a parallel +retail acclient.exe connected to the same local ACE), the visible leg-cycle +animation does not always switch when the actor changes locomotion mode. +The body translates at the right speed (server-driven velocity is fine), but +the legs keep playing whichever cycle was active before. + +The full set of transitions where the bug may surface: + +| # | Transition | Wire change | Likely cause | +|---|---|---|---| +| 1 | Run forward ↔ Walk forward (Shift toggle while W held) | HoldKey + ForwardSpeed only | ACE doesn't broadcast UM (no ForwardCommand change); cycle refinement must come from UP-derived velocity | +| 2 | Run backward ↔ Walk backward (Shift toggle while S held) | HoldKey + ForwardSpeed only | Same as #1, backward axis | +| 3 | Forward ↔ Backward (W→S or S→W direct flip) | ForwardCommand changes (e.g. `WalkForward` → `WalkBackward`) | ACE broadcasts a UM; cycle should update from UM directly | +| 4 | Fast strafe-left ↔ Slow strafe-left (Shift toggle while A held) | HoldKey + SideSpeed only | Same as #1 / #2 — speed-bucket only | +| 5 | Fast strafe-right ↔ Slow strafe-right (Shift toggle while D held) | HoldKey + SideSpeed only | Same | +| 6 | Strafe-Left ↔ Strafe-Right (A↔D direct flip) | SideCommand changes | ACE broadcasts UM | +| 7 | Forward ↔ Strafe (W↔A, W↔D, etc.) | ForwardCommand or SideCommand changes | UM broadcast | + +#39 in ISSUES.md focused on #1. This investigation widens scope to the +whole matrix because the underlying wire pattern is the same shape: half +the transitions broadcast a UM, the other half rely on UP-derived velocity. +A correct fix should handle both classes uniformly. + +--- + +## What we already know (don't re-discover) + +From the prior 2026-05-03 investigation (`docs/research/2026-05-03-remote-anim-cycle/`) +and #39 in `docs/ISSUES.md`: + +- **ACE only broadcasts a fresh UpdateMotion when the wire's `ForwardCommand` + byte changes** — i.e. on direction-key state changes. Toggling Shift while + W held changes `ForwardSpeed` and `HoldKey` but NOT `ForwardCommand`, so + ACE does NOT broadcast a UM for the demote/promote. +- **Speed changes still propagate via UpdatePosition (UP).** 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. +- **The fix sketch is small (~10 lines):** un-gate + `ApplyServerControlledVelocityCycle` for player remotes when + `currentMotion` is a locomotion cycle. Add a `LastUMUpdateTime` grace + window so fresh UMs win over UP-velocity refinement. +- **Outbound from acdream is fine** — the matrix in #39 confirms retail + observers see acdream's transitions correctly. The bug is purely in + acdream's RECEIVE path. + +What's NOT yet confirmed (and is the primary goal of this investigation): + +- **Which exact retail function** does the cycle refinement when no UM + arrives? Hypothesis: `CPhysicsObj::unpack_movement` → + `MovementManager::unpack_movement` → some velocity-aware cycle picker. + We need to identify the function name and what state it reads. +- **What grace window** does retail use between UM and UP-derived + refinement? Our hypothesis is ~500 ms but it may be different. +- **Does retail use SideSpeed or SideCommand changes** the same way for + strafe transitions? + +--- + +## TTD-driven workflow + +The toolchain shipped in #44 (commit `e3e5bf5`). Bootstrap is one-time +per machine — see CLAUDE.md "TTD recordings (offline replay)" or +`tools/ttd-record.ps1` header. + +### Recording scenario + +You need **two retail acclient.exe instances** running on the same local +ACE — one as the recorded process, one as the actor. Acdream can be +involved as a third client for cross-checking but is not strictly required +for the recording itself. + +Setup: +1. Launch retail #1 on `testaccount` / `+Acdream` (or any character). +2. Launch retail #2 on a second account / character. It must be on the + same landblock so retail #1 can see it. +3. Position retail #2 in retail #1's view, ~10 m away. +4. In an **elevated** PowerShell, attach TTD to retail #1 (the observer): + ```powershell + tools\ttd-record.ps1 -RingMaxMB 512 + ``` +5. Wait for `Recording is now active`. Retail #1 will be ~10× slower — + that's expected. + +Scenario (drive on retail #2, ~60 seconds total): + +``` +Phase 1 — Forward speed: Hold W (2 s) → +Shift → -Shift → +Shift → -Shift → release W (idle 1 s) +Phase 2 — Backward speed: Hold S (2 s) → +Shift → -Shift → +Shift → -Shift → release S (idle 1 s) +Phase 3 — Forward↔Back flip: Hold W (2 s) → release W → Hold S (2 s) → release S +Phase 4 — Strafe-Left speed: Hold A (2 s) → +Shift → -Shift → +Shift → -Shift → release A +Phase 5 — Strafe-Right speed: Hold D (2 s) → +Shift → -Shift → +Shift → -Shift → release D +Phase 6 — Strafe L↔R flip: Hold A (2 s) → release A → Hold D (2 s) → release D +Phase 7 — Forward↔Strafe: Hold W (2 s) → release W → Hold D (2 s) → release D +``` + +After phase 7, Ctrl+C the TTD recording. Trace lands at +`~/.ttd/traces/acclientNN.run`. + +### Query strategy + +The recording captured ~7 phases in ~60 sec. Each phase produced specific +UM and/or UP traffic. Goal: for each phase identify what retail's receive +code did to update the visible cycle. + +Start with a `.cdb` script under `tools/ttd-queries/locomotion.cdb`: + +``` +.echo === Top motion-receive entry points === +dx -r2 @$cursession.TTD.Calls("acclient!CPhysicsObj::unpack_movement").Take(20) +dx -r2 @$cursession.TTD.Calls("acclient!MovementManager::unpack_movement").Take(20) + +.echo === Cycle-update functions called === +dx -r2 @$cursession.TTD.Calls("acclient!*set_animation*").GroupBy(c => c.Function).Select(g => new { Name = g.First().Function, Count = g.Count() }).OrderByDescending(x => x.Count) +dx -r2 @$cursession.TTD.Calls("acclient!*SetCycle*").GroupBy(c => c.Function).Select(g => new { Name = g.First().Function, Count = g.Count() }).OrderByDescending(x => x.Count) +dx -r2 @$cursession.TTD.Calls("acclient!*play_*").GroupBy(c => c.Function).Select(g => new { Name = g.First().Function, Count = g.Count() }).OrderByDescending(x => x.Count) + +.echo === Velocity / position-update handlers === +dx -r2 @$cursession.TTD.Calls("acclient!*UpdatePosition*").GroupBy(c => c.Function).Select(g => new { Name = g.First().Function, Count = g.Count() }).OrderByDescending(x => x.Count) +dx -r2 @$cursession.TTD.Calls("acclient!CPhysicsObj::set_velocity").Take(15) + +.echo === DONE === +q +``` + +Run it: +```powershell +tools\ttd-query.ps1 -Script tools\ttd-queries\locomotion.cdb +``` + +**Then iterate.** Use the function-name hits to guide the next query — +inspect call args (`Take(N)` instead of `Count()`), navigate to specific +TTD positions and dump struct fields with `dt acclient!ClassName `. + +The named-retail decomp at `docs/research/named-retail/acclient_2013_pseudo_c.txt` +has the source for every retail function. Grep it by class::method to +read what the function does, then use TTD to confirm what it does at +runtime. + +### Specifically what to look for + +For each phase in the recording, answer: + +1. **Did a UM arrive?** Look for calls to the receive entry points around + that phase's time window. UM dispatch should fire on direction-key + changes (phases 3, 6, 7) but NOT on Shift-only toggles (phases 1, 2, 4, 5). +2. **What function refined the cycle when no UM arrived?** Hypothesis is + something in the velocity-aware path. Find the symbol, read the + decomp, document the conditions. +3. **What threshold/grace logic is in place?** Retail must have some way + to prevent UP-velocity refinement from fighting a fresh UM. Usually + a timestamp comparison. + +--- + +## The fix + +The fix lives in `src/AcDream.App/Rendering/GameWindow.cs:3274` +(`ApplyServerControlledVelocityCycle`). Current code returns early for +player remotes. The fix is to: + +1. **Un-gate the early return** when the current motion is a locomotion + cycle. Locomotion cycles to detect: `0x44000007` (Run forward), + `0x45000005` (Walk forward), backward equivalents, sidestep variants. + Keep the gate when the motion is non-locomotion (e.g. emotes, attacks). +2. **Add a `LastUMUpdateTime` per remote.** Touch it in the UM handler + path. In `ApplyServerControlledVelocityCycle`, skip refinement if + `(now - LastUMUpdateTime) < graceMs`. Start with `graceMs = 500` + and tune against the TTD findings. +3. **Use UP-derived velocity to pick the speed bucket.** Existing logic + in `ServerControlledLocomotion.PlanFromVelocity` already does this — + verify thresholds match retail's bucketing (TTD will tell you the + exact boundaries retail uses). + +For direction-flip transitions (phases 3, 6, 7), the UM handler path +should already work — the visible cycle should update from the UM +directly. Confirm in-client that those transitions are clean BEFORE +shipping the velocity-fallback fix; if they're broken too, that's a +separate UM-handler bug that needs its own investigation. + +--- + +## Acceptance criteria + +In acdream observing a retail-driven character: + +- **Phases 1, 2, 4, 5 (Shift toggles):** visible leg cycle switches + Run↔Walk / Fast↔Slow within ~200 ms of the wire change. +- **Phases 3, 6, 7 (direction flips):** visible cycle updates within + ~100 ms (UM is direct-path, faster than UP). +- **No regression** on already-working cases: + - acdream-on-acdream (matrix row in #39) + - retail observers viewing acdream (works today) + - Idle ↔ Run transitions + - Idle ↔ Walk transitions + - Combat-mode locomotion (if testable) +- **No spurious cycle thrashing during turning while running** — + ObservedOmega-driven body rotation must not trigger + velocity-bucket changes mid-cycle. + +--- + +## Files to read first + +In this order: + +1. `docs/ISSUES.md` — issue #39 (Run↔Walk specific) and #44 (TTD toolchain) +2. `docs/research/2026-05-03-remote-anim-cycle/investigation-prompt.md` — + prior investigation, what's been tried, what works +3. `CLAUDE.md` — "Retail debugger toolchain" section (live cdb + + "TTD recordings (offline replay)" subsection) +4. `memory/project_retail_debugger.md` — durable lessons + TTD addendum +5. `memory/project_retail_motion_outbound.md` — what retail's outbound + motion path looks like (cdb live trace from 2026-05-01) + +Then: + +6. `src/AcDream.App/Rendering/GameWindow.cs` — `OnLiveMotionUpdated` + (~line 3203) and `ApplyServerControlledVelocityCycle` (~line 3274) +7. `src/AcDream.Core/Physics/ServerControlledLocomotion.cs` — speed + bucket thresholds in `PlanFromVelocity` +8. `docs/research/named-retail/acclient_2013_pseudo_c.txt` — grep by + the symbol names you discover from the TTD trace +9. `references/ACE/Source/ACE.Server/Network/GameMessages/Messages/GameMessageUpdateMotion.cs` + — what ACE actually sends for each transition (ground truth for + wire content) + +--- + +## Watchouts + +- **The named-retail leaf `MoveToStatePack::UnPack` returned 0 hits in + TTD.** Don't query it directly; start at `CPhysicsObj::unpack_movement` + or `MovementManager::unpack_movement` and walk down the call chain. + Release-build inlining + virtual dispatch hide leaf decoders from + TTD's static call counting (#44 lesson). +- **Verify positioning IN-CLIENT before recording.** A trace where + retail #1 can't see retail #2 is wasted bytes. Walk both characters + to within visible range, confirm the actor is rendered on the + observer's screen, THEN attach TTD. The 30-min validation experiment + on 2026-05-06 wasted its main probe because of this — don't repeat. +- **Outbound encoding quirk in our wire path** (CLAUDE.md "Outbound + motion wire format"): acdream sends `WalkForward (0x05) + HoldKey.Run` + for run, which ACE auto-upgrades to `RunForward (0x07)` when relaying + to remote observers. So the inbound parser sees `fwd=0x07` for + "remote is running." Don't get confused if outbound code shows 0x05 + but inbound shows 0x07 — that's normal ACE behavior. +- **TTD ring buffer with 512 MB max + ~60 sec recording** should fit + the whole scenario without rollover. If you record longer, consider + bumping to 1024 MB or chopping the scenario into multiple recordings. +- **Don't kill the TTD process** with Stop-Process — it kills retail + too. Use Ctrl+C in the TTD window or `TTD.exe -stop ` from + another elevated shell. + +--- + +## Definition of done + +This investigation is done when: + +1. The TTD trace + queries identify the exact retail function(s) that + refine the cycle from UP-derived velocity. Function name(s) + + address(es) documented in `docs/research/2026-05-06-locomotion-cycle-transitions/findings.md`. +2. The threshold/grace logic retail uses is documented (timing values, + conditions). +3. `ApplyServerControlledVelocityCycle` un-gating shipped in acdream + with the corresponding test and visual verification on all 7 phases. +4. ISSUES.md #39 closed with the commit SHA. +5. Memory `project_retail_motion_outbound.md` (or a new note for the + inbound side) gets the durable lessons appended. + +--- + +## Time estimate + +- Recording: ~30 min (setup, two retail clients, scenario execution) +- Initial query passes + decomp cross-reference: ~1–2 hours +- Implementation + iteration: ~2–4 hours +- Visual verification + commit: ~30 min + +Total: half a day to a full day depending on how clean retail's path is. + +If after the first TTD pass the retail receive code looks fundamentally +different from what `ApplyServerControlledVelocityCycle` is doing, stop +and reassess — the fix shape may need to change. Don't try to ram a +mismatched approach through. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 01ffc0d..3c2454f 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -383,6 +383,19 @@ public sealed class GameWindow : IDisposable /// public float MaxSeqSpeedSinceLastUP; + /// + /// Seconds-since-epoch timestamp of the most recent UpdateMotion (UM) + /// for this remote. Used by the player-remote velocity-fallback cycle + /// refinement to skip refinement while a fresh UM is authoritative — + /// retail's outbound MoveToState gives us direction-explicit cycles + /// on direction-key changes (W press, W release, W↔S flip), and we + /// only want UP-derived velocity to refine the speed bucket within + /// a direction when no UM has arrived recently. Defaults to 0 + /// (epoch) so the first UP after spawn is allowed to refine + /// immediately if velocity already differs from the spawn cycle. + /// + public double LastUMTime; + public RemoteMotion() { Body = new AcDream.Core.Physics.PhysicsBody @@ -2590,6 +2603,19 @@ public sealed class GameWindow : IDisposable if (!_entitiesByServerGuid.TryGetValue(update.Guid, out var entity)) return; if (!_animatedEntities.TryGetValue(entity.Id, out var ae)) return; + // #39 (2026-05-06): stamp the per-remote LastUMTime so the + // UP-velocity fallback path in ApplyServerControlledVelocityCycle + // can skip refinement while a UM is fresh. UMs are authoritative + // for direction-key changes (W press / release / W↔S flip); + // velocity refinement only helps for HoldKey-only changes (Shift + // toggle while a direction key is held — retail does NOT broadcast + // a fresh MoveToState in that case). + if (_remoteDeadReckon.TryGetValue(update.Guid, out var rmStateForUm)) + { + rmStateForUm.LastUMTime = + (System.DateTime.UtcNow - System.DateTime.UnixEpoch).TotalSeconds; + } + // Re-resolve using the new stance/command. Keep the setup and // motion-table we already know about — the server's motion // updates override state within the same table, not swap tables. @@ -3317,13 +3343,38 @@ public sealed class GameWindow : IDisposable return low is 0x05 or 0x06 or 0x07 or 0x0F or 0x10; } + /// + /// Grace window in seconds after a UM arrives during which UP-derived + /// velocity refinement is suppressed for a player remote. UMs are + /// authoritative; the velocity fallback only fills the gap when retail + /// does not send a fresh MoveToState (Shift toggle while direction key + /// held). 0.5 s is a defensible default — UPs arrive at ~5–10 Hz, so + /// a Shift toggle's first UP after the toggle is typically ~100–200 ms + /// after the most recent UM, well past the grace. + /// + private const double UmGraceSeconds = 0.5; + + /// + /// Speed (m/s) above which a player-remote currently in WalkForward + /// is promoted to RunForward by velocity refinement. Tuned to player + /// speeds: walk ≈ 3.12 m/s (WalkAnimSpeed × 1.0), run ≈ 8–12 m/s + /// (RunAnimSpeed × runRate ≈ 4.0 × 2.0–3.0). Hysteresis with + /// avoids thrashing at the boundary. + /// + private const float PlayerRunPromoteSpeed = 5.5f; + + /// + /// Speed (m/s) below which a player-remote currently in RunForward + /// is demoted to WalkForward by velocity refinement. + /// + private const float PlayerRunDemoteSpeed = 4.5f; + private void ApplyServerControlledVelocityCycle( uint serverGuid, AnimatedEntity ae, RemoteMotion rm, System.Numerics.Vector3 velocity) { - if (IsPlayerGuid(serverGuid)) return; if (rm.Airborne) return; if (ae.Sequencer is null) return; // MoveTo packets already seeded the retail speed/runRate cycle. @@ -3332,6 +3383,32 @@ public sealed class GameWindow : IDisposable // velocity-estimated animation. if (rm.ServerMoveToActive) return; + if (IsPlayerGuid(serverGuid)) + { + // #39 (2026-05-06): player-remote forward-direction speed-bucket + // refinement. The bug case: actor toggles Shift while holding W + // (or releases Shift). Retail's outbound apparently does NOT + // broadcast a fresh MoveToState for HoldKey-only changes + // (verified via static analysis of CommandInterpreter::SendMovementEvent + // call sites; needs cdb confirmation). ACE has nothing to + // broadcast → no UM arrives at the observer → cycle stays at + // whichever direction-bucket was last set. Velocity DOES change + // (UP carries new pace), so this code path uses UP-derived + // velocity to refine the speed bucket within the same direction. + // + // Conservative scope: + // - Forward direction only (low byte 0x05 or 0x07). Sidestep + // and backward HoldKey toggles are deferred until the TTD + // trace described in + // docs/research/2026-05-06-locomotion-cycle-transitions/ + // confirms retail's exact algorithm. + // - Hysteresis (4.5 m/s demote / 5.5 m/s promote) prevents + // thrashing at the boundary. + // - 500 ms UM grace window — a fresh UM is always authoritative. + ApplyPlayerLocomotionRefinement(serverGuid, ae, rm, velocity); + return; + } + var plan = AcDream.Core.Physics.ServerControlledLocomotion .PlanFromVelocity(velocity); uint currentMotion = ae.Sequencer.CurrentMotion; @@ -3344,10 +3421,7 @@ public sealed class GameWindow : IDisposable // D2 (Commit A 2026-05-03): UPCYCLE diag — proves whether // ApplyServerControlledVelocityCycle is racing UpdateMotion-driven - // SetCycle for player-driven remotes. If this fires every ~100-200ms - // during a Walk→Run press with `motion` flipping between buckets, - // H2 (UP-vs-UM race) is confirmed. UPs (5-10 Hz) would then - // perpetually overwrite the cycle the UM just set. + // SetCycle for non-player remotes (NPCs / monsters). if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") { System.Console.WriteLine( @@ -3361,6 +3435,103 @@ public sealed class GameWindow : IDisposable ae.Sequencer.SetCycle(style, plan.Motion, plan.SpeedMod); } + private void ApplyPlayerLocomotionRefinement( + uint serverGuid, + AnimatedEntity ae, + RemoteMotion rm, + System.Numerics.Vector3 velocity) + { + // UM grace: a fresh UM is authoritative. + double nowSec = (System.DateTime.UtcNow - System.DateTime.UnixEpoch).TotalSeconds; + double sinceUm = nowSec - rm.LastUMTime; + if (sinceUm < UmGraceSeconds) return; + + uint currentMotion = ae.Sequencer!.CurrentMotion; + uint lowByte = currentMotion & 0xFFu; + + // Forward-only refinement scope. WalkForward = 0x05, RunForward = 0x07. + // Sidestep (0x0F/0x10), WalkBackward (0x06), turns and any other + // motion (emote, attack, etc.) are left to UM-driven SetCycle. + const uint LowWalkForward = 0x05u; + const uint LowRunForward = 0x07u; + bool isForward = lowByte == LowWalkForward || lowByte == LowRunForward; + if (!isForward) return; + + float horizSpeed = MathF.Sqrt(velocity.X * velocity.X + velocity.Y * velocity.Y); + + // Hysteresis: stay in current bucket unless we cross the appropriate + // threshold. Below StopSpeed → don't refine (let UM Ready stop signal + // handle the stop transition; we don't want UP momentary 0-velocity + // to drop the cycle to Ready while the actor is mid-stride). + if (horizSpeed < AcDream.Core.Physics.ServerControlledLocomotion.StopSpeed) + return; + + uint targetMotion; + float speedMod; + if (lowByte == LowRunForward) + { + if (horizSpeed < PlayerRunDemoteSpeed) + { + targetMotion = AcDream.Core.Physics.MotionCommand.WalkForward; + speedMod = MathF.Min(MathF.Max( + horizSpeed / AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + } + else + { + targetMotion = AcDream.Core.Physics.MotionCommand.RunForward; + speedMod = MathF.Min(MathF.Max( + horizSpeed / AcDream.Core.Physics.MotionInterpreter.RunAnimSpeed, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + } + } + else + { + // currently WalkForward (0x05) + if (horizSpeed > PlayerRunPromoteSpeed) + { + targetMotion = AcDream.Core.Physics.MotionCommand.RunForward; + speedMod = MathF.Min(MathF.Max( + horizSpeed / AcDream.Core.Physics.MotionInterpreter.RunAnimSpeed, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + } + else + { + targetMotion = AcDream.Core.Physics.MotionCommand.WalkForward; + speedMod = MathF.Min(MathF.Max( + horizSpeed / AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + } + } + + // Skip the SetCycle if neither motion nor speedMod changed + // meaningfully — avoids replaying transition links every UP. + bool motionChanged = currentMotion != targetMotion + && (currentMotion & 0xFFu) != (targetMotion & 0xFFu); + bool speedChanged = MathF.Abs(ae.Sequencer.CurrentSpeedMod - speedMod) > 0.05f; + if (!motionChanged && !speedChanged) + return; + + if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") + { + System.Console.WriteLine( + $"[UPCYCLE_PLAYER] guid={serverGuid:X8} " + + $"|v|={horizSpeed:F2} cur=0x{currentMotion:X8} " + + $"-> motion=0x{targetMotion:X8} speedMod={speedMod:F2} " + + $"sinceUM={sinceUm:F2}s " + + $"motionChg={motionChanged} speedChg={speedChanged}"); + } + + uint style = ae.Sequencer.CurrentStyle != 0 + ? ae.Sequencer.CurrentStyle + : 0x8000003Du; + ae.Sequencer.SetCycle(style, targetMotion, speedMod); + } + private void OnLivePositionUpdated(AcDream.Core.Net.WorldSession.EntityPositionUpdate update) { // Phase A.1: track the most recently updated entity's landblock so the @@ -3685,15 +3856,20 @@ public sealed class GameWindow : IDisposable rmState.Body.Velocity = rmState.ServerVelocity; } - if (!IsPlayerGuid(update.Guid) - && rmState.HasServerVelocity + if (rmState.HasServerVelocity && _animatedEntities.TryGetValue(entity.Id, out var aeForVelocity)) { + // #39 (2026-05-06): un-gated for player remotes — the + // function itself routes player remotes into the dedicated + // ApplyPlayerLocomotionRefinement path (forward-direction + // speed bucket only, with UM grace + hysteresis). Non-player + // remotes use the existing PlanFromVelocity path. + // // D2 (Commit A 2026-05-03): tag whether the velocity feeding // ApplyServerControlledVelocityCycle is wire-explicit (rare for // player remotes — ACE almost never sets HasVelocity on player // UPs) or synthesized from position deltas (the common case). - // Pairs with the [UPCYCLE] line printed inside the call. + // Pairs with the [UPCYCLE]/[UPCYCLE_PLAYER] line printed inside. if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") { string velSrc = update.Velocity is null ? "synth" : "wire";