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>
1939 lines
81 KiB
Markdown
1939 lines
81 KiB
Markdown
# 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, &corner1, &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 <= 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?**
|