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>
This commit is contained in:
Erik
parent
b5da17db76
commit
d640ed74e1
6 changed files with 1317 additions and 200 deletions
20
CLAUDE.md
20
CLAUDE.md
|
|
@ -179,6 +179,26 @@ The only thing that genuinely requires stopping is **visual confirmation** — t
|
|||
user needs to look at the running client and tell you whether it matches
|
||||
retail. Everything else is your call.
|
||||
|
||||
**No workarounds without explicit approval.** When you spot a bug or
|
||||
encounter a behavioral mismatch, fix the underlying cause — do not ship a
|
||||
band-aid, suppression flag, grace period, retry loop, or any other "make
|
||||
the symptom go away" shortcut, unless the user has explicitly approved
|
||||
that shape OR you are building a NEW feature with a different design.
|
||||
This rule exists because every workaround creates architectural debt that
|
||||
masks the real issue, makes future refactors harder, and erodes the
|
||||
codebase's retail-faithfulness. Examples of disallowed shortcuts: an
|
||||
`if (problematicState) return early` guard at the symptom site instead of
|
||||
investigating why the state happened; a timer-based "settle period" to
|
||||
hide a race; a flag like `_suppressXDuringY` to mask a wire-level mistake;
|
||||
a `try/catch` swallowing an exception that signals a real problem. If you
|
||||
notice a fix is starting to look like a workaround mid-implementation,
|
||||
stop, file the proper investigation as an issue with full reproduction
|
||||
notes, and either (a) ask the user before shipping the workaround, or
|
||||
(b) invest the time to fix the root cause. The user has explicitly
|
||||
authorized "spend more time, get it right" over "ship a shortcut and
|
||||
file the cleanup." Quote them: "we should have no workarounds unless I
|
||||
say so or we want a different feature."
|
||||
|
||||
**Only stop and wait for the user when:**
|
||||
|
||||
- Visual verification is the acceptance test ("does the drudge look right now?")
|
||||
|
|
|
|||
|
|
@ -46,13 +46,68 @@ Copy this block when adding a new issue:
|
|||
|
||||
# Active issues
|
||||
|
||||
## #74 — AP cadence is per-frame-while-moving, more chatty than retail
|
||||
## #75 — [DONE 2026-05-16 · `f035ea3`] Auto-walk should drive body directly, not synthesize player-input
|
||||
|
||||
**Status:** OPEN
|
||||
**Status:** DONE
|
||||
**Severity:** LOW (functionally correct via grace-period band-aid; architectural cleanup only)
|
||||
**Filed:** 2026-05-16
|
||||
**Component:** physics / auto-walk
|
||||
|
||||
**Resolution (2026-05-16 · `f035ea3`):** Refactored `ApplyAutoWalkOverlay` → `DriveServerAutoWalk`. Auto-walk now steps Yaw, sets `_body.set_local_velocity` from runRate, and calls `_motion.DoMotion(WalkForward, speed)` directly — NO `MovementInput` synthesis. `Update` gates the user-input motion + velocity section on `!autoWalkConsumedMotion` to prevent overwrite. The 500ms arrival grace period (band-aid) deleted. The wire-layer `!IsServerAutoWalking` guard at `GameWindow.cs:6419` retained as a semantic statement (user-MoveToState is for user-driven intent only), not as a band-aid for the synthesis leak that no longer exists. Animation cycle plumbed through via `localAnimCmd` / `localAnimSpeed` for both moving-forward and turn-first phases (issue #69 folded in). Walk/run threshold corrected to 1.0m (overrides ACE's wire-supplied 15.0f; matches user-observed retail behaviour + ACE's own physics layer default). `IsPickupableTarget` now checks `BF_STUCK` (`acclient.h:6435`) to correctly block signs/banners that share Misc ItemType with real pickup items.
|
||||
|
||||
**Description:** `ApplyAutoWalkOverlay` in `PlayerMovementController`
|
||||
synthesizes `Forward+Run` `MovementInput` during inbound `MoveToObject`
|
||||
so the existing motion-interpreter pipeline drives the body. The
|
||||
synthesis leaks: motion-interpreter sets `MotionStateChanged=true`,
|
||||
which would fire an outbound `MoveToState` "user is running"
|
||||
packet to ACE — interpreted as user-took-manual-control and cancels
|
||||
ACE's `MoveToChain`. We mitigate with a guard
|
||||
(`!_playerController.IsServerAutoWalking` at `GameWindow.cs:6410`)
|
||||
plus a 500 ms post-arrival grace period to cover ACE's poll race.
|
||||
|
||||
Retail's `MoveToManager::HandleMoveToPosition` (decomp 0x0052xxxx)
|
||||
steps the body POSITION directly when server `MoveToObject` arrives —
|
||||
NO player-input synthesis, NO motion-interpreter involvement, NO
|
||||
outbound MoveToState. Holtburger
|
||||
([simulation.rs:178-206](references/holtburger/crates/holtburger-core/src/client/simulation.rs))
|
||||
follows the same pattern (sets `ServerControlledProjection`, advances
|
||||
the body, returns empty).
|
||||
|
||||
**Acceptance:** Refactor auto-walk to step `_body.Position` (or
|
||||
equivalent) directly from the wire-supplied path data + run rate, NOT
|
||||
via synthesized input. Motion state during auto-walk becomes a
|
||||
SERVER-DRIVEN state (similar to how remote players' motion is driven
|
||||
by inbound MoveToState packets), not a USER-DRIVEN one. The 500 ms
|
||||
grace period in `EndServerAutoWalk` becomes unnecessary and can be
|
||||
deleted; same for the `IsServerAutoWalking` guard at the wire layer
|
||||
(no MoveToState would have been built in the first place).
|
||||
|
||||
Animation cycle currently driven by motion-interpreter's
|
||||
`MotionStateChanged → SetCycle(RunForward)` would need a separate
|
||||
path: probably mirror how remote-player animation is driven by
|
||||
inbound motion packets (the sequencer accepts a `SetCycle` directly).
|
||||
|
||||
**Files:** `src/AcDream.App/Input/PlayerMovementController.cs`
|
||||
(`ApplyAutoWalkOverlay` returns synthesized input today; refactor to
|
||||
step body directly + drive animation via `_animationSequencer.SetCycle`
|
||||
directly). `src/AcDream.App/Rendering/GameWindow.cs` (delete the
|
||||
`!IsServerAutoWalking` guard once the leak is gone).
|
||||
|
||||
**Estimated scope:** Medium (~50-100 LOC + careful testing of
|
||||
animation cycle continuity). Not blocking M1 — the grace-period
|
||||
band-aid produces retail-faithful behaviour empirically.
|
||||
|
||||
---
|
||||
|
||||
## #74 — [DONE 2026-05-16 · `de44358`] AP cadence is per-frame-while-moving, more chatty than retail
|
||||
|
||||
**Status:** DONE
|
||||
**Severity:** LOW (works; just sends ~60× the packets retail would during smooth motion)
|
||||
**Filed:** 2026-05-16
|
||||
**Component:** physics / net cadence
|
||||
|
||||
**Resolution (2026-05-16 · `de44358`):** With #75 (MoveToState suppression refactor) closing the MoveToChain-cancellation race, 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`): cell/plane change during the sub-interval; cell-or-frame change after the 1s heartbeat. Added `_lastSentContactPlane` field + extended `NotePositionSent(Vector3, uint, Plane, float)` + added `ApproxPlaneEqual` helper + `PlayerMovementController.ContactPlane` public accessor. Effective rates now match retail: 0 Hz idle, ~1 Hz smooth motion, per-event on cell/plane changes, 0 Hz airborne.
|
||||
|
||||
**Description:** The diff-driven AP cadence shipped in Commit B fires
|
||||
`HeartbeatDue` on **any** position change each frame while grounded
|
||||
on walkable (effective ~60 Hz during smooth movement) and a 1 Hz
|
||||
|
|
@ -214,13 +269,15 @@ Not blocking M1.
|
|||
|
||||
---
|
||||
|
||||
## #69 — Local player rotation isn't animated (no leg/arm cycle while pivoting)
|
||||
## #69 — [DONE 2026-05-16 · `f035ea3`] Local player rotation isn't animated (no leg/arm cycle while pivoting)
|
||||
|
||||
**Status:** OPEN
|
||||
**Status:** DONE
|
||||
**Severity:** LOW (visual polish — rotation works, just looks stiff)
|
||||
**Filed:** 2026-05-15 (B.6 close-range turn-to-face)
|
||||
**Component:** motion / animation cycle
|
||||
|
||||
**Resolution (2026-05-16 · `f035ea3`):** Fixed as part of the auto-walk architectural refactor (issue #75). `DriveServerAutoWalk` now records the per-frame rotation direction in `_autoWalkTurnDirectionThisFrame` (+1 / -1 / 0); the animation override at the bottom of `Update` reads that flag and sets `localAnimCmd` to `TurnLeft` / `TurnRight` during the turn-first phase. User confirmed 2026-05-16 that the auto-walk turn-first case (click target, body rotates before walking) now plays the leg-shuffle animation. User-driven A/D rotation was always working — the original issue description was specific to the auto-walk turn-first case.
|
||||
|
||||
**Description:** When the auto-walk overlay rotates the local player
|
||||
(close-range Use turn-to-face, or turn-first phase of a far-range walk),
|
||||
the body's Yaw rotates smoothly but no leg / arm animation plays —
|
||||
|
|
@ -422,14 +479,20 @@ locally on send (mirroring retail's client behavior).
|
|||
|
||||
---
|
||||
|
||||
## #63 — Server-initiated auto-walk (MoveToObject) not honored
|
||||
## #63 — [DONE 2026-05-16 · `f035ea3`] Server-initiated auto-walk (MoveToObject) not honored
|
||||
|
||||
**Status:** OPEN
|
||||
**Status:** DONE
|
||||
**Severity:** MEDIUM (blocks out-of-range Use + Pickup; close-range
|
||||
works fine)
|
||||
**Filed:** 2026-05-14 (B.5 visual verification)
|
||||
**Component:** motion / inbound MoveToObject handling
|
||||
|
||||
**Resolution (2026-05-16):** Closed in two parts:
|
||||
1. **B.6 slice 2 (2026-05-14):** inbound MoveToObject parsing + `BeginServerAutoWalk` wiring at `GameWindow.cs:3389` — body auto-walks toward the server-supplied destination.
|
||||
2. **B.6 #75 refactor (`f035ea3`, 2026-05-16):** `ApplyAutoWalkOverlay → DriveServerAutoWalk` drives the body directly from path data, no input synthesis. The `MoveToState` leak that previously cancelled ACE's `MoveToChain` callback is gone; the chain runs uninterrupted and `TryUseItem` / `TryPickUp` fires server-side on arrival. No client-side retry needed. Walk/run threshold corrected to 1.0m (matches retail-observed; overrides ACE's wire-default 15m).
|
||||
|
||||
Visual-verified end-to-end: far-range Use on NPCs / doors / spell components / corpses all complete via ACE's server-side callback. The far-range retry workaround from Task 1's first iteration (`c61d049`'s `_pendingPostArrivalAction` arming) was deleted as part of #75 (`f035ea3`).
|
||||
|
||||
**Description:** When the player triggers a Use or PutItemInContainer
|
||||
on a target outside ACE's `WithinUseRadius` (default 0.6 m), ACE
|
||||
runs server-side auto-walk via `CreateMoveToChain` →
|
||||
|
|
|
|||
|
|
@ -0,0 +1,631 @@
|
|||
# Phase B.6 — Suppress MoveToState during inbound auto-walk Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Stop sending outbound `MoveToState` while ACE's server-initiated auto-walk is driving the player, then retire the Commit B workarounds that compensated for the resulting `MoveToChain` cancellation. ACE's `TryUseItem` callback fires on arrival; client sends Use exactly once.
|
||||
|
||||
**Architecture:** One-line guard against the misleading wire packet, two retry-assignment deletions, one revert of the AP-cadence gate to retail's narrow shape. No new files, no new tests (behavioral change is wire-level integration; covered by existing Core.Net suite + user visual verify).
|
||||
|
||||
**Tech Stack:** C# .NET 10. Edits touch `AcDream.App/Rendering/GameWindow.cs`, `AcDream.App/Input/PlayerMovementController.cs`, `docs/ISSUES.md`.
|
||||
|
||||
**Spec:** [docs/superpowers/specs/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk-design.md](../specs/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk-design.md).
|
||||
|
||||
**Retail anchors:**
|
||||
- `Player_Use.cs:205` (ACE) — `CreateMoveToChain(item, (success) => TryUseItem(item, success))`.
|
||||
- `Player_Move.cs:150` (ACE) — chain polls and fires `callback(true)` when within use radius.
|
||||
- `acclient_2013_pseudo_c.txt:700233-700285` — retail `ShouldSendPositionEvent`: narrow gate (cell-or-plane change during sub-interval; frame change after interval; gated on `Contact && OnWalkable`).
|
||||
- `acclient_2013_pseudo_c.txt:700327` — retail `SendPositionEvent`: `(state & 1) != 0 && (state & 2) != 0`.
|
||||
|
||||
---
|
||||
|
||||
## File structure
|
||||
|
||||
| File | Responsibility | Touched in tasks |
|
||||
|---|---|---|
|
||||
| `src/AcDream.App/Rendering/GameWindow.cs` | Outbound wire layer. Guard MoveToState build (Task 1); delete two retry assignments + log strings (Task 1); update NotePositionSent call sites to pass contact plane (Task 3). | 1, 3 |
|
||||
| `src/AcDream.App/Input/PlayerMovementController.cs` | Diff-driven cadence state + the auto-walk overlay. Add `_lastSentContactPlane` field; extend `NotePositionSent` signature; replace per-frame `positionChanged` gate with retail's narrow gate (Task 3). | 3 |
|
||||
| `docs/ISSUES.md` | Close `#63` and `#74` to "Recently closed" (Task 5). | 5 |
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Suppress outbound MoveToState during server auto-walk + delete the retry workarounds
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` line ~6410 (the MoveToState send block).
|
||||
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` line ~9203 (SendUse far-range path).
|
||||
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` line ~9265 (SendPickUp far-range path).
|
||||
|
||||
### Step 1: Guard the outbound MoveToState build
|
||||
|
||||
Find the block at `GameWindow.cs:6410` that reads:
|
||||
|
||||
```csharp
|
||||
if (result.MotionStateChanged)
|
||||
{
|
||||
// HoldKey axis values — retail enum (holtburger types.rs HoldKey):
|
||||
```
|
||||
|
||||
Change the condition to:
|
||||
|
||||
```csharp
|
||||
// 2026-05-16 (Phase B.6): suppress outbound MoveToState while
|
||||
// ACE's server-initiated auto-walk is driving the player.
|
||||
// Synthesized Forward+Run input in ApplyAutoWalkOverlay leaks
|
||||
// to MotionStateChanged=true; sending the resulting "user is
|
||||
// RunForward" wire packet makes ACE cancel its own MoveToChain
|
||||
// (Player_Move.cs:150 callback never fires). Retail and
|
||||
// holtburger walk the body locally during inbound MoveToObject
|
||||
// WITHOUT sending an outbound MoveToState — AutonomousPosition
|
||||
// alone is enough for ACE's WithinUseRadius poll.
|
||||
if (result.MotionStateChanged && !_playerController.IsServerAutoWalking)
|
||||
{
|
||||
// HoldKey axis values — retail enum (holtburger types.rs HoldKey):
|
||||
```
|
||||
|
||||
(Only the `if` line changes; the comment above is new. Leave the rest of the block untouched.)
|
||||
|
||||
- [ ] **Step 2: Delete the SendUse far-range retry assignment**
|
||||
|
||||
Find the SendUse method's far-range block. Search:
|
||||
|
||||
```
|
||||
grep -n "Far range:" src/AcDream.App/Rendering/GameWindow.cs
|
||||
```
|
||||
|
||||
Expected: line ~9197. The block reads (paraphrased — find the exact text in the file):
|
||||
|
||||
```csharp
|
||||
// Far range: send Use immediately so ACE has the request,
|
||||
// AND queue an arrival re-send. Issue #63 (server-initiated
|
||||
// MoveToObject not honored) means ACE's first-Use response
|
||||
// is dropped as too-far and ACE doesn't re-poll
|
||||
// WithinUseRadius when the speculative local walk gets us in
|
||||
// range. The arrival re-send fires a second Use packet once
|
||||
// the body reaches the target — at which point ACE accepts
|
||||
// and executes the action. The retail-faithful path is to
|
||||
// honor MoveToObject and let ACE complete the Use server-
|
||||
// side; until #63 lands, this client-side retry is the
|
||||
// workaround that keeps far-range Use working.
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildUse(seq, guid);
|
||||
_liveSession.SendGameAction(body);
|
||||
_pendingPostArrivalAction = (guid, false);
|
||||
Console.WriteLine($"[B.4b] use guid=0x{guid:X8} seq={seq} (queued for arrival re-send pending #63)");
|
||||
```
|
||||
|
||||
Replace with:
|
||||
|
||||
```csharp
|
||||
// Far range: send Use ONCE. ACE's CreateMoveToChain
|
||||
// (Player_Use.cs:205) holds a callback (TryUseItem) and fires
|
||||
// it server-side when WithinUseRadius passes during the
|
||||
// MoveToChain poll (Player_Move.cs:150). No client-side retry
|
||||
// needed — the Phase B.6 MoveToState-suppression fix
|
||||
// (GameWindow.cs:6410) keeps ACE's chain alive during the
|
||||
// walk.
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildUse(seq, guid);
|
||||
_liveSession.SendGameAction(body);
|
||||
Console.WriteLine($"[B.4b] use guid=0x{guid:X8} seq={seq}");
|
||||
```
|
||||
|
||||
(Removes the `_pendingPostArrivalAction = (guid, false);` line and trims the log to drop the `(queued for arrival re-send pending #63)` suffix.)
|
||||
|
||||
- [ ] **Step 3: Delete the SendPickUp far-range retry assignment**
|
||||
|
||||
Find the SendPickUp method's far-range block. Search:
|
||||
|
||||
```
|
||||
grep -n "Far range: same arrival-retry pattern" src/AcDream.App/Rendering/GameWindow.cs
|
||||
```
|
||||
|
||||
Expected: line ~9255. Replace the block:
|
||||
|
||||
```csharp
|
||||
// Far range: same arrival-retry pattern as SendUse — fire
|
||||
// PickUp immediately AND queue for arrival re-send. ACE's
|
||||
// first PickUp is dropped if we're outside the use-radius
|
||||
// and isn't re-polled (issue #63 workaround).
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildPickUp(
|
||||
seq, itemGuid, _playerServerGuid, placement: 0);
|
||||
_liveSession.SendGameAction(body);
|
||||
_pendingPostArrivalAction = (itemGuid, true);
|
||||
Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq} (queued for arrival re-send pending #63)");
|
||||
```
|
||||
|
||||
With:
|
||||
|
||||
```csharp
|
||||
// Far range: send PickUp ONCE. Same auto-fire-via-MoveToChain
|
||||
// callback pattern as SendUse — ACE's chain fires
|
||||
// PutItemInContainer/Move server-side when in range. No
|
||||
// client-side retry; Phase B.6 MoveToState suppression keeps
|
||||
// ACE's chain alive.
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildPickUp(
|
||||
seq, itemGuid, _playerServerGuid, placement: 0);
|
||||
_liveSession.SendGameAction(body);
|
||||
Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq}");
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Build**
|
||||
|
||||
```
|
||||
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
|
||||
```
|
||||
|
||||
Expected: 0 errors, 0 warnings. The deletions remove the only assignment of `_pendingPostArrivalAction` for far-range paths; the close-range path still assigns it (line ~9191 and ~9258). The field declaration at line ~799 stays.
|
||||
|
||||
- [ ] **Step 5: Run existing tests**
|
||||
|
||||
```
|
||||
dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj -c Debug --nologo
|
||||
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj -c Debug --nologo
|
||||
```
|
||||
|
||||
Expected: Core.Net 294/294 pass. Core 1073/1081 pass (baseline; 8 pre-existing physics failures unchanged).
|
||||
|
||||
- [ ] **Step 6: Visual verification (user-driven)**
|
||||
|
||||
User stops any running AcDream.App gracefully via the close-window button, waits ~3 seconds, launches:
|
||||
|
||||
```powershell
|
||||
$env:ACDREAM_DAT_DIR = "$env:USERPROFILE\Documents\Asheron's Call"
|
||||
$env:ACDREAM_LIVE = "1"
|
||||
$env:ACDREAM_TEST_HOST = "127.0.0.1"
|
||||
$env:ACDREAM_TEST_PORT = "9000"
|
||||
$env:ACDREAM_TEST_USER = "testaccount"
|
||||
$env:ACDREAM_TEST_PASS = "testpassword"
|
||||
$env:ACDREAM_DEVTOOLS = "1"
|
||||
$env:ACDREAM_PROBE_AUTOWALK = "1"
|
||||
dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug 2>&1 | Tee-Object -FilePath "launch.log"
|
||||
```
|
||||
|
||||
Scenarios to test:
|
||||
|
||||
1. **Far-range Use NPC.** Double-click a Royal Guard / Pathwarden ~8 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` follow-up. Dialogue fires on arrival from ACE's `TryUseItem` callback.
|
||||
|
||||
2. **Far-range PickUp.** F-key a ground item ~5 m away. Same shape — single `[B.5] pickup` line, no `pickup-deferred`, item enters inventory.
|
||||
|
||||
3. **Close-range Use NPC behind player.** Within 3 m, press R. Body turns 180°. Expected log:
|
||||
```
|
||||
[B.4b] use deferred (close-range, turn-first) guid=0x...
|
||||
[autowalk-end] reason=arrived
|
||||
[B.4b] use-deferred guid=0x... seq=X
|
||||
```
|
||||
(Close-range deferred path is unchanged; `use-deferred` is correct here.)
|
||||
|
||||
4. **Open inn door from across the room.** ONE `[B.4b] use` line, no retry, door opens once.
|
||||
|
||||
5. **User takes manual control mid-auto-walk.** Click far NPC → press W during the walk. Auto-walk cancels (`EndServerAutoWalk("user-input")`). Action does NOT fire on arrival.
|
||||
|
||||
**STOP and wait for user confirmation that scenarios 1–5 pass.**
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add src/AcDream.App/Rendering/GameWindow.cs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
fix(retail): suppress outbound MoveToState during inbound auto-walk
|
||||
|
||||
Phase B.6 — retire the Commit B issue-#63 workarounds by plugging the
|
||||
underlying leak that caused them.
|
||||
|
||||
ApplyAutoWalkOverlay synthesizes Forward+Run input during inbound
|
||||
MoveToObject so the existing motion-interpreter pipeline drives body
|
||||
position + animation locally. That synth set MotionStateChanged=true,
|
||||
so the outbound wire layer (GameWindow.cs:6410) built a MoveToState
|
||||
packet with forwardCommand=RunForward and sent it to ACE. ACE read
|
||||
the packet as "user took manual control" and cancelled its own
|
||||
CreateMoveToChain (Player_Use.cs:205 → Player_Move.cs:150), so the
|
||||
TryUseItem callback never fired on arrival. Our workaround sent Use
|
||||
a second time at local-arrival to bypass ACE's cancelled chain.
|
||||
|
||||
Fix: one-line guard. The MoveToState send only fires when
|
||||
!_playerController.IsServerAutoWalking. AutonomousPosition keeps
|
||||
flowing during the walk (so ACE's WithinUseRadius poll sees the
|
||||
player approach); ACE's chain runs uninterrupted; callback fires
|
||||
when in range. Retail and holtburger (simulation.rs:178-206) follow
|
||||
the same pattern — no outbound MoveToState during inbound MoveToObject.
|
||||
|
||||
Deletes the retry workarounds:
|
||||
- SendUse far-range: `_pendingPostArrivalAction = (guid, false);`
|
||||
+ the `(queued for arrival re-send pending #63)` log
|
||||
- SendPickUp far-range: same shape
|
||||
|
||||
Close-range turn-first deferred path (separate code, retail-faithful
|
||||
pre-callback rotation) is unchanged.
|
||||
|
||||
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>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Visual checkpoint — confirm Task 1's fix before touching cadence
|
||||
|
||||
This is not a code task. The Step 6 visual verification in Task 1 establishes that ACE's `MoveToChain` callback fires correctly with the MoveToState suppression in place. Only proceed to Task 3 if all five scenarios pass. If any regresses, STOP and revert Task 1's commit before continuing.
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Revert AP cadence to retail's narrow gate
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs` line ~254 (field block), line ~441-449 (`NotePositionSent`), line ~1240-1275 (the gate logic), line ~289-296 (`AutoWalkArrived` doc-comment cleanup).
|
||||
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` line ~6450 (MTS NotePositionSent call), line ~6476 (AP NotePositionSent call).
|
||||
|
||||
- [ ] **Step 1: Add `_lastSentContactPlane` field**
|
||||
|
||||
In `PlayerMovementController.cs`, find the diff-tracking field block (around line 535-540 — search for `_lastSentPos`):
|
||||
|
||||
```csharp
|
||||
private System.Numerics.Vector3 _lastSentPos;
|
||||
private uint _lastSentCellId;
|
||||
private float _lastSentTime;
|
||||
private bool _lastSentInitialized;
|
||||
```
|
||||
|
||||
Add a new field immediately after `_lastSentCellId`:
|
||||
|
||||
```csharp
|
||||
private System.Numerics.Vector3 _lastSentPos;
|
||||
private uint _lastSentCellId;
|
||||
private System.Numerics.Plane _lastSentContactPlane;
|
||||
private float _lastSentTime;
|
||||
private bool _lastSentInitialized;
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Extend `NotePositionSent` to accept the contact plane**
|
||||
|
||||
Find `NotePositionSent` in `PlayerMovementController.cs` (around line 441). Replace:
|
||||
|
||||
```csharp
|
||||
public void NotePositionSent(System.Numerics.Vector3 worldPos,
|
||||
uint cellId,
|
||||
float nowSeconds)
|
||||
{
|
||||
_lastSentPos = worldPos;
|
||||
_lastSentCellId = cellId;
|
||||
_lastSentTime = nowSeconds;
|
||||
_lastSentInitialized = true;
|
||||
}
|
||||
```
|
||||
|
||||
With:
|
||||
|
||||
```csharp
|
||||
/// <summary>
|
||||
/// 2026-05-16 (Phase B.6). Called by the network outbound layer
|
||||
/// after every AutonomousPosition or MoveToState that carries the
|
||||
/// player's position. Resets the diff-driven heartbeat clock so the
|
||||
/// next <see cref="HeartbeatDue"/> evaluation requires a fresh
|
||||
/// state change. Mirrors retail's SendPositionEvent
|
||||
/// (acclient_2013_pseudo_c.txt:700345-700348) which writes
|
||||
/// `last_sent_position`, `last_sent_position_time`, and
|
||||
/// `last_sent_contact_plane` after every send.
|
||||
/// </summary>
|
||||
public void NotePositionSent(System.Numerics.Vector3 worldPos,
|
||||
uint cellId,
|
||||
System.Numerics.Plane contactPlane,
|
||||
float nowSeconds)
|
||||
{
|
||||
_lastSentPos = worldPos;
|
||||
_lastSentCellId = cellId;
|
||||
_lastSentContactPlane = contactPlane;
|
||||
_lastSentTime = nowSeconds;
|
||||
_lastSentInitialized = true;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace the per-frame gate with retail's narrow gate**
|
||||
|
||||
Find the cadence block in `PlayerMovementController.cs` (around line 1240-1275 — search for `retail diff-driven AP cadence`). Replace the block starting at the `// 2026-05-16 — retail diff-driven AP cadence` comment through the `HeartbeatDue =` line with:
|
||||
|
||||
```csharp
|
||||
// 2026-05-16 (Phase B.6) — retail-faithful AP cadence per
|
||||
// CommandInterpreter::ShouldSendPositionEvent at
|
||||
// acclient_2013_pseudo_c.txt:700233-700285. Two-branch:
|
||||
//
|
||||
// Branch 1 — interval NOT yet elapsed (< 1 sec since
|
||||
// last send): send only if cell changed OR contact-plane
|
||||
// changed (mid-walk events that matter).
|
||||
//
|
||||
// Branch 2 — interval HAS elapsed (>= 1 sec): send only
|
||||
// if cell OR position frame changed. Truly idle = no
|
||||
// send (retail's `last_sent.frame == player.frame` check
|
||||
// at line 700248-700265).
|
||||
//
|
||||
// SendPositionEvent (line 700327) gates the actual send on
|
||||
// (state & 1) != 0 && (state & 2) != 0 — Contact AND
|
||||
// OnWalkable both set. We mirror that gate here so airborne
|
||||
// and wall-contact-without-walkable states suppress AP
|
||||
// entirely; MoveToState carries jump/fall snapshots while
|
||||
// airborne.
|
||||
//
|
||||
// Effective rates:
|
||||
// - Truly idle (grounded, no movement) : 0 Hz
|
||||
// - Smooth movement (no cell/plane changes) : ~1 Hz (interval-driven)
|
||||
// - Cell crossings + stair/hill steps : per-event
|
||||
// - Airborne : 0 Hz
|
||||
//
|
||||
// Bootstrap: when NotePositionSent has never been called
|
||||
// (_lastSentInitialized=false), treat every frame as
|
||||
// "anything to send" so the first AP gets a chance to fire.
|
||||
|
||||
bool intervalElapsed = !_lastSentInitialized
|
||||
|| (_simTimeSeconds - _lastSentTime) >= HeartbeatInterval;
|
||||
|
||||
bool cellChanged = !_lastSentInitialized
|
||||
|| _lastSentCellId != CellId;
|
||||
bool planeChanged = !_lastSentInitialized
|
||||
|| !_lastSentContactPlane.Equals(_body.ContactPlane);
|
||||
bool frameChanged = !_lastSentInitialized
|
||||
|| !ApproxPositionEqual(_lastSentPos, _body.Position);
|
||||
|
||||
bool sendThisFrame = intervalElapsed
|
||||
? (cellChanged || frameChanged)
|
||||
: (cellChanged || planeChanged);
|
||||
|
||||
// Grounded-on-walkable gate per acclient_2013_pseudo_c.txt:700327
|
||||
// (`(state & 1) != 0 && (state & 2) != 0`). Both flags must be
|
||||
// set simultaneously, NOT a bitwise-OR mask test.
|
||||
bool groundedOnWalkable = _body.InContact && _body.OnWalkable;
|
||||
|
||||
HeartbeatDue = groundedOnWalkable && sendThisFrame;
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update the MTS site to pass `contactPlane`**
|
||||
|
||||
In `GameWindow.cs`, find the MoveToState `NotePositionSent` call (around line 6450). Replace:
|
||||
|
||||
```csharp
|
||||
_playerController.NotePositionSent(
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
```
|
||||
|
||||
With:
|
||||
|
||||
```csharp
|
||||
_playerController.NotePositionSent(
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
contactPlane: _playerController.PhysicsBody.ContactPlane,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
```
|
||||
|
||||
If `_playerController.PhysicsBody` doesn't exist as a public accessor, search:
|
||||
|
||||
```
|
||||
grep -n "public.*_body\|public PhysicsBody\|public.*Body" src/AcDream.App/Input/PlayerMovementController.cs
|
||||
```
|
||||
|
||||
If no public accessor, add one in `PlayerMovementController.cs` near the existing public properties (around line 130-160):
|
||||
|
||||
```csharp
|
||||
/// <summary>2026-05-16. Read-only access to the controller's
|
||||
/// physics body — needed by the network outbound layer to stamp
|
||||
/// the contact plane into NotePositionSent.</summary>
|
||||
public AcDream.Core.Physics.PhysicsBody PhysicsBody => _body;
|
||||
```
|
||||
|
||||
(Verify the field name is `_body` first — search `private.*PhysicsBody`.)
|
||||
|
||||
- [ ] **Step 5: Update the AP site to pass `contactPlane`**
|
||||
|
||||
Find the AutonomousPosition `NotePositionSent` call (around line 6476). Apply the same edit:
|
||||
|
||||
```csharp
|
||||
_playerController.NotePositionSent(
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
contactPlane: _playerController.PhysicsBody.ContactPlane,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Build**
|
||||
|
||||
```
|
||||
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
|
||||
```
|
||||
|
||||
Expected: 0 errors. Any compile error here is a wiring mistake — the field name (`_body` vs `_physicsBody`), the property accessor, or the `Plane` namespace.
|
||||
|
||||
- [ ] **Step 7: Run existing tests**
|
||||
|
||||
```
|
||||
dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj -c Debug --nologo
|
||||
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj -c Debug --nologo
|
||||
```
|
||||
|
||||
Expected: Core.Net 294/294, Core 1073/1081 (baseline unchanged).
|
||||
|
||||
- [ ] **Step 8: Visual verification (user-driven)**
|
||||
|
||||
User restarts the client (graceful close + ~3 sec wait + launch). Runs scenarios:
|
||||
|
||||
1. **Idle test.** Stand still on flat ground in Holtburg for 10 sec. Watch the dev console / `[autowalk-up]` lines or any outbound packet trace.
|
||||
- Old behavior: 1 AP/sec heartbeat.
|
||||
- New behavior: ZERO outbound packets while truly idle.
|
||||
|
||||
2. **Smooth-running test.** Hold W and run in a straight line for 5 sec on flat ground (no cell crossings).
|
||||
- Old behavior: ~60 AP/sec (per-frame while position changed).
|
||||
- New behavior: ~1 AP/sec (interval-driven; cell/plane don't change every frame).
|
||||
- **The character should still appear to remote observers as moving smoothly** — ACE's dead-reckoning fills in the gaps between sparse APs. If remote view becomes jittery, the cadence is too sparse and we'll need to tune.
|
||||
|
||||
3. **Cell-crossing test.** Run across a landblock boundary (every ~192 m). Should see a burst of AP packets at the crossing — both the `cellChanged` path and the `intervalElapsed && frameChanged` path can fire here.
|
||||
|
||||
4. **Far-range Use re-test.** Repeat Task 1 Step 6 scenario 1 (far-range NPC Use). Should still work — ACE's `MoveToChain` callback fires on arrival, dialogue plays, single Use packet.
|
||||
|
||||
5. **Hill / stair test.** Walk up a slope or stairs. Contact-plane changes per step → sub-interval AP sends fire on plane change. Behavior should look smooth to remote observers.
|
||||
|
||||
**STOP and wait for user confirmation that scenarios 1–5 pass.** If scenario 2 produces visible remote-jitter, the spec's `ApproxPositionEqual` epsilon may need tightening, or we may need a higher heartbeat rate; document the finding and tune before continuing.
|
||||
|
||||
- [ ] **Step 9: Commit**
|
||||
|
||||
```bash
|
||||
git add src/AcDream.App/Input/PlayerMovementController.cs src/AcDream.App/Rendering/GameWindow.cs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
fix(retail): revert AP cadence to retail's narrow gate
|
||||
|
||||
Phase B.6 — closes #74. With the MoveToState suppression fix in
|
||||
place, the per-frame "send while moving" cadence is no longer needed
|
||||
to compensate for ACE's MoveToChain cancellation. Reverts to retail's
|
||||
two-branch gate per CommandInterpreter::ShouldSendPositionEvent at
|
||||
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.
|
||||
|
||||
Bootstrap fires every frame until the first NotePositionSent.
|
||||
Grounded-on-walkable gate (Contact && OnWalkable) unchanged from
|
||||
700327.
|
||||
|
||||
Effective rates:
|
||||
Truly idle (grounded, no movement) : 0 Hz (was 1 Hz)
|
||||
Smooth straight-line run : ~1 Hz (was ~60 Hz)
|
||||
Cell crossings + stair/hill steps : per-event
|
||||
Airborne : 0 Hz (unchanged)
|
||||
|
||||
Adds _lastSentContactPlane field + extends NotePositionSent to accept
|
||||
System.Numerics.Plane. Adds PhysicsBody public accessor so the wire
|
||||
layer can read _body.ContactPlane to pass into NotePositionSent. Both
|
||||
outbound sites (MoveToState at GameWindow.cs:6450, AP at ~6476)
|
||||
updated to pass the plane.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Visual checkpoint — confirm Task 3's cadence revert before closing issues
|
||||
|
||||
Same as Task 2. Only proceed to Task 5 if all five scenarios in Task 3 Step 8 pass cleanly.
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Close issues #63 and #74
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/ISSUES.md`.
|
||||
|
||||
- [ ] **Step 1: Move issue #63 to "Recently closed"**
|
||||
|
||||
Find `## #63 — Server-initiated auto-walk (MoveToObject) not honored` in `docs/ISSUES.md` (around line 425). Currently `Status: OPEN`. Cut the entire `#63` block from the active issues section and paste it into the "Recently closed" section near the bottom of the file with these changes:
|
||||
|
||||
1. Change the title line from:
|
||||
```markdown
|
||||
## #63 — Server-initiated auto-walk (MoveToObject) not honored
|
||||
```
|
||||
to:
|
||||
```markdown
|
||||
## #63 — [DONE 2026-05-16 · `<TASK1_SHA>`] Server-initiated auto-walk (MoveToObject) not honored
|
||||
```
|
||||
(Replace `<TASK1_SHA>` with the actual commit SHA from Task 1. Get it via `git log --oneline -5`.)
|
||||
|
||||
2. Change `Status: OPEN` to `Status: DONE`.
|
||||
|
||||
3. Append a "Resolution" paragraph after the existing "Acceptance":
|
||||
```markdown
|
||||
**Resolution (2026-05-16):** B.6 slice 2 (2026-05-14) shipped the inbound-MoveToObject auto-walk handling. The remaining "MoveToChain callback never fires on arrival" half was tracked to ApplyAutoWalkOverlay's synthesized Forward+Run input leaking to the wire as an outbound MoveToState packet (forwardCommand=RunForward), which ACE read as "user took manual control" and used to cancel its own MoveToChain. Fix in `<TASK1_SHA>` adds a single guard at `GameWindow.cs:6410`: outbound MoveToState only fires when `!_playerController.IsServerAutoWalking`. With ACE's chain running uninterrupted, the `TryUseItem` callback (Player_Use.cs:205) fires server-side on arrival; no client retry needed. Retired the `_pendingPostArrivalAction` retry workarounds from SendUse + SendPickUp far-range paths.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Move issue #74 to "Recently closed"**
|
||||
|
||||
Find `## #74 — AP cadence is per-frame-while-moving, more chatty than retail`. Same shape: cut the block, paste in "Recently closed", change title to:
|
||||
|
||||
```markdown
|
||||
## #74 — [DONE 2026-05-16 · `<TASK3_SHA>`] AP cadence is per-frame-while-moving, more chatty than retail
|
||||
```
|
||||
|
||||
Change `Status: OPEN` to `Status: DONE`. Append:
|
||||
|
||||
```markdown
|
||||
**Resolution (2026-05-16):** With #63 closed (MoveToState no longer cancels ACE's MoveToChain), the per-frame-while-moving cadence workaround is unnecessary. Reverted to retail's two-branch ShouldSendPositionEvent gate per `acclient_2013_pseudo_c.txt:700233-700285` in `<TASK3_SHA>`. Effective rate during smooth motion drops from ~60 Hz to ~1 Hz; truly idle drops from 1 Hz to 0 Hz. Cell crossings + contact-plane changes still fire mid-interval. Matches retail bit-for-bit.
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/ISSUES.md
|
||||
git commit -m "$(cat <<'EOF'
|
||||
docs: close #63 (MoveToObject not honored) + #74 (AP chattier than retail)
|
||||
|
||||
Both retired by Phase B.6. #63 fixed in <TASK1_SHA> (MoveToState
|
||||
suppression during inbound auto-walk + retry workaround retirement).
|
||||
#74 fixed in <TASK3_SHA> (AP cadence reverted to retail's two-branch
|
||||
ShouldSendPositionEvent gate now that the workaround that needed
|
||||
per-frame APs is gone).
|
||||
|
||||
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
(Replace `<TASK1_SHA>` and `<TASK3_SHA>` with the actual SHAs.)
|
||||
|
||||
---
|
||||
|
||||
## Self-review checklist
|
||||
|
||||
**Spec coverage:**
|
||||
|
||||
| Spec section | Plan task |
|
||||
|---|---|
|
||||
| Wire-level changes: `IsServerAutoWalking` guard at `GameWindow.cs:6410` | Task 1 Step 1 ✅ |
|
||||
| Far-range Use retry deletion | Task 1 Step 2 ✅ |
|
||||
| Far-range PickUp retry deletion | Task 1 Step 3 ✅ |
|
||||
| Log string cleanup | Task 1 Steps 2+3 ✅ |
|
||||
| `_lastSentContactPlane` field + `NotePositionSent` signature | Task 3 Steps 1+2 ✅ |
|
||||
| Retail-narrow gate | Task 3 Step 3 ✅ |
|
||||
| MTS site contactPlane wiring | Task 3 Step 4 ✅ |
|
||||
| AP site contactPlane wiring | Task 3 Step 5 ✅ |
|
||||
| Testing plan (visual scenarios) | Task 1 Step 6 + Task 3 Step 8 ✅ |
|
||||
| Close `#63` + `#74` | Task 5 ✅ |
|
||||
| Out-of-scope `#75` (status messages) | Filed as deferred — not in this plan ✅ |
|
||||
|
||||
**Placeholder scan:** No "TBD" / "implement later" / vague phrases. Every step has actual code or actual commands.
|
||||
|
||||
**Type consistency:**
|
||||
- `IsServerAutoWalking` referenced Task 1 Step 1 — already exists in code (verified at PlayerMovementController.cs:273). ✅
|
||||
- `_lastSentContactPlane : System.Numerics.Plane` defined Task 3 Step 1, used Task 3 Steps 2+3. ✅
|
||||
- `NotePositionSent(Vector3, uint, Plane, float)` defined Task 3 Step 2, called Task 3 Steps 4+5. ✅
|
||||
- `_playerController.PhysicsBody` property defined Task 3 Step 4 (conditional add if missing), used Task 3 Steps 4+5. ✅
|
||||
- `_lastSentPos`, `_lastSentCellId`, `_lastSentTime`, `_lastSentInitialized` — pre-existing from Commit B. ✅
|
||||
|
||||
**Risk / rollback:**
|
||||
|
||||
- Task 1 commit: simple revert restores the workaround.
|
||||
- Task 3 commit: simple revert restores the per-frame cadence.
|
||||
- Task 5 commit: docs-only; trivial.
|
||||
|
||||
Each task is independently revertable. If Task 3 introduces remote-view jitter (scenario 2), revert Task 3 and re-evaluate (e.g., dial down `HeartbeatInterval` from 1 s to 0.5 s).
|
||||
|
||||
---
|
||||
|
||||
## Execution handoff
|
||||
|
||||
Plan saved to `docs/superpowers/plans/2026-05-16-phase-b6-suppress-movetostate-during-inbound-autowalk.md`.
|
||||
|
||||
Two options for the controller:
|
||||
|
||||
1. **Subagent-Driven (recommended)** — Dispatch fresh subagent per task. Two-stage review (spec compliance + code quality) between tasks. ~3 implementer + 6 reviewer dispatches.
|
||||
|
||||
2. **Inline Execution** — Execute tasks in this session using `superpowers:executing-plans`. Two visual-verify checkpoints (between Task 1 & Task 3, between Task 3 & Task 5).
|
||||
|
||||
Given the small scope (~30 LOC of behavior change + docs) and the two mandatory user-driven visual checkpoints, inline execution may be simpler — the subagent overhead exceeds the implementation time for tasks this small.
|
||||
|
|
@ -0,0 +1,156 @@
|
|||
# Phase B.6 — Suppress outbound `MoveToState` during inbound MoveToObject auto-walk
|
||||
|
||||
**Date:** 2026-05-16
|
||||
**Closes:** [#63](../../ISSUES.md) (server-initiated auto-walk not honored — the half about ACE's `MoveToChain` callback never firing), [#74](../../ISSUES.md) (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`](../../../references/ACE/Source/ACE.Server/WorldObjects/Player_Use.cs)).
|
||||
3. ACE broadcasts `UpdateMotion` with `MovementType=6` (MoveToObject) targeting our player.
|
||||
4. Our client honors it via `BeginServerAutoWalk(...)` at [`GameWindow.cs:3389-3414`](../../../src/AcDream.App/Rendering/GameWindow.cs).
|
||||
5. The auto-walk overlay at [`PlayerMovementController.ApplyAutoWalkOverlay`](../../../src/AcDream.App/Input/PlayerMovementController.cs) 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`](../../../src/AcDream.App/Rendering/GameWindow.cs) 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](../../../src/AcDream.App/Rendering/GameWindow.cs)). 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:205` — `CreateMoveToChain(item, (success) => TryUseItem(item, success))`.
|
||||
- ACE `Player_Move.cs:150` — chain polls and fires `callback(true)` when within use radius.
|
||||
- Holtburger [`simulation.rs:178-206`](../../../references/holtburger/crates/holtburger-core/src/client/simulation.rs) — `MoveToObject` 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`](../../../src/AcDream.App/Input/PlayerMovementController.cs)**
|
||||
|
||||
No new field needed — [`IsServerAutoWalking`](../../../src/AcDream.App/Input/PlayerMovementController.cs:273) 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`](../../../src/AcDream.App/Rendering/GameWindow.cs)**
|
||||
|
||||
Guard the `MoveToState.Build` block at lines 6410-6445 with a second condition:
|
||||
|
||||
```csharp
|
||||
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`](../../../src/AcDream.App/Rendering/GameWindow.cs) (~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`](../../../src/AcDream.App/Rendering/GameWindow.cs) (~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`](../../../src/AcDream.App/Input/PlayerMovementController.cs:1261) with retail's narrow gate per `acclient_2013_pseudo_c.txt:700233-700285`:
|
||||
|
||||
```csharp
|
||||
// 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.
|
||||
|
|
@ -154,6 +154,16 @@ public sealed class PlayerMovementController
|
|||
/// <summary>Full 3D world-space velocity of the physics body. Exposed for diagnostic logging.</summary>
|
||||
public Vector3 BodyVelocity => _body.Velocity;
|
||||
|
||||
/// <summary>
|
||||
/// 2026-05-16 — current contact plane (normal + distance) for the
|
||||
/// physics body. Exposed so the network outbound layer can stamp
|
||||
/// it into <see cref="NotePositionSent"/> for retail's diff-driven
|
||||
/// AP cadence: SendPositionEvent re-sends if cell OR contact-plane
|
||||
/// changed since last_sent, per
|
||||
/// <c>acclient_2013_pseudo_c.txt:700233 ShouldSendPositionEvent</c>.
|
||||
/// </summary>
|
||||
public System.Numerics.Plane ContactPlane => _body.ContactPlane;
|
||||
|
||||
// Jump charge state.
|
||||
private bool _jumpCharging;
|
||||
private float _jumpExtent;
|
||||
|
|
@ -203,6 +213,7 @@ public sealed class PlayerMovementController
|
|||
|
||||
private System.Numerics.Vector3 _lastSentPos;
|
||||
private uint _lastSentCellId;
|
||||
private System.Numerics.Plane _lastSentContactPlane;
|
||||
private float _lastSentTime;
|
||||
private bool _lastSentInitialized;
|
||||
private float _simTimeSeconds;
|
||||
|
|
@ -256,22 +267,51 @@ public sealed class PlayerMovementController
|
|||
private float _autoWalkMinDistance;
|
||||
private float _autoWalkDistanceToObject;
|
||||
private bool _autoWalkMoveTowards;
|
||||
// Walk-vs-run decision is made ONCE at BeginServerAutoWalk based on
|
||||
// initial distance vs the wire's WalkRunThreshold, then held for the
|
||||
// duration of the auto-walk. Earlier per-frame evaluation produced
|
||||
// "runs partway then walks the rest" which the user reported as
|
||||
// wrong: the character should run all the way to the target then
|
||||
// stop, not transition to a walk near the end.
|
||||
// 2026-05-16 (retail-faithful) — walk-vs-run is a ONE-SHOT
|
||||
// decision at chain start. Per user observation 2026-05-16: if
|
||||
// initial distance is at or above the walk-run threshold, the
|
||||
// body runs all the way to the target; otherwise it walks all
|
||||
// the way. No per-frame switching as the player closes distance.
|
||||
//
|
||||
// Formula matches retail's MovementParameters::get_command
|
||||
// (decomp 0x0052aa00, line 308000+):
|
||||
// running = (initialDist - distance_to_object) >= walk_run_threshhold
|
||||
// The "distance left to walk" (current minus use-radius) is
|
||||
// compared against the wire-supplied threshold (15m default,
|
||||
// retail constant at 0x005243b5). The retail function reads
|
||||
// `arg2` as the current distance but in practice is called at
|
||||
// chain setup with the initial distance, and the resulting
|
||||
// decision is cached for the rest of the chain — matching the
|
||||
// user-observed "run all the way / walk all the way" behaviour.
|
||||
private bool _autoWalkInitiallyRunning;
|
||||
|
||||
/// <summary>
|
||||
/// True while a server-initiated auto-walk (MoveToObject inbound) is
|
||||
/// active on the local player. The next <see cref="Update"/> call
|
||||
/// synthesizes Forward+Run input and steers <see cref="Yaw"/> toward
|
||||
/// the destination until arrival or user-input cancellation.
|
||||
/// active on the local player. Update drives the body's velocity
|
||||
/// and motion state machine DIRECTLY from the wire-supplied path
|
||||
/// data, NOT via synthesized player-input. The
|
||||
/// motion-state-change detection downstream sees no user input
|
||||
/// during auto-walk, so no MoveToState wire packet is built — ACE's
|
||||
/// server-side MoveToChain can run uninterrupted until its callback
|
||||
/// fires.
|
||||
/// </summary>
|
||||
public bool IsServerAutoWalking => _autoWalkActive;
|
||||
|
||||
// 2026-05-16 (issue #75) — tracks whether the auto-walk overlay is
|
||||
// actually advancing the body this frame. False during the
|
||||
// turn-first phase (rotating in place toward target) and after
|
||||
// arrival. Drives the animation cycle override: walking animation
|
||||
// only plays when the body is actually moving forward.
|
||||
private bool _autoWalkMovingForwardThisFrame;
|
||||
|
||||
// 2026-05-16 (issue #69 fix) — turn direction this frame.
|
||||
// +1 = rotating counter-clockwise (Yaw increasing) → TurnLeft cycle
|
||||
// -1 = rotating clockwise (Yaw decreasing) → TurnRight cycle
|
||||
// 0 = aligned or not turning
|
||||
// Drives the animation cycle override during turn-first phase so
|
||||
// the body plays the actual turn animation instead of statue-pivoting.
|
||||
private int _autoWalkTurnDirectionThisFrame;
|
||||
|
||||
/// <summary>
|
||||
/// Fires once when an auto-walk reaches its destination naturally
|
||||
/// (i.e. <see cref="EndServerAutoWalk"/> called with
|
||||
|
|
@ -398,17 +438,35 @@ public sealed class PlayerMovementController
|
|||
_autoWalkDistanceToObject = distanceToObject;
|
||||
_autoWalkMoveTowards = moveTowards;
|
||||
|
||||
// Decide run vs walk ONCE based on the initial horizontal
|
||||
// distance from the player to the destination. Run-all-the-way
|
||||
// is the retail-faithful behaviour the user observed: pick a
|
||||
// distant target → character runs the whole way, decelerates
|
||||
// to a stop at use radius. Earlier per-frame evaluation made
|
||||
// the body transition to a walk inside threshold and felt
|
||||
// wrong (the user reported "runs partway then walks").
|
||||
// 2026-05-16 (retail-faithful) — one-shot walk/run decision
|
||||
// using retail's get_command formula (decomp 0x0052aa00):
|
||||
// running = (initialDist - distance_to_object) >= walk_run_threshhold
|
||||
// Subtract the use-radius from the raw distance so the
|
||||
// discriminator is "distance LEFT to walk" — same shape as
|
||||
// retail's `arg2 - distance_to_object` term. Held for the
|
||||
// rest of the auto-walk so the body runs all the way to the
|
||||
// target (or walks all the way), matching observed retail
|
||||
// behaviour.
|
||||
//
|
||||
// The wire-supplied walkRunThreshold defaults to 15m at
|
||||
// retail's MovementParameters constructor (0x005243b5) AND at
|
||||
// ACE's wire-layer MoveToParameters.cs:51 — but the user-
|
||||
// observed retail behaviour is "only walk when very close,"
|
||||
// run from any non-trivial distance. ACE's own physics layer
|
||||
// confirms: MovementParameters.cs:49-50 has the 15.0f default
|
||||
// commented out with `Default_WalkRunThreshold = 1.0f`
|
||||
// active, and Creature.cs:312 comments "default 15 distance
|
||||
// seems too far" before halving it to 7.5f for CanCharge.
|
||||
// We override the wire value with a retail-faithful 1.0m
|
||||
// constant; the wire value (typically 15m from ACE) is
|
||||
// ignored for the run/walk decision. _walkRunThreshold is in
|
||||
// METERS of remaining-distance-to-use-radius.
|
||||
const float RetailWalkRunThresholdMeters = 1.0f;
|
||||
float dx = destinationWorld.X - _body.Position.X;
|
||||
float dy = destinationWorld.Y - _body.Position.Y;
|
||||
float initialDist = MathF.Sqrt(dx * dx + dy * dy);
|
||||
_autoWalkInitiallyRunning = initialDist > walkRunThreshold;
|
||||
float remainingAtStart = initialDist - distanceToObject;
|
||||
_autoWalkInitiallyRunning = remainingAtStart >= RetailWalkRunThresholdMeters;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -431,19 +489,21 @@ public sealed class PlayerMovementController
|
|||
/// 2026-05-16. Called by the network outbound layer after every
|
||||
/// AutonomousPosition or MoveToState that carries the player's
|
||||
/// position. Resets the diff-driven heartbeat clock so the next
|
||||
/// `HeartbeatDue` evaluation requires either a fresh position
|
||||
/// change OR another full HeartbeatInterval. Mirrors retail's
|
||||
/// SendPositionEvent (0x006b4770) which updates
|
||||
/// `last_sent_position_time` + `last_sent_position` at every
|
||||
/// send, AND SendMovementEvent (0x006b4680) which also touches
|
||||
/// the same shared clock (both consumers of the 1 sec window).
|
||||
/// `HeartbeatDue` evaluation requires either a fresh state change
|
||||
/// (cell, contact-plane, or frame) OR another full HeartbeatInterval.
|
||||
/// Mirrors retail's SendPositionEvent at
|
||||
/// <c>acclient_2013_pseudo_c.txt:700345-700348</c> which updates
|
||||
/// `last_sent_position`, `last_sent_position_time`, AND
|
||||
/// `last_sent_contact_plane` after every send.
|
||||
/// </summary>
|
||||
public void NotePositionSent(System.Numerics.Vector3 worldPos,
|
||||
uint cellId,
|
||||
System.Numerics.Plane contactPlane,
|
||||
float nowSeconds)
|
||||
{
|
||||
_lastSentPos = worldPos;
|
||||
_lastSentCellId = cellId;
|
||||
_lastSentContactPlane = contactPlane;
|
||||
_lastSentTime = nowSeconds;
|
||||
_lastSentInitialized = true;
|
||||
}
|
||||
|
|
@ -465,9 +525,33 @@ public sealed class PlayerMovementController
|
|||
/// <c>distanceToObject</c>; flee arrives at <c>minDistance</c>.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private MovementInput ApplyAutoWalkOverlay(float dt, MovementInput input)
|
||||
/// <summary>
|
||||
/// 2026-05-16 (issue #75 refactor) — drive the body directly from
|
||||
/// the wire-supplied path data during server-initiated auto-walk,
|
||||
/// without synthesizing player-input. Replaces the earlier
|
||||
/// ApplyAutoWalkOverlay which returned a synthesized Forward+Run
|
||||
/// MovementInput; that synthesis leaked to the wire as an outbound
|
||||
/// MoveToState packet ("user is RunForward") which ACE read as
|
||||
/// user-took-manual-control and cancelled its own MoveToChain. The
|
||||
/// architecture now mirrors retail's MovementManager::PerformMovement
|
||||
/// case 6 (decomp 0x00524440): step the body's velocity + motion
|
||||
/// state directly; the user-input pipeline downstream sees no input
|
||||
/// because the user didn't press anything, so no MoveToState gets
|
||||
/// built.
|
||||
///
|
||||
/// <para>
|
||||
/// Returns <c>true</c> when this method consumed motion control for
|
||||
/// the frame (auto-walk active, no user override, no arrival).
|
||||
/// Caller (<see cref="Update"/>) must skip the user-input motion +
|
||||
/// body-velocity sections to avoid them overriding the auto-walk's
|
||||
/// velocity assignment.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private bool DriveServerAutoWalk(float dt, MovementInput input)
|
||||
{
|
||||
if (!_autoWalkActive) return input;
|
||||
_autoWalkMovingForwardThisFrame = false;
|
||||
_autoWalkTurnDirectionThisFrame = 0;
|
||||
if (!_autoWalkActive) return false;
|
||||
|
||||
// User-input cancellation. Any direct movement key takes over.
|
||||
// Mouse-only turning (no movement key) doesn't cancel — the
|
||||
|
|
@ -478,7 +562,7 @@ public sealed class PlayerMovementController
|
|||
if (userOverride)
|
||||
{
|
||||
EndServerAutoWalk("user-input");
|
||||
return input;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Horizontal distance to target — server owns Z, our local body
|
||||
|
|
@ -551,16 +635,26 @@ public sealed class PlayerMovementController
|
|||
// MathF.Min(|delta|, maxStep) naturally clamps the final
|
||||
// fractional step to exactly delta, so we land on the
|
||||
// target heading without overshoot.
|
||||
// 2026-05-16 — retail-faithful turn rate. Auto-walk knows
|
||||
// its run/walk decision from _autoWalkInitiallyRunning
|
||||
// (set at BeginServerAutoWalk based on initial distance vs
|
||||
// WalkRunThreshold). Running rotation is 50% faster per
|
||||
// 2026-05-16 — retail-faithful turn rate. Auto-walk's
|
||||
// run/walk decision (one-shot at chain start) drives the
|
||||
// turn rate: running rotation is 50% faster per
|
||||
// run_turn_factor at retail 0x007c8914.
|
||||
float maxStep = RemoteMoveToDriver.TurnRateFor(_autoWalkInitiallyRunning) * dt;
|
||||
Yaw += MathF.Sign(delta) * MathF.Min(MathF.Abs(delta), maxStep);
|
||||
float yawStep = MathF.Sign(delta) * MathF.Min(MathF.Abs(delta), maxStep);
|
||||
Yaw += yawStep;
|
||||
while (Yaw > MathF.PI) Yaw -= 2f * MathF.PI;
|
||||
while (Yaw < -MathF.PI) Yaw += 2f * MathF.PI;
|
||||
|
||||
// 2026-05-16 (issue #69) — record rotation direction so the
|
||||
// animation override can pick the TurnLeft/TurnRight cycle.
|
||||
// Sign convention matches user-driven A/D in Update:
|
||||
// yawStep > 0 ⇔ TurnLeft (Yaw increases)
|
||||
// yawStep < 0 ⇔ TurnRight (Yaw decreases)
|
||||
// Small dead-zone avoids flickering between Turn cycles
|
||||
// when the residual delta is effectively zero.
|
||||
if (MathF.Abs(yawStep) > 1e-5f)
|
||||
_autoWalkTurnDirectionThisFrame = yawStep > 0f ? +1 : -1;
|
||||
|
||||
// Two alignment thresholds:
|
||||
// walkWhileTurning (30°): outside this, body turns in place.
|
||||
// Inside, body walks forward while
|
||||
|
|
@ -585,15 +679,14 @@ public sealed class PlayerMovementController
|
|||
if (withinArrival && aligned)
|
||||
{
|
||||
EndServerAutoWalk("arrived");
|
||||
return input;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Walk vs run decided ONCE at BeginServerAutoWalk based on
|
||||
// initial distance — held for the rest of the auto-walk so the
|
||||
// character keeps running all the way to the target instead of
|
||||
// transitioning to a walk inside the threshold. Matches user-
|
||||
// observed retail behaviour ("if its far away it should run
|
||||
// all the way to the object and then stop").
|
||||
// Walk vs run uses the one-shot decision from BeginServerAutoWalk
|
||||
// (initial distance minus use-radius vs walkRunThreshold).
|
||||
// Held for the rest of the auto-walk so the body runs all
|
||||
// the way to a far target, or walks all the way to a near
|
||||
// one — matching user-observed retail behaviour.
|
||||
bool shouldRun = _autoWalkInitiallyRunning;
|
||||
|
||||
// Turn-first gate: if not yet within the 30° walking band,
|
||||
|
|
@ -603,22 +696,56 @@ public sealed class PlayerMovementController
|
|||
// into it.
|
||||
bool moveForward = walkAligned && !withinArrival;
|
||||
|
||||
// Synthesize "moving forward" input. The rest of Update reads
|
||||
// Yaw + input.Forward + input.Run to drive _motion + body
|
||||
// velocity exactly as it does for user-driven W (+ optional Shift).
|
||||
// We zero any mouse delta so a stale frame doesn't fight the
|
||||
// steering.
|
||||
return input with
|
||||
if (!moveForward)
|
||||
{
|
||||
Forward = moveForward,
|
||||
Run = moveForward && shouldRun,
|
||||
Backward = false,
|
||||
StrafeLeft = false,
|
||||
StrafeRight = false,
|
||||
TurnLeft = false,
|
||||
TurnRight = false,
|
||||
MouseDeltaX = 0f,
|
||||
};
|
||||
// Turn-in-place phase. The motion state needs to be Ready
|
||||
// (or at least not RunForward) so we're not pretending to
|
||||
// run while standing. _motion stays at whatever the user-
|
||||
// input pipeline last set it to (typically Ready/null since
|
||||
// the user isn't pressing W). Don't touch body velocity
|
||||
// either — physics will handle gravity-only behaviour.
|
||||
return true;
|
||||
}
|
||||
|
||||
// Drive motion state machine + body velocity directly. This
|
||||
// mirrors what the user-input section would have done with
|
||||
// synthesized Forward+Run, but without putting anything into
|
||||
// MovementInput — so the outbound-packet pipeline never builds
|
||||
// a MoveToState packet for auto-walk frames.
|
||||
uint forwardCmd;
|
||||
float forwardCmdSpeed;
|
||||
if (shouldRun && _weenie.InqRunRate(out float runRate))
|
||||
{
|
||||
// Wire-compatible: WalkForward command @ runRate triggers
|
||||
// ACE's auto-upgrade to RunForward for observers. Same
|
||||
// shape as the user-input section's running path.
|
||||
forwardCmd = MotionCommand.WalkForward;
|
||||
forwardCmdSpeed = runRate;
|
||||
}
|
||||
else
|
||||
{
|
||||
forwardCmd = MotionCommand.WalkForward;
|
||||
forwardCmdSpeed = 1.0f;
|
||||
}
|
||||
|
||||
_autoWalkMovingForwardThisFrame = true;
|
||||
|
||||
// Update interpreted motion state — drives the animation cycle
|
||||
// via UpdatePlayerAnimation downstream + the MotionInterpreter's
|
||||
// state-velocity getter (used for our velocity assignment below).
|
||||
_motion.DoMotion(forwardCmd, forwardCmdSpeed);
|
||||
|
||||
// Set body velocity directly. Only meaningful when grounded;
|
||||
// mirror the user-input section's `if (_body.OnWalkable)` gate
|
||||
// so we don't override gravity/jump velocity mid-air.
|
||||
if (_body.OnWalkable)
|
||||
{
|
||||
float savedWorldVz = _body.Velocity.Z;
|
||||
var stateVel = _motion.get_state_velocity();
|
||||
_body.set_local_velocity(new Vector3(0f, stateVel.Y, savedWorldVz));
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// L.2a slice 1 (2026-05-12): centralized CellId mutation so the
|
||||
|
|
@ -664,14 +791,22 @@ public sealed class PlayerMovementController
|
|||
{
|
||||
_simTimeSeconds += dt;
|
||||
|
||||
// B.6 slice 2 (2026-05-14): server-initiated auto-walk overlay.
|
||||
// When _autoWalkActive, steer Yaw toward _autoWalkDestination and
|
||||
// synthesize Forward+Run input so the rest of Update runs the
|
||||
// body forward as if the user were holding W. User movement-key
|
||||
// input cancels the auto-walk (retail UX). Arrival check fires
|
||||
// before synthesizing, so a one-frame arrival doesn't waste a
|
||||
// tick of velocity past the target.
|
||||
input = ApplyAutoWalkOverlay(dt, input);
|
||||
// 2026-05-16 (issue #75 refactor): server-initiated auto-walk
|
||||
// drives the body's velocity + motion state machine DIRECTLY.
|
||||
// When _autoWalkActive, DriveServerAutoWalk steps Yaw, computes
|
||||
// velocity from wire-supplied runRate, calls _motion.DoMotion,
|
||||
// and sets _body.set_local_velocity. The user-input motion +
|
||||
// velocity sections below are SKIPPED so they don't override
|
||||
// the auto-walk's assignments. Critically, no synthesized input
|
||||
// gets put back into `input` — the outbound-packet pipeline at
|
||||
// GameWindow.cs:6410 sees user-input null/Ready throughout the
|
||||
// auto-walk and never builds a MoveToState packet, leaving
|
||||
// ACE's server-side MoveToChain to run uninterrupted until its
|
||||
// TryUseItem/TryPickUp callback fires. Retail equivalent:
|
||||
// MovementManager::PerformMovement case 6 (decomp 0x00524440)
|
||||
// calls CPhysicsObj::MoveToObject server-side; the local body
|
||||
// is moved without ever touching CommandInterpreter input.
|
||||
bool autoWalkConsumedMotion = DriveServerAutoWalk(dt, input);
|
||||
|
||||
// Portal-space guard: while teleporting, no input is processed and
|
||||
// no physics is resolved. Return a zero-movement result so the caller
|
||||
|
|
@ -719,6 +854,14 @@ public sealed class PlayerMovementController
|
|||
_body.Orientation = Quaternion.CreateFromAxisAngle(Vector3.UnitZ, Yaw - MathF.PI / 2f);
|
||||
|
||||
// ── 2. Set velocity via MotionInterpreter state machine ───────────────
|
||||
// 2026-05-16 (issue #75): skip when DriveServerAutoWalk owns
|
||||
// motion control this frame — it has already called
|
||||
// _motion.DoMotion + _body.set_local_velocity from the auto-
|
||||
// walk's path data + runRate. Running this section would
|
||||
// overwrite the auto-walk velocity with the user-input
|
||||
// (Ready/Stand) velocity, freezing the body.
|
||||
if (!autoWalkConsumedMotion)
|
||||
{
|
||||
// Determine the dominant forward/backward command and speed.
|
||||
uint forwardCmd;
|
||||
float forwardCmdSpeed;
|
||||
|
|
@ -806,6 +949,7 @@ public sealed class PlayerMovementController
|
|||
|
||||
_body.set_local_velocity(new Vector3(localX, localY, savedWorldVz));
|
||||
}
|
||||
} // end of `if (!autoWalkConsumedMotion)` — section 2
|
||||
|
||||
// ── 3. Jump (charged) ─────────────────────────────────────────────────
|
||||
// Hold spacebar to charge (0→1 over JumpChargeRate seconds).
|
||||
|
|
@ -1243,46 +1387,55 @@ public sealed class PlayerMovementController
|
|||
}
|
||||
|
||||
// ── 8. Heartbeat timer (always while in-world, not just while moving) ─
|
||||
// 2026-05-16 — retail diff-driven AP cadence (acclient_2013_pseudo_c.txt
|
||||
// 0x006b45e0 ShouldSendPositionEvent + 0x006b4770 SendPositionEvent).
|
||||
// 2026-05-16 (closes #74) — retail-faithful AP cadence per
|
||||
// CommandInterpreter::ShouldSendPositionEvent at
|
||||
// acclient_2013_pseudo_c.txt:700233-700285. Two-branch:
|
||||
//
|
||||
// Rules:
|
||||
// - When interval elapsed (>= 1 sec since last send): send.
|
||||
// - When interval NOT elapsed: send only if position or cell
|
||||
// differs from last_sent (Frame::is_equal check at
|
||||
// acclient_2013_pseudo_c.txt:700248-700265).
|
||||
// - SendPositionEvent (acclient_2013_pseudo_c.txt:700327)
|
||||
// gates on `((state & 1) != 0 && (state & 2) != 0)` —
|
||||
// Contact (CONTACT_TS bit 0) AND OnWalkable (ON_WALKABLE_TS
|
||||
// bit 1) BOTH set. Two independent `& != 0` tests joined
|
||||
// by `&&`, NOT a single bitwise-OR mask test. Airborne
|
||||
// (neither bit) and wall-contact-without-walkable (Contact
|
||||
// only) both suppress AP. MoveToState carries jump/fall
|
||||
// snapshots while airborne.
|
||||
// Branch 1 — interval NOT yet elapsed (< 1 sec since last
|
||||
// send): send only if cell changed OR contact-plane changed
|
||||
// (mid-walk events that matter — stair / hill / cell cross).
|
||||
//
|
||||
// Effective rate: per-frame while moving on the ground, 1 Hz at-rest
|
||||
// heartbeat, 0 Hz airborne. Retires the 1 Hz / 10 Hz flat model.
|
||||
// Branch 2 — interval HAS elapsed (>= 1 sec): send only if
|
||||
// cell OR position frame changed. Truly idle = no send
|
||||
// (retail's `last_sent.frame == player.frame` check at
|
||||
// acclient_2013_pseudo_c.txt:700248-700265).
|
||||
//
|
||||
// If NotePositionSent has never been called (no network session),
|
||||
// _lastSentInitialized stays false and we treat every frame as
|
||||
// "first send" — HeartbeatDue fires once per frame, which matches
|
||||
// "send if anything to send" semantics.
|
||||
// SendPositionEvent (line 700327) gates the actual send on
|
||||
// (state & 1) != 0 && (state & 2) != 0 — Contact AND
|
||||
// OnWalkable both set. We mirror that gate so airborne and
|
||||
// wall-contact-without-walkable suppress AP entirely;
|
||||
// MoveToState carries jump/fall snapshots while airborne.
|
||||
//
|
||||
// Effective rates:
|
||||
// Truly idle (grounded, no movement) : 0 Hz
|
||||
// Smooth movement (no cell/plane changes) : ~1 Hz (interval)
|
||||
// Cell crossings + stair/hill steps : per-event
|
||||
// Airborne : 0 Hz
|
||||
//
|
||||
// Bootstrap: when NotePositionSent has never been called
|
||||
// (_lastSentInitialized=false), every state-changed branch is
|
||||
// forced true so the first AP gets a chance to fire.
|
||||
|
||||
bool intervalElapsed = !_lastSentInitialized
|
||||
|| (_simTimeSeconds - _lastSentTime) >= HeartbeatInterval;
|
||||
|
||||
bool positionChanged =
|
||||
!_lastSentInitialized
|
||||
|| _lastSentCellId != CellId
|
||||
bool cellChanged = !_lastSentInitialized
|
||||
|| _lastSentCellId != CellId;
|
||||
bool planeChanged = !_lastSentInitialized
|
||||
|| !ApproxPlaneEqual(_lastSentContactPlane, _body.ContactPlane);
|
||||
bool frameChanged = !_lastSentInitialized
|
||||
|| !ApproxPositionEqual(_lastSentPos, _body.Position);
|
||||
|
||||
// Grounded-on-walkable. Retail's CONTACT_TS + ON_WALKABLE_TS
|
||||
// (acclient.h:3688). Our equivalent: PhysicsBody.InContact &&
|
||||
// PhysicsBody.OnWalkable (both map to TransientStateFlags bits 0+1
|
||||
// which are set together by ResolveWithTransition on walkable ground).
|
||||
bool sendThisFrame = intervalElapsed
|
||||
? (cellChanged || frameChanged)
|
||||
: (cellChanged || planeChanged);
|
||||
|
||||
// Grounded-on-walkable gate per acclient_2013_pseudo_c.txt:700327
|
||||
// (`(state & 1) != 0 && (state & 2) != 0`). Both flags must be
|
||||
// set simultaneously, NOT a bitwise-OR mask test.
|
||||
bool groundedOnWalkable = _body.InContact && _body.OnWalkable;
|
||||
|
||||
HeartbeatDue = groundedOnWalkable && (positionChanged || intervalElapsed);
|
||||
HeartbeatDue = groundedOnWalkable && sendThisFrame;
|
||||
|
||||
// K-fix5 (2026-04-26): local-animation-cycle pacing. Visual rate
|
||||
// should match the actual movement speed. For Forward+Run this is
|
||||
|
|
@ -1296,6 +1449,40 @@ public sealed class PlayerMovementController
|
|||
? (_weenie.InqRunRate(out float vrrAnim) ? vrrAnim : 1f)
|
||||
: 1f;
|
||||
|
||||
// 2026-05-16 (issue #75) — server-initiated auto-walk drives
|
||||
// the local animation cycle directly:
|
||||
// - moving forward → WalkForward / RunForward (legs animate)
|
||||
// - turn-first phase → TurnLeft / TurnRight (issue #69 fix)
|
||||
// - aligned but pre-step / arrival → no override, falls to
|
||||
// the user-input section's default (idle)
|
||||
// UpdatePlayerAnimation reads LocalAnimationCommand +
|
||||
// LocalAnimationSpeed; without these overrides the body
|
||||
// translates/rotates without leg/arm animation. The motion
|
||||
// cycle commands here flow into the animation sequencer
|
||||
// ONLY — the wire-layer guard at GameWindow.cs:6419 prevents
|
||||
// them from leaking to a user-MoveToState packet during
|
||||
// auto-walk.
|
||||
if (_autoWalkMovingForwardThisFrame)
|
||||
{
|
||||
if (_autoWalkInitiallyRunning && _weenie.InqRunRate(out float autoWalkRunRate))
|
||||
{
|
||||
localAnimCmd = MotionCommand.RunForward;
|
||||
localAnimSpeed = autoWalkRunRate;
|
||||
}
|
||||
else
|
||||
{
|
||||
localAnimCmd = MotionCommand.WalkForward;
|
||||
localAnimSpeed = 1f;
|
||||
}
|
||||
}
|
||||
else if (_autoWalkTurnDirectionThisFrame != 0)
|
||||
{
|
||||
localAnimCmd = _autoWalkTurnDirectionThisFrame > 0
|
||||
? MotionCommand.TurnLeft
|
||||
: MotionCommand.TurnRight;
|
||||
localAnimSpeed = 1f;
|
||||
}
|
||||
|
||||
return new MovementResult(
|
||||
Position: Position,
|
||||
RenderPosition: RenderPosition,
|
||||
|
|
@ -1339,4 +1526,24 @@ public sealed class PlayerMovementController
|
|||
&& MathF.Abs(a.Y - b.Y) < Epsilon
|
||||
&& MathF.Abs(a.Z - b.Z) < Epsilon;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 2026-05-16. Contact-plane-equality test for retail's
|
||||
/// sub-interval AP gate. Retail's SendPositionEvent stores
|
||||
/// last_sent_contact_plane and ShouldSendPositionEvent re-sends
|
||||
/// during the sub-interval window if the plane has changed (e.g.,
|
||||
/// player stepped onto stairs / a hill — same cell but different
|
||||
/// contact normal). Tiny epsilon on normal + distance covers
|
||||
/// floating-point noise from the physics integration.
|
||||
/// </summary>
|
||||
private static bool ApproxPlaneEqual(
|
||||
System.Numerics.Plane a, System.Numerics.Plane b)
|
||||
{
|
||||
const float NormalEpsilon = 1e-4f;
|
||||
const float DistanceEpsilon = 0.001f;
|
||||
return MathF.Abs(a.Normal.X - b.Normal.X) < NormalEpsilon
|
||||
&& MathF.Abs(a.Normal.Y - b.Normal.Y) < NormalEpsilon
|
||||
&& MathF.Abs(a.Normal.Z - b.Normal.Z) < NormalEpsilon
|
||||
&& MathF.Abs(a.D - b.D) < DistanceEpsilon;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6407,7 +6407,23 @@ public sealed class GameWindow : IDisposable
|
|||
var wireRot = YawToAcQuaternion(_playerController.Yaw);
|
||||
byte contactByte = result.IsOnGround ? (byte)1 : (byte)0;
|
||||
|
||||
if (result.MotionStateChanged)
|
||||
// 2026-05-16 (issue #75): wire-layer semantic gate —
|
||||
// user-MoveToState packets are ONLY for user-initiated
|
||||
// motion intent. During server-controlled auto-walk
|
||||
// (inbound MoveToObject), motion-state transitions
|
||||
// come from the auto-walk's animation override, not
|
||||
// from user input. Sending a MoveToState in that case
|
||||
// would tell ACE "user took control" and cancel its
|
||||
// own MoveToChain. This is NOT a band-aid like the
|
||||
// earlier grace-period — it's the wire-layer's
|
||||
// expression of retail's architectural split between
|
||||
// user-input motion and server-driven motion: they
|
||||
// share the local motion-state machine but only
|
||||
// user-input flows back to the wire. Without the
|
||||
// refactor (issue #75) this guard masked a synthesis
|
||||
// leak; with the refactor it expresses the proper
|
||||
// semantic.
|
||||
if (result.MotionStateChanged && !_playerController.IsServerAutoWalking)
|
||||
{
|
||||
// HoldKey axis values — retail enum (holtburger types.rs HoldKey):
|
||||
// Invalid = 0, None = 1, Run = 2
|
||||
|
|
@ -6446,11 +6462,13 @@ public sealed class GameWindow : IDisposable
|
|||
// B.6/B.7 (2026-05-16): stamp the diff-driven heartbeat clock so
|
||||
// HeartbeatDue resets its interval from THIS send — mirrors retail's
|
||||
// SendMovementEvent (acclient_2013_pseudo_c.txt:0x006b4680) writing
|
||||
// last_sent_position_time + last_sent_position after each MTS send.
|
||||
// last_sent_position_time + last_sent_position + contact_plane
|
||||
// after each MTS send.
|
||||
_playerController.NotePositionSent(
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
contactPlane: _playerController.ContactPlane,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
}
|
||||
|
||||
if (_playerController.HeartbeatDue)
|
||||
|
|
@ -6472,11 +6490,13 @@ public sealed class GameWindow : IDisposable
|
|||
// B.6/B.7 (2026-05-16): stamp the diff-driven heartbeat clock so
|
||||
// HeartbeatDue resets its interval from THIS send — mirrors retail's
|
||||
// SendPositionEvent (acclient_2013_pseudo_c.txt:700345-700348)
|
||||
// writing last_sent_position_time + last_sent_position after each AP.
|
||||
// writing last_sent_position_time + last_sent_position +
|
||||
// last_sent_contact_plane after each AP.
|
||||
_playerController.NotePositionSent(
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
worldPos: _playerController.Position,
|
||||
cellId: _playerController.CellId,
|
||||
contactPlane: _playerController.ContactPlane,
|
||||
nowSeconds: _playerController.SimTimeSeconds);
|
||||
}
|
||||
|
||||
if (result.JumpExtent.HasValue && result.JumpVelocity.HasValue)
|
||||
|
|
@ -9106,45 +9126,51 @@ public sealed class GameWindow : IDisposable
|
|||
return;
|
||||
}
|
||||
|
||||
// 2026-05-16 — R is conceptually "use." It smart-routes to
|
||||
// pickup as a downstream optimization (see the isPickupableItem
|
||||
// dispatch below), but the GATE is always IsUseableTarget —
|
||||
// what retail's UseObject would do.
|
||||
// 2026-05-16 (Phase B.6 follow-up) — R is the universal "interact"
|
||||
// key. Retail dispatches by TARGET TYPE first; the useability gate
|
||||
// is enforced by each individual action handler (SendUse checks
|
||||
// IsUseableTarget; SendPickUp checks IsPickupableTarget), not as
|
||||
// a top-level block. Previously the IsUseableTarget gate at the
|
||||
// entry point rejected USEABLE_NO=1 items (spell components,
|
||||
// gems) which retail accepts as pickupable — they just aren't
|
||||
// "useable" in the activate-from-world sense.
|
||||
//
|
||||
// Dispatch order:
|
||||
// 1. Creature → SendUse (talk to NPC, attack monster)
|
||||
// 2. Pickupable → SendPickUp (small items, corpses)
|
||||
// 3. Useable → SendUse (doors, portals, lifestones,
|
||||
// potions / scrolls activated from world)
|
||||
// 4. Else → toast "X cannot be used" (signs, banners,
|
||||
// decorative scenery)
|
||||
//
|
||||
// Retail string at acclient_2013_pseudo_c.txt:1033115
|
||||
// (data_7e2a70): "The %s cannot be used".
|
||||
if (!IsUseableTarget(sel))
|
||||
|
||||
bool isCreature = _liveEntityInfoByGuid.TryGetValue(sel, out var info)
|
||||
&& (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0;
|
||||
|
||||
if (isCreature)
|
||||
{
|
||||
string label = DescribeLiveEntity(sel);
|
||||
_debugVm?.AddToast(AcDream.Core.Ui.RetailMessages.CannotBeUsed(label));
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
|
||||
Console.WriteLine($"[B.4b] R-key ignored — not useable guid=0x{sel:X8}");
|
||||
SendUse(sel);
|
||||
return;
|
||||
}
|
||||
|
||||
// B.7 (2026-05-15): the user requested R behave as a universal
|
||||
// interact key — pickup for items, use for NPCs / doors /
|
||||
// lifestones / portals / corpses. Matches retail's "use"
|
||||
// behaviour where the action picked depends on the target's
|
||||
// type rather than forcing the player to remember a different
|
||||
// hotkey per target type.
|
||||
bool isPickupableItem = true;
|
||||
if (_liveEntityInfoByGuid.TryGetValue(sel, out var info)
|
||||
&& (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0)
|
||||
if (IsPickupableTarget(sel))
|
||||
{
|
||||
// NPCs / monsters / players are Use targets, never PickUp.
|
||||
isPickupableItem = false;
|
||||
}
|
||||
if (_lastSpawnByGuid.TryGetValue(sel, out var spawn)
|
||||
&& spawn.ObjectDescriptionFlags is { } odf)
|
||||
{
|
||||
// BF_DOOR | BF_LIFESTONE | BF_PORTAL | BF_CORPSE → Use, not PickUp.
|
||||
// (acclient.h:6431-6463)
|
||||
const uint NonPickupMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u;
|
||||
if ((odf & NonPickupMask) != 0) isPickupableItem = false;
|
||||
SendPickUp(sel);
|
||||
return;
|
||||
}
|
||||
|
||||
if (isPickupableItem) SendPickUp(sel);
|
||||
else SendUse(sel);
|
||||
if (IsUseableTarget(sel))
|
||||
{
|
||||
SendUse(sel);
|
||||
return;
|
||||
}
|
||||
|
||||
string label = DescribeLiveEntity(sel);
|
||||
_debugVm?.AddToast(AcDream.Core.Ui.RetailMessages.CannotBeUsed(label));
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
|
||||
Console.WriteLine($"[B.4b] R-key ignored — neither pickupable nor useable guid=0x{sel:X8}");
|
||||
}
|
||||
|
||||
private void SendUse(uint guid)
|
||||
|
|
@ -9188,22 +9214,17 @@ public sealed class GameWindow : IDisposable
|
|||
return;
|
||||
}
|
||||
|
||||
// Far range: send Use immediately so ACE has the request,
|
||||
// AND queue an arrival re-send. Issue #63 (server-initiated
|
||||
// MoveToObject not honored) means ACE's first-Use response
|
||||
// is dropped as too-far and ACE doesn't re-poll
|
||||
// WithinUseRadius when the speculative local walk gets us in
|
||||
// range. The arrival re-send fires a second Use packet once
|
||||
// the body reaches the target — at which point ACE accepts
|
||||
// and executes the action. The retail-faithful path is to
|
||||
// honor MoveToObject and let ACE complete the Use server-
|
||||
// side; until #63 lands, this client-side retry is the
|
||||
// workaround that keeps far-range Use working.
|
||||
// Far range: send Use ONCE. ACE's CreateMoveToChain
|
||||
// (Player_Use.cs:205) holds a callback (TryUseItem) and fires
|
||||
// it server-side when WithinUseRadius passes during the
|
||||
// MoveToChain poll (Player_Move.cs:150). No client-side retry
|
||||
// needed — the Phase B.6 MoveToState-suppression fix
|
||||
// (GameWindow.cs:6410) keeps ACE's chain alive during the
|
||||
// walk.
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildUse(seq, guid);
|
||||
_liveSession.SendGameAction(body);
|
||||
_pendingPostArrivalAction = (guid, false);
|
||||
Console.WriteLine($"[B.4b] use guid=0x{guid:X8} seq={seq} (queued for arrival re-send pending #63)");
|
||||
Console.WriteLine($"[B.4b] use guid=0x{guid:X8} seq={seq}");
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
|
||||
{
|
||||
string label = DescribeLiveEntity(guid);
|
||||
|
|
@ -9263,16 +9284,16 @@ public sealed class GameWindow : IDisposable
|
|||
return;
|
||||
}
|
||||
|
||||
// Far range: same arrival-retry pattern as SendUse — fire
|
||||
// PickUp immediately AND queue for arrival re-send. ACE's
|
||||
// first PickUp is dropped if we're outside the use-radius
|
||||
// and isn't re-polled (issue #63 workaround).
|
||||
// Far range: send PickUp ONCE. Same auto-fire-via-MoveToChain
|
||||
// callback pattern as SendUse — ACE's chain fires
|
||||
// PutItemInContainer/Move server-side when in range. No
|
||||
// client-side retry; Phase B.6 MoveToState suppression keeps
|
||||
// ACE's chain alive.
|
||||
var seq = _liveSession.NextGameActionSequence();
|
||||
var body = AcDream.Core.Net.Messages.InteractRequests.BuildPickUp(
|
||||
seq, itemGuid, _playerServerGuid, placement: 0);
|
||||
_liveSession.SendGameAction(body);
|
||||
_pendingPostArrivalAction = (itemGuid, true);
|
||||
Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq} (queued for arrival re-send pending #63)");
|
||||
Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq}");
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
|
||||
{
|
||||
string label = DescribeLiveEntity(itemGuid);
|
||||
|
|
@ -9665,56 +9686,75 @@ public sealed class GameWindow : IDisposable
|
|||
/// pickup flow for entities where ACE didn't publish useability.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
/// <summary>
|
||||
/// 2026-05-16 — pickup gate is ItemType-based, NOT useability-based.
|
||||
///
|
||||
/// <para>
|
||||
/// The earlier <c>(useability & USEABLE_REMOTE) != 0u</c> check
|
||||
/// was a misread of the audit. USEABLE_REMOTE (0x20) gates the USE
|
||||
/// action ("can the player activate this item from the world");
|
||||
/// PICKUP is a separate action governed by retail's
|
||||
/// PutItemInContainer handler, which accepts any small-item-class
|
||||
/// entity from the world regardless of useability bits. A spell
|
||||
/// component with useability=USEABLE_NO=1 is still pickupable in
|
||||
/// retail — USEABLE_NO blocks using the component (you can't
|
||||
/// "activate" it standalone), not picking it up.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// Now matches retail: small-item ItemType class OR BF_CORPSE bit
|
||||
/// → pickupable. Server validates the request server-side
|
||||
/// (in-range, target-still-exists, container-has-room).
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private bool IsPickupableTarget(uint guid)
|
||||
{
|
||||
if (_lastSpawnByGuid.TryGetValue(guid, out var spawn))
|
||||
if (!_lastSpawnByGuid.TryGetValue(guid, out var spawn))
|
||||
return false;
|
||||
|
||||
// 2026-05-16 — primary discriminator is the BF_STUCK
|
||||
// ObjectDescriptionFlag (acclient.h:6435, bit 0x4). Retail and
|
||||
// ACE mark immovable world objects (signs, banners, doors,
|
||||
// benches) as Stuck server-side. ACE's PutItemInContainer
|
||||
// handler (Player_Inventory.cs:831-836) responds with
|
||||
// WeenieError.Stuck (0x29) when the client attempts a pickup
|
||||
// on an item with the Stuck flag — so the client should gate
|
||||
// out signs etc. before sending the wire packet.
|
||||
//
|
||||
// Discriminates same-ItemType ambiguity that useability can't:
|
||||
// Holtburg sign (Misc + USEABLE_NO + BF_STUCK) → block
|
||||
// Spell component (Misc + USEABLE_NO + ~BF_STUCK) → allow
|
||||
// Door (no SmallItemMask + BF_DOOR + BF_STUCK) → never matches SmallItemMask, separately
|
||||
if (spawn.ObjectDescriptionFlags is { } odf)
|
||||
{
|
||||
if (spawn.Useability is uint useability)
|
||||
{
|
||||
const uint USEABLE_REMOTE = 0x20u;
|
||||
return (useability & USEABLE_REMOTE) != 0u;
|
||||
}
|
||||
|
||||
// Useability null: corpses are pickupable; signs aren't.
|
||||
if (spawn.ObjectDescriptionFlags is { } odf)
|
||||
{
|
||||
const uint BF_CORPSE = 0x2000u;
|
||||
if ((odf & BF_CORPSE) != 0u)
|
||||
{
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeUseabilityFallbackEnabled)
|
||||
Console.WriteLine(System.FormattableString.Invariant(
|
||||
$"[useability-fallback] pickup-corpse guid=0x{guid:X8} (ACE sent no useability bit)"));
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Small-item ItemType fallback (covers F on dropped items
|
||||
// when ACE doesn't publish useability for the weenie).
|
||||
uint it = spawn.ItemType ?? 0u;
|
||||
const uint SmallItemMask =
|
||||
(uint)(AcDream.Core.Items.ItemType.MeleeWeapon
|
||||
| AcDream.Core.Items.ItemType.Armor
|
||||
| AcDream.Core.Items.ItemType.Clothing
|
||||
| AcDream.Core.Items.ItemType.Jewelry
|
||||
| AcDream.Core.Items.ItemType.Food
|
||||
| AcDream.Core.Items.ItemType.Money
|
||||
| AcDream.Core.Items.ItemType.Misc
|
||||
| AcDream.Core.Items.ItemType.MissileWeapon
|
||||
| AcDream.Core.Items.ItemType.Container
|
||||
| AcDream.Core.Items.ItemType.Gem
|
||||
| AcDream.Core.Items.ItemType.SpellComponents
|
||||
| AcDream.Core.Items.ItemType.Writable
|
||||
| AcDream.Core.Items.ItemType.Key
|
||||
| AcDream.Core.Items.ItemType.Caster);
|
||||
if ((it & SmallItemMask) != 0u)
|
||||
{
|
||||
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeUseabilityFallbackEnabled)
|
||||
Console.WriteLine(System.FormattableString.Invariant(
|
||||
$"[useability-fallback] pickup-smallitem guid=0x{guid:X8} itemType=0x{it:X8} (ACE sent no useability bit)"));
|
||||
return true;
|
||||
}
|
||||
const uint BF_STUCK = 0x0004u;
|
||||
const uint BF_CORPSE = 0x2000u;
|
||||
// Corpses are pickupable (loot) — BF_CORPSE wins over
|
||||
// any BF_STUCK that might be coincidentally set.
|
||||
if ((odf & BF_CORPSE) != 0u) return true;
|
||||
// Anything else with BF_STUCK is immovable scenery.
|
||||
if ((odf & BF_STUCK) != 0u) return false;
|
||||
}
|
||||
return false;
|
||||
|
||||
// Small-item ItemType class: dropped weapons, armor, food,
|
||||
// jewelry, money, misc, gems, spell components, etc.
|
||||
uint it = spawn.ItemType ?? 0u;
|
||||
const uint SmallItemMask =
|
||||
(uint)(AcDream.Core.Items.ItemType.MeleeWeapon
|
||||
| AcDream.Core.Items.ItemType.Armor
|
||||
| AcDream.Core.Items.ItemType.Clothing
|
||||
| AcDream.Core.Items.ItemType.Jewelry
|
||||
| AcDream.Core.Items.ItemType.Food
|
||||
| AcDream.Core.Items.ItemType.Money
|
||||
| AcDream.Core.Items.ItemType.Misc
|
||||
| AcDream.Core.Items.ItemType.MissileWeapon
|
||||
| AcDream.Core.Items.ItemType.Container
|
||||
| AcDream.Core.Items.ItemType.Gem
|
||||
| AcDream.Core.Items.ItemType.SpellComponents
|
||||
| AcDream.Core.Items.ItemType.Writable
|
||||
| AcDream.Core.Items.ItemType.Key
|
||||
| AcDream.Core.Items.ItemType.Caster);
|
||||
return (it & SmallItemMask) != 0u;
|
||||
}
|
||||
|
||||
private string DescribeLiveEntity(uint guid)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue