# Locomotion-cycle transitions — static-analysis findings

**Date:** 2026-05-06 (follow-up to `investigation-prompt.md`)
**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
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`.

---

## 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.