acdream/docs/superpowers/specs/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk-design.md
Erik d640ed74e1 feat(retail): Phase B.6 — server-driven auto-walk done right
Closes #63, #69, #74, #75. Replaces the chain of Commit-B workarounds
that compensated for ACE's MoveToChain getting cancelled by a leaked
user-MoveToState packet during inbound auto-walk. The fix is
architectural — auto-walk drives the body directly from the
server-supplied path data, no player-input synthesis, no spurious
wire-packet transitions, no grace-period band-aid.

Architectural change (closes #75):
  PlayerMovementController.ApplyAutoWalkOverlay → DriveServerAutoWalk.
  - Steps Yaw toward target at retail-faithful turn rates.
  - Computes desired forward velocity from path runRate.
  - Calls _motion.DoMotion(WalkForward, speed) directly for the
    motion-interpreter state (drives animation cycle).
  - Sets _body.set_local_velocity directly when grounded.
  - Returns true to gate the user-input motion + velocity section
    in Update so user-input flow doesn't overwrite auto-walk
    velocity or motion state.
  Mirrors retail's MovementManager::PerformMovement case 6 (decomp
  0x00524440) which never touches the user-input pipeline during
  server-controlled auto-walk.

Wire-layer guard at GameWindow.cs:6419 retained as a SEMANTIC
statement (`if (result.MotionStateChanged && !IsServerAutoWalking)`):
user-MoveToState packets are for user-driven motion intent. During
server-controlled auto-walk, the motion-state transitions caused by
the animation override (RunForward / WalkForward / TurnLeft /
TurnRight cycles) must not leak as user-cancellation packets. This
is NOT the deleted 500ms grace-period band-aid; it's the wire-layer
expressing the user-vs-server motion split.

