From 8fa04af4c79a3c458db9132f16b54d85c5267de3 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 06:34:20 +0200 Subject: [PATCH 01/15] =?UTF-8?q?fix(motion):=20#39=20candidate=20?= =?UTF-8?q?=E2=80=94=20un-gate=20UP=20velocity-cycle=20for=20player=20remo?= =?UTF-8?q?tes=20(forward=20only)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a player-remote velocity-fallback path to ApplyServerControlledVelocityCycle so that when retail (the actor) toggles Shift while holding W and acdream is the observer, the visible leg cycle switches Run↔Walk within ~200–500 ms even though no fresh UM arrives. Static analysis (ACE GameActionMoveToState + MovementData.cs auto-upgrade + acdream's prior diag traces) suggests retail does NOT broadcast a fresh MoveToState on HoldKey-only changes — acdream's UMs handle direction-key changes and our local +Acdream's transitions, but retail-driven actors leave the cycle stuck. Changes (all in src/AcDream.App/Rendering/GameWindow.cs): - New RemoteMotion.LastUMTime field, stamped in OnLiveMotionUpdated - ApplyServerControlledVelocityCycle: removed inner IsPlayerGuid gate; routes player remotes to new ApplyPlayerLocomotionRefinement - ApplyPlayerLocomotionRefinement (forward-direction only): - 500 ms UM grace window (UMs win when fresh) - Forward-direction-only (low byte 0x05 / 0x07) - Hysteresis: Run → Walk demote at < 4.5 m/s; Walk → Run promote > 5.5 m/s - Skip SetCycle when neither motion ID nor speedMod changed meaningfully - [UPCYCLE_PLAYER] diag gated on ACDREAM_REMOTE_VEL_DIAG=1 - Outer call site in OnLivePositionUpdated un-gated (!IsPlayerGuid removed); per-remote routing now lives inside the function Scope: case #1 (Run↔Walk forward) only. Cases #2–#7 (backward, sidestep speed-buckets, direction-flips) remain deferred — PlanFromVelocity is forward-only and its NPC-tuned thresholds (RunThreshold=1.25) do not separate player Walk (~2.5 m/s) from player Run (~9 m/s); a TTD trace of retail's per-direction algorithm should ground the wider fix. ISSUES.md #39 updated with progress; investigation-prompt.md and a new findings-static.md committed under docs/research/2026-05-06-locomotion-cycle-transitions/ (the prompt was authored on a parallel branch in commit 7a38da3 and is brought into this worktree here so the next session can find it without branch-hopping). Build clean. The 8 pre-existing test failures on this branch (BSPStepUpTests.C3_Path6_AirborneMoverHitsSteepSlope, MotionInterpreter WalkBackward GetMaxSpeed, etc.) are unrelated to this change — verified by running them with the diff stashed. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/ISSUES.md | 23 ++ .../findings-static.md | 167 ++++++++++ .../investigation-prompt.md | 305 ++++++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 192 ++++++++++- 4 files changed, 679 insertions(+), 8 deletions(-) create mode 100644 docs/research/2026-05-06-locomotion-cycle-transitions/findings-static.md create mode 100644 docs/research/2026-05-06-locomotion-cycle-transitions/investigation-prompt.md diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 11173ada..449b129d 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 00000000..c75e24f3 --- /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 00000000..84b2c09d --- /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 01ffc0d8..3c2454f9 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"; From 5660f3483dbc26d451d0e109a0306114460e6a9f Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 07:21:42 +0200 Subject: [PATCH 02/15] =?UTF-8?q?docs(motion):=20#39=20=E2=80=94=20candida?= =?UTF-8?q?te=20fix=20ineffective;=20refute=20Shift-toggle=20wire=20hypoth?= =?UTF-8?q?esis?= 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 449b129d..518253f2 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 c75e24f3..6358ac1d 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. From 863d96bb23a608ce0fadfe4eb3fcb690caea43e2 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 07:28:21 +0200 Subject: [PATCH 03/15] =?UTF-8?q?fix(motion):=20#39=20=E2=80=94=20skip=20t?= =?UTF-8?q?ransition=20link=20for=20cyclic=E2=86=92cyclic=20locomotion=20i?= =?UTF-8?q?n=20SetCycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause identified by research-agent read of AnimationSequencer.SetCycle + Advance + the per-tick TickAnimations call site: - SetCycle enqueues transition link + new cycle, then forces _currNode onto firstNew (the LINK), per the 357dcc0 fix that pinned _currNode to the most-recently-enqueued node. - Advance plays the link to completion (~100–300 ms at Framerate 30 × link runSpeed) before AdvanceToNextAnimation moves _currNode forward to the cycle. - For Walk↔Run direct toggles faster than the link's drain time, the next UM arrives, SetCycle restarts _currNode on a fresh link, and the cycle node at the queue tail is never reached. - BuildBlendedFrame returns frames from the link the entire time — user observes the link's interpolation pose ("blips forward in walking animation"), never the new Walk or Run cycle. Confirmed by [SCFULL] currNodeIsCyclic=False after every direct Walk↔Run transition in launch-39-candidate.log. Fix: when prev motion AND new motion are both locomotion cycles (WalkForward, WalkBackward, RunForward, SideStep L/R), land _currNode on _firstCyclic (the new cycle node) instead of firstNew (the link), and remove the just-enqueued link from the queue. Conditional on BOTH being locomotion to avoid regressing cases that DO need the link to play: - Idle→Run (link is the wind-up pose) - Falling→Ready (landing animation) - Ready→Sitting/Crouching/Sleeping - Combat substates (attack/parry/ready transitions) Reverted commit c06b6c5 demonstrated that unconditional link skip breaks all of those — this fix is narrower. Retail reference: cdb live trace 2026-05-03 of a Walk→Run direct transition logged add_to_queue(45000005) followed by add_to_queue(44000007) with truncate_animation_list never firing — matching the observed semantics this fix implements. 42/42 AnimationSequencer tests pass. The 8 pre-existing test failures elsewhere on the branch (BSPStepUp, MotionInterpreter WalkBackward, etc.) are unrelated to this change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Physics/AnimationSequencer.cs | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/AcDream.Core/Physics/AnimationSequencer.cs b/src/AcDream.Core/Physics/AnimationSequencer.cs index 9687feac..56e03e43 100644 --- a/src/AcDream.Core/Physics/AnimationSequencer.cs +++ b/src/AcDream.Core/Physics/AnimationSequencer.cs @@ -525,7 +525,59 @@ public sealed class AnimationSequencer // newly-added node; if preEnqueueTail was null (queue was empty // before enqueue), the first new node is _queue.First. var firstNew = preEnqueueTail is null ? _queue.First : preEnqueueTail.Next; - if (firstNew is not null) + + // #39 Fix B (2026-05-06): for direct cyclic-locomotion → + // cyclic-locomotion transitions (Walk↔Run on Shift toggle, + // W↔S direct flip, A↔D, Forward↔Strafe), land _currNode on + // the new CYCLE (_firstCyclic), NOT on the link (firstNew), + // and remove the just-enqueued link from the queue. + // + // Why: the transition link's drain time (~100–300 ms at + // Framerate 30 × link runSpeed) gets restarted before it can + // end if the user toggles Shift faster than that. _currNode + // sits on a fresh link every UM and Advance never reaches + // the cycle. User observes "blips forward in walking + // animation" — what they're seeing is the link's + // interpolation pose, never the new cycle. + // + // Conditional on BOTH old AND new being locomotion cycles to + // avoid regressing the cases where the link IS the right + // animation: + // - Idle (Ready) → any cycle: link is the wind-up pose + // - Falling → Ready: landing animation + // - Ready → Sitting/Crouching: pose-change links + // - Combat substates (attack/parry/ready transitions) + // Commit c06b6c5 (reverted in a2ae2ae) demonstrated that + // unconditionally skipping the link breaks all of these. + // + // Retail reference: cdb live trace 2026-05-03 of a Walk→Run + // direct transition logged + // add_to_queue(45000005, looping=1) walk + // add_to_queue(44000007, looping=1) run + // with truncate_animation_list never firing — i.e. retail + // appends the new cycle directly without a separate link + // enqueue or visible link pose for cyclic→cyclic. Our + // structural mismatch was always enqueueing link+cycle and + // forcing _currNode onto the link; this fix matches retail's + // observed semantics for the locomotion subset. + bool prevIsLocomotion = IsLocomotionCycleLowByte(CurrentMotion & 0xFFu); + bool newIsLocomotion = IsLocomotionCycleLowByte(motion & 0xFFu); + if (prevIsLocomotion && newIsLocomotion && _firstCyclic is not null) + { + // Drop the just-enqueued link node (firstNew) from the + // queue if it's distinct from the cycle — nothing should + // ever play it, and leaving stale non-cyclic nodes ahead + // of _currNode contributes to the unbounded queue growth + // observed in [SCFULL] (qCount climbing past 49 over + // ~30 transitions). + if (firstNew is not null && firstNew != _firstCyclic) + { + _queue.Remove(firstNew); + } + _currNode = _firstCyclic; + _framePosition = _firstCyclic.Value.GetStartFramePosition(); + } + else if (firstNew is not null) { _currNode = firstNew; _framePosition = _currNode.Value.GetStartFramePosition(); @@ -1384,6 +1436,20 @@ public sealed class AnimationSequencer return result; } + /// + /// True if the given motion-low-byte names a locomotion cycle — + /// WalkForward (0x05), WalkBackward (0x06), RunForward (0x07), + /// SideStepRight (0x0F), or SideStepLeft (0x10). + /// Used by to recognise cyclic→cyclic + /// direct transitions and bypass the transition link in that case + /// (retail's observed add_to_queue semantics). + /// + private static bool IsLocomotionCycleLowByte(uint lowByte) + { + return lowByte == 0x05u || lowByte == 0x06u || lowByte == 0x07u + || lowByte == 0x0Fu || lowByte == 0x10u; + } + /// /// Quaternion slerp matching the retail client's FUN_005360d0 /// (chunk_00530000.c:4799-4846): From bb026b79914896c67bd94b6a84231a71d5592a14 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:17:56 +0200 Subject: [PATCH 04/15] =?UTF-8?q?diag(motion):=20#39=20=E2=80=94=20per-tic?= =?UTF-8?q?k=20[CURRNODE]=20for=20sequencer=20node=20identity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Visual-verify of fix #2 (commit 863d96b) showed [SCFULL] correctly reports currNodeIsCyclic=True after each direct Walk↔Run SetCycle (the link is removed and _currNode is set to _firstCyclic). User report still: - Animation continues running visually after Shift toggle to Walk - Body slows ("speed decreases"), causing rubber-banding - Adding a turn motion in that state makes the cycle finally transition to walking So either: - _currNode is reset to a stale node BETWEEN SetCycle and Advance - _currNode is correctly on the new cycle but its AnimRef is wrong (e.g., the same Animation as the previous cycle, dat-side issue) - BuildBlendedFrame reads from somewhere other than _currNode Adds CurrentNodeDiag + FirstCyclicAnimRefHash properties on AnimationSequencer that expose the active node's Animation identity-hash, IsLooping, Framerate, frame range, and FramePosition. TickAnimations logs them on every SEQSTATE tick (1 Hz throttle, gated on ACDREAM_REMOTE_VEL_DIAG=1). The [CURRNODE] line with animRef vs firstCyclicAnimRef proves whether _currNode is actually on the new cycle's anim or has drifted to something else. Compared across SetCycle SCFULL log lines + the following CURRNODE ticks, we can see the exact moment the cycle diverges from what SetCycle set. No code-behavior changes. Pure read-only instrumentation. Per Phase 4.5 of systematic-debugging — STOP attempting fixes; gather evidence first. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 17 ++++++++++ .../Physics/AnimationSequencer.cs | 33 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 3c2454f9..12828de3 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -6805,6 +6805,23 @@ public sealed class GameWindow : IDisposable System.Console.WriteLine( $"[SEQSTATE] guid={serverGuid:X8} CurrentMotion=0x{ae.Sequencer.CurrentMotion:X8} " + $"CurrentSpeedMod={ae.Sequencer.CurrentSpeedMod:F3}"); + + // #39 fix-3 evidence (2026-05-06): CURRNODE proves + // whether _currNode is actually on the cycle (anim + // ref hash matches FirstCyclic) or stuck somewhere + // else. SCFULL captures _currNode==_firstCyclic only + // at SetCycle return; this captures it per render + // tick so we can see if something resets it later. + var d = ae.Sequencer.CurrentNodeDiag; + int firstHash = ae.Sequencer.FirstCyclicAnimRefHash; + System.Console.WriteLine( + $"[CURRNODE] guid={serverGuid:X8} " + + $"animRef=0x{d.AnimRefHash:X8} firstCyclicAnimRef=0x{firstHash:X8} " + + $"isOnCyclic={d.AnimRefHash == firstHash && firstHash != 0} " + + $"isLooping={d.IsLooping} fr={d.Framerate:F2} " + + $"frame=[{d.StartFrame}..{d.EndFrame}] " + + $"pos={d.FramePosition:F2} qCount={d.QueueCount}"); + rmDiag.LastSeqStateLogTime = nowSec; } } diff --git a/src/AcDream.Core/Physics/AnimationSequencer.cs b/src/AcDream.Core/Physics/AnimationSequencer.cs index 56e03e43..4bae5088 100644 --- a/src/AcDream.Core/Physics/AnimationSequencer.cs +++ b/src/AcDream.Core/Physics/AnimationSequencer.cs @@ -254,6 +254,39 @@ public sealed class AnimationSequencer public int QueueCount => _queue.Count; public bool HasCurrentNode => _currNode != null; + /// + /// Diagnostic snapshot of _currNode's identity + frame state, for + /// the per-tick CURRNODE log line in GameWindow.TickAnimations. + /// Lets the caller see whether the actual node being read by Advance / + /// BuildBlendedFrame is what SetCycle was supposed to leave it on. + /// AnimRefHash uses object-identity hashing on the Animation reference + /// so different Walk vs Run anim resources can be distinguished even + /// without exposing the full Animation type. + /// + public (int AnimRefHash, bool IsLooping, double Framerate, int StartFrame, int EndFrame, double FramePosition, int QueueCount) CurrentNodeDiag + { + get + { + if (_currNode is null) + return (0, false, 0.0, 0, 0, 0.0, _queue.Count); + var n = _currNode.Value; + int hash = System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(n.Anim); + return (hash, n.IsLooping, n.Framerate, n.StartFrame, n.EndFrame, _framePosition, _queue.Count); + } + } + + /// + /// Diagnostic: the AnimRefHash for the FIRST cyclic node in the queue + /// (i.e., what SetCycle is trying to land us on for a locomotion cycle). + /// Compare against 's AnimRefHash to see + /// whether _currNode is actually pointing at the new cycle or + /// something stale. + /// + public int FirstCyclicAnimRefHash => + _firstCyclic is null + ? 0 + : System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(_firstCyclic.Value.Anim); + // ── Private state ──────────────────────────────────────────────────────── private readonly Setup _setup; From 2653b307c70e5779d149d540cf859473e85bb687 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:25:10 +0200 Subject: [PATCH 05/15] =?UTF-8?q?fix(motion):=20#39=20=E2=80=94=20wire=20A?= =?UTF-8?q?pplyServerControlledVelocityCycle=20into=20player-remote=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Visual verify with the proper Shift-toggle scenario revealed that fix #1's ApplyPlayerLocomotionRefinement was UNREACHABLE for player remotes — the L.3 M2 routing at line 3626+ returns at line 3755, BEFORE the call site at line 3879. The legacy NPC-only block that compute server velocity + calls ApplyServerControlledVelocityCycle never runs for players. [UPCYCLE_PLAYER] count = 0 in launch-39-fix2.log and launch-39-diag2.log proved the velocity-fallback path was completely dead code for players. Wire-level evidence (launch-39-diag2.log): - [FWD_WIRE] for retail actor 0x50000001 over a clean Hold-W-press-Shift- release-Shift-release-W test shows ONLY Ready→Run and Run→Ready transitions. NO Walk wire transitions for the Shift toggle. So retail's outbound MoveToState logic does NOT emit a fresh packet on HoldKey-only changes (refutes the launch-39-fix2 hypothesis that both directions emit; the earlier fix2 log's many Walk↔Run transitions came from W press/release cycles WITH Shift held continuously, not from Shift toggling alone). - [VEL_DIAG] over the same test shows clear walk-pace (~2.5 m/s) and run-pace (~11.5 m/s) periods, so the actor's actual physical speed IS changing despite the wire silence. Fix: in OnLivePositionUpdated's L.3 M2 player-remote block, after the near-enqueue / far-snap routing and before the early `return`, compute synth velocity from PrevServerPos / LastServerPos and call into ApplyServerControlledVelocityCycle. The function's internal routing (commit 8fa04af) sends player remotes through ApplyPlayerLocomotionRefinement which has the 500 ms UM grace + forward-direction + hysteresis logic to flip Run↔Walk only when no fresh UM is authoritative. Build clean. Diagnostics: [UPCYCLE_SRC] now prints `src=synth-player` when the player-remote path fires (distinct from `src=synth`/`src=wire` in the legacy NPC path). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 12828de3..4d7e278c 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -3745,6 +3745,51 @@ public sealed class GameWindow : IDisposable isMovingTo: false, currentBodyPosition: rmState.Body.Position); } + // #39 fix-3 (2026-05-06): velocity-fallback cycle refinement + // for player remotes. Wire-level evidence (`launch-39-diag2.log`): + // when retail's actor toggles Shift while a direction key + // is held, retail's outbound MoveToState logic does NOT + // emit a fresh packet (only Ready ↔ Run UMs visible in + // `[FWD_WIRE]`, despite a clear walk-pace ↔ run-pace + // velocity transition in `[VEL_DIAG]`). ACE has nothing + // to broadcast → no UM arrives at the observer → cycle + // sticks at whatever the last UM set. Compute the + // synth-velocity here in the player-remote path AND + // call into ApplyServerControlledVelocityCycle, which + // routes through the direction-preserving + UM-grace + // ApplyPlayerLocomotionRefinement helper (added in + // commit 8fa04af). + // + // The legacy non-player block below (3759+) covers NPCs + // and is gated `!IsPlayerGuid`; this block fills the + // matching gap for players. + if (rmState.PrevServerPosTime > 0.0) + { + double nowSecVel = rmState.LastServerPosTime; + double dtPos = nowSecVel - rmState.PrevServerPosTime; + if (dtPos > 0.001) + { + var synthVel = (worldPos - rmState.PrevServerPos) / (float)dtPos; + rmState.ServerVelocity = synthVel; + rmState.HasServerVelocity = true; + + if (_animatedEntities.TryGetValue(entity.Id, out var aeForVel) + && aeForVel.Sequencer is not null) + { + if (System.Environment.GetEnvironmentVariable("ACDREAM_REMOTE_VEL_DIAG") == "1") + { + System.Console.WriteLine( + $"[UPCYCLE_SRC] guid={update.Guid:X8} src=synth-player"); + } + ApplyServerControlledVelocityCycle( + update.Guid, + aeForVel, + rmState, + synthVel); + } + } + } + // Sync the visible entity to the body — overrides the unconditional // entity.Position = worldPos snap at the top of this function. // For the far-snap branch this is a no-op (body == worldPos); for From cc62e1cfde63e38aa4e1d510fd55d90e50a4e032 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:36:22 +0200 Subject: [PATCH 06/15] =?UTF-8?q?fix(motion):=20#39=20=E2=80=94=20handle?= =?UTF-8?q?=20backward=20sign=20+=20sidestep=20in=20ApplyPlayerLocomotionR?= =?UTF-8?q?efinement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report from fix #3 visual verify (commit 2653b30): - Forward Run↔Walk Shift toggle: WORKS now - Strafe Shift toggle: no transition (was out of scope) - "When I shift walk backwards, the retail char gets animated walking slow forward but blipping backwards" — REGRESSION Root cause of the backward regression: ACE encodes WalkBackward as `WalkForward` motion with NEGATIVE speedMod (MovementData.cs:115 `interpState.ForwardSpeed *= -0.65f`). My fix #1's hysteresis branches treated lowByte 0x05 / 0x07 as "forward" and computed positive speedMod from horizSpeed, overwriting the negative sign. Result: animation played forward-walk while body kept moving backward (the rubber-band). Strafe gap: sidestep (low byte 0x0F / 0x10) wasn't in fix #1's scope, so ApplyPlayerLocomotionRefinement returned early for sidestep cycles. Retail does the same wire-silence on Shift toggle for sidestep, so observer-side cycle refinement must also fire for it. Fix: - Probe `currentSign = sign(CurrentSpeedMod)` to detect backward direction - For sidestep (lowByte 0x0F or 0x10): keep motion ID, refine speedMod magnitude = horizSpeed / WalkAnimSpeed, preserve sign - For backward (forward-class lowByte AND currentSign < 0): keep WalkForward motion (per ACE encoding), refine magnitude, preserve negative sign — no "RunBackward" motion exists, only |speedMod| changes between Walk-back (~0.65) and Run-back (~1.91 = runRate × 0.65) - Forward (currentSign >= 0): existing Walk↔Run hysteresis unchanged Build clean. Diagnostics: [UPCYCLE_PLAYER] line still prints; the new sidestep / backward branches use the same SetCycle call so their decisions appear in [SCFULL] / [CURRNODE] for inspection. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 75 ++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 4d7e278c..7eb151c4 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -3448,14 +3448,24 @@ public sealed class GameWindow : IDisposable uint currentMotion = ae.Sequencer!.CurrentMotion; uint lowByte = currentMotion & 0xFFu; + float currentSign = MathF.Sign(ae.Sequencer.CurrentSpeedMod); + if (currentSign == 0f) currentSign = 1f; - // 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; + // Recognised locomotion directions: + // 0x05 (WalkForward) — also encodes WalkBackward via negative speed + // (ACE convention: SidestepCommand= cancel, ForwardCommand= + // WalkForward, ForwardSpeed *= -0.65) + // 0x07 (RunForward) + // 0x0F (SideStepRight) + // 0x10 (SideStepLeft) + // Other motions (Ready, Turn, emotes, attacks) are left to UM-driven SetCycle. + const uint LowWalkForward = 0x05u; + const uint LowRunForward = 0x07u; + const uint LowSideStepRight = 0x0Fu; + const uint LowSideStepLeft = 0x10u; + bool isForwardClass = lowByte == LowWalkForward || lowByte == LowRunForward; + bool isSidestep = lowByte == LowSideStepRight || lowByte == LowSideStepLeft; + if (!isForwardClass && !isSidestep) return; float horizSpeed = MathF.Sqrt(velocity.X * velocity.X + velocity.Y * velocity.Y); @@ -3468,7 +3478,54 @@ public sealed class GameWindow : IDisposable uint targetMotion; float speedMod; - if (lowByte == LowRunForward) + + if (isSidestep) + { + // Sidestep: motion ID stays the same (SideStepLeft / SideStepRight). + // Retail's wire encoding for sidestep speed buckets uses the same + // motion ID with different SidestepSpeed (slow ≈ 1.25 multiplier, + // fast ≈ 3.0 clamp per ACE MovementData.cs:124-131). On Shift + // toggle while a strafe key is held, retail does NOT broadcast a + // fresh MoveToState (same wire-silence rule as the forward case), + // so observer-side cycle refinement must come from UP-derived + // velocity here. Preserve the sign — SideStepLeft is sometimes + // emitted with negative speedMod by the adjust_motion path. + // + // Magnitude: horizSpeed / WalkAnimSpeed maps the observed speed + // back to a speedMod the sequencer can apply as a framerate + // multiplier. WalkAnimSpeed is the reasonable base because + // sidestep cycles use the WalkAnim equivalent (no separate + // RunSidestep cycle in the dat). + float sideMag = horizSpeed / AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed; + sideMag = MathF.Min(MathF.Max( + sideMag, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + targetMotion = currentMotion; + speedMod = sideMag * currentSign; + } + else if (currentSign < 0f) + { + // BACKWARD walk: ACE encodes WalkBackward as `WalkForward` motion + // with NEGATIVE speedMod (MovementData.cs:115 `interpState.ForwardSpeed *= -0.65f`). + // No "RunBackward" motion exists — Shift toggle on backward + // changes only the magnitude of speedMod (slow back ≈ -0.65, + // fast back ≈ -1.91 = -runRate × 0.65). Keep WalkForward motion, + // refine magnitude, preserve negative sign. + // + // Without this branch (the original fix #1), backward refinement + // computed a positive speedMod from horizSpeed and overwrote the + // negative sign, making the legs play forward-walk while the body + // continued moving backward (the rubber-banding the user reported). + float backMag = horizSpeed / AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed; + backMag = MathF.Min(MathF.Max( + backMag, + AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), + AcDream.Core.Physics.ServerControlledLocomotion.MaxSpeedMod); + targetMotion = AcDream.Core.Physics.MotionCommand.WalkForward; + speedMod = -backMag; + } + else if (lowByte == LowRunForward) { if (horizSpeed < PlayerRunDemoteSpeed) { @@ -3489,7 +3546,7 @@ public sealed class GameWindow : IDisposable } else { - // currently WalkForward (0x05) + // currently WalkForward (0x05) with positive speedMod = walking forward. if (horizSpeed > PlayerRunPromoteSpeed) { targetMotion = AcDream.Core.Physics.MotionCommand.RunForward; From 349ba65f3ec008cd371d883010415770b882145e Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:40:50 +0200 Subject: [PATCH 07/15] =?UTF-8?q?fix(motion):=20#39=20=E2=80=94=20use=20Si?= =?UTF-8?q?destepAnimSpeed=20(1.25)=20as=20sidestep=20mapping=20base?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #4 (commit cc62e1c) divided the observed horizSpeed by WalkAnimSpeed (3.12 m/s) when computing the sidestep speedMod. That made slow strafe come out 2.5× too slow because retail's sidestep cycle uses SidestepAnimSpeed (1.25 m/s) — a smaller base — per MotionInterpreter.cs:592 `velocity.X = SidestepAnimSpeed * SideStepSpeed`. User report: "Strafe left and right slowly now is SUPER slow :)". Replace MotionInterpreter.WalkAnimSpeed with MotionInterpreter.SidestepAnimSpeed in the sidestep branch only. Forward / backward branches continue using WalkAnimSpeed (correct for those motions). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 7eb151c4..588e4480 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -3491,12 +3491,14 @@ public sealed class GameWindow : IDisposable // velocity here. Preserve the sign — SideStepLeft is sometimes // emitted with negative speedMod by the adjust_motion path. // - // Magnitude: horizSpeed / WalkAnimSpeed maps the observed speed - // back to a speedMod the sequencer can apply as a framerate - // multiplier. WalkAnimSpeed is the reasonable base because - // sidestep cycles use the WalkAnim equivalent (no separate - // RunSidestep cycle in the dat). - float sideMag = horizSpeed / AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed; + // Magnitude: horizSpeed / SidestepAnimSpeed maps the observed + // speed back to a SideStepSpeed the sequencer can apply as a + // framerate multiplier. Retail's get_state_velocity for + // sidestep cycles is `velocity.X = SidestepAnimSpeed * + // SideStepSpeed` (MotionInterpreter.cs:592 — constant 1.25 + // m/s). Dividing by WalkAnimSpeed (3.12) here was wrong by + // 2.5× and made slow strafe play visibly slower than retail. + float sideMag = horizSpeed / AcDream.Core.Physics.MotionInterpreter.SidestepAnimSpeed; sideMag = MathF.Min(MathF.Max( sideMag, AcDream.Core.Physics.ServerControlledLocomotion.MinSpeedMod), From 69cdd7f492893fdb3d5fdabffc23b65fed471b1c Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:45:50 +0200 Subject: [PATCH 08/15] docs(issues): file #45 (local sidestep walk too slow); update #39 progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #45 — local +Acdream slow-strafe walking renders too slow. User observed during fix #5 visual verify of #39: the observer-side fix landed, then the user noticed the matching animation on the local player was also playing at sub-walk cadence. Likely the same SidestepAnimSpeed (1.25) vs WalkAnimSpeed (3.12) mismatch as fix #5 but on the local UpdatePlayerAnimation path. Filed for separate investigation. #39 — added "Progress 2026-05-06" section listing the five-commit fix sequence (8fa04af → 863d96b → bb026b7 → 2653b30 → cc62e1c → 349ba65), the wire-level finding that retail genuinely does NOT broadcast on Shift toggle (refuting the earlier confused trace analysis), the user-verified working cases (1/2/4/5) and the residual items (latency from 500ms UM grace, direction-flip cases 3/6/7 not yet explicitly verified). Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/ISSUES.md | 86 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 518253f2..f5c59cf4 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -130,41 +130,33 @@ 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 INEFFECTIVE per visual verify:** +**Progress 2026-05-06 — Shift-toggle cases (#1, #2, #4, #5) fixed; user-verified:** -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: +Five-commit sequence on this branch (`claude/determined-solomon-d0356d`): -- `[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. +| Commit | Effect | +|---|---| +| `8fa04af` | First candidate — added `RemoteMotion.LastUMTime` + `ApplyPlayerLocomotionRefinement` with 500 ms UM grace + forward-direction hysteresis. **Ineffective** because the call site lived in dead code for player remotes. | +| `863d96b` | Skip transition link in SetCycle for direct cyclic-locomotion → cyclic-locomotion. **Reduces queue accumulation** (qCount climbs slower); not the actual case-#1 fix but architecturally correct. | +| `bb026b7` | Per-tick `[CURRNODE]` diagnostic — exposed that `_currNode` was correctly tracking SetCycle's intent and so the bug was elsewhere. Read-only. | +| `2653b30` | **Wire `ApplyServerControlledVelocityCycle` into the L.3 M2 player-remote path.** Found via the diag — the existing call site at `OnLivePositionUpdated` line ~3879 was unreachable for players because the L.3 M2 routing returns at line 3755. New synth-velocity computation + call inserted in the player branch. **User-verified working** for forward Run↔Walk via Shift toggle. | +| `cc62e1c` | Handle backward (`CurrentSpeedMod < 0` → preserve negative sign) and sidestep (low byte 0x0F / 0x10 → keep motion ID, refine magnitude). Backward regression resolved. | +| `349ba65` | Use `SidestepAnimSpeed` (1.25) instead of `WalkAnimSpeed` (3.12) when computing sidestep magnitude — fix #4's mapping was 2.5× too small for slow strafe. | -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. +**Wire-level finding refuting the original ISSUES.md root-cause hypothesis: Earlier diagnostic claims that ACE broadcasts UMs on Shift toggle were misread.** A clean test (`launch-39-diag2.log`) holding W and toggling Shift while held shows `[FWD_WIRE]` for retail-driven actor only emitting `Ready ↔ Run` transitions — no Walk wire transitions at all, despite a clear walk-pace ↔ run-pace shift visible in `[VEL_DIAG]`. So retail's outbound DOES go silent on HoldKey-only changes. The earlier launch's many Walk↔Run `[FWD_WIRE]` lines came from W press/release cycles with Shift held continuously — different scenarios. -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. +**Verified working (user, 2026-05-06):** -**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. +- Forward Run↔Walk via Shift toggle (case #1) +- Backward Walk slow↔fast via Shift toggle (case #2) — animation matches direction, no rubber-band +- Strafe-left / strafe-right slow↔fast via Shift toggle (cases #4 / #5) — cadence visibly changes + +**Residual / not yet verified:** + +- "Not as fast as retail" — ~500 ms `UmGraceSeconds` window adds latency on top of the UP cadence (5–10 Hz). Could be tuned shorter once cases #3 / #6 / #7 are validated. +- Direction-flip cases (#3 W↔S, #6 A↔D, #7 W↔A/D) — believed to work via direct UM, not explicitly verified yet. + +**New related issue filed: #45** — local-player slow-strafe-walk renders too slow. Same `SidestepAnimSpeed` vs `WalkAnimSpeed` mismatch pattern as fix #5, but on the local-player render path (`UpdatePlayerAnimation`), not the observer side. ## #42 — [DONE 2026-05-05 · ec59a08] Airborne XY drift on observed player remote jumps (~1 m horizontal offset over arc) @@ -1266,6 +1258,40 @@ If hypothesis (a) is correct, this issue effectively rolls into **#28** — the --- +## #45 — Local +Acdream sidestep walking renders too slow + +**Status:** OPEN +**Severity:** LOW (visible animation cadence; not a correctness/wire bug) +**Filed:** 2026-05-06 +**Component:** physics / animation (local player path: `PlayerMovementController` → `UpdatePlayerAnimation` → `AnimationSequencer.SetCycle`) + +**Description:** When the local +Acdream character strafes (A or D held) at slow pace (no Shift), the visible leg cycle for the local player plays slower than retail's equivalent. Fast strafe (with Shift held) appears correct. Observed by user 2026-05-06 immediately after fix #5 (commit `349ba65`) landed the matching fix on the *observer-side* `ApplyPlayerLocomotionRefinement` for retail-driven remotes. + +**Root cause / status:** + +Likely the same constant mismatch as fix #5: the local player's sidestep speedMod was being computed off `WalkAnimSpeed` (3.12 m/s) where it should use `SidestepAnimSpeed` (1.25 m/s). Because the wire-emitted `SideStepSpeed` already encodes a 0.5× multiplier (per ACE `MovementData.cs:124-131`), dividing the wrong base on either send or render side compresses the slow strafe to a sub-walk cadence. + +To confirm: look at `UpdatePlayerAnimation` in `src/AcDream.App/Rendering/GameWindow.cs` (~line 7000) and trace where the strafe speedMod is sourced from. Either the speedMod passed to `SetCycle` is too small, or the framerate computation downstream applies an extra 0.5×. + +Cross-reference: +- `MotionInterpreter.SidestepAnimSpeed = 1.25f` +- `MotionInterpreter.WalkAnimSpeed = 3.12f` +- ACE `MovementData(MoveToState)`: `interpState.SidestepSpeed = speed * 3.12f / 1.25f * 0.5f` + +**Files:** + +- `src/AcDream.App/Rendering/GameWindow.cs` — `UpdatePlayerAnimation` (~line 7000) +- `src/AcDream.App/Input/PlayerMovementController.cs` — wire-builder for outbound MoveToState (sidestep path) +- `src/AcDream.Core/Physics/MotionInterpreter.cs:243-247` — anim-speed constants + +**Acceptance:** + +- Local +Acdream slow strafe (A or D, no Shift) plays at the same visible cadence as retail's slow strafe in a side-by-side comparison. +- Local fast strafe (A or D + Shift) does not regress. +- Forward / backward / run cycles do not regress. + +--- + --- # Recently closed From 898d7cd2cf3f514979bf1f93d19d7a0707573e4d Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:50:44 +0200 Subject: [PATCH 09/15] =?UTF-8?q?tune(motion):=20#39=20=E2=80=94=20tighten?= =?UTF-8?q?=20UmGraceSeconds=200.5=20=E2=86=92=200.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User observed the Shift-toggle cycle transition was "not as fast as retail" after fixes #3-#5 landed the velocity-fallback path. Worst-case added latency was the full 500 ms grace window before the first UP could refine the cycle. 200 ms covers the actual UM/UP race — UMs arrive on direction-key events, UPs at 5-10 Hz, so the first UP after a fresh UM lands ~100-200 ms behind it. Below that, fallback could prematurely overwrite a UM's cycle decision; above that adds latency for no correctness benefit. Direction flips (W↔S, A↔D, Forward↔Strafe) update via UM directly so they're unaffected by this change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 588e4480..81d27ae4 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -3348,11 +3348,14 @@ public sealed class GameWindow : IDisposable /// 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. + /// held). 200 ms covers the worst-case UM/UP race — UMs arrive on + /// direction-key events and UPs at 5–10 Hz, so the first UP after a + /// fresh UM lands ~100–200 ms behind. Tightened from 500 ms (commit + /// 8fa04af original) per user observation that the Shift-toggle + /// transition was visibly slower than retail; with 0.2 s the worst-case + /// added latency is just the UP cadence below it. /// - private const double UmGraceSeconds = 0.5; + private const double UmGraceSeconds = 0.2; /// /// Speed (m/s) above which a player-remote currently in WalkForward From e9e080db8c7ab5dbf2b5a1772f13e006eb0cd8b2 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 08:58:41 +0200 Subject: [PATCH 10/15] =?UTF-8?q?fix(motion):=20close=20#45=20=E2=80=94=20?= =?UTF-8?q?scale=20local=20sidestep=20speedMod=20by=20ACE's=20wire=20facto?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User observed (during fix #5 visual verify of #39): "our own Acdream client renders sidestep walking too slow". Filed as #45. Root cause: PlayerMovementController.cs:871 computes localAnimSpeed as the raw `runRate || 1.0`, while ACE's BroadcastMovement converts inbound MoveToState SidestepSpeed via speed × 3.12 / 1.25 × 0.5 (Network/Motion/MovementData.cs:124-131). Observer-side cycles play at the ACE-scaled value (~1.248 slow / ~3.0 fast clamped); the local cycle was playing at the raw 1.0 / runRate — about 80% of retail cadence for slow strafe. Fix: in UpdatePlayerAnimation, when animCommand is SideStepLeft / Right (low byte 0x0F or 0x10), multiply animSpeed by WalkAnimSpeed / SidestepAnimSpeed × 0.5 = 3.12 / 1.25 × 0.5 = 1.248 before calling SetCycle. Same factor as ACE; no clamp on the local side (sequencer handles MultiplyCyclicFramerate naturally). Forward / backward / turn cycles unchanged — those use WalkAnimSpeed or RunAnimSpeed as base, where localAnimSpeed = wire ForwardSpeed already produces the right cadence. Build clean. Visual verify pending: user reports slow-strafe cadence should match retail / our own observed-remote rendering after this. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 81d27ae4..0bae2428 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -7244,6 +7244,27 @@ public sealed class GameWindow : IDisposable // For everything else (Walk → Run, Run → Ready, etc.) we // keep the link so transitions stay smooth. bool skipLink = animCommand == AcDream.Core.Physics.MotionCommand.Falling; + + // #45 (2026-05-06): scale sidestep speedMod to match ACE's + // wire formula. PlayerMovementController hands us a raw + // localAnimSpeed (1.0 slow / runRate fast), but ACE's + // BroadcastMovement converts SidestepSpeed via + // `speed × 3.12 / 1.25 × 0.5` + // (Network/Motion/MovementData.cs:124-131). Without the + // matching multiplier here, the local sidestep cycle plays + // at speedMod = 1.0 while the observer-side cycle plays at + // ~1.248 — local strafe visibly slower than retail (user + // report at #45 close-out of #39). + // Factor = WalkAnimSpeed / SidestepAnimSpeed × 0.5 + // = 3.12 / 1.25 × 0.5 = 1.248. + uint cmdLow = animCommand & 0xFFu; + if (cmdLow == 0x0Fu /* SideStepRight */ || cmdLow == 0x10u /* SideStepLeft */) + { + animSpeed *= AcDream.Core.Physics.MotionInterpreter.WalkAnimSpeed + / AcDream.Core.Physics.MotionInterpreter.SidestepAnimSpeed + * 0.5f; + } + ae.Sequencer.SetCycle(fullStyle, animCommand, animSpeed * animScale, skipTransitionLink: skipLink); } From 24407fec3c3c616873e6935738cfc0990c503607 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 09:03:35 +0200 Subject: [PATCH 11/15] docs(issues): close #45 (sidestep slow); file #46 (retail observer of acdream blippy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #45 — closed by commit e9e080d. PlayerMovementController hands a raw localAnimSpeed (1.0 slow / runRate fast); UpdatePlayerAnimation now scales sidestep cycles by WalkAnimSpeed/SidestepAnimSpeed × 0.5 to match ACE's BroadcastMovement formula. User-verified. #46 — filed. Retail clients observing acdream's local +Acdream character see visibly blippy / laggy movement. Local acdream view of the same character is fine; acdream observing retail-driven characters is also fine (after #39 / #45). The degradation is specifically on the OUTBOUND path. Likely culprits ranked: AutoPos heartbeat cadence (acdream's fixed 200 ms is suspect per project_retail_motion_outbound memory), MoveToState send conditions, sequence counters, or absent HasVelocity on UPs. Verification approach documented (two retail clients + one acdream side-by-side; cdb breakpoint count of MovementManager::unpack_movement on retail observer). Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/ISSUES.md | 56 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index f5c59cf4..65777506 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -1258,37 +1258,61 @@ If hypothesis (a) is correct, this issue effectively rolls into **#28** — the --- -## #45 — Local +Acdream sidestep walking renders too slow +## #46 — Retail observer of acdream sees blippy / laggy movement **Status:** OPEN -**Severity:** LOW (visible animation cadence; not a correctness/wire bug) +**Severity:** MEDIUM (degrades external perception of acdream-driven characters) **Filed:** 2026-05-06 -**Component:** physics / animation (local player path: `PlayerMovementController` → `UpdatePlayerAnimation` → `AnimationSequencer.SetCycle`) +**Component:** net / motion (acdream's outbound path: `PlayerMovementController` → `MoveToState` (0xF61C) / `AutonomousPosition` heartbeat → ACE → retail observer) -**Description:** When the local +Acdream character strafes (A or D held) at slow pace (no Shift), the visible leg cycle for the local player plays slower than retail's equivalent. Fast strafe (with Shift held) appears correct. Observed by user 2026-05-06 immediately after fix #5 (commit `349ba65`) landed the matching fix on the *observer-side* `ApplyPlayerLocomotionRefinement` for retail-driven remotes. +**Description:** When viewing acdream's local +Acdream character through a parallel retail acclient.exe, the retail observer sees the character's movement as visibly blippy and laggy — position appears to step in discrete jumps rather than translating smoothly. The local acdream view of the same character looks fine, and acdream observing a retail-driven character (after #39 / #45) also looks fine. The degradation is specifically on the **outbound** side: what acdream sends to ACE for relay to other clients. **Root cause / status:** -Likely the same constant mismatch as fix #5: the local player's sidestep speedMod was being computed off `WalkAnimSpeed` (3.12 m/s) where it should use `SidestepAnimSpeed` (1.25 m/s). Because the wire-emitted `SideStepSpeed` already encodes a 0.5× multiplier (per ACE `MovementData.cs:124-131`), dividing the wrong base on either send or render side compresses the slow strafe to a sub-walk cadence. +Unverified. The likely culprits, ranked by suspected probability: -To confirm: look at `UpdatePlayerAnimation` in `src/AcDream.App/Rendering/GameWindow.cs` (~line 7000) and trace where the strafe speedMod is sourced from. Either the speedMod passed to `SetCycle` is too small, or the framerate computation downstream applies an extra 0.5×. +1. **AutonomousPosition heartbeat cadence.** `memory/project_retail_motion_outbound.md` notes acdream's fixed 200 ms heartbeat is a probable retail mismatch. Retail's `CommandInterpreter::SendPositionEvent` gates on transient_state (Contact + OnWalkable + valid Position) and may broadcast at a different cadence — fewer / more / variable. If acdream sends too rarely, observer dead-reckons too long between updates and visibly stutters when each AutoPos arrives. +2. **MoveToState send conditions.** `PlayerMovementController.cs:813-840` decides when a fresh MoveToState fires (state-change detection). If important transitions are missed (e.g., direction changes that don't flip ForwardCommand/SidestepCommand), the observer's last-known motion stays stale and AutoPos updates blip the body to the new authoritative position. +3. **InstanceSequence / ObjectMovement sequence counters.** ACE rejects out-of-order packets. If acdream's sequence stamping is off, ACE silently drops some packets; observer dead-reckons through the gap. +4. **Velocity field absent on AutoPos.** ACE relays UPs without HasVelocity for player characters (per `OnLivePositionUpdated` comment). Observer's dead-reckoning between UPs may extrapolate using stale velocity, producing visible position drift that snaps back on the next UP — exactly the blippy pattern. -Cross-reference: -- `MotionInterpreter.SidestepAnimSpeed = 1.25f` -- `MotionInterpreter.WalkAnimSpeed = 3.12f` -- ACE `MovementData(MoveToState)`: `interpState.SidestepSpeed = speed * 3.12f / 1.25f * 0.5f` +**Verification approach:** + +- Run two retail clients + one acdream client. Drive acdream; observe acdream's character on retail #1 and on retail #2 (both retail observers see the same wire). Compare to a retail-driven character observed from the same retail clients — does it look smooth there? If yes, the issue is acdream-outbound-specific. If both look blippy, it's something on the ACE side (less likely). +- cdb-attach a retail observer client and breakpoint `MovementManager::unpack_movement` to count UPs and UMs received per second from the acdream-driven character vs from another retail character. The cadence delta will identify which packet stream is misbehaving. +- Compare acdream's outbound packet timing against holtburger's `client/movement/system.rs` heartbeat logic — that's the closest known-working reference for how a non-retail client should pace its outbound. **Files:** -- `src/AcDream.App/Rendering/GameWindow.cs` — `UpdatePlayerAnimation` (~line 7000) -- `src/AcDream.App/Input/PlayerMovementController.cs` — wire-builder for outbound MoveToState (sidestep path) -- `src/AcDream.Core/Physics/MotionInterpreter.cs:243-247` — anim-speed constants +- `src/AcDream.App/Input/PlayerMovementController.cs` — outbound state-change detection + heartbeat +- `src/AcDream.Core.Net/WorldSession.cs` — sequence counters + send path +- `src/AcDream.Core.Net/Net/Outbound/...MoveToState.cs` / `AutonomousPosition.cs` — wire builders +- `references/holtburger/crates/holtburger-core/src/client/movement/system.rs` — reference cadence **Acceptance:** -- Local +Acdream slow strafe (A or D, no Shift) plays at the same visible cadence as retail's slow strafe in a side-by-side comparison. -- Local fast strafe (A or D + Shift) does not regress. -- Forward / backward / run cycles do not regress. +- Side-by-side comparison: retail observer of acdream-driven character and retail observer of retail-driven character look equally smooth during running, walking, sidestepping, turning, and stopping. +- No visible "step" pattern when acdream-driven character translates between AutoPos updates. + +**Cross-reference:** + +- `memory/project_retail_motion_outbound.md` — 2026-05-01 cdb live trace of retail's outbound (`CommandInterpreter::SendMovementEvent` for WASD, `Event_Jump` per-frame while charging). +- CLAUDE.md "Outbound motion wire format" — the `WalkForward + HoldKey.Run` ↔ `RunForward` auto-upgrade ACE applies on broadcast. + +--- + +## #45 — [DONE 2026-05-06 · e9e080d] Local +Acdream sidestep walking renders too slow + +**Status:** DONE +**Closed:** 2026-05-06 +**Commit:** `e9e080d` +**Component:** physics / animation (local player path: `UpdatePlayerAnimation`) + +**Resolution:** `PlayerMovementController.cs:871` computes `localAnimSpeed` as raw `runRate || 1.0`, but ACE's `BroadcastMovement` converts the inbound `MoveToState.SidestepSpeed` via `speed × 3.12 / 1.25 × 0.5` (`Network/Motion/MovementData.cs:124-131`). Observer-side cycles play at the ACE-scaled value (~1.248 slow / ~3.0 fast clamped); the local cycle was playing at the raw 1.0 / runRate — about 80% of retail cadence for slow strafe. + +`UpdatePlayerAnimation` now multiplies `animSpeed` by `WalkAnimSpeed / SidestepAnimSpeed × 0.5 = 1.248` when `animCommand` is `SideStepLeft / Right` (low byte 0x0F or 0x10). User-verified: local strafe cadence matches retail / observer-side rendering. + +**Original investigation note (preserved):** Same constant mismatch pattern as #39 fix #5 (commit `349ba65`) but on the local-player render path instead of the observer-side `ApplyPlayerLocomotionRefinement` — both fixed by aligning the speedMod base to ACE's wire formula. --- From e471527924c5d456f712a44c7c4593a53df7a1e2 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 10:46:14 +0200 Subject: [PATCH 12/15] feat(net): wire 0xF625 ObjDescEvent for live appearance updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retail-driven players observed from acdream rendered with stale appearance — wrong skin/hair palettes, missing clothing — because ACE's mid-session appearance broadcasts (equip/unequip/tailoring/ recipe/option-toggle) ride opcode 0xF625 ObjDescEvent and acdream silently dropped them. Initial CreateObject carries the appearance at spawn time, but every later equip change only updates via 0xF625 (per Skunkwors protocol docs in ACE/.../GameMessageObjDescEvent.cs). Retail handles via SmartBox::HandleObjDescEvent (named-retail 0x453340). Why: the retail observer sees the *server-relayed* view of remotes, not retail's local build, so dropping ObjDescEvent freezes appearance at the partial state in the first CreateObject. How: - Extract CreateObject's ModelData parsing into reusable CreateObject.ReadModelData(span, ref pos) returning (BasePaletteId, SubPalettes, TextureChanges, AnimPartChanges). - Add ObjDescEvent.cs (parser for 0xF625): body = u32 opcode | u32 guid | ModelData | u32 instanceSeq | u32 visualDescSeq. - WorldSession.AppearanceUpdated event + dispatcher branch. - GameWindow.OnLiveAppearanceUpdated splices new ModelData onto the cached spawn and replays via OnLiveEntitySpawned. The dedup at the start of OnLiveEntitySpawnedLocked tears down the old GPU/animated/ collision state cleanly before rebuild. - _lastSpawnByGuid cache populated at spawn-end and tracked through UpdatePosition so re-applies use current position (no pop-back to login spot on equip toggle). - ACDREAM_DUMP_APPEARANCE=1 env var prints structured SP/TC/APC decode for every 0xF625 — replaces the earlier raw-hex preview. - ACDREAM_DUMP_CLOTHING extended with setup.Parts.Count, flatten.Count, and per-part triangle counts for offline polygon-budget audit. Tests: 4 new ObjDescEvent tests (round-trip + parser drift guard); 269 net tests green. User-verified live: skin/hair colors match retail's character data; equip/unequip no longer pops position. Note: a separate "puffy arms / bulky body" geometry issue remains where base body parts visibly overlap clothing meshes — different root cause, tracked separately. --- src/AcDream.App/Rendering/GameWindow.cs | 73 ++++++++- src/AcDream.Core.Net/Messages/CreateObject.cs | 149 ++++++++++------- src/AcDream.Core.Net/Messages/ObjDescEvent.cs | 74 +++++++++ src/AcDream.Core.Net/WorldSession.cs | 54 ++++++- .../Messages/ObjDescEventTests.cs | 153 ++++++++++++++++++ 5 files changed, 441 insertions(+), 62 deletions(-) create mode 100644 src/AcDream.Core.Net/Messages/ObjDescEvent.cs create mode 100644 tests/AcDream.Core.Net.Tests/Messages/ObjDescEventTests.cs diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 0bae2428..00ff3ce6 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -682,6 +682,13 @@ public sealed class GameWindow : IDisposable /// private readonly Dictionary _entitiesByServerGuid = new(); private readonly Dictionary _liveEntityInfoByGuid = new(); + /// + /// Latest for each + /// guid. Captured at the end of so + /// can reuse the position/setup/motion + /// fields when a 0xF625 ObjDescEvent arrives carrying only updated visuals. + /// + private readonly Dictionary _lastSpawnByGuid = new(); private uint? _selectedTargetGuid; private readonly record struct LiveEntityInfo( string? Name, @@ -1476,6 +1483,7 @@ public sealed class GameWindow : IDisposable _liveSession.PositionUpdated += OnLivePositionUpdated; _liveSession.VectorUpdated += OnLiveVectorUpdated; _liveSession.TeleportStarted += OnTeleportStarted; + _liveSession.AppearanceUpdated += OnLiveAppearanceUpdated; // Phase 6c — PlayScript (0xF754) arrives from the server as // a (guid, scriptId) pair. Resolve the guid's current world @@ -1988,7 +1996,7 @@ public sealed class GameWindow : IDisposable && setup.Parts.Count >= 10; if (dumpClothing) { - Console.WriteLine($"\n=== DUMP_CLOTHING: guid=0x{spawn.Guid:X8} name='{spawn.Name}' setup=0x{setup.Id:X8} APC={animPartChanges.Count} ==="); + Console.WriteLine($"\n=== DUMP_CLOTHING: guid=0x{spawn.Guid:X8} name='{spawn.Name}' setup=0x{setup.Id:X8} setup.Parts.Count={setup.Parts.Count} flatten.Count={flat.Count} APC={animPartChanges.Count} ==="); foreach (var c in animPartChanges) Console.WriteLine($" APC part={c.PartIndex:D2} -> gfx=0x{c.NewModelId:X8}"); @@ -2158,14 +2166,27 @@ public sealed class GameWindow : IDisposable var scaleMat = System.Numerics.Matrix4x4.CreateScale(scale); var meshRefs = new List(); + int dumpClothingTotalTris = 0; for (int partIdx = 0; partIdx < parts.Count; partIdx++) { var mr = parts[partIdx]; var gfx = _dats.Get(mr.GfxObjId); - if (gfx is null) continue; + if (gfx is null) + { + if (dumpClothing) + Console.WriteLine($" EMIT part={partIdx:D2} gfx=0x{mr.GfxObjId:X8} GFXOBJ_DAT_MISSING -> 0 tris"); + continue; + } _physicsDataCache.CacheGfxObj(mr.GfxObjId, gfx); var subMeshes = AcDream.Core.Meshing.GfxObjMesh.Build(gfx, _dats); _staticMesh.EnsureUploaded(mr.GfxObjId, subMeshes); + if (dumpClothing) + { + int tris = 0; int subs = 0; + foreach (var sm in subMeshes) { tris += sm.Indices.Length / 3; subs++; } + dumpClothingTotalTris += tris; + Console.WriteLine($" EMIT part={partIdx:D2} gfx=0x{mr.GfxObjId:X8} subMeshes={subs} tris={tris}"); + } IReadOnlyDictionary? surfaceOverrides = null; if (resolvedOverridesByPart is not null && resolvedOverridesByPart.TryGetValue(partIdx, out var partOverrides)) @@ -2194,6 +2215,8 @@ public sealed class GameWindow : IDisposable $"(guid=0x{spawn.Guid:X8})"); return; } + if (dumpClothing) + Console.WriteLine($" TOTAL tris={dumpClothingTotalTris} meshRefs={meshRefs.Count} (parts.Count={parts.Count})"); // Build optional per-entity palette override from the server's base // palette + subpalette overlays. The renderer applies these to @@ -2241,6 +2264,10 @@ public sealed class GameWindow : IDisposable // UpdateMotion / UpdatePosition events can reseat this entity by guid. _entitiesByServerGuid[spawn.Guid] = entity; + // Cache the spawn so OnLiveAppearanceUpdated can replay it with new + // appearance fields when a later 0xF625 ObjDescEvent arrives. + _lastSpawnByGuid[spawn.Guid] = spawn; + // Commit B 2026-04-29 — live-entity collision registration. The // local player is the simulator (its PhysicsBody is the source of // truth for our own movement); only remotes register as targets. @@ -2470,6 +2497,40 @@ public sealed class GameWindow : IDisposable } } + /// + /// Server broadcast a 0xF625 ObjDescEvent — a creature/player's + /// appearance changed (equip / unequip / tailoring / recipe result / + /// character option toggle). The wire payload only carries the new + /// ModelData (palette + texture + animpart changes), not position or + /// motion, so we splice it onto the cached spawn and replay through + /// . The dedup at the start of + /// tears down the previous + /// rendering state (GpuWorldState entry, animated entity, collision + /// registration) before rebuilding. + /// + private void OnLiveAppearanceUpdated(AcDream.Core.Net.Messages.ObjDescEvent.Parsed update) + { + if (!_lastSpawnByGuid.TryGetValue(update.Guid, out var oldSpawn)) + { + // Server can broadcast ObjDescEvent before we've seen a + // CreateObject for this guid (race on landblock entry, or + // if the entity is in a state we couldn't render). Drop — + // when CreateObject lands, ACE includes the same ModelData + // body inside it, so the appearance won't be lost. + return; + } + + var md = update.ModelData; + var newSpawn = oldSpawn with + { + AnimPartChanges = md.AnimPartChanges, + TextureChanges = md.TextureChanges, + SubPalettes = md.SubPalettes, + BasePaletteId = md.BasePaletteId, + }; + OnLiveEntitySpawned(newSpawn); + } + /// /// Commit B 2026-04-29 — register a live (server-spawned) entity into /// the as a single collision body. @@ -2580,6 +2641,7 @@ public sealed class GameWindow : IDisposable _remoteLastMove.Remove(serverGuid); _liveEntityInfoByGuid.Remove(serverGuid); _entitiesByServerGuid.Remove(serverGuid); + _lastSpawnByGuid.Remove(serverGuid); if (_selectedTargetGuid == serverGuid) _selectedTargetGuid = null; @@ -3614,6 +3676,13 @@ public sealed class GameWindow : IDisposable var rot = new System.Numerics.Quaternion(p.RotationX, p.RotationY, p.RotationZ, p.RotationW); DumpMovementTruthServerEcho(update, worldPos); + // Keep the cached spawn's Position in sync with server truth so a + // later ObjDescEvent (which only carries new appearance, not new + // position) re-applies at the entity's CURRENT location instead of + // popping back to its login spot. See OnLiveAppearanceUpdated. + if (_lastSpawnByGuid.TryGetValue(update.Guid, out var cached)) + _lastSpawnByGuid[update.Guid] = cached with { Position = update.Position }; + // Capture the pre-update render position for the soft-snap residual // calculation below. Assign entity.Position to the server truth up // front; if we then compute a snap residual, we restore the rendered diff --git a/src/AcDream.Core.Net/Messages/CreateObject.cs b/src/AcDream.Core.Net/Messages/CreateObject.cs index 4b68d428..a5fcd7b7 100644 --- a/src/AcDream.Core.Net/Messages/CreateObject.cs +++ b/src/AcDream.Core.Net/Messages/CreateObject.cs @@ -269,6 +269,18 @@ public static class CreateObject /// public readonly record struct AnimPartChange(byte PartIndex, uint NewModelId); + /// + /// The ModelData block — palette/texture/animpart changes — that lives + /// inside both CreateObject (initial spawn) and ObjDescEvent (0xF625 + /// appearance update). Factored out so both sites parse the same wire + /// shape with one implementation. + /// + public readonly record struct ModelData( + uint? BasePaletteId, + IReadOnlyList SubPalettes, + IReadOnlyList TextureChanges, + IReadOnlyList AnimPartChanges); + /// /// Parse a reassembled CreateObject body. must /// start with the 4-byte opcode. Returns null if the body is @@ -310,64 +322,11 @@ public static class CreateObject uint guid = ReadU32(body, ref pos); - // --- ModelData --- - // Header: byte 0x11 marker, byte subPalettes, byte textureChanges, byte animPartChanges - if (body.Length - pos < 4) return null; - byte _marker = body[pos]; pos += 1; - byte subPaletteCount = body[pos]; pos += 1; - byte textureChangeCount = body[pos]; pos += 1; - byte animPartChangeCount = body[pos]; pos += 1; - - uint? basePaletteId = null; - if (subPaletteCount > 0) - basePaletteId = ReadPackedDwordOfKnownType(body, ref pos, PaletteTypePrefix); - - var subPalettes = subPaletteCount == 0 - ? (IReadOnlyList)Array.Empty() - : new SubPaletteSwap[subPaletteCount]; - for (int i = 0; i < subPaletteCount; i++) - { - uint subPalId = ReadPackedDwordOfKnownType(body, ref pos, PaletteTypePrefix); - if (body.Length - pos < 2) return null; - byte offset = body[pos]; pos += 1; - byte length = body[pos]; pos += 1; - ((SubPaletteSwap[])subPalettes)[i] = new SubPaletteSwap(subPalId, offset, length); - } - - var textureChanges = textureChangeCount == 0 - ? (IReadOnlyList)Array.Empty() - : new TextureChange[textureChangeCount]; - for (int i = 0; i < textureChangeCount; i++) - { - if (body.Length - pos < 1) return null; - byte partIndex = body[pos]; pos += 1; - uint oldTex = ReadPackedDwordOfKnownType(body, ref pos, SurfaceTextureTypePrefix); - uint newTex = ReadPackedDwordOfKnownType(body, ref pos, SurfaceTextureTypePrefix); - ((TextureChange[])textureChanges)[i] = new TextureChange(partIndex, oldTex, newTex); - } - - // Extract AnimPartChanges — the server uses these to replace - // base Setup parts with armored/statue/whatever-specific meshes. - // Without decoding these, characters render "naked" and custom - // weenies render as whatever their base Setup looks like. - // - // NOTE: ACE writes the NewModelId through WritePackedDwordOfKnownType - // with knownType=0x01000000 (GfxObj type prefix). That writer STRIPS - // the high-byte type if present before writing the PackedDword. We - // have to OR it back on read or our GfxObj dat lookup will fail - // (silently, producing no mesh refs — hence the Phase 4.7h regression). - var animParts = animPartChangeCount == 0 - ? (IReadOnlyList)Array.Empty() - : new AnimPartChange[animPartChangeCount]; - for (int i = 0; i < animPartChangeCount; i++) - { - if (body.Length - pos < 1) return null; - byte partIndex = body[pos]; pos += 1; - uint newModelId = ReadPackedDwordOfKnownType(body, ref pos, GfxObjTypePrefix); - ((AnimPartChange[])animParts)[i] = new AnimPartChange(partIndex, newModelId); - } - - AlignTo4(ref pos); + var modelData = ReadModelData(body, ref pos); + uint? basePaletteId = modelData.BasePaletteId; + var subPalettes = modelData.SubPalettes; + var textureChanges = modelData.TextureChanges; + var animParts = modelData.AnimPartChanges; // --- PhysicsData --- if (body.Length - pos < 8) return null; @@ -559,6 +518,80 @@ public static class CreateObject } } + /// + /// Read the ModelData block — palette swaps + texture overrides + + /// animation-part replacements — that lives inside both CreateObject + /// (initial spawn) and ObjDescEvent (0xF625 appearance update). + /// + /// Layout: byte marker (0x11), byte subPaletteCount, byte + /// textureChangeCount, byte animPartChangeCount. Then: + /// + /// BasePaletteId (PackedDword of palette type), only present when subPaletteCount > 0 + /// SubPalettes[subPaletteCount]: PackedDword id + byte offset + byte length + /// TextureChanges[textureChangeCount]: byte partIndex + PackedDword oldTex + PackedDword newTex + /// AnimPartChanges[animPartChangeCount]: byte partIndex + PackedDword newModelId + /// 4-byte alignment pad + /// + /// + /// Throws on truncated input — + /// callers wrap in try/catch and convert to a null result. Advances + /// past the alignment pad so the caller can + /// continue reading the next field. + /// + public static ModelData ReadModelData(ReadOnlySpan body, ref int pos) + { + if (body.Length - pos < 4) throw new FormatException("truncated ModelData header"); + byte _marker = body[pos]; pos += 1; + byte subPaletteCount = body[pos]; pos += 1; + byte textureChangeCount = body[pos]; pos += 1; + byte animPartChangeCount = body[pos]; pos += 1; + + uint? basePaletteId = null; + if (subPaletteCount > 0) + basePaletteId = ReadPackedDwordOfKnownType(body, ref pos, PaletteTypePrefix); + + var subPalettes = subPaletteCount == 0 + ? (IReadOnlyList)Array.Empty() + : new SubPaletteSwap[subPaletteCount]; + for (int i = 0; i < subPaletteCount; i++) + { + uint subPalId = ReadPackedDwordOfKnownType(body, ref pos, PaletteTypePrefix); + if (body.Length - pos < 2) throw new FormatException("truncated SubPaletteSwap"); + byte offset = body[pos]; pos += 1; + byte length = body[pos]; pos += 1; + ((SubPaletteSwap[])subPalettes)[i] = new SubPaletteSwap(subPalId, offset, length); + } + + var textureChanges = textureChangeCount == 0 + ? (IReadOnlyList)Array.Empty() + : new TextureChange[textureChangeCount]; + for (int i = 0; i < textureChangeCount; i++) + { + if (body.Length - pos < 1) throw new FormatException("truncated TextureChange"); + byte partIndex = body[pos]; pos += 1; + uint oldTex = ReadPackedDwordOfKnownType(body, ref pos, SurfaceTextureTypePrefix); + uint newTex = ReadPackedDwordOfKnownType(body, ref pos, SurfaceTextureTypePrefix); + ((TextureChange[])textureChanges)[i] = new TextureChange(partIndex, oldTex, newTex); + } + + // ACE writes NewModelId via WritePackedDwordOfKnownType(0x01000000) + // which strips the high-byte type if present before packing. + // ReadPackedDwordOfKnownType ORs it back on read. + var animParts = animPartChangeCount == 0 + ? (IReadOnlyList)Array.Empty() + : new AnimPartChange[animPartChangeCount]; + for (int i = 0; i < animPartChangeCount; i++) + { + if (body.Length - pos < 1) throw new FormatException("truncated AnimPartChange"); + byte partIndex = body[pos]; pos += 1; + uint newModelId = ReadPackedDwordOfKnownType(body, ref pos, GfxObjTypePrefix); + ((AnimPartChange[])animParts)[i] = new AnimPartChange(partIndex, newModelId); + } + + AlignTo4(ref pos); + return new ModelData(basePaletteId, subPalettes, textureChanges, animParts); + } + private static uint ReadU32(ReadOnlySpan source, ref int pos) { if (source.Length - pos < 4) throw new FormatException("truncated u32"); diff --git a/src/AcDream.Core.Net/Messages/ObjDescEvent.cs b/src/AcDream.Core.Net/Messages/ObjDescEvent.cs new file mode 100644 index 00000000..b6d966aa --- /dev/null +++ b/src/AcDream.Core.Net/Messages/ObjDescEvent.cs @@ -0,0 +1,74 @@ +using System.Buffers.Binary; + +namespace AcDream.Core.Net.Messages; + +/// +/// Inbound ObjDescEvent GameMessage (opcode 0xF625). ACE +/// broadcasts this whenever a creature/player's appearance changes after +/// the initial spawn — equip / unequip +/// (Creature_Equipment.cs:365), tailoring (Tailoring.cs:504), recipe +/// results (RecipeManager.cs:403), character-option toggles. Skunkwors +/// protocol docs: "F625: Change Model — Sent whenever a character changes +/// their clothes. It contains the entire description of what they're +/// wearing (and possibly their facial features as well). This message is +/// only sent for changes; when the character is first created, the body +/// of this message is included inside the creation message." +/// +/// Retail handles it via SmartBox::HandleObjDescEvent +/// (named-retail symbol 0x453340). acdream silently dropped it through +/// 2026-05-06 — the bug was that retail-driven characters observed from +/// acdream rendered with the wrong skin/hair palettes because the +/// follow-up appearance updates were never applied. +/// +/// Wire layout (ACE WorldObject_Networking.cs:48-54 +/// SerializeUpdateModelData): +/// +/// u32 opcode (0xF625) +/// u32 guid — target object +/// ModelData block — see +/// u32 instanceSequence +/// u32 visualDescSequence +/// +/// +public static class ObjDescEvent +{ + public const uint Opcode = 0xF625u; + + /// + /// One ObjDescEvent: target guid + the new ModelData. Sequence + /// counters are read but not surfaced (subscribers don't need them + /// — the event always carries the full new appearance). + /// + public readonly record struct Parsed(uint Guid, CreateObject.ModelData ModelData); + + /// + /// Parse an ObjDescEvent body (must start with the 4-byte opcode). + /// Returns null on truncation or wrong opcode. + /// + public static Parsed? TryParse(ReadOnlySpan body) + { + try + { + int pos = 0; + if (body.Length - pos < 4) return null; + uint opcode = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(pos)); + pos += 4; + if (opcode != Opcode) return null; + + if (body.Length - pos < 4) return null; + uint guid = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(pos)); + pos += 4; + + var modelData = CreateObject.ReadModelData(body, ref pos); + + // Trailing instanceSeq + visualDescSeq are read for completeness + // but not surfaced — subscribers re-render unconditionally on + // every event since each carries the full appearance. + return new Parsed(guid, modelData); + } + catch + { + return null; + } + } +} diff --git a/src/AcDream.Core.Net/WorldSession.cs b/src/AcDream.Core.Net/WorldSession.cs index 3e76509c..af7d6956 100644 --- a/src/AcDream.Core.Net/WorldSession.cs +++ b/src/AcDream.Core.Net/WorldSession.cs @@ -139,6 +139,20 @@ public sealed class WorldSession : IDisposable /// public event Action? TeleportStarted; + /// + /// Fires when the server broadcasts an ObjDescEvent (0xF625) — + /// a creature/player's appearance changed after the initial CreateObject + /// (equip / unequip / tailoring / recipe result / character option toggle). + /// Subscribers re-apply the new ModelData to the existing entity: + /// AnimPartChanges replace mesh refs, TextureChanges update per-part + /// surface texture overrides, and SubPalettes rebuild the palette + /// override (the channel that carries skin/hair tone). Without this, + /// retail-driven characters observed from acdream end up "stuck" at + /// whatever appearance was in their first CreateObject — see issue + /// notes in commit history around 2026-05-06. + /// + public event Action? AppearanceUpdated; + /// /// Phase H.1: fires when a local or ranged speech message (0x02BB / /// 0x02BC) is received. Subscribers typically feed these into a @@ -314,12 +328,18 @@ public sealed class WorldSession : IDisposable private readonly FragmentAssembler _assembler = new(); // Issue #5 diagnostics (env-var-gated): - // ACDREAM_DUMP_OPCODES=1 → log first occurrence of each unhandled opcode - // ACDREAM_DUMP_VITALS=1 → log every PrivateUpdateVital(Current) parse + // ACDREAM_DUMP_OPCODES=1 → log first occurrence of each unhandled opcode + // ACDREAM_DUMP_VITALS=1 → log every PrivateUpdateVital(Current) parse + // ACDREAM_DUMP_APPEARANCE=1 → log every 0xF625 ObjDescEvent + 0xF7DB UpdateObject + // with body len, target guid, hex preview. Used to + // debug remote-player appearance asymmetry (retail + // observer in acdream renders wrong skin/hair). private static readonly bool DumpOpcodesEnabled = Environment.GetEnvironmentVariable("ACDREAM_DUMP_OPCODES") == "1"; private static readonly bool DumpVitalsEnabled = Environment.GetEnvironmentVariable("ACDREAM_DUMP_VITALS") == "1"; + private static readonly bool DumpAppearanceEnabled = + Environment.GetEnvironmentVariable("ACDREAM_DUMP_APPEARANCE") == "1"; private readonly System.Collections.Generic.HashSet _seenUnhandledOpcodes = new(); private IsaacRandom? _inboundIsaac; @@ -861,6 +881,36 @@ public sealed class WorldSession : IDisposable _teleportSequence = sequence; // track for outbound movement messages TeleportStarted?.Invoke(sequence); } + else if (op == ObjDescEvent.Opcode) + { + // 0xF625 ObjDescEvent — per-entity appearance update. ACE + // broadcasts on equip/unequip/tailoring/recipe/option-change + // (Creature_Equipment.cs:365, Tailoring.cs:504, + // RecipeManager.cs:403, GameActionSetSingleCharacterOption.cs:27). + // Retail handler: SmartBox::HandleObjDescEvent (named-retail + // 0x453340). Body layout: u32 opcode | u32 guid | ModelData | + // u32 instanceSeq | u32 visualDescSeq. + var parsed = ObjDescEvent.TryParse(body); + if (parsed is not null) + { + if (DumpAppearanceEnabled) + { + var md = parsed.Value.ModelData; + Console.WriteLine($"appearance: 0xF625 guid=0x{parsed.Value.Guid:X8} basePal=0x{(md.BasePaletteId ?? 0):X8} subPals={md.SubPalettes.Count} texChanges={md.TextureChanges.Count} animParts={md.AnimPartChanges.Count}"); + foreach (var sp in md.SubPalettes) + Console.WriteLine($" SP id=0x{sp.SubPaletteId:X8} offset={sp.Offset} length={sp.Length}"); + foreach (var tc in md.TextureChanges) + Console.WriteLine($" TC part={tc.PartIndex:D2} oldTex=0x{tc.OldTexture:X8} -> newTex=0x{tc.NewTexture:X8}"); + foreach (var apc in md.AnimPartChanges) + Console.WriteLine($" APC part={apc.PartIndex:D2} -> gfx=0x{apc.NewModelId:X8}"); + } + AppearanceUpdated?.Invoke(parsed.Value); + } + else if (DumpAppearanceEnabled) + { + Console.WriteLine($"appearance: 0xF625 PARSE FAILED body.len={body.Length}"); + } + } else if (DumpOpcodesEnabled) { // ACDREAM_DUMP_OPCODES=1 — emit a one-line trace per diff --git a/tests/AcDream.Core.Net.Tests/Messages/ObjDescEventTests.cs b/tests/AcDream.Core.Net.Tests/Messages/ObjDescEventTests.cs new file mode 100644 index 00000000..5610dc2f --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/Messages/ObjDescEventTests.cs @@ -0,0 +1,153 @@ +using System.Buffers.Binary; +using AcDream.Core.Net.Messages; + +namespace AcDream.Core.Net.Tests.Messages; + +public sealed class ObjDescEventTests +{ + [Fact] + public void TryParse_RejectsWrongOpcode() + { + byte[] body = new byte[16]; + BinaryPrimitives.WriteUInt32LittleEndian(body, 0xF745u); // CreateObject opcode + Assert.Null(ObjDescEvent.TryParse(body)); + } + + [Fact] + public void TryParse_RejectsTruncatedBody() + { + Assert.Null(ObjDescEvent.TryParse(new byte[3])); + } + + /// + /// Round-trip a synthesized body: opcode + guid + ModelData (3 SubPalettes, + /// 4 TextureChanges, 0 AnimPartChanges — same shape as the captured retail + /// 152-byte +Je ObjDescEvent body) + trailing sequence pair. Verifies the + /// parser surfaces the same fields the writer wrote. + /// + [Fact] + public void TryParse_SynthesizedBody_ExtractsGuidAndModelData() + { + // Build a body matching the wire shape we see from ACE. + var bytes = new List(); + AppendU32(bytes, ObjDescEvent.Opcode); + AppendU32(bytes, 0x50000001u); // target guid + + // ModelData header: marker, subPalCount, texCount, animPartCount. + bytes.Add(0x11); + bytes.Add(3); // subPalCount + bytes.Add(4); // texChangeCount + bytes.Add(0); // animPartCount + + // BasePaletteId (palette type prefix stripped before packing). + AppendPackedDword(bytes, 0x0400007Eu, 0x04000000u); + + // SubPalettes — three skin/hair-style overlays at varied offsets. + AppendPackedDword(bytes, 0x04001FE3u, 0x04000000u); + bytes.Add(24); bytes.Add(8); + AppendPackedDword(bytes, 0x040002BAu, 0x04000000u); + bytes.Add(0); bytes.Add(24); + AppendPackedDword(bytes, 0x040002BCu, 0x04000000u); + bytes.Add(32); bytes.Add(8); + + // TextureChanges — four part textures. + for (byte partIdx = 0; partIdx < 4; partIdx++) + { + bytes.Add(partIdx); + AppendPackedDword(bytes, 0x05000100u + partIdx, 0x05000000u); + AppendPackedDword(bytes, 0x05000200u + partIdx, 0x05000000u); + } + + // 4-byte align after AnimPartChanges (none here, so just align). + while (bytes.Count % 4 != 0) bytes.Add(0); + + // Trailing instance + visual-desc sequences (consumed but ignored). + AppendU32(bytes, 0x12345678u); + AppendU32(bytes, 0x9ABCDEF0u); + + var parsed = ObjDescEvent.TryParse(bytes.ToArray()); + + Assert.NotNull(parsed); + Assert.Equal(0x50000001u, parsed!.Value.Guid); + + var md = parsed.Value.ModelData; + Assert.Equal(0x0400007Eu, md.BasePaletteId); + Assert.Equal(3, md.SubPalettes.Count); + Assert.Equal(0x04001FE3u, md.SubPalettes[0].SubPaletteId); + Assert.Equal(24, md.SubPalettes[0].Offset); + Assert.Equal(8, md.SubPalettes[0].Length); + Assert.Equal(0x040002BAu, md.SubPalettes[1].SubPaletteId); + Assert.Equal(0, md.SubPalettes[1].Offset); + Assert.Equal(24, md.SubPalettes[1].Length); + + Assert.Equal(4, md.TextureChanges.Count); + Assert.Equal(0, md.TextureChanges[0].PartIndex); + Assert.Equal(0x05000100u, md.TextureChanges[0].OldTexture); + Assert.Equal(0x05000200u, md.TextureChanges[0].NewTexture); + Assert.Equal(3, md.TextureChanges[3].PartIndex); + + Assert.Empty(md.AnimPartChanges); + } + + /// + /// Confirms ReadModelData (the shared helper) round-trips identically + /// when called from CreateObject and from ObjDescEvent — same bytes, + /// same parsed output. Guards against the two callers drifting. + /// + [Fact] + public void ReadModelData_SameOutputFromBothCallers() + { + // Bare ModelData block — used as a substring in both messages. + var modelDataBytes = new List(); + modelDataBytes.Add(0x11); + modelDataBytes.Add(1); // subPalCount + modelDataBytes.Add(0); // texCount + modelDataBytes.Add(0); // animPartCount + AppendPackedDword(modelDataBytes, 0x0400007Eu, 0x04000000u); + AppendPackedDword(modelDataBytes, 0x04001084u, 0x04000000u); + modelDataBytes.Add(80); modelDataBytes.Add(12); + while (modelDataBytes.Count % 4 != 0) modelDataBytes.Add(0); + + ReadOnlySpan span = modelDataBytes.ToArray(); + int pos = 0; + var md = CreateObject.ReadModelData(span, ref pos); + + Assert.Equal(0x0400007Eu, md.BasePaletteId); + Assert.Single(md.SubPalettes); + Assert.Equal(0x04001084u, md.SubPalettes[0].SubPaletteId); + Assert.Equal(80, md.SubPalettes[0].Offset); + Assert.Equal(12, md.SubPalettes[0].Length); + } + + private static void AppendU32(List dest, uint value) + { + Span tmp = stackalloc byte[4]; + BinaryPrimitives.WriteUInt32LittleEndian(tmp, value); + dest.AddRange(tmp.ToArray()); + } + + /// + /// Mirror of ACE's WritePackedDwordOfKnownType: strip the type prefix + /// if it matches , then write as a 16- or + /// 32-bit packed value. + /// + private static void AppendPackedDword(List dest, uint value, uint knownType) + { + uint packed = (value & 0xFF000000u) == knownType ? (value & ~knownType) : value; + if (packed <= 0x7FFFu) + { + Span tmp = stackalloc byte[2]; + BinaryPrimitives.WriteUInt16LittleEndian(tmp, (ushort)packed); + dest.AddRange(tmp.ToArray()); + } + else + { + ushort high = (ushort)((packed >> 16) | 0x8000); + ushort low = (ushort)(packed & 0xFFFFu); + Span tmp = stackalloc byte[4]; + BinaryPrimitives.WriteUInt16LittleEndian(tmp, high); + BinaryPrimitives.WriteUInt16LittleEndian(tmp.Slice(2), low); + dest.AddRange(tmp.ToArray()); + } + } +} From e697a9ad1ee6ffac77c51d6685893ad9499a2cdd Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 11:30:41 +0200 Subject: [PATCH 13/15] docs(issues): file #47 (humanoid bulky-shape bug); land DUMP_CLOTHING diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filed #47 in docs/ISSUES.md — humanoid characters using Setup 0x02000001 (players + Woodsman + other Aluvian NPCs) render visibly bulkier and less shape-defined than retail's view. Drudges and other monster setups render identically. Independent of equipment (naked +Je still shows it). Investigation this session ruled out 0xF625 ObjDescEvent drops (real bug, fixed in e471527, but doesn't explain shape), HiddenParts overlap, ParentIndex walking (animation frames are setup-root coords already), and player-specific data flow (NPCs using same setup affected too). Diagnostic infrastructure landed alongside the issue (env-var-gated, no runtime cost when off): - ACDREAM_DUMP_CLOTHING=1 now also prints: - setup.Parts.Count, flatten.Count, APC count on header - ParentIndex[] and DefaultScale[] arrays - IdleFrame per-part Origin + Orientation (first 17 parts) - per-part EMIT line: gfx, subMeshes count, triangle count - TOTAL triangle / meshRef counts per entity This is what nailed down "all 34 parts emit" + "animation frames are setup-root not parent-local" + "humans get setup-wide 180°-Z rotation that drudges don't" — saved hours next session. Open hypotheses for #47 next session: per-face vs smoothed vertex normals (per-vertex normals from dat may be face-style for human GfxObjs but smooth for monsters), low cell ambient leaving back faces flat-shadowed, missing MSAA on the GL window. --- docs/ISSUES.md | 98 +++++++++++++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 27 +++++++ 2 files changed, 125 insertions(+) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 65777506..98e44253 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -1258,6 +1258,104 @@ If hypothesis (a) is correct, this issue effectively rolls into **#28** — the --- +## #47 — Humanoid Setup 0x02000001 renders bulky / lacks shape detail vs retail + +**Status:** OPEN +**Severity:** MEDIUM (cosmetic — characters readable but visibly different from retail) +**Filed:** 2026-05-06 +**Component:** rendering / mesh / character animation + +**Description:** Every humanoid character using Setup `0x02000001` +(Aluvian Male) renders in acdream with a "bulky, less-defined" silhouette +compared to retail's view of the same character. Specifically: shoulders +look smoother/rounder where retail has pointier shoulder pads; back has +less contour; arms appear puffier. The effect is identical for player +characters (`+Acdream`, `+Je`) and for humanoid NPCs using the same +setup (e.g. Woodsman, Sedor Wystan the Blacksmith, Thelnoth Cort). +Drudges and other monster setups (e.g. `0x020007DD`) render +identically to retail, so this is *not* a pipeline-wide bug. + +The bug is independent of equipment — `+Je` stripped naked still +shows the same bulky silhouette. + +**Investigation 2026-05-06 (~3 hr session, ruled out many hypotheses):** + +What was ruled out: + +- **0xF625 ObjDescEvent appearance updates being dropped.** Was a real + bug for skin/hair colors; fixed in commit e471527. Does not affect + the bulky-shape issue (which persists with the fix in place and + with no equipment). +- **Position-pop on equip toggle.** Caused by re-applying with cached + spawn's stale position; fixed in same commit. Doesn't affect shape. +- **Clothing/armor overlapping the base body** (HiddenParts hypothesis). + User stripped naked; bulky shape persists. +- **ParentIndex hierarchy not walked in `SetupMesh.Flatten`.** Setup + `0x02000001` has a real hierarchy (`-1, -1, 1, 2, 3, -1, 5, 6, 7, 0, + 9, 10, 11, 12, 13, 14, 15, 0, ...`), but implementing parent-walk + produced **no visible change** — confirming AC's idle animation + frames are already in setup-root coordinates, not parent-local. +- **Equipment / wielded items.** No equipment on `+Je` and bug persists. +- **Player-specific data flow.** Humanoid NPCs using same setup + (Woodsman) show same bug. + +What was confirmed (data captured via `ACDREAM_DUMP_CLOTHING=1`): + +- Setup `0x02000001`: `setup.Parts.Count = 34`, `flatten.Count = 34`, + `APC = 34..38` depending on equipment. +- All 34 parts emit triangles successfully (no silent GfxObj load + failures). Total ~648-700 tris per character. +- Idle animation frames place parts at sensible humanoid Z-heights + (head Z=1.587, mid-body Z=0.5-1.0, ground Z=0.085). +- Per-part orientations are nearly all 180° around -Z (W≈0, + Z≈-1) — a setup-wide coordinate-flip convention. Drudges have + varied per-part orientations. +- `setup.DefaultScale.Count = 0` for both humans and drudges → all + parts use Vector3.One scale. + +**Working hypotheses (next session):** + +1. **Per-vertex normal style.** AC dat may store per-face normals + for human GfxObjs (one normal per polygon, copied to all 3 + vertices) but smooth normals for monster GfxObjs. acdream uses + dat normals directly. Test by computing smooth normals from face + adjacency and comparing render. User said "not shaders" but the + screenshots clearly show smooth-vs-faceted lighting differences. +2. **Lighting setup.** Cell ambient may be too low, leaving back- + facing surfaces in flat shadow. Compare `uCellAmbient` value + against retail's behaviour at the same time-of-day. +3. **Anti-aliasing.** Retail may use MSAA; acdream window may not. + Polygon edges in acdream would be visibly stair-stepped, reading + as "more faceted" / blockier. +4. **Surface flags interpretation.** Specific Surface.Type bits for + character textures (skin, fabric) may need handling acdream + doesn't yet do (e.g. `SmoothShade` flag, or a mip bias). + +**Diagnostic infrastructure landed this session** (env-var-gated, no +runtime cost when off): + +- `ACDREAM_DUMP_CLOTHING=1` extended: + - `setup.Parts.Count`, `flatten.Count`, `APC` count on header line + - `ParentIndex[]` array dump + - `DefaultScale[]` array dump + - `IdleFrame.Frames[]` per-part Origin + Orientation (first 17 parts) + - `EMIT part=NN gfx=0xXX subMeshes=N tris=N` per part + - `TOTAL tris=N meshRefs=N` per entity + +**Files (suspect surface area for next investigation):** + +- `src/AcDream.Core/Meshing/SetupMesh.cs` — Flatten composition +- `src/AcDream.Core/Meshing/GfxObjMesh.cs` — polygon emission + + vertex normal handling (line 142) +- `src/AcDream.App/Rendering/Shaders/mesh.frag` — lighting eq +- `src/AcDream.App/Rendering/Shaders/mesh.vert` — normal transform + +**Acceptance:** Side-by-side screenshots of `+Acdream` (or any humanoid +NPC using `0x02000001`) viewed from the same angle in acdream and +retail show matching silhouette and shape definition. + +--- + ## #46 — Retail observer of acdream sees blippy / laggy movement **Status:** OPEN diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 00ff3ce6..726f829b 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -1997,6 +1997,33 @@ public sealed class GameWindow : IDisposable if (dumpClothing) { Console.WriteLine($"\n=== DUMP_CLOTHING: guid=0x{spawn.Guid:X8} name='{spawn.Name}' setup=0x{setup.Id:X8} setup.Parts.Count={setup.Parts.Count} flatten.Count={flat.Count} APC={animPartChanges.Count} ==="); + // Dump the Setup's ParentIndex + DefaultScale arrays to verify hierarchy. + var parentStr = string.Join(",", setup.ParentIndex.Take(Math.Min(34, setup.ParentIndex.Count)).Select(p => p == 0xFFFFFFFFu ? "-1" : p.ToString())); + Console.WriteLine($" ParentIndex[{setup.ParentIndex.Count}]: {parentStr}"); + var scaleStr = string.Join(",", setup.DefaultScale.Take(Math.Min(34, setup.DefaultScale.Count)).Select(s => $"({s.X:F2},{s.Y:F2},{s.Z:F2})")); + Console.WriteLine($" DefaultScale[{setup.DefaultScale.Count}]: {scaleStr}"); + // Dump the resolved idle frame's per-part Origin + Orientation. + // If retail composes parent_world * animation_local but acdream + // treats animation_local as world-relative, we'd see specific + // patterns of non-zero per-part origins/rotations that should + // be parent-relative. For setups whose idle has all parts at + // (0,0,0)/identity, parent walking would be a no-op (which + // matches my earlier "no change" experiment if that was the + // human-idle case) — diagnostic confirms. + if (idleFrame is not null) + { + Console.WriteLine($" IdleFrame.Frames[{idleFrame.Frames.Count}]:"); + int dumpCount = Math.Min(idleFrame.Frames.Count, 17); // first 17 (real body parts, not the 17-33 placeholders) + for (int fi = 0; fi < dumpCount; fi++) + { + var f = idleFrame.Frames[fi]; + Console.WriteLine($" [{fi:D2}] Origin=({f.Origin.X:F3},{f.Origin.Y:F3},{f.Origin.Z:F3}) Orient=(W={f.Orientation.W:F3} X={f.Orientation.X:F3} Y={f.Orientation.Y:F3} Z={f.Orientation.Z:F3})"); + } + } + else + { + Console.WriteLine($" IdleFrame: NULL"); + } foreach (var c in animPartChanges) Console.WriteLine($" APC part={c.PartIndex:D2} -> gfx=0x{c.NewModelId:X8}"); From 8d7cad5b14e0640832e3691688cc94c9b4f9fd25 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 11:34:25 +0200 Subject: [PATCH 14/15] docs(research): #47 handoff prompt for next-session agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-contained pickup brief for the bulky-humanoid bug. Has: - the bug + acceptance criterion - everything ruled out this session (with evidence) - starting facts confirmed via diagnostics - 4 ranked hypotheses (per-vertex normals → ambient → MSAA → frame composition) with concrete tests for each - diagnostic env vars + their output shapes - the CLAUDE.md grep-named-first workflow - files most likely to need edits - live test workflow (env vars, expected entities in Holtburg) - constraints (don't break drudges / scenery / +Acdream local view) Designed to drop straight into a fresh agent's prompt window. --- docs/research/2026-05-06-issue-47-handoff.md | 287 +++++++++++++++++++ 1 file changed, 287 insertions(+) create mode 100644 docs/research/2026-05-06-issue-47-handoff.md diff --git a/docs/research/2026-05-06-issue-47-handoff.md b/docs/research/2026-05-06-issue-47-handoff.md new file mode 100644 index 00000000..45da384c --- /dev/null +++ b/docs/research/2026-05-06-issue-47-handoff.md @@ -0,0 +1,287 @@ +# Issue #47 handoff — humanoid Setup 0x02000001 renders bulky vs retail + +**Use this whole document as the prompt** when handing off to a fresh agent. +Everything they need to pick up cold is below. + +--- + +## The bug, in one paragraph + +acdream renders any character that uses Setup `0x02000001` (Aluvian Male) +with a visibly **bulkier, less shape-defined silhouette** than the same +character viewed in retail's AC client. Specifically: shoulders look +smoother / rounder where retail has pointier shoulder pads; the back lacks +contour ("flat back"); arms appear puffier. The bug applies equally to +players (`+Acdream`, `+Je`) and humanoid NPCs using the same setup +(`Woodsman`, `Sedor Wystan the Blacksmith`, `Thelnoth Cort the Healer`, +others). It is **independent of equipment** — `+Je` stripped naked still +shows the bulky shape. It is **specific to Setup 0x02000001** — drudges +(setup `0x020007DD`) and other monster setups render identically to retail +through the same pipeline. See ISSUES.md `#47` for the full filing. + +## Acceptance criterion + +Side-by-side screenshots of the same humanoid (player or NPC) viewed from +the same approximate angle in acdream and retail show **matching silhouette +and shape definition** — pointy shoulders where retail has them, contoured +back, no "puffy" arms. User confirms visually. NPCs, drudges, and scenery +must continue to render correctly (no regressions). + +## What's already been ruled out (don't redo these) + +1. **0xF625 ObjDescEvent appearance updates being dropped.** Was a real + bug for skin/hair colors. Fixed in commit `e471527`. Does NOT affect + the bulky-shape issue (persists with the fix in place AND with no + equipment). +2. **Position-pop on equip toggle.** Side effect of the appearance fix, + also resolved in `e471527`. Doesn't affect shape. +3. **Clothing/armor overlapping the base body** (the HiddenParts + hypothesis). User stripped `+Je` naked; bulky shape persists. +4. **`ParentIndex` hierarchy not walked in `SetupMesh.Flatten`.** Setup + `0x02000001` has a real hierarchy + (`-1, -1, 1, 2, 3, -1, 5, 6, 7, 0, 9, 10, 11, 12, 13, 14, 15, 0, ...`) + but a parent-walk implementation produced **no visible change**. + Confirms AC's idle animation frames are already in setup-root + coordinates, not parent-local. +5. **Equipment / wielded items.** No equipment on `+Je` and bug persists. +6. **Player-specific data flow.** Humanoid NPCs using the same setup + (Woodsman et al) show the same bug. +7. **Silent GfxObj load failures or polygon drops.** `ACDREAM_DUMP_CLOTHING=1` + confirmed every one of the 34 parts emits triangles (`EMIT part=NN + gfx=0xXX subMeshes=N tris=N`); total ~648-700 tris per character. + +## What's confirmed (use this as starting facts) + +- `Setup.Parts.Count = 34` for `0x02000001`. `flatten.Count = 34`. + `AnimPartChanges = 34..38` depending on equipment. All match. +- Idle animation frames place parts at sensible humanoid Z-heights + (`head Z=1.587, mid-body Z=0.5-1.0, ground Z=0.085`). +- All 34 per-part orientations are nearly identical: 180° around -Z + axis (`W≈0, Z≈-1`). This is a setup-wide coordinate-flip convention. + Drudges have varied per-part orientations — different layout. +- `setup.DefaultScale.Count = 0` for both humans and drudges → all parts + use `Vector3.One` scale. +- Same fragment shader (`mesh.frag`) is used for humans and drudges. + Per-pixel diffuse with interpolated `vWorldNormal`. + +## Top hypotheses, ordered by likely payoff + +### Hypothesis A — per-face vs smoothed vertex normals + +**Strongest candidate.** AC's dat stores ONE normal per `SWVertex`. If +human-character GfxObjs (e.g. `0x01001212`, `0x0100004B`-`0x01000059`) +were authored with **per-face flat normals** (each vertex's normal copied +from its triangle's face normal) while monster GfxObjs were authored with +**smoothed normals** (averaged across adjacent faces), acdream's +`Vector3.Normalize(sw.Normal)` would produce flat shading on humans and +smooth shading on monsters. The screenshots strongly support this — retail +characters look smooth-shaded, acdream characters look facet-edged. + +User said "not shaders" but they may not realize per-vertex normal +*style* is part of the shader pipeline. + +**Test:** in `src/AcDream.Core/Meshing/GfxObjMesh.cs:142`, replace the +direct `sw.Normal` read with a smooth normal computed per-load via +face-adjacency accumulation: + +```csharp +// Pre-pass: for each polygon, compute face normal; accumulate onto each vertex. +// Post-pass: normalize. +``` + +Verify retail does this — see `docs/research/deepdives/r13-dynamic-lighting.md:107` +for the `v.normal += t.face_normal` pattern, and +`docs/plans/2026-04-13-rendering-rebuild.md:50` for the +`AdjustPlanes: face-normal accumulation + per-vertex lighting` note (terrain context but +same math applies to characters). + +If smooth-normals fixes humans and ALSO doesn't break drudges (because +drudge dat normals were already smooth, computing them again gives the +same answer modulo precision), this is the bug. + +### Hypothesis B — cell ambient too low + +Back-facing surfaces (the parts of the character not lit by the directional +sun) fall to `uCellAmbient` in `mesh.frag`. If ambient is very dark, the +back of any character looks uniformly black — reading as "flat" because no +detail variation is visible. Retail likely has higher ambient that lets +unlit surfaces still show their geometry through subtle gradients. + +**Test:** dump `uCellAmbient` UBO values during a player render and compare +to retail's behaviour. Try bumping ambient temporarily and see if back- +detail emerges. + +### Hypothesis C — anti-aliasing + +acdream's GL window may not have MSAA enabled. Without it, polygon edges +visibly stair-step, exaggerating the faceted look at low triangle counts +(~700 tris per character). Retail likely has AA on by default. + +**Test:** check the Silk.NET window creation code for `Samples`/MSAA +config. Try enabling `Samples=4` and re-render. + +### Hypothesis D — orientation composition order or sign + +The 180°-around-Z rotation on every part is unusual. If acdream applies +it correctly but retail applies it differently (e.g. as a post-multiply +or with the inverse), parts could be subtly mis-positioned in ways that +read as "bulky" rather than "broken". My investigation didn't fully rule +this out — `parent-walk` was a no-op, but a *single-level* orientation +composition discrepancy might be invisible without comparing actual +post-transform vertex positions to retail. + +**Test:** attach cdb to retail (see CLAUDE.md "Retail debugger toolchain"), +break in `Frame::combine` (`0x518FD0`) with a player guid, dump the +resulting `Frame` for parts 0, 9, 16. Compare to acdream's per-part +world matrices (add a diagnostic). + +## Diagnostic infrastructure already built + +All env-var-gated, no runtime cost when off: + +```powershell +$env:ACDREAM_DUMP_CLOTHING = "1" +``` + +Prints, for every humanoid spawn (gate: `setup.Parts.Count >= 10`): + +- Header: `setup.Parts.Count`, `flatten.Count`, `APC` count +- `ParentIndex[N]: -1,-1,1,2,3,...` array +- `DefaultScale[N]: ...` array +- `IdleFrame.Frames[N]:` per-part `Origin` + `Orientation` (first 17 parts) +- `EMIT part=NN gfx=0xXXXXXXXX subMeshes=N tris=N` per part +- `TOTAL tris=N meshRefs=N` per entity + +```powershell +$env:ACDREAM_DUMP_APPEARANCE = "1" +``` + +Prints structured decode of every `0xF625 ObjDescEvent` (mid-session +appearance change) — SubPalettes, TextureChanges, AnimPartChanges. + +Source: `src/AcDream.App/Rendering/GameWindow.cs` around `dumpClothing` / +`OnLiveEntitySpawnedLocked`, and `src/AcDream.Core.Net/WorldSession.cs` +`DumpAppearanceEnabled`. + +## Workflow (per `CLAUDE.md`) + +This is AC-specific behavior. Follow the mandatory workflow: + +1. **Step 0 — grep named first.** + `grep "Class::method" docs/research/named-retail/acclient_2013_pseudo_c.txt` + for any function name from a hypothesis. The Sept 2013 PDB is named — + most things have real names. Don't decompile fresh until you've grep'd. +2. **Step 1 — cross-reference.** Check ACE / ACME / ACViewer / Chorizite / + AC2D / holtburger as appropriate. The reference hierarchy in CLAUDE.md + spells out which ref is authoritative for each domain. For rendering: + ACME `WorldBuilder.Tests/ClientReference.cs` is the closest oracle. +3. **Step 2 — pseudocode.** Write the algorithm in plain language under + `docs/research/*_pseudocode.md` before porting. +4. **Step 3 — port faithfully.** Match retail line-by-line, same variable + names, same control flow. No "improvements". +5. **Step 4 — conformance test.** Write a test using a captured wire body + or known dat values as golden output. +6. **Step 5 — visual verification.** User confirms in-client. + +If grep-named-first turns up nothing relevant, the function is likely +in the unnamed minority. Fall back to Ghidra chunks under +`docs/research/decompiled/` keyed by address. + +## Files most likely to need edits + +- `src/AcDream.Core/Meshing/GfxObjMesh.cs` — polygon emission, vertex + normal handling at line 142. **Hypothesis A lives here.** +- `src/AcDream.Core/Meshing/SetupMesh.cs` — per-part transform + composition. The parent-walk experiment lives here (and was reverted — + see git history if you want to revisit it). +- `src/AcDream.App/Rendering/Shaders/mesh.frag` — per-pixel lighting + equation. **Hypothesis B lives here** if ambient is the cause. +- `src/AcDream.App/Rendering/Shaders/mesh.vert` — normal transform + (`mat3(uModel) * aNormal`). Watch for non-uniform scale issues if + any future change touches scale. +- The Silk.NET window setup code (search for `IWindow.Create` / + `WindowOptions`) — **Hypothesis C lives here** if MSAA needs enabling. + +## Test workflow (live verification) + +```powershell +# Always: kill stale + 3-5s wait (ACE session lingers briefly). +Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force +Start-Sleep -Seconds 4 + +$env:ACDREAM_DUMP_CLOTHING = "1" # verbose per-spawn diagnostics +$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call" +$env:ACDREAM_LIVE = "1" +$env:ACDREAM_TEST_HOST = "127.0.0.1" +$env:ACDREAM_TEST_PORT = "9000" +$env:ACDREAM_TEST_USER = "testaccount" +$env:ACDREAM_TEST_PASS = "testpassword" +dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | + Tee-Object -FilePath "launch_issue47.log" +``` + +The user's `+Acdream` character logs in at Holtburg. Nearby: +- Other player(s) like `+Je` (driven from a parallel retail acclient) +- Humanoid NPCs (Woodsman, Sedor Wystan the Blacksmith, Thelnoth Cort + the Healer, Sontella Dagroff the Bowyer, Archmage Cindrue, Monyra the + Jeweler, Ecutha the Tailor) — most use Setup 0x02000001 +- Drudges and slinkers nearby (different setup, render correctly — use + as control) +- The Foundry's "Nullified Statue of a Drudge" — different setup, used + for prior diagnostic work + +User has the retail acclient open in parallel for side-by-side comparison. +Visual verification is the acceptance test — there's no automated way to +say "this looks like retail." + +## Constraints / don't-break + +- Do **not** break drudge / monster rendering. They're correct now. +- Do **not** break scenery (terrain, buildings, statues). They're correct. +- Do **not** break the local `+Acdream` (own-character) view either — + same pipeline so any rendering change applies to it. +- Tests must stay green: `dotnet test AcDream.slnx` should report 0 + failures. There are 8 pre-existing motion test failures in + `AcDream.Core.Tests` that are NOT yours — don't try to fix them in + this work. +- Build must stay green: `dotnet build AcDream.slnx -c Debug`. + +## When to stop and ask + +Per CLAUDE.md, ask only for: +- Visual verification (user looking at the client) +- Genuine architectural disagreements with the roadmap +- Hard-to-reverse destructive actions + +Otherwise act. Don't ask "should I continue?". + +## References to consult + +- `references/ACME/WorldBuilder/Editors/Landscape/StaticObjectManager.cs` — + ACME's mesh hydration; same stack (Silk.NET) as us, used as our + closest "should look right" oracle. +- `references/ACViewer/ACViewer/Physics/PartArray.cs:614` (`UpdateParts`) + and `references/ACViewer/ACViewer/Physics/Animation/AFrame.cs:43` + (`Combine`) — frame composition math. +- `references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Networking.cs:48` + (`SerializeUpdateModelData`) and `:978` (`AddBaseModelData`) — what + ACE puts on the wire for character appearance. +- `docs/research/named-retail/acclient_2013_pseudo_c.txt` — Sept 2013 PDB + with 18,366 named functions. **Grep this FIRST.** +- `docs/plans/2026-04-13-rendering-rebuild.md` — earlier rendering + rebuild plan with notes on `AdjustPlanes` (terrain, but the + face-normal accumulation pattern is the smooth-normal pattern). +- `docs/research/deepdives/r13-dynamic-lighting.md` — has the smooth- + normal accumulation pseudocode at line 107. + +## Final note + +This is a **rendering-fidelity** issue, not a wire-protocol one. The +network data is correct (the previous session's `0xF625` work confirmed +it). The bug is in how acdream interprets that data into pixels. + +The smoothest path is probably Hypothesis A (smooth normals), one +contained change in `GfxObjMesh.Build`, gated behind a feature flag for +A/B testing, then verified live by the user. If that doesn't fix it, +move to B (ambient), then C (MSAA), then D (frame composition with cdb +trace). From 0bd9b9693b674099fb17c1afaa50985f8bfb011f Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 6 May 2026 16:46:23 +0200 Subject: [PATCH 15/15] =?UTF-8?q?fix(rendering):=20#47=20=E2=80=94=20walk?= =?UTF-8?q?=20DIDDegrade=20for=20retail=20close-detail=20meshes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Humanoid bodies (Setup 0x02000001 + heritage variants) rendered visibly flat / bulky vs retail because we drew the base GfxObj id from Setup / AnimPartChange directly. Retail's CPhysicsPart::LoadGfxObjArray (0x0050DCF0) treats that base id as the entry point to a DIDDegrade table; close/player rendering uses Degrades[0].Id, which is the higher-detail mesh that carries bicep / deltoid / shoulder geometry. ACViewer also has this bug — it was the key signal it isn't acdream- specific. Both clients drew the LOD-3 base mesh (e.g. 14 verts / 17 polys for Aluvian Male upper arm 0x01000055), missing the close- detail variant (0x01001795: 32 verts / 60 polys). Adds GfxObjDegradeResolver that walks the table with safe fallbacks at every step. Wired in GameWindow after AnimPartChange application and before texture-change resolution so texture overrides match the resolved mesh's surfaces. Gated by ACDREAM_RETAIL_CLOSE_DEGRADES=1 and scoped to humanoid setups (34 parts with >=8 null-sentinel attachment slots) while the fix bakes — the change is harmless on non-humanoid setups (resolver falls back to base when no degrade table) but we hold the broader sweep until LOD distance plumbing lands. User confirmed visually 2026-05-06: bicep, deltoid, and back-muscle definition match retail. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/ISSUES.md | 33 ++- ...05-06-issue-47-close-degrade-pseudocode.md | 224 ++++++++++++++++++ src/AcDream.App/Rendering/GameWindow.cs | 67 ++++++ .../Meshing/GfxObjDegradeResolver.cs | 144 +++++++++++ .../Meshing/GfxObjDegradeResolverTests.cs | 182 ++++++++++++++ 5 files changed, 648 insertions(+), 2 deletions(-) create mode 100644 docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md create mode 100644 src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs create mode 100644 tests/AcDream.Core.Tests/Meshing/GfxObjDegradeResolverTests.cs diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 98e44253..3b790aea 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -1258,13 +1258,42 @@ If hypothesis (a) is correct, this issue effectively rolls into **#28** — the --- -## #47 — Humanoid Setup 0x02000001 renders bulky / lacks shape detail vs retail +## #47 — [DONE 2026-05-06] Humanoid Setup 0x02000001 renders bulky / lacks shape detail vs retail -**Status:** OPEN +**Status:** DONE (commit pending) +**Closed:** 2026-05-06 **Severity:** MEDIUM (cosmetic — characters readable but visibly different from retail) **Filed:** 2026-05-06 **Component:** rendering / mesh / character animation +**Resolution:** Root cause was that we drew the base GfxObj id from +Setup / `AnimPartChange` directly. Retail's `CPhysicsPart::LoadGfxObjArray` +(`0x0050DCF0`) treats that base id as an **entry point to the +`DIDDegrade` table**; for close/player rendering it draws +`Degrades[0].Id`, which is the higher-detail mesh that carries the +bicep / deltoid / shoulder geometry. ACViewer also has this bug — +that was the key signal it wasn't acdream-specific. + +Concrete swaps the resolver now performs: +- Aluvian Male upper arm `0x01000055` → `0x01001795` (14/17 → 32/60 verts/polys) +- Aluvian Male lower arm `0x01000056` → `0x0100178F` +- Heritage variants: `0x010004BF → 0x010017A8`, `0x010004BD → 0x010017A7`, + `0x010004B7 → 0x0100179A`, etc. + +Fix landed as `GfxObjDegradeResolver`, gated behind +`ACDREAM_RETAIL_CLOSE_DEGRADES=1` and scoped to humanoid setups +(34-part with ≥8 null-sentinel attachment slots). User confirmed +visually 2026-05-06. + +Files: `src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`, +`src/AcDream.App/Rendering/GameWindow.cs` (wiring), 5 unit tests in +`tests/AcDream.Core.Tests/Meshing/GfxObjDegradeResolverTests.cs`. +Research note: `docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md`. + +--- + +### Original investigation (kept for reference) + **Description:** Every humanoid character using Setup `0x02000001` (Aluvian Male) renders in acdream with a "bulky, less-defined" silhouette compared to retail's view of the same character. Specifically: shoulders diff --git a/docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md b/docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md new file mode 100644 index 00000000..2c22a4ed --- /dev/null +++ b/docs/research/2026-05-06-issue-47-close-degrade-pseudocode.md @@ -0,0 +1,224 @@ +# Issue #47 — humanoid bulky/flat rendering: GfxObj close-degrade fix + +**Status:** root cause identified and patched (2026-05-06). +**Flag:** `ACDREAM_RETAIL_CLOSE_DEGRADES=1` enables; off by default +while the fix bakes. +**Files:** `src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs`, +wiring in `src/AcDream.App/Rendering/GameWindow.cs`. + +## The bug, in one sentence + +acdream and ACViewer both rendered humanoid body parts using the +**low-detail** GfxObj that the Setup / `AnimPartChange` references, +instead of walking that base GfxObj's `DIDDegrade` table to slot 0 +(the close-detail mesh) the way retail does. + +## How we got here + +We spent a session investigating Issue #47 ("humanoid Setup 0x02000001 +renders bulky vs retail") and ruled out, with screenshots, every +hypothesis on the original handoff list: + +- per-face vs smoothed vertex normals (smooth-normal pass had no + visible effect; dat normals were already smooth) +- transform composition (acdream's `Scale * RotPart * TransPart * + RotEntity * TransEntity` matches retail's `Frame::combine` at + `0x518FD0` algebraically) +- ambient floor / cell ambient tuning (lighting tweak, doesn't change + silhouette) +- MSAA (anti-aliasing doesn't change silhouette thickness) +- `client_highres.dat` precedence (retail does prefer HighRes over + Portal in `CLCache::GetDiskController` at `0x4f8fa0`, but the + humanoid body GfxObjs we were drawing don't get high-res + replacements — they get LOD replacements via DIDDegrade) +- the `0x010001EC` null-part stubs in slots 17-33 (correctly skipped + per ACE's "essentially a null part" comment, but they were 1-tri + meshes — visually negligible, not the bug) + +The user critically reported that **ACViewer showed the same flat +arms**, which meant the bug couldn't be in our renderer alone — it +had to be in something both renderers shared. Both load from the same +dat. Both run the AnimPartChange ids through their renderers as +final mesh ids. Neither walks DIDDegrade. + +A side-by-side screenshot pair of `+Acdream` in retail vs acdream +made the symptom precise: retail showed clear per-face linear gradients +with visible bicep / deltoid / pectoral edges; acdream showed a smooth +featureless tube. + +## Why retail looks different + +Retail's CPhysicsPart load and draw flow walks the degrade table: + +| Function | Address | What it does | +|-----------------------------------------|-------------|---------------------------------------------------------------------------------------------| +| `CPhysicsPart::LoadGfxObjArray` | `0x0050DCF0`| Loads the base GfxObj only to read `DIDDegrade`. If a `GfxObjDegradeInfo` exists, retail loads each entry from `Degrades` into the part's render array. | +| `CPhysicsPart::UpdateViewerDistance` | `0x0050E030`| Picks `deg_level` per part by camera distance. For close / player rendering `deg_level == 0`. | +| `CPhysicsPart::Draw` | `0x0050D7A0`| Draws `gfxobj[deg_level]`. | + +So for close / player rendering the actual mesh is +`GfxObjDegradeInfo.Degrades[0].Id`, NOT the base GfxObj id. + +## Concrete evidence + +Comparing the base meshes the server hands us (post-AnimPartChange) +against the close-detail meshes their `DIDDegrade` tables point at: + +| Body part | Base id | Base verts/polys | Degrade table | Slot 0 close id | Close verts/polys | +|---------------------------------|----------------|------------------|---------------|-----------------|-------------------| +| Aluvian Male upper arm | `0x01000055` | 14 / 17 | `0x110006D0` | `0x01001795` | 32 / 60 | +| Aluvian Male lower arm | `0x01000056` | 8 / 6 | (per dat) | `0x0100178F` | 22 / 39 | +| Heritage variant upper arm | `0x010004BF` | (low) | (per dat) | `0x010017A8` | (high) | +| Heritage variant lower arm-A | `0x010004BD` | (low) | (per dat) | `0x010017A7` | (high) | +| Heritage variant lower arm-B | `0x010004B7` | (low) | (per dat) | `0x0100179A` | (high) | + +Drawing the base ids gave us visibly LOD-3 bodies on close-up players — +no bicep, no deltoid contour, no shoulder geometry. The degrade-slot-0 +meshes have the geometry that produces the per-face gradients the user +expected. + +## Pseudocode + +``` +TryResolveCloseGfxObj(getGfxObj, getDegradeInfo, gfxObjId) + → resolvedId, resolvedGfxObj + + base = getGfxObj(gfxObjId) + if base is null: + return (gfxObjId, null, false) # caller drops the part + + resolved = (gfxObjId, base) + + if base.Flags HasDIDDegrade is clear OR base.DIDDegrade == 0: + return (resolved, true) + + info = getDegradeInfo(base.DIDDegrade) + if info is null OR info.Degrades is empty: + return (resolved, true) + + closeId = info.Degrades[0].Id + if closeId == 0: + return (resolved, true) + + closeObj = getGfxObj(closeId) + if closeObj is null: + return (resolved, true) + + return ((closeId, closeObj), true) +``` + +Every fallback leaves the base mesh selected — better to render the +low-detail variant than nothing at all when the dat is partial. + +## Wiring in `GameWindow.OnLiveEntitySpawnedLocked` + +The order matters because the resolver has to see the **final** +per-part GfxObj id, and downstream consumers (texture-change +resolution, palette detection, mesh build) have to see the resolved +mesh's surfaces: + +``` +1. Setup flatten → per-part transforms with default GfxObj ids. +2. Apply server AnimPartChanges → replace per-part ids with the + body / clothing / head GfxObjs the server picked. +3. *** NEW *** If retail close-degrades enabled AND the setup is a + humanoid (34 parts with ≥8 null-sentinel slots in 17–33), run + each part's id through GfxObjDegradeResolver and swap to slot 0. +4. Resolve TextureChanges against the resolved GfxObj's surfaces. +5. Build palette overrides. +6. GfxObjMesh.Build / texture upload. +``` + +Wiring it before AnimPartChanges would replace Setup defaults that +will get overwritten anyway. Wiring it after texture-change resolution +would point texture overrides at the wrong surface ids. + +## Scope + +For now the swap is gated to humanoid setups only. The detector is +purely structural: 34 parts with at least 8 of slots 17-33 wired to +the AC null-part sentinel `0x010001EC`. This matches Aluvian Male +(`0x02000001`), the heritage variants, and any future 34-part +humanoid sibling without enumerating ids. + +Why scoped vs. always-on: + +- Scenery and creatures may have degrade tables too (buildings + certainly do). For non-humanoids we haven't visually verified + that swapping to slot 0 is correct for the current camera distance, + so we hold the change. +- True LOD plumbing (distance-based `deg_level` selection per + `CPhysicsPart::UpdateViewerDistance`) is still future work; until + then "always slot 0" is right for player + nearby NPCs but might + over-detail far-distance scenery. + +When the close-degrade path is validated everywhere, drop the +humanoid scoping and remove the env-var flag. + +## Verification + +```powershell +# Acceptance: side-by-side screenshots of `+Acdream` (or any humanoid +# NPC) in acdream vs retail show matching shoulder / bicep / back +# definition. Drudges and other monster setups stay correct. + +Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force +Start-Sleep -Seconds 4 + +$env:ACDREAM_RETAIL_CLOSE_DEGRADES = "1" +$env:ACDREAM_DUMP_CLOTHING = "1" # log resolver swaps per spawn +$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call" +$env:ACDREAM_LIVE = "1" +$env:ACDREAM_TEST_HOST = "127.0.0.1" +$env:ACDREAM_TEST_PORT = "9000" +$env:ACDREAM_TEST_USER = "testaccount" +$env:ACDREAM_TEST_PASS = "testpassword" +dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | + Tee-Object -FilePath "launch_issue47_close_degrade.log" +``` + +Expected log lines (per spawn): + +``` +DEGRADE part=01 gfx=0x0100004F -> close=0x0100178D +DEGRADE part=02 gfx=0x0100004D -> close=0x01001787 +DEGRADE part=10 gfx=0x0100122B -> close=0x01001795 +… +``` + +(Exact ids vary by which body parts AnimPartChange installs for the +character's heritage / equipped clothing. The `->` arrow confirms +the swap fired.) + +## What was rejected + +These were diagnostic experiments during the investigation, NOT part +of the fix: + +- Smooth-normal recompute behind `ACDREAM_SMOOTH_NORMALS` +- HighRes-first lookup in `TextureCache.DecodeFromDats` +- Skipping `0x010001EC` null-part placeholders +- Per-vertex Gouraud shader rewrite of `mesh.frag` +- Cell ambient floor / minimum diffuse tuning +- MSAA toggle +- Identity per-part orientation +- Positive-only polygon emission + +The successful fix is ONLY the close GfxObj degrade slot 0 swap. All +of the above were reverted before this patch landed. + +## References + +- `acclient!CPhysicsPart::LoadGfxObjArray` at `0x0050DCF0` — + `docs/research/named-retail/acclient_2013_pseudo_c.txt` +- `acclient!CPhysicsPart::UpdateViewerDistance` at `0x0050E030` +- `acclient!CPhysicsPart::Draw` at `0x0050D7A0` +- DatReaderWriter: + `references/DatReaderWriter/DatReaderWriter/Generated/DBObjs/GfxObj.generated.cs` + (`HasDIDDegrade` flag, `DIDDegrade` field) + `references/DatReaderWriter/DatReaderWriter/Generated/DBObjs/GfxObjDegradeInfo.generated.cs` + (`Degrades : List`) +- ACE: `references/ACE/Source/ACE.DatLoader/FileTypes/GfxObjDegradeInfo.cs` +- ACViewer + ACME both miss this same step — they draw the base id + directly. ACViewer's confirmation was the key signal that the bug + isn't acdream-specific. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 726f829b..21e72ead 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -172,6 +172,38 @@ public sealed class GameWindow : IDisposable // Diagnostic: hide a specific humanoid part (>=10 parts) at render. private static readonly int s_hidePartIndex = int.TryParse(Environment.GetEnvironmentVariable("ACDREAM_HIDE_PART"), out var hp) ? hp : -1; + + // Issue #47 — opt in to retail's close-detail GfxObj selection on + // humanoid setups. When enabled, every per-part GfxObj id (after + // server AnimPartChanges are applied) is replaced with Degrades[0] + // from its DIDDegrade table when present. See GfxObjDegradeResolver + // for the full retail-decomp citation. Off by default while the fix + // bakes; flip to default-on once we've confirmed no scenery/setup + // regressions. + private static readonly bool s_retailCloseDegrades = + string.Equals(Environment.GetEnvironmentVariable("ACDREAM_RETAIL_CLOSE_DEGRADES"), "1", StringComparison.Ordinal); + + /// + /// Issue #47 humanoid-setup detector. Matches Aluvian Male + /// (0x02000001) and the 34-part heritage sibling setups + /// (Aluvian Female, Sho M/F, Gharu M/F, Viamont/Empyrean, etc.) + /// by structure rather than id list: a humanoid setup has exactly + /// 34 parts, and the trailing attachment slots (parts 17–33) are + /// the AC null-part sentinel 0x010001EC. Non-humanoid + /// 34-part setups (rare) won't have the sentinel pattern. + /// + private static bool IsIssue47HumanoidSetup(DatReaderWriter.DBObjs.Setup setup) + { + if (setup.Parts.Count != 34) return false; + const uint NullPartGfx = 0x010001ECu; + int nullSlots = 0; + for (int i = 17; i < setup.Parts.Count; i++) + if ((uint)setup.Parts[i] == NullPartGfx) nullSlots++; + // At least half of slots 17–33 wired to the null sentinel — enough + // to distinguish humanoids from any future 34-part creature setup. + return nullSlots >= 8; + } + private readonly HashSet _activeSkyPes = new(); private readonly HashSet _missingSkyPes = new(); @@ -2076,6 +2108,41 @@ public sealed class GameWindow : IDisposable } } + // Issue #47 — retail's close/player rendering path resolves each + // part's base GfxObj through its DIDDegrade table to the close- + // detail mesh in slot 0. Without this, humanoid arms/torso draw + // the LOW-detail base GfxObj (e.g. 0x01000055, 14 verts / 17 + // polys) instead of the close mesh (0x01001795, 32 verts / 60 + // polys), losing all bicep/shoulder/back geometry. See + // for the named-retail + // citation (CPhysicsPart::LoadGfxObjArray at 0x0050DCF0, + // ::UpdateViewerDistance at 0x0050E030, ::Draw at 0x0050D7A0). + // + // Order matters: the swap happens AFTER AnimPartChanges have + // installed the server's body/clothing/head ids, BEFORE texture + // changes resolve (which match against the resolved mesh's + // surfaces) and BEFORE the GfxObjMesh.Build / texture upload + // path consumes the part list. + if (s_retailCloseDegrades && IsIssue47HumanoidSetup(setup)) + { + for (int partIdx = 0; partIdx < parts.Count; partIdx++) + { + var part = parts[partIdx]; + if (!AcDream.Core.Meshing.GfxObjDegradeResolver.TryResolveCloseGfxObj( + _dats, part.GfxObjId, + out uint resolvedId, out _)) + continue; + if (resolvedId == part.GfxObjId) + continue; + + parts[partIdx] = new AcDream.Core.World.MeshRef( + resolvedId, part.PartTransform); + + if (dumpClothing) + Console.WriteLine($" DEGRADE part={partIdx:D2} gfx=0x{part.GfxObjId:X8} -> close=0x{resolvedId:X8}"); + } + } + // Build per-part texture overrides. The server sends TextureChanges as // (partIdx, oldSurfaceTextureId, newSurfaceTextureId) where both ids // are in the SurfaceTexture (0x05) range. Our sub-meshes are keyed diff --git a/src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs b/src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs new file mode 100644 index 00000000..c8d38bf7 --- /dev/null +++ b/src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs @@ -0,0 +1,144 @@ +using DatReaderWriter; +using DatReaderWriter.DBObjs; +using DatReaderWriter.Enums; + +namespace AcDream.Core.Meshing; + +/// +/// Resolve a base GfxObj id to its retail "close-detail" mesh by walking +/// the DIDDegrade table to Degrades[0]. +/// +/// +/// Why this exists (Issue #47). Many AC GfxObjs — most notably +/// humanoid body parts — store the LOW-detail mesh as the GfxObj that +/// the Setup or AnimPartChange references. The high-detail mesh used +/// for close/player rendering is reached indirectly: the base GfxObj's +/// HasDIDDegrade flag is set, DIDDegrade points at a +/// , and at +/// Degrades[0] is the close-detail variant. Drawing the base +/// GfxObj id directly produces the LOD-3 mesh — visibly bulky and +/// detail-less — which is exactly what acdream and ACViewer were both +/// rendering for humanoid body parts before this fix. +/// +/// +/// +/// Concrete example. The Aluvian Male upper-arm GfxObj +/// 0x01000055 is a 14-vertex / 17-poly low-detail stub. Its +/// degrade table 0x110006D0 points at 0x01001795, the +/// 32-vertex / 60-poly close-detail mesh that carries the bicep / +/// deltoid / shoulder geometry retail draws on the player. Same story +/// for the lower arm 0x01000056 → 0x0100178F and matching +/// heritage variants (0x010004BF → 0x010017A8, +/// 0x010004BD → 0x010017A7, 0x010004B7 → 0x0100179A). +/// +/// +/// +/// Retail flow (named-retail decomp). +/// +/// +/// acclient!CPhysicsPart::LoadGfxObjArray at 0x0050DCF0 +/// loads the base GfxObj solely to discover DIDDegrade; if +/// a exists, retail loads each entry +/// in Degrades into the part's render array. +/// +/// +/// acclient!CPhysicsPart::UpdateViewerDistance at +/// 0x0050E030 picks deg_level per part by distance. +/// For close / player rendering deg_level == 0. +/// +/// +/// acclient!CPhysicsPart::Draw at 0x0050D7A0 +/// draws gfxobj[deg_level]. +/// +/// +/// +/// +/// +/// We don't yet have distance-based LOD plumbing, so this resolver +/// always returns slot 0 (the close-detail mesh). That's correct for +/// player + nearby NPC rendering; far-distance LOD is a future concern. +/// +/// +public static class GfxObjDegradeResolver +{ + /// + /// DatCollection-backed convenience overload. Production callers use + /// this; tests use the callback overload below for easy fakes. + /// + public static bool TryResolveCloseGfxObj( + DatCollection dats, + uint gfxObjId, + out uint resolvedId, + out GfxObj? resolvedGfxObj) + => TryResolveCloseGfxObj( + id => dats.Get(id), + id => dats.Get(id), + gfxObjId, + out resolvedId, + out resolvedGfxObj); + + /// + /// Loader-callback overload. Returns the close-detail GfxObj id and + /// loaded object when a degrade table is present, otherwise the + /// base id and base GfxObj. + /// + /// + /// Lookup for a GfxObj by id. May return null when not found. + /// + /// + /// Lookup for a GfxObjDegradeInfo by id. May return null. + /// + /// Base GfxObj id (post-AnimPartChange). + /// + /// The id to actually render. Same as + /// when no degrade table exists; Degrades[0].Id when it does. + /// + /// + /// The loaded GfxObj for , cached so + /// callers don't have to re-read. + /// + /// + /// true if a usable GfxObj was resolved (either base or + /// degrade slot 0 loaded). false only when the base GfxObj + /// itself was missing — caller should drop this part. + /// + public static bool TryResolveCloseGfxObj( + Func getGfxObj, + Func getDegradeInfo, + uint gfxObjId, + out uint resolvedId, + out GfxObj? resolvedGfxObj) + { + var gfxObj = getGfxObj(gfxObjId); + if (gfxObj is null) + { + resolvedId = gfxObjId; + resolvedGfxObj = null; + return false; + } + + // Default: base mesh stays selected unless the degrade table + // resolves cleanly. Every fallback below leaves these set. + resolvedId = gfxObjId; + resolvedGfxObj = gfxObj; + + if (!gfxObj.Flags.HasFlag(GfxObjFlags.HasDIDDegrade) || gfxObj.DIDDegrade == 0) + return true; + + var degradeInfo = getDegradeInfo(gfxObj.DIDDegrade); + if (degradeInfo is null || degradeInfo.Degrades.Count == 0) + return true; + + uint closeId = (uint)degradeInfo.Degrades[0].Id; + if (closeId == 0) + return true; + + var closeGfxObj = getGfxObj(closeId); + if (closeGfxObj is null) + return true; + + resolvedId = closeId; + resolvedGfxObj = closeGfxObj; + return true; + } +} diff --git a/tests/AcDream.Core.Tests/Meshing/GfxObjDegradeResolverTests.cs b/tests/AcDream.Core.Tests/Meshing/GfxObjDegradeResolverTests.cs new file mode 100644 index 00000000..54dc9c28 --- /dev/null +++ b/tests/AcDream.Core.Tests/Meshing/GfxObjDegradeResolverTests.cs @@ -0,0 +1,182 @@ +using AcDream.Core.Meshing; +using DatReaderWriter.DBObjs; +using DatReaderWriter.Enums; +using DatReaderWriter.Types; + +namespace AcDream.Core.Tests.Meshing; + +/// +/// Unit tests for . The resolver is +/// the Issue #47 fix: route a base GfxObj id to its retail close-detail +/// mesh via the DIDDegrade table's slot 0. Tests use the callback +/// overload so we can stand up tiny in-memory fixtures without dragging +/// in a real DatCollection. +/// +public class GfxObjDegradeResolverTests +{ + /// + /// When the base GfxObj has no degrade table (HasDIDDegrade flag + /// clear), the resolver returns the base id unchanged. + /// + [Fact] + public void NoDegradeTable_ReturnsBaseMesh() + { + const uint baseId = 0x01001212u; + var baseGfx = new GfxObj { Flags = 0, DIDDegrade = 0 }; + var gfxObjs = new Dictionary { [baseId] = baseGfx }; + + bool ok = GfxObjDegradeResolver.TryResolveCloseGfxObj( + id => gfxObjs.GetValueOrDefault(id), + _ => null, + baseId, + out uint resolvedId, + out var resolvedGfx); + + Assert.True(ok); + Assert.Equal(baseId, resolvedId); + Assert.Same(baseGfx, resolvedGfx); + } + + /// + /// When the base GfxObj has a populated DIDDegrade table, the + /// resolver returns Degrades[0].Id and its loaded GfxObj — the + /// close-detail mesh retail draws for nearby objects. + /// + [Fact] + public void ValidDegradeTable_ReturnsSlotZero() + { + const uint baseId = 0x01000055u; // low-detail Aluvian Male upper arm + const uint degradeInfoId = 0x110006D0u; + const uint closeId = 0x01001795u; // retail close-detail variant + + var baseGfx = new GfxObj + { + Flags = GfxObjFlags.HasDIDDegrade, + DIDDegrade = degradeInfoId, + }; + var closeGfx = new GfxObj { Flags = 0 }; + var degradeInfo = new GfxObjDegradeInfo + { + Degrades = { new GfxObjInfo { Id = closeId } }, + }; + + var gfxObjs = new Dictionary + { + [baseId] = baseGfx, + [closeId] = closeGfx, + }; + var degradeInfos = new Dictionary + { + [degradeInfoId] = degradeInfo, + }; + + bool ok = GfxObjDegradeResolver.TryResolveCloseGfxObj( + id => gfxObjs.GetValueOrDefault(id), + id => degradeInfos.GetValueOrDefault(id), + baseId, + out uint resolvedId, + out var resolvedGfx); + + Assert.True(ok); + Assert.Equal(closeId, resolvedId); + Assert.Same(closeGfx, resolvedGfx); + } + + /// + /// If the degrade table references a GfxObj that isn't present in + /// the dat (corrupt / partial dat), the resolver falls back to the + /// base mesh rather than returning null. Better to render the + /// low-detail variant than nothing at all. + /// + [Fact] + public void MissingSlotZeroMesh_FallsBackToBase() + { + const uint baseId = 0x01000055u; + const uint degradeInfoId = 0x110006D0u; + const uint missingCloseId = 0xDEADBEEFu; + + var baseGfx = new GfxObj + { + Flags = GfxObjFlags.HasDIDDegrade, + DIDDegrade = degradeInfoId, + }; + var degradeInfo = new GfxObjDegradeInfo + { + Degrades = { new GfxObjInfo { Id = missingCloseId } }, + }; + var gfxObjs = new Dictionary { [baseId] = baseGfx }; + var degradeInfos = new Dictionary + { + [degradeInfoId] = degradeInfo, + }; + + bool ok = GfxObjDegradeResolver.TryResolveCloseGfxObj( + id => gfxObjs.GetValueOrDefault(id), + id => degradeInfos.GetValueOrDefault(id), + baseId, + out uint resolvedId, + out var resolvedGfx); + + Assert.True(ok); + Assert.Equal(baseId, resolvedId); + Assert.Same(baseGfx, resolvedGfx); + } + + /// + /// Empty Degrades list (table present but no entries) falls back + /// to base. Mirrors retail's "no LOD entries → just draw the base" + /// behavior. + /// + [Fact] + public void EmptyDegradesList_FallsBackToBase() + { + const uint baseId = 0x01000055u; + const uint degradeInfoId = 0x110006D0u; + + var baseGfx = new GfxObj + { + Flags = GfxObjFlags.HasDIDDegrade, + DIDDegrade = degradeInfoId, + }; + var degradeInfo = new GfxObjDegradeInfo(); // empty Degrades + + var gfxObjs = new Dictionary { [baseId] = baseGfx }; + var degradeInfos = new Dictionary + { + [degradeInfoId] = degradeInfo, + }; + + bool ok = GfxObjDegradeResolver.TryResolveCloseGfxObj( + id => gfxObjs.GetValueOrDefault(id), + id => degradeInfos.GetValueOrDefault(id), + baseId, + out uint resolvedId, + out var resolvedGfx); + + Assert.True(ok); + Assert.Equal(baseId, resolvedId); + Assert.Same(baseGfx, resolvedGfx); + } + + /// + /// When the base GfxObj itself is missing from the dat, the + /// resolver returns false so the caller can drop the part rather + /// than trying to render a null mesh. + /// + [Fact] + public void MissingBaseGfxObj_ReturnsFalse() + { + const uint baseId = 0xDEADBEEFu; + + bool ok = GfxObjDegradeResolver.TryResolveCloseGfxObj( + _ => null, + _ => null, + baseId, + out uint resolvedId, + out var resolvedGfx); + + Assert.False(ok); + Assert.Equal(baseId, resolvedId); + Assert.Null(resolvedGfx); + } +}