From 17a9ff11583718603d06a61b21d51c9337cb7a26 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 2 May 2026 16:11:15 +0200 Subject: [PATCH] fix(motion): jump direction, AutoPos cadence, backward/strafe wire & anim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes a multi-bug knot in player motion outbound + remote inbound, discovered via cdb live trace of retail (2026-05-01) and follow-up visual verification. Outbound (acdream → ACE): - JumpAction velocity is BODY-LOCAL, not world (per retail CPhysicsObj::get_local_physics_velocity at 0x00512140 + ACE Player.HandleActionJump's set_local_velocity call). Was sending world; observers saw jump rotated by player yaw. - Capture get_jump_v_z BEFORE LeaveGround() — the latter resets JumpExtent to 0, after which get_jump_v_z returned 0. Was sending Z=0 in every JumpAction. - Backward/strafe-left jumps lost their horizontal velocity because LeaveGround → get_state_velocity returns zero for non-canonical motion (faithful to retail's FUN_00528960; retail papers over via adjust_motion translation, not yet ported). Compute the correct body-local launch velocity from input directly and push it back into the body so local prediction matches what we send. - IsRunning HoldKey was gated on `input.Run && input.Forward`, so strafe-run and backward-run incorrectly broadcast as walk to observers — ACE then animated walk + dead-reckoned at walk speed while server position moved at run speed (visible as observer lag). Fixed: gate on any active directional axis. - AutonomousPosition heartbeat 0.2s → 1.0s to match holtburger's AUTONOMOUS_POSITION_HEARTBEAT_INTERVAL and the ~1Hz observed in retail trace. - Heartbeat now fires while in-world regardless of motion state (matches holtburger + retail's transient_state-based gate, not motion-based). Pre-fix the at-rest heartbeat was suppressed. Inbound (ACE → acdream, remote retail player): - Remote backward walk arrives as cmd=WalkForward + speed=-1.91 (retail's adjust_motion'd form). Two bugs were stacking: 1. AnimationSequencer fast-path returned without updating when sign(speedMod) flipped while motion stayed equal — kept playing forward at old positive framerate. Fixed: bypass fast-path on sign change so the full re-setup runs. 2. GameWindow clamped negative speedMod to 1.0 when stuffing InterpretedState.ForwardSpeed, making get_state_velocity produce forward velocity. Fixed: pass speedMod through verbatim so the dead-reckoning body translates backward. Issue #38 filed: 30Hz physics tick produces a chase-camera smoothness regression at 60+ FPS render. Standard render-time interpolation is the recommended fix (separate phase). Findings + comparison vs retail/holtburger: docs/research/2026-05-01-retail-motion-trace/findings.md docs/research/2026-05-01-retail-motion-trace/fixes.md TODO: port retail's adjust_motion (FUN_00528010) properly so get_state_velocity works for all directions natively — would let us drop the workaround in PlayerMovementController jump path and the clamp in GameWindow. Co-Authored-By: Claude Opus 4.7 --- docs/ISSUES.md | 68 +++++ .../findings.md | 233 +++++++++++++++ .../2026-05-01-retail-motion-trace/fixes.md | 267 ++++++++++++++++++ .../Input/PlayerMovementController.cs | 111 ++++++-- src/AcDream.App/Rendering/GameWindow.cs | 23 +- .../Physics/AnimationSequencer.cs | 11 +- 6 files changed, 691 insertions(+), 22 deletions(-) create mode 100644 docs/research/2026-05-01-retail-motion-trace/findings.md create mode 100644 docs/research/2026-05-01-retail-motion-trace/fixes.md diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 7f9f11c..c95ec3b 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,74 @@ Copy this block when adding a new issue: # Active issues +## #38 — Chase camera + player feel "30 fps" since L.5 physics-tick gate + +**Status:** OPEN +**Severity:** MEDIUM (gameplay-feel regression; not a correctness bug) +**Filed:** 2026-05-01 +**Component:** rendering / physics / camera + +**Description:** User reports that running around in third-person / +chase camera feels less smooth than it did before the L.5 physics-tick +work. FPS counter still reads 60+, but the *motion* of the player +character + camera looks like it's updating at ~30 fps. + +**Root cause / status:** + +Almost certainly the L.5 `_physicsAccum` gate in +`PlayerMovementController.cs` (lines ~448-456). Retail integrates +physics at 30 Hz (`MinQuantum = 1/30 s`); we ported that faithfully so +collision behavior matches. Side effect: `_body.Position` only updates +on physics ticks, i.e. every 33 ms. Render runs at 60+ Hz but the +chase camera follows `_body.Position` directly — so the *visible* +position changes in 33 ms steps, even though we render at 60+ FPS. +First-person is less affected because the world rotates with Yaw (which +*does* update every render frame); third-person is hit hardest because +the character itself is the moving thing. + +Retail in 2013 didn't see this because render was also ~30 fps — +render rate ≈ physics rate. Our 60+ Hz render exposes the gap. + +Discussion + fix options at the end of `docs/research/2026-05-01-retail-motion-trace/findings.md` +("Other things still don't have…" → camera smoothness discussion in +chat, not yet captured in the doc — TODO migrate the discussion in). + +Recommended fix: **render-time interpolation between physics ticks** +(standard fixed-timestep + interpolated rendering pattern from Quake / +Source / Unreal). Snapshot `_prevPhysicsPos` and `_currPhysicsPos` at +each tick; render player + camera target at +`Lerp(_prev, _curr, _physicsAccum / PhysicsTick)`. Cost: ~33 ms visual +latency between input and what you see (matches retail's perceived +latency anyway). Network outbound stays on the discrete tick value — +no wire change. + +Quick confirmation test before any code change: temporarily set +`PhysicsTick` to `1.0/60.0` and see if chase camera feels smooth again. +If yes, gate is confirmed cause. (Don't ship that — it'd undo the L.5 +collision fixes.) + +**Files:** + +- `src/AcDream.App/Input/PlayerMovementController.cs:172` — `PhysicsTick` constant +- `src/AcDream.App/Input/PlayerMovementController.cs:448-456` — `_physicsAccum` gate +- `src/AcDream.App/Rendering/GameWindow.cs` — wherever player render position + chase camera read `_body.Position` + +**Research:** + +- L.5 background: `memory/project_retail_debugger.md` (the 30 Hz + MinQuantum gate, the cdb trace evidence) +- Discussed during 2026-05-01 motion-trace work + +**Acceptance:** + +- Chase-camera run-around at 60+ FPS feels as smooth as render rate + suggests (no perceptual stepping) +- Network outbound (MoveToState / AutonomousPosition cadence + values) + unchanged from current behavior +- Collision behavior unchanged (the L.5 wedge / steep-roof scenarios + still resolve correctly) +- Observer view from a parallel retail client unchanged + ## #37 — Humanoid coat doesn't extend up to neck (visible "skin stub" between hair and coat) **Status:** OPEN diff --git a/docs/research/2026-05-01-retail-motion-trace/findings.md b/docs/research/2026-05-01-retail-motion-trace/findings.md new file mode 100644 index 0000000..3632f88 --- /dev/null +++ b/docs/research/2026-05-01-retail-motion-trace/findings.md @@ -0,0 +1,233 @@ +# Retail motion outbound trace — findings + +**Date:** 2026-05-01 +**Tool:** cdb 10.0.28000.1839 attached non-invasively to live retail +acclient.exe v11.4186 (Sept 2013 EoR build, GUID +`9e847e2f-777c-4bd9-886c-22256bb87f32`). +**Symbols:** `refs/acclient.pdb` — 18,366 named functions, 5,371 named +struct/class types. +**Server:** local ACE on `127.0.0.1:9000`. Character: `+Acdream`, +spawned in Holtburg. + +## TL;DR + +- **WASD movement does NOT go through `ACCmdInterp::SendDoMovementEvent`.** + Across two captures totalling ~5 minutes of running, jumping, and + turning, that breakpoint fired **zero** times. `SendDoMovementEvent` is + apparently slash-command-only (`/run`, `/walk`, `/sneak`, etc.). The + per-frame movement send path is `CommandInterpreter::SendMovementEvent` + (which we couldn't trace stably — see "Open" below). +- **Jump goes through `CM_Movement::Event_Jump`.** It fires + **per-frame while the spacebar is held** (charge phase), not just once + per jump. 44 hits in ~30 sec of jumping. Same `JumpPack` stack pointer + every time — function probably packs every frame but conditionally + sends. +- **AutoPos heartbeat fires often during sustained motion** (~5 Hz at + rest after gating, much higher under motion). It's gated by + `transient_state & 1 && transient_state & 2 && Position::IsValid` so + **does not fire when the player is standing still**. +- **`set_heading` is for player rotation only, not camera mouse-look.** + Camera pan does not fire it; turn keys / strafe / character autoface do. +- **`set_state`** XOR mask shows which physics-state bits change on each + call. Distinct values observed: 11 different new-state bitmasks + across two captures (see "set_state bitmask atlas" below). + +## What we ran + +| Run | Duration | Activity | Hits | +|-----|----------|----------|------| +| v1 | ~60 sec | Run forward + turn | 127 | +| v2 | ~3 min | Run + jump + turn (holding W, repeated jumps) | 247 | + +Both with the same six-breakpoint set: +`SendDoMovementEvent`, `SendStopMovementEvent`, `set_state`, +`set_heading`, `Event_Jump`, `Event_AutonomousPosition`. Logs at +[motion_trace_v1_walk_and_turn_2026-05-01.log](../../../.claude/worktrees/jovial-blackburn-773942/motion_trace_v1_walk_and_turn_2026-05-01.log) +and [motion_trace_v2_run_jump_turn_2026-05-01.log](../../../.claude/worktrees/jovial-blackburn-773942/motion_trace_v2_run_jump_turn_2026-05-01.log). + +A v3 attempt added `CommandInterpreter::SendMovementEvent` (the +per-frame MoveToState gateway). The cdb attach + breakpoint setup with +that 7th breakpoint added enough overhead that retail's network thread +starved; ACE timed out the session and retail crashed within seconds of +attach. That bp is too high-frequency to instrument with a `printf` in +the action — needs a separate, minimal-overhead trace (counter only, +no print). + +## Hit distribution (v2) + +| BP | Hits | Notes | +|----|------|-------| +| `set_state` | 159 | All 11 distinct new-masks; clusters around motion transitions | +| `Event_Jump` | 44 | Bursts during sustained spacebar-hold | +| `Event_AutonomousPosition` (printed) | 38 | Throttle 1/10 → ~370 actual hits | +| `set_heading` (printed) | 6 | Throttle 1/10 → ~60 actual; only fires on player rotate | +| `SendDoMovementEvent` | 0 | **Confirmed unused for keyboard motion** | +| `SendStopMovementEvent` | 0 | **Confirmed unused** | + +## set_state bitmask atlas + +Captured `new` masks (current state was `0x00400c08` baseline, sometimes +`0x00410c08`). Distinct values across both runs: + +| `new` | bits set | Likely meaning (cross-checked vs `PHYSICS_STATE` enum candidates) | +|-------|----------|--------------------------------------------------------------------| +| `0x00000408` | 3, 10 | Base "in world, animating" | +| `0x00000410` | 4, 10 | Walk / sidestep variant | +| `0x00000414` | 2, 4, 10 | Walking + something | +| `0x00000418` | 3, 4, 10 | Walking + alt | +| `0x00000c0c` | 2, 3, 10, 11 | Multi-state combo | +| `0x00000c14` | 2, 4, 10, 11 | Combo near jump | +| `0x00010008` | 3, 16 | Bit 16 likely "scripted/cinematic"? | +| `0x00020414` | 2, 4, 10, 17 | Bit 17 likely something high | +| `0x00200418` | 3, 4, 10, 21 | Bit 21 likely "frozen/inert" | +| `0x00600418` | 3, 4, 10, 21, 22 | Bits 21+22 — common during motion | +| `0x0060041c` | 2, 3, 4, 10, 21, 22 | Same + bit 2 | + +**Action item:** decode the `PhysicsState` enum bits from +`docs/research/named-retail/acclient.h` and label this table +authoritatively. The `cur` value `0x00400c08` (bits 3, 10, 11, 22) +should be the "neutral, in-world, alive" base. + +## set_heading angle samples + +Angles captured (4-byte float bit pattern, decoded): + +| Hex | Float (degrees) | +|-----|-----------------| +| `0x00000000` | 0.0 (north) | +| `0x41c8af72` | 25.09 | +| `0x43070000` | 135.0 | +| `0x43083c22` | 136.23 | +| `0x430d7596` | 141.46 | +| `0x43340000` | 180.0 | +| `0x433a1741` | 186.09 | +| `0x43a9d6c7` | 339.68 | + +Confirms heading is **degrees, float, 0–360 wrap**. Matches the +acdream-side encoding in the outbound builders. + +## Event_Jump pattern + +44 hits in v2 — way more than user keypresses. Pattern: + +``` +[70..92] burst of 9 hits (one charged jump session) +[97..138] burst of 11 hits (another charged jump) +[251..408] continuous hits (multiple chained jumps) +``` + +Same `JumpPack` pointer (`001af5f8`) on every hit — stack-allocated +local in the caller. Consistent with `Event_Jump` being called +**per-frame while the jump button is held to charge**, with the +function itself deciding whether to actually send the pack on the wire. +We did not break inside the function so we don't have the send/no-send +ratio. + +**Action item:** trace one jump in isolation — set a bp on +`Event_Jump` AND on the BSTREAM-write deeper in the function. Capture +how many of the per-frame Event_Jump calls actually emit a 0xF61B +JumpAction packet. acdream's [JumpAction.cs](../../../src/AcDream.Core.Net/Messages/JumpAction.cs) +sends one JumpAction per spacebar release — if retail sends one per +charge-frame, our outbound is wrong (or vice-versa). + +## AutoPos heartbeat behaviour + +`CM_Movement::Event_AutonomousPosition` is called from +`CommandInterpreter::SendPositionEvent` (006b4770), which is +**guarded** by: + +```c +if (this->smartbox != 0 && this->player != 0 && + (transient_state & 1) != 0 && (transient_state & 2) != 0 && + Position::IsValid(&player->m_position)) +``` + +`transient_state` bits 0 and 1 are required — this is why AutoPos +**doesn't fire when the player is fully at rest**. During sustained +motion it fires ~5 Hz at the throttle interval we saw (1 print every +10 hits = ~1 line/sec → ~50 hits in 5 sec). + +**Action item:** acdream's outbound heartbeat in +[GameWindow.cs](../../../src/AcDream.App/Rendering/GameWindow.cs) +is a 200ms periodic pulse. Verify whether retail's cadence matches our +fixed 200ms or is event-driven (e.g. fires per-frame while moving and +not at all at rest). If event-driven, we may be sending stale +heartbeats while standing still that retail would suppress. + +## Confirmed correct vs acdream-side + +| Behaviour | Retail observed | acdream | Match | +|-----------|-----------------|---------|-------| +| Heading encoding | float degrees 0–360 | float degrees 0–360 in MoveToState | ✓ | +| Heartbeat at rest | does not send | sends every 200ms | **MISMATCH** — ours sends extras | +| Slash-command motion path | `SendDoMovementEvent` (unused for WASD) | n/a (acdream doesn't send slash motions) | ✓ | +| Jump per-frame charge | `Event_Jump` fires every frame while spacebar held | acdream sends 1 JumpAction on release | **POSSIBLE MISMATCH** — needs deeper trace | + +## Open / what we still don't have + +1. **The actual 0xF61C MoveToState send path** — retail's per-frame + movement message. Our breakpoint + on `CommandInterpreter::SendMovementEvent` (0x006B4680) was the + right entry but adding it to the trace caused retail to crash + (cumulative bp-action overhead). Needs a SEPARATE focused trace with + just that bp + a counter (no `printf`). +2. **The exact wire bytes of MoveToState / AutonomousPosition / Jump.** + Was planned for the v3 wire-trace; not run. Would require + `RawMotionState::Pack` and `AutonomousPositionPack::Pack` breakpoints + with `dt` struct dump + `db` byte dump. Same overhead concerns; do + one bp at a time. +3. **Sequence-counter behaviour.** Our trace captured nothing about + how retail bumps the four `Instance/ServerControl/Teleport/ForcePosition` + sequence counters across messages. Needs taps inside + `MoveToStatePack` / `AutonomousPositionPack` constructors. +4. **The `PhysicsState` enum decode.** Names map for the 0x408 / 0x418 + / 0x600418 / 0x10008 etc. bitmasks lives in + `docs/research/named-retail/acclient.h`. Pull the `PHYSICS_STATE` + enum and label this trace's set_state column authoritatively. +5. **DoMov / StopMov context.** Confirmed unused for WASD, but + `SendDoMovementEvent(0x85000001, 0x3f800000, 0)` was found in the + pseudo-C as a real call site — find what triggers it (slash command? + autorun toggle?) and document so our IS_SLASH_COMMAND test paths know + what to expect. + +## Toolchain notes + +- **`qd` is not allowed inside breakpoint command actions** in cdb. + Empirically silently ignored (no parse error). Quoting the docs: + "you cannot use commands that wait for input or end the debugger + session in breakpoint command lists." `qd`, `q`, `qq` all fall under + this. **Use `.detach` (a meta-command) instead** when you need a + bp-driven detach. +- **`cdb -pd` does NOT survive `Stop-Process -Force`.** `-pd` makes the + *graceful* exit not terminate the debuggee. `TerminateProcess` (which + is what `Stop-Process -Force` does) bypasses cleanup and the OS + treats debugger termination as debuggee termination. Need either + `qd`/`.detach` from inside, or send `CTRL_BREAK_EVENT` to cdb (via + P/Invoke) and feed `qd` to its stdin. +- **Per-frame breakpoints with `printf` actions are too expensive.** + Adding `CommandInterpreter::SendMovementEvent` (called every motion + tick, ~30 Hz) with a `printf` action lagged retail enough that the + ACE session timed out within a few seconds. **For per-frame + breakpoints, use counter-only actions** (`r $tN = @$tN + 1; gc`) — + no `.printf`, no `poi` reads. + +## Files + +- [motion_discovery.cdb](../../../.claude/worktrees/jovial-blackburn-773942/motion_discovery.cdb) + — symbol resolution + prologue dump (one-shot, kept for reference) +- [motion_trace.cdb](../../../.claude/worktrees/jovial-blackburn-773942/motion_trace.cdb) + — current motion-trace script (v4, with `.detach`) +- [run_motion_trace.ps1](../../../.claude/worktrees/jovial-blackburn-773942/run_motion_trace.ps1) + — launcher with timer-based fallback +- v1 log: walk + turn, no jump +- v2 log: run + jump + turn + +## Next session + +1. Decode `PHYSICS_STATE` enum from `acclient.h`. 30 minutes. +2. Single-bp focused trace of `Event_Jump` with `db` byte dump on the + pack, to determine send-rate vs charge-frame-rate. +3. Single-bp focused trace of `CommandInterpreter::SendMovementEvent` + (counter only) to confirm per-frame fire and measure rate. +4. Wire-bytes trace using `RawMotionState::Pack` + post-`Pack` byte + dump for one each of: idle, run-forward, run+turn, jump. diff --git a/docs/research/2026-05-01-retail-motion-trace/fixes.md b/docs/research/2026-05-01-retail-motion-trace/fixes.md new file mode 100644 index 0000000..c890a83 --- /dev/null +++ b/docs/research/2026-05-01-retail-motion-trace/fixes.md @@ -0,0 +1,267 @@ +# acdream vs retail motion outbound — gap analysis + fixes + +Companion to [findings.md](findings.md). Compares acdream's current +outbound motion code to retail's observed behaviour AND to holtburger +(the most authoritative working AC client we have, Rust TUI). + +## Summary table + +| Behaviour | Retail (live trace) | Holtburger (Rust client) | acdream | Verdict | +|-----------|---------------------|--------------------------|---------|---------| +| MoveToState dispatch trigger | Per-frame in `CommandInterpreter::SendMovementEvent`, gated by `InqRawMotionState != 0` and a `last_sent_position_time` rate-limit (rate unknown) | On motion-intent change (`last_server_motion_intent != current`) | On motion-state change (`MotionStateChanged`) | acdream ≈ holtburger; retail probably more aggressive but rate-limited — **likely OK** | +| AutonomousPosition cadence | ~1 Hz observed in v2 (~380 hits over ~5 min of activity) | **1.0 sec** const (`AUTONOMOUS_POSITION_HEARTBEAT_INTERVAL`) | **0.2 sec** (5 Hz) | **acdream is 5× too aggressive** | +| AutoPos at rest | Likely yes (gated by `transient_state & 1 && transient_state & 2`, not by motion) | Yes (gated by `has_autonomous_position_sync_target`, not motion) | **No** — acdream only ticks heartbeat while `isMoving` | **acdream MISSING at-rest heartbeat** | +| MoveToState content (heading) | float degrees 0–360 | float degrees 0–360 | float degrees 0–360 | ✓ match | +| HoldKey enum | Invalid=0 / None=1 / Run=2 | Same | Same (`HoldKeyNone=1`, `HoldKeyRun=2`) | ✓ match | +| Slash-command motion path | `SendDoMovementEvent` (zero hits in WASD trace — confirmed unused) | n/a | n/a | ✓ both correctly skip | +| Jump dispatch | `Event_Jump` fires per-frame while spacebar held; same `JumpPack` ptr suggests function-internal gating | (not yet checked) | Single `JumpAction` on spacebar release | **Unknown — needs deeper retail trace** | + +## Concrete fixes + +### Fix #1 — Heartbeat interval 200 ms → 1000 ms + +**Confidence:** high. Holtburger's constant is explicit and named; our retail +trace ratio (~380 hits / ~5 min) matches 1 Hz; ACE has no expectation +of a 200 ms cadence anywhere I can find. + +**Where:** [PlayerMovementController.cs:172](src/AcDream.App/Input/PlayerMovementController.cs:172) + +```csharp +public const float HeartbeatInterval = 0.2f; // 200ms +``` + +Change to: + +```csharp +// Holtburger constant + retail trace 2026-05-01. +// Ours used to be 0.2s — far too aggressive; observers may have +// interpreted the dense pulse stream as jitter. +public const float HeartbeatInterval = 1.0f; // 1000ms +``` + +**Risk:** dropping the heartbeat rate by 5× means observers' dead-reckoning +extrapolates over a longer interval between confirmation pulses. If our +position drift is bounded (no extreme client-side prediction), this is +fine — that's exactly what retail/holtburger do. Expect slightly less +"crisp" remote-observer view of the player's *idle* position, no +practical effect during motion (MoveToState fires on every state change, +which is much more frequent than 1 Hz under WASD activity). + +### Fix #2 — Send AutonomousPosition at rest, not just while moving + +**Confidence:** medium-high. Holtburger explicitly schedules the heartbeat +as long as a sync target exists, regardless of whether the player is +moving. Our retail trace was inconclusive at rest (cdb's attach +overhead made the "stand still" scenario unreliable) but the named +`SendPositionEvent` gate is on `transient_state` (in-world, alive, +valid pose), not on `is_moving`. + +**Where:** [PlayerMovementController.cs:765-779](src/AcDream.App/Input/PlayerMovementController.cs:765) + +```csharp +bool isMoving = outForwardCmd is not null + || outSidestepCmd is not null + || outTurnCmd is not null; +if (isMoving) +{ + _heartbeatAccum += dt; + HeartbeatDue = _heartbeatAccum >= HeartbeatInterval; + if (HeartbeatDue) _heartbeatAccum = 0f; +} +else +{ + _heartbeatAccum = 0f; + HeartbeatDue = false; +} +``` + +Change to (drop the `isMoving` gate, replace with a "valid pose" gate +that mirrors retail's `transient_state & 1 && transient_state & 2 && +Position::IsValid`): + +```csharp +// Heartbeat is a SYNC PULSE, not a motion broadcast. It fires whenever +// the player is in a valid in-world pose — at rest OR moving — so the +// server's last-known-position stays fresh. Holtburger uses 1 Hz +// regardless of motion; retail's SendPositionEvent gate is +// transient_state-based, not motion-based. +if (PlayerState == PlayerState.InWorld && Position.HasValidPose()) +{ + _heartbeatAccum += dt; + HeartbeatDue = _heartbeatAccum >= HeartbeatInterval; + if (HeartbeatDue) _heartbeatAccum = 0f; +} +else +{ + _heartbeatAccum = 0f; + HeartbeatDue = false; +} +``` + +(`Position.HasValidPose()` is illustrative — use whatever validity +predicate corresponds to "logged in, not portaling, not dead". The +`PlayerState.PortalSpace` early-return at the top of `Update` already +handles portal travel; the InWorld check above covers the rest.) + +**Risk:** the server gets ~1 extra packet per second while the player is +idle. Negligible bandwidth. Aligns with holtburger and (likely) retail. +Possible win: ACE may have been silently dropping the player's session +or marking them stale during long idle periods because we stopped +heart-beating. + +### Fix #3 — Jump velocity must be sent in body-LOCAL space, not world space + +**Confidence: very high.** This is THE bug behind "acdream jumps in +the wrong direction when observed from retail." + +**Evidence chain:** + +1. Retail's jump caller at `0x0056b1e7` does + ```c + CPhysicsObj::get_local_physics_velocity(player, &var_70); + JumpPack::JumpPack(&var_64, extent, &var_70 /*velocity*/, ...); + CM_Movement::Event_Jump(&var_64); + ``` + It explicitly uses `get_LOCAL_physics_velocity`, not the world-space + accessor. +2. The retail header (`acclient.h:54020`) defines + `JumpPack::velocity` as `AC1Legacy::Vector3`, with no + "this is local space" comment — but the construction site decides + this. The server reverses on receive: server applies the player's + heading rotation to transform the body-space velocity back into + world space for broadcast. +3. `get_local_physics_velocity` body + (`acclient.exe:0x00512140`) does + `local = fl2gv * worldVelocity` where `fl2gv` is a 3×3 row-major + matrix whose rows are the player's local axes expressed in world + coordinates. That's the inverse of the heading rotation — exactly + the world→local transform. +4. acdream's + [JumpAction.cs:64](src/AcDream.App/Input/PlayerMovementController.cs:64) + declares `JumpVelocity` as + `// world-space launch velocity (sent in jump packet)`. + [JumpAction.cs:433](src/AcDream.App/Input/PlayerMovementController.cs:433) + captures `outJumpVelocity = _body.Velocity;` — `_body.Velocity` is + in world space (verified by the gravity integration code that + subtracts world-Z from it each frame). +5. **Result:** observers receive an unrotated world vector, the + server's broadcast pipeline rotates it AGAIN by the player's + heading at receive-time. Net effect: jump direction is rotated by + `yaw` worth of rotation in the wrong direction. Standing facing + north and jumping forward → observer sees the player jump + sideways. Standing facing east and jumping forward → observer sees + the player jump backward. Etc. **Exactly the symptom reported.** + +**Where to fix:** [PlayerMovementController.cs:433](src/AcDream.App/Input/PlayerMovementController.cs:433) +right where `outJumpVelocity = _body.Velocity` is captured. + +```csharp +// BEFORE: +outJumpVelocity = _body.Velocity; // capture after LeaveGround applies it +``` + +```csharp +// AFTER: +// Retail sends jump velocity in BODY-LOCAL space (forward / right / +// up relative to the player's facing) — see CPhysicsObj:: +// get_local_physics_velocity at 0x00512140 and the call site at +// 0x0056b1e7. The server applies the player's heading on receive to +// rotate body→world; if we send world-space, observers see the +// jump rotated by `yaw`. Convert via inverse-yaw rotation around Z. +var worldVel = _body.Velocity; +float cy = MathF.Cos(Yaw); +float sy = MathF.Sin(Yaw); +outJumpVelocity = new Vector3( + cy * worldVel.X + sy * worldVel.Y, // local-axis-0 component + -sy * worldVel.X + cy * worldVel.Y, // local-axis-1 component + worldVel.Z); // local-axis-2 (gravity-aligned) +``` + +**Caveat — verify the sign of `Yaw` and the AC local-axis convention +before merging.** AC's heading convention is 0° = north = +Y, growing +clockwise toward east = +X. The above formula assumes +`world = R(Yaw) * local` so `local = R(-Yaw) * world`. If acdream's +`Yaw` is signed the other way, flip the signs of the `sy` terms. A +two-test verification: jump while facing north (cy=1, sy=0) — the +rotation should be a no-op and `local == world`; jump while facing +east (cy=0, sy=1) — the world `(1, 0, 0)` should map to local +`(0, -1, 0)` (i.e., to the right of facing-north when standing facing +east means body-axis-1 negative). + +**Confirm via cdb later (not blocking the fix):** trace a single +retail jump-forward, dump the JumpPack at `Event_Jump` entry via +`dt acclient!JumpPack poi(esp+4)`. The `velocity.x/y/z` fields will be +the canonical local-space values for "jump forward". Compare to +acdream's outbound after the fix — they should match within FP +rounding. + +### Fix #4 — Event_Jump send-rate (research-only, low priority) + +Retail's `Event_Jump` fires per-frame while the spacebar is held to +charge. Same `JumpPack` ptr (`001af5f8`) on every hit indicates a +stack-allocated local in the caller — function probably has internal +gating that decides send-vs-skip based on a state delta. acdream +sends ONE `JumpAction` per spacebar release. Until we trace inside +`Event_Jump` to count actual sends, leave acdream's behaviour alone. +Filed at low priority because the wrong-direction bug (Fix #3) is +the bigger issue. + +## Things that look RIGHT + +These don't need changes — written down so we don't second-guess them +later: + +- **MoveToState send-on-state-change.** Holtburger does the same. The + motion-state delta detection in + [PlayerMovementController.cs:744-756](src/AcDream.App/Input/PlayerMovementController.cs:744) + (forward-cmd, sidestep, turn, speed, run-hold, local-anim-cmd) is the + right set of fields to compare. +- **WASD does not invoke `SendDoMovementEvent`.** That's a slash-command + path retail keeps for `/run`, `/walk`, etc. Our outbound correctly + skips it. +- **HoldKey encoding** (Invalid=0/None=1/Run=2). Matches retail and + holtburger. +- **Heading float-degrees encoding.** Matches retail (we decoded 8 + distinct angles cleanly to 0–360°). +- **Per-axis hold-key broadcast.** acdream's + [GameWindow.cs:4949-4952](src/AcDream.App/Rendering/GameWindow.cs:4949) + sends `axisHoldKey` for every active axis — matches holtburger's + `build_motion_state_raw_motion_state`. + +## Test plan after fixes + +For Fix #1 + Fix #2 together: + +1. Build + run acdream, log in as `+Acdream`. +2. Open Wireshark on loopback `127.0.0.1`, filter on UDP port 9000. +3. Walk forward for 5 sec, stop for 5 sec, walk again for 5 sec, stop + for 5 sec, log out. +4. In Wireshark, count outbound 0xF753 (AutonomousPosition) packets + over the 20 sec window. + - **Pre-fix:** expect ~50 (5 Hz × 10 sec moving + 0 at rest). + - **Post-fix:** expect ~20 (1 Hz × 20 sec, both moving and at rest). +5. Have a retail observer in-world watching `+Acdream`. Stand still + for 30 sec without moving. Pre-fix: retail might mark us stale or + show our idle pose desyncing. Post-fix: should stay synced. + +## Files to modify + +Both fixes touch one file: + +- [src/AcDream.App/Input/PlayerMovementController.cs](src/AcDream.App/Input/PlayerMovementController.cs) + - Line 172: `HeartbeatInterval` constant + - Lines 765–779: `isMoving` gate around `_heartbeatAccum` update + +No changes to message builders ([MoveToState.cs](src/AcDream.Core.Net/Messages/MoveToState.cs), +[AutonomousPosition.cs](src/AcDream.Core.Net/Messages/AutonomousPosition.cs), +[JumpAction.cs](src/AcDream.Core.Net/Messages/JumpAction.cs)) needed. +No changes to wire-level dispatch in +[GameWindow.cs](src/AcDream.App/Rendering/GameWindow.cs) needed. + +## Tracking + +File these as ISSUES.md entries when implementing: + +- **#motion.heartbeat-interval** (Fix #1) — `HeartbeatInterval = 1.0f`. ~10 lines, plus test. +- **#motion.heartbeat-at-rest** (Fix #2) — drop the `isMoving` gate. ~15 lines, plus test that exercises the at-rest pulse. +- **#motion.jump-charge-rate** (Fix #3) — research-only until retail trace lands. Don't change code yet. diff --git a/src/AcDream.App/Input/PlayerMovementController.cs b/src/AcDream.App/Input/PlayerMovementController.cs index e4fc8ec..c904b26 100644 --- a/src/AcDream.App/Input/PlayerMovementController.cs +++ b/src/AcDream.App/Input/PlayerMovementController.cs @@ -61,7 +61,7 @@ public readonly record struct MovementResult( float LocalAnimationSpeed = 1f, bool JustLanded = false, // true on the single frame we transitioned airborne → grounded float? JumpExtent = null, // non-null when a jump was triggered this frame - Vector3? JumpVelocity = null); // world-space launch velocity (sent in jump packet) + Vector3? JumpVelocity = null); // BODY-LOCAL launch velocity (forward/right/up relative to facing) — see PlayerMovementController jump path for the inverse-yaw conversion. Server rotates body→world on broadcast. /// /// Portal-space state for the player movement controller. @@ -168,8 +168,13 @@ public sealed class PlayerMovementController private uint? _prevLocalAnimCmd; // Heartbeat timer. + // Cadence is 1.0 sec to match holtburger's + // AUTONOMOUS_POSITION_HEARTBEAT_INTERVAL and the retail trace + // (2026-05-01 motion-trace findings.md): retail sends ~1 Hz at rest, + // not the 5 Hz our pre-fix code used. Sending at 5 Hz was harmless + // but wasteful and probably looked like jitter to observers. private float _heartbeatAccum; - public const float HeartbeatInterval = 0.2f; // 200ms + public const float HeartbeatInterval = 1.0f; // 1 sec — retail / holtburger public bool HeartbeatDue { get; private set; } // L.5 retail physics-tick gate (2026-04-30). @@ -428,9 +433,72 @@ public sealed class PlayerMovementController var jumpResult = _motion.jump(_jumpExtent); if (jumpResult == WeenieError.None) { + // Capture jump_v_z BEFORE LeaveGround() — that call resets + // JumpExtent back to 0 (faithful to retail's FUN_00529710), + // after which get_jump_v_z() returns 0 because the extent + // gate at the top of the function fires. + float jumpVz = _motion.get_jump_v_z(); _motion.LeaveGround(); outJumpExtent = _jumpExtent; - outJumpVelocity = _body.Velocity; // capture after LeaveGround applies it + // BODY-LOCAL jump-launch velocity, computed directly from input. + // + // Why not read _body.Velocity? Because _motion.LeaveGround() + // routes through get_leave_ground_velocity → get_state_velocity, + // which is a faithful port of retail's FUN_00528960. Retail's + // version only handles WalkForward (0x45000005) / RunForward + // (0x44000007) / SideStepRight (0x6500000F); WalkBackwards + // and SideStepLeft return zero. Retail papers over this in + // adjust_motion (FUN_00528010) by translating + // WalkBackwards → WalkForward + speed × -0.65 + // SideStepLeft → SideStepRight + speed × -1 + // before they reach InterpretedState — but we don't yet port + // adjust_motion, so InterpretedState holds the un-translated + // command and get_state_velocity returns (0,0,0) for it. + // LeaveGround then writes (0,0,jumpZ) to the body, wiping the + // correct strafe/backward velocity the controller had just set + // a few lines up. Result: backward/strafe jumps go straight up. + // + // Until adjust_motion is ported, we mirror the grounded-velocity + // computation from the block above and stuff the result into + // outJumpVelocity directly. Local frame: +Y forward, +X right, + // +Z up — matches retail's body-frame convention. Server + // rotates body→world on receive, so observers see the jump + // in the correct world direction. + float jumpRunMul = 1.0f; + if (input.Run && _weenie.InqRunRate(out float jvrr)) + jumpRunMul = jvrr; + + // Forward uses get_state_velocity (which knows Walk vs Run vs + // animation-cycle pacing). Backward / Strafe use the same + // hardcoded scaled formulas the grounded-velocity block above + // uses (lines 397-408). + float localY = 0f; + if (input.Forward) + { + var stateVel = _motion.get_state_velocity(); + localY = stateVel.Y; + } + else if (input.Backward) + { + localY = -(MotionInterpreter.WalkAnimSpeed * 0.65f * jumpRunMul); + } + + float localX = 0f; + if (input.StrafeRight) + localX = MotionInterpreter.SidestepAnimSpeed * jumpRunMul; + else if (input.StrafeLeft) + localX = -MotionInterpreter.SidestepAnimSpeed * jumpRunMul; + + outJumpVelocity = new Vector3(localX, localY, jumpVz); + + // Local-prediction fix: LeaveGround above wrote (0, 0, jumpZ) + // to the body for backward/strafe-left (same get_state_velocity + // zero-for-non-canonical-motion bug as on the wire side). + // Push the corrected body-local velocity back so the local + // client renders the jump in the same world direction the + // server is broadcasting to observers. Same vector we just + // sent in JumpAction — local + remote stay in sync. + _body.set_local_velocity(outJumpVelocity.Value); } _jumpCharging = false; _jumpExtent = 0f; @@ -762,21 +830,19 @@ public sealed class PlayerMovementController return System.Math.Abs(a.Value - b.Value) < 1e-4f; } - // ── 8. Heartbeat timer (only while moving) ──────────────────────────── - bool isMoving = outForwardCmd is not null - || outSidestepCmd is not null - || outTurnCmd is not null; - if (isMoving) - { - _heartbeatAccum += dt; - HeartbeatDue = _heartbeatAccum >= HeartbeatInterval; - if (HeartbeatDue) _heartbeatAccum = 0f; - } - else - { - _heartbeatAccum = 0f; - HeartbeatDue = false; - } + // ── 8. Heartbeat timer (always while in-world, not just while moving) ─ + // Holtburger fires AutonomousPosition heartbeat at 1 Hz regardless of + // motion state (gated only by has_autonomous_position_sync_target). + // Retail's CommandInterpreter::SendPositionEvent gates on + // transient_state (Contact + OnWalkable + valid Position), not on + // motion. The pre-fix isMoving gate stopped acdream from heart-beating + // at rest, which left observers with stale last-known positions during + // long idle periods. PortalSpace (handled at the top of Update via + // early return) skips Update entirely, so reaching this line implies + // we're in a valid in-world pose. + _heartbeatAccum += dt; + HeartbeatDue = _heartbeatAccum >= HeartbeatInterval; + if (HeartbeatDue) _heartbeatAccum = 0f; // K-fix5 (2026-04-26): local-animation-cycle pacing. Visual rate // should match the actual movement speed. For Forward+Run this is @@ -801,7 +867,14 @@ public sealed class PlayerMovementController ForwardSpeed: outForwardSpeed, SidestepSpeed: outSidestepSpeed, TurnSpeed: outTurnSpeed, - IsRunning: input.Run && input.Forward, + // Run hold-key applies to ANY active directional axis, not just + // forward (per holtburger's build_motion_state_raw_motion_state: + // "uses the same value for every active per-axis hold key"). The + // pre-fix condition `input.Run && input.Forward` made strafe-run + // and backward-run incorrectly broadcast as walk to observers, + // who then animated walk + dead-reckoned at walk speed while the + // server position moved at run speed — visible as observer lag. + IsRunning: input.Run && anyDirectional, LocalAnimationCommand: localAnimCmd, LocalAnimationSpeed: localAnimSpeed, JustLanded: justLanded, diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 34e2df9..e03217b 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -1926,6 +1926,20 @@ public sealed class GameWindow : IDisposable Console.WriteLine($"\n=== DUMP_CLOTHING: guid=0x{spawn.Guid:X8} name='{spawn.Name}' setup=0x{setup.Id:X8} APC={animPartChanges.Count} ==="); foreach (var c in animPartChanges) Console.WriteLine($" APC part={c.PartIndex:D2} -> gfx=0x{c.NewModelId:X8}"); + + // #37: per-spawn palette swaps. The server's clothing pipeline + // sends a basePalette + a list of (subPaletteId, offset, length) + // triples that splice palette ranges into the rendered character. + // We need their IDs to know whether the coat texture's underlying + // palette is being overridden by a coat-tone subPalette or left + // alone (in which case the texture's DefaultPaletteId — a SKIN + // palette — leaks through and the coat ends up neck-colored). + Console.WriteLine($" basePalette=0x{(spawn.BasePaletteId ?? 0):X8} subPalettes={(spawn.SubPalettes?.Count ?? 0)}"); + if (spawn.SubPalettes is { } subPaletteList) + { + foreach (var subPal in subPaletteList) + Console.WriteLine($" SP id=0x{subPal.SubPaletteId:X8} offset={subPal.Offset} length={subPal.Length}"); + } } foreach (var change in animPartChanges) { @@ -2722,7 +2736,14 @@ public sealed class GameWindow : IDisposable // get_state_velocity returns 0 because the gate is // RunForward||WalkForward — body stops moving forward. remoteMot.Motion.InterpretedState.ForwardCommand = fullMotion; - remoteMot.Motion.InterpretedState.ForwardSpeed = speedMod <= 0f ? 1f : speedMod; + // Pass speedMod through verbatim — preserve sign so retail's + // adjust_motion'd backward walk (cmd=WalkForward, spd<0) + // produces backward velocity in get_state_velocity, NOT + // forward. Pre-fix used `<=0 ? 1 : speedMod` which clamped + // negative to 1.0 and made the dead-reckoned body translate + // forward despite the reverse-playback animation — visually + // "still walking forward" from the observer's POV. + remoteMot.Motion.InterpretedState.ForwardSpeed = speedMod; if (update.MotionState.IsServerControlledMoveTo && update.MotionState.MoveToPath is { } path) diff --git a/src/AcDream.Core/Physics/AnimationSequencer.cs b/src/AcDream.Core/Physics/AnimationSequencer.cs index ffce8e1..a6d57ba 100644 --- a/src/AcDream.Core/Physics/AnimationSequencer.cs +++ b/src/AcDream.Core/Physics/AnimationSequencer.cs @@ -389,11 +389,18 @@ public sealed class AnimationSequencer // This keeps the run/walk loop smooth when a new UpdateMotion arrives // with a different ForwardSpeed (e.g. when the server broadcasts a // player's updated RunRate mid-step). + // + // **Sign-flip case (2026-05-02):** when the server sends adjust_motion'd + // backward walk as `WalkForward + speed=-N`, motion stays 0x45000005 + // but speedMod sign flips. We MUST do a full cycle restart in that case + // so the new (negative) framerate takes effect; otherwise the cycle + // keeps playing forward with the old positive framerate and the + // observer sees the player walking forward despite the negative speed. if (CurrentStyle == style && CurrentMotion == motion - && _firstCyclic != null && _queue.Count > 0) + && _firstCyclic != null && _queue.Count > 0 + && MathF.Sign(speedMod) == MathF.Sign(CurrentSpeedMod)) { if (MathF.Abs(speedMod - CurrentSpeedMod) > 1e-4f - && MathF.Sign(speedMod) == MathF.Sign(CurrentSpeedMod) && MathF.Abs(CurrentSpeedMod) > 1e-6f) { MultiplyCyclicFramerate(speedMod / CurrentSpeedMod);