From 9e1d33a5f7467222465e537323a7772cf2fe4bff Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 14 May 2026 17:56:35 +0200 Subject: [PATCH] docs(B.6): retail decomp settles Option A; revise spec with 4-slice plan Grounded the design in named-retail evidence. MovementManager::Perform Movement at 0x00524440 case 6 (decomp lines 300628-300648) shows the retail client's local-side dispatcher for inbound MoveToObject: unpacks the wire, sets motion_interpreter->my_run_rate, calls CPhysicsObj::MoveToObject on the LOCAL player's physics body. Same code path retail used for every creature chasing the player. Conclusion: Option A (run a local driver against the player's body) is retail-faithful. Option C (server-position-blend) is a non-retail shortcut and is now eliminated from consideration. Re-scoped the spec into 4 slices: 1. ACDREAM_PROBE_AUTOWALK diagnostic baseline (~30 LOC) 2. PlayerMovementController.BeginServerAutoWalk + reuse of RemoteMoveToDriver against the local player's body (~100 LOC) 3. Animation cycle selection during auto-walk (~20 LOC) 4. Local pickup-animation echo (closes #64, ~10 LOC) Total ~160 LOC, no new files. All existing acdream infrastructure (RemoteMoveToDriver, ServerControlledLocomotion, MotionState.MoveTo Path parsing) is reused; the work is wiring it for _playerServerGuid in addition to remote guids. --- .../specs/2026-05-14-phase-b6-design.md | 196 +++++++++++++----- 1 file changed, 142 insertions(+), 54 deletions(-) diff --git a/docs/superpowers/specs/2026-05-14-phase-b6-design.md b/docs/superpowers/specs/2026-05-14-phase-b6-design.md index 214a683..f6e03bc 100644 --- a/docs/superpowers/specs/2026-05-14-phase-b6-design.md +++ b/docs/superpowers/specs/2026-05-14-phase-b6-design.md @@ -164,22 +164,73 @@ ACE's broadcast cadence is too sparse, the motion is choppy. ## Recommendation -Start with **Option C** for the minimum viable fix; promote to -**Option A** if Option C produces choppy/janky motion. Skip Option B -unless A and C both prove harder than they look. +**Option A is the retail-faithful path; Options B and C are non-retail +shortcuts. Implement Option A.** -**Why C first.** -- Smallest blast radius. Can be a single-commit hotfix. -- Reveals exactly what ACE is sending in practice (the diagnostic - layer below tells us cadence + format). -- If Option C works smoothly, B.6 is done in a single session. -- If Option C is too choppy, the diagnostic data informs Option A's - design — what cadence does the per-tick driver need to fill in - between server-position broadcasts? +### Retail evidence (settles A vs C) -**Why not B.** Tweens aren't retail-faithful; we'd carry a -non-retail visual band-aid into M2 and have to revisit when combat -movement starts depending on real physics paths. +`MovementManager::PerformMovement` at retail address `0x00524440` +(decomp `docs/research/named-retail/acclient_2013_pseudo_c.txt` lines +300628–300648) is the inbound-motion dispatcher. The switch on movement +type has explicit cases for `MoveToObject (6)` and `MoveToPosition (7)` +that: + +1. Call `MovementManager::MakeMoveToManager(this)` to ensure the + per-physics-object manager exists. +2. Unpack the target guid + origin position + `MovementParameters` from + the wire. +3. Set `this->motion_interpreter->my_run_rate` from the packet. +4. Call **`CPhysicsObj::MoveToObject(this->physics_obj, target_guid, + ¶ms)`** — which kicks off a **fully local** auto-walk on the + player's own physics body (`0x00512860`, decomp line 280598). +5. Fall through to `MoveToManager::MoveToPosition` only if the target + guid isn't found in the local physics world (rare; usually means the + client hasn't streamed in the target yet). + +This is the **same code path** that runs for every remote creature +chasing the player — retail did not have a separate "remote-only" vs +"local-only" auto-walk pipeline. The local client's MoveToManager +actively drove the local player's body forward when the server sent +MoveToObject. ACE's server-side `PhysicsObj.MoveToObject` simulation +runs in parallel for authoritative-position tracking + arrival +detection (`MoveToChain.WithinUseRadius`), but the visible movement on +the local client comes from the local MoveToManager — not from +inbound UpdatePosition packets. + +Option C would diverge from retail by relying on server position +broadcasts instead of local physics integration. That risks combat +movement, environment-trigger interactions, and animation hooks all +diverging from retail because they'd be driven by sparse server-side +position snapshots rather than smooth local integration. + +### Existing acdream infrastructure that's already retail-shaped + +We have most of the building blocks already: + +- [`RemoteMoveToDriver`](../../../src/AcDream.Core/Physics/RemoteMoveToDriver.cs) + is the per-tick steering loop — heading correction, arrival via + `min_distance` / `distance_to_object`, ±20° aux-turn tolerance — + ported from retail's `MoveToManager::HandleMoveToPosition` + (`0x00529d80`). It's already exercised on every NPC chase. The + retail-faithful fix for the local player **reuses this driver**, + installed against the local player's body instead of a remote's + dead-reckoned body. +- [`ServerControlledLocomotion.PlanMoveToStart`](../../../src/AcDream.Core/Physics/ServerControlledLocomotion.cs) + already does what retail's `MovementParameters::get_command` + (`0x0052AA00`) does: seed `WalkForward` / `RunForward` depending on + `CanRun` + `MoveToSpeed` + `MoveToRunRate`. +- `MotionState.MoveToPath` is fully parsed on the wire. Remote chase + reads it at `GameWindow.cs:3401–3417`. + +The B.6 work is essentially **"do for `_playerServerGuid` what we +already do for remotes,"** with one extra concern: the local player has +a user-input motion source (`PlayerMovementController`) that has to +yield to the auto-walk while it's active. + +### Why not B + +Tweens aren't retail-faithful and would diverge worse than C. +Eliminated. --- @@ -201,45 +252,72 @@ characterized in detail. We need a live trace of: local-player auto-walk attempts. Roughly 30 LOC; mirrors L.2a slice 1's `ACDREAM_PROBE_RESOLVE` / `ACDREAM_PROBE_CELL` pattern. -The first 30 minutes of the next B.6 session should produce a clean -trace of a single failed auto-walk attempt, from outbound packet -through ACE's `ActionCancelled`. The trace decides between Option A -and Option C. +The trace is no longer needed to *decide between options* (retail +decomp settles that — Option A wins), but it remains valuable as a +**baseline measurement** for the Option A implementation: knowing what +ACE sends today lets us verify the local driver behaves equivalently +on the wire (no extra packets needed, position broadcasts arrive at +expected cadence, the auto-walk completes inside +`defaultMoveToTimeout` instead of timing out). --- -## File-level scope sketch (Option C path) +## File-level scope sketch (Option A — retail-faithful) -If the trace confirms ACE broadcasts adequate `UpdatePosition` during -auto-walk: +Mirror retail's `MovementManager::PerformMovement` case 6 against +acdream's existing `PlayerMovementController` + `RemoteMoveToDriver`. -- **Modify:** [`src/AcDream.Core/Physics/PlayerMovementController.cs`](../../../src/AcDream.Core/Physics/PlayerMovementController.cs) - — add an `IsServerAutoWalking` flag + setter. While true, suppress - the user-input snap-back path in the reconciliation loop. -- **Modify:** [`src/AcDream.App/Rendering/GameWindow.cs`](../../../src/AcDream.App/Rendering/GameWindow.cs) - `OnLiveMotionUpdated` — when `update.Guid == _playerServerGuid` and - `IsServerControlledMoveTo`, set the flag on the controller. When a - non-MoveTo motion arrives for the player (Ready, RunForward initiated - by user, etc.), clear it. -- **Modify:** `GameWindow.OnLivePositionUpdated` (or equivalent) — while - the auto-walk flag is set, trust server position without - reconciliation. +**Slice 1 — diagnostic baseline (~30 LOC).** +- **Modify:** `src/AcDream.Core/Physics/PhysicsDiagnostics.cs` — add + `ProbeAutoWalkEnabled` flag gated on `ACDREAM_PROBE_AUTOWALK=1`. +- **Modify:** `GameWindow.OnLiveMotionUpdated`, `OnLivePositionUpdated`, + `OnVectorUpdated`, `SendUse`, `SendPickUp` — when probe is on and the + guid is `_playerServerGuid`, log one line per event with timestamp + + payload. Mirror the `[resolve]` / `[cell-transit]` line format from + L.2a. -Total: estimated ~60 LOC + diagnostic. +**Slice 2 — install auto-walk on inbound MoveToObject (~100 LOC).** +- **Modify:** `PlayerMovementController` — add `BeginServerAutoWalk( + Vector3 destinationWorld, float minDistance, float distanceToObject, + bool moveTowards, float moveToSpeed, float moveToRunRate, bool + canRun)` + `EndServerAutoWalk(reason)` methods. The controller owns + the "input vs auto-walk" state. While auto-walking, the per-tick + update calls `RemoteMoveToDriver.Step(...)` against its own body, and + the user-input motion path is suppressed. +- **Modify:** `GameWindow.OnLiveMotionUpdated` — when `update.Guid == + _playerServerGuid && IsServerControlledMoveTo && MoveToPath is + not null`, translate the path's `OriginCellId + OriginXYZ` to world + space (same `RemoteMoveToDriver.OriginToWorld` helper the remote + path uses), call `_playerController.BeginServerAutoWalk(...)`. + Otherwise (a non-MoveTo motion arrives for the player), call + `EndServerAutoWalk(reason="motion-changed")`. +- **Modify:** `PlayerMovementController.Tick` — if auto-walking, + consume input only for cancellation (W/A/S/D pressed → cancel + auto-walk → restore input); skip the input-driven velocity solve; + let `RemoteMoveToDriver.Step` set the body's velocity + heading; + apply arrival check via `min_distance` / `distance_to_object`; on + arrival, call `EndServerAutoWalk(reason="arrived")`. -## File-level scope sketch (Option A path, if needed) +**Slice 3 — animation cycle selection (~20 LOC).** +- **Modify:** `GameWindow` `UpdatePlayerAnimation` (the path that + drives the local player's animation cycle from user input) — when + the controller is in `IsServerAutoWalking` state, source the cycle + from `ServerControlledLocomotion.PlanMoveToStart(...)` instead of the + user-input MotionInterpreter. This is what makes the local player + visibly walk + animate during the auto-walk. -- All of Option C's changes, plus: -- **New:** `src/AcDream.Core/Physics/LocalAutoWalkDriver.cs` — port the - `RemoteMoveToDriver` logic adapted for the local player's body, including - arrival detection + cycle selection. -- **Modify:** `PlayerMovementController` — integrate the driver into the - per-tick update loop when auto-walking; suppress input-derived - velocity while active. -- **Modify:** `GameWindow.OnLiveMotionUpdated` — wire the driver - installation + teardown. +**Slice 4 — local pickup animation (probably closes #64, ~10 LOC).** +- **Modify:** `OnLiveMotionUpdated` — for `_playerServerGuid`, allow + `Motion(Pickup)` / `Motion(Pickup5/10/15/20)` to drive `SetCycle` + bypassing the existing self-echo filter at `GameWindow.cs:3289`. + This is the bend-down animation retail observers see when we pick up + an item. Same one-shot mechanism retail used; `UpdatePlayerAnimation` + doesn't predict it because it's server-initiated, so admitting the + echo is correct. -Total: estimated ~150–250 LOC including tests. +Total: estimated ~160 LOC + unit tests for the controller state +machine. No new files; all changes land in existing physics + render +infrastructure. --- @@ -281,18 +359,28 @@ Total: estimated ~150–250 LOC including tests. ## Carry-overs from B.5 -- **#64** — local-player pickup animation. Likely related: same self- - echo filter at `OnLiveMotionUpdated:3289` that ignores MoveToObject - also drops the inbound `Motion(Pickup)` that retail observers render - correctly. May fix in the same B.6 work, or as a follow-up depending - on how the auto-walk fix shakes out. +- **#64** — local-player pickup animation. Same self-echo filter at + `OnLiveMotionUpdated:3289` that ignores MoveToObject also drops the + inbound `Motion(Pickup)` retail observers render correctly. Slice 4 + above admits server-initiated one-shot motions through the filter + for the local player, which should close #64. Verify in the same + visual-test pass. --- ## State at design freeze -- **Main HEAD:** `5053e40` (post-B.5 polish; M1 mechanically clean). -- **No code changes in this commit** — design document only. -- **Next session entry point:** add the `ACDREAM_PROBE_AUTOWALK` - diagnostic, run a failed auto-walk reproduction, decide A vs C from - the trace. +- **Main HEAD:** `281d125` (initial B.6 design spec committed). +- **No code changes in this spec commit** — design document only. +- **Spec update 2026-05-14 (this commit):** retail decomp at + `MovementManager::PerformMovement` (`0x00524440` case 6, decomp lines + 300628–300648) decisively settles A vs C in favor of **Option A**. + Retail's local client ran its own `MoveToManager` and called + `CPhysicsObj::MoveToObject` on the local player's body. Option C + (server-position-blend) is not retail-faithful and is no longer + considered. +- **Next session entry point:** Slice 1 — add the + `ACDREAM_PROBE_AUTOWALK` diagnostic as the baseline, run a failed + auto-walk reproduction for a clean trace, then proceed to Slice 2 + (`PlayerMovementController.BeginServerAutoWalk` + `RemoteMoveToDriver` + reuse for the local player).