fix(motion): jump direction, AutoPos cadence, backward/strafe wire & anim

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 <noreply@anthropic.com>
2026-05-02 16:11:15 +02:00 · 2026-05-02 16:11:15 +02:00 · 17a9ff1158
commit 17a9ff1158
parent 09e013b7bd
6 changed files with 691 additions and 22 deletions
--- a/docs/research/2026-05-01-retail-motion-trace/fixes.md
+++ 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.