From b5da17db767ea8da558a1c428d636d25afa736a4 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 16 May 2026 13:56:08 +0200 Subject: [PATCH] =?UTF-8?q?feat(retail):=20Commit=20B=20=E2=80=94=20retail?= =?UTF-8?q?-faithful=20AP=20cadence=20+=20screen-rect=20picker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CLAUDE.md | 46 +- docs/ISSUES.md | 168 +++++ .../2026-05-16-retail-faithfulness-fixes.md | 637 ++++++++++++++---- .../Input/PlayerMovementController.cs | 143 +++- src/AcDream.App/Rendering/GameWindow.cs | 401 ++++------- src/AcDream.App/UI/TargetIndicatorPanel.cs | 156 +---- .../Selection/ScreenProjection.cs | 86 +++ src/AcDream.Core/Selection/WorldPicker.cs | 87 +++ .../Selection/ScreenProjectionTests.cs | 64 ++ .../Selection/WorldPickerRectOverloadTests.cs | 133 ++++ 10 files changed, 1348 insertions(+), 573 deletions(-) create mode 100644 src/AcDream.Core/Selection/ScreenProjection.cs create mode 100644 tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs create mode 100644 tests/AcDream.Core.Tests/Selection/WorldPickerRectOverloadTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index b57414f..688ba12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -926,20 +926,50 @@ for the window to close. ### Logout-before-reconnect -**ACE keeps your last session alive briefly after a disconnect.** If you -relaunch the client within a few seconds of the last close, the handshake -fails with `live: session failed: CharacterList not received` and the -process exits with code 29. Wait ~3–5 seconds between launches, or explicitly -kill stale processes: +**ACE keeps your last session alive after a disconnect, and the duration +depends on HOW the client exited.** Two cases: + +1. **Graceful close (client sent logout packet to ACE):** session clears + in ~3–5 seconds. Wait briefly between launches. +2. **Hard kill (Stop-Process, crash, force-close):** no logout packet + reached ACE. ACE keeps the session marked logged-in until its own + timeout — observed in practice at ~3+ minutes. Subsequent relaunches + fail with `live: session failed: CharacterList not received` (exit 29) + the entire time. **There is no admin command available to us to kick + the stale session.** Either wait it out, or use the graceful path + below. + +**Prefer the graceful close path when ending a launch.** PowerShell's +`Stop-Process` is a hard kill — it bypasses the client's shutdown hook +which is where the logout packet would have been sent. The graceful +alternative sends WM_CLOSE so the window's close handler runs: ```powershell -Get-Process -Name AcDream.App -ErrorAction SilentlyContinue | Stop-Process -Force +$proc = Get-Process -Name AcDream.App -ErrorAction SilentlyContinue +if ($proc) { + $proc.CloseMainWindow() | Out-Null + if (-not $proc.WaitForExit(5000)) { + # Fell through to hard-kill — session WILL be stuck on ACE. + $proc | Stop-Process -Force + } +} Start-Sleep -Seconds 3 # ... then launch ... ``` -The user has repeatedly confirmed this — don't treat exit-29-after-rapid-relaunch -as a code bug. It's a server-side session-cleanup delay. +If `WaitForExit(5000)` returns false (the client didn't exit in 5 seconds +after WM_CLOSE), the client is unresponsive and a hard kill is the only +option — accept that ACE will be unhappy for a few minutes. + +**When recovering from a hard-killed session that ACE still considers +active:** the only honest answer is to wait. Don't bother retrying every +30 seconds — make a single retry attempt ~3 minutes after the kill, and +if it still fails wait another 2 minutes before trying again. The user +will likely volunteer when ACE has cleared the session if you ask. + +The user has repeatedly confirmed this — don't treat exit-29-after-relaunch +as a code bug. It's a server-side session-cleanup delay whose duration is +governed by whether the previous shutdown was graceful or forced. ### Test character diff --git a/docs/ISSUES.md b/docs/ISSUES.md index d55d87c..fecf2d9 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,6 +46,174 @@ Copy this block when adding a new issue: # Active issues +## #74 — AP cadence is per-frame-while-moving, more chatty than retail + +**Status:** OPEN +**Severity:** LOW (works; just sends ~60× the packets retail would during smooth motion) +**Filed:** 2026-05-16 +**Component:** physics / net cadence + +**Description:** The diff-driven AP cadence shipped in Commit B fires +`HeartbeatDue` on **any** position change each frame while grounded +on walkable (effective ~60 Hz during smooth movement) and a 1 Hz +heartbeat when idle. Retail's `ShouldSendPositionEvent` +(`acclient_2013_pseudo_c.txt:700233`) only sends during the +sub-interval when cell or contact-plane changes, and only sends the +1 Hz heartbeat if `(cellId, frame)` changed since `last_sent` — +truly idle = 0 Hz. So retail during continuous smooth movement is +effectively 1 Hz (cell crosses + plane changes don't happen every +frame); we are ~60 Hz. + +**Root cause / status:** Deliberate ACE-targeted choice. The +per-frame cadence is load-bearing for ACE's `WithinUseRadius` poll +to see the player arrive at a target during local speculative +auto-walk (issue #63's workaround chain). Going to 1 Hz would +re-introduce the arrival-lag bug for far-range Use/PickUp. + +**Files:** [PlayerMovementController.cs:1240-1275](src/AcDream.App/Input/PlayerMovementController.cs) +— the `HeartbeatDue = groundedOnWalkable && (positionChanged || intervalElapsed)` +gate. + +**Acceptance:** Either (a) fix issue #63 so we honor ACE's +`MoveToObject` server-side, removing the need for the per-frame +cadence, then revert to retail's `cell-or-plane-change || (interval && frame-change)` +shape (~5 LOC change); or (b) document this as a permanent +divergence and update commit messages / code comments to match. + +**Estimated scope:** Small (~5 LOC + commit-message rewrite) once +#63 is fixed. Currently blocked by #63. + +--- + +## #73 — Retail-message centralization plan — per-feature string sweeps + +**Status:** OPEN +**Severity:** LOW (per-feature work, not infrastructure) +**Filed:** 2026-05-16 +**Component:** ui / retail messages + +**Description:** Commit A added `AcDream.Core.Ui.RetailMessages` as +the home for retail-decomp-sourced UI strings (`CannotBeUsed`, +`CantBePickedUp`, `CannotPickUpCreatures`). The retail decomp has +~750 more user-facing strings we'll need over time — combat misses, +spell fizzles, vendor dialogs, "you do not have enough" etc. Rather +than bulk-port them once, port per-feature as the feature lands: +when wiring vendor purchase, sweep vendor strings into +`RetailMessages.Vendor.*`; when wiring spell-cast feedback, sweep +`RetailMessages.Spell.*`. + +**Status:** No infrastructure work pending. Pattern is established; +new strings get added to `RetailMessages.cs` with retail anchor +comments at the call site that triggered the need. + +**Files:** [RetailMessages.cs](src/AcDream.Core/Ui/RetailMessages.cs) +— class-level doc comment already describes the per-feature sweep +pattern. + +**Acceptance:** Each phase / feature that adds new user-facing +strings sweeps its retail-anchor strings into `RetailMessages` and +calls them by name rather than literal-in-place. Closing condition: +"all M1 demo strings are in RetailMessages" or similar per-milestone +gate, decided when M1 ships. + +--- + +## #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:** Commit A's rotation rate 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 — and the run-multiplier +`run_turn_factor = 1.5` at retail `0x007c8914` from +`apply_run_to_command` (decomp 0x00527be0) likewise hasn't been +verified live. + +**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 walking, then running. If `±π/2` walking and `±π/2 × 1.5 ≈ 2.356` +running, close as confirmed. If different, file as a regression and +fix the constants in +[RemoteMoveToDriver.cs](src/AcDream.Core/Physics/RemoteMoveToDriver.cs). + +**Estimated scope:** ~30 min cdb session + 1 commit if confirmed, +or +small fix if different. Not blocking M1. + +--- + +## #71 — WorldPicker Stage B — polygon refine for retail-accurate clicks + +**Status:** OPEN +**Severity:** LOW (Stage A — screen-rect 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. Per-part 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. + +Commit B's Stage A +([WorldPicker.cs](src/AcDream.Core/Selection/WorldPicker.cs)) does +screen-space rect hit-test against the projected +`Setup.SelectionSphere` (matching the indicator rect, deliberately +broader than the visible mesh polygons). Stage B would tighten clicks +to the visible mesh — under-pick what looks like empty space inside +the rect, catch visible mesh that pokes past the sphere boundary +(creature outstretched arm, sign edge). + +**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. Acceptance test: visible-mesh accuracy on +Holtburg sign, Royal Guard outstretched bow arm, inn-door wood +frame edges. + +**Estimated scope:** Medium (~4-6 hours). Defer until visual +verification surfaces a Stage A miss in real play. The user +confirmed 2026-05-16 that "I can click on longer ranges now so +good" — Stage A is enough for M1's "click an NPC" demo. + +--- + +## #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. + +**Files:** [TargetIndicatorPanel.cs](src/AcDream.App/UI/TargetIndicatorPanel.cs) +— `TriangleSize` constant + the four `AddTriangleFilled` calls. + +**Estimated scope:** Small (~1-2 hours, mostly dat exploration). +Not blocking M1. + +--- + ## #69 — Local player rotation isn't animated (no leg/arm cycle while pivoting) **Status:** OPEN diff --git a/docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md b/docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md index 36dd6b1..f4abfa4 100644 --- a/docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md +++ b/docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md @@ -18,8 +18,9 @@ - `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 use `Setup.SelectionSphere` as a simpler equivalent for Stage A). +- `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. --- @@ -40,10 +41,11 @@ | 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 use `TryGetEntitySelectionSphere` (already exists at line ~9605); drop per-type `radiusForGuid` / `verticalOffsetForGuid` callbacks. | -| `src/AcDream.Core/Selection/WorldPicker.cs` | Add new `Pick(...)` overload taking `Func sphereForEntity`. Keep existing per-radius-callback 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. | -| `tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs` (new or modify existing) | Unit tests for the new sphere-resolver overload. | +| `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`). | --- @@ -620,10 +622,14 @@ Replace with: // - 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 gates on transient_state & - // (CONTACT_TS | ON_WALKABLE_TS) — i.e., grounded on a - // walkable surface. Airborne suppresses AP entirely. - // MoveToState carries jump/fall snapshots while airborne. + // - 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. @@ -641,20 +647,17 @@ Replace with: || _lastSentCellId != CellId || !ApproxPositionEqual(_lastSentPos, _body.Position); - // Grounded-on-walkable. Retail's CONTACT_TS + ON_WALKABLE_TS - // (acclient.h:3688). Our equivalent: PhysicsBody.IsGrounded. - bool groundedOnWalkable = _body.IsGrounded; + // 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); ``` -If `PhysicsBody.IsGrounded` doesn't exist, search the codebase for the equivalent predicate: - -``` -grep -n "IsGrounded\|OnGround\|HasContact\|GroundContact" src/AcDream.Core/Physics/PhysicsBody.cs -``` - -Use whichever exists. If neither, derive from `_body.ContactPlane.Normal.Z > 0.5f` (humanoid walkable-normal threshold per retail FloorZ ≈ 0.66). Document the choice in a comment at the call site. +`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** @@ -1010,15 +1013,204 @@ Expected: 0 errors. If there are remaining "isRetryAfterArrival" references, fix --- -### Task B7: Add `WorldPicker.Pick` overload taking sphere-resolver +### 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 the failing test** +- [ ] **Step 1: Write failing tests for `ScreenProjection`** -Find or create `tests/AcDream.Core.Tests/Selection/WorldPickerTests.cs`. Add: +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; + +/// +/// Shared screen-space projection math for the target indicator and the +/// world picker. Both call into +/// so the click hit-area is guaranteed to match the visible indicator +/// rect — "what you see is what you click". +/// +/// +/// Retail equivalent: SmartBox::GetObjectBoundingBox at +/// 0x00452e20, which uses +/// Render::GetViewerBBox(selection_sphere, &corner1, &corner2) +/// 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. +/// +/// +public static class ScreenProjection +{ + /// + /// Project a world-space sphere to a screen-space axis-aligned square + /// rectangle. + /// + /// Sphere center in world space. + /// Sphere radius in world space. + /// View matrix (System.Numerics row-vector convention). + /// Projection matrix. M22 = cot(fovY/2) + /// for a standard right-handed perspective. + /// Viewport size in pixels (X = width, Y = height). + /// Out: top-left corner of the rect in viewport pixels. + /// Out: bottom-right corner of the rect in viewport pixels. + /// Out: camera-space depth (clip.W) of the sphere + /// center — use this for nearest-first sorting when multiple rects overlap. + /// Minimum side length of the rect. Distant + /// entities clamp to this so they remain pickable / visible. 12 px + /// matches the indicator's clamp floor. + /// + /// true if the sphere is in front of the camera and the rect was + /// produced; false if the center is behind the camera + /// (clip.W <= 0) or the rect is more than a screen offset + /// from the viewport (obviously off-screen). + /// + 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; @@ -1027,166 +1219,280 @@ using AcDream.Core.World; namespace AcDream.Core.Tests.Selection; -public sealed class WorldPickerSphereOverloadTests +public sealed class WorldPickerRectOverloadTests { - [Fact] - public void Pick_SphereResolver_ReturnsNearestHit() + // Same right-handed perspective as ScreenProjectionTests. + private static (Matrix4x4 view, Matrix4x4 proj, Vector2 viewport) StdCam() { - // Two entities along the +Y axis. Sphere-resolver gives each a - // tight world-space sphere centered on the entity. A ray from - // origin pointing along +Y should hit the closer entity first. - var e1 = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 5, 0), Scale = 1f }; - var e2 = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 15, 0), Scale = 1f }; + 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); + } - var origin = new Vector3(0, 0, 0); - var dir = new Vector3(0, 1, 0); - Vector3 SphereCenter(WorldEntity e) => e.Position + new Vector3(0, 0, 0.9f); + [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( - origin, dir, new[] { e1, e2 }, + mouseX: 400f, mouseY: 300f, + view, proj, viewport, + new[] { e }, skipServerGuid: 0u, - sphereForEntity: e => ((Vector3, float)?)(SphereCenter(e), 1.0f)); + sphereForEntity: x => ((Vector3, float)?)(x.Position, 1.0f), + inflatePixels: 0f); Assert.Equal(0x10001u, picked); } [Fact] - public void Pick_SphereResolver_NullSkipsCandidates() + public void Pick_RectHitTest_ReturnsNullWhenMouseOutsideRect() { - // Resolver returning null should make the picker skip the - // candidate (matches retail "no Setup → not pickable" behaviour). - var e1 = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 5, 0), Scale = 1f }; - var e2 = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 10, 0), Scale = 1f }; + var (view, proj, viewport) = StdCam(); + var e = new WorldEntity { ServerGuid = 0x10001u, Position = new Vector3(0, 0, -10), Scale = 1f }; - var origin = new Vector3(0, 0, 0); - var dir = new Vector3(0, 1, 0); + // 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( - origin, dir, new[] { e1, e2 }, + mouseX: 400f, mouseY: 300f, + view, proj, viewport, + new[] { far, near } /* deliberately reversed */, skipServerGuid: 0u, - sphereForEntity: e => e.ServerGuid == 0x10001u + 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)?)(e.Position + new Vector3(0, 0, 0.9f), 1.0f)); + : ((Vector3, float)?)(x.Position, 1.0f), + inflatePixels: 0f); Assert.Equal(0x10002u, picked); } [Fact] - public void Pick_SphereResolver_RespectsSkipServerGuid() + public void Pick_RectHitTest_RespectsSkipServerGuid() { - var e1 = new WorldEntity { ServerGuid = 0x50000001u, Position = new Vector3(0, 5, 0), Scale = 1f }; - var e2 = new WorldEntity { ServerGuid = 0x10002u, Position = new Vector3(0, 10, 0), Scale = 1f }; - - var origin = new Vector3(0, 0, 0); - var dir = new Vector3(0, 1, 0); + 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( - origin, dir, new[] { e1, e2 }, - skipServerGuid: 0x50000001u, // skip player - sphereForEntity: e => ((Vector3, float)?)(e.Position + new Vector3(0, 0, 0.9f), 1.0f)); + 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 constructor calls accordingly — check `src/AcDream.Core/World/WorldEntity.cs` for the actual constructor / object-initializer pattern. The tests assume property initializers are allowed; if not, switch to whatever constructor the type exposes. +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 2: Run tests to verify they fail** +- [ ] **Step 6: Run `WorldPicker` rect tests — confirm failure** -Run: ``` -dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~WorldPickerSphereOverloadTests" --no-build +dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~WorldPickerRectOverloadTests" --no-build ``` -Expected: build failure ("sphereForEntity parameter not found") or runtime failure. +Expected: build failure (the new `Pick` overload doesn't exist yet). -- [ ] **Step 3: Add the new `Pick` overload to `WorldPicker.cs`** +- [ ] **Step 7: Add the new `Pick` overload to `WorldPicker.cs`** -Append to `src/AcDream.Core/Selection/WorldPicker.cs` (do NOT delete the existing `Pick` overload — it stays for back-compat; its consumers are removed in Task B8): +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 /// - /// 2026-05-16. Retail-faithful picker overload. Caller supplies a - /// per-entity world-space sphere via - /// — typically scaled by entity - /// scale and rotated into world space (mirroring retail - /// CPartArray::GetSelectionSphere at 0x00518b80). Resolver returning - /// null skips the candidate (matches retail "no Setup → not pickable"). + /// 2026-05-16. Screen-space rect-hit-test picker overload. Each + /// candidate's world-space sphere (via ) + /// projects to a screen-space rectangle through + /// . The + /// rect is inflated by on every side + /// (matches the indicator's TriangleSize outer brackets) and + /// hit-tested against the mouse pixel. Among rects that contain the + /// mouse, the entity with the nearest camera-space depth wins. /// /// - /// Replaces the older - /// overload with the per-type-radius / vertical-offset heuristics. - /// Those heuristics existed because we didn't have the dat-supplied - /// SelectionSphere plumbed through. With this overload, the click - /// geometry matches what the target indicator draws — what you see - /// is what you click. + /// 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 helper with + /// TargetIndicatorPanel, the click rect and the drawn rect + /// cannot drift. /// /// /// - /// Stage A of the picker port. Retail also does a polygon-accurate - /// refine via CPolygon::polygon_hits_ray when the sphere - /// hits (decomp 0x0054c889) — that's Stage B, deferred until visual - /// testing surfaces a sphere-only miss (issue #71). + /// Resolver returning null skips the candidate (matches retail + /// "no Setup → not pickable" behavior). Entities with + /// ServerGuid == 0 (atlas-tier scenery) and the player's own + /// guid are also skipped. + /// + /// + /// + /// Stage A of the picker port. Stage B (polygon refine via + /// CPolygon::polygon_hits_ray 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. /// /// + /// Pixel inflate on each side of the + /// projected rect. Pass the indicator's TriangleSize (8 px) + /// so the click area extends to where the visible bracket corners + /// sit — the user perceives the inflated rect as the clickable area. public static uint? Pick( - System.Numerics.Vector3 origin, System.Numerics.Vector3 direction, + float mouseX, float mouseY, + System.Numerics.Matrix4x4 view, + System.Numerics.Matrix4x4 projection, + System.Numerics.Vector2 viewport, IEnumerable candidates, uint skipServerGuid, Func sphereForEntity, - float maxDistance = 50f) + float inflatePixels = 8f) { - if (direction.LengthSquared() < 1e-10f) return null; + uint? bestGuid = null; + float bestDepth = float.PositiveInfinity; - uint? bestGuid = null; - float bestT = float.PositiveInfinity; foreach (var entity in candidates) { - if (entity.ServerGuid == 0u) continue; - if (entity.ServerGuid == skipServerGuid) continue; + 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; - // Geometric ray-sphere (same math as the older overload). - var oc = origin - center; - float b = System.Numerics.Vector3.Dot(oc, direction); - float c = System.Numerics.Vector3.Dot(oc, oc) - radius * radius; - float d = b * b - c; - if (d < 0f) continue; - float sqrtD = MathF.Sqrt(d); - float t = -b - sqrtD; - if (t < 0f) t = -b + sqrtD; // ray origin inside sphere → use far exit - if (t < 0f) continue; // both roots behind ray - if (t >= maxDistance) continue; - if (t < bestT) + 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) { - bestT = t; - bestGuid = entity.ServerGuid; + bestDepth = depth; + bestGuid = entity.ServerGuid; } } return bestGuid; } ``` -- [ ] **Step 4: Run tests to verify they pass** +- [ ] **Step 8: Refactor `TargetIndicatorPanel` to use the shared helper** -Run: +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)) ``` -dotnet build tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj -c Debug -dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "FullyQualifiedName~WorldPickerSphereOverloadTests" --no-build + +(The `out _` discards the depth — the indicator doesn't need it. `minSidePixels: 12f` preserves the existing clamp.) + +- [ ] **Step 9: Build all touched projects** + ``` -Expected: 3/3 pass. +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 sphere-resolver overload +### Task B8: Switch `GameWindow` picker call to the screen-rect overload **Files:** -- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — find the `WorldPicker.Pick` call. +- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — `PickAndStoreSelection` method around line 9006. - [ ] **Step 1: Locate the existing call** @@ -1194,34 +1500,93 @@ Run: ``` grep -n "WorldPicker.Pick(" src/AcDream.App/Rendering/GameWindow.cs ``` +Expected: one match at ~line 9018. -- [ ] **Step 2: Replace the call with the sphere-resolver overload** +- [ ] **Step 2: Replace the ray-build + ray-pick chain with the screen-rect picker** -Replace the existing `WorldPicker.Pick(...)` invocation (which uses `radiusForGuid` + `verticalOffsetForGuid` callbacks) with: +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, - sphereForEntity: e => - TryGetEntitySelectionSphere(e.ServerGuid, out var c, out var r) - ? ((System.Numerics.Vector3, float)?)(c, r) - : ((System.Numerics.Vector3, float)?)null, - maxDistance: 50f); + 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 (around line ~9037 and ~9054) become dead with this change. Delete them. +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** -Run: ``` dotnet build src/AcDream.App/AcDream.App.csproj -c Debug ``` -Expected: 0 errors. The old `WorldPicker.Pick` overload with `radiusForGuid` callback still exists in `WorldPicker.cs` but has no callers — leave it for now; not blocking. +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. --- @@ -1332,6 +1697,8 @@ User runs the client and confirms: 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.** @@ -1344,11 +1711,13 @@ 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 + sphere picker retires 4 workarounds +fix(retail): per-tick AP cadence + screen-rect picker retires 4 workarounds Single coherent commit. Audit findings from 2026-05-16: @@ -1390,20 +1759,29 @@ Single coherent commit. Audit findings from 2026-05-16: Renamed to OnAutoWalkArrivedSendDeferredAction to clarify it's a FIRST send, not a retry. -3. WorldPicker switched to a Setup.SelectionSphere overload. - Retail's picker uses CGfxObj.drawing_sphere + polygon refine - (acclient_2013_pseudo_c.txt:0x0054c740 GfxObjUnderSelectionRay), - which we approximate via Setup.SelectionSphere (same data - path as the target indicator since f4f4143). Effect: click - geometry matches the visible indicator — what you see is what - you click. Retires the per-type radius (1.0/1.5/2.0 m) and - vertical-offset (0.9/1.0/1.5 m) heuristic callbacks. +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 SelectionSphere. + when the picker switched to screen-rect against SelectionSphere. -Test suite: 290+ Core.Net unchanged, +3 WorldPickerSphereOverloadTests. +Test suite: 290+ Core.Net unchanged, +3 ScreenProjectionTests, +6 +WorldPickerRectOverloadTests. Plan: docs/superpowers/plans/2026-05-16-retail-faithfulness-fixes.md @@ -1532,7 +1910,7 @@ Deferred follow-ups from the 2026-05-16 retail-faithfulness audit - 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 (sphere picker): Tasks B7 + B8. ✅ +- 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. ✅ @@ -1544,7 +1922,8 @@ Deferred follow-ups from the 2026-05-16 retail-faithfulness audit - `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. ✅ -- `WorldPicker.Pick(...sphereForEntity...)` defined Task B7, called Task B8. ✅ +- `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. ✅ diff --git a/src/AcDream.App/Input/PlayerMovementController.cs b/src/AcDream.App/Input/PlayerMovementController.cs index 5a126b8..d42166c 100644 --- a/src/AcDream.App/Input/PlayerMovementController.cs +++ b/src/AcDream.App/Input/PlayerMovementController.cs @@ -186,10 +186,32 @@ public sealed class PlayerMovementController // (2026-05-01 motion-trace findings.md): retail sends ~1 Hz at rest, // not the 5 Hz our pre-fix code used. Sending at 5 Hz was harmless // but wasteful and probably looked like jitter to observers. - private float _heartbeatAccum; - public const float HeartbeatInterval = 1.0f; // 1 sec — retail / holtburger + /// + /// 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. + /// + 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; } + /// Sim-time accumulator (advanced by dt at the top of Update). + /// Exposed for the network outbound layer to stamp NotePositionSent. + public float SimTimeSeconds => _simTimeSeconds; + // L.5 retail physics-tick gate (2026-04-30). // // Retail's CPhysicsObj::update_object subdivides per-frame dt into @@ -405,6 +427,27 @@ public sealed class PlayerMovementController AutoWalkArrived?.Invoke(); } + /// + /// 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). + /// + public void NotePositionSent(System.Numerics.Vector3 worldPos, + uint cellId, + float nowSeconds) + { + _lastSentPos = worldPos; + _lastSentCellId = cellId; + _lastSentTime = nowSeconds; + _lastSentInitialized = true; + } + /// /// B.6 slice 2 (2026-05-14). If a server-initiated auto-walk is /// active, either cancel it (user pressed a movement key) or @@ -463,11 +506,17 @@ public sealed class PlayerMovementController float arrivalThreshold = _autoWalkMoveTowards ? _autoWalkDistanceToObject : _autoWalkMinDistance; - const float TinyMargin = 0.05f; - float effectiveArrival = MathF.Max(arrivalThreshold - TinyMargin, 0.1f); + // 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 <= effectiveArrival) + && dist <= arrivalThreshold) || (!_autoWalkMoveTowards && dist >= arrivalThreshold + RemoteMoveToDriver.ArrivalEpsilon); @@ -613,6 +662,8 @@ public sealed class PlayerMovementController public MovementResult Update(float dt, MovementInput input) { + _simTimeSeconds += dt; + // B.6 slice 2 (2026-05-14): server-initiated auto-walk overlay. // When _autoWalkActive, steer Yaw toward _autoWalkDestination and // synthesize Forward+Run input so the rest of Update runs the @@ -1192,31 +1243,46 @@ public sealed class PlayerMovementController } // ── 8. Heartbeat timer (always while in-world, not just while moving) ─ - // Holtburger fires AutonomousPosition heartbeat at 1 Hz regardless of - // motion state (gated only by has_autonomous_position_sync_target). - // Retail's CommandInterpreter::SendPositionEvent gates on - // transient_state (Contact + OnWalkable + valid Position), not on - // motion. The pre-fix isMoving gate stopped acdream from heart-beating - // at rest, which left observers with stale last-known positions during - // long idle periods. PortalSpace (handled at the top of Update via - // early return) skips Update entirely, so reaching this line implies - // we're in a valid in-world pose. - _heartbeatAccum += dt; - // B.6+B.7 (2026-05-15): bump heartbeat from 1 Hz to ~10 Hz while - // the body is actively moving (auto-walk OR user pressing W/A/S/D). - // ACE's server-side CreateMoveToChain polls WithinUseRadius every - // ~0.1 s using the latest Player.Location; 1 Hz heartbeats leave - // up to 1 s of stale position data on the server, which meant - // ACE's MoveToChain rejected our re-sent Use action as still - // out-of-range. With 10 Hz updates ACE sees us approaching in - // ~real-time and the server-side chain converges normally — - // retires the arrival-margin / re-send / flush-AP workarounds. - bool activelyMoving = _autoWalkActive - || input.Forward || input.Backward - || input.StrafeLeft || input.StrafeRight; - float effectiveInterval = activelyMoving ? 0.1f : HeartbeatInterval; - HeartbeatDue = _heartbeatAccum >= effectiveInterval; - if (HeartbeatDue) _heartbeatAccum = 0f; + // 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_TS + ON_WALKABLE_TS + // (acclient.h:3688). Our equivalent: PhysicsBody.InContact && + // PhysicsBody.OnWalkable (both map to TransientStateFlags bits 0+1 + // which are set together by ResolveWithTransition on walkable ground). + bool groundedOnWalkable = _body.InContact && _body.OnWalkable; + + HeartbeatDue = groundedOnWalkable && (positionChanged || intervalElapsed); // K-fix5 (2026-04-26): local-animation-cycle pacing. Visual rate // should match the actual movement speed. For Forward+Run this is @@ -1256,4 +1322,21 @@ public sealed class PlayerMovementController JumpExtent: outJumpExtent, JumpVelocity: outJumpVelocity); } + + /// + /// 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. + /// + 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; + } } diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 457ac11..40b2f61 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -791,13 +791,11 @@ public sealed class GameWindow : IDisposable // Current selection: written by Q-cycle (combat) and LMB click (interact); cleared on entity despawn. private uint? _selectedGuid; - // B.6+B.7 (2026-05-15): pending action that triggered an auto-walk. - // When the local body arrives at the auto-walk target, - // OnAutoWalkArrivedReSendAction re-sends the action close-range so - // ACE completes it via WithinUseRadius even if its server-side - // MoveToChain already timed out. Cleared before each re-send to - // prevent infinite loops (the re-sent action's auto-walk would - // arrive immediately at the same position, infinite re-fire). + // B.6/B.7 (2026-05-16): pending close-range action that will be fired + // once the local auto-walk overlay reports arrival (body has finished + // rotating to face the target). Only set for close-range Use/PickUp; + // far-range sends fire the wire packet immediately at SendUse/SendPickUp + // time. Cleared before the deferred send fires — single-fire, no retry. private (uint Guid, bool IsPickup)? _pendingPostArrivalAction; private readonly record struct LiveEntityInfo( string? Name, @@ -6445,6 +6443,14 @@ public sealed class GameWindow : IDisposable DumpMovementTruthOutbound( "MTS", seq, result, wirePos, wireCellId, contactByte); _liveSession.SendGameAction(body); + // B.6/B.7 (2026-05-16): stamp the diff-driven heartbeat clock so + // HeartbeatDue resets its interval from THIS send — mirrors retail's + // SendMovementEvent (acclient_2013_pseudo_c.txt:0x006b4680) writing + // last_sent_position_time + last_sent_position after each MTS send. + _playerController.NotePositionSent( + worldPos: _playerController.Position, + cellId: _playerController.CellId, + nowSeconds: _playerController.SimTimeSeconds); } if (_playerController.HeartbeatDue) @@ -6463,6 +6469,14 @@ public sealed class GameWindow : IDisposable DumpMovementTruthOutbound( "AP", seq, result, wirePos, wireCellId, contactByte); _liveSession.SendGameAction(body); + // B.6/B.7 (2026-05-16): stamp the diff-driven heartbeat clock so + // HeartbeatDue resets its interval from THIS send — mirrors retail's + // SendPositionEvent (acclient_2013_pseudo_c.txt:700345-700348) + // writing last_sent_position_time + last_sent_position after each AP. + _playerController.NotePositionSent( + worldPos: _playerController.Position, + cellId: _playerController.CellId, + nowSeconds: _playerController.SimTimeSeconds); } if (result.JumpExtent.HasValue && result.JumpVelocity.HasValue) @@ -9007,69 +9021,41 @@ public sealed class GameWindow : IDisposable { if (_cameraController is null || _window is null) return; + // 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 (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 viewport = new System.Numerics.Vector2((float)_window.Size.X, (float)_window.Size.Y); var picked = AcDream.Core.Selection.WorldPicker.Pick( - origin, direction, - _entitiesByServerGuid.Values, + mouseX: _lastMouseX, mouseY: _lastMouseY, + view: camera.View, projection: camera.Projection, + viewport: viewport, + candidates: _entitiesByServerGuid.Values, skipServerGuid: _playerServerGuid, - maxDistance: 50f, - // B.7 (2026-05-15): widen the pick sphere for large flat - // objects (doors, lifestones, portals, corpses) so their - // visible surface stays clickable even though the entity - // origin is a single point. 0.7 m default is fine for - // humanoids and most items; doors / portals need ~2 m - // to cover the doorframe. - // - // 2026-05-15 sign-class extension: post-mounted scenery - // (Holtburg town sign etc.) needs the sphere TALLER than - // wider. We classify "non-creature, non-flat, non-small-item" - // as tall scenery and grow the sphere to 1.6 m radius lifted - // to 1.5 m vertical offset — covers a 3 m post from - // ground to top. Mirrors TargetIndicatorPanel.EntityHeightFor's - // 3 m default so the click sphere matches the visible box. - radiusForGuid: g => - { - if (_lastSpawnByGuid.TryGetValue(g, out var s) - && s.ObjectDescriptionFlags is { } odf) - { - // BF_DOOR = 0x1000, BF_LIFESTONE = 0x4000, - // BF_PORTAL = 0x40000, BF_CORPSE = 0x2000 - // (acclient.h:6431-6463) - const uint LargeFlatMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u; - if ((odf & LargeFlatMask) != 0) return 2.0f; - } - if (IsTallSceneryGuid(g)) return 1.6f; - // 1.0 m sphere centred at chest height (see - // verticalOffsetForGuid) covers a 1.8 m humanoid from - // shin to crown without overlapping neighbours. - return 1.0f; - }, - verticalOffsetForGuid: g => - { - // Lift the pick sphere to mid-body so chest/head clicks - // hit instead of missing past the top of a feet-anchored - // sphere. WorldEntity.Position is at feet level - // (Z=ground); humanoids reach ~1.8 m, items sit close - // to the ground (~0.2 m above their feet). - if (_liveEntityInfoByGuid.TryGetValue(g, out var info) - && (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0) - return 0.9f; // humanoid mid-body - if (_lastSpawnByGuid.TryGetValue(g, out var s) - && s.ObjectDescriptionFlags is { } odf) - { - const uint LargeFlatMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u; - if ((odf & LargeFlatMask) != 0) return 1.0f; // mid-door - } - if (IsTallSceneryGuid(g)) return 1.5f; // mid-pole height - return 0.2f; // small ground item — sphere just above feet - }); + // Resolver: Setup's SelectionSphere is the ONLY input. If the + // entity's Setup didn't bake a SelectionSphere, return null — + // the picker skips it, which matches retail behaviour + // (Render::GfxObjUnderSelectionRay at 0x0054c740 skips + // candidates with no drawing_sphere data). Earlier defensive + // 1.5 m × scale synth was removed 2026-05-16 — it made + // dat-incomplete entities click as phantom hitboxes the size + // of an NPC, diverging from retail and masking real Setup- + // loading bugs. + sphereForEntity: e => + TryGetEntitySelectionSphere(e.ServerGuid, out var c, out var r) + ? ((System.Numerics.Vector3, float)?)(c, r) + : null, + // 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); if (picked is uint guid) { @@ -9102,7 +9088,7 @@ public sealed class GameWindow : IDisposable string radStr = pickUseRadius.HasValue ? pickUseRadius.Value.ToString("F2", System.Globalization.CultureInfo.InvariantCulture) : "null"; string setupStr = pickSetupId.HasValue ? $"0x{pickSetupId.Value:X8}" : "null"; 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})")); + $"[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})")); _debugVm?.AddToast($"Selected: {label}"); if (useImmediately) SendUse(guid); } @@ -9161,7 +9147,7 @@ public sealed class GameWindow : IDisposable else SendUse(sel); } - private void SendUse(uint guid, bool isRetryAfterArrival = false) + private void SendUse(uint guid) { if (_liveSession is null || _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld) @@ -9169,69 +9155,64 @@ public sealed class GameWindow : IDisposable _debugVm?.AddToast("Not in world"); return; } - // 2026-05-15: defense-in-depth useability gate. Double-click flows - // directly through SendUse without passing UseCurrentSelection's - // dispatcher gate, so re-check here. Silent ignore matches retail - // (acclient.h:6478 ITEM_USEABLE — USEABLE_REMOTE bit required). - // isRetryAfterArrival bypasses the gate because we only retry an - // action we previously gated through. - if (!isRetryAfterArrival && !IsUseableTarget(guid)) + + // Retail-faithful useability gate (acclient_2013_pseudo_c.txt:256455 + // ItemUses::IsUseable). Signs / banners with useability=0 silently + // ignore Use. + if (!IsUseableTarget(guid)) { - // Retail-style client-side toast for unusable targets - // (signs, decorative scenery with USEABLE_NO / USEABLE_UNDEF). - // Retail string at acclient_2013_pseudo_c.txt:1033115 - // (data_7e2a70): "The %s cannot be used" (no trailing period). - string label = DescribeLiveEntity(guid); - _debugVm?.AddToast(AcDream.Core.Ui.RetailMessages.CannotBeUsed(label)); 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 a speculative auto-walk on the local - // player toward the target. For far targets ACE will overwrite - // this with its own MovementType=6 wire payload (and a better - // wire-supplied use-radius). For close-range targets ACE skips - // MoveToChain entirely and just rotates server-side; our - // overlay provides the matching local rotation. Either way the - // alignment-gated arrival ensures the body finishes facing - // the target before stopping. + + // 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. // - // User feedback (2026-05-15): 'first is rotation, when you are - // facing, then using.' For close-range we DEFER the wire packet - // until our local overlay arrives (turn-then-fire). The - // _pendingPostArrivalAction handler will re-fire SendUse with - // isRetryAfterArrival=true after the body finishes turning. - // For far range we still send immediately so ACE can start - // its MoveToChain. - if (!isRetryAfterArrival) + // 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) { - InstallSpeculativeTurnToTarget(guid); + // Defer the wire packet — OnAutoWalkArrivedSendDeferredAction + // will fire it after rotation completes. _pendingPostArrivalAction = (guid, false); - if (IsCloseRangeTarget(guid)) - { - if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) - Console.WriteLine($"[B.4b] use deferred (close-range, turn-first) guid=0x{guid:X8}"); - return; // wait for AutoWalkArrived to fire the wire send - } + if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) + Console.WriteLine($"[B.4b] use deferred (close-range, turn-first) guid=0x{guid:X8}"); + return; } + // Far range: send Use immediately so ACE has the request, + // AND queue an arrival re-send. Issue #63 (server-initiated + // MoveToObject not honored) means ACE's first-Use response + // is dropped as too-far and ACE doesn't re-poll + // WithinUseRadius when the speculative local walk gets us in + // range. The arrival re-send fires a second Use packet once + // the body reaches the target — at which point ACE accepts + // and executes the action. The retail-faithful path is to + // honor MoveToObject and let ACE complete the Use server- + // side; until #63 lands, this client-side retry is the + // workaround that keeps far-range Use working. 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}"); + _pendingPostArrivalAction = (guid, false); + Console.WriteLine($"[B.4b] use guid=0x{guid:X8} seq={seq} (queued for arrival re-send pending #63)"); 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}")); } - // Remember this action so OnAutoWalkArrivedReSendAction can - // re-fire it close-range. Skip when this IS the re-send. - if (!isRetryAfterArrival) - _pendingPostArrivalAction = (guid, false); } - private void SendPickUp(uint itemGuid, bool isRetryAfterArrival = false) + private void SendPickUp(uint itemGuid) { if (_liveSession is null || _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld) @@ -9245,7 +9226,7 @@ public sealed class GameWindow : IDisposable // "cannot pick up creatures!" message instead of the generic // "can't be picked up!". // Retail string acclient_2013_pseudo_c.txt:401642 (data_7e22b4). - if (!isRetryAfterArrival && IsLiveCreatureTarget(itemGuid)) + if (IsLiveCreatureTarget(itemGuid)) { _debugVm?.AddToast(AcDream.Core.Ui.RetailMessages.CannotPickUpCreatures); if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) @@ -9256,7 +9237,7 @@ public sealed class GameWindow : IDisposable // Generic non-pickupable gate (signs, banners, decorative scenery). // Retail string acclient_2013_pseudo_c.txt:401589 (sprintf // "The %s can't be picked up!"). - if (!isRetryAfterArrival && !IsPickupableTarget(itemGuid)) + if (!IsPickupableTarget(itemGuid)) { string label = DescribeLiveEntity(itemGuid); _debugVm?.AddToast(AcDream.Core.Ui.RetailMessages.CantBePickedUp(label)); @@ -9264,66 +9245,75 @@ public sealed class GameWindow : IDisposable Console.WriteLine($"[B.5] SendPickUp ignored — not pickupable item=0x{itemGuid:X8}"); return; } + // B.6 (2026-05-15): same speculative turn-to-target + deferral as // SendUse — close-range pickup rotates locally to face the // item first, then the wire packet fires when the local // overlay reports arrival. - if (!isRetryAfterArrival) + // + // 2026-05-16: simplified — FIRST send on arrival, not a retry. + bool closeRange = IsCloseRangeTarget(itemGuid); + InstallSpeculativeTurnToTarget(itemGuid); + + if (closeRange) { - InstallSpeculativeTurnToTarget(itemGuid); _pendingPostArrivalAction = (itemGuid, true); - if (IsCloseRangeTarget(itemGuid)) - { - if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) - Console.WriteLine($"[B.5] pickup deferred (close-range, turn-first) item=0x{itemGuid:X8}"); - return; - } + if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) + Console.WriteLine($"[B.5] pickup deferred (close-range, turn-first) item=0x{itemGuid:X8}"); + return; } + // Far range: same arrival-retry pattern as SendUse — fire + // PickUp immediately AND queue for arrival re-send. ACE's + // first PickUp is dropped if we're outside the use-radius + // and isn't re-polled (issue #63 workaround). 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}"); + _pendingPostArrivalAction = (itemGuid, true); + Console.WriteLine($"[B.5] pickup item=0x{itemGuid:X8} container=0x{_playerServerGuid:X8} seq={seq} (queued for arrival re-send pending #63)"); 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}")); } - // Remember this action so OnAutoWalkArrivedReSendAction can - // re-fire it close-range. Skip when this IS the re-send. - if (!isRetryAfterArrival) - _pendingPostArrivalAction = (itemGuid, true); } /// - /// B.6+B.7 (2026-05-15). Fires when - /// signals natural arrival at an auto-walk target. Force-flushes - /// the player's current authoritative position to ACE first, then - /// re-sends the Use/PickUp. Without the position flush, ACE - /// processes the re-sent Use before the next per-frame - /// AutonomousPosition heartbeat — so ACE's Player.Location is - /// still stale (back where the auto-walk started) and ACE replies - /// with another MoveToObject instead of completing the action. + /// 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 / time + /// and never touch _pendingPostArrivalAction. /// - private void OnAutoWalkArrivedReSendAction() + private void OnAutoWalkArrivedSendDeferredAction() { if (_pendingPostArrivalAction is not (uint guid, bool isPickup) pending) return; - // Clear FIRST to break any retry loop. _pendingPostArrivalAction = null; - // Send a fresh AutonomousPosition NOW so ACE's server-side - // Player.Location updates to our arrived position before ACE - // sees the re-sent action. - SendAutonomousPositionNow(); + if (_liveSession is null + || _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld) + return; - if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) - Console.WriteLine(System.FormattableString.Invariant( - $"[autowalk-arrived-resend] guid=0x{guid:X8} isPickup={isPickup}")); - if (isPickup) SendPickUp(guid, isRetryAfterArrival: true); - else SendUse(guid, isRetryAfterArrival: true); + 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}"); + } } /// @@ -9421,48 +9411,6 @@ public sealed class GameWindow : IDisposable walkRunThreshold: 15.0f); } - /// - /// B.6+B.7 (2026-05-15). Send an out-of-frame AutonomousPosition - /// packet using the controller's current authoritative state. - /// Used to flush position to ACE on auto-walk arrival before - /// re-sending the Use/PickUp action; without it, ACE's - /// Player.Location is the pre-walk position and the action - /// resolves out-of-range. - /// - private void SendAutonomousPositionNow() - { - if (_liveSession is null - || _liveSession.CurrentState != AcDream.Core.Net.WorldSession.State.InWorld - || _playerController is null) - return; - - var pos = _playerController.Position; - int lbX = _liveCenterX + (int)MathF.Floor(pos.X / 192f); - int lbY = _liveCenterY + (int)MathF.Floor(pos.Y / 192f); - float localX = pos.X - (lbX - _liveCenterX) * 192f; - float localY = pos.Y - (lbY - _liveCenterY) * 192f; - uint wireCellId = ((uint)lbX << 24) | ((uint)lbY << 16) | (_playerController.CellId & 0xFFFFu); - var wirePos = new System.Numerics.Vector3(localX, localY, pos.Z); - var wireRot = YawToAcQuaternion(_playerController.Yaw); - byte contactByte = _playerController.IsAirborne ? (byte)0 : (byte)1; - - var seq = _liveSession.NextGameActionSequence(); - var body = AcDream.Core.Net.Messages.AutonomousPosition.Build( - gameActionSequence: seq, - cellId: wireCellId, - position: wirePos, - rotation: wireRot, - instanceSequence: _liveSession.InstanceSequence, - serverControlSequence: _liveSession.ServerControlSequence, - teleportSequence: _liveSession.TeleportSequence, - forcePositionSequence: _liveSession.ForcePositionSequence, - lastContact: contactByte); - _liveSession.SendGameAction(body); - if (AcDream.Core.Physics.PhysicsDiagnostics.ProbeAutoWalkEnabled) - Console.WriteLine(System.FormattableString.Invariant( - $"[autowalk-flush-ap] seq={seq} cell=0x{wireCellId:X8} pos=({wirePos.X:F2},{wirePos.Y:F2},{wirePos.Z:F2})")); - } - private uint? SelectClosestCombatTarget(bool showToast) { if (!_entitiesByServerGuid.TryGetValue(_playerServerGuid, out var playerEntity)) @@ -9515,74 +9463,6 @@ public sealed class GameWindow : IDisposable return (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0; } - /// - /// 2026-05-15. True when the entity is "tall scenery" — has a known - /// non-zero ItemType that is NOT in the small-carry-item mask AND - /// has no door/lifestone/portal/corpse PWD bits AND is not a - /// creature. The Holtburg town sign is the canonical example: a - /// 3 m post-mounted entity that needs the pick sphere lifted to - /// mid-pole with a wider radius so the user can click any part of - /// the visible mesh, not just the pole base. - /// - /// - /// Mirrors 's - /// classification — both fall into the "everything else: 3 m default" - /// branch — so the visible indicator box and the click sphere - /// match. - /// - /// - private bool IsTallSceneryGuid(uint guid) - { - // Creatures are never "tall scenery" — picker uses humanoid sphere. - if (_liveEntityInfoByGuid.TryGetValue(guid, out var info) - && (info.ItemType & AcDream.Core.Items.ItemType.Creature) != 0) - return false; - - if (!_lastSpawnByGuid.TryGetValue(guid, out var spawn)) - return false; - - // Doors / lifestones / portals / corpses → LargeFlatMask branch. - if (spawn.ObjectDescriptionFlags is { } odf) - { - const uint LargeFlatMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u; - if ((odf & LargeFlatMask) != 0) return false; - } - - uint it = spawn.ItemType ?? 0u; - - // 2026-05-15 — useability-based discriminator. Mirrors - // TargetIndicatorPanel.EntityHeightFor exactly so the click - // sphere matches the visible box. A small-item ItemType is - // a REAL pickup item only if it is also useable from the - // world (USEABLE_REMOTE bit, acclient.h:6478). Otherwise - // (Misc + USEABLE_UNDEF — the Holtburg sign case) it's tall - // scenery and gets the lifted+widened sphere. - const uint USEABLE_REMOTE_BIT = 0x20u; - bool useableFromWorld = spawn.Useability is uint u - && (u & USEABLE_REMOTE_BIT) != 0; - - const uint SmallItemMask = - (uint)(AcDream.Core.Items.ItemType.MeleeWeapon - | AcDream.Core.Items.ItemType.Armor - | AcDream.Core.Items.ItemType.Clothing - | AcDream.Core.Items.ItemType.Jewelry - | AcDream.Core.Items.ItemType.Food - | AcDream.Core.Items.ItemType.Money - | AcDream.Core.Items.ItemType.Misc - | AcDream.Core.Items.ItemType.MissileWeapon - | AcDream.Core.Items.ItemType.Container - | AcDream.Core.Items.ItemType.Gem - | AcDream.Core.Items.ItemType.SpellComponents - | AcDream.Core.Items.ItemType.Writable - | AcDream.Core.Items.ItemType.Key - | AcDream.Core.Items.ItemType.Caster); - // Real pickup item: small-item-class AND useable. NOT tall scenery. - if ((it & SmallItemMask) != 0 && useableFromWorld) return false; - - // Everything else (signs / banners / untyped scenery / - // Misc-typed-but-non-useable): tall scenery. - return true; - } /// /// 2026-05-16 — retail-faithful port of @@ -9955,15 +9835,10 @@ public sealed class GameWindow : IDisposable _playerController = new AcDream.App.Input.PlayerMovementController(_physicsEngine); - // B.6+B.7 (2026-05-15): re-send the Use/PickUp action on local - // auto-walk arrival. ACE's server-side MoveToChain may have - // already timed out by the time the local body arrives (we - // walk locally but don't send tracking position updates to - // ACE during the walk yet, so ACE's WithinUseRadius check may - // never have passed). Resending close-range hits ACE's - // CreateMoveToChain WithinUseRadius shortcut (Player_Move.cs:66) - // and completes the action immediately. - _playerController.AutoWalkArrived += OnAutoWalkArrivedReSendAction; + // B.6/B.7 (2026-05-16): fire the deferred close-range Use/PickUp + // action (first send, not a retry) when the local auto-walk overlay + // reports arrival (body finished rotating to face the target). + _playerController.AutoWalkArrived += OnAutoWalkArrivedSendDeferredAction; // K-fix7 (2026-04-26): if PlayerDescription already arrived, the // server's Run / Jump skill values are cached here — push them diff --git a/src/AcDream.App/UI/TargetIndicatorPanel.cs b/src/AcDream.App/UI/TargetIndicatorPanel.cs index c65d249..364bbc1 100644 --- a/src/AcDream.App/UI/TargetIndicatorPanel.cs +++ b/src/AcDream.App/UI/TargetIndicatorPanel.cs @@ -84,89 +84,18 @@ public sealed class TargetIndicatorPanel public float EntityHeight { get; set; } = 1.8f; /// - /// Resolve the world-space height to use for a given entity's - /// indicator box. The base height per type is multiplied by the - /// entity's so an upscaled sign or NPC - /// gets a proportionally bigger box. - /// - /// Per-type base height: - /// - /// Creature (NPC / monster / player): 1.8 m (humanoid) - /// Door / Lifestone / Portal: 2.4 m (door-frame tall) - /// Small carry items (Money, Food, Gem, SpellComponents, - /// Misc, Weapons, Armour, Clothing, Jewelry, Container): - /// 0.8 m (item dropped on the ground) - /// Everything else (signs on a pole, generic tall scenery, - /// untyped scenery interactables): 3.0 m (post-on-ground - /// tall — bumped from 1.5 m on 2026-05-15 because the - /// Holtburg sign was getting a tiny pole-only box. Most - /// non-typed non-flat AC scenery is either small-item-on- - /// ground (handled above) or post-mounted; 3 m is the - /// right midpoint for the post case. Scale > 1 grows - /// the box proportionally.) - /// - /// - /// - /// Future refinement (deferred): read the entity's actual mesh - /// AABB at registration time and use the projected silhouette - /// for an exact-fit box. - /// - /// already caches per-GfxObj AABBs; combining them across a - /// multi-part Setup gives the entity-level bounds we'd want. - /// + /// 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. /// public float EntityHeightFor(uint itemType, uint pwdBitfield, float scale, uint? useability = null) { - if (scale <= 0f) scale = 1f; // defensive - bool isCreature = (itemType & (uint)AcDream.Core.Items.ItemType.Creature) != 0; - if (isCreature) return 1.8f * scale; - - // BF_DOOR = 0x1000, BF_LIFESTONE = 0x4000, BF_PORTAL = 0x40000, - // BF_CORPSE = 0x2000 (acclient.h:6431-6463). - const uint TallStructureMask = 0x1000u | 0x4000u | 0x40000u | 0x2000u; - if ((pwdBitfield & TallStructureMask) != 0) return 2.4f * scale; - - // 2026-05-15 — KEY DISCRIMINATOR. Misc-class ItemTypes are - // ambiguous in retail: dropped jewellery / coins / food / tapers - // are Misc, but so are signs, banners, and decorative scenery. - // ACE distinguishes the two via ITEM_USEABLE (acclient.h:6478): - // a real pickup item has USEABLE_REMOTE (0x20) set; a sign has - // USEABLE_UNDEF (0). If we know useability and it lacks - // USEABLE_REMOTE, treat the entity as tall scenery regardless - // of ItemType. This is what fixes the Holtburg town sign - // showing a tiny pole-base box. - const uint USEABLE_REMOTE_BIT = 0x20u; - bool useableFromWorld = useability is uint u - && (u & USEABLE_REMOTE_BIT) != 0; - - // Small carry items: weapons / armour / clothing / jewellery / - // money / food / misc / weapons / containers / gems / spell - // components / writable / keys / casters / lockables. - const uint SmallItemMask = - (uint)(AcDream.Core.Items.ItemType.MeleeWeapon - | AcDream.Core.Items.ItemType.Armor - | AcDream.Core.Items.ItemType.Clothing - | AcDream.Core.Items.ItemType.Jewelry - | AcDream.Core.Items.ItemType.Food - | AcDream.Core.Items.ItemType.Money - | AcDream.Core.Items.ItemType.Misc - | AcDream.Core.Items.ItemType.MissileWeapon - | AcDream.Core.Items.ItemType.Container - | AcDream.Core.Items.ItemType.Gem - | AcDream.Core.Items.ItemType.SpellComponents - | AcDream.Core.Items.ItemType.Writable - | AcDream.Core.Items.ItemType.Key - | AcDream.Core.Items.ItemType.Caster); - - // Real pickup item: ItemType is small-item-class AND the server - // marked it useable from the world. 0.8 m × scale box. - if ((itemType & SmallItemMask) != 0 && useableFromWorld) return 0.8f * scale; - - // Tall scenery: anything else (signs, banners, untyped - // post-mounted objects, AND Misc-typed-but-non-useable entities - // like the Holtburg sign). 3 m × scale covers a typical - // post-mounted sign from ground to top. - return 3.0f * scale; + if (scale <= 0f) scale = 1f; + return 1.5f * scale; } /// @@ -214,8 +143,10 @@ public sealed class TargetIndicatorPanel if (info.WorldSphereCenter is Vector3 sphereCenter && info.WorldSphereRadius is float sphereRadius - && TryComputeScreenRectFromSphere(sphereCenter, sphereRadius, view, projection, viewport, - out var rMin, out var rMax)) + && AcDream.Core.Selection.ScreenProjection.TryProjectSphereToScreenRect( + sphereCenter, sphereRadius, view, projection, viewport, + out var rMin, out var rMax, out _, + minSidePixels: 12f)) { // 2026-05-16 — retail-faithful path per // SmartBox::GetObjectBoundingBox (decomp 0x00452e20). @@ -294,67 +225,6 @@ public sealed class TargetIndicatorPanel drawList.AddTriangleFilled(bl + new Vector2( t, -t), bl + new Vector2( t, 0), bl + new Vector2(0, -t), col); } - /// - /// 2026-05-16. Project a world-space sphere (center + radius) as a - /// screen-space square and return its bounding rectangle. Matches - /// retail SmartBox::GetObjectBoundingBox (decomp - /// 0x00452e20) which uses - /// Render::GetViewerBBox(sphere, &corner1, &corner2) - /// to compute a camera-aligned BBox of the sphere then projects - /// the 2 corner points. - /// - /// - /// Mathematical equivalent (faster, no per-corner reprojection): - /// project the sphere center to screen, then compute the - /// screen-space radius as - /// worldRadius * projection.M22 * viewport.Y / (2 * clip.W) - /// where M22 = 1/tan(fovY/2) for a standard right-handed - /// perspective. The rect is centered at the projected sphere - /// center with side length 2 * screenRadius. - /// - /// - /// - /// Returns false if the sphere center is behind the camera - /// (clip.W <= 0). - /// - /// - private static bool TryComputeScreenRectFromSphere( - Vector3 worldCenter, float worldRadius, - Matrix4x4 view, Matrix4x4 projection, Vector2 viewport, - out Vector2 rectMin, out Vector2 rectMax) - { - rectMin = default; - rectMax = default; - - var viewProj = view * projection; - var clip = Vector4.Transform(new Vector4(worldCenter, 1f), viewProj); - if (clip.W <= 0.001f) return false; - - 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 (positive in front of camera for - // standard right-handed perspective). - 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 MinSide so distant entities still get a visible indicator. - const float MinSide = 12f; - if (screenRadius < MinSide * 0.5f) screenRadius = MinSide * 0.5f; - - rectMin = new Vector2(screenX - screenRadius, screenY - screenRadius); - rectMax = new Vector2(screenX + screenRadius, screenY + screenRadius); - return true; - } - /// /// Project a world-space point to screen-space pixels. Returns /// false if the point is behind the camera or outside the diff --git a/src/AcDream.Core/Selection/ScreenProjection.cs b/src/AcDream.Core/Selection/ScreenProjection.cs new file mode 100644 index 0000000..ec71ca6 --- /dev/null +++ b/src/AcDream.Core/Selection/ScreenProjection.cs @@ -0,0 +1,86 @@ +using System.Numerics; + +namespace AcDream.Core.Selection; + +/// +/// Shared screen-space projection math for the target indicator and the +/// world picker. Both call into +/// so the click hit-area is guaranteed to match the visible indicator +/// rect — "what you see is what you click". +/// +/// +/// Retail equivalent: SmartBox::GetObjectBoundingBox at +/// 0x00452e20, which uses +/// Render::GetViewerBBox(selection_sphere, &corner1, &corner2) +/// 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. +/// +/// +public static class ScreenProjection +{ + /// + /// Project a world-space sphere to a screen-space axis-aligned square + /// rectangle. + /// + /// Sphere center in world space. + /// Sphere radius in world space. + /// View matrix (System.Numerics row-vector convention). + /// Projection matrix. M22 = cot(fovY/2) + /// for a standard right-handed perspective. + /// Viewport size in pixels (X = width, Y = height). + /// Out: top-left corner of the rect in viewport pixels. + /// Out: bottom-right corner of the rect in viewport pixels. + /// Out: camera-space depth (clip.W) of the sphere + /// center — use this for nearest-first sorting when multiple rects overlap. + /// Minimum side length of the rect. Distant + /// entities clamp to this so they remain pickable / visible. 12 px + /// matches the indicator's clamp floor. + /// + /// true if the sphere is in front of the camera and the rect was + /// produced; false if the center is behind the camera + /// (clip.W <= 0) or the rect is more than a screen offset + /// from the viewport (obviously off-screen). + /// + 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; + } +} diff --git a/src/AcDream.Core/Selection/WorldPicker.cs b/src/AcDream.Core/Selection/WorldPicker.cs index dfc0300..f95a93f 100644 --- a/src/AcDream.Core/Selection/WorldPicker.cs +++ b/src/AcDream.Core/Selection/WorldPicker.cs @@ -158,4 +158,91 @@ public static class WorldPicker } return bestGuid; } + + /// + /// 2026-05-16. Screen-space rect-hit-test picker overload. Each + /// candidate's world-space sphere (via ) + /// projects to a screen-space rectangle through + /// . The + /// rect is inflated by on every side + /// (matches the indicator's TriangleSize outer brackets) and + /// hit-tested against the mouse pixel. Among rects that contain the + /// mouse, the entity with the nearest camera-space depth wins. + /// + /// + /// 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 helper with + /// TargetIndicatorPanel, the click rect and the drawn rect + /// cannot drift. + /// + /// + /// + /// Resolver returning null skips the candidate (matches retail + /// "no Setup → not pickable" behavior). Entities with + /// ServerGuid == 0 (atlas-tier scenery) and the player's own + /// guid are also skipped. + /// + /// + /// + /// Stage A of the picker port. Stage B (polygon refine via + /// CPolygon::polygon_hits_ray 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. + /// + /// + /// Pixel inflate on each side of the + /// projected rect. Pass the indicator's TriangleSize (8 px) + /// so the click area extends to where the visible bracket corners + /// sit — the user perceives the inflated rect as the clickable area. + public static uint? Pick( + float mouseX, float mouseY, + Matrix4x4 view, + Matrix4x4 projection, + Vector2 viewport, + IEnumerable candidates, + uint skipServerGuid, + Func 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; + } } diff --git a/tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs b/tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs new file mode 100644 index 0000000..586b565 --- /dev/null +++ b/tests/AcDream.Core.Tests/Selection/ScreenProjectionTests.cs @@ -0,0 +1,64 @@ +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 (System.Numerics + // CreatePerspectiveFieldOfView is right-handed; camera at origin + // looks down -Z). + 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() + { + 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); + Assert.Equal(rMax.X - rMin.X, rMax.Y - rMin.Y, precision: 3); + 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); + } +} diff --git a/tests/AcDream.Core.Tests/Selection/WorldPickerRectOverloadTests.cs b/tests/AcDream.Core.Tests/Selection/WorldPickerRectOverloadTests.cs new file mode 100644 index 0000000..176f68e --- /dev/null +++ b/tests/AcDream.Core.Tests/Selection/WorldPickerRectOverloadTests.cs @@ -0,0 +1,133 @@ +using System; +using System.Numerics; +using AcDream.Core.Selection; +using AcDream.Core.World; + +namespace AcDream.Core.Tests.Selection; + +public sealed class WorldPickerRectOverloadTests +{ + 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); + } + + private static WorldEntity MakeEntity(uint serverGuid, Vector3 position) => new() + { + Id = serverGuid == 0u ? 1u : serverGuid, + ServerGuid = serverGuid, + SourceGfxObjOrSetupId = 0u, + Position = position, + Rotation = Quaternion.Identity, + MeshRefs = Array.Empty(), + }; + + [Fact] + public void Pick_RectHitTest_ReturnsHitWhenMouseInsideRect() + { + var (view, proj, viewport) = StdCam(); + var e = MakeEntity(0x10001u, new Vector3(0, 0, -10)); + 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 = MakeEntity(0x10001u, new Vector3(0, 0, -10)); + 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(); + var near = MakeEntity(0x10001u, new Vector3(0, 0, -8)); + var far = MakeEntity(0x10002u, new Vector3(0, 0, -15)); + 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 = MakeEntity(0x10001u, new Vector3(0, 0, -10)); + var e2 = MakeEntity(0x10002u, new Vector3(0, 0, -20)); + 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 = MakeEntity(0x5000000Au, new Vector3(0, 0, -10)); + var npc = MakeEntity(0x10002u, new Vector3(0, 0, -15)); + 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 = MakeEntity(0x10001u, new Vector3(0, 0, -10)); + + 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); + + 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); + } +}