Animation plumbed for auto-walk phases (closes #69):
  - Moving forward → WalkForward (speed=1.0) / RunForward (speed=runRate)
  - Turn-first phase → TurnLeft / TurnRight (sign of yawStep)
  - Aligned-but-pre-step / arrival → no override (idle)
Driven via _autoWalkMovingForwardThisFrame + _autoWalkTurnDirectionThisFrame
fields set in DriveServerAutoWalk and read in the MovementResult
construction at the bottom of Update. UpdatePlayerAnimation picks up
the localAnimCmd as the highest-priority animation source.

Walk/run threshold = 1.0m, retail-observed. ACE's wire-default of
15.0f is too generous; ACE's own physics layer uses 1.0f at
MovementParameters.cs:50 (with the 15.0f line commented out) and
Creature.cs:312 notes "default 15 distance seems too far". The
formula matches retail's MovementParameters::get_command at decomp
0x0052aa00: running = (initialDist - distance_to_object) >=
threshold, evaluated ONCE at chain start and held for the rest of
the auto-walk (matches retail "runs all the way / walks all the way"
behaviour). Wire-supplied threshold is ignored.

Pickup gate (IsPickupableTarget) now uses BF_STUCK
(acclient.h:6435, bit 0x4) to discriminate immovable scenery from
real pickup items that share a Misc ItemType. Sign (pwd=0x14 with
BF_STUCK) → blocked; spell component (pwd=0x10, no BF_STUCK) →
allowed. ACE's PutItemInContainer (Player_Inventory.cs:831-836)
responds with WeenieError.Stuck (0x29) on stuck items so the gate
prevents wasted wire packets + a UX dead-end.

R-key dispatch by target type. UseCurrentSelection's top-level
IsUseableTarget gate was wrong (blocked USEABLE_NO=1 items that
ARE pickupable). Reordered:
  1. Creature → SendUse
  2. Pickupable → SendPickUp
  3. Useable → SendUse
  4. Otherwise → "cannot be used" toast
Each handler keeps its own gate. Matches retail's per-action
server-side validation.

AP cadence revert (closes #74). With the MoveToChain race fixed,
the per-frame "send while moving" cadence is no longer load-bearing.
Reverted to retail's two-branch ShouldSendPositionEvent gate
(acclient_2013_pseudo_c.txt:700233-700285):
  Interval NOT elapsed (< 1 sec): send if cell or contact-plane changed.
  Interval elapsed (>= 1 sec):    send if cell or position frame changed.
Adds _lastSentContactPlane field + ApproxPlaneEqual helper +
PlayerMovementController.ContactPlane public accessor. Extended
NotePositionSent(Vector3, uint, Plane, float) — both outbound sites
(MoveToState + AP) pass _playerController.ContactPlane.
Effective rates: 0 Hz idle, ~1 Hz smooth motion, per-event on
cell/plane changes, 0 Hz airborne.

CLAUDE.md updated with no-workarounds rule (commit `da126f9` on
the worktree branch). Saved as feedback memory at
memory/feedback_no_workarounds.md.

Tests: build green; Core.Net 294/294; Core 1073/1081 (baseline,
8 pre-existing Physics failures unchanged). Visual-verified
end-to-end on 2026-05-16 for far/near Use + PickUp on NPCs,
doors, items, spell components, signs (correctly blocked), corpses,
turn-first animation, run/walk thresholds, idle quiet, smooth-
motion 1Hz.

Spec: docs/superpowers/specs/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk-design.md
Plan: docs/superpowers/plans/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 16:14:44 +02:00

10 KiB

Phase B.6 — Suppress outbound MoveToState during inbound MoveToObject auto-walk

Date: 2026-05-16 Closes: #63 (server-initiated auto-walk not honored — the half about ACE's MoveToChain callback never firing), #74 (AP cadence chattier than retail) Milestone: M1 — Walkable + clickable world

Goal

Make ACE's server-side MoveToChain callback fire the Use action on arrival, eliminating the client-side retry workaround that today's feat(retail): Commit B (b5da17d) restored as an issue-#63 mitigation.

Background

When the player double-clicks an NPC across the room in our current build:

  1. We send Use(guid) wire packet immediately at click.
  2. ACE: player not in WithinUseRadius → server-side CreateMoveToChain(item, (success) => TryUseItem(item, success)) (Player_Use.cs:205).
  3. ACE broadcasts UpdateMotion with MovementType=6 (MoveToObject) targeting our player.
  4. Our client honors it via BeginServerAutoWalk(...) at GameWindow.cs:3389-3414.
  5. The auto-walk overlay at PlayerMovementController.ApplyAutoWalkOverlay synthesizes input.Forward = true, input.Run = true (line 611-619) so the rest of Update runs as if the user pressed W.
  6. Bug: the rest of Update computes MotionStateChanged = true and the outbound wire layer at GameWindow.cs:6410-6445 builds and sends a MoveToState packet with forwardCommand=RunForward, holdKey=Run.
  7. ACE reads that as user took manual control and cancels its own MoveToChain — the TryUseItem callback never fires.
  8. We've worked around this by sending Use a second time on local-arrival (_pendingPostArrivalAction in GameWindow.SendUse far-range path). ACE accepts the second Use because we're now in range. This is what we're retiring.

The retail client and holtburger's Rust client don't have this bug: neither synthesizes player-input MoveToState during inbound MoveToObject. Their auto-walk is body-level only; ACE's chain runs uninterrupted; the callback fires on arrival.

Retail anchor

  • ACE Player_Use.cs:205CreateMoveToChain(item, (success) => TryUseItem(item, success)).
  • ACE Player_Move.cs:150 — chain polls and fires callback(true) when within use radius.
  • Holtburger simulation.rs:178-206MoveToObject handler returns Ok(Vec::new()), no outbound packets in response. No retry mechanism anywhere in its codebase.

Design

Single-flag suppression of the outbound MoveToState send while a server-initiated auto-walk is active. The body keeps walking locally (synth still drives the animation cycle + per-frame position update), AutonomousPosition keeps flowing (so ACE's WithinUseRadius poll sees us approach), but the misleading "user is RunForward" packet is silenced.

Wire-level changes

File: src/AcDream.App/Input/PlayerMovementController.cs

No new field needed — IsServerAutoWalking already exists, added by B.6 slice 2 (2026-05-14) for this purpose. Backs onto private _autoWalkActive (line 254). Set true by BeginServerAutoWalk, false by EndServerAutoWalk. User-input cancellation at line 478-482 already clears the flag, so manual takeover transitions correctly to "user is in control, send MoveToState normally."

File: src/AcDream.App/Rendering/GameWindow.cs

Guard the MoveToState.Build block at lines 6410-6445 with a second condition:

if (result.MotionStateChanged && !_playerController.IsServerAutoWalking)
{
    // ... existing MoveToState build + SendGameAction ...
}

No other path needs adjustment; the AutonomousPosition path remains unchanged (heartbeat-driven, gated on Contact && OnWalkable).

Single-clause add; IsServerAutoWalking already documented at line 267-273 as the predicate for "the next Update synthesizes Forward+Run."

Workaround retirement

After the wire fix is verified, the following Commit B safeguards are deleted:

  1. Far-range Use retry. In GameWindow.SendUse (~line 9201-9204), remove the _pendingPostArrivalAction = (guid, false); assignment from the far-range path and rewrite the comment block to say "ACE auto-fires Use via MoveToChain callback; client sends Use exactly once."

  2. Far-range PickUp retry. Same shape in GameWindow.SendPickUp (~line 9265).

  3. Log strings. Drop (queued for arrival re-send pending #63) from the same paths.

  4. AP cadence revert. Replace the per-frame positionChanged gate in PlayerMovementController.cs:1261-1275 with retail's narrow gate per acclient_2013_pseudo_c.txt:700233-700285:

// During sub-interval: send only on cell-or-contact-plane change.
// After interval elapses: send only if position/frame changed.
// Retail-faithful per acclient_2013_pseudo_c.txt:700233 ShouldSendPositionEvent.
bool intervalElapsed = !_lastSentInitialized
    || (_simTimeSeconds - _lastSentTime) >= HeartbeatInterval;

bool cellChanged  = _lastSentCellId != CellId;
bool planeChanged = !_lastSentContactPlane.Equals(_body.ContactPlane);
bool frameChanged = !ApproxPositionEqual(_lastSentPos, _body.Position);

bool sendThisFrame = intervalElapsed
    ? (cellChanged || frameChanged)
    : (cellChanged || planeChanged);

HeartbeatDue = _body.InContact && _body.OnWalkable && sendThisFrame;

_lastSentContactPlane requires adding a System.Numerics.Plane _lastSentContactPlane; field next to the other _lastSent* fields. System.Numerics.Plane is a struct with IEquatable<Plane> so direct !_lastSentContactPlane.Equals(_body.ContactPlane) works. Extend NotePositionSent to accept a Plane and stamp it alongside position/cell/time. The outbound layer at GameWindow.cs:6410-6445 (MoveToState) and ~line 6310 (AutonomousPosition) both gain a contactPlane: _body.ContactPlane argument.

Issues closed

  • #63 — server-initiated auto-walk not honored. (Both halves: B.6 slice 2 shipped the inbound handling on 2026-05-14; this commit fixes the MoveToChain callback-cancellation gap.)
  • #74 — AP cadence chattier than retail. (Direct revert to retail's narrow gate enabled by the workaround retirement.)

Out of scope (file as new issues)

  • Retail status messages ("Approaching {target}" / "Using the {target}" on screen). Client-side UX polish; not wire-critical. Mentioned by user during brainstorm 2026-05-16 as nice-to-have to match retail feel. File as #75 — Retail-faithful pending-action status messages on screen.

Testing plan

Unit tests

No new unit tests required. The behavioral change is wire-level integration; existing Core.Net 294 baseline must hold. Existing 1073 Core tests baseline must hold (8 pre-existing physics failures unchanged).

Visual verification (user-driven)

  1. Far-range Use NPC. Double-click a Pathwarden / Royal Guard ~8-15 m away. Expected log shape:

    [B.4b] use guid=0x... seq=X
    [autowalk-mt] mt=0x06 isMoveTo=True ...
    [autowalk-begin] dest=...
    [autowalk-end] reason=arrived
    

    Expected: NO [B.4b] use-deferred line. Dialogue fires from ACE's callback. One Use packet on the wire, not two.

  2. Far-range PickUp item. F-key a ground item ~5-10 m away. Same shape — one PickUp, no retry.

  3. Close-range Use NPC behind player. Within 3 m of an NPC facing away, press R. Body turns 180°, then dialogue fires. The close-range deferred path is unchanged by this work; should still produce:

    [B.4b] use deferred (close-range, turn-first)
    [autowalk-end] reason=arrived
    [B.4b] use-deferred
    

    (use-deferred is correct here — close-range deferral is retail-faithful turn-first, not a workaround.)

  4. Open inn door from across the room. Walks, opens. ONE [B.4b] use line, no retry, no double-open.

  5. User takes manual control mid-auto-walk. Click far NPC → during the walk, press W. Auto-walk cancels via user-input, normal MoveToState resumes firing. The action does NOT fire on arrival (user cancelled).

  6. Rapid click between two targets. Click NPC, before arrival click a second NPC. ACE re-broadcasts MoveToObject with the new target. Our overlay re-targets (existing BeginServerAutoWalk overwrite). Action fires on arrival at the SECOND target. No stale _pendingPostArrivalAction from the first click (close-range path keeps its handling; far-range no longer queues anything).

Pre-conditions

  • ACE running on 127.0.0.1:9000.
  • +Acdream character at Holtburg (test character).
  • ACDREAM_PROBE_AUTOWALK=1 for the trace lines.
  • Build green: dotnet build src/AcDream.App/AcDream.App.csproj -c Debug.

Acceptance criteria

  • All six visual-verify scenarios pass.
  • No use-deferred log line for FAR-range Use (only for close-range turn-first defer, which is correct).
  • Core.Net tests pass (294/294).
  • Core tests baseline holds (1073/1081).
  • Issues #63 and #74 close with this commit's SHA.

Risk + rollback

Risk: If ACE's MoveToChain somehow STILL doesn't fire the callback (e.g., a separate ACE bug we haven't identified), Use action breaks for far-range targets. The user would observe "walked to NPC, no dialogue."

Rollback: trivial git revert. The fix is one guard clause + workaround deletions. Reverting restores the working-but-chatty Commit B state at b5da17d.

Detection during visual verify: if scenario 1 fails (no dialogue after arrival), revert before merging.

Plan handoff

This design is the input for superpowers:writing-plans, which produces the task-by-task plan with code blocks ready to paste.