docs+cleanup: env-var regression + Run↔Walk cycle bug filed; re-throttle diags

End-of-session cleanup of the 2026-05-03 remote-motion debug session.

Documentation:

- CLAUDE.md: add ⚠️ DO-NOT-ENABLE warning for ACDREAM_INTERP_MANAGER=1
  in the diagnostic env-vars list. Add an "Outbound motion wire format"
  section documenting acdream's WalkForward+HoldKey.Run encoding (which
  ACE auto-upgrades to RunForward on relay) so future sessions don't
  re-derive it.

- docs/ISSUES.md: file two new issues:
  * #39 — Run↔Walk cycle transition not visible on observed
    retail-driven player remotes when watched from acdream. Root cause
    located: ApplyServerControlledVelocityCycle is gated by
    IsPlayerGuid, excluding the exact case where ACE doesn't broadcast
    a UM (shift toggle while direction key held). Fix sketch ~10
    lines, separate commit. Cross-references the four-agent
    investigation prompt.
  * #40 — ACDREAM_INTERP_MANAGER=1 env-var path regressed. Documents
    why (e94e791 conflated MoveOrTeleport with update_object), the
    visible symptoms (staircase Z, position blips), and why
    Commit B (039149a)'s ResolveWithTransition port was insufficient
    (env-var path also clears body.Velocity → no horizontal Euler
    motion → sweep input is queue catch-up only, which stair-steps).
    Fix path = separate L.3 follow-up to re-integrate PositionManager
    additively on top of the legacy chain.

Code:

- GameWindow.cs:6042: prepend a ⚠️ REGRESSED warning block at the top
  of the env-var per-frame branch so anyone reading the code is
  immediately aware not to enable it. Cross-references ISSUES.md #40.

- AnimationSequencer.cs: re-throttle [SCFAST]/[SCFULL] diagnostics to
  0.5s per instance (rolled back from A.1's unthrottled experiment).
  Already served its purpose; throttled is enough for steady-state
  diagnostics. Restore _lastSetCycleDiagTime field.

No behavior change for any current launch (env-var unset = legacy
path unchanged). Build green; baseline test failures (8) unchanged
from prior commit, none introduced by this session.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-04 10:10:10 +02:00
parent 039149a9d0
commit a3f53c2644
4 changed files with 238 additions and 24 deletions

View file

@ -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.42.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

View file

@ -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

View file

@ -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)
//

View file

@ -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;