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:
parent
039149a9d0
commit
a3f53c2644
4 changed files with 238 additions and 24 deletions
35
CLAUDE.md
35
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
|
||||
|
||||
|
|
|
|||
148
docs/ISSUES.md
148
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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
//
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue