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>
This commit is contained in:
Erik
parent
df7b096d6f
commit
fc5a2cda28
1 changed files with 726 additions and 0 deletions
726
docs/research/2026-07-02-r3-motioninterp/w6-cutover-map.md
Normal file
726
docs/research/2026-07-02-r3-motioninterp/w6-cutover-map.md
Normal file
|
|
@ -0,0 +1,726 @@
|
||||||
|
# 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`.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue