acdream/docs/research/2026-07-02-r3-motioninterp/w6-cutover-map.md
Erik fc5a2cda28 docs(R3-W6a): local-player cutover map — re-anchored edit list + edge table + risk decisions
Five discoveries beyond the plan text: ChargeJump never called by
production code (StandingLongJump has never armed — W6 must wire the
charge-start call); the DefaultSink→sequencer path already exists
end-to-end (W6 = bind one property + delete UpdatePlayerAnimation +
the edge detector); skipTransitionLink already gone (stale plan
bullet); the #45 sidestep factor + ACDREAM_ANIM_SPEED_SCALE have no
equivalent in the shared sink path (decision required); ApplyServerRunRate's
apply_current_movement goes live once DefaultSink binds (fast re-speed
absorbs it — retail's own mid-run speed-change mechanism).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-03 09:06:42 +02:00

726 lines
41 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.

# R3-W6 cutover-execution-map — local player motion unification
Research-only doc. Re-anchors `r3-port-plan.md` §3 W6 (which cites stale
line numbers) against the CURRENT code, post-W1-W5. Every anchor below was
read directly from the working tree at HEAD (`e214acdf`) during this
session — no line number here is inherited from the plan.
Inputs read: `r3-port-plan.md` §3 W6 + §1 row J15 + §4 (sink wiring
contract); `r3-motioninterp-decomp.md` §5c/§8 (`set_hold_run` callers);
`src/AcDream.App/Input/PlayerMovementController.cs` (full file, 1547
lines); `src/AcDream.App/Rendering/GameWindow.cs` (targeted sections via
subagent + direct reads); `src/AcDream.Core/Physics/MotionInterpreter.cs`
(full file, 3083 lines); `src/AcDream.Core/Physics/RawMotionState.cs`;
`src/AcDream.Core/Physics/Motion/MovementParameters.cs`; the test tree.
---
## §1 Current-state inventory — every member/block that DIES
### 1a. Per-frame `RawMotionState` rebuild + `apply_raw_movement(RawMotionState)` call
**`PlayerMovementController.cs:892-924`** (comment block + body, inside
`Update`). Every frame, regardless of whether input changed, builds a
fresh `RawMotionState` from `MovementInput` and pushes it through the
snapshot-consuming overload:
```csharp
var axisHold = input.Run ? HoldKey.Run : HoldKey.None;
var raw = new RawMotionState { CurrentHoldKey = axisHold, ForwardCommand = ..., ... };
_motion.apply_raw_movement(raw);
```
This is retail's `apply_raw_movement(RawMotionState raw)` overload
(`MotionInterpreter.cs:1346-1371`) — it copies 6 fields into
`InterpretedState` and normalizes each axis via `adjust_motion`, but does
**not** drive velocity or dispatch through a sink. It's a **level-triggered**
substitute for retail's **edge-triggered** `DoMotion`/`StopMotion`/
`set_hold_run` calls that mutate `RawState` incrementally via
`RawMotionState.ApplyMotion`/`RemoveMotion`. DIES in W6 — replaced by §2's
edge table.
### 1b. Section 2 grounded-velocity write (STAYS, not a W6 kill — flagged for clarity)
**`PlayerMovementController.cs:962-991`**. Reads
`_motion.get_state_velocity()` and writes `_body.set_local_velocity(...)`
when grounded. The plan (§3 W6 "STAYS") keeps this — it's the R6-deferred
"grounded velocity from state" adaptation (register row). Listed here only
so the doc distinguishes it from 1a, which it's textually adjacent to.
### 1c. Jump charge/fire block
**`PlayerMovementController.cs:993-1043`**. Charge accumulates via
`_jumpCharging`/`_jumpExtent` (unchanged — AP-24, stays controller-side).
On release, calls:
```csharp
var jumpResult = _motion.jump(_jumpExtent);
```
**Confirmed gap (not in the plan's text, found this session):**
`MotionInterpreter.ChargeJump()` (`MotionInterpreter.cs:1765-1789`, the
verbatim `charge_jump` port, the ONLY place `StandingLongJump` arms) is
**never called** by the controller today — only by
`MotionInterpreterJumpFamilyTests.cs`. The doc comment at
`MotionInterpreter.cs:1756-1762` says outright: *"no regression today
since nothing calls ChargeJump yet, so remotes/local both continue
unaffected."* W6 must add a `ChargeJump()` call at charge-START (the
retail caller is the SmartBox/input boundary at `0x0056afac`, out of
CMotionInterp's own scope but exactly where the controller's charge
block lives). Without this, `StandingLongJump` never arms for the local
player post-cutover either — same bug, just moved. See §6 risk R1.
The manual `LeaveGround()` pre-capture this block used to need was
**already deleted in W4** (comment at `:1016-1021` confirms: "the manual
LeaveGround call is DELETED — jump() clears OnWalkable, and the SAME
frame's grounded→airborne edge... fires `_motion.LeaveGround()`"). This
part of the plan's W6 description is stale — it's W4-done, not a W6 task.
### 1d. `LocalAnimationCommand` / `LocalAnimationSpeed` synthesis
**`MovementResult` record fields**: `PlayerMovementController.cs:55`
(`LocalAnimationCommand`) and `:62` (`LocalAnimationSpeed`), part of the
record at `:42-65`.
**Synthesis sites**:
- `:1308-1322``localAnimCmd` set from `input.Forward`/`input.Backward`
+ `_weenie.InqRunRate` (Run cycle selection independent of wire command).
- `:1443-1481` — K-fix5 `localAnimSpeed` computation (run-rate pacing for
Backward+Run/Strafe+Run) + auto-walk override block
(`_autoWalkMovingForwardThisFrame`/`_autoWalkTurnDirectionThisFrame`
branches overwrite `localAnimCmd`/`localAnimSpeed`).
- `:1355-1377` — change-detection block includes `localAnimCmd !=
_prevLocalAnimCmd` as a `changed` trigger (7th line of the OR-chain,
`:1371`) and tracks `_prevLocalAnimCmd` (`:191`, `:1377`).
- `:1503-1504` — final `MovementResult` construction passes both fields
through.
DIES: the two fields, all three synthesis sites, and the
`_prevLocalAnimCmd` change-detection leg (the OTHER six `changed`
triggers survive unchanged — see §5).
### 1e. `SetPosition`'s arrival-idle `DoMotion(Ready)`
**`PlayerMovementController.cs:839`**, inside `SetPosition(Vector3, uint,
Vector3)`:
```csharp
_motion.DoMotion(MotionCommand.Ready, 1.0f);
```
Per the plan: "the `:825` `DoMotion(Ready)` arrival-idle becomes
`StopCompletely()` — retail's teleport idle is a full stop." Confirmed
`StopCompletely()` (`MotionInterpreter.cs:1054-1084`) is the correct
retail-faithful substitute: it resets forward/sidestep/turn COMMANDS
(not speeds, per J9), zeros `PhysicsObj.set_velocity`, enqueues the A9
jump-snapshot node, and fires `RemoveLinkAnimations` if uncelled — a
strictly more complete reset than the current one-field `DoMotion(Ready)`
call. Same substitution applies to the identical call inside
`DriveServerAutoWalk`'s turn-in-place branch (`:732`) — **the plan does
NOT call this one out**, but it is textually the same pattern
(`_motion.DoMotion(MotionCommand.Ready, 1.0f)`) serving the same "go
idle" role mid-auto-walk. Flagged as a judgment call in §6 (R4) — recommend
leaving `:732` as `DoMotion(Ready)` since auto-walk survives R3 verbatim
per the KEEP LIST and StopCompletely's jump-snapshot/queue side effects
are not obviously wanted mid-auto-walk-turn.
### 1f. Shift/Run detection
**Today**: `input.Run` is read directly into `axisHold` (1a) every frame
— no edge detection exists; the level-triggered rebuild makes edge
detection moot today. W6 replaces this with an explicit prev-Shift-state
edge feeding `set_hold_run(bool holdingRun, bool interrupt)`
(`MotionInterpreter.cs:2453-2463`), which itself has an internal XOR
no-op guard (so calling it every frame with the same value is safe, but
the controller should still gate on an edge per the plan's "tiny
prev-key-state edge detector").
### 1g. Outbound section 6 — re-derivation from `input`
**`PlayerMovementController.cs:1288-1354`** (`outForwardCmd`/
`outForwardSpeed`/`outSidestepCmd`/etc.) re-derives the wire commands
from `MovementInput` fields (`input.Forward`, `input.StrafeRight`, etc.)
completely independently of `_motion.RawState`, which by this point in
the frame (post 1a) already holds equivalent values. This is redundant
computation that DIES — the outbound-wire-build in GameWindow (§4/§5)
should read `_motion.RawState` (now the SAME instance the controller has
been mutating via `DoMotion`/`StopMotion`/`set_hold_run`) directly instead
of `MovementResult.ForwardCommand`/etc. **Note**: this doc treats "which
fields of `MovementResult` survive" as a §5 design decision, not a
foregone conclusion — see §5.
### 1h. GameWindow: `UpdatePlayerAnimation` (DELETE) + its caller
**Definition**: `GameWindow.cs:10139-10307` (169 lines).
**Single caller**: `GameWindow.cs:8006`, inside `OnUpdate`'s
`if (_playerController is not null)` block, immediately after the
JumpAction wire-send block (`:7984-8002`).
Reads `result.IsOnGround` (airborne→Falling override, `:10164-10165`),
`result.LocalAnimationCommand` (`:10166`, preferred source),
`result.ForwardCommand`/`SidestepCommand`/`TurnCommand` (`:10168-10172`,
fallback chain when `LocalAnimationCommand` is null),
`result.LocalAnimationSpeed` (`:10186`, `:10255-10257`, feeds `SetCycle`).
Calls `ae.Sequencer.SetCycle(fullStyle, animCommand, animSpeed *
animScale)` directly (`:10297`) — this is a **parallel path** to
`MotionTableDispatchSink.ApplyMotion` that remotes use; it does NOT go
through `_playerController.Motion` at all. DIES in full — the player's
`AnimatedEntity.Sequencer` gets driven through the SAME
`MotionTableDispatchSink` → `DefaultSink` funnel remotes use (§3).
### 1i. `input.` reads for MOTION (not camera) — full inventory
All in `PlayerMovementController.Update`, all replaced by edge calls
except where noted:
- `input.Forward`/`input.Backward` (`:907-909`, `:1309-1322`) → W press/S
press/release edges.
- `input.StrafeLeft`/`input.StrafeRight` (`:912-914`, `:1328-1337`) →
strafe press/release edges.
- `input.TurnLeft`/`input.TurnRight` (`:917-919`, `:1344-1353`) → turn
press/release edges. **Note**: `input.TurnRight`/`TurnLeft` ALSO drive
Yaw integration directly at `:946-951` — that consumption STAYS (reads
`_motion.InterpretedState.TurnCommand`, unchanged per the plan).
- `input.Run` (`:903`, `:910`, `:915`, `:920`, `:1313`, `:1365`,
`:1445-1446`, `:1502`) → collapses to the Shift edge → `set_hold_run`.
- `input.Jump` (`:1000`) → STAYS as the charge/fire trigger (unchanged
shape, gains the `ChargeJump()` call per 1c).
- `input.MouseDeltaX` (`:952`) → STAYS (camera/Yaw only, never a motion
command — deliberately, per the existing comment on why mouse-turn
generates no wire command).
---
## §2 The edge-detector design
### 2a. Minimal prev-state
The controller needs exactly 6 booleans (one per discrete axis-direction)
plus the existing Shift/Run bool, replacing the current
`_prevForwardCmd`/`_prevSidestepCmd`/`_prevTurnCmd`/`_prevForwardSpeed`/
`_prevRunHold`/`_prevLocalAnimCmd` fields (`:186-191`) which tracked
OUTPUT state for change-detection. The new fields track INPUT state for
edge-detection:
```csharp
private bool _prevForwardHeld; // input.Forward
private bool _prevBackwardHeld; // input.Backward
private bool _prevStrafeLeftHeld;
private bool _prevStrafeRightHeld;
private bool _prevTurnLeftHeld;
private bool _prevTurnRightHeld;
private bool _prevRunHeld; // input.Run (Shift)
```
Six of the seven map 1:1 onto retail's per-axis `RawMotionState` channels
(forward, sidestep, turn — each a single "current command" slot, so
Forward-press-while-Backward-held needs the SAME "switch channel" handling
`ApplyMotion` already does verbatim — no new logic needed, just route
both to the forward channel).
### 2b. Edge → call table
| Edge | Call | MovementParameters |
|---|---|---|
| Forward pressed (was false, now true) | `DoMotion(MotionCommand.WalkForward, p)` | ctor defaults (`new MovementParameters()`) — `Speed=1f` is the retail raw speed (ACE recomputes broadcast speed; D6.2b finding already established this). |
| Forward released (was true, now false), AND Backward not held | `StopMotion(MotionCommand.WalkForward, p)` | ctor defaults. |
| Backward pressed | `DoMotion(MotionCommand.WalkBackward, p)` | ctor defaults (`adjust_motion` applies `BackwardsFactor` — unchanged, already verbatim). |
| Backward released, Forward not held | `StopMotion(MotionCommand.WalkBackward, p)` | ctor defaults. |
| StrafeRight pressed | `DoMotion(MotionCommand.SideStepRight, p)` | ctor defaults. |
| StrafeLeft pressed | `DoMotion(MotionCommand.SideStepLeft, p)` | ctor defaults. |
| Strafe released (both false) | `StopMotion(MotionCommand.SideStepRight, p)` (retail always stops via the RIGHT id per `adjust_motion`'s SideStepLeft→SideStepRight canonicalization — confirmed in `RawMotionState.ApplyMotion`'s switch, which treats `0x6500000f`/`0x65000010` as the same channel) | ctor defaults. |
| TurnRight pressed | `DoMotion(MotionCommand.TurnRight, p)` | ctor defaults. |
| TurnLeft pressed | `DoMotion(MotionCommand.TurnLeft, p)` | ctor defaults. |
| Turn released (both false) | `StopMotion(MotionCommand.TurnRight, p)` | ctor defaults. |
| Shift edge (any transition) | `set_hold_run(input.Run, interrupt: true)` | N/A — direct call, no `MovementParameters`. `interrupt:true` matches the decomp's SECOND caller (`006b33ca`, `arg3=1`, explicitly the "player input/command layer" call site per `r3-motioninterp-decomp.md:1001`) — the FIRST caller (`0058b303`) omits arg3 in the decomp text (ambiguous, likely also non-zero given calling convention defaults per the decomp's own hedge) and is not the player-input site anyway. |
| Jump release (existing charge-fire block) | `_motion.jump(_jumpExtent)` — UNCHANGED call, only gains the `ChargeJump()` call at charge-START (see §1c/§6 R1) | N/A — `jump(float, int)` takes no `MovementParameters`. |
| Auto-walk `DoMotion(Ready)`/`DoMotion(WalkForward)` (`:732`, `:767`) | UNCHANGED — plan explicitly keeps this. | 2-arg compat overload, unchanged. |
**TODO flag (per the task's own instruction)**: the decomp does not show
a `DoMotion`/`StopMotion` call site at the CommandInterpreter boundary —
only `set_hold_run`'s two callers are visible in
`r3-motioninterp-decomp.md` §5c/§8 (`0058b303`/`006b33ca`). The
DoMotion/StopMotion edge calls above are this doc's inference from the
verbatim `DoMotion`/`StopMotion` bodies' own defaulting behavior (both
build a **fresh** `MovementParameters` internally for the
`DoInterpretedMotion`/`StopInterpretedMotion` tail-call regardless of what
the caller passes — see `MotionInterpreter.cs:915`, `:992` — so the
caller-supplied `p`'s only LIVE effects are `CancelMoveTo` (interrupt),
`SetHoldKey`+`HoldKeyToApply` (hold-key push), `Speed`, and
`ModifyRawState`/`ModifyInterpretedState` gates). Ctor defaults
(`CancelMoveTo=true, SetHoldKey=true, ModifyRawState=true,
ModifyInterpretedState=true, Speed=1f, HoldKeyToApply=Invalid`) are the
right choice UNLESS retail's real CommandInterpreter caller passes
`HoldKeyToApply` per-axis (plausible for Run) — **TODO(W6 impl): verify
against a fresh cdb capture of the CommandInterpreter boundary before
implementing**, per the task's own note. This doc marks it a TODO rather
than guessing further, consistent with CLAUDE.md's "do not guess" rule.
### 2c. Why `DoMotion`'s own re-defaulting makes this safe
`DoMotion(motion, p)` (`MotionInterpreter.cs:908-949`) builds `var local =
new MovementParameters()` (fresh re-default) and only carries
`local.Speed = speed` and `local.HoldKeyToApply = p.HoldKeyToApply`
through to `DoInterpretedMotion`. The CALLER's `p` only matters for:
`p.CancelMoveTo` (interrupt-before-dispatch), `p.SetHoldKey` (whether to
call `SetHoldKey` first), `p.ModifyRawState` (whether the raw-state mirror
runs at the end). This means the controller passing ctor-default
`MovementParameters` per edge is not a simplification that loses
information — it is EXACTLY what retail's own `DoMotion` does internally
regardless of caller intent, for every field except the four above. Low
risk.
---
## §3 The player sequencer drive — tracing DefaultSink to the sequencer
### 3a. Call path confirmed present, ONE binding missing
Full trace, verified directly in `MotionInterpreter.cs`:
1. Controller calls `DoMotion(motion, p)` (edge) →
2. `DoMotion` → `DoInterpretedMotion(motion, local)` (`:943`) →
3. `DoInterpretedMotion(uint, MovementParameters)` (`:2837-2838`) →
`DoInterpretedMotion(motion, p, DefaultSink)` (the sink-parameterized
core) →
4. Inside (`:2850-2947`): `sink?.ApplyMotion(motion, p.Speed)` (`:2884`)
— **this is the dispatch to the animation backend**.
Separately, for the CONTINUOUS axes (forward/sidestep/turn re-applied
every physics event, e.g. on `LeaveGround`/`HitGround`/`set_hold_run`):
1. `set_hold_run`/`SetHoldKey`/`LeaveGround`/`HitGround` all call
`apply_current_movement(cancelMoveTo, allowJump)` (`:1404-1417`) →
2. Dual-dispatch gate (A3): `isThePlayer = WeenieObj is null ||
WeenieObj.IsThePlayer()`. Local player: `WeenieObj` is
`PlayerWeenie` (constructed at `PlayerMovementController.cs:354`,
presumably `IsThePlayer() == true` — **needs one-line confirmation in
IWeenieObject/PlayerWeenie, not re-derived here**) AND
`PhysicsObj.LastMoveWasAutonomous` is set `true` at construction
(`PlayerMovementController.cs:360`, with the comment "R3-W6 refines
this per-motion") → routes to `apply_raw_movement(cancelMoveTo,
allowJump)` (the bool-arg overload, `:1443-1450`) →
3. That overload calls `apply_raw_movement(RawState)` (mutates
`InterpretedState` from the interpreter's OWN `RawState` — no longer
an externally-built snapshot, this is the key W6 shift) then
`ApplyCurrentMovementInterpreted(cancelMoveTo, allowJump)` (`:1449`) →
4. `ApplyCurrentMovementInterpreted` (`:1491-1536`): `if (DefaultSink is
not null) { ApplyInterpretedMovement(InterpretedState.CurrentStyle,
DefaultSink, cancelMoveTo, allowJump); return; }` (`:1502-1506`) —
**this IS the full funnel dispatch** (style → forward-or-Falling →
sidestep(-stop) → turn(-stop)), the SAME `ApplyInterpretedMovement`
the inbound remote funnel uses and the SAME dispatch-order the S2a
183-case suite pins.
**CONFIRMED: the call path from an edge-driven `DoMotion`/`set_hold_run`
all the way to `AnimationSequencer.SetCycle`/`PerformMovement` already
exists in Core, fully wired, contingent on ONE thing: `DefaultSink` must
be non-null for the player's `MotionInterpreter`.**
### 3b. The missing binding — confirmed via direct read + subagent cross-check
`MotionInterpreter.cs:1479-1489` doc comment states explicitly: *"Null
(the local player until R3-W6, interp-less entities) → the AP-77
physics-only tail."* Confirmed live: at
`GameWindow.cs:12987-13000` (`EnterPlayerModeNow`), the player's
`_playerController.Motion` gets THREE of the four seams bound
(`RemoveLinkAnimations`, `InitializeMotionTables`,
`CheckForCompletedMotions`) but explicitly NOT `DefaultSink` — inline
comment: *"DefaultSink stays null until R3-W6 (UpdatePlayerAnimation
still drives the player's cycles)."*
Compare against the remote pattern, `EnsureRemoteMotionBindings`
(`GameWindow.cs:4195-4224`), called lazily from `OnLiveMotionUpdated`
(`:4679`) and `OnLiveVectorUpdated`(`:4859`):
```csharp
rm.Sink = new MotionTableDispatchSink(ae.Sequencer)
{
TurnApplied = (turnMotion, turnSpeed) => { /* ObservedOmega seed */ },
TurnStopped = () => rmForSink.ObservedOmega = Vector3.Zero,
};
rm.Motion.DefaultSink = rm.Sink; // <-- MISSING for player
rm.Motion.RemoveLinkAnimations = ae.Sequencer.RemoveAllLinkAnimations;
rm.Motion.InitializeMotionTables = () => ae.Sequencer.Manager.InitializeState();
rm.Motion.CheckForCompletedMotions = ae.Sequencer.Manager.CheckForCompletedMotions;
```
**W6 action**: at `GameWindow.cs:12987-13000` (`EnterPlayerModeNow`),
construct a `MotionTableDispatchSink(playerSeq)` and bind
`_playerController.Motion.DefaultSink = <that sink>` alongside the three
existing binds. The `TurnApplied`/`TurnStopped` callbacks exist for the
`ObservedOmega` remote dead-reckoning seed — the LOCAL player has no
`ObservedOmega` concept (it's a remote-only field on `RemoteMotion`), so
these two callbacks can be no-ops for the player's sink, OR omitted
entirely if `MotionTableDispatchSink`'s constructor doesn't require them
(confirm the class's actual required-vs-optional members before
implementing — not independently re-verified in this pass beyond the
subagent's citation of `MotionTableDispatchSink.cs:34-72`).
**Where to store the sink instance**: unlike `RemoteMotion` (a wrapper
struct with `.Sink`/`.Motion`/`.ObservedOmega` fields), the player has no
equivalent wrapper — `_playerController` plays that role directly. The
sink instance needs a home: either (a) a new field on
`PlayerMovementController` (parallels `Motion` being exposed there
already), or (b) a `GameWindow`-local field alongside the existing
`_playerController` field. Recommend (a) for symmetry with `Motion`'s
existing exposure pattern (`PlayerMovementController.cs:375`) — a new
`internal MotionTableDispatchSink? Sink { get; set; }` property, set from
`EnterPlayerModeNow`, mirrors `RemoteMotion.Sink` exactly.
### 3c. What this replaces
Once `DefaultSink` is bound, `UpdatePlayerAnimation`'s direct
`ae.Sequencer.SetCycle(...)` call becomes fully redundant — the funnel
dispatch (`ApplyInterpretedMovement` → `DefaultSink.ApplyMotion` →
`MotionTableDispatchSink.ApplyMotion` → `sequencer.PerformMovement`) now
fires on every edge-driven `DoMotion`/`StopMotion`/`set_hold_run`/
`LeaveGround`/`HitGround` call, exactly mirroring how remotes already
work. The airborne→Falling swap "falls out of `contact_allows_move` +
`apply_current_movement`" per the plan — confirmed: `ApplyInterpretedMovement`
(`:2731-2733`) already has `if (!contact_allows_move(...))
DispatchInterpretedMotion(MotionCommand.Falling, 1.0f, sink);` as its
SECOND dispatch (unconditional check every call), so once the player's
`apply_current_movement` reaches this funnel on the LeaveGround edge, the
Falling swap happens automatically with no App-side airborne branch
needed at all — `UpdatePlayerAnimation`'s `:10164-10165` airborne
override becomes dead code, confirming full deletion is correct, not just
plumbing-safe.
---
## §4 GameWindow edit list
| # | Current anchor | What replaces it |
|---|---|---|
| 1 | `UpdatePlayerAnimation` definition, `:10139-10307` | DELETE the method entirely. |
| 2 | `UpdatePlayerAnimation(result)` call, `:8006` | DELETE the call (the tail of the `if (_playerController is not null)` block in `OnUpdate`). |
| 3 | `EnterPlayerModeNow`, binding block `:12987-13000` | ADD the `DefaultSink` bind (§3b) as a 4th line alongside the existing 3 seam binds. |
| 4 | Outbound wire-build block, `:7893-7913` (fresh `RawMotionState` built from `MovementResult` fields) | Read `_playerController.Motion.RawState` directly instead of re-deriving. See §5 for exactly which fields still need `MovementResult` vs which now come from `RawState`. |
| 5 | `ApplyServerRunRate` call site, `OnLiveMotionUpdated` `:4373` (per subagent citation) | UNCHANGED — `ApplyServerRunRate` (`PlayerMovementController.cs:410-414`) already calls `_motion.apply_current_movement(false, false)`, which after 3b's bind now ALSO drives the sequencer via `DefaultSink` (a currently-inert call becomes live) — no GameWindow-side edit needed, but flag as a **behavior change**, not just plumbing (see §6 R2). |
| 6 | JumpAction wire-build block, `:7984-8002` | UNCHANGED — still reads `result.JumpExtent`/`result.JumpVelocity` from `MovementResult` (§5 keeps these fields; the jump path is not touched by the DoMotion/StopMotion edge redesign). |
| 7 | `OnLiveVectorUpdated`'s remote-jump `LeaveGround()`+`DefaultSink` pattern, `:4845-4864` | UNCHANGED — this is the ALREADY-MIGRATED reference pattern (W4/J19 closed) that item 3 mirrors for the player. Cited here only as the proof-of-pattern, not an edit site. |
Two GameWindow items the plan names that turned out to be **already
resolved, not pending W6 work** (found this session, not anticipated by
the plan text):
- **`skipTransitionLink`**: confirmed via repo-wide grep — zero
occurrences in any production `.cs` file. Fully deleted in W4. The
plan's W6 bullet listing this as something to delete is stale; no
action needed.
- **`ACDREAM_ANIM_SPEED_SCALE`/#45 sidestep factor**: both live entirely
inside the doomed `UpdatePlayerAnimation` (`:10258-10295`) — they die
WITH the method, they don't need a separate "move into the
CMotionTable-driven speedMod path" step, because there is no equivalent
concept upstream in `MotionTableDispatchSink`/`AnimationSequencer.
PerformMovement` today. See §6 R3 — this is a real behavior-loss risk,
not a mechanical relocation, and needs a decision before deletion.
---
## §5 Wire-parity plan
### 5a. Current outbound flow (pre-W6)
`GameWindow.cs:7893-7913` builds a FRESH `RawMotionState` from
`MovementResult.ForwardCommand`/`SidestepCommand`/`TurnCommand`/
`ForwardSpeed`/`SidestepSpeed`/`TurnSpeed`/`IsRunning`, then passes it to
`MoveToState.Build(...)` (`:7916`), which internally calls
`RawMotionStatePacker.Pack(w, rawMotionState)`
(`MoveToState.cs:80`).
### 5b. Post-W6 target flow
`_playerController.Motion.RawState` is, by construction, the SAME
`RawMotionState` instance the edge-driven `DoMotion`/`StopMotion` calls
have been mutating via `ApplyMotion`/`RemoveMotion` all frame (per J2's
W1 fold: `RawState` IS the packer-compatible type, action FIFO included).
GameWindow's outbound block should read directly from
`_playerController.Motion.RawState` instead of reconstructing one.
**Field-by-field disposition**:
| `RawMotionState` field | Source post-W6 | Notes |
|---|---|---|
| `CurrentHoldKey` | `RawState.CurrentHoldKey`, set by `set_hold_run` edge | Was `result.IsRunning ? Run : None` — now the interpreter's own tracked value. |
| `ForwardCommand`/`ForwardHoldKey`/`ForwardSpeed` | `RawState.*`, set by `ApplyMotion`/`RemoveMotion` on the Forward/Backward edges | Was `result.ForwardCommand ?? Default`. |
| `SidestepCommand`/`SidestepHoldKey`/`SidestepSpeed` | `RawState.*` | Was `result.SidestepCommand ?? Default`. |
| `TurnCommand`/`TurnHoldKey`/`TurnSpeed` | `RawState.*` | Was `result.TurnCommand ?? Default`. |
| `Actions` | `RawState.Actions` | Was ALWAYS EMPTY pre-W1 (TS-24); post-W1 populated by `ApplyMotion`'s action-class branch — this is a genuine NEW behavior surfacing on the wire once W6 reads live `RawState` instead of a `MovementResult`-rebuilt empty-actions snapshot. Flag in §6 R5. |
**`MovementResult` fields that SURVIVE** (still needed by GameWindow for
non-wire purposes): `Position`, `RenderPosition`, `CellId`, `IsOnGround`
(camera/HUD), `JustLanded` (landing SFX/anim triggers if any exist
outside `UpdatePlayerAnimation` — verify no other consumer before
deleting used-only-by-UpdatePlayerAnimation fields), `JumpExtent`/
`JumpVelocity` (JumpAction wire build, item 6 in §4, untouched by this
redesign since jump keeps its existing dedicated path).
**`MovementResult` fields that DIE**: `LocalAnimationCommand`,
`LocalAnimationSpeed` (§1d). **Candidates for deletion pending §5c
decision**: `ForwardCommand`/`SidestepCommand`/`TurnCommand`/
`ForwardSpeed`/`SidestepSpeed`/`TurnSpeed`/`IsRunning`/
`MotionStateChanged` — these become redundant with `RawState` reads, but
deleting them is a larger surface change to `MovementResult`'s consumers
(the golden-byte tests test `RawMotionStatePacker`/`MoveToState`
directly, NOT `MovementResult`, so they're unaffected either way — see
5d). Recommend KEEPING these fields in `MovementResult` for this W6 slice
(mark `[Obsolete]` or comment "now diagnostic-only, GameWindow reads
RawState") rather than deleting, to keep the W6 diff scoped to the
motion-drive mechanism, not a `MovementResult` API redesign — a follow-up
cleanup commit can trim the struct once nothing reads it.
### 5c. `MotionStateChanged` gate — needs a replacement signal
`GameWindow.cs:7874` gates the whole outbound MoveToState build on
`result.MotionStateChanged && !_playerController.IsServerAutoWalking`.
Post-W6, `MovementResult.MotionStateChanged` (computed from the DYING
`_prevForwardCmd`/etc. output-comparison block, §1d) needs a replacement.
**Recommend**: derive change-detection from the edge table itself — ANY
edge fired this frame (forward/backward/strafe/turn/run edges, §2a) is
definitionally a state change; expose a `bool MotionChangedThisFrame`
computed from `_prevXHeld != inputXHeld` ORed across all axes, computed
in the SAME `Update` call, replacing the output-comparison approach with
an input-edge approach. This is a strictly simpler equivalent (retail's
CommandInterpreter only calls `DoMotion`/`StopMotion` ON edges in the
first place, so "an edge fired" and "state changed" are the same
predicate by construction — no separate comparison needed). Confirm this
doesn't regress the `runHold`-without-directional-edge case (Shift toggle
while already moving) — the edge table's Shift-edge row already covers
this (`set_hold_run` fires independently of the directional edges), so
OR-ing in the Shift edge too closes that case.
### 5d. Golden-byte tests — confirmed unaffected
`RawMotionStatePackTests.cs`, `MoveToStateGoldenTests.cs`,
`MoveToStateTests.cs`, `AutonomousPositionTests.cs`, `JumpActionTests.cs`,
`PositionPackTests.cs` (all under
`tests/AcDream.Core.Net.Tests/Messages/`) construct `RawMotionState`/call
`MoveToState.Build` with EXPLICIT field values directly — none reference
`PlayerMovementController` or `MovementResult`. They pin
`RawMotionStatePacker`'s bit-packing contract, which W6 does not touch
(only WHERE the packed values are sourced from changes). These stay green
as a structural guarantee, not something W6 needs to specifically verify
beyond "still compiles, still passes" — but a NEW test is needed for the
end-to-end path: build a `RawMotionState` via live `DoMotion`/`StopMotion`
edges, feed it to `RawMotionStatePacker.Pack`, and diff against the SAME
byte sequence the pre-cutover `MovementResult`-rebuilt path produced for
an identical input script (see §7).
---
## §6 Risk list
**R1 — `ChargeJump()` never called (BLOCKER-adjacent discovery, not in
the plan text).** Confirmed via grep: `MotionInterpreter.ChargeJump()`
has zero production call sites — only `MotionInterpreterJumpFamilyTests.cs`
calls it. `StandingLongJump` therefore never arms for ANY entity today
(local or remote), despite W3 having fully ported the mechanism. W6's
jump-charge block (§1c) MUST add a `ChargeJump()` call when charge begins
(mirroring `input.Jump && !_jumpCharging` → also call `_motion.ChargeJump()`,
checking its `WeenieError` return the same way the fire-path already
checks `jump()`'s). Without this, the "standing long-jump" feature stays
silently dead post-cutover — same bug, just no longer excusable as "W3
hasn't wired the call site yet." **Recommendation: treat as in-scope for
W6, not a separate follow-up** — it's the SAME jump-charge block W6 is
already editing to add `ChargeJump`'s sibling gate calls.
**R2 — `ApplyServerRunRate`'s `apply_current_movement` call goes from
inert to live.** Pre-W6, `ApplyServerRunRate` (`PlayerMovementController.cs:
410-414`) calls `_motion.apply_current_movement(false, false)`, but
`DefaultSink` is null so `ApplyCurrentMovementInterpreted` falls through
to the AP-77 physics-only tail (a `get_state_velocity` → `set_local_velocity`
write, no sequencer touch). Post-W6 (once `DefaultSink` is bound, §3b),
the SAME call now ALSO dispatches through the funnel
(`ApplyInterpretedMovement` → `SetCycle`) on every server RunRate echo —
this could fire a redundant/premature `SetCycle` call on every
`UpdateMotion (0xF61C)` echo for the player's own guid, independent of
any local edge. **Recommendation**: verify this doesn't cause visible
cycle-restart flicker (retail's real `apply_current_movement` call here IS
supposed to re-dispatch — this may be CORRECT retail behavior surfacing
for the first time — but it's a genuine visual-risk item for the "ONE
user visual pass" acceptance test, not a mechanical no-op).
**R3 — `#45` sidestep factor + `ACDREAM_ANIM_SPEED_SCALE` have no
upstream equivalent.** Per §4, both die with `UpdatePlayerAnimation`
because `MotionTableDispatchSink.ApplyMotion`/`AnimationSequencer.
PerformMovement` (the remote path, now shared) has no known
sidestep-specific speed-scale multiplier or global visual-pacing knob.
Two sub-risks: (a) sidestep animation pacing may visibly change (the
1.248× factor matched ACE's `BroadcastMovement` wire formula — losing it
means the sidestep CYCLE plays at `SidestepAnimSpeed`-derived pace instead
of the wire-matched pace); (b) `ACDREAM_ANIM_SPEED_SCALE` (a debug/tuning
knob) simply stops having any effect for the local player post-cutover
(remotes never had it either, so this makes local and remote consistent —
arguably correct, but a functionality LOSS for whoever uses that env var).
**Recommendation**: confirm with the user whether (a) is an
expected-diff (remotes today do NOT get the 1.248× factor either — so
post-cutover the LOCAL player's sidestep pacing becomes consistent with
how remotes have always looked, which may in fact be the retail-faithful
answer the #45 comment's "matching ACE's wire formula" was compensating
for a DIFFERENT bug in) before deleting; document as an expected-diff in
the W6 commit either way, per the plan's own instruction ("expected-diff
annotations required").
**R4 — `DriveServerAutoWalk`'s `DoMotion(Ready)` at `:732` is not named
in the plan's W6 SetPosition-only StopCompletely note.** §1e flags this
textual sibling. Auto-walk survives R3 verbatim per the KEEP LIST, so
changing `:732` is OUT of scope — but a future reader diffing "every
`DoMotion(Ready)` call site" against the plan's single cited line (`:825`
old numbering) could reasonably assume both change. **Recommendation**:
leave `:732` untouched, but add a one-line comment there cross-referencing
this doc so the distinction is explicit at the code site, not just here.
**R5 — `RawState.Actions` starts reaching the wire for the first time.**
Per §5b, reading live `RawState` instead of a `MovementResult`-rebuilt
snapshot means the `Actions` FIFO (populated since W1/W2 by
`ApplyMotion`'s action-class branch, e.g. `motion & 0x10000000` bit —
action-class motions like emotes/attacks IF the local player ever calls
`DoMotion` with an action-class id) starts appearing in the packed
`MoveToState` bytes where it was previously always empty (TS-24). If
nothing in the current player-input path ever issues an action-class
`DoMotion` call, this is a no-op in practice for W6's scope (movement axes
only) — but flag it so a future action/emote wiring phase doesn't get
blindsided by "wait, did this already work?"
**R6 — `IWeenieObject.IsThePlayer()` for `PlayerWeenie` not
independently re-verified this session.** §3a's dispatch-path trace
ASSUMES `PlayerWeenie.IsThePlayer() == true` (required for
`apply_current_movement`'s A3 gate to route to `apply_raw_movement`,
which is the path that reaches `DefaultSink`). This is almost certainly
already correct (W3 added `IsThePlayer()` to `IWeenieObject` specifically
for this dispatch, per J5's gap list), but a W6 implementer should grep
`PlayerWeenie.cs` for the override before writing code, not assume from
this doc alone.
---
## §7 Test plan
### 7a. New unit tests — edge table
Add to `tests/AcDream.Core.Tests/Input/PlayerMovementControllerTests.cs`
(or a new `PlayerMovementControllerEdgeTests.cs` alongside it, matching
the existing per-concern file split seen in
`DispatcherToMovementIntegrationTests.cs`):
- Press-W → `_motion.RawState.ForwardCommand == WalkForward` after one
`Update` call (was: never directly observable pre-W6 since the raw
state was an ephemeral per-frame local; post-W6 it's the persistent
field — this test is only possible/meaningful after the cutover).
- Press-W, hold 3 frames, release → `RawState.ForwardCommand ==
MotionCommand.Ready` after release frame (StopMotion fired).
- Press-W then press-S same frame → forward channel ends on S (or
whichever wins per `ApplyMotion`'s single-channel-per-axis semantics —
pin the actual retail-faithful tie-break, likely "last processed wins"
given the controller processes Forward before Backward in its own
`MovementInput` field order — assert whichever the implementation
produces, don't guess a preference here).
- Shift press while W held → `RawState.CurrentHoldKey == Run` +
`set_hold_run`'s internal `apply_current_movement` fired (observable
via a velocity change or a `DefaultSink` mock's call count once 3b
lands).
Shift press while NOT moving → `CurrentHoldKey == Run` still updates
(retail's `set_hold_run` doesn't gate on movement) but no velocity
visible change.
- Redundant Shift-held-while-already-Run (two consecutive `Update` calls
with `input.Run=true`) → `set_hold_run`'s XOR guard no-ops on the
second call (assert via a call-counting mock on
`apply_current_movement`'s side effect, e.g. `DefaultSink.ApplyMotion`
invocation count stays flat).
- Jump charge-then-release still produces `WeenieError.None` from
`jump()` AND (new) `ChargeJump()` was called at charge-start with a
`StandingLongJump` arm when idle+grounded (R1's fix, needs its own
assertion — grounded+idle+charge+release → `StandingLongJump == true`
observable via `_motion.StandingLongJump` before the release clears it,
or via the funnel's state-only dispatch branch firing).
- SetPosition (teleport) → `RawState.ForwardCommand == Ready` AND
`PendingMotions` gained the A9 snapshot node (StopCompletely's
`AddToQueue` call) — this is a NEW observable side effect vs the old
`DoMotion(Ready)` call, worth a dedicated assertion.
### 7b. Existing tests needing re-pin
Per the tests-agent's findings:
- **`PlayerMovementControllerTests.cs`** (15 facts, 29 `Update` calls) —
the two jump tests (`Update_JumpOnFlatTerrain_BecomesAirborne`,
`Update_AirborneFrames_ZRiseThenFalls`) are HIGH risk for needing
re-pin: they exercise the exact charge→release block §1c/R1 touches.
Re-run after the `ChargeJump()` addition — if `StandingLongJump` arms
during a stationary charge-then-release-while-still-idle scenario these
tests use, a NEW dispatch path (`DispatchInterpretedMotion(Ready)` +
`DispatchStopInterpretedMotion(SideStepRight)` per
`ApplyInterpretedMovement`'s StandingLongJump branch) fires that didn't
before — verify it doesn't change `IsAirborne`/`VerticalVelocity`/
`Position.Z` assertions (it shouldn't, since that branch is state-only
no-dispatch-to-velocity per J6, but MUST be checked, not assumed).
- **`DispatcherToMovementIntegrationTests.cs`** (6 facts) — asserts
dispatcher-built `MovementInput` produces IDENTICAL results to
hand-built input. Should stay green unchanged (it exercises the
`MovementInput`→`Update` boundary, not the internals W6 rewrites) —
but re-run explicitly since it's the integration seam most likely to
surface an edge-detection regression (e.g. if a test builds two
DIFFERENT `MovementInput` values across two `Update` calls without the
edge-detector's `_prevXHeld` fields being exercised the same way a
real per-frame loop would).
- **`CellarUpTrajectoryReplayTests.cs`**'s
`IndoorCell_FullController_AtRestNoInput_RenderPositionBitStable` — a
600-frame bit-stability regression guard with NO input held. Should be
UNAFFECTED (no edges ever fire), but it's cited in memory
(`feedback_render_perf_measurement.md`-adjacent class of test) as a
historically fragile guard — run it explicitly, don't assume "no input
means no risk" given this test exists specifically because a prior
change broke exactly this "nothing should happen when nothing is
pressed" invariant.
### 7c. Pre-cutover trace capture (plan-mandated, not yet done)
Per the plan's Fixture source line: "pre-cutover recorded traces +
golden-byte + suite." This doc does not capture the trace (out of
research-only scope) but flags that the W6 IMPLEMENTATION session must
run `ACDREAM_DUMP_MOTION=1` (or the equivalent `SetCycle`/dispatch trace
capture) against the CURRENT (pre-W6) code first, save the log, THEN
implement the cutover, THEN replay the same input script and diff — this
is a sequencing dependency on the implementation session, not something
this research pass can pre-stage without running the live client.
### 7d. Suites that must stay green throughout, unmodified
`RetailObserverTraceConformanceTests.cs` (184-case funnel dispatch-order
suite) + `MotionInterpreterFunnelTests.cs` (13 facts, synthetic
action-class supplement) — both exercise the INBOUND remote funnel path
(`DispatchInterpretedMotion`/`MoveToInterpretedState`), a sibling path to
what W6 touches. Should require zero changes; green throughout is the
signal that W6's edits to the SHARED `MotionInterpreter` machinery
(`DoMotion`/`PerformMovement`/`apply_current_movement`) didn't perturb
the remote dispatch order. All six golden-byte packer test files (§5d)
likewise stay green unmodified.
---
## Anchor summary (file:line, this session, HEAD `e214acdf`)
- `src/AcDream.App/Input/PlayerMovementController.cs` — full file read,
1547 lines.
- `src/AcDream.App/Rendering/GameWindow.cs` — `UpdatePlayerAnimation`
`:10139-10307`; caller `:8006`; `ApplyServerRunRate` call `:4373`;
JumpAction block `:7984-8002`; outbound RawMotionState build
`:7893-7913`; `EnterPlayerModeNow` binding block `:12987-13000`;
`EnsureRemoteMotionBindings` `:4195-4224`; `OnLiveVectorUpdated`
LeaveGround pattern `:4845-4864`.
- `src/AcDream.Core/Physics/MotionInterpreter.cs` — full file read, 3083
lines; key anchors: `DoMotion` `:856-949`; `StopMotion` `:958-1008`;
`StopCompletely` `:1054-1084`; `apply_raw_movement` (2 overloads)
`:1346-1371`, `:1443-1450`; `apply_current_movement` `:1404-1417`;
`DefaultSink` property `:1489`; `ApplyCurrentMovementInterpreted`
`:1491-1536`; `ChargeJump` `:1765-1789`; `jump` `:1822-1839`;
`LeaveGround` `:2310-2330`; `HitGround` `:2361-2375`;
`EnterDefaultState` `:2415-2427`; `set_hold_run` `:2453-2463`;
`SetHoldKey` `:2506-2525`; `MotionDone` `:2132-2154`;
`ApplyInterpretedMovement` `:2715-2762`; `DoInterpretedMotion`
`:2837-2948`.
- `src/AcDream.Core/Physics/RawMotionState.cs` — full file, `ApplyMotion`
`:208-273`, `RemoveMotion` `:293-321`.
- `src/AcDream.Core/Physics/Motion/MovementParameters.cs` — full file,
fields `:84-186`.