From 5660f3483dbc26d451d0e109a0306114460e6a9f Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 07:21:42 +0200 Subject: [PATCH] =?UTF-8?q?docs(motion):=20#39=20=E2=80=94=20candidate=20f?= =?UTF-8?q?ix=20ineffective;=20refute=20Shift-toggle=20wire=20hypothesis?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Visual-verify of commit 8fa04af in launch-39-candidate.log refutes the static-analysis hypothesis that retail does not broadcast UMs on HoldKey-only changes. The log shows: - [FWD_WIRE] for retail actor 0x50000001 contains many direct Walk↔Run transitions (0x44000007 ↔ 0x45000005). ACE IS sending UMs on Shift toggle. - [SETCYCLE] fires correctly per UM; Sequencer.CurrentMotion cycles through Walk / Run / Turn / Sidestep correctly per [UM_RAW]. - [UPCYCLE_PLAYER] never fired — UM grace correctly suppressed it (UMs at >2 Hz, well within 500 ms grace). - User reports legs visually stuck in walking animation despite the wire/sequencer saying Run. Conclusion: bug is downstream of Sequencer.CurrentMotion — same as 2026-05-03 hypothesis F. Most likely _currNode lands on the walk-to-run transition link after SetCycle (`currNodeIsCyclic=False` confirmed in [SCFULL]) and Advance does not progress past it to the cycle. The candidate fix code (LastUMTime, ApplyPlayerLocomotionRefinement, hysteresis constants, un-gated call site) is left in place — harmless because UM grace blocks the velocity-fallback path while UMs arrive, and the infrastructure may be useful for cases #2–#7 if those need velocity fallback. But it does not close case #1. Updates ISSUES.md #39 with refuted hypothesis + new evidence + next step pointer. findings-static.md gains "Visual-verify result" § documenting the diagnostic dump and recommending the next investigation target be AnimationSequencer.Advance queue-handling. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/ISSUES.md | 47 +++++++--- .../findings-static.md | 87 ++++++++++++++++++- 2 files changed, 119 insertions(+), 15 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 449b129..518253f 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -130,22 +130,41 @@ 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):** +**Progress 2026-05-06 — candidate fix INEFFECTIVE per 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`. +Candidate fix `8fa04af` shipped a velocity-fallback path +(`LastUMTime` + `ApplyPlayerLocomotionRefinement`), built on the +hypothesis that retail does not send a fresh MoveToState on +HoldKey-only changes. **That hypothesis is refuted.** Visual-verify +log (`launch-39-candidate.log`) shows: -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". +- `[FWD_WIRE]` for retail-driven actor `0x50000001` contains many + direct Walk↔Run UM transitions (`0x44000007 ↔ 0x45000005`). ACE + IS broadcasting UMs on Shift toggle. +- `[SETCYCLE]` fires correctly for each transition; `Sequencer.CurrentMotion` + cycles through Walk/Run/Turn/Sidestep at varied sample moments. +- `[UPCYCLE_PLAYER]` never fires (UM grace correctly suppresses it + while UMs arrive at >2 Hz). +- User report: "blips forward in walking animation" — the visible + cycle stays in Walk despite the wire / sequencer saying Run. + +So the bug is **downstream of `Sequencer.CurrentMotion`** — same as +2026-05-03 hypothesis F. Most likely `_currNode` lands on the +walk-to-run transition link after `SetCycle`, the link plays as a +walking-like pose, and `Advance` never advances past it to the +run cycle. `[SCFULL]` confirms `currNodeIsCyclic=False` after every +Walk↔Run transition. + +The candidate fix code is left in place — the infrastructure is +harmless (UM grace blocks all calls in normal flow) and may be +useful for cases #2–#7 follow-up if those turn out to need +velocity fallback. But it does NOT close case #1. + +**Next investigation step:** target `AnimationSequencer.Advance` +queue-handling for cyclic→cyclic direct transitions. See +[findings-static.md](research/2026-05-06-locomotion-cycle-transitions/findings-static.md) +§"Where the bug actually lives" + §"What to do next" for the +specific code paths and recommended diagnostic additions. ## #42 — [DONE 2026-05-05 · ec59a08] Airborne XY drift on observed player remote jumps (~1 m horizontal offset over arc) 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 index c75e24f..6358ac1 100644 --- a/docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md +++ b/docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md @@ -1,7 +1,7 @@ # 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. +**Status:** static analysis complete; **candidate fix INEFFECTIVE per visual verify** — see "Visual-verify result" at bottom; root cause is the same as 2026-05-03 hypothesis F (cycle stuck downstream of `Sequencer.CurrentMotion`). This is what code reading + ACE cross-reference + named-retail grep established before any TTD/cdb trace was run. It scopes the candidate @@ -165,3 +165,88 @@ above before any further fix attempt. No changes to `ServerControlledLocomotion.cs`, `MotionInterpreter.cs`, or `AnimationSequencer.cs`. + +--- + +## Visual-verify result (2026-05-06, evening) + +**The candidate fix did not switch the cycle.** User report: "if I shift +walk and then release shift it just blips forward in walking animation." + +Diagnostic dump from `launch-39-candidate.log` (filtered to the +retail-driven actor, guid `0x50000001`): + +| Diag | Result | +|---|---| +| `[FWD_WIRE]` | **Many direct Walk↔Run transitions present** — `oldCmd=0x44000007 → newCmd=0x45000005` and reverse. ACE IS broadcasting UMs on Shift toggle (refutes the static-analysis hypothesis that retail doesn't send MoveToState on HoldKey-only changes). | +| `[UM_RAW]` `seq.CurrentMotion` distribution for `0x50000001` | 0x41000003: 63, 0x45000005: 52, 0x6500000D: 31, 0x44000007: 20, 0x6500000F: 6 — i.e., the sequencer's `CurrentMotion` field DOES change to Walk / Run / Turn / Sidestep at various sample moments. | +| `[SETCYCLE]` | Many direct Walk↔Run SetCycle calls — `old=(motion=0x45000005…) new=(motion=0x44000007…)` — confirming the picker correctly resolves Run cycle from the inbound UM. | +| `[SCFULL]` | After Walk→Run SetCycle: `currNodeIsCyclic=False`, `qCount=12, 13, 14, … 41, 42, 49`. Queue grows steadily over ~30 transitions; `_currNode` lands on a non-cyclic transition link rather than the new cycle. | +| `[PARTSDIAG]` `seqHash` | 447 distinct values across 467 samples — animation IS being rendered, frames ARE changing. The cycle just isn't the right one. | +| `[UPCYCLE_PLAYER]` | **Never fired.** The 500 ms UM grace window blocks every call (UMs arrive at >2 Hz, well within the grace), which is the correct behavior — UMs are authoritative when fresh. | + +### What this proves + +1. **Wire is correct.** ACE delivers UMs on Shift toggle (refutes + hypothesis A from this doc, refutes the original 2026-05-06 + investigation-prompt's wire-level hypothesis). +2. **Dispatch is correct.** `OnLiveMotionUpdated` parses each UM, + resolves the right cycle, calls `AnimationSequencer.SetCycle`. +3. **Sequencer state updates.** `Sequencer.CurrentMotion` reaches the + new motion ID (Run, Walk, Turn, Sidestep all observed in `[UM_RAW]`). +4. **Visible animation does NOT match `CurrentMotion`.** This is exactly + the same residual symptom the 2026-05-03 investigation chased and + the SetCycle force-`_currNode` fix in commit `357dcc0` attempted but + did not fully resolve. + +### Where the bug actually lives + +Downstream of `Sequencer.CurrentMotion`, in one of: + +- `AnimationSequencer.Advance(dt)` consuming the wrong queue node +- `AnimationSequencer.BuildBlendedFrame()` reading from a stale + `_currNode` +- The transition link from old-cycle to new-cycle never advancing to + the cycle (link plays as the old motion) +- Queue-management gap: retail's `MotionTableManager::CheckForCompletedMotions` / + `remove_redundant_links` (named-retail addresses 0x0051BE00 / 0x0051BF20) + are not ported. Our `qCount=49` after ~30 transitions confirms + unbounded queue growth — but `_currNode` should still be on the most + recent SetCycle's first node, so this alone may not be the bug. + +The 2026-05-03 prompt's hypothesis F is the most likely: + +> `Advance` plays through stale link frames before reaching the cycle. +> Our `357dcc0` fix forces `_currNode` onto the first newly-enqueued +> node — but for `Walk→Run`, the newly-enqueued sequence is `[walk-to-run +> link, run cycle]`. `_currNode` lands on the **link**, the link plays +> for ~0.5–1 second, then the run cycle starts. User perceives the +> link's "transition pose" as "still walking." + +The SCFULL data (`currNodeIsCyclic=False` after every Walk↔Run direct +transition) is consistent with this — `_currNode` is on the link, +not the cycle, immediately after SetCycle. + +### What to do next + +The candidate fix code (`LastUMTime`, `ApplyPlayerLocomotionRefinement`, +hysteresis constants, un-gated call site) is **harmless but ineffective** +— left in place because: +- The infrastructure may be useful for cases #2–#7 if/when those turn + out to need velocity fallback. +- The UM grace window is correctly preventing it from interfering with + the working UM-driven path. +- Reverting would lose context tied to the findings doc. + +The next investigation should target **AnimationSequencer.Advance / queue +management for cyclic→cyclic direct transitions** — this is the same +problem 2026-05-03 left open. Recommended path: + +1. Add per-tick diag for `_currNode.Anim.Id` + `_framePosition` (per + the 2026-05-03 prompt's "Concrete next steps" §1). +2. Cross-reference retail's `CSequence::update` via cdb live attach OR + read it in named-retail (it's named — search `acclient_2013_pseudo_c.txt` + for `CSequence::update_internal`). +3. Compare retail's queue-handling for direct cyclic→cyclic with our + `AnimationSequencer.Advance` — particularly the conditions under which + `_currNode` advances past a link to the next cycle.