acdream/docs/research/2026-05-01-retail-motion-trace/fixes.md
Erik 17a9ff1158 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

267 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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 0360 | float degrees 0360 | float degrees 0360 | ✓ 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 0360°).
- **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 765779: `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.