diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 6f504034..dbe75065 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -67,6 +67,75 @@ Copy this block when adding a new issue: --- +## #231 — F-key pickup omits retail pending destination-slot presentation + +**Status:** IN-PROGRESS — isolated as Slice 1 corrective commit +**Severity:** MEDIUM +**Filed:** 2026-07-21 +**Component:** selection / inventory / retained UI + +**Description:** World double-click loot reserves its destination cell with the +gray pending mesh before the request, but `SelectionPickUp` queues `SendPickUp` +directly and skips that presentation. The item can therefore settle into the +backpack without the same immediate first-slot reservation retail shows. + +**Root cause / status:** Retail `CPlayerSystem::PlaceInBackpack @ 0x0055D8C0` +always sends `CM_Item::SendNotice_ShowPendingInPlayer` before +`AttemptToPlaceInContainer`, including the F-key action from +`CPlayerSystem::OnAction @ 0x00561964`. acdream's double-click path enters +`ItemInteractionController` and raises `PendingBackpackPlacementRequested`; +the F-key switch case bypasses that owner. + +**Files:** `src/AcDream.App/Rendering/GameWindow.cs`; +`src/AcDream.App/UI/ItemInteractionController.cs`; +future `src/AcDream.App/Interaction/SelectionInteractionController.cs`. + +**Research:** +[`docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md`](plans/2026-07-21-gamewindow-slice-1-selection-interaction.md). + +**Acceptance:** F-key pickup validates the target, publishes the exact +destination/placement pending reservation, then sends one authoritative request. +Server confirmation settles it; server failure removes it without speculative +ownership mutation. + +--- + +## #230 — Selection hits and deferred actions can cross live GUID incarnations + +**Status:** IN-PROGRESS — isolated as Slice 1 prerequisite hardening +**Severity:** HIGH +**Filed:** 2026-07-21 +**Component:** selection / live entity lifetime / interaction + +**Description:** The completed render-selection frame and a pending close-range +Use/PickUp store only the server GUID. If the object hides, is deleted, or is +replaced with the same GUID before the next pick/completion, old presentation or +movement state can resolve to the replacement incarnation. Session reset also +leaves item target mode and the render-selection pulse/frame outside one atomic +interaction reset. + +**Root cause / status:** `RetailSelectionPart`/`RetailSelectionHit`, click +lighting, and `PendingPostArrivalAction` lack the canonical local-entity/record +identity owned by `LiveEntityRuntime`. Teardown's GUID-only replacement guard +correctly protects new selection, but necessarily skips clearing a GUID-only +pending action. `CombatTargetController` also treats SessionReset like an +ordinary clear and can re-acquire before live records drain. + +**Files:** `src/AcDream.Core/Selection/RetailSelectionMesh.cs`; +`src/AcDream.App/Rendering/Selection/`; +`src/AcDream.App/Rendering/GameWindow.cs`; +`src/AcDream.App/Combat/CombatTargetController.cs`. + +**Research:** +[`docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md`](plans/2026-07-21-gamewindow-slice-1-selection-interaction.md). + +**Acceptance:** Hidden/pending/deleted/replaced objects reject stale render hits; +old click lighting never colors a replacement; natural MoveTo completion sends +only for the captured current visible incarnation; session reset clears the +complete interaction lifetime and never auto-targets. + +--- + ## #228 — Clean Release build emits 17 test-project warnings **Status:** OPEN diff --git a/docs/architecture/code-structure.md b/docs/architecture/code-structure.md index 3c8fa7b7..5f1d379f 100644 --- a/docs/architecture/code-structure.md +++ b/docs/architecture/code-structure.md @@ -411,6 +411,9 @@ the complete connected lifecycle gate. #### Slice 1 — finish selection/interaction ownership — NEXT +Detailed execution plan: +[`docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md`](../plans/2026-07-21-gamewindow-slice-1-selection-interaction.md). + Split the old Step 4 into three reviewable commits: 1. `WorldSelectionQuery` receives camera/scene/live-object read seams and owns diff --git a/docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md b/docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md new file mode 100644 index 00000000..8a4629da --- /dev/null +++ b/docs/plans/2026-07-21-gamewindow-slice-1-selection-interaction.md @@ -0,0 +1,331 @@ +# GameWindow Slice 1 — selection and interaction ownership + +**Status:** Approved for execution 2026-07-21. +**Parent program:** [`docs/architecture/code-structure.md`](../architecture/code-structure.md), Slice 1. +**Baseline:** `cf50ee3d`; `GameWindow.cs` is 15,723 lines before this slice. +**Behavior rule:** Existing retail behavior moves unchanged. Any defect found by +the extraction audit is fixed in a separately identified commit before the +mechanical cutover. + +## 1. Outcome + +`GameWindow` stops owning world selection and selection-driven interaction. +After this slice it retains only composition and narrow lifecycle calls: + +- construct the query/controller; +- feed the completed render-selection frame; +- drain outbound interactions at the existing retail frame boundary; +- forward input actions and MoveTo completion; +- forward entity hidden/removal and session-reset edges. + +Two App owners replace the current body: + +1. `WorldSelectionQuery` owns read-only world selection, eligibility, + classification, naming, closest-hostile lookup, target bounds, and range + queries over canonical live state. +2. `SelectionInteractionController` owns selection intents, click routing, + Use/PickUp ordering, speculative facing/movement, deferred close-range + requests, and the input-action boundary. + +Core `SelectionState` remains the sole selected-object truth. +`ItemInteractionController` remains the sole retail ItemHolder policy owner. +`LiveEntityRuntime` remains the sole GUID/incarnation/visibility owner. + +## 2. Retail and project oracles + +No AC algorithm is invented in this slice. Move the existing ports with these +references attached: + +- [`2026-07-17-retail-world-selection-pseudocode.md`](../research/2026-07-17-retail-world-selection-pseudocode.md) + — render-coupled part selection, selection sphere, vivid target, click pulse. +- [`2026-07-10-retail-toolbar-interaction-pseudocode.md`](../research/2026-07-10-retail-toolbar-interaction-pseudocode.md) + — target-mode precedence, ItemHolder Use, selection notices. +- [`2026-07-17-retail-external-container-looting-pseudocode.md`](../research/2026-07-17-retail-external-container-looting-pseudocode.md) + — corpse/container open, range, pending loot placement. +- [`2026-07-12-death-and-auto-target-pseudocode.md`](../research/2026-07-12-death-and-auto-target-pseudocode.md) + — synchronous selection clear and reentrant Auto Target. +- `Render::GfxObjUnderSelectionRay @ 0x0054C740`. +- `ACCWeenieObject::SetSelectedObject @ 0x0058C2E0`. +- `CPlayerSystem::OnAction @ 0x00561890`. +- `CPlayerSystem::PlaceInBackpack @ 0x0055D8C0`. +- `ItemHolder::UseObject @ 0x00588A80`. +- `UIElement_SmartBoxWrapper::RecvNotice_SmartBoxObjectFound @ 0x004E5AD0`. + +Existing registered adaptations remain adaptations; this structural slice does +not silently “improve” them: + +- IA-19: automatic acquisition is narrowed to hostile non-player monsters. +- AD-11: ACE null-useability fallback. +- AD-27: natural `MoveToComplete` is the client completion seam. +- AP-23: speculative local Use movement and 3/2/0.6-unit radius buckets. + +Their file pointers move in the same commit as their implementation bodies. + +## 3. Invariants + +### 3.1 Render selection + +- The renderer builds a private list, then publishes it only at + `CompleteFrame`; input reads the previous complete frame. +- A hit carries server GUID **and local entity ID**. The query accepts it only + if `LiveEntityRuntime` still resolves the same current, visible top-level + incarnation. +- Plain selection excludes self. Active target mode includes self. +- A successful hit starts the HIGH click-light pulse before target-mode or + selection routing. +- Target-mode success or rejection consumes the click and never falls through + to ordinary selection/use. +- A miss does not clear selection. A miss during target mode leaves target mode + active. +- Drag release may pulse the target, but does not select it. +- Vivid target markers deliberately query materialized state so selected + objects remain indicated through walls and long distances. Picking and new + combat acquisition use visible interaction state only. + +### 3.2 Input and outbound ordering + +- Keep the one `InputDispatcher.Fired` subscription in `GameWindow`. +- Combat attack handling, Press/DoubleClick filtering, and retained-UI capture + occur before delegation exactly as today. +- The controller handles only the currently implemented semantic actions: + previous selection, closest monster, left selection, double-left selection, + Use selected, and PickUp selected. +- Input callbacks enqueue Use/PickUp. The queue drains after local movement + output and before inbound packet dispatch. +- The queue drains only its boundary snapshot; reentrant enqueue waits until + the next frame. + +### 3.3 Use/PickUp and movement + +- `ItemInteractionController` decides Use/open/target mode. The new controller + does not copy ItemHolder policy. +- Far target: install speculative MoveTo and send exactly one wire request + immediately. ACE owns its arrival callback. +- Close target: install TurnToObject and defer the **first** request until + natural `MoveToComplete(None)`. +- Cancel/error completion never sends. +- The movement intent preserves target cylinder radius/height, the AP-23 use + radius, and the 7.5-unit CanCharge prediction. +- Every queued/deferred request is bound to the captured live incarnation and + revalidated before sending. +- Pickup preserves destination container and placement. Retail pending-slot + presentation occurs before the request. + +### 3.4 Lifecycle + +- Hidden, logical removal, session reset, and GUID replacement invalidate old + hits and pending actions immediately. +- Token-specific pending cleanup is allowed even after GUID replacement; + GUID-only selection cleanup retains the existing incarnation guard so old + teardown cannot clear a newly selected replacement. +- Session reset atomically clears selection history, outbound queue, deferred + action, item target mode/busy state, published selection frame, and click + pulse. +- `CombatTargetController` must not auto-acquire during `SessionReset`. +- No new GUID map, selection store, visibility store, or retry timer is added. + +## 4. Target interfaces + +### 4.1 Render hit identity + +`RetailSelectionPart` and `RetailSelectionHit` carry `LocalEntityId` alongside +`ServerGuid`. `RetailSelectionScene.Pick` returns the complete hit. Its lighting +source is also keyed by both identities so a reused GUID cannot inherit an old +click pulse. + +`RetailSelectionScene.Reset` clears building/published lists, frame keys, +frustum state, and pulse state. Geometry resolution receives a narrow internal +source interface so publication behavior is testable without installed DAT. + +### 4.2 `WorldSelectionQuery` + +The query receives: + +- canonical `LiveEntityRuntime`; +- `ClientObjectTable`; +- the shared `RetailSelectionScene`; +- player GUID and current player position providers; +- camera/viewport and cursor providers; +- a DAT selection-sphere resolver; +- the existing Setup-cylinder resolver. + +It exposes read-only operations for: + +- pick at cursor/screen point with exact-incarnation validation; +- current interaction record/entity lookup; +- item type, name, and description; +- creature/hostile/health eligibility; +- closest visible hostile result without mutating selection; +- combat camera target; +- vivid target information and selection sphere; +- useability/pickupability; +- AP-23 use radius, close-range, CanCharge, and target movement data; +- external-container cylinder range. + +The query never mutates selection, queues work, sends packets, or starts +movement. + +### 4.3 `SelectionInteractionController` + +The controller receives: + +- `SelectionState` and `WorldSelectionQuery`; +- `ItemInteractionController` provider; +- a typed Use/PickUp transport; +- a narrow movement-intent sink; +- toast/system diagnostic sinks; +- Auto Target setting provider. + +It owns `OutboundInteractionQueue` and an incarnation-bound +`PendingPostArrivalAction`. Public methods form the complete host seam: + +```text +HandleInputAction +PickAtCursor / PlaceDraggedItem +SelectClosestCombatTarget +GetSelectedOrClosestCombatTarget +SendUse / SendPickUp +DrainOutbound +OnNaturalMoveToComplete +OnEntityHidden / OnEntityRemoved +ResetSession +``` + +The controller does not subscribe directly to Silk input and does not own +rendering, combat state, the session, or item policy. + +## 5. Execution sequence + +### Commit 0 — plan and defect registration + +- Land this plan. +- Register #230 for GUID-incarnation/session-reset leakage. +- Register #231 for F-key pickup missing the pending destination reservation. +- No runtime behavior change. + +### Commit 1 — lifecycle identity hardening (#230) + +- Add local-entity identity to render parts/hits and selection lighting. +- Add scene reset and deterministic publication tests. +- Validate published hits against current visible incarnation. +- Bind pending post-arrival actions to a live record/local identity. +- Clear only the matching pending token on teardown, including GUID reuse. +- Make session reset clear scene and item interaction state. +- Prevent combat Auto Target during `SessionReset`. +- Keep this root-cause correction independently bisectable. + +### Commit 2 — retail pending pickup presentation (#231) + +- Add one ItemInteractionController entry point for a validated world pickup. +- Publish `PendingBackpackPlacementRequested` before calling the existing + placement delegate. +- Route F-key pickup through it with destination/placement unchanged. +- Do not optimistically move ownership; server confirmation/failure remains + authoritative. + +### Commit 3 — extract `WorldSelectionQuery` + +- Move every read-only selection/classification/range body from `GameWindow`. +- Preserve materialized-versus-visible semantics. +- Extend the live animation read seam with current motion so dead eligibility + does not depend on `GameWindow.AnimatedEntity`. +- Rewire cursor, vivid target, toolbar health, combat camera, external-container + range, drag target, and door-name reads through the query. +- Delete the corresponding `GameWindow` methods. + +### Commit 4 — extract `SelectionInteractionController` + +- Move world click routing, selection mutation, closest-target selection, + outbound queue, Use/PickUp, movement intent, deferred completion, and + hidden/removal/reset handling. +- Compose `ItemInteractionController`; do not duplicate its policies. +- Update IA-19/AD-11/AD-27/AP-23 file pointers. +- Add focused controller tests for every invariant in §3. + +### Commit 5 — input cutover and host cleanup + +- Replace the six selection/interaction switch cases with one controller + delegation call at the same ordered position. +- Forward render-frame, update-frame, MoveTo completion, hidden/removal, + session-reset, cursor, and drag hooks. +- Delete old fields/records/wrappers from `GameWindow`. +- Prove `GameWindow` no longer owns a selection algorithm or interaction + request body. + +### Commit 6 — review, gate, and ledger close + +- Run retail-conformance, architecture/integration, and adversarial read-only + reviews against the complete slice diff. +- Fix confirmed findings and repeat review until clean. +- Run focused Core/App tests, Release solution build, and full Release suite. +- Update code-structure ownership status, roadmap/current queue if necessary, + both session instruction files, divergence pointers, and durable interaction + memory. +- Record the new `GameWindow` line/field/method counts. + +## 6. Automated acceptance matrix + +### Render/query + +- Building frame is invisible until `CompleteFrame`. +- Last complete frame remains queryable while the next frame builds. +- Empty completion clears prior hits. +- Hidden, pending, deleted, and replaced incarnations reject stale hits. +- A replacement GUID is not pickable until its own part is published. +- Pulse is identity-bound and cannot color a replacement. +- Plain pick excludes self; target-mode pick includes self. +- Vivid target can use materialized state while combat candidates cannot. +- Closest hostile excludes self, players, pets, friendly NPCs, dead, Hidden, + pending, and deleted objects. + +### Interaction + +- Pulse precedes target-mode and selection routing. +- Target-mode success/rejection consumes the click without selection drift. +- Single click selects; double click captures the clicked GUID and queues one + activation. +- Miss preserves selection and target mode. +- Previous selection preserves current SelectionState semantics. +- Queue drains after movement and only once per captured boundary. +- Far Use/PickUp sends once; close sends only on natural completion. +- Cancel/error/Hidden/delete/reset sends zero. +- Delete then same-GUID create sends zero to the replacement. +- Useability and pickupability fallbacks remain byte-for-byte behaviorally + equivalent. +- Creature and Stuck pickup rejection messages remain exact. +- Corpse double-click/Use opens it and never attempts to pick up the corpse. +- F pickup reserves the first destination slot before wire send. +- Pickup destination/placement survive the deferred path. + +### Lifecycle/input + +- Session reset clears previous selection, target mode, busy state, queue, + pending completion, published picks, and pulse; repeated reset is safe. +- SessionReset cannot trigger combat auto-target. +- Modal/retained UI capture still prevents controller invocation upstream. +- Escape target-mode cancellation remains ahead of window-close behavior. + +## 7. Connected gate + +Run one normal capped Release client and verify: + +1. select a nearby door/NPC and see the click pulse and target corners; +2. double-click/Use the door or NPC; +3. select a distant corpse, press R, approach, and open it; +4. double-click one loot item and use F on another; both reserve the expected + backpack slot and settle after ACE confirmation; +5. enter combat, select a hostile, use closest-target and previous-selection; +6. kill the selected target with Auto Target enabled and observe only a hostile + replacement; +7. portal or relog and confirm no old selection/pulse/action survives. + +Any failure returns to the owning commit. No retry, grace period, or +GUID-only suppression is acceptable. + +## 8. Subagent policy for this slice + +The implementation is tightly coupled across `GameWindow`, live lifetime, +render publication, input order, and player MoveTo. One primary agent performs +all edits. Independent subagents are useful for read-only retail, +architecture, and adversarial reviews; they do not edit the shared slice +concurrently.