acdream/docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md
Erik b5da17db76 feat(retail): Commit B — retail-faithful AP cadence + screen-rect picker
Retires divergences flagged in the 2026-05-16 faithfulness audit:

1. AP cadence. Replaces the 1 Hz idle / 10 Hz active flat heartbeat
   with a diff-driven model gated on `Contact && OnWalkable`
   (acclient_2013_pseudo_c.txt:700327 SendPositionEvent). Sends on
   position or cell change while grounded on walkable, plus a 1 sec
   heartbeat; suppressed entirely airborne. PlayerMovementController
   exposes `NotePositionSent(pos, cellId, now)` which GameWindow stamps
   after each AutonomousPosition / MoveToState send — mirrors retail's
   shared `last_sent_position_time` between SendPositionEvent
   (0x006b4770) and SendMovementEvent (0x006b4680). Known divergence
   from retail: ours is per-frame-while-moving, retail's effective rate
   is ~1 Hz during smooth motion (cell/plane checks). Filed as #74,
   blocked by #63 — when #63 lands we revert to retail's narrower gate.

2. Workaround retirement. Removes TinyMargin (0.05 m inside arrival)
   and the AP-flush before re-send (`SendAutonomousPositionNow`). The
   diff-driven cadence makes both obsolete. Close-range turn-first
   deferred Use is kept (it IS retail — ACE Player_Move.cs:66-87
   mirrors retail's CreateMoveToChain pre-callback rotation), renamed
   `OnAutoWalkArrivedSendDeferredAction` to clarify it's a FIRST send.
   `isRetryAfterArrival` parameter dropped.

3. Far-range Use/PickUp retry. Restored — was load-bearing, not the
   "redundant cleanup" the Group 2 audit thought. Issue #63 means ACE
   drops the first Use as too-far without re-polling on subsequent APs;
   the arrival re-send is what makes far-range Use complete. Logs
   include `(queued for arrival re-send pending #63)` to make this
   explicit. Removes when #63 closes.

4. Screen-rect picker. New `AcDream.Core.Selection.ScreenProjection`
   helper shared by `WorldPicker` and `TargetIndicatorPanel`. The
   `Setup.SelectionSphere` projects to a screen-space square (retail
   anchor `SmartBox::GetObjectBoundingBox` 0x00452e20); picker
   hit-tests the mouse pixel against the same rect the indicator draws,
   inflated by 8 px (`TriangleSize`). Guarantees what-you-see is
   what-you-click — including rect corners that were dead zones under
   the old ray-sphere picker. Per-type radius (1.0/1.6/2.0 m) and
   vertical-offset (0.2/0.9/1.0/1.5 m) heuristic lambdas retired;
   `IsTallSceneryGuid` deleted; `EntityHeightFor` trimmed to 1.5 m × scale
   defensive default. No defensive sphere synth — entities without a
   baked `SelectionSphere` are skipped, matching retail's
   `GfxObjUnderSelectionRay` (0x0054c740).

5. Rotation rate run multiplier (Commit A precursor). `TurnRateFor(running)`
   helper applies retail's `run_turn_factor = 1.5f` (PDB-named
   0x007c8914) under HoldKey.Run, matching `apply_run_to_command` at
   0x00527be0 (line 305098). Effective: walking ≈ 90°/s, running ≈ 135°/s.
   Keyboard A/D + ApplyAutoWalkOverlay both use it.

6. Useability gate (Commit A precursor). `IsUseableTarget` corrected to
   `useability != 0` per `ItemUses::IsUseable` at 256455 — ANY non-zero
   passes (USEABLE_NO=1, USEABLE_CONTAINED=8, etc.), not just the
   USEABLE_REMOTE bit. Cross-checked against 4 call sites in retail
   (ItemHolder::UseObject 0x00588a80, DetermineUseResult 0x402697,
   UsingItem 0x367638, disable-button-state 0x198826). Added
   `ProbeUseabilityFallbackEnabled` diagnostic
   (`ACDREAM_PROBE_USEABILITY_FALLBACK=1`) to measure how often the
   creature/BF_DOOR fallback fires for ACE-seed-DB entities with
   null useability.

CLAUDE.md updated with the graceful-shutdown rule for relaunch:
Stop-Process bypasses the logout packet, leaving ACE's session marked
logged-in for ~3+ min. CloseMainWindow() sends WM_CLOSE so the
shutdown hook runs and the logout packet reaches ACE.

Tests: +3 ScreenProjectionTests + 6 WorldPickerRectOverloadTests = +9.
Core.Net 294/294 pass; Core 1073/1081 (8 pre-existing Physics failures
unchanged). Visual-verified 2026-05-16: rotation rate, useability,
screen-rect click area, double-click + R-key + F-key Use/PickUp at
short and long range — dialogue/door/pickup fire on arrival.

Filed follow-ups #70 (triangle apex/size DAT sprite), #71 (picker
Stage B polygon refine), #72 (cdb omega.z probe), #73 (retail-message
sweep pattern), #74 (per-frame AP chattier than retail — blocked by
#63). Old ray-sphere `WorldPicker.Pick(origin, direction, ...)`
overload kept for back-compat; no callers in acdream proper.

Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md

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

1939 lines
81 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Retail-faithfulness fixes (Commit A + Commit B) 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:** Retire the retail divergences flagged in the 2026-05-16 faithfulness audit. Bring rotation rate, useability gate, AP cadence, picker geometry, and the four B.6 Tier-4 workarounds in line with the named-retail decomp.
**Architecture:** Two coherent commits.
- **Commit A** (low-risk, no protocol change): rotation rate constants, useability gate semantics, and a diagnostic probe for the useability-fallback path.
- **Commit B** (coupled rework): retail-faithful AutonomousPosition cadence (diff-driven, grounded-gated) → which retires four workarounds → plus sphere-based picker using `Setup.SelectionSphere` → plus deletion of two heuristics that become dead code.
**Tech Stack:** C# .NET 10, xUnit tests, ImGui.NET (selection indicator path). All edits land on `main` (the user runs the client from there); investigation evidence is in `docs/research/named-retail/acclient_2013_pseudo_c.txt` + `acclient.h`.
**Retail anchors** (cited throughout):
- `0x007c8914` `run_turn_factor = 1.5f` — rotation rate multiplier under HoldKey.Run.
- `0x4fccc0` `ItemUses::IsUseable` — gate semantics is `_useability != USEABLE_UNDEF (0)`.
- `0x006b3efb` `time_between_position_events = 1.0s` — at-rest AP heartbeat.
- `0x006b45e0` `ShouldSendPositionEvent` — diff-driven + interval cadence.
- `0x006b4770` `SendPositionEvent``transient_state & (CONTACT_TS | ON_WALKABLE_TS)` gate.
- `0x00588a80` `ItemHolder::UseObject` — single fire-and-forget `Event_UseEvent`.
- `0x00564900` `Handle_Item__UseDone` — server signals "use done" inbound.
- `0x0054c740` `Render::GfxObjUnderSelectionRay` — retail picker uses per-part `drawing_sphere` + polygon refine. We approximate with a screen-space rect hit-test that uses the exact rect the target indicator already draws (guarantees click area = visible bracket area, zero corner dead zones).
- `0x00518b80` `CPartArray::GetSelectionSphere` — scale formula.
- `0x00452e20` `SmartBox::GetObjectBoundingBox` — projects `CSetup.SelectionSphere` to a screen-aligned rect. The indicator AND the new picker call into the same projection helper.
---
## File structure
**Files touched in Commit A:**
| File | Responsibility |
|---|---|
| `src/AcDream.Core/Physics/RemoteMoveToDriver.cs` | Add `BaseTurnRateRadPerSec`, `RunTurnFactor`, `TurnRateFor(running)` helper. Keep `TurnRateRadPerSec` as the walking-rate constant (back-compat). |
| `src/AcDream.App/Input/PlayerMovementController.cs` | Wire `TurnRateFor(input.Run)` into keyboard A/D path (line ~640-643) and `TurnRateFor(_autoWalkInitiallyRunning)` into `ApplyAutoWalkOverlay` (line ~505). |
| `src/AcDream.Core/Physics/PhysicsDiagnostics.cs` | Add `ProbeUseabilityFallbackEnabled` flag + `ACDREAM_PROBE_USEABILITY_FALLBACK` env var. |
| `src/AcDream.App/Rendering/GameWindow.cs` | Replace useability gate `& USEABLE_REMOTE_BIT` with `!= 0`; add diagnostic line in fallback branches. |
| `tests/AcDream.Core.Tests/Physics/RemoteMoveToDriverTests.cs` | New file — unit tests for `TurnRateFor`. |
**Files touched in Commit B:**
| File | Responsibility |
|---|---|
| `src/AcDream.App/Input/PlayerMovementController.cs` | Replace `_heartbeatAccum` with diff-driven `HeartbeatDue` (position/cell change + 1s heartbeat + `IsGrounded` gate). Add `NotePositionSent(pos, cellId, now)` + `SimTimeSeconds` accessor. Delete `TinyMargin` from `ApplyAutoWalkOverlay`. Add `ApproxPositionEqual` helper. |
| `src/AcDream.App/Rendering/GameWindow.cs` | Delete `OnAutoWalkArrivedReSendAction`, `SendAutonomousPositionNow`, `IsTallSceneryGuid`, the `isRetryAfterArrival` parameter on `SendUse`/`SendPickUp`. Simplify `_pendingPostArrivalAction` to close-range-deferred-Use only (no retry). Add `OnAutoWalkArrivedSendDeferredAction` (FIRST send, not retry). Call `_playerController.NotePositionSent(...)` after each `SendMoveToState` / `SendAutonomousPosition`. Wire `WorldPicker.Pick` to the new screen-rect overload using `TryGetEntitySelectionSphere` (already exists at line ~9605); drop per-type `radiusForGuid` / `verticalOffsetForGuid` callbacks. |
| `src/AcDream.Core/Selection/ScreenProjection.cs` (new) | Shared math: `TryProjectSphereToScreenRect(worldCenter, worldRadius, view, projection, viewport, out rectMin, out rectMax, out depth, minSidePixels)`. Factored out of `TargetIndicatorPanel.TryComputeScreenRectFromSphere` so the picker AND the indicator project identically. |
| `src/AcDream.Core/Selection/WorldPicker.cs` | Add new `Pick(mouseX, mouseY, view, projection, viewport, candidates, skipGuid, sphereForEntity, inflatePixels=8f)` screen-rect-hit-test overload. Keep existing ray-sphere overload for now (no callers after B8). |
| `src/AcDream.App/UI/TargetIndicatorPanel.cs` | Trim `EntityHeightFor` per-type branches to a single 1.5 m × scale defensive default. Replace private `TryComputeScreenRectFromSphere` with a call into `ScreenProjection.TryProjectSphereToScreenRect`. |
| `tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs` (new or modify existing) | Unit tests for the new rect-hit-test overload + tests for `ScreenProjection.TryProjectSphereToScreenRect`. |
| `docs/ISSUES.md` | File 3 deferred follow-ups (Triangle apex/size UX; Stage B polygon refine; cdb-probe to verify `omega.z = π/2`). |
---
## Commit A — Retail-faithful useability + rotation rate
### Task A1: Add `TurnRateFor(running)` helper to `RemoteMoveToDriver`
**Files:**
- Modify: `src/AcDream.Core/Physics/RemoteMoveToDriver.cs:77` (around `TurnRateRadPerSec` constant)
- Test: `tests/AcDream.Core.Tests/Physics/RemoteMoveToDriverTests.cs` (new file)
- [ ] **Step 1: Write the failing test**
Create `tests/AcDream.Core.Tests/Physics/RemoteMoveToDriverTests.cs`:
```csharp
using AcDream.Core.Physics;
namespace AcDream.Core.Tests.Physics;
public sealed class RemoteMoveToDriverTests
{
[Fact]
public void TurnRateFor_WalkingReturnsBaseRate()
{
// Retail: omega.z = ±π/2 × turn_speed (1.0) = π/2 rad/s ≈ 90°/s
// Anchor: docs/research/named-retail/acclient_2013_pseudo_c.txt
// CMotionInterp::apply_run_to_command 0x00527be0 only
// multiplies under HoldKey.Run — walking is unscaled.
float rate = RemoteMoveToDriver.TurnRateFor(running: false);
Assert.Equal(MathF.PI / 2.0f, rate, precision: 5);
}
[Fact]
public void TurnRateFor_RunningAppliesRunTurnFactor()
{
// Retail: omega.z = ±π/2 × turn_speed × run_turn_factor
// run_turn_factor = 1.5f at 0x007c8914 (PDB-named).
// apply_run_to_command (acclient_2013_pseudo_c.txt:305098)
// multiplies turn_speed by 1.5f when input is TurnRight
// under HoldKey.Run.
float rate = RemoteMoveToDriver.TurnRateFor(running: true);
Assert.Equal(MathF.PI / 2.0f * 1.5f, rate, precision: 5);
}
[Fact]
public void TurnRateRadPerSec_BackCompatStillResolvesToWalkingRate()
{
// Existing call sites that haven't yet migrated to TurnRateFor
// (e.g., RemoteMoveToDriver.Drive's TurnSpeed=1.0 callers) still
// see the walking-rate constant. Same numerical value as
// BaseTurnRateRadPerSec.
Assert.Equal(RemoteMoveToDriver.BaseTurnRateRadPerSec,
RemoteMoveToDriver.TurnRateRadPerSec, precision: 5);
}
}
```
- [ ] **Step 2: Run test to verify it fails**
Run:
```
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~RemoteMoveToDriverTests" --no-build
```
Expected: build fails — `RemoteMoveToDriver.TurnRateFor` / `BaseTurnRateRadPerSec` don't exist yet.
- [ ] **Step 3: Add the constants + helper to `RemoteMoveToDriver.cs`**
In `src/AcDream.Core/Physics/RemoteMoveToDriver.cs`, immediately after the existing `TurnRateRadPerSec` constant declaration (around line 77), add:
```csharp
/// <summary>
/// Retail base turn rate for the player Humanoid when turn_speed
/// scalar = 1.0. Convention default <c>omega.z = ±π/2 rad/s</c>
/// derived from <c>add_motion</c> at <c>0x005224b0</c> + the
/// HasOmega-cleared MotionData fallback documented in
/// <c>AnimationSequencer.cs:734-741</c>. ~90°/s.
/// </summary>
public const float BaseTurnRateRadPerSec = MathF.PI / 2.0f;
/// <summary>
/// Retail's <c>run_turn_factor</c> constant at <c>0x007c8914</c>
/// (PDB-named). <see cref="CMotionInterp.ApplyRunToCommand"/>
/// equivalent (decomp <c>0x00527be0</c>, line 305098 of
/// <c>acclient_2013_pseudo_c.txt</c>) multiplies <c>turn_speed</c>
/// by 1.5 when <c>HoldKey.Run</c> is active on a TurnRight/TurnLeft
/// command. Effect: running rotation is 50 % faster than walking.
/// </summary>
public const float RunTurnFactor = 1.5f;
/// <summary>
/// Retail-faithful local-player turn rate.
/// <list type="bullet">
/// <item><b>Walking</b>: <c>BaseTurnRateRadPerSec</c> ≈ 90°/s.</item>
/// <item><b>Running</b>: <c>BaseTurnRateRadPerSec × RunTurnFactor</c>
/// ≈ 135°/s.</item>
/// </list>
/// Replaces the fixed <c>TurnRateRadPerSec</c> for paths that have
/// access to the player's run/walk state (keyboard A/D, auto-walk
/// overlay turn-first). NPC/monster remotes that lack the
/// information continue to use the constant which equals
/// <c>BaseTurnRateRadPerSec</c>.
/// </summary>
public static float TurnRateFor(bool running)
=> running ? BaseTurnRateRadPerSec * RunTurnFactor
: BaseTurnRateRadPerSec;
```
Keep the existing `TurnRateRadPerSec = MathF.PI / 2.0f;` declaration as-is — it now numerically equals `BaseTurnRateRadPerSec` and acts as the back-compat alias for callers that don't yet know walk-vs-run.
- [ ] **Step 4: Run test to verify it passes**
Run:
```
dotnet build src/AcDream.Core/AcDream.Core.csproj -c Debug
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~RemoteMoveToDriverTests" --no-build
```
Expected: 3/3 pass.
- [ ] **Step 5: Defer commit to end of Commit A (Task A6).**
---
### Task A2: Wire keyboard A/D to `TurnRateFor`
**Files:**
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs:640-643`
- [ ] **Step 1: Replace the `WalkAnimSpeed × 0.5f` formula with `TurnRateFor`**
In `src/AcDream.App/Input/PlayerMovementController.cs` at line 640-643, replace:
```csharp
if (input.TurnRight)
Yaw -= MotionInterpreter.WalkAnimSpeed * 0.5f * dt; // ~90°/s
if (input.TurnLeft)
Yaw += MotionInterpreter.WalkAnimSpeed * 0.5f * dt;
```
with:
```csharp
// 2026-05-16 — retail-faithful turn rate.
// Anchor: docs/research/named-retail/acclient_2013_pseudo_c.txt
// - CMotionInterp::apply_run_to_command 0x00527be0
// multiplies turn_speed by run_turn_factor (1.5) under
// HoldKey.Run on TurnRight/TurnLeft commands.
// - Base rate ±π/2 rad/s comes from add_motion 0x005224b0
// with HasOmega-cleared MotionData fallback.
// Effective: walking ≈ 90°/s, running ≈ 135°/s.
// Previously: WalkAnimSpeed*0.5 ≈ 89.4°/s — coincidentally
// close to retail walking but no run differentiation.
float keyboardTurnRate = RemoteMoveToDriver.TurnRateFor(input.Run);
if (input.TurnRight)
Yaw -= keyboardTurnRate * dt;
if (input.TurnLeft)
Yaw += keyboardTurnRate * dt;
```
- [ ] **Step 2: Build to confirm no breakage**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: build succeeds with 0 errors.
- [ ] **Step 3: No commit yet — continue to A3.**
---
### Task A3: Wire `ApplyAutoWalkOverlay` to `TurnRateFor`
**Files:**
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs:505`
- [ ] **Step 1: Replace the constant with the helper**
In `ApplyAutoWalkOverlay`, locate the line:
```csharp
float maxStep = RemoteMoveToDriver.TurnRateRadPerSec * dt;
```
Replace with:
```csharp
// 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
// run_turn_factor at retail 0x007c8914.
float maxStep = RemoteMoveToDriver.TurnRateFor(_autoWalkInitiallyRunning) * dt;
```
- [ ] **Step 2: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task A4: Add `ProbeUseabilityFallbackEnabled` diagnostic flag
**Files:**
- Modify: `src/AcDream.Core/Physics/PhysicsDiagnostics.cs`
- [ ] **Step 1: Add the flag following the existing pattern**
In `src/AcDream.Core/Physics/PhysicsDiagnostics.cs`, after the existing `ProbeAutoWalkEnabled` flag (around line 121), add:
```csharp
/// <summary>
/// 2026-05-16. Logs one line per `IsUseableTarget` call that takes
/// the null-useability fallback path (creature pass / BF_DOOR pass /
/// BF_LIFESTONE pass / etc.). Used to measure how often ACE's seed
/// DB ships entities without `_useability` set — settles whether
/// the fallback is live code or theoretical defense.
///
/// <para>
/// Retail has NO fallback; null/zero useability blocks Use entirely
/// (acclient_2013_pseudo_c.txt:402923 ItemHolder::UseObject —
/// IsUseable==0 falls through to "cannot be used" branch). Our
/// fallback exists because ACE genuinely sends null for many seed
/// weenies. The probe quantifies "many".
/// </para>
///
/// <para>Toggle via env var <c>ACDREAM_PROBE_USEABILITY_FALLBACK=1</c>
/// or DebugPanel checkbox.</para>
/// </summary>
public static bool ProbeUseabilityFallbackEnabled { get; set; } =
Environment.GetEnvironmentVariable("ACDREAM_PROBE_USEABILITY_FALLBACK") == "1";
```
- [ ] **Step 2: Build**
Run:
```
dotnet build src/AcDream.Core/AcDream.Core.csproj -c Debug
```
Expected: 0 errors.
---
### Task A5: Fix useability gate to `!= 0` + add fallback diagnostic
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — function `IsUseableTarget` (search for `private bool IsUseableTarget`).
- [ ] **Step 1: Replace the primary gate**
Locate `IsUseableTarget` in `src/AcDream.App/Rendering/GameWindow.cs`. The body currently begins with:
```csharp
if (_lastSpawnByGuid.TryGetValue(guid, out var spawn))
{
// Authoritative path: server published Useability.
if (spawn.Useability is uint useability)
{
// USEABLE_REMOTE (0x20) — bit set in every from-world
// useable variant per acclient.h:6478 ITEM_USEABLE enum.
const uint USEABLE_REMOTE_BIT = 0x20u;
return (useability & USEABLE_REMOTE_BIT) != 0;
}
// ... fallback (ObjectDescriptionFlags + creature) ...
}
```
Replace the `if (spawn.Useability is uint useability)` block (the inner content, NOT the whole method) with:
```csharp
// Authoritative path: server published Useability.
// 2026-05-16 — retail-faithful gate per ItemUses::IsUseable
// at acclient_2013_pseudo_c.txt:256455 (4 call-site cross-
// checks confirm: ItemHolder::UseObject 0x00588a80,
// DetermineUseResult 0x402697, UsingItem 0x367638,
// disable-button state 0x198826 — all key off non-zero).
// BN's `!(x) & 1` rendering is a mis-decompile of the
// setne+and test-flag inliner. Real semantic:
//
// IsUseable(_useability) := (_useability != USEABLE_UNDEF)
//
// ANY non-zero value passes (including USEABLE_NO=1,
// USEABLE_CONTAINED=8, etc.). Retail trusts the server to
// have only set non-zero on entities where Use is sensible.
//
// Previous implementation (B.8) checked
// `(useability & USEABLE_REMOTE_BIT) != 0` which is STRICTER
// than retail — a USEABLE_NO door would be blocked locally
// but pass retail's gate. Now matches retail bit-for-bit.
if (spawn.Useability is uint useability)
{
return useability != 0u;
}
```
- [ ] **Step 2: Add diagnostic to the fallback branches**
In the same `IsUseableTarget` method, locate the fallback branches that read (paraphrased):
```csharp
if (spawn.ObjectDescriptionFlags is { } odf)
{
const uint UseableFlatMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u;
if ((odf & UseableFlatMask) != 0) return true;
}
}
// Creatures (NPCs / players) are always Use targets ...
if (_liveEntityInfoByGuid.TryGetValue(guid, out var info)
&& (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0)
{
return true;
}
return false;
```
Modify to:
```csharp
if (spawn.ObjectDescriptionFlags is { } odf)
{
const uint UseableFlatMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u;
if ((odf & UseableFlatMask) != 0)
{
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeUseabilityFallbackEnabled)
Console.WriteLine(System.FormattableString.Invariant(
$"[useability-fallback] flat-class guid=0x{guid:X8} odf=0x{odf:X8} (ACE sent no useability bit)"));
return true;
}
}
}
// Creatures (NPCs / players) are always Use targets in our
// fallback even when ACE didn't publish useability. Retail
// would have blocked here (null → USEABLE_UNDEF → 0 → block),
// but ACE's seed DB has many talk-only NPC weenies with
// `ItemUseable = null`; without the fallback the M1 "click NPC"
// flow regresses. The diagnostic line below lets us measure
// how often this branch fires in real play.
if (_liveEntityInfoByGuid.TryGetValue(guid, out var info)
&& (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0)
{
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeUseabilityFallbackEnabled)
Console.WriteLine(System.FormattableString.Invariant(
$"[useability-fallback] creature guid=0x{guid:X8} (ACE sent no useability bit)"));
return true;
}
return false;
```
- [ ] **Step 3: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task A6: Run full test suite, visual verify, commit Commit A
- [ ] **Step 1: Run full Core.Net test suite**
Run:
```
dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj -c Debug
```
Expected: PASS, count >= 290 (previous baseline).
- [ ] **Step 2: Run full Core test suite (RemoteMoveToDriver tests included)**
Run:
```
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj -c Debug
```
Expected: PASS for `RemoteMoveToDriverTests` (3/3); pre-existing failure baseline unchanged.
- [ ] **Step 3: Visual verify**
The user runs the client and confirms:
- Pressing W + Shift (run) + A/D: character rotates noticeably faster than W (walk) + A/D. Estimated 50% faster (135°/s vs 90°/s).
- Auto-walk to a far target: character turns at 135°/s during the turn-first phase.
- Pressing R on a sign: still silent no-op (Useability=0 still blocks; `0 != 0` is false).
- Pressing R on an NPC: still walks + dialogue fires (creature fallback covers null useability).
- With `ACDREAM_PROBE_USEABILITY_FALLBACK=1`: `[useability-fallback]` lines appear in console for each NPC/door interaction that takes the fallback path.
**STOP and wait for user confirmation before committing.**
- [ ] **Step 4: Commit A**
When user approves, in `C:\Users\erikn\source\repos\acdream` (main checkout):
```bash
git add src/AcDream.Core/Physics/RemoteMoveToDriver.cs \
src/AcDream.App/Input/PlayerMovementController.cs \
src/AcDream.Core/Physics/PhysicsDiagnostics.cs \
src/AcDream.App/Rendering/GameWindow.cs \
tests/AcDream.Core.Tests/Physics/RemoteMoveToDriverTests.cs
git commit -m "$(cat <<'EOF'
fix(retail): rotation rate run multiplier + useability gate semantics
Two retail divergences from the 2026-05-16 faithfulness audit:
1. Rotation rate ignored HoldKey.Run. Retail's CMotionInterp::
apply_run_to_command (decomp 0x00527be0 line 305098) multiplies
turn_speed by run_turn_factor (1.5, PDB-named symbol at 0x007c8914)
when input is TurnRight/TurnLeft under HoldKey.Run. Effective
running rotation is 50% faster (~135°/s vs walking ~90°/s).
Our keyboard A/D and ApplyAutoWalkOverlay used a fixed rate.
New: RemoteMoveToDriver.TurnRateFor(running) helper. Keyboard
path passes input.Run; auto-walk overlay passes
_autoWalkInitiallyRunning. The walking-rate base
(BaseTurnRateRadPerSec = π/2) is unchanged; TurnRateRadPerSec
constant is preserved as the walking-rate alias for callers
that don't have run/walk state (NPC remotes).
2. IsUseableTarget gated on `useability & USEABLE_REMOTE (0x20)`,
stricter than retail. Per ItemUses::IsUseable
(acclient_2013_pseudo_c.txt:256455) cross-referenced with 4
call sites (ItemHolder::UseObject, DetermineUseResult,
UsingItem, disable-button state), retail's real gate is
`_useability != USEABLE_UNDEF (0)` — ANY non-zero passes.
The Binary Ninja `!(x) & 1` pseudo-C is a mis-decompile of a
setne+and test-flag inliner. Now matches retail.
3. Added ProbeUseabilityFallbackEnabled diagnostic
(env var ACDREAM_PROBE_USEABILITY_FALLBACK=1) to log every
time the null-useability fallback fires. Settles whether the
fallback (allow creature + BF_DOOR/LIFESTONE/PORTAL/CORPSE
when ACE didn't publish useability) is live code or
theoretical defense.
Tests: +3 RemoteMoveToDriverTests cover TurnRateFor walking/running/
back-compat. Existing 290+ Core.Net tests unchanged.
Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EOF
)"
```
---
## Commit B — Retire B.6 workarounds via retail AP cadence + sphere-based picker
### Task B1: Add diff-driven position state + `NotePositionSent` to `PlayerMovementController`
**Files:**
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs` — state block at line 189-191, public API.
- [ ] **Step 1: Replace the `_heartbeatAccum` state with diff-tracking state**
In `src/AcDream.App/Input/PlayerMovementController.cs`, the existing state declaration is:
```csharp
private float _heartbeatAccum;
public const float HeartbeatInterval = 1.0f; // 1 sec — retail / holtburger
public bool HeartbeatDue { get; private set; }
```
Replace with:
```csharp
/// <summary>
/// 2026-05-16 — retail-faithful AP cadence. Matches retail's
/// CommandInterpreter::ShouldSendPositionEvent (acclient_2013_pseudo_c.txt
/// at address 0x006b45e0) which gates on either (a) position-or-cell
/// change since the last send, or (b) at-rest 1 sec heartbeat elapsed.
/// `time_between_position_events` constant at 0x006b3efb = 1.0 sec.
///
/// Old model: a 1 Hz idle / 10 Hz active flat accumulator. That
/// missed retail's per-frame-while-moving behaviour and forced the
/// four B.6 workarounds (arrival margin, re-send on arrival, AP
/// flush, retry flag) to compensate for the lag in ACE's server-side
/// WithinUseRadius poll. Replaced by diff-driven cadence below.
/// </summary>
public const float HeartbeatInterval = 1.0f; // retail 0x006b3efb
private System.Numerics.Vector3 _lastSentPos;
private uint _lastSentCellId;
private float _lastSentTime;
private bool _lastSentInitialized;
private float _simTimeSeconds;
public bool HeartbeatDue { get; private set; }
/// <summary>Sim-time accumulator (advanced by dt at the top of Update).
/// Exposed for the network outbound layer to stamp NotePositionSent.</summary>
public float SimTimeSeconds => _simTimeSeconds;
```
- [ ] **Step 2: Add the `NotePositionSent` API**
In the same file, after `EndServerAutoWalk` (~line 410 — find a logical place near the public movement API), add:
```csharp
/// <summary>
/// 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).
/// </summary>
public void NotePositionSent(System.Numerics.Vector3 worldPos,
uint cellId,
float nowSeconds)
{
_lastSentPos = worldPos;
_lastSentCellId = cellId;
_lastSentTime = nowSeconds;
_lastSentInitialized = true;
}
```
- [ ] **Step 3: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors. The `_heartbeatAccum` field declaration is gone; existing references to `HeartbeatDue` still compile (Task B2 will populate it correctly).
---
### Task B2: Replace `_heartbeatAccum` update with retail diff-driven cadence
**Files:**
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs:1188-1203` (the per-frame heartbeat block).
- [ ] **Step 1: Add `_simTimeSeconds` increment at top of `Update`**
Locate the start of `Update(float dt, ...)` and add immediately after entry:
```csharp
_simTimeSeconds += dt;
```
- [ ] **Step 2: Replace the accumulator update**
Locate the existing code around line 1188-1203 that reads (paraphrased):
```csharp
_heartbeatAccum += dt;
bool activelyMoving = _autoWalkActive
|| input.Forward || input.Backward
|| input.StrafeLeft || input.StrafeRight
|| input.TurnLeft || input.TurnRight;
float effectiveInterval = activelyMoving ? 0.1f : HeartbeatInterval;
HeartbeatDue = _heartbeatAccum >= effectiveInterval;
if (HeartbeatDue) _heartbeatAccum = 0f;
```
Replace with:
```csharp
// 2026-05-16 — retail diff-driven AP cadence (acclient_2013_pseudo_c.txt
// 0x006b45e0 ShouldSendPositionEvent + 0x006b4770 SendPositionEvent).
//
// 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.
//
// 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.
//
// 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.
bool intervalElapsed = !_lastSentInitialized
|| (_simTimeSeconds - _lastSentTime) >= HeartbeatInterval;
bool positionChanged =
!_lastSentInitialized
|| _lastSentCellId != CellId
|| !ApproxPositionEqual(_lastSentPos, _body.Position);
// Grounded-on-walkable. Retail's `Contact AND OnWalkable`
// (acclient_2013_pseudo_c.txt:700327). PhysicsBody exposes the
// two transient-state bits as InContact + OnWalkable; combine
// with `&&` to match retail's two-`& != 0`-tests-joined-by-`&&`
// pattern (NOT a single bitwise-OR mask test).
bool groundedOnWalkable = _body.InContact && _body.OnWalkable;
HeartbeatDue = groundedOnWalkable && (positionChanged || intervalElapsed);
```
`PhysicsBody.InContact` and `PhysicsBody.OnWalkable` are existing convenience properties that wrap `TransientState.HasFlag(TransientStateFlags.Contact)` / `.OnWalkable`. The same `Contact && OnWalkable` "grounded" pattern is used elsewhere in our physics layer (e.g., `MotionInterpreter.cs:808-809` for jump-start gating), so this matches the codebase convention.
- [ ] **Step 3: Add `ApproxPositionEqual` helper**
In the same file, near the end of the class (after the existing helper methods), add:
```csharp
/// <summary>
/// 2026-05-16. Position-equality test for diff-driven AP cadence.
/// Retail uses Frame::is_equal at acclient_2013_pseudo_c.txt:700263
/// which is essentially exact float comparison after a memcmp of
/// the frame struct. For floating-point safety we use a tiny epsilon
/// — sub-millimeter — that's well below any movement we'd want to
/// suppress sending for.
/// </summary>
private static bool ApproxPositionEqual(
System.Numerics.Vector3 a, System.Numerics.Vector3 b)
{
const float Epsilon = 0.001f; // 1 mm
return MathF.Abs(a.X - b.X) < Epsilon
&& MathF.Abs(a.Y - b.Y) < Epsilon
&& MathF.Abs(a.Z - b.Z) < Epsilon;
}
```
- [ ] **Step 4: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task B3: Wire `GameWindow` to call `NotePositionSent`
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — find the outbound network send sites.
- [ ] **Step 1: Identify send call sites**
Run:
```
grep -n "SendAutonomousPosition\|SendMoveToState\|_playerController.HeartbeatDue\|SendGameAction" src/AcDream.App/Rendering/GameWindow.cs | head -20
```
Note line numbers — typically there's:
- One heartbeat-driven AP send site (around line 6310 area).
- One input-driven MoveToState site (around the `MotionStateChanged` check).
- [ ] **Step 2: After each successful AP/MoveToState send, call `NotePositionSent`**
Immediately after each `_liveSession.SendGameAction(body)` for AP and MoveToState, add:
```csharp
_playerController.NotePositionSent(
worldPos: _playerController.Position,
cellId: _playerController.CellId,
nowSeconds: _playerController.SimTimeSeconds);
```
For the AP heartbeat site, this is post-send. For the MoveToState site, same — also stamps the clock.
NOTE: Do NOT add this for outbound Use / PickUp / JumpAction packets — those don't carry position.
- [ ] **Step 3: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task B4: Delete `TinyMargin` from `ApplyAutoWalkOverlay`
**Files:**
- Modify: `src/AcDream.App/Input/PlayerMovementController.cs:466-472`
- [ ] **Step 1: Remove the margin clamp**
Locate lines 466-472:
```csharp
const float TinyMargin = 0.05f;
float effectiveArrival = MathF.Max(arrivalThreshold - TinyMargin, 0.1f);
bool withinArrival =
(_autoWalkMoveTowards
&& dist <= effectiveArrival)
|| (!_autoWalkMoveTowards
&& dist >= arrivalThreshold + RemoteMoveToDriver.ArrivalEpsilon);
```
Replace with:
```csharp
// 2026-05-16 — retail "stop at the radius" semantics.
// Previously had a 0.05 m TinyMargin inside the threshold to
// ensure ACE's server-side WithinUseRadius poll saw us inside
// the radius before our next AP heartbeat. With the
// diff-driven AP cadence (Task B2) ACE sees the final position
// the same frame we arrive — no margin needed. Retail's
// arrival check is `dist <= radius` exact at
// CMotionInterp::apply_interpreted_movement integration.
bool withinArrival =
(_autoWalkMoveTowards
&& dist <= arrivalThreshold)
|| (!_autoWalkMoveTowards
&& dist >= arrivalThreshold + RemoteMoveToDriver.ArrivalEpsilon);
```
- [ ] **Step 2: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task B5: Delete `OnAutoWalkArrivedReSendAction` + `SendAutonomousPositionNow`
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` (delete two methods + subscription)
- [ ] **Step 1: Find the subscription**
Run:
```
grep -n "AutoWalkArrived" src/AcDream.App/Rendering/GameWindow.cs
```
Expected: at least one `+=` subscription site and the method definition at line ~9302.
- [ ] **Step 2: Delete the subscription line**
In `GameWindow.cs`, find the line:
```csharp
_playerController.AutoWalkArrived += OnAutoWalkArrivedReSendAction;
```
Delete it (entire line). (A NEW subscription is added in Task B6 Step 4 — keep that file region noted.)
- [ ] **Step 3: Delete the `OnAutoWalkArrivedReSendAction` method**
Locate the method at line ~9302:
```csharp
private void OnAutoWalkArrivedReSendAction()
{
if (_pendingPostArrivalAction is not (uint guid, bool isPickup) pending)
return;
_pendingPostArrivalAction = null;
// ... probe log ...
SendAutonomousPositionNow();
// ... probe log ...
if (isPickup) SendPickUp(guid, isRetryAfterArrival: true);
else SendUse(guid, isRetryAfterArrival: true);
}
```
Delete the entire method (including any doc-comment block above it).
- [ ] **Step 4: Delete `SendAutonomousPositionNow`**
Locate the method at line ~9424. Delete the entire method. With retail-faithful diff-driven cadence (Task B2), the next regular AP send carries the arrived position naturally.
- [ ] **Step 5: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: build will likely FAIL with "cannot find SendAutonomousPositionNow" or "cannot find OnAutoWalkArrivedReSendAction" or similar — those errors get fixed in B6.
---
### Task B6: Remove `isRetryAfterArrival` from `SendUse`/`SendPickUp`; simplify deferred-Use to close-range turn-first only
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — methods `SendUse` and `SendPickUp` (lines ~9162 and ~9226).
- [ ] **Step 1: Replace `SendUse` body**
Current method signature: `private void SendUse(uint guid, bool isRetryAfterArrival = false)`. Change to `private void SendUse(uint guid)` and the body to:
```csharp
private void SendUse(uint guid)
{
if (_liveSession is null
|| _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld)
{
_debugVm?.AddToast("Not in world");
return;
}
// Retail-faithful useability gate (acclient_2013_pseudo_c.txt:256455
// ItemUses::IsUseable). Signs / banners with useability=0 silently
// ignore Use.
if (!IsUseableTarget(guid))
{
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
Console.WriteLine($"[B.4b] SendUse ignored — not useable guid=0x{guid:X8}");
return;
}
// B.6 (2026-05-15): install speculative local auto-walk against
// the target so close-range Use rotates the body to face before
// the action fires. For FAR targets, ACE's CreateMoveToChain
// (Player_Move.cs:37-179) takes over via inbound MovementType=6
// and our overlay is overwritten by ACE's wire-supplied radius.
//
// 2026-05-16: simplified — close-range deferral now fires the
// wire packet ONCE on AutoWalkArrived (turn-first done), not a
// retry of an earlier failed send. No re-send path.
bool closeRange = IsCloseRangeTarget(guid);
InstallSpeculativeTurnToTarget(guid);
if (closeRange)
{
// Defer the wire packet — OnAutoWalkArrivedSendDeferredAction
// will fire it after rotation completes.
_pendingPostArrivalAction = (guid, false);
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
Console.WriteLine($"[B.4b] use deferred (close-range, turn-first) guid=0x{guid:X8}");
return;
}
// Far range: fire immediately. ACE auto-walks server-side; the
// retail-faithful AP cadence keeps ACE's WithinUseRadius poll
// in sync, so the action completes when the body arrives
// without any client-side re-send.
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}");
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
{
string label = DescribeLiveEntity(guid);
Console.WriteLine(System.FormattableString.Invariant(
$"[autowalk-out] op=use target=0x{guid:X8} name=\"{label}\" seq={seq}"));
}
}
```
- [ ] **Step 2: Replace `SendPickUp` body the same way**
Drop the `isRetryAfterArrival` parameter. Final shape:
```csharp
private void SendPickUp(uint itemGuid)
{
if (_liveSession is null
|| _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld)
{
_debugVm?.AddToast("Not in world");
return;
}
if (!IsUseableTarget(itemGuid))
{
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
Console.WriteLine($"[B.5] SendPickUp ignored — not useable item=0x{itemGuid:X8}");
return;
}
// Block creature pickup (silent — matches retail).
if (IsLiveCreatureTarget(itemGuid))
return;
bool closeRange = IsCloseRangeTarget(itemGuid);
InstallSpeculativeTurnToTarget(itemGuid);
if (closeRange)
{
_pendingPostArrivalAction = (itemGuid, true);
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
Console.WriteLine($"[B.5] pickup deferred (close-range, turn-first) item=0x{itemGuid:X8}");
return;
}
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}");
if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled)
{
string label = DescribeLiveEntity(itemGuid);
Console.WriteLine(System.FormattableString.Invariant(
$"[autowalk-out] op=pickup target=0x{itemGuid:X8} name=\"{label}\" seq={seq}"));
}
}
```
- [ ] **Step 3: Add `OnAutoWalkArrivedSendDeferredAction` handler**
Add this method to `GameWindow.cs` (anywhere in the class body — near the deleted `OnAutoWalkArrivedReSendAction` site is fine):
```csharp
/// <summary>
/// 2026-05-16. Fires the deferred close-range Use/PickUp action
/// once the local auto-walk overlay reports arrival (i.e. the body
/// has finished rotating to face the target). Unlike the old
/// `OnAutoWalkArrivedReSendAction`, this is a FIRST send — not a
/// retry of an earlier failed send. Far-range Use/PickUp paths
/// fire the wire packet immediately at `SendUse`/`SendPickUp` time
/// and never touch `_pendingPostArrivalAction`.
/// </summary>
private void OnAutoWalkArrivedSendDeferredAction()
{
if (_pendingPostArrivalAction is not (uint guid, bool isPickup) pending)
return;
_pendingPostArrivalAction = null;
if (_liveSession is null
|| _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld)
return;
var seq = _liveSession.NextGameActionSequence();
if (isPickup)
{
var body = AcDream.Core.Net.Messages.InteractRequests.BuildPickUp(
seq, guid, _playerServerGuid, placement: 0);
_liveSession.SendGameAction(body);
Console.WriteLine($"[B.5] pickup-deferred item=0x{guid:X8} container=0x{_playerServerGuid:X8} seq={seq}");
}
else
{
var body = AcDream.Core.Net.Messages.InteractRequests.BuildUse(seq, guid);
_liveSession.SendGameAction(body);
Console.WriteLine($"[B.4b] use-deferred guid=0x{guid:X8} seq={seq}");
}
}
```
- [ ] **Step 4: Subscribe to `AutoWalkArrived`**
Find where `_playerController` is constructed and other subscriptions happen (the deleted subscription from B5 Step 2 was here). Add:
```csharp
_playerController.AutoWalkArrived += OnAutoWalkArrivedSendDeferredAction;
```
- [ ] **Step 5: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors. If there are remaining "isRetryAfterArrival" references, fix them — those are callers that need to drop the named-argument.
---
### Task B7: Factor screen-rect projection into shared helper + add screen-rect picker overload
**Goal:** Retail's picker uses two-tier sphere-then-polygon selection (`Render::GfxObjUnderSelectionRay` 0x0054c740). Our Stage A approximates Stage 1 (sphere reject) — but the indicator draws a SCREEN-SPACE rect, not a 3D sphere. User feedback 2026-05-16: "the range we can click + the area of the object that's clickable should be faithful to retail" — meaning the click hit-area must match what the indicator visibly bounds. A pure world-space ray-sphere picker can't make that guarantee (rect corners are sphere dead zones).
**Approach:** Picker hit-tests against the SAME screen-space rect the indicator draws. The projection math is factored into a shared `ScreenProjection` helper so the indicator and the picker can't drift apart.
**Files:**
- Create: `src/AcDream.Core/Selection/ScreenProjection.cs`
- Modify: `src/AcDream.Core/Selection/WorldPicker.cs`
- Modify: `src/AcDream.App/UI/TargetIndicatorPanel.cs` (use shared helper; private copy deleted)
- Test: `tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs` (new file)
- Test: `tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs` (modify or create)
- [ ] **Step 1: Write failing tests for `ScreenProjection`**
Create `tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs`:
```csharp
using System.Numerics;
using AcDream.Core.Selection;
namespace AcDream.Core.Tests.Selection;
public sealed class ScreenProjectionTests
{
// Standard right-handed perspective + identity view. Sphere centered
// at z=+10 in front of camera, radius 1, viewport 800x600.
private static (Matrix4x4 view, Matrix4x4 proj, Vector2 viewport) StdCam()
{
var view = Matrix4x4.Identity;
var proj = Matrix4x4.CreatePerspectiveFieldOfView(
MathF.PI * 0.5f /*fovY 90°*/, 800f / 600f, 0.1f, 100f);
var viewport = new Vector2(800, 600);
return (view, proj, viewport);
}
[Fact]
public void TryProject_SphereInFront_ReturnsSquareRect()
{
// System.Numerics CreatePerspectiveFieldOfView is right-handed
// (looks down -Z). Place sphere at -10 along Z so it sits in
// front of the camera.
var (view, proj, viewport) = StdCam();
bool ok = ScreenProjection.TryProjectSphereToScreenRect(
new Vector3(0, 0, -10), worldRadius: 1f,
view, proj, viewport,
out var rMin, out var rMax, out var depth,
minSidePixels: 0f);
Assert.True(ok);
// 90° FOV at depth 10 -> screen half-extent = 1 unit projects to
// viewport.Y/2 pixels per world-unit at near plane = 1/tan(45°).
// The rect should be a square (width == height).
Assert.Equal(rMax.X - rMin.X, rMax.Y - rMin.Y, precision: 3);
// Rect should be centered (approximately) on the screen center.
Assert.InRange((rMin.X + rMax.X) * 0.5f, 399f, 401f);
Assert.InRange((rMin.Y + rMax.Y) * 0.5f, 299f, 301f);
Assert.True(depth > 0f);
}
[Fact]
public void TryProject_SphereBehindCamera_ReturnsFalse()
{
var (view, proj, viewport) = StdCam();
bool ok = ScreenProjection.TryProjectSphereToScreenRect(
new Vector3(0, 0, +10) /* behind RH camera at origin */,
worldRadius: 1f,
view, proj, viewport,
out _, out _, out _,
minSidePixels: 0f);
Assert.False(ok);
}
[Fact]
public void TryProject_FarSphereClampsToMinSide()
{
var (view, proj, viewport) = StdCam();
bool ok = ScreenProjection.TryProjectSphereToScreenRect(
new Vector3(0, 0, -90) /* very far */, worldRadius: 0.01f /* tiny */,
view, proj, viewport,
out var rMin, out var rMax, out _,
minSidePixels: 12f);
Assert.True(ok);
Assert.True(rMax.X - rMin.X >= 12f);
Assert.True(rMax.Y - rMin.Y >= 12f);
}
}
```
- [ ] **Step 2: Build the test project — confirm test failure**
```
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~ScreenProjectionTests" --no-build
```
Expected: build failure (`ScreenProjection` doesn't exist yet).
- [ ] **Step 3: Create `src/AcDream.Core/Selection/ScreenProjection.cs`**
```csharp
using System.Numerics;
namespace AcDream.Core.Selection;
/// <summary>
/// Shared screen-space projection math for the target indicator and the
/// world picker. Both call into <see cref="TryProjectSphereToScreenRect"/>
/// so the click hit-area is guaranteed to match the visible indicator
/// rect — "what you see is what you click".
///
/// <para>
/// Retail equivalent: <c>SmartBox::GetObjectBoundingBox</c> at
/// <c>0x00452e20</c>, which uses
/// <c>Render::GetViewerBBox(selection_sphere, &amp;corner1, &amp;corner2)</c>
/// to compute a camera-aligned bbox of the sphere and projects the two
/// corner points. We use the mathematical equivalent (project center,
/// compute screen radius analytically) — both produce identical pixel
/// rects for a standard right-handed perspective.
/// </para>
/// </summary>
public static class ScreenProjection
{
/// <summary>
/// Project a world-space sphere to a screen-space axis-aligned square
/// rectangle.
/// </summary>
/// <param name="worldCenter">Sphere center in world space.</param>
/// <param name="worldRadius">Sphere radius in world space.</param>
/// <param name="view">View matrix (System.Numerics row-vector convention).</param>
/// <param name="projection">Projection matrix. <c>M22 = cot(fovY/2)</c>
/// for a standard right-handed perspective.</param>
/// <param name="viewport">Viewport size in pixels (X = width, Y = height).</param>
/// <param name="rectMin">Out: top-left corner of the rect in viewport pixels.</param>
/// <param name="rectMax">Out: bottom-right corner of the rect in viewport pixels.</param>
/// <param name="depth">Out: camera-space depth (<c>clip.W</c>) of the sphere
/// center — use this for nearest-first sorting when multiple rects overlap.</param>
/// <param name="minSidePixels">Minimum side length of the rect. Distant
/// entities clamp to this so they remain pickable / visible. 12 px
/// matches the indicator's clamp floor.</param>
/// <returns>
/// <c>true</c> if the sphere is in front of the camera and the rect was
/// produced; <c>false</c> if the center is behind the camera
/// (<c>clip.W &lt;= 0</c>) or the rect is more than a screen offset
/// from the viewport (obviously off-screen).
/// </returns>
public static bool TryProjectSphereToScreenRect(
Vector3 worldCenter, float worldRadius,
Matrix4x4 view, Matrix4x4 projection, Vector2 viewport,
out Vector2 rectMin, out Vector2 rectMax, out float depth,
float minSidePixels = 12f)
{
rectMin = default;
rectMax = default;
depth = 0f;
var viewProj = view * projection;
var clip = Vector4.Transform(new Vector4(worldCenter, 1f), viewProj);
if (clip.W <= 0.001f) return false;
depth = clip.W;
float ndcX = clip.X / clip.W;
float ndcY = clip.Y / clip.W;
float screenX = (ndcX * 0.5f + 0.5f) * viewport.X;
float screenY = (1f - (ndcY * 0.5f + 0.5f)) * viewport.Y;
// Screen-space radius. projection.M22 = cot(fovY/2). clip.W is
// the camera-space distance.
float scaleY = projection.M22;
if (scaleY <= 0f) return false;
float screenRadius = worldRadius * scaleY * viewport.Y / (2f * clip.W);
// Cull obviously-off-screen entities (more than a screen away).
if (screenX + screenRadius < -viewport.X || screenX - screenRadius > 2f * viewport.X) return false;
if (screenY + screenRadius < -viewport.Y || screenY - screenRadius > 2f * viewport.Y) return false;
// Floor at minSidePixels so distant entities still get a visible /
// clickable rect. The picker must apply the same floor as the
// indicator or distant clicks won't match the visible bracket.
if (screenRadius < minSidePixels * 0.5f) screenRadius = minSidePixels * 0.5f;
rectMin = new Vector2(screenX - screenRadius, screenY - screenRadius);
rectMax = new Vector2(screenX + screenRadius, screenY + screenRadius);
return true;
}
}
```
- [ ] **Step 4: Run `ScreenProjection` tests — confirm pass**
```
dotnet build src/AcDream.Core/AcDream.Core.csproj -c Debug
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~ScreenProjectionTests" --no-build
```
Expected: 3/3 pass.
- [ ] **Step 5: Write failing tests for the new `WorldPicker.Pick` rect overload**
In `tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs` (create if missing):
```csharp
using System.Numerics;
using AcDream.Core.Selection;
using AcDream.Core.World;
namespace AcDream.Core.Tests.Selection;
public sealed class WorldPickerRectOverloadTests
{
// Same right-handed perspective as ScreenProjectionTests.
private static (Matrix4x4 view, Matrix4x4 proj, Vector2 viewport) StdCam()
{
var view = Matrix4x4.Identity;
var proj = Matrix4x4.CreatePerspectiveFieldOfView(
MathF.PI * 0.5f, 800f / 600f, 0.1f, 100f);
var viewport = new Vector2(800, 600);
return (view, proj, viewport);
}
[Fact]
public void Pick_RectHitTest_ReturnsHitWhenMouseInsideRect()
{
var (view, proj, viewport) = StdCam();
var e = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -10), Scale = 1f };
// Mouse at screen center; entity at world (0,0,-10) projects to
// screen center. Rect contains screen center → hit.
uint? picked = WorldPicker.Pick(
mouseX: 400f, mouseY: 300f,
view, proj, viewport,
new[] { e },
skipServerGuid: 0u,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Equal(0x10001u, picked);
}
[Fact]
public void Pick_RectHitTest_ReturnsNullWhenMouseOutsideRect()
{
var (view, proj, viewport) = StdCam();
var e = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -10), Scale = 1f };
// Mouse far from entity rect → no hit.
uint? picked = WorldPicker.Pick(
mouseX: 50f, mouseY: 50f,
view, proj, viewport,
new[] { e },
skipServerGuid: 0u,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Null(picked);
}
[Fact]
public void Pick_RectHitTest_PicksNearerWhenRectsOverlap()
{
var (view, proj, viewport) = StdCam();
// Two entities both at screen center but different depths.
var near = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -8), Scale = 1f };
var far = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 0, -15), Scale = 1f };
uint? picked = WorldPicker.Pick(
mouseX: 400f, mouseY: 300f,
view, proj, viewport,
new[] { far, near } /* deliberately reversed */,
skipServerGuid: 0u,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Equal(0x10001u, picked);
}
[Fact]
public void Pick_RectHitTest_NullResolverSkipsCandidates()
{
var (view, proj, viewport) = StdCam();
var e1 = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -10), Scale = 1f };
var e2 = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 0, -20), Scale = 1f };
uint? picked = WorldPicker.Pick(
mouseX: 400f, mouseY: 300f,
view, proj, viewport,
new[] { e1, e2 },
skipServerGuid: 0u,
sphereForEntity: x => x.ServerGuid == 0x10001u
? ((Vector3, float)?)null
: ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Equal(0x10002u, picked);
}
[Fact]
public void Pick_RectHitTest_RespectsSkipServerGuid()
{
var (view, proj, viewport) = StdCam();
var player = new WorldEntity { ServerGuid = 0x5000000Au, Position = new Vector3(0, 0, -10), Scale = 1f };
var npc = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 0, -15), Scale = 1f };
uint? picked = WorldPicker.Pick(
mouseX: 400f, mouseY: 300f,
view, proj, viewport,
new[] { player, npc },
skipServerGuid: 0x5000000Au,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Equal(0x10002u, picked);
}
[Fact]
public void Pick_RectHitTest_InflateExpandsClickableArea()
{
var (view, proj, viewport) = StdCam();
var e = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -10), Scale = 1f };
// First: with inflate=0, a mouse 30 px outside the rect misses.
uint? withoutInflate = WorldPicker.Pick(
mouseX: 400f + 200f, mouseY: 300f,
view, proj, viewport,
new[] { e },
skipServerGuid: 0u,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 0f);
Assert.Null(withoutInflate);
// Then: same mouse position, with a 250 px inflate, now hits.
uint? withInflate = WorldPicker.Pick(
mouseX: 400f + 200f, mouseY: 300f,
view, proj, viewport,
new[] { e },
skipServerGuid: 0u,
sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f),
inflatePixels: 250f);
Assert.Equal(0x10001u, withInflate);
}
}
```
NOTE: If `WorldEntity` is `init`-only / immutable, adjust the test object-initializer calls accordingly — check `src/AcDream.Core/World/WorldEntity.cs` for the actual constructor / property pattern.
- [ ] **Step 6: Run `WorldPicker` rect tests — confirm failure**
```
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~WorldPickerRectOverloadTests" --no-build
```
Expected: build failure (the new `Pick` overload doesn't exist yet).
- [ ] **Step 7: Add the new `Pick` overload to `WorldPicker.cs`**
Append to `src/AcDream.Core/Selection/WorldPicker.cs` (do NOT delete the existing ray-sphere overload — its caller is migrated in B8, but the API stays for back-compat):
```csharp
/// <summary>
/// 2026-05-16. Screen-space rect-hit-test picker overload. Each
/// candidate's world-space sphere (via <paramref name="sphereForEntity"/>)
/// projects to a screen-space rectangle through
/// <see cref="ScreenProjection.TryProjectSphereToScreenRect"/>. The
/// rect is inflated by <paramref name="inflatePixels"/> on every side
/// (matches the indicator's <c>TriangleSize</c> outer brackets) and
/// hit-tested against the mouse pixel. Among rects that contain the
/// mouse, the entity with the nearest camera-space depth wins.
///
/// <para>
/// Why screen-space instead of world-space ray-sphere: the indicator
/// draws a screen-space RECT. A world-space sphere projects to a
/// screen CIRCLE inscribed in that rect — leaving the four rect
/// corners as click dead zones. Per user feedback 2026-05-16, the
/// click area must match the visible indicator extent exactly. By
/// sharing the <see cref="ScreenProjection"/> helper with
/// <c>TargetIndicatorPanel</c>, the click rect and the drawn rect
/// cannot drift.
/// </para>
///
/// <para>
/// Resolver returning <c>null</c> skips the candidate (matches retail
/// "no Setup → not pickable" behavior). Entities with
/// <c>ServerGuid == 0</c> (atlas-tier scenery) and the player's own
/// guid are also skipped.
/// </para>
///
/// <para>
/// Stage A of the picker port. Stage B (polygon refine via
/// <c>CPolygon::polygon_hits_ray</c> 0x0054c889) remains deferred
/// per issue #71 — only needed if visual testing surfaces a Stage A
/// over-pick on entities whose visible mesh is well inside the
/// indicator rect.
/// </para>
/// </summary>
/// <param name="inflatePixels">Pixel inflate on each side of the
/// projected rect. Pass the indicator's <c>TriangleSize</c> (8 px)
/// so the click area extends to where the visible bracket corners
/// sit — the user perceives the inflated rect as the clickable area.</param>
public static uint? Pick(
float mouseX, float mouseY,
System.Numerics.Matrix4x4 view,
System.Numerics.Matrix4x4 projection,
System.Numerics.Vector2 viewport,
IEnumerable<AcDream.Core.World.WorldEntity> candidates,
uint skipServerGuid,
Func<AcDream.Core.World.WorldEntity, (System.Numerics.Vector3 CenterWorld, float Radius)?> sphereForEntity,
float inflatePixels = 8f)
{
uint? bestGuid = null;
float bestDepth = float.PositiveInfinity;
foreach (var entity in candidates)
{
if (entity.ServerGuid == 0u) continue;
if (entity.ServerGuid == skipServerGuid) continue;
var sphere = sphereForEntity(entity);
if (sphere is null) continue;
var (center, radius) = sphere.Value;
if (radius <= 0f) continue;
if (!ScreenProjection.TryProjectSphereToScreenRect(
center, radius, view, projection, viewport,
out var rMin, out var rMax, out var depth))
continue;
// Inflate by inflatePixels on each side — extend hit area to
// where the indicator brackets sit.
float minX = rMin.X - inflatePixels;
float minY = rMin.Y - inflatePixels;
float maxX = rMax.X + inflatePixels;
float maxY = rMax.Y + inflatePixels;
if (mouseX < minX || mouseX > maxX) continue;
if (mouseY < minY || mouseY > maxY) continue;
if (depth < bestDepth)
{
bestDepth = depth;
bestGuid = entity.ServerGuid;
}
}
return bestGuid;
}
```
- [ ] **Step 8: Refactor `TargetIndicatorPanel` to use the shared helper**
In `src/AcDream.App/UI/TargetIndicatorPanel.cs`:
1. Delete the private `TryComputeScreenRectFromSphere` method (lines 321-356 inclusive — keep `TryProjectToScreen` since the fallback branch still uses it).
2. Change the call site at line 217 from `TryComputeScreenRectFromSphere(sphereCenter, sphereRadius, view, projection, viewport, out var rMin, out var rMax)` to:
```csharp
&& AcDream.Core.Selection.ScreenProjection.TryProjectSphereToScreenRect(
sphereCenter, sphereRadius, view, projection, viewport,
out var rMin, out var rMax, out _,
minSidePixels: 12f))
```
(The `out _` discards the depth — the indicator doesn't need it. `minSidePixels: 12f` preserves the existing clamp.)
- [ ] **Step 9: Build all touched projects**
```
dotnet build src/AcDream.Core/AcDream.Core.csproj -c Debug
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors on both.
- [ ] **Step 10: Run all picker + projection tests**
```
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~ScreenProjectionTests|FullyQualifiedName~WorldPickerRectOverloadTests" --no-build
```
Expected: 9/9 pass (3 ScreenProjection + 6 WorldPicker rect-overload).
---
### Task B8: Switch `GameWindow` picker call to the screen-rect overload
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs``PickAndStoreSelection` method around line 9006.
- [ ] **Step 1: Locate the existing call**
Run:
```
grep -n "WorldPicker.Pick(" src/AcDream.App/Rendering/GameWindow.cs
```
Expected: one match at ~line 9018.
- [ ] **Step 2: Replace the ray-build + ray-pick chain with the screen-rect picker**
The existing block reads (paraphrased; lines 9011-9072):
```csharp
var camera = _cameraController.Active;
var (origin, direction) = AcDream.Core.Selection.WorldPicker.BuildRay(
mouseX: _lastMouseX, mouseY: _lastMouseY,
viewportW: _window.Size.X, viewportH: _window.Size.Y,
view: camera.View, projection: camera.Projection);
if (direction.LengthSquared() < 1e-6f) return; // degenerate ray
var picked = AcDream.Core.Selection.WorldPicker.Pick(
origin, direction,
_entitiesByServerGuid.Values,
skipServerGuid: _playerServerGuid,
maxDistance: 50f,
radiusForGuid: g => { /* per-type heuristics ... */ },
verticalOffsetForGuid: g => { /* per-type heuristics ... */ });
```
Replace the entire block with:
```csharp
// 2026-05-16 — retail-faithful screen-rect picker. The hit area
// is the same screen-space rect the target indicator draws
// (computed via the shared AcDream.Core.Selection.ScreenProjection
// helper). Per user feedback 2026-05-16: clicking the indicator
// brackets — including the rect corners — must select the entity.
// The per-type radius/offset heuristics retired here (1.0/1.6/2.0
// m radii, 0.2/0.9/1.0/1.5 m vertical offsets, IsTallSceneryGuid)
// existed to make a 3D ray-sphere picker approximate the visible
// rect; the new picker doesn't need them.
var camera = _cameraController.Active;
var viewport = new System.Numerics.Vector2(_window.Size.X, _window.Size.Y);
var picked = AcDream.Core.Selection.WorldPicker.Pick(
mouseX: _lastMouseX, mouseY: _lastMouseY,
view: camera.View, projection: camera.Projection,
viewport: viewport,
candidates: _entitiesByServerGuid.Values,
skipServerGuid: _playerServerGuid,
sphereForEntity: e =>
{
// Authoritative: Setup's SelectionSphere (matches the
// indicator's input).
if (TryGetEntitySelectionSphere(e.ServerGuid, out var c, out var r))
return ((System.Numerics.Vector3, float)?)(c, r);
// Fallback for entities whose Setup didn't bake a
// SelectionSphere (rare). Synthesize a 1.5 m × scale
// sphere centered on body-mid — same intent as B9's
// simplified EntityHeightFor fallback, so the picker
// and indicator agree even on the fallback path.
float scale = 1f;
if (_lastSpawnByGuid.TryGetValue(e.ServerGuid, out var s) && s.ObjScale is float es && es > 0f)
scale = es;
float half = 0.75f * scale;
var center = e.Position + new System.Numerics.Vector3(0, 0, half);
return ((System.Numerics.Vector3, float)?)(center, half);
},
// Match the indicator's TriangleSize (8 px) so the click area
// extends out to the bracket corners — what the user perceives
// as "selectable extent."
inflatePixels: 8f);
```
The local `origin`/`direction` variables and the `if (direction.LengthSquared() < 1e-6f) return;` guard are no longer needed — delete them along with the `BuildRay` call.
- [ ] **Step 3: Delete the per-type `radiusForGuid` and `verticalOffsetForGuid` lambda blocks**
Both lambdas (the entire `radiusForGuid: g => { ... }` and `verticalOffsetForGuid: g => { ... }` blocks at ~9037 and ~9054) are gone with the rewrite in Step 2. Confirm there are no dangling references via:
```
grep -n "radiusForGuid\|verticalOffsetForGuid" src/AcDream.App/Rendering/GameWindow.cs
```
Expected: 0 matches in `GameWindow.cs` after the edit.
- [ ] **Step 4: Build**
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors. The old `WorldPicker.Pick(origin, direction, ...)` ray-sphere overload still exists in `WorldPicker.cs` but has no callers — leave it for now; cleaning it up is a follow-up.
---
### Task B9: Trim `EntityHeightFor` fallback to a single defensive default
**Files:**
- Modify: `src/AcDream.App/UI/TargetIndicatorPanel.cs` — function `EntityHeightFor`.
- [ ] **Step 1: Simplify the per-type cascade**
Locate `EntityHeightFor` in `TargetIndicatorPanel.cs`. It currently has per-type branches (Creature 1.8m, Door/Lifestone/Portal/Corpse 2.4m, small items 0.8m, default 3.0m). With the sphere-projection path handling every entity that has a `SelectionSphere`, this method is now a defensive rescue path only.
Replace the entire method body with:
```csharp
/// <summary>
/// Defensive fallback height when the entity has no usable
/// SelectionSphere (Radius ≤ 1e-4f). With B.7's sphere-projection
/// path active (since commit f4f4143), this fallback only fires
/// for entities whose Setup didn't bake a selection sphere —
/// rare in practice. The single 1.5 m × scale default is a sane
/// midpoint; per-type branches were retired in the 2026-05-16
/// Commit B because the sphere path is authoritative.
/// </summary>
public float EntityHeightFor(uint itemType, uint pwdBitfield, float scale, uint? useability = null)
{
if (scale <= 0f) scale = 1f;
return 1.5f * scale;
}
```
Keep all the SmallItemMask / TallStructureMask / Creature constants out — they're no longer needed. The method signature stays unchanged for ABI compat with any external callers.
- [ ] **Step 2: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors.
---
### Task B10: Delete `IsTallSceneryGuid`
**Files:**
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — method `IsTallSceneryGuid` and all callers.
- [ ] **Step 1: Find call sites**
Run:
```
grep -n "IsTallSceneryGuid" src/AcDream.App/Rendering/GameWindow.cs
```
Expected hits:
- Definition (~line 9550-9610 area)
- Two call sites inside the (deleted in B8) `radiusForGuid` / `verticalOffsetForGuid` lambdas — should be gone after B8
- One call site in the `[B.7] pick-info` diagnostic line at ~line 9105
- [ ] **Step 2: Remove `IsTallSceneryGuid` from the pick-info diagnostic**
In the `[B.7] pick-info` log, find the line:
```csharp
Console.WriteLine(System.FormattableString.Invariant(
$"[B.7] pick-info guid=0x{guid:X8} itemType=0x{rawItemType:X8} pwd=0x{pwdBits:X8} use={useStr} useRadius={radStr} scale={pickScale:F2} setup={setupStr} tallScenery={IsTallSceneryGuid(guid)} color=({col.R},{col.G},{col.B})"));
```
Replace with (drop the `tallScenery=` field):
```csharp
Console.WriteLine(System.FormattableString.Invariant(
$"[B.7] pick-info guid=0x{guid:X8} itemType=0x{rawItemType:X8} pwd=0x{pwdBits:X8} use={useStr} useRadius={radStr} scale={pickScale:F2} setup={setupStr} color=({col.R},{col.G},{col.B})"));
```
- [ ] **Step 3: Delete the `IsTallSceneryGuid` method**
Find the method definition (the comment block above it starts with "2026-05-15. True when the entity is 'tall scenery'..."). Delete the entire method (comment block + body).
- [ ] **Step 4: Build**
Run:
```
dotnet build src/AcDream.App/AcDream.App.csproj -c Debug
```
Expected: 0 errors. No remaining `IsTallSceneryGuid` references.
---
### Task B11: Run full test suite, visual verify, commit Commit B
- [ ] **Step 1: Run full test suite**
Run:
```
dotnet test tests/AcDream.Core.Net.Tests/AcDream.Core.Net.Tests.csproj -c Debug
dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj -c Debug
```
Expected: Core.Net 290+ pass; Core baseline unchanged + 3 new WorldPickerSphereOverloadTests pass.
- [ ] **Step 2: Visual verify with `ACDREAM_PROBE_USEABILITY_FALLBACK=1` + `ACDREAM_PROBE_AUTOWALK=1`**
User runs the client and confirms:
1. **Far-range Use on NPC.** Click NPC at 8 m, press R. Expected: walks, turns to face, dialogue fires. Log shows ONE `[B.4b] use` line, NOT multiple. No `use-deferred` line (far-range fires immediately).
2. **Close-range Use on NPC behind player.** Stand within 2 m of NPC facing away. Press R. Expected: character turns 180°, then dialogue fires. Log shows `[B.4b] use deferred (close-range, turn-first)` followed by `[B.4b] use-deferred` (the deferred fire on arrival). Exactly ONE deferred fire.
3. **Open inn door from across the room.** Walks, opens. ONE `[B.4b] use` line.
4. **Pickup item from across the room.** Walks, picks up. ONE `[B.5] pickup` line.
5. **Click the Holtburg sign.** Indicator triangles match the sign size (unchanged from previous ship). Press R. Silent no-op (`SendUse ignored — not useable`).
6. **Click rapidly between NPC and item.** No spurious "I clicked X earlier and now it's firing on Y" cross-contamination. The `_pendingPostArrivalAction` simplification should make this clean.
7. **Click in the indicator's bracket corners.** Hover the mouse so it sits in the rect-corner region of the indicator brackets (where the four triangle marks sit) — NOT on the entity body. Click. The entity selects. Old behaviour: corners were sphere dead zones and the click missed; new behaviour: click area = visible bracket bounding rect, corner clicks land.
8. **Click adjacent to an entity but outside the indicator rect.** Mouse just outside the bracket extent. No selection. Old behaviour: sphere over-pick let cursor land far from the visible rect and still select; new behaviour: rect edges are tight.
**If any of (1)-(6) regresses, the cadence fix in Task B2 likely needs tuning. Common cause: `IsGrounded` is too restrictive (suppressing AP on slopes); relax to `ContactPlane.Normal.Z > 0.3f` or similar.**
**STOP and wait for user confirmation before committing.**
- [ ] **Step 3: Commit B**
When user approves:
```bash
git add src/AcDream.App/Input/PlayerMovementController.cs \
src/AcDream.App/Rendering/GameWindow.cs \
src/AcDream.Core/Selection/ScreenProjection.cs \
src/AcDream.Core/Selection/WorldPicker.cs \
src/AcDream.App/UI/TargetIndicatorPanel.cs \
tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs \
tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs
git commit -m "$(cat <<'EOF'
fix(retail): per-tick AP cadence + screen-rect picker retires 4 workarounds
Single coherent commit. Audit findings from 2026-05-16:
1. AutonomousPosition cadence was 1 Hz idle / 10 Hz active flat.
Retail (CommandInterpreter::ShouldSendPositionEvent at
acclient_2013_pseudo_c.txt:0x006b45e0) is diff-driven:
- Position or cell changed since last send → send (per frame
while moving, 30-60 Hz in practice).
- Otherwise, 1 sec heartbeat (time_between_position_events
constant at 0x006b3efb).
- Gated on transient_state & (CONTACT_TS | ON_WALKABLE_TS) —
suppressed airborne.
Replaced HeartbeatAccum with diff-driven HeartbeatDue +
NotePositionSent (resets the clock at every wire send, including
MoveToState — matches retail's SendMovementEvent 0x006b4680
sharing the same `last_sent_position_time` clock).
2. The diff-driven cadence retires all 4 B.6/B.7 workarounds:
- TinyMargin (0.05 m inside arrival): retail stops at the radius
exact; the safety margin existed because ACE's WithinUseRadius
poll missed our infrequent position updates. With per-frame
AP while moving, ACE sees us arrive the same frame.
- OnAutoWalkArrivedReSendAction: retail's Event_UseEvent
(acclient_2013_pseudo_c.txt:0x00588a80 ItemHolder::UseObject
line 403043) is single-fire, fire-and-forget. Arrival is
signaled INBOUND by the server via 0xF63E UseDone
(Handle_Item__UseDone 0x00564900) — the client doesn't
re-send. Our re-send was actively re-triggering ACE's
MoveToChain via StopExistingMoveToChains, masking the
cadence bug.
- SendAutonomousPositionNow flush: retail has no flush event;
per-frame AP while moving makes it unnecessary.
- isRetryAfterArrival flag: plumbing for the re-send;
deleted with it.
Close-range turn-first deferred Use is KEPT (it IS retail —
ACE's Player_Move.cs:66-87 Rotate(target) before callback
mirrors retail's CreateMoveToChain pre-callback rotation).
Renamed to OnAutoWalkArrivedSendDeferredAction to clarify
it's a FIRST send, not a retry.
3. WorldPicker switched to a screen-rect hit-test against the same
rect the target indicator draws. Retail's picker uses
CGfxObj.drawing_sphere + polygon refine
(acclient_2013_pseudo_c.txt:0x0054c740 GfxObjUnderSelectionRay
feeding SmartBox::GetObjectBoundingBox at 0x00452e20). Stage A
approximates the sphere-reject step by projecting Setup.SelectionSphere
to screen and hit-testing the mouse pixel against that rect
inflated by 8 px (the indicator's TriangleSize). The new
AcDream.Core.Selection.ScreenProjection helper is shared between
the picker and the indicator so their rects cannot drift.
Effect: click area matches the visible indicator extent
bit-for-bit — including the rect corners, which were dead
zones under the old ray-sphere picker. Retires the per-type
radius (1.0/1.6/2.0 m) and vertical-offset (0.2/0.9/1.0/1.5 m)
heuristic callbacks. Old ray-sphere overload remains in
WorldPicker for back-compat; not load-bearing.
4. EntityHeightFor fallback trimmed to a single 1.5 m default.
IsTallSceneryGuid deleted entirely — both became dead code
when the picker switched to screen-rect against SelectionSphere.
Test suite: 290+ Core.Net unchanged, +3 ScreenProjectionTests, +6
WorldPickerRectOverloadTests.
Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EOF
)"
```
---
## Deferred follow-ups (file as issues, not in this plan's commits)
### Task DF1: File issue — triangle apex/size UX
- [ ] **Step 1: Append issue to `docs/ISSUES.md`**
After the existing "Active issues" section, add a new entry:
```markdown
## #70 — Triangle apex/size — final retail-feel UX pass
**Status:** OPEN
**Severity:** LOW (cosmetic — indicator already retail-anchored, this is final-feel polish)
**Filed:** 2026-05-16
**Component:** ui / target indicator
**Description:** Per 2026-05-16 user feedback during the
SelectionSphere indicator ship, the triangle apex direction (flipped
to point inward at the target) and sprite size (currently 8 px legs)
are heuristic visual choices. Retail uses an actual DAT sprite from
`UIRegion::GetChild(0x1000003a/3b/3c)` — the bitmap shape and size
come from the dat, not constants.
**Acceptance:** Extract the retail triangle sprite from the dat
(probably via `tools/UiLayoutMockup` or a new `DatSpriteProbe`) and
either (a) blit the exact bitmap, or (b) pick a procedural size +
shape that matches it pixel-for-pixel at standard zoom.
**Estimated scope:** Small (~1-2 hours, mostly dat exploration).
Not blocking M1.
```
### Task DF2: File issue — picker Stage B polygon refine
- [ ] **Step 1: Append issue to `docs/ISSUES.md`**
```markdown
## #71 — WorldPicker Stage B — polygon refine for retail-accurate clicks
**Status:** OPEN
**Severity:** LOW (Stage A — sphere picker — is sufficient for M1)
**Filed:** 2026-05-16
**Component:** selection / picker
**Description:** Retail's mouse picker does two-tier sphere-then-polygon
selection (acclient_2013_pseudo_c.txt:0x0054c740
`Render::GfxObjUnderSelectionRay`):
1. Sphere reject via `CGfxObj::drawing_sphere`.
2. Polygon-accurate refine via `CPolygon::polygon_hits_ray` on every
visual polygon; closest-t polygon hit wins over any sphere hit.
Our Stage A (shipped 2026-05-16 Commit B) does sphere-only against
`Setup.SelectionSphere`. This will under-pick visible mesh that
extends beyond the sphere (creature's outstretched arm, sign edge
poking past sphere boundary) — exactly what retail's polygon refine
catches.
**Acceptance:** Pipe per-part GfxObj visual polygons through a
`PickPolygonProvider` interface (don't duplicate mesh decoding —
hook the existing `ObjectMeshManager` cached data). Two-tier in
`WorldPicker.Pick`: sphere reject → polygon scan → polygon hit
dominates sphere hit.
**Estimated scope:** Medium (~4-6 hours). Defer until visual
verification surfaces a Stage A miss in real play.
```
### Task DF3: File issue — cdb probe to confirm `omega.z = π/2` base rate
- [ ] **Step 1: Append issue to `docs/ISSUES.md`**
```markdown
## #72 — Confirm Humanoid TurnRight/TurnLeft `omega.z` base rate via cdb
**Status:** OPEN
**Severity:** LOW (current ±π/2 fallback matches all corroborating
evidence; cdb probe would settle the open question for good)
**Filed:** 2026-05-16
**Component:** physics / rotation / research
**Description:** The retail rotation rate landed in Commit A
(2026-05-16) uses `BaseTurnRateRadPerSec = π/2` based on the
documented `AnimationSequencer.cs:734-741` claim that the Humanoid
motion table ships TurnRight/TurnLeft with `HasOmega` cleared
(forcing the convention fallback). The constant has 3 corroborating
sources but the actual dat content was never dumped.
**Acceptance:** Set a cdb breakpoint on `CSequence::set_omega`
(acclient_2013_pseudo_c.txt — find exact symbol address) while
holding A or D in a retail client. Capture the `omega.z` argument
value. If `±π/2` (or numerically identical via the MotionData
formula), close as confirmed. If different, file as a regression
+ fix the constant.
**Estimated scope:** 30 min cdb session + 1 commit if confirmed,
+ small fix if different. Not blocking M1.
```
- [ ] **Step 2: Commit all 3 issue entries in one docs commit**
```bash
git add docs/ISSUES.md
git commit -m "docs: file #70 (triangle apex/size UX), #71 (picker Stage B), #72 (cdb omega.z probe)
Deferred follow-ups from the 2026-05-16 retail-faithfulness audit
(docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md)."
```
---
## Self-review checklist
**Spec coverage:**
- Fix #1 (rotation TurnRateFor + 1.5× run): Task A1 (helper + tests), A2 (keyboard), A3 (auto-walk). ✅
- Fix #4 (gate `!= 0`): Task A5 step 1. ✅
- Fix #5 (useability fallback probe): Task A4 (flag), A5 step 2 (log lines). ✅
- Fix #2 (per-tick diff-driven AP): Tasks B1 + B2 + B3. ✅
- Fix #6 (delete 4 workarounds): Task B4 (TinyMargin), B5 (handler + SendAutonomousPositionNow), B6 (isRetryAfterArrival). ✅
- Fix #3 Stage A (screen-rect picker against indicator rect): Tasks B7 + B8. ✅
- Fix #7 (trim EntityHeightFor): Task B9. ✅
- Fix #8 (delete IsTallSceneryGuid): Task B10. ✅
- Deferred issues (triangle/Stage B/cdb): Tasks DF1, DF2, DF3. ✅
**Placeholder scan:** No "TBD" / "implement later" / vague handwaves. Every code step has actual code.
**Type consistency:**
- `TurnRateFor(bool running)` defined Task A1, called Task A2 + A3. ✅
- `NotePositionSent(Vector3, uint, float)` defined B1, called B3. ✅
- `SimTimeSeconds` accessor defined B1, read B3. ✅
- `OnAutoWalkArrivedSendDeferredAction()` defined Task B6 Step 3, subscribed in Task B6 Step 4. ✅
- `ScreenProjection.TryProjectSphereToScreenRect(...)` defined Task B7 Step 3, called Task B7 Step 7 (WorldPicker.Pick) and Step 8 (TargetIndicatorPanel). ✅
- `WorldPicker.Pick(mouseX, mouseY, view, projection, viewport, candidates, skipServerGuid, sphereForEntity, inflatePixels)` defined Task B7 Step 7, called Task B8 Step 2. ✅
- `TryGetEntitySelectionSphere` referenced in Task B8 — already exists in `GameWindow.cs` at line ~9605 per audit. ✅
- `ApproxPositionEqual` defined Task B2 Step 3, called Task B2 Step 2. ✅
---
## Execution handoff
Plan saved to `docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md`. Two execution options:
1. **Subagent-Driven (recommended)** — Dispatch fresh subagent per task; review between tasks; fast iteration.
2. **Inline Execution** — Execute tasks in this session using executing-plans; batch with checkpoints for review.
**Which approach?**