From 6bb4cfa795d879774acf86825382157d8f308694 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 8 Aug 2026 20:18:51 +0200 Subject: [PATCH] =?UTF-8?q?feat(ui):=20the=20spell-bar=20drop=20ring=20?= =?UTF-8?q?=E2=80=94=20retail's=20authored=20drag-accept=20state,=20and=20?= =?UTF-8?q?the=20ring=20exposed=20a=20real=20drop=20off-by-one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The green ring is retail's own art: every UIItem cell carries an authored DragAccept child (catalog 0x21000037, child 0x1000045A), and the spell bar's drag-over handler (SpellCastSubMenu::OnItemListDragOver @0x004C5990) flips it to the Accept state (0x10000040 -> surface 0x060011F9) for any spell payload. Ported through a per-slot SetDragAcceptVisual seam + a catalog DragOverAcceptance hook; other lists are untouched (null acceptance = neutral). A polarity error in our older docs (Accept/Reject state ids swapped) was corrected against three independent sources; the shipped art was always right, only the labels lied. The ring shares ONE landing computation with the drop (FavoriteDropIndex) — and that requirement exposed a genuine #354 off-by-one: the empty-tail path double-applied the -1 adjustment (retail gates it on the lift's removal @0x004C7157), landing a reordered spell second-to-last instead of last. Fixed; discriminator-verified both ways. AP-172 narrowed + its false empty-tail claim corrected. Clean-room complete solution: 11,545 passed / 4 skipped / 0 failed. Co-Authored-By: Claude Sonnet 4.5 --- docs/ISSUES.md | 30 +++ .../retail-divergence-register.md | 2 +- ...item-slot-icon-dragdrop-spine-deep-dive.md | 13 + ...-13-retail-item-drag-visuals-pseudocode.md | 20 +- .../UI/Layout/InventoryController.cs | 2 +- .../UI/Layout/SpellcastingUiController.cs | 72 +++-- src/AcDream.App/UI/UiCatalogSlot.cs | 33 +++ src/AcDream.App/UI/UiItemSlot.cs | 46 ++-- .../Layout/SpellcastingUiControllerTests.cs | 250 ++++++++++++++++++ 9 files changed, 434 insertions(+), 34 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 20164d32..0b16933b 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -60,6 +60,36 @@ bytes for `AddSpellFavorite`/`RemoveSpellFavorite` (opcodes 0x1E3/0x1E4) and `RuntimeCharacterState.TryAddFavorite`/`TryRemoveFavorite` were already covered and needed no change. +**UPDATE 2026-08-08 (follow-up session, drop-ring gate finding):** the user's +next gate finding — "Should be the green ring indicator where the spell icon +should land, like in retail" — is implemented. Retail's mechanism (grepped and +byte-confirmed): the ring is a per-cell authored slot STATE, not a synthetic +overlay — `SpellCastSubMenu::OnItemListDragOver` @0x004C5990 sets the shared +UIItem prototype's `m_elem_Icon_DragAccept` child (element 0x1000045A, bound in +`UIElement_UIItem::PostInit` @0x004E1870, catalog LayoutDesc 0x21000037) to +`ItemSlot_DragOver_Accept` (UIStateId 0x10000040 → authored art 0x060011F9) +whenever the dragged payload carries a spell id; leave resets to +`ItemSlot_DragOver_Normal` 0x1000003F @0x004E1438. Wired through +`UiCatalogSlot.DragOverAcceptance` (the catalog port of retail's per-list drag +handler) → the shared `UiItemSlot.DrawDragAcceptOverlay`. Two corrections +landed with it: (1) the accept/reject UIStateId labels were SWAPPED in +`UiItemSlot`/`InventoryController` comments and in the 2026-06-16 / 2026-07-13 +research docs (art-per-semantic was always right; polarity pinned by paperdoll +`AutoWearIsLegal` @0x004A3AC9/0x004A3AEB, `VendorSellUI` @0x004C2327/0x004C2336, +DatReaderWriter's `UIStateId` enum, and the 2026-06-25 layout dump); (2) a real +off-by-one in #354's drop path: the `-1` adjustment double-corrected the +empty-tail cell's live-count-clamped index, landing a lifted non-last favorite +second-to-last instead of last — retail's adjustment is gated on +`RemoveSpellFromMenu`'s return (@0x004C7157), which is `-1` (no adjustment) at +drop time because the spell left the live list at lift. `FavoriteDropIndex` is +now THE one landing computation shared by the ring and the drop +(discriminator-verified: the pre-fix computation fails +`SpellFavoriteDrag_DroppedOnTheEmptyTail_AppendsAtTheEnd` with landed index 1 +vs 2). AP-172 narrowed + corrected in the same change-set. New tests: ring +appears/tracks/survives-a-tick/clears-on-drop, empty-tail append, ring clears +on leave + off-bar release keeps the lift removal, physical payloads stay +neutral while both spell payload kinds ring. + ## #353 — Toolbar selected-object text: count field ignores authored HJustify; name field does not wrap to its authored two lines **Status:** CLOSED 2026-08-08 — user-passed ("Ok slider bar looks ok!" + the wrap confirmed); the OneLine routing fix (4cfcc8b3) completed it. (RightAligned on the authored HJustify=2 entry; two stacked centered one-line labels wrapping at the authored 140 px via WrapNameTwoLines). diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index dcd65649..38a82b89 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -178,7 +178,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| -| AP-172 | **Filed 2026-08-08 (#354 fix — spell-bar drag reorder).** Retail removes a lifted favorite from `PlayerModule` (+ UI list + wire) the instant a drag starts and the remaining shortcuts visibly slide left to close the gap for the rest of the gesture (`RecvNotice_ItemListBeginDrag` → `RemoveSpellFromMenu`, live). acdream's controller performs the same PlayerModule/wire removal at drag-begin but DEFERS the whole favorite-list's visual rebuild until the drag concludes (drop or off-bar release) — the lifted cell's icon stays visible in its old slot and siblings do not slide until release, instead of reflowing continuously through the gesture. `DropFavorite` compensates by porting retail's own `AddFavorite`-side index adjustment (decrement the target index by one when the lifted item's original index was before it) against the now-intentionally-stale sibling numbering, so the FINAL landed position is byte-identical to retail's in every case exercised (`DragFavoriteOntoAnotherSlot_ThroughTheRealPointerPipeline_ReordersAndSyncsWire`). | `src/AcDream.App/UI/Layout/SpellcastingUiController.cs` (`BeginFavoriteDrag`, `EndFavoriteDrag`, `DropFavorite`, `Tick` — the `_favoriteDragActive` gate) | `UiRoot`'s subtree-removal safety net (`ClearSubtreeOwnership`, `UiRoot.cs:240-247`) cancels any in-flight drag whose source widget is destroyed, and `Rebuild()` tears down and recreates every cell in the list (`UiItemList.Flush` → `RemoveChild` per cell) rather than incrementally diffing. Left unguarded, the press-time removal's `SpellbookChanged` event would let the very next per-frame `Tick()` (production drives this unconditionally via `RetailUiRuntime.Tick`) destroy the cell driving the gesture and silently cancel the reorder before the user could complete the drop — this was the reported bug. Deferring the rebuild for the gesture's duration is the minimal fix that does not touch the shared `UiRoot` drag machinery every other panel (toolbar/inventory/vendor/paperdoll) also depends on. | A future rewrite that makes `Rebuild()` an incremental per-cell diff (add/remove/reflow one cell) instead of flush-and-recreate-all would make this deferral unnecessary and should retire this row along with it — until then, a player watching their OWN spell bar mid-drag sees the vacated slot's icon linger and siblings snap into place only on release, rather than reflowing live as retail does. No effect on final position, the wire pair sent, or any other panel (empty-tail-slot drops and cross-window spellbook→favorite drops were already-live-count-relative and are untouched). | `gmSpellcastingUI::RecvNotice_ItemListBeginDrag` @0x004C7360 (`SpellCastSubMenu::RemoveSpellFromMenu`, immediate live-list removal at lift); `SpellCastSubMenu::AddFavorite` @0x004C7060 (`RemoveSpellFromMenu`'s return value gating the `-1`-if-lifted-before-target `m_numSpells` adjustment before `ItemList_InsertSpellShortcut`); `PlayerModule::AddSpellFavorite` @0x005D43E0 (`InsertPos`); `PlayerModule::RemoveSpellFavorite` @0x005D4910 | +| AP-172 | **Filed 2026-08-08 (#354 fix — spell-bar drag reorder).** Retail removes a lifted favorite from `PlayerModule` (+ UI list + wire) the instant a drag starts and the remaining shortcuts visibly slide left to close the gap for the rest of the gesture (`RecvNotice_ItemListBeginDrag` → `RemoveSpellFromMenu`, live). acdream's controller performs the same PlayerModule/wire removal at drag-begin but DEFERS the whole favorite-list's visual rebuild until the drag concludes (drop or off-bar release) — the lifted cell's icon stays visible in its old slot and siblings do not slide until release, instead of reflowing continuously through the gesture. `DropFavorite` compensates by porting retail's own `AddFavorite`-side index adjustment (decrement the target index by one when the lifted item's original index was before it) against the now-intentionally-stale sibling numbering, so the FINAL landed position is byte-identical to retail's in every case exercised (`DragFavoriteOntoAnotherSlot_ThroughTheRealPointerPipeline_ReordersAndSyncsWire`). **NARROWED + CORRECTED 2026-08-08 (drop-ring change).** Correction: this row originally claimed empty-tail-slot drops were "already-live-count-relative and are untouched" — false. The #354 `-1` adjustment sat inside `DropFavorite`, which the empty-cell path also calls, so its live-count-clamped (post-lift-numbered) index was double-corrected: lifting a non-last favorite onto the empty tail landed it second-to-last instead of last. `FavoriteDropIndex` is now THE one landing computation and applies retail's rule exactly — the `-1` is gated on the lifted spell's pre-lift-numbered removal site (retail's `RemoveSpellFromMenu`-return-gated decrement @0x004C7157), which for a live-numbered empty-tail target is retail's `RemoveSpellFromMenu == -1` no-adjustment case (test `SpellFavoriteDrag_DroppedOnTheEmptyTail_AppendsAtTheEnd` fails against the double-correcting code). Narrowing: the mid-drag presentation now includes retail's authored drag-over Accept ring — `SpellCastSubMenu::OnItemListDragOver` @0x004C5990 setting the per-cell authored DragAccept child (element 0x1000045A, `UIElement_UIItem::PostInit` @0x004E1870) to `ItemSlot_DragOver_Accept` (UIStateId 0x10000040 → authored art 0x060011F9) on the hovered cell while a spell drag is live, cleared on leave/drop (`UiCatalogSlot.DragOverAcceptance` → `UiItemSlot.DrawDragAcceptOverlay`), with the ring and the drop sharing `FavoriteDropIndex` so the ring cannot promise a different landing. | `src/AcDream.App/UI/Layout/SpellcastingUiController.cs` (`BeginFavoriteDrag`, `EndFavoriteDrag`, `DropFavorite`, `Tick` — the `_favoriteDragActive` gate) | `UiRoot`'s subtree-removal safety net (`ClearSubtreeOwnership`, `UiRoot.cs:240-247`) cancels any in-flight drag whose source widget is destroyed, and `Rebuild()` tears down and recreates every cell in the list (`UiItemList.Flush` → `RemoveChild` per cell) rather than incrementally diffing. Left unguarded, the press-time removal's `SpellbookChanged` event would let the very next per-frame `Tick()` (production drives this unconditionally via `RetailUiRuntime.Tick`) destroy the cell driving the gesture and silently cancel the reorder before the user could complete the drop — this was the reported bug. Deferring the rebuild for the gesture's duration is the minimal fix that does not touch the shared `UiRoot` drag machinery every other panel (toolbar/inventory/vendor/paperdoll) also depends on. | A future rewrite that makes `Rebuild()` an incremental per-cell diff (add/remove/reflow one cell) instead of flush-and-recreate-all would make this deferral unnecessary and should retire this row along with it — until then, a player watching their OWN spell bar mid-drag sees the vacated slot's icon linger and siblings snap into place only on release, rather than reflowing live as retail does — and one ring consequence of that frozen bar: when dragging rightward past the source, the Accept ring's SCREEN slot sits one cell right of where the icon finally lands (retail's live-reflowed bar makes them coincide); the ring is on the correct CELL in both — the spell lands immediately before that cell's spell, retail's exact insert-before semantic. No effect on final position, the wire pair sent, or any other panel; cross-window spellbook→favorite drops are live-count-relative and untouched (the empty-tail claim this sentence used to carry was corrected 2026-08-08 — see the Divergence column). | `gmSpellcastingUI::RecvNotice_ItemListBeginDrag` @0x004C7360 (`SpellCastSubMenu::RemoveSpellFromMenu`, immediate live-list removal at lift); `SpellCastSubMenu::AddFavorite` @0x004C7060 (`RemoveSpellFromMenu`'s return value gating the `-1`-if-lifted-before-target `m_numSpells` adjustment before `ItemList_InsertSpellShortcut`); `PlayerModule::AddSpellFavorite` @0x005D43E0 (`InsertPos`); `PlayerModule::RemoveSpellFavorite` @0x005D4910 | | AP-160 | **Filed 2026-08-07, Slice 5.3 (vendor browse lifecycle). CORRECTED AND EXTENDED 2026-08-07 at the Slice 5.3 review corrections (fixes 4/5).** **Correction (fix 4):** this row's own Retail-oracle citation originally grouped `WorldObject_Use.cs:50,57` under the SAME citation as `Vendor.CheckClose`/`GetCylinderDistance`, which read as if the `wo.UseRadius ?? 0.6f` fallback lived inside the close watcher. It does not: `WorldObject_Use.cs:50,57` is `WorldObject.IsWithinUseRadiusOf`, the APPROACH check ("how close you need to be to open the shop") — a wholly different method from `Vendor.CheckClose`, which reads `UseRadius` directly with no fallback of its own (`UseRadius` is `float?`; a nullable comparison against a null right operand is always `false`, so `CheckClose` never closes at all on an unauthored radius). `EnforceRange`'s own code comment carried the same mis-attribution and, worse, actually APPLIED that mis-borrowed 0.6f as its fallback; it now passes the raw authored `UseRadius` with no fallback of any kind (0 when absent/unauthored, matching retail's own memset-zero `PublicWeenieDesc::_useRadius` default — a plain `float` field, `acclient.h:37181`, no sentinel). Retail's own behavior for a radius-0 handler is exactly this: close on the very first nonzero-distance check. **Extension (fix 5):** the watcher reads the SERVER-ECHOED ACCEPTED position snapshot (`RuntimeEntityRecord.Snapshot.Position`), sampled once per advanced frame at the post-network-command-phase, not retail's continuous live-pose push (retail's own client simulates and renders every entity's pose every frame; `CPlayerSystem`'s range handler reads that live pose, never a periodically-echoed one). Between accepted-position updates the watcher's distance measurement is therefore up to one update-interval stale. The one BLIND WINDOW this staleness could open into a wrong in/out-of-range verdict — an in-session portal/teleport, where the player's and vendor's position snapshots can briefly sit in DIFFERENT landblock coordinate frames mid-transit — is closed unconditionally by this same review's fix 1b (`RuntimeWorldTransitState.HasPendingTeleportStart`/`IsTeleportActive` short-circuit the whole distance computation before it runs, closing the session instead of measuring across the transit), so the staleness itself never reaches that particular failure mode; it remains recorded here as a standing precision gap for the window fix 1b does NOT cover (ordinary out-of-transit movement between the same-generation position updates a slow network tick can leave briefly stale). **Original text:** The client-local vendor-panel distance watcher closes on PLAIN 3D center-to-center distance instead of retail/ACE's CYLINDER-GAP distance (both objects' own collision radius and height subtracted from the center distance before comparing to `UseRadius`). Retail: `gmVendorUI::OpenVendor` registers `CPlayerSystem::RegisterObjectRangeHandler` keyed to the vendor's own `PublicWeenieDesc._useRadius`; ACE's server-side belt-and-suspenders `Vendor.CheckClose` closes on `GetCylinderDistance(lastPlayer) > UseRadius`, i.e. `Position::cylinder_distance`/`Physics.Common.Position.CylinderDistance` with each side's real `GetRadius()`/`GetHeight()`. **NARROWED 2026-08-08 (vendor-verify gate): the watcher now measures retail's cylinder-gap via the ResolveObjectTableHost radii — the plain-center shortcut was self-closing sessions inside the walk-to-use acceptance band (opened at 4.29 m center vs authored radius 3, closed same frame). Residuals: heights pass 0, unresolvable hosts degrade to center distance (close-early only).** | `src/AcDream.Runtime/Gameplay/RuntimeVendorRangeQuery.cs` (`EnforceRange`) | `AcDream.Runtime` does not resolve a live per-entity collision radius/height for an arbitrary NPC outside the App-layer's Setup-cylinder resolver (`WorldSelectionQuery`'s `_setupCylinder`, App-only — out of Runtime's reach per the Core-structure rules, and `PhysicsBody`/`RuntimeEntityRecord` carry no radius/height field). Plain center distance is a well-defined, non-degenerate substitute (using `ObjectRangeMath.ObjectsInRange`'s existing `useRadii: false` branch rather than inventing a new metric) for a CLIENT-LOCAL UI convenience that never touches the wire or any authoritative state — closing the panel is not gated by, nor gates, anything server-visible. Reading the accepted-position snapshot rather than a continuously-integrated live pose is the same "Runtime has no live render-side pose, only the last accepted wire snapshot" constraint every other Runtime-side distance query in this codebase already accepts. | The panel can close up to (player radius + vendor radius) sooner than exact retail — typically well under a meter for a two-legged NPC — so a player standing exactly at the boundary of a large-radius vendor's `UseRadius` may see the panel close slightly earlier than retail would. No effect on any transaction, wire message, or authoritative state (Slice 6's buy/sell owns those). Retiring the cylinder-gap half requires a Runtime-owned per-entity collision radius/height source, which does not exist today; retiring the staleness half requires a continuously-updated live-pose source Runtime does not keep either. | `CPlayerSystem::RegisterObjectRangeHandler` pc:203677/0x004C4C34; `gmVendorUI::OnObjectRangeExit` pc:199486/0x004C02F0; ACE `Vendor.CheckClose`/`GetCylinderDistance` (`references/ACE/Source/ACE.Server/WorldObjects/Vendor.cs:322-367`) — a SEPARATE method, `WorldObject.IsWithinUseRadiusOf` (`WorldObject_Use.cs:44-52`), owns the unrelated `?? 0.6f` approach-check fallback; `acclient.h:37181` (`float _useRadius`, plain memset-zero field, no sentinel); `docs/research/2026-08-08-slice5-vendor-browse-research.md` §A.3/§B.1/§B.2 | | AP-141 | **Filed 2026-08-04, C4 route 5 (projectile authoritative placement); NARROWED 2026-08-04 at the round-2 delta review (B1/B2) — the far-branch clause was factually wrong for the adopted-body case and is corrected below.** Three related projectile-only shapes, all pinned by design (D-P4) rather than ported: (a) the near-`Interpolate` disposition is a NO-OP for a live missile, where retail would lazily build interpolation machinery (`InterpolateTo` @0x005163AF) for it; (b) the post-operation `ConstrainTo` @0x00454272 (`MakePositionManager` @0x00510523 then `PositionManager::ConstrainTo`) is never ARMED for a projectile — retail's single arming site has no kind test, so retail WOULD build a `PositionManager` on demand and arm a missile's leash on any nonzero `MoveOrTeleport` return; acdream never arms it on any disposition, including the adopted-body case (whose PRE-EXISTING leash the teleport/far branches now un-arm or clear queue state for, but never RE-anchor, per retail's post-operation `ConstrainTo`); (c) a null-classified or `Rejected*` accepted Position for a missile is swallowed (write nothing) rather than caught up through any remote-shaped policy. | `src/AcDream.Runtime/Session/RuntimeRemotePlacementDriveController.cs` (`ApplyAcceptedProjectilePosition`) | acdream deliberately does not construct an `EntityPhysicsHost`/`PositionManager`/`InterpolationManager` chain for a ballistic body — the route-5b split the C4 route 5 contract rejected. The context that makes this safe rather than merely convenient: ACE never sends `UpdatePosition` for a missile (`references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Tick.cs:333-334`, `SendUpdatePosition()` commented out inside the `PhysicsState.Missile` branch at `:265`) — every half of this row is deterministic-test-gated only, never exercised against a real server. **The far branch's `StopInterpolating` skip is retail-faithful ONLY for a BARE missile** (no `RemoteMotion` — retail's own `position_manager != 0` guard @0x005163C9 skips it for a never-interpolated object, so acdream's skip is faithful by consequence there). For the ADOPTED-BODY case (`TryBind`'s shared-body branch: an ordinary remote whose Missile bit was set by a later State packet, still carrying its `RemoteMotion`), retail's guard IS satisfied and retail WOULD clear the queue — acdream now ports this (`route.StopInterpolating && record.RemoteMotion is RemoteMotion adopted → adopted.Interp.Clear()`), matching the teleport branch's equivalent `StopInterpolating` action inside `teleport_hook`. What remains divergent for the adopted case is the post-operation `ConstrainTo` re-anchor @0x00454272 — retail re-anchors an existing leash at the just-updated position on every nonzero return; acdream never arms/re-anchors it on any projectile disposition (clause (b)). | A future change that DOES give projectiles a `PositionManager` (or a headless/no-window remote-motion consumer that expects one) must re-decide this row rather than silently building the machinery ad hoc; until then, a live missile never shows an ARMED constraint leash and never catches up via the near/UnroutedCatchUp policy — both unreachable in play. An adopted-body missile's INHERITED leash (armed before it became a missile) is un-armed by the teleport hook, has its queue cleared by both teleport and far, but is never re-anchored at the new position by either — its brake accumulator (`ConstraintPosOffset`) is not reset to zero at each accepted Position the way retail's @0x00454272 re-anchor does. **Correction, round 3 (2026-08-04): the round-2 wording here — that a stale leash "would drag the body toward a stale anchor" — was wrong and is retracted.** `ConstraintManager.ConstraintPos` is write-only in both retail and the port (never read by `AdjustOffset`), and `ConstraintManager::adjust_offset` @0x00556180 only tapers or zeroes an already-composed per-tick offset while `InContact` — a leash brakes motion the interp/sticky chain already produced; it has no mechanism to move anything toward the anchor. The real residual is confined to one tick of un-reset brake accumulator, contact-gated, and it cannot move an airborne far-snapped missile at all (the clamp branch does not run while airborne). | `CPhysicsObj::MoveOrTeleport` 0x00516330 (`InterpolateTo` @0x005163AF, `IsMovingTo` @0x0050EB10 returning 0 without a `MovementManager`; far branch `StopInterpolating` @0x005163C9-@0x005163CB); `SmartBox::HandleReceivedPosition` 0x00453FD0 (`ConstrainTo` arming site @0x00454272); `CPhysicsObj::ConstrainTo` 0x00510520 (`MakePositionManager` @0x00510523); `ConstraintManager::adjust_offset` 0x00556180 (brake-only taper, write-only anchor); `WorldObject_Tick.cs:333-334`/`:265` (ACE never-sends evidence) | | AP-142 | **Filed 2026-08-04 (C4 route 7, pickup/parent/delete). AMENDED 2026-08-04 at the dual-Opus retail-conformance/architecture review round (R1/A8 MAJOR+LOW; R10 MINOR) — clause (d) added, clause (b) corrected. AMENDED AGAIN 2026-08-04 at the round-3 dual review (N1/N2/N4, B3) — clause (d)'s reasoning corrected and its risk-column scope widened; clause (e) RETIRED — the depth cap it described is deleted outright, replaced by an iterative worklist with no depth concept at all. AMENDED AGAIN 2026-08-05 at the #319 fix — clause (f) added. AMENDED AGAIN 2026-08-05 at the #319 dual-review round (retail PASS, architecture FAIL/6 MAJORs) — clause (f) rewritten: the tripwire moved above the canonical commit and no longer throws (A1), and the deferred late-bind queue A1's fix text originally described was deleted per A6 (both reviews proved it production-unreachable for both producers).** acdream collapses retail's `CPhysicsObj` pair — a `cell` pointer plus a separately-written `objcell_id` — into ONE canonical `RuntimeEntityRecord.FullCellId`, which is also the residency/liveness predicate acdream reads at 45+ sites. Four consequences, all intentional: (a) the removal path propagates ZERO to a subtree's children (withdrawal, delete, `EndGeneration`), where retail's `leave_cell` recursion nulls only each child's `cell` pointer and leaves a STALE non-zero `objcell_id` (`change_cell`'s removal tail @0x005133C1 never touches a child's id) — reproducing that stale-id residue would leave a child "resident" per every acdream predicate while retail's own gating field (`cell == nullptr`) says it is not; (b) retail's same-cell depth-1 per-tick `objcell_id` refresh (`SetPositionInternal` @0x0051539c-@0x005153d8, gated on the parent NOT crossing a cell) is subsumed by the value-idempotent propagation chokepoint (`RuntimeEntityDirectory.SetFullCell`'s "skip a child whose `FullCellId` already equals the target" guard) rather than ported as a separate tick loop — a same-value restamp is unobservable with one field playing both retail roles. **Correction (R10): this is a clean equivalence only on the REMOVAL side.** The skip ALSO prunes the child's whole subtree on a same-value WRITE, which retail's `enter_cell` does not do — it recurses over children unconditionally (@0x00510f03); only `leave_cell` prunes (@0x00510f5b, on `cell != 0`). Currently unreachable-by-construction (after D4 nothing writes a grandchild's cell independently of its own committed parent), but it is an asymmetry, not a proven equivalence; (c) the sustaining propagation itself: retail re-cells children when the parent crosses a cell, recursively, on EVERY `SetPositionInternal`/`change_cell` (@0x00515372/@0x00513390), not only at attach — acdream ports this as a single hook every canonical cell-write funnels through, so an attach-only write (the pre-existing shape) is deliberately NOT what shipped. **(d) retail's `enter_cell` gates its ENTIRE body — the write AND the recursion into children — on `this->part_array != 0` (@0x00510ed8); a child with a null part array receives nothing and its whole subtree is skipped. acdream's propagation has NO analogue and writes unconditionally. CORRECTED reasoning (round-3 review, N1/N2): the original draft of this clause argued acdream's `HasPartArray` means something semantically different from retail's `part_array` (a "renderer built a mesh" flag vs. "this CPhysicsObj has any part array"). That framing is WRONG — retail's `part_array` has exactly ONE assignment site, `CPhysicsObj::makeAnimObject` @0x0050e930 → `CPartArray::CreateSetup`, assigned @0x0050e94d, so retail's flag is ALSO a mesh-construction product; the two are near-synonyms, not different concepts. The REAL reason acdream cannot gate the canonical D1/D2 write on `HasPartArray` is LAYERING, not semantics: Slice J made the Runtime canonical layer presentation-independent by design (`docs/research/2026-07-25-slice-j1-runtime-contract-closeout.md` and the Slice J campaign generally), and `HasPartArray` is populated exclusively by App/graphical code (`EquippedChildRenderController.cs:609`, `DatLiveEntityProjectionMaterializer.cs:203`) — the canonical layer structurally cannot depend on a flag only the presentation layer ever writes, headless or not. CORRECTED scope (round-3 review): this is NOT headless-only. `PrepareAndTryRealize` calls `CommitAcceptedParentCellless` (hence D1's re-cell) BEFORE `TryRealize` sets `HasPartArray = true` at `:609` — so at the exact moment D1 runs, `child.HasPartArray` is FALSE in the GRAPHICAL host too, and gating on it would break attach there as well, not just headless. Retail has no equivalent window at all: `part_array` is assigned once at construction and `enter_cell`'s guard reads that same, already-settled field.** The guard is deliberately NOT reproduced at the canonical layer. **(e) RETIRED 2026-08-04 (round-3 review, N4/B3 — both reviews independently found the same defect).** Previously: recursion depth capped at 64 levels as hostile/buggy-server hardening. The cap's actual failure mode was worse than what it guarded against: a subtree beyond the cap was left at its PRIOR — on the withdraw path, STALE NONZERO — cell PERMANENTLY, logged only under a probe flag nobody runs by default. On the withdraw path that is the #184 shape verbatim: an entity every acdream residency predicate calls resident that retail (and clause (a) above) says is not. Shipping that inside the slice whose headline is fixing exactly this class was unacceptable. Retired by deleting the cap outright and replacing the recursion with an iterative worklist (`RuntimeEntityDirectory._propagationWorklist`), which has no stack-frame-bounded depth at all — the only limit is the number of committed relations actually in the system, matching retail's own genuinely unbounded recursion with no acdream-only cap and therefore no register row for one. **(f) Filed 2026-08-05 (#319 fix).** A CreateObject-carried parent relation (the raw spawn's `Physics.Parent` field, and the same-generation `CreateParentUpdate` envelope) names the parent's GUID and location only — neither wire shape carries a parent instance sequence, matching retail's own GUID-only attach (`PhysicsDesc::get_parent_id` @0x00558a18 → `CObjectMaint::GetObjectA` @0x00558a2d → `CPhysicsObj::set_parent` @0x00558a3e; the reverse `CObjectMaint::SetChildren` @0x00509370 hash-walks by guid with a `GetNullObject` placeholder @0x005093e6 — no instance-sequence field or comparison exists anywhere in either direction). acdream's committed-relation table is nonetheless keyed by (guid, incarnation) (clause (c)'s D1/D2 requirement), so a CreateObject-carried relation must adopt SOME incarnation to file under; it now LATE-BINDS to the parent's LIVE incarnation at accept time (`EquippedChildRenderController.AcceptLateBoundCreateObjectRelation`, both the raw-CreateObject and same-generation `CreateParentUpdate` producers) rather than the previously-hardcoded 0, which silently mis-keyed every player-parented CreateObject relation (a player's `ObjectInstance` is `Character.TotalLogins`, never 0) and defeated D1/D2 for the local player's own login equipment and every remote player's observed equipment (#319). A commit-time tripwire (`ParentAttachmentState.CanCommitIncarnation`, checked BEFORE either half of the commit mutates state — architecture review A1, 2026-08-05, moved it there after the original throw-after-canonical-commit shape was shown to tear the transaction it was built to protect) refuses (logs, returns false, never throws) rather than silently filing a relation under a mismatched incarnation whenever the parent is currently addressable. **A1 also settled A6's design question**: an initial revision queued a relation whose parent was not yet addressable through a deferred/late-bind retry mechanism; both reviews independently proved that queue was structurally unreachable in production for BOTH producers (`RuntimeEntityObjectLifetime.RegisterEntityCore`'s `EnqueueDeferredCreate` gate defers the ENTIRE CreateObject, for both wire shapes, before either producer ever runs) while carrying three latent defects of its own (a missing child POSITION_TS gate, a placeholder-incarnation collision with the generation filters, unbounded accumulation) — it was deleted rather than fixed in place; the unaddressable-parent case now logs and refuses outright, matching the invariant the layer above already enforces. | `src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs` (`SetFullCell`, `PropagateFullCellToChildren`, `RefreshSnapshot`); `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs` (`CommitAcceptedParentCellless`'s D1 half, `WithdrawCommittedChildrenToCellless`); `src/AcDream.Runtime/Entities/ParentAttachmentState.cs` (`TryGetCommittedParent`, `CanCommitIncarnation`, `CommitProjection`); `src/AcDream.Runtime/Entities/RuntimeEntityRecord.cs` (`HasPartArray`); `src/AcDream.App/Rendering/EquippedChildRenderController.cs` (`AcceptLateBoundCreateObjectRelation`, `OnSpawn`, `OnCreateParentAccepted`, `PrepareAndTryRealize`); `src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs` (`ResolveAndCommitChildAttachment`) | Reproducing retail's pointer/id split would require a second field acdream's 45+ liveness call sites would then have to be individually audited for which half they mean — the single-field model is a stated, load-bearing simplification, not an oversight; see `docs/research/2026-08-04-retail-parent-cell-propagation.md` and `docs/research/2026-08-04-c4-route-7-contract.md` D2/D3/D9. Clause (f) is retail-faithful for the identical reason clauses (a)-(d) are: retail's attach has no incarnation gate on this path at all, so adopting the current holder of the guid IS the retail behavior, not an approximation of it. | A future consumer that expects retail's exact stale-`objcell_id`-under-a-null-`cell` shape (none identified) would see a fully cell-less child instead. (d)'s risk: acdream celling a child retail would leave nowhere — none identified in play against a well-behaved ACE, since a server-authored equip always names a real, DAT-resolvable Setup, and the graphical host's own brief pre-`TryRealize` window is bridged by D1 running inside the same synchronous transaction as the rest of the attach commit, not by `HasPartArray` being true. (f)'s risk: none identified against a well-behaved ACE — a CreateObject's parent guid always names the entity that currently holds it by construction. | `CPhysicsObj::change_cell` 0x00513390 (@0x005133C1 removal tail); `CPhysicsObj::enter_cell` 0x00510ed0 (@0x00510ed8 the `part_array` guard); `CPhysicsObj::leave_cell` 0x00510f50; `CPhysicsObj::SetPositionInternal` 0x00515330 (@0x0051536d branch, @0x0051539c-@0x005153d8 same-cell loop, @0x00515372 cell-change branch); `CPhysicsObj::makeAnimObject` 0x0050e930 (`CPartArray::CreateSetup` assignment @0x0050e94d); `PhysicsDesc::get_parent_id` 0x00558a18; `CObjectMaint::GetObjectA` 0x00558a2d; `CPhysicsObj::set_parent` 0x00558a3e; `CObjectMaint::SetChildren` 0x00509370 (`GetNullObject` placeholder @0x005093e6) | diff --git a/docs/research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md b/docs/research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md index 127f077d..868692a3 100644 --- a/docs/research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md +++ b/docs/research/2026-06-16-ui-item-slot-icon-dragdrop-spine-deep-dive.md @@ -141,6 +141,19 @@ internal element states `SetDragAcceptState` writes — both are real; the Layou states and the `0x1000003x/4x` UIStateIds are the same overlay seen from the dat side vs. the C++ side. CONFIRMED. +> **Correction 2026-08-08 (spell-bar drop-ring research):** the parenthetical +> above has the accept/reject ids SWAPPED. The true mapping is +> `ItemSlot_DragOver_Accept = 0x10000040 → 0x060011F9` and +> `ItemSlot_DragOver_Reject = 0x10000041 → 0x060011F8`, confirmed by +> DatReaderWriter's retail-derived `UIStateId` enum, by the legal/illegal +> branches of `gmPaperDollUI::HandlePaperDollDragOver` @ 0x004A3AC9/0x004A3AEB +> and `VendorSellUI::OnItemListDragOver` @ 0x004C2327/0x004C2336, and by the +> machine layout dump (`2026-06-25-retail-ui-layout-dump.json`, states +> 268435520/268435521 on elements 0x1000046D/0x1000046C). The table row's +> name→art column above was always right; only this paragraph's numeric +> pairing was inverted (and propagated into +> `2026-07-13-retail-item-drag-visuals-pseudocode.md`, corrected the same day). + ### 2.3 Key methods + the update pass (`UIItem_Update`, decomp 230226) `UIItem_Update` is the per-change refresh; the controller calls it whenever the bound diff --git a/docs/research/2026-07-13-retail-item-drag-visuals-pseudocode.md b/docs/research/2026-07-13-retail-item-drag-visuals-pseudocode.md index d1ca97ad..e7c8adf2 100644 --- a/docs/research/2026-07-13-retail-item-drag-visuals-pseudocode.md +++ b/docs/research/2026-07-13-retail-item-drag-visuals-pseudocode.md @@ -79,12 +79,28 @@ if target list is a container selector target.SetDragAcceptState(0x10000046) # ItemSlot_DragOver_DropIn # 0x060011F7 green arrow else if target accepts an ordinary item-list placement: - target.SetDragAcceptState(0x10000041) # ItemSlot_DragOver_Accept + target.SetDragAcceptState(0x10000040) # ItemSlot_DragOver_Accept # 0x060011F9 green circle else: - target.SetDragAcceptState(0x10000040) # 0x060011F8 reject + target.SetDragAcceptState(0x10000041) # ItemSlot_DragOver_Reject + # 0x060011F8 reject ``` +> **Correction 2026-08-08 (spell-bar drop-ring research):** the block above +> originally had the Accept/Reject numeric ids swapped (`0x10000041` labeled +> Accept, `0x10000040` labeled reject). Three primary sources agree the true +> mapping is `ItemSlot_DragOver_Accept = 0x10000040 → 0x060011F9` and +> `ItemSlot_DragOver_Reject = 0x10000041 → 0x060011F8`: DatReaderWriter's +> retail-derived `UIStateId` enum; the legal/illegal branches in +> `gmPaperDollUI::HandlePaperDollDragOver` (`AutoWearIsLegal` → 0x10000040 +> @ 0x004A3AC9, else 0x10000041 @ 0x004A3AEB) and +> `VendorSellUI::OnItemListDragOver` (`DragItemAcceptable` → 0x10000040 +> @ 0x004C2327, else 0x10000041 @ 0x004C2336); and the machine layout dump +> (`2026-06-25-retail-ui-layout-dump.json`: state 268435520 = 0x10000040 → +> image 0x060011F9, state 268435521 = 0x10000041 → 0x060011F8). The +> art-per-semantic mapping in the shipped code was always correct; only the +> numeric labels here were swapped. + Therefore the backpack contents grid uses the green circle; the side-bag column and main-pack container cell use the green drop-in arrow. The selected/open indicators remain visible while `m_elem_Icon_Ghosted` is active, so the diff --git a/src/AcDream.App/UI/Layout/InventoryController.cs b/src/AcDream.App/UI/Layout/InventoryController.cs index ec4f5e1b..f39b8e83 100644 --- a/src/AcDream.App/UI/Layout/InventoryController.cs +++ b/src/AcDream.App/UI/Layout/InventoryController.cs @@ -527,7 +527,7 @@ public sealed class InventoryController : IItemListDragHandler, IRetainedPanelCo /// /// Bind the exact ItemList_DragOver state for this destination. A normal contents-grid - /// insertion uses ItemSlot_DragOver_Accept (0x10000041 → green circle 0x060011F9). + /// insertion uses ItemSlot_DragOver_Accept (UIStateId 0x10000040 → green circle 0x060011F9). /// An occupied container selector uses ItemSlot_DragOver_DropIn /// (0x10000046 → green arrow 0x060011F7). Retail: 0x004e3400. /// diff --git a/src/AcDream.App/UI/Layout/SpellcastingUiController.cs b/src/AcDream.App/UI/Layout/SpellcastingUiController.cs index 8a6cf40e..d30366b5 100644 --- a/src/AcDream.App/UI/Layout/SpellcastingUiController.cs +++ b/src/AcDream.App/UI/Layout/SpellcastingUiController.cs @@ -349,7 +349,8 @@ public sealed class SpellcastingUiController : IRetainedPanelController uint id = spellId; int position = list.GetNumUIItems(); _spellbook.TryGetMetadata(id, out SpellMetadata? metadata); - var slot = new UiCatalogSlot + UiCatalogSlot? slot = null; + slot = new UiCatalogSlot { EntryId = id, CatalogIconTexture = metadata is null ? 0u : _resolveSpellIcon(id), @@ -358,10 +359,11 @@ public sealed class SpellcastingUiController : IRetainedPanelController CatalogDragPayload = new SpellFavoriteDragPayload(tab, position, id), DragBegan = payload => BeginFavoriteDrag((SpellFavoriteDragPayload)payload), DragEnded = payload => EndFavoriteDrag((SpellFavoriteDragPayload)payload), + DragOverAcceptance = FavoriteDragOverAcceptance, Dropped = payload => { if (payload is SpellFavoriteDragPayload favorite) - DropFavorite(favorite, targetTab, position); + DropFavorite(favorite, targetTab, list, slot!); else if (payload is SpellbookShortcutDragPayload shortcut) DropSpellbookShortcut(shortcut, targetTab, position); }, @@ -386,15 +388,17 @@ public sealed class SpellcastingUiController : IRetainedPanelController slot = new UiCatalogSlot { SpriteResolve = list.SpriteResolve, + DragOverAcceptance = FavoriteDragOverAcceptance, Dropped = payload => { - int position = Math.Max(0, list.IndexOf(slot!)); - int favoriteCount = _spellbook.GetFavorites(targetTab).Count; - position = Math.Min(position, favoriteCount); if (payload is SpellFavoriteDragPayload favorite) - DropFavorite(favorite, targetTab, position); + DropFavorite(favorite, targetTab, list, slot!); else if (payload is SpellbookShortcutDragPayload shortcut) + { + int position = Math.Max(0, list.IndexOf(slot!)); + position = Math.Min(position, _spellbook.GetFavorites(targetTab).Count); DropSpellbookShortcut(shortcut, targetTab, position); + } }, }; ConfigureShortcutOverlay(slot, shortcutIndex); @@ -450,16 +454,54 @@ public sealed class SpellcastingUiController : IRetainedPanelController _favoriteDragActive = false; } - private void DropFavorite(SpellFavoriteDragPayload payload, int targetTab, int targetPosition) + /// + /// SpellCastSubMenu::OnItemListDragOver @ 0x004C5990: while a drag hovers a + /// favorite-bar cell (occupied OR empty — retail's tail cells are UIItems in + /// the same list), retail sets the authored per-cell DragAccept child + /// (element 0x1000045A, bound in UIElement_UIItem::PostInit @ 0x004E1870) + /// to ItemSlot_DragOver_Accept (UIStateId 0x10000040 → authored ring + /// 0x060011F9) when the dragged payload carries a spell id, and leaves it + /// neutral otherwise — the handler returns 1, so the generic physical-item + /// fallback @ 0x004E3492 never runs for this list. + /// + internal static ItemDragAcceptance FavoriteDragOverAcceptance(object payload) + => payload is SpellFavoriteDragPayload or SpellbookShortcutDragPayload + ? ItemDragAcceptance.Accept + : ItemDragAcceptance.None; + + /// + /// THE one favorite-landing computation: the index a + /// drop on + /// applies. The drag-over Accept ring and both key + /// off the same hovered cell through this method, so the ring can never + /// promise a different landing than the drop delivers. Two numbering spaces, + /// both matching retail SpellCastSubMenu::AddFavorite @ 0x004C7060: + /// an OCCUPIED sibling cell is still numbered against the PRE-lift bar + /// (Rebuild defers for the gesture — AP-172), so retail's + /// -1-if-removed-from-before-target adjustment applies (the + /// RemoveSpellFromMenu-return-gated decrement @ 0x004C7157); an EMPTY tail + /// cell's index clamps to the LIVE favorite count — a post-lift number whose + /// lifted spell is already out of the live list, exactly retail's + /// RemoveSpellFromMenu == -1 no-adjustment case, so applying the -1 there too + /// would double-correct (the off-by-one this method retired: lifting a + /// non-last favorite onto the empty tail landed it second-to-last instead of + /// last). + /// + internal int FavoriteDropIndex( + SpellFavoriteDragPayload payload, int targetTab, UiItemList list, UiItemSlot cell) { - // Rebuild() was deferred for the whole gesture (see BeginFavoriteDrag), so - // every sibling slot's captured target index is still numbered against the - // PRE-lift list. Retail's own SpellCastSubMenu::AddFavorite @ 0x004C7060 - // corrects for exactly this staleness: when the lifted item's original - // index was before the drop target, the target index shifts down by one - // to land where the target visually sits once the gap closes. - if (payload.SourceTab == targetTab && payload.SourcePosition < targetPosition) - targetPosition -= 1; + int index = Math.Max(0, list.IndexOf(cell)); + if (cell.IsEmptySlot) + return Math.Min(index, _spellbook.GetFavorites(targetTab).Count); + if (payload.SourceTab == targetTab && payload.SourcePosition < index) + index -= 1; + return index; + } + + private void DropFavorite( + SpellFavoriteDragPayload payload, int targetTab, UiItemList list, UiItemSlot cell) + { + int targetPosition = FavoriteDropIndex(payload, targetTab, list, cell); _addFavorite?.Invoke(targetTab, targetPosition, payload.SpellId); _selected[targetTab] = payload.SpellId; } diff --git a/src/AcDream.App/UI/UiCatalogSlot.cs b/src/AcDream.App/UI/UiCatalogSlot.cs index a6d36372..cdb11c91 100644 --- a/src/AcDream.App/UI/UiCatalogSlot.cs +++ b/src/AcDream.App/UI/UiCatalogSlot.cs @@ -36,6 +36,19 @@ public sealed class UiCatalogSlot : UiItemSlot public Action? DragEnded { get; init; } public Action? Dropped { get; init; } + /// + /// Per-panel drag-over acceptance — the catalog-cell port of retail's + /// per-list drag handler deciding the rollover state + /// (UIElement_ItemList::ItemList_DragOver @ 0x004E3400 → + /// m_dragHandler->OnItemListDragOver; the spell bar's is + /// SpellCastSubMenu::OnItemListDragOver @ 0x004C5990). While a drag + /// hovers this cell, the returned acceptance drives the authored + /// DragAccept frame (accept 0x060011F9 / reject 0x060011F8). Null (the + /// default for every list that never set one) keeps the cell neutral — + /// no overlay, exactly the pre-existing presentation. + /// + public Func? DragOverAcceptance { get; init; } + protected override bool IsShortcutOccupied => EntryId != 0u; public override bool IsEmptySlot => EntryId == 0u; @@ -77,9 +90,25 @@ public sealed class UiCatalogSlot : UiItemSlot if (e.Payload is not null) DragBegan?.Invoke(e.Payload); return true; case UiEventType.DragEnter: + // Pointer entered mid-drag: ask the panel's acceptance seam and show + // the authored frame (retail ItemList_DragOver → the list handler's + // SetDragAcceptState on the hovered UIItem's 0x1000045A child). + SetDragAcceptVisual(e.Payload is { } enterPayload + ? DragOverAcceptance?.Invoke(enterPayload) switch + { + ItemDragAcceptance.Accept => DragAcceptState.Accept, + ItemDragAcceptance.Reject => DragAcceptState.Reject, + _ => DragAcceptState.None, + } + : DragAcceptState.None); + return true; case UiEventType.DragOver: + // UiRoot fires DragOver on LEAVE — retail's dwParam1 == 0 reset to + // ItemSlot_DragOver_Normal (0x1000003F) @ 0x004E1438-0x004E1456. + SetDragAcceptVisual(DragAcceptState.None); return true; case UiEventType.DropReleased: + SetDragAcceptVisual(DragAcceptState.None); if (e.Payload is not null) Dropped?.Invoke(e.Payload); return true; default: @@ -141,6 +170,10 @@ public sealed class UiCatalogSlot : UiItemSlot if (!string.IsNullOrWhiteSpace(Detail)) ctx.DrawString(Detail!, MathF.Max(labelLeft, Width - 48f), 3f, Vector4.One); } + + // Drag-rollover frame last — same layering as the physical UIItem draw + // (accept/reject above selection; catalog cells have no cooldown layer). + DrawDragAcceptOverlay(ctx); } private void DrawSelection(UiRenderContext ctx) diff --git a/src/AcDream.App/UI/UiItemSlot.cs b/src/AcDream.App/UI/UiItemSlot.cs index a63a6766..e6a67c74 100644 --- a/src/AcDream.App/UI/UiItemSlot.cs +++ b/src/AcDream.App/UI/UiItemSlot.cs @@ -46,10 +46,13 @@ public class UiItemSlot : UiElement public ItemDragSource SourceKind { get; set; } = ItemDragSource.Inventory; /// Drag-rollover accept frame (retail ItemSlot_DragOver_Accept 0x060011F9, - /// state id 0x10000041). Configurable; guard id != 0 before resolving. + /// UIStateId 0x10000040 — the state paperdoll AutoWearIsLegal @ 0x004A3AC7 and + /// VendorSellUI DragItemAcceptable @ 0x004C2320 set on a LEGAL target; the + /// 2026-06-16 deep-dive's prose swapped the 40/41 ids, the art column was right). + /// Configurable; guard id != 0 before resolving. public uint DragAcceptSprite { get; set; } = 0x060011F9u; /// Drag-rollover reject frame (retail ItemSlot_DragOver_Reject 0x060011F8, - /// state id 0x10000040). + /// UIStateId 0x10000041 — the illegal-target branch at 0x004A3AEB / 0x004C2336). public uint DragRejectSprite { get; set; } = 0x060011F8u; /// True when this cell is the OPEN container (its contents fill the grid). Draws the @@ -85,6 +88,13 @@ public class UiItemSlot : UiElement /// Current overlay state — internal so unit tests can assert it (InternalsVisibleTo). internal DragAcceptState DragAcceptVisual => _dragAccept; + /// Set the drag-rollover overlay state — the port of retail + /// UIElement_UIItem::SetDragAcceptState @ 0x004E1290 flipping the authored + /// per-cell m_elem_Icon_DragAccept child (element 0x1000045A, bound in + /// PostInit @ 0x004E1870). Subclasses whose drops bypass + /// (catalog cells) drive the same visual here. + private protected void SetDragAcceptVisual(DragAcceptState state) => _dragAccept = state; + /// Empty-slot sprite. Default = the generic toolbar empty-slot border /// 0x060074CF (uiitem template 0x21000037, state ItemSlot_Empty). Configurable so /// paperdoll equip slots can use their per-slot silhouettes later. @@ -445,19 +455,7 @@ public class UiItemSlot : UiElement ctx.DrawSprite(tex, 0f, 0f, Width, Height, 0f, 0f, 1f, 1f, Vector4.One); } - // Drag-rollover accept/reject frame (retail SetDragAcceptState 0x10000041/40). - // Guard id != 0 BEFORE resolving — resolve(0) returns the 1×1 magenta placeholder - // with a non-zero GL handle (feedback_ui_resolve_zero_magenta). - if (_dragAccept != DragAcceptState.None && SpriteResolve is not null) - { - uint id = _dragAccept == DragAcceptState.Accept ? DragAcceptSprite : DragRejectSprite; - if (id != 0) - { - var (tex, _, _) = SpriteResolve(id); - if (tex != 0) - ctx.DrawSprite(tex, 0f, 0f, Width, Height, 0f, 0f, 1f, 1f, Vector4.One); - } - } + DrawDragAcceptOverlay(ctx); // UIElement_UIItem::UpdateCooldownDisplay @ 0x004E1E20 selects exactly // one of ten authored radial overlays. The shared prototype gives these @@ -482,6 +480,24 @@ public class UiItemSlot : UiElement } } + /// Draws the drag-rollover accept/reject frame (retail + /// SetDragAcceptState writing ItemSlot_DragOver_Accept 0x10000040 / + /// _Reject 0x10000041 on the authored 0x1000045A child). Shared with catalog + /// cells the same way retail shares the one UIItem prototype (catalog 0x21000037). + /// Guard id != 0 BEFORE resolving — resolve(0) returns the 1×1 magenta placeholder + /// with a non-zero GL handle (feedback_ui_resolve_zero_magenta). + protected void DrawDragAcceptOverlay(UiRenderContext ctx) + { + if (_dragAccept == DragAcceptState.None || SpriteResolve is null) + return; + uint id = _dragAccept == DragAcceptState.Accept ? DragAcceptSprite : DragRejectSprite; + if (id == 0) + return; + var (tex, _, _) = SpriteResolve(id); + if (tex != 0) + ctx.DrawSprite(tex, 0f, 0f, Width, Height, 0f, 0f, 1f, 1f, Vector4.One); + } + /// Draws the shared retail shortcut-number layer. Catalog spell cells /// call this after their icon layer so magic favorites and physical shortcuts /// use the same UIItem presentation path. diff --git a/tests/AcDream.App.Tests/UI/Layout/SpellcastingUiControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/SpellcastingUiControllerTests.cs index 71c07b3a..e0acedc4 100644 --- a/tests/AcDream.App.Tests/UI/Layout/SpellcastingUiControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/SpellcastingUiControllerTests.cs @@ -275,6 +275,256 @@ public sealed class SpellcastingUiControllerTests Assert.Equal(3u, Assert.IsType(list.GetItem(2)).EntryId); } + /// + /// The user's retail memory after #354 landed: "the green ring indicator where + /// the spell icon should land, like in retail." Retail's mechanism is a slot + /// STATE, not a synthetic overlay: while a drag hovers any favorite-bar cell, + /// SpellCastSubMenu::OnItemListDragOver @ 0x004C5990 sets the authored + /// per-cell DragAccept child (element 0x1000045A, bound in + /// UIElement_UIItem::PostInit @ 0x004E1870) to ItemSlot_DragOver_Accept + /// (UIStateId 0x10000040 -> authored ring art 0x060011F9) whenever the + /// dragged payload carries a spell id; the pointer leaving the cell resets it + /// to ItemSlot_DragOver_Normal (0x1000003F) @ 0x004E1438. This drives the + /// whole gesture through UiRoot's real pointer pipeline and asserts the ring + /// appears on the hovered cell, tracks the pointer, survives a live per-frame + /// tick, clears on drop, and -- through the ONE shared computation + /// () -- never + /// promises a landing index different from the one the drop applies. + /// + [Fact] + public void SpellFavoriteDrag_ShowsAuthoredAcceptRing_TracksPointer_AndNeverLiesAboutTheLanding() + { + var (controller, spellbook, screen, list, adds, _) = BindPointerFixture(); + using SpellcastingUiController _1 = controller; + UiCatalogSlot slot0 = Assert.IsType(list.GetItem(0)); + UiCatalogSlot slot1 = Assert.IsType(list.GetItem(1)); + UiCatalogSlot slot2 = Assert.IsType(list.GetItem(2)); + (int x0, int y0) = CellCenter(slot0); + (int x1, int y1) = CellCenter(slot1); + (int x2, int y2) = CellCenter(slot2); + + screen.OnMouseDown(UiMouseButton.Left, x0, y0); + screen.OnMouseMove(x0 + 10, y0); + Assert.Same(slot0, screen.DragSource); + + // Hover spell 3's cell: the ring appears there and nowhere else, and the + // cell's accept frame is the authored ring art. + screen.OnMouseMove(x2, y2); + Assert.Equal(2, SingleAcceptRingIndex(list)); + Assert.Equal(0x060011F9u, slot2.DragAcceptSprite); + + // A live per-frame tick (production RetailUiRuntime.Tick) must not + // destroy the ringed cell -- the #354 deferral keeps the frozen bar. + controller.Tick(); + Assert.Equal(2, SingleAcceptRingIndex(list)); + + // The ring tracks the pointer: moving onto spell 2's cell moves it. + screen.OnMouseMove(x1, y1); + Assert.Equal(1, SingleAcceptRingIndex(list)); + Assert.Equal(UiItemSlot.DragAcceptState.None, slot2.DragAcceptVisual); + + // Back over spell 3's cell; capture what the ring promises through the + // one shared computation, then drop there. + screen.OnMouseMove(x2, y2); + var payload = Assert.IsType(screen.DragPayload); + int ringIndex = SingleAcceptRingIndex(list); + Assert.Equal(2, ringIndex); + int promised = controller.FavoriteDropIndex(payload, 0, list, list.GetItem(ringIndex)!); + + screen.OnMouseUp(UiMouseButton.Left, x2, y2); + + Assert.Null(screen.DragSource); + Assert.Equal([(0, promised, 1u)], adds); + Assert.Equal(new uint[] { 2u, 1u, 3u }, spellbook.GetFavorites(0)); + Assert.Equal(-1, SingleAcceptRingIndex(list)); + } + + /// + /// The empty-tail landing: retail's -1 adjustment in + /// SpellCastSubMenu::AddFavorite @ 0x004C7060 is gated on + /// RemoveSpellFromMenu's RETURN -- the dragged spell's LIVE list index at + /// add-time. At a drag-drop the spell already left the live list at lift + /// (RecvNotice_ItemListBeginDrag @ 0x004C7360), so a live-numbered target + /// gets NO adjustment. acdream's empty-cell index is clamped to the live + /// favorite count (a post-lift number); applying the pre-lift -1 on top of + /// that double-corrected: lifting a non-last favorite onto the empty tail + /// landed it second-to-last ([2,1,3]) instead of last ([2,3,1]). This drags + /// spell 1 onto the first empty cell through the real pointer pipeline and + /// pins the retail-faithful append -- and that the ring's promise equals + /// the landing. + /// + [Fact] + public void SpellFavoriteDrag_DroppedOnTheEmptyTail_AppendsAtTheEnd() + { + var (controller, spellbook, screen, list, adds, _) = BindPointerFixture(); + using SpellcastingUiController _1 = controller; + UiCatalogSlot slot0 = Assert.IsType(list.GetItem(0)); + UiCatalogSlot empty = Assert.IsType(list.GetItem(3)); + Assert.True(empty.IsEmptySlot); + (int x0, int y0) = CellCenter(slot0); + (int ex, int ey) = CellCenter(empty); + + screen.OnMouseDown(UiMouseButton.Left, x0, y0); + screen.OnMouseMove(x0 + 10, y0); + Assert.Same(slot0, screen.DragSource); + + // Retail's empty tail cells are UIItems in the same list -- they ring too. + screen.OnMouseMove(ex, ey); + Assert.Equal(3, SingleAcceptRingIndex(list)); + var payload = Assert.IsType(screen.DragPayload); + int promised = controller.FavoriteDropIndex(payload, 0, list, empty); + Assert.Equal(2, promised); // live count after the lift -- append at end + + screen.OnMouseUp(UiMouseButton.Left, ex, ey); + + Assert.Equal([(0, promised, 1u)], adds); + Assert.Equal(new uint[] { 2u, 3u, 1u }, spellbook.GetFavorites(0)); + Assert.Equal(-1, SingleAcceptRingIndex(list)); + } + + /// + /// Leaving the bar clears the ring (retail's dwParam1 == 0 reset to + /// ItemSlot_DragOver_Normal 0x1000003F @ 0x004E1438), and an off-bar release + /// keeps the lift's removal (retail's shape: the drag-begin + /// RemoveSpellFavorite stands when no drop lands). + /// + [Fact] + public void SpellFavoriteDrag_RingClearsOnLeave_AndAnOffBarReleaseKeepsTheLiftRemoval() + { + var (controller, spellbook, screen, list, adds, removes) = BindPointerFixture(); + using SpellcastingUiController _1 = controller; + UiCatalogSlot slot0 = Assert.IsType(list.GetItem(0)); + UiCatalogSlot slot2 = Assert.IsType(list.GetItem(2)); + (int x0, int y0) = CellCenter(slot0); + (int x2, int y2) = CellCenter(slot2); + + screen.OnMouseDown(UiMouseButton.Left, x0, y0); + screen.OnMouseMove(x0 + 10, y0); + screen.OnMouseMove(x2, y2); + Assert.Equal(2, SingleAcceptRingIndex(list)); + + // Pointer leaves every cell (screen corner, off the bar). + screen.OnMouseMove((int)screen.Width - 2, (int)screen.Height - 2); + Assert.Equal(-1, SingleAcceptRingIndex(list)); + + screen.OnMouseUp(UiMouseButton.Left, (int)screen.Width - 2, (int)screen.Height - 2); + + Assert.Null(screen.DragSource); + Assert.Empty(adds); + Assert.Equal([(0, 1u)], removes); + Assert.Equal(new uint[] { 2u, 3u }, spellbook.GetFavorites(0)); + Assert.Equal(-1, SingleAcceptRingIndex(list)); + } + + /// + /// Retail's spell-bar handler keys the ring off the dragged payload carrying + /// a spell id (@ 0x004C5990: SetDragAcceptState(0x10000040) only when the + /// InqDropIconInfo spellID != 0, returning 1 so the generic physical-item + /// fallback @ 0x004E3492 never runs for this list). A physical inventory + /// payload therefore leaves favorite cells NEUTRAL -- no ring, no reject -- + /// while both spell payload kinds ring, on occupied and empty cells alike. + /// + [Fact] + public void FavoriteCellRing_AcceptsSpellPayloads_StaysNeutralForPhysicalItems() + { + var (controller, _, _, list, _, _) = BindPointerFixture(); + using SpellcastingUiController _1 = controller; + UiCatalogSlot occupied = Assert.IsType(list.GetItem(0)); + UiCatalogSlot empty = Assert.IsType(list.GetItem(3)); + var physical = new ItemDragPayload(9u, ItemDragSource.Inventory, 0, new UiItemSlot(), null); + + occupied.OnEvent(new UiEvent(0, occupied, UiEventType.DragEnter, Payload: physical)); + Assert.Equal(UiItemSlot.DragAcceptState.None, occupied.DragAcceptVisual); + + occupied.OnEvent(new UiEvent( + 0, occupied, UiEventType.DragEnter, Payload: new SpellbookShortcutDragPayload(42u))); + Assert.Equal(UiItemSlot.DragAcceptState.Accept, occupied.DragAcceptVisual); + occupied.OnEvent(new UiEvent(0, occupied, UiEventType.DragOver)); + Assert.Equal(UiItemSlot.DragAcceptState.None, occupied.DragAcceptVisual); + + empty.OnEvent(new UiEvent( + 0, empty, UiEventType.DragEnter, Payload: new SpellFavoriteDragPayload(0, 0, 1u))); + Assert.Equal(UiItemSlot.DragAcceptState.Accept, empty.DragAcceptVisual); + empty.OnEvent(new UiEvent(0, empty, UiEventType.DropReleased, Payload: physical)); + Assert.Equal(UiItemSlot.DragAcceptState.None, empty.DragAcceptVisual); + } + + /// Shared real-pointer fixture: favorites [1,2,3] on tab 0, the layout + /// mounted in a UiRoot, adds/removes recorded and applied to the Spellbook. + private (SpellcastingUiController Controller, + Spellbook Spellbook, + UiRoot Screen, + UiItemList List, + List<(int Tab, int Position, uint SpellId)> Adds, + List<(int Tab, uint SpellId)> Removes) BindPointerFixture() + { + ImportedLayout layout = LayoutImporter.Build( + FixtureLoader.LoadCombatInfos(), NoTex, datFont: null); + RetailCombatLayout.FitFavoriteSlots(layout); + var spellbook = new Spellbook(); + spellbook.OnSpellLearned(1u, 1f); + spellbook.OnSpellLearned(2u, 1f); + spellbook.OnSpellLearned(3u, 1f); + spellbook.SetFavorite(0, 0, 1u); + spellbook.SetFavorite(0, 1, 2u); + spellbook.SetFavorite(0, 2, 3u); + var objects = new ClientObjectTable(); + objects.AddOrUpdate(new ClientObject { ObjectId = 1u, Name = "Player" }); + var selection = new SelectionState(); + var casting = new RuntimeSpellCastState(spellbook, selection, new NoopSpellCastOperations()); + var adds = new List<(int Tab, int Position, uint SpellId)>(); + var removes = new List<(int Tab, uint SpellId)>(); + + SpellcastingUiController? controller = SpellcastingUiController.Bind( + layout, spellbook, casting, objects, () => 1u, + spellId => spellId, + item => item.ObjectId, + _ => { }, + selection, + (tab, position, spellId) => + { + adds.Add((tab, position, spellId)); + spellbook.SetFavorite(tab, position, spellId); + }, + (tab, spellId) => + { + removes.Add((tab, spellId)); + spellbook.RemoveFavorite(tab, spellId); + }); + Assert.True(controller is not null, DescribeBinding(layout)); + + var screen = new UiRoot { Width = 1280f, Height = 800f }; + screen.AddChild(layout.Root); + controller!.Tick(); + + UiElement group = layout.FindElement(0x100000AAu)!; + UiItemList list = Descendants(group).OfType().First(); + return (controller, spellbook, screen, list, adds, removes); + } + + private static (int X, int Y) CellCenter(UiItemSlot cell) + { + System.Numerics.Vector2 p = cell.ScreenPosition; + return ((int)(p.X + cell.Width / 2f), (int)(p.Y + cell.Height / 2f)); + } + + /// Index of the single cell showing the Accept ring, or -1 when none; + /// fails the test if more than one cell rings at once. + private static int SingleAcceptRingIndex(UiItemList list) + { + int found = -1; + for (int i = 0; i < list.GetNumUIItems(); i++) + { + if (list.GetItem(i) is { } cell + && cell.DragAcceptVisual == UiItemSlot.DragAcceptState.Accept) + { + Assert.Equal(-1, found); + found = i; + } + } + return found; + } + [Fact] public void FavoriteDrop_IgnoresForeignInventoryPayload() {