From e602f84be2ed5bafeb337dccf22c5f12b6dbf8c3 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 7 Aug 2026 18:26:17 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20Slice=205.4=20review=20corrections?= =?UTF-8?q?=20=E2=80=94=20the=20dropdown=20renders=20from=20its=20authored?= =?UTF-8?q?=20popup,=20retail=20cost=20semantics,=20auto-select,=20icon=20?= =?UTF-8?q?overlays?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All nine review findings closed at root (one sub-item consciously deferred): F1 the category dropdown now draws: sprites/fonts wired and the popup geometry read from the vendor menu's own authored popup LayoutDesc 0x21000043 (root 0x1000034F — correcting the review's 0x1000014F transcription) per UIElement_Menu::MakePopup (pc:120705); chat's menu is untouched and its tests prove it. The new test drives selection through the REAL open/hit path the review flagged as bypassed. F2+F3 the selected-item cost display ports VendorItemsUI::UpdateItemsUI verbatim: quantity via the 0xDC41CB0 split-size mask (whole-stack for ammo, per-unit for groceries/components; mask lives at the toolbar SEEDING site pc:198784), plural names with retail's fall-back-to-singular (pc:409056 — correcting the review's "name+s" guess), full cost sentences with comma grouping and the player's coin total, and Buy/Add buttons that disable without a selection. F4 category switches auto-select the first filtered item (pc:201180). F5 icon underlay/overlay/effects + plural name forwarded from the already-parsed wire fields through VendorShopItem to the icon composer. F6 a DIFFERENT vendor opens on its own first category; same-vendor refresh preserves per the clamp. F7 scroll resets on rebuild and authored empty slots fill; the right-click examine route is consciously DEFERRED (shop items are not in ClientObjectTable and the appraisal panel hard-requires it — documented, not faked). F8 VendorState.Apply's fanout gets the same per-listener isolation as Close/Reset. F9 AP-110/AP-161 wording corrected ("quantity-correct pricing") and AP-161 rewritten to exactly the remaining conscious gaps. Clean-room complete solution with the #348 cursor fix in the same tree: 11,334 passed / 4 skipped / 0 failed. Co-Authored-By: Claude Sonnet 4.5 --- .../retail-divergence-register.md | 6 +- .../UI/Layout/VendorUiController.cs | 335 +++++++++++++++++- src/AcDream.Core.Net/GameEventWiring.cs | 13 +- src/AcDream.Core/Items/VendorState.cs | 45 ++- .../UI/Layout/VendorUiControllerTests.cs | 309 +++++++++++++++- .../Items/VendorStateTests.cs | 23 ++ 6 files changed, 696 insertions(+), 35 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index cc6a59fb..12869862 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -170,7 +170,7 @@ readiness/requeue adaptation. See --- -## 3. Documented approximation (AP) — 112 active rows (AP-161 filed 2026-08-09 at Slice 5.4 (vendor browse panel) — the authored "Buying"/"Selling" tabs render and switch pages but carry no data binding, per contract decision 8's required successor to AP-110's narrowing; AP-110 NARROWED the same day — "vendor" is retired from its absent-panels list now that the "Items" browse tab is user-reachable. AP-160 filed 2026-08-07 at Slice 5.3 — the client-local vendor-panel distance watcher closes on plain 3D center distance instead of retail/ACE's cylinder-gap distance, because Runtime has no per-entity collision radius/height source outside the App-layer's Setup-cylinder resolver. AP-158 RETIRED 2026-08-06 by the #333 fix, closing #337 — the `maxReach` distance pre-filter is DELETED rather than re-centred, because retail has none: `CObjCell::find_obj_collisions` @0x0052b750 walks the cell's shadow list and calls `CPhysicsObj::FindObjCollisions` unconditionally. The row's predicted symptom was observed live at Neftet before it was fixed — a tall prop AP-156 had just placed correctly still not blocking, plus jumps sinking into the mesh and corpses falling through. Perf measured, not assumed: at the live-maximum 38 in-cell candidates 10.61 µs → 16.68 µs per resolve. AP-159 filed 2026-08-06 at the #334 fix — the INDOOR half of AP-156’s traversal residual is all that remains of it; the outdoor half is CLOSED by the `find_bbox_cell_list` port, and AP-156’s RISK COLUMN IS CORRECTED at the same commit: it recorded the residual as “extra broadphase candidates, never a missed one”, which generalised the indoor direction to the whole row and is exactly why #334 — a MISSED one, and a user-observed loss of collision on landblock-spanning formations — sat inside it unnoticed. AP-158 filed 2026-08-06 at the AP-156 fix review — the shadow broadphase's `maxReach` distance pre-filter is acdream's own invention with NO retail counterpart, and it measures from the part origin, so it can discard a genuine contact for exactly the off-centre parts AP-156 just placed correctly; issue #333. AP-156 CORRECTED at the same review: its population was understated — 172 is AP-152's DISPATCH population, not AP-156's CONTAINMENT population. AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 428 of the 530 BSP-bearing Setups (the AP-156 fix review corrected the originally-recorded '170 of 172'), because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) +## 3. Documented approximation (AP) — 112 active rows (AP-161 REWRITTEN 2026-08-09 at the Slice 5.4 review (findings F1-F8) — the popup-never-rendered, wrong-quantity-price, no-auto-select, dropped-icon-layer, stale-category-on-vendor-switch, and unguarded-Apply-fanout bugs the review found are fixed (`VendorUiController.cs`, `VendorState.cs`, `GameEventWiring.cs`, `RetailUiRuntime.cs`); the row now records only the four consciously-deferred residuals it still owns (private per-panel selection vs. retail's global `ACCWeenieObject::selectedID`, the unwired shop-item examine route, the dropdown button-face arrow-cap glyph, and the alt-currency held-amount's `m_last_sale`-free simplification). AP-110's "retail-correct per-unit prices" phrasing is corrected the same day to "quantity-correct pricing" — the OLD phrase mischaracterized what retail even shows (a `GetObjectSplitSize`-quantity price, not literally one unit) independent of whether the code was buggy. AP-161 filed 2026-08-09 at Slice 5.4 (vendor browse panel) — the authored "Buying"/"Selling" tabs render and switch pages but carry no data binding, per contract decision 8's required successor to AP-110's narrowing; AP-110 NARROWED the same day — "vendor" is retired from its absent-panels list now that the "Items" browse tab is user-reachable. AP-160 filed 2026-08-07 at Slice 5.3 — the client-local vendor-panel distance watcher closes on plain 3D center distance instead of retail/ACE's cylinder-gap distance, because Runtime has no per-entity collision radius/height source outside the App-layer's Setup-cylinder resolver. AP-158 RETIRED 2026-08-06 by the #333 fix, closing #337 — the `maxReach` distance pre-filter is DELETED rather than re-centred, because retail has none: `CObjCell::find_obj_collisions` @0x0052b750 walks the cell's shadow list and calls `CPhysicsObj::FindObjCollisions` unconditionally. The row's predicted symptom was observed live at Neftet before it was fixed — a tall prop AP-156 had just placed correctly still not blocking, plus jumps sinking into the mesh and corpses falling through. Perf measured, not assumed: at the live-maximum 38 in-cell candidates 10.61 µs → 16.68 µs per resolve. AP-159 filed 2026-08-06 at the #334 fix — the INDOOR half of AP-156’s traversal residual is all that remains of it; the outdoor half is CLOSED by the `find_bbox_cell_list` port, and AP-156’s RISK COLUMN IS CORRECTED at the same commit: it recorded the residual as “extra broadphase candidates, never a missed one”, which generalised the indoor direction to the whole row and is exactly why #334 — a MISSED one, and a user-observed loss of collision on landblock-spanning formations — sat inside it unnoticed. AP-158 filed 2026-08-06 at the AP-156 fix review — the shadow broadphase's `maxReach` distance pre-filter is acdream's own invention with NO retail counterpart, and it measures from the part origin, so it can discard a genuine contact for exactly the off-centre parts AP-156 just placed correctly; issue #333. AP-156 CORRECTED at the same review: its population was understated — 172 is AP-152's DISPATCH population, not AP-156's CONTAINMENT population. AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 428 of the 530 BSP-bearing Setups (the AP-156 fix review corrected the originally-recorded '170 of 172'), because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) Wave-0 UI ledger repair (2026-07-10) retired stale AP-38, resolved the AP-84 collision, restored overwritten paperdoll rows as AP-92/AP-93, and registered @@ -287,8 +287,8 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | ~~AP-107~~ | **RETIRED 2026-07-11 (Wave 3.3 / #197)** — typed `OfferPrimaryClick` returns `NotActive`, `ConsumedSuccess`, or `ConsumedRejected`; every retained item surface plus radar/world offers active target mode before local selection/open/use fallback. Rejections are consumed and cannot drift selection. | `src/AcDream.App/UI/ItemInteractionController.cs`; inventory/paperdoll/toolbar/radar/world call sites | — | — | `UIElement_ItemList::HandleTargetedUseLeftClick @ 0x004E24D0` | | AP-108 | Paperdoll/AutoWield still omit the remaining missile/held restrictions and corrupt-mask branch of full `AutoWieldIsLegal`, dual-wield/off-hand rules, double-click examine/drag from the doll, body-part selection lighting, and retail's synchronous `" - cannot unwield the %s"` failure suffix (the current send seam reports rejection asynchronously). **AutoWear legality retired from this row 2026-07-23:** inventory activation and paperdoll drops now apply the retail clothing-priority/location blocker lookup and exact `"You must remove your %s to wear that"` system notice. **Primary replacement retired 2026-07-14; Aetheria retired 2026-07-13.** | `src/AcDream.App/UI/Layout/PaperdollController.cs`; `src/AcDream.App/UI/AutoWieldController.cs` | Basic equip slots, Aetheria, live doll, AutoWear conflict reporting, and primary weapon/incompatible shield/mismatched ammo blocker sequencing work in peace and war | Remaining illegal/off-hand cases, asynchronous dequip rejection wording, doll examine/drag, and selection lighting still differ functionally | `CPlayerSystem::AutoWieldIsLegal @ 0x0055ED60`; `CPlayerSystem::AutoWearIsLegal @ 0x0055EF40`; `CPlayerSystem::AutoWield @ 0x00560A60`; `gmPaperDollUI @ 0x004A3590..0x004A5F90` | | AP-109 | Character Titles page is inert and live displayed-title/luminance state is absent | `src/AcDream.App/UI/Layout/CharacterStatController.cs`; `CharacterSheetProvider.cs` | Attributes/skills core output is user-accepted | Titles cannot be selected/displayed and level-200 luminance fields are missing | `gmCharacterTitleUI @ 0x0049A610`; `gmStatManagementUI::UpdateExperience @ 0x004F0A70` | -| AP-110 | **NARROWED 2026-08-09 (Slice 5.4, vendor browse panel) — "vendor" retired from the absent-panels list; see AP-161 for the precise successor (Buy/Sell transaction UI, Slice 6).** Remaining retained gameplay panels and world HUD are absent: advanced-combat powerbar, residual social/floating chat, quests/map/options/smartbox, trade/salvage/tinkering, mini-game gameplay, Link Status NAK/retransmission packet-loss averaging, and D.6 nameplates/floaters. Examination has its independent authored floaty layout, inscription transaction, retail creature stat/rating/animated-preview presentation, default selection-follow, authored local spell subview with appropriate-formula component state, and the full EoR item-report dispatch: appraisal-only unknowns; exact equipment-set/rating/tinkering/weapon/armor/caster/requirement/XP/healer/rare prose and intentional blank section rows; ordinary/enchantment DAT spell descriptions; live material-decorated appropriate titles plus DAT material and creature names; expiry, decorated material/gem descriptions; and portal/PK restrictions with authored item colors. It still lacks item-object preview, player-dependent effective shield projection, live cooldown-remaining projection, localized augmentation-cost `StringInfo`, exhaustive character detail regions, and exact creature appraisal FontInfo-list selection. | `src/AcDream.App/UI/RetailUiRuntime.cs`; `src/AcDream.App/UI/Layout/AppraisalUiController.cs`; `src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs`; `src/AcDream.App/UI/Layout/RetailAppraisalNameResolver.cs`; `src/AcDream.App/UI/Layout/CreatureAppraisalRows.cs`; `src/AcDream.App/Rendering/CreatureAppraisalPresentation.cs`; `src/AcDream.Core.Net/LinkStatusSnapshot.cs`; D.5/D.6 roadmap | Basic combat, M3 magic/Link/Vitae surfaces, the Slice 5.4 vendor "Items" browse panel (category-filtered stock list, retail-correct per-unit prices), and the core examination request/presentation/inscription/creature-preview/item-report loop cover the active loops; the residual examination mechanisms require live player/enchantment/localization state or object-preview ownership rather than fabricated content | Item assessments omit only the listed live/localized/preview projections; enchanted/incomplete creature appraisal rows use the normal authored font until the exact FontInfo list is bound; other absent panels remain unavailable; real packet loss is displayed as 0.00% instead of retail's moving average | `BasicCreatureExamineUI::Init @ 0x004AB9C0`; `CreatureExamineUI::SetAppraiseInfo @ 0x004B3FF0`; `gmExaminationUI::RecvNotice_SelectionChanged @ 0x004AB3D0`; `gmExaminationUI::ExamineSpell @ 0x004B6900`; `SpellExamineUI::ExamineSpell @ 0x004B6210`; `AttributeInfoRegion::Update @ 0x004F1D90`; `gmExaminationUI::SetAppraiseInfo @ 0x004ADAE0`; `ACCWeenieObject::GetObjectName @ 0x0058E6E0`; `ItemExamineUI::SetAppraiseInfo @ 0x004B72B0`; `ItemExamineUI::AddItemInfo @ 0x004AC050`; `ItemExamineUI::Appraisal_ShowCapacity @ 0x004B2680`; `ItemExamineUI::Appraisal_ShowSpecialProperties @ 0x004B0140`; `ItemExamineUI::Appraisal_ShowWeaponAndArmorData @ 0x004B10E0`; `ItemExamineUI::Appraisal_ShowMagicInfo @ 0x004B2E10`; `ItemExamineUI::Appraisal_ShowDescription @ 0x004B6990`; `MaterialTypeEnumMapper::MaterialTypeToString @ 0x005CD500`; `ItemExamineUI::SetInscription @ 0x004AE2F0`; `CM_Writing::Event_SetInscription @ 0x006A98B0`; `CLinkStatusAverages::GetAveragePacketLoss @ 0x00546610`; LayoutDesc catalog | -| AP-161 | **Filed 2026-08-09, Slice 5.4 (vendor browse panel) — the precise successor row decision 8 of the Slice 5 contract requires.** `VendorUiController` mounts LayoutDesc `0x21000012`/root `0x100000B7` and fully wires only the authored "Items" tab (`0x100000B9` — `VendorItemsUI`: category-filtered browse list, retail-correct per-unit prices). The other two authored tabs render and switch pages (so the layout looks complete) but are otherwise INERT: "Buying" (`0x100000BA`, `VendorBuyUI` — staged-purchase review/confirm, buttons `0x100000C9`/`CA`/`CB`/`CC`) and "Selling" (`0x100000BB`, `VendorSellUI` — staged-sale review/confirm, buttons `0x100000D2`/`D3`/`D4`/`D5`) have no data binding at all. The "Items" page's own `Buy`/`Add to List` buttons (`0x100000C2`/`C3`) are also present but unwired. No buy/sell wire message exists anywhere in this slice; `VendorProfile::InqAcceptability` (sell-eligibility filtering) is unread. | `src/AcDream.App/UI/Layout/VendorUiController.cs` | Slice 6 (`docs/plans/2026-07-23-world-interaction-completion.md`) owns the authoritative buy/sell transaction command, quantity/stack-split selection, drag-to-sell consumption, and `InqAcceptability`-gated sell UI — Slice 5.4's charter is browse only | A player can look at prices and select items but cannot buy or sell anything; clicking "Buying"/"Selling" shows an empty page with no error or explanation, matching "present but does nothing" rather than a disabled/hidden affordance | `gmVendorUI::OpenVendor` pc:203650 (`m_itemsUI`/`m_buyUI`/`m_sellUI` construction, `PostInit` pc:199906); `VendorBuyUI::VendorBuyUI` pc:199717; `VendorSellUI::VendorSellUI` pc:199753; `VendorProfile::InqAcceptability` pc:484768-484797; `docs/research/2026-08-08-slice5-vendor-browse-research.md` §B.4, §D | +| AP-110 | **NARROWED 2026-08-09 (Slice 5.4, vendor browse panel) — "vendor" retired from the absent-panels list; see AP-161 for the precise successor (Buy/Sell transaction UI, Slice 6).** Remaining retained gameplay panels and world HUD are absent: advanced-combat powerbar, residual social/floating chat, quests/map/options/smartbox, trade/salvage/tinkering, mini-game gameplay, Link Status NAK/retransmission packet-loss averaging, and D.6 nameplates/floaters. Examination has its independent authored floaty layout, inscription transaction, retail creature stat/rating/animated-preview presentation, default selection-follow, authored local spell subview with appropriate-formula component state, and the full EoR item-report dispatch: appraisal-only unknowns; exact equipment-set/rating/tinkering/weapon/armor/caster/requirement/XP/healer/rare prose and intentional blank section rows; ordinary/enchantment DAT spell descriptions; live material-decorated appropriate titles plus DAT material and creature names; expiry, decorated material/gem descriptions; and portal/PK restrictions with authored item colors. It still lacks item-object preview, player-dependent effective shield projection, live cooldown-remaining projection, localized augmentation-cost `StringInfo`, exhaustive character detail regions, and exact creature appraisal FontInfo-list selection. | `src/AcDream.App/UI/RetailUiRuntime.cs`; `src/AcDream.App/UI/Layout/AppraisalUiController.cs`; `src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs`; `src/AcDream.App/UI/Layout/RetailAppraisalNameResolver.cs`; `src/AcDream.App/UI/Layout/CreatureAppraisalRows.cs`; `src/AcDream.App/Rendering/CreatureAppraisalPresentation.cs`; `src/AcDream.Core.Net/LinkStatusSnapshot.cs`; D.5/D.6 roadmap | Basic combat, M3 magic/Link/Vitae surfaces, the Slice 5.4 vendor "Items" browse panel (category-filtered stock list, retail's quantity-correct pricing — `ItemHolder::GetObjectSplitSize`'s split-exempt mask, not a flat per-unit price), and the core examination request/presentation/inscription/creature-preview/item-report loop cover the active loops; the residual examination mechanisms require live player/enchantment/localization state or object-preview ownership rather than fabricated content | Item assessments omit only the listed live/localized/preview projections; enchanted/incomplete creature appraisal rows use the normal authored font until the exact FontInfo list is bound; other absent panels remain unavailable; real packet loss is displayed as 0.00% instead of retail's moving average | `BasicCreatureExamineUI::Init @ 0x004AB9C0`; `CreatureExamineUI::SetAppraiseInfo @ 0x004B3FF0`; `gmExaminationUI::RecvNotice_SelectionChanged @ 0x004AB3D0`; `gmExaminationUI::ExamineSpell @ 0x004B6900`; `SpellExamineUI::ExamineSpell @ 0x004B6210`; `AttributeInfoRegion::Update @ 0x004F1D90`; `gmExaminationUI::SetAppraiseInfo @ 0x004ADAE0`; `ACCWeenieObject::GetObjectName @ 0x0058E6E0`; `ItemExamineUI::SetAppraiseInfo @ 0x004B72B0`; `ItemExamineUI::AddItemInfo @ 0x004AC050`; `ItemExamineUI::Appraisal_ShowCapacity @ 0x004B2680`; `ItemExamineUI::Appraisal_ShowSpecialProperties @ 0x004B0140`; `ItemExamineUI::Appraisal_ShowWeaponAndArmorData @ 0x004B10E0`; `ItemExamineUI::Appraisal_ShowMagicInfo @ 0x004B2E10`; `ItemExamineUI::Appraisal_ShowDescription @ 0x004B6990`; `MaterialTypeEnumMapper::MaterialTypeToString @ 0x005CD500`; `ItemExamineUI::SetInscription @ 0x004AE2F0`; `CM_Writing::Event_SetInscription @ 0x006A98B0`; `CLinkStatusAverages::GetAveragePacketLoss @ 0x00546610`; LayoutDesc catalog | +| AP-161 | **Filed 2026-08-09, Slice 5.4 (vendor browse panel); REWRITTEN 2026-08-09 at the Slice 5.4 review (findings F1-F8) — the popup-never-rendered, wrong-quantity-price, no-auto-select, dropped-icon-layers, stale-category, and unguarded-fanout bugs the review found are all FIXED; this row now covers exactly what CONSCIOUSLY remains.** `VendorUiController` mounts LayoutDesc `0x21000012`/root `0x100000B7` and fully wires only the authored "Items" tab (`0x100000B9` — `VendorItemsUI`: category-filtered browse list with retail's quantity-correct pricing, `ItemHolder::GetObjectSplitSize`'s `0xDC41CB0` split-exempt mask ported locally rather than a flat per-unit price). The other two authored tabs render and switch pages (so the layout looks complete) but are otherwise INERT: "Buying" (`0x100000BA`, `VendorBuyUI` — staged-purchase review/confirm, buttons `0x100000C9`/`CA`/`CB`/`CC`) and "Selling" (`0x100000BB`, `VendorSellUI` — staged-sale review/confirm, buttons `0x100000D2`/`D3`/`D4`/`D5`) have no data binding at all. The "Items" page's own `Buy`/`Add to List` buttons (`0x100000C2`/`C3`) now correctly enable/disable with selection (`UiButton.Enabled`, retail `SetState(1)`/`SetState(0xd)`) but clicking them (once Slice 6 wires an action) remains unimplemented. No buy/sell wire message exists anywhere in this slice; `VendorProfile::InqAcceptability` (sell-eligibility filtering) is unread. Four divergences the F1-F8 fix pass consciously LEFT: (1) item selection is a PRIVATE per-panel `_selectedItemGuid`, not retail's global `ACCWeenieObject::selectedID`/`GenItemHolder::splitSize` pair — a toolbar Examine/Use on the same guid, or another panel's selection, neither reads nor drives this panel's highlight; (2) right-click examine on a shop item is NOT wired (F7c, STOPped rather than faked) — `AppraisalUiController.Apply` requires a live `ClientObjectTable` entry (`_objects.Get(appraisal.Guid)`, `src/AcDream.App/UI/Layout/AppraisalUiController.cs:418-420`) and vendor items are never registered there (`GameEventWiring.cs`'s `ApproachVendor` handler only calls `vendor?.Apply(...)`, never touches `items`/`ClientObjectTable`); faking the wiring would silently swallow every appraisal response instead of surfacing the gap; (3) the closed-dropdown button face reuses the row template's own two sprites (`0x060012B3` normal/`0x060012B4` open) through `UiMenu`'s existing single-texture 3-slice `DrawButtonFace` instead of retail's authored two-piece label+arrow-cap assembly (label `0x1000034D` + a separate 17x19 arrow cap `0x1000034E` with its own `0x060012B1`/`0x060012B2` states) — a cosmetic gap only; the popup panel and its rows render with the exact authored geometry and sprites; (4) the alt-currency "you have" holding reads `VendorShopProfile.AlternateCurrencyAmount` directly instead of tracking retail's `gmVendorUI.m_last_sale` purchase debit — moot until Slice 6 adds a purchase path, since `m_last_sale` only changes on a completed buy (retail's own `m_last_sale == 0` case, `pc:204091`/`OpenVendor`'s `this->m_last_sale = 0` reset at `pc:203790`/`203800`). | `src/AcDream.App/UI/Layout/VendorUiController.cs`; `src/AcDream.Core/Items/VendorState.cs`; `src/AcDream.Core.Net/GameEventWiring.cs`; `src/AcDream.App/UI/RetailUiRuntime.cs` | Slice 6 (`docs/plans/2026-07-23-world-interaction-completion.md`) owns the authoritative buy/sell transaction command, quantity/stack-split selection, drag-to-sell consumption, and `InqAcceptability`-gated sell UI — Slice 5.4's charter is browse only. The global `ACCWeenieObject::selectedID` coupling and shop-item `ClientObjectTable` registration are the SAME Slice 6/global-selection-seam work already fenced by the original filing; the F1-F8 review fix did not expand that fence, it only corrected the popup/pricing/selection/icon DISPLAY bugs living inside the existing private-selection model. | A player can look at prices and select items but cannot buy or sell anything; clicking "Buying"/"Selling" shows an empty page with no error or explanation, matching "present but does nothing" rather than a disabled/hidden affordance. Right-clicking a shop item to examine it does nothing at all (not even a request sent) — Slice 6 must add the `ClientObjectTable` registration before wiring examine, not aim it at an unrelated player-inventory or world-entity guid. The dropdown's closed-state button face is missing its separate arrow-cap glyph — a minor visual gap, not a functional one; the open popup itself is pixel-faithful to the authored template. | `gmVendorUI::OpenVendor` pc:203650 (`m_itemsUI`/`m_buyUI`/`m_sellUI` construction, `PostInit` pc:199906, `m_last_sale` reset pc:203790/203800); `VendorBuyUI::VendorBuyUI` pc:199717; `VendorSellUI::VendorSellUI` pc:199753; `VendorProfile::InqAcceptability` pc:484768-484797; `UIElement_Menu::MakePopup` pc:120705-120764, `::Initialize` pc:120789-120828; `VendorItemsUI::UpdateItemsUI` pc:202539-202820; `VendorItemsUI::UpdateItemsList` pc:201029-201190; `ItemHolder::GetObjectSplitSize` pc:401465-401477; `gmToolbarUI::HandleSelectionChanged` pc:198740-198790 (mask `0xDC41CB0` at pc:198784); `ACCWeenieObject::GetObjectName` pc:409056-409132; `docs/research/2026-08-08-slice5-vendor-browse-research.md` §B.4, §D | | ~~AP-111~~ | **RETIRED 2026-07-11 (M2 held-object parenting)** — equipped hand items are no longer omitted from the render world. CreateObject now preserves Placement/Parent/position timestamp bootstrap; live `0xF749` ParentEvent is parsed with retail sequence freshness; a focused render controller resolves `Setup.HoldingLocations`, applies the child's placement frame, and recomposes the separate child entity after every parent animation tick. Pickup retains the weenie's visual metadata for a later wield. | `src/AcDream.Core.Net/Messages/{CreateObject,ParentEvent}.cs`; `src/AcDream.Core/Meshing/EquippedChildAttachment.cs`; `src/AcDream.App/Rendering/EquippedChildRenderController.cs` | — | — | `ClientCombatSystem::GetDefaultCombatMode @ 0x0056B310`; `SmartBox::HandleParentEvent @ 0x004535D0`; `CPhysicsObj::set_parent @ 0x00515A90`; `CPhysicsObj::UpdateChild @ 0x00512D50` | | AP-112 | The basic combat bar ports visibility, height selection, desired-power slider, exact 1.0/0.8-second charge, ready-stance gating, request/release, `MaybeStopCompletely`, server-response queueing, and auto-repeat, but still omits `StartAttackRequest`'s `FinishJump` call and exact trained-Recklessness visibility semantics (IA-20 keeps the dark range as the accepted baseline) | `src/AcDream.Runtime/Gameplay/RuntimeCombatAttackState.cs`; `src/AcDream.App/UI/Layout/CombatUiController.cs` | The shared player movement owner now performs retail's server-control-gated full stop and movement report before an attack build; the remaining seams require the jump owner and a distinct Recklessness treatment | Starting an attack while charging a jump may not finish that jump exactly when retail does; trained/untrained Recklessness presentation is identical | `ClientCombatSystem::StartAttackRequest @ 0x0056C040`; `CommandInterpreter::MaybeStopCompletely @ 0x006B3B90`; `gmCombatUI::ListenToElementMessage @ 0x004CC430` | | AP-113 | Invalid lifestone-command arguments display the local text `Usage: /lifestone`; retail definitely emits a local usage/error line but Binary Ninja misidentifies the referenced wide-string address, so its exact wording is not yet recovered | `src/AcDream.UI.Abstractions/Panels/Chat/ChatCommandRouter.cs`; `RetailClientCommandCatalog.cs` | The behavior boundary is exact (handled locally, no chat and no game action); only a low-impact diagnostic sentence differs | `/ls now` can show different wording/color from retail while still refusing the invalid request correctly | `ClientCommunicationSystem::DoLifestone @ 0x0056FC70` | diff --git a/src/AcDream.App/UI/Layout/VendorUiController.cs b/src/AcDream.App/UI/Layout/VendorUiController.cs index 5c09d323..7a514d8a 100644 --- a/src/AcDream.App/UI/Layout/VendorUiController.cs +++ b/src/AcDream.App/UI/Layout/VendorUiController.cs @@ -2,7 +2,9 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using AcDream.App.Rendering; using AcDream.Core.Items; +using AcDream.Core.Properties; namespace AcDream.App.UI.Layout; @@ -76,6 +78,90 @@ public sealed class VendorUiController : IRetainedPanelController public const uint BuyingPageId = 0x100000C4u; public const uint SellingPageId = 0x100000CDu; + /// + /// F1 (Slice 5.4 review): the category dropdown's authored popup. + /// Retail UIElement_Menu::MakePopup (0x0046D310, + /// pc:120705-120764) reads a PER-MENU popup LayoutDesc + /// from element attribute 7 (DataID) and a root element id from + /// attribute 6 (Enum); Initialize (0x0046D440, + /// pc:120789-120828) then locates the popup's + /// UIElement_ListBox child via attribute 2. Element + /// 0x100000BF's resolved attributes — verified against the + /// committed fixture (vendor_21000012_100000B7.json, property + /// state 4294967295) AND a live-dat scan of LayoutDesc + /// 0x21000043 — are 7=0x21000043 (popup catalog layout), + /// 6=0x1000034F (popup root), 2=0x10000350 (its + /// ListBox), 9=0x10000352 (the row-template element the + /// dropdown's rows inherit from). NOTE: the original review's citation + /// of 6=0x1000014F was a transcription slip — the fixture and a + /// live-dat re-scan both read 0x1000034F. + /// + /// draws its OWN simplified column-major grid, not + /// retail's nested ListBox tree, so this port reads off the + /// AUTHORED VALUES instead of instantiating that subtree: row size + /// 100x18 (0x10000352's own Width/Height), 6 rows per column + /// (the ListBox 0x10000350's own Height 108 / row height 18), + /// and the row sprites 0x060012B3 (Normal)/0x060012B4 + /// (Highlight) — INSTEAD of reusing 's class-default + /// constants, which are chat's own authored values (191x17, + /// RowsPerColumn 7, sprites 0x0600124C/4D/4E: chat's channel menu + /// (element 0x10000014 in LayoutDesc 0x21000006) has the + /// SAME three attributes pointing at a completely different, bespoke + /// popup subtree it authors within its own layout — + /// 7=0x21000006 (its own layout id), 6=0x1000001C, + /// 9=0x1000001E. This keeps + /// byte-identical (it is not touched by this fix) while giving + /// vendor's dropdown its own authored geometry instead of chat's. + /// + /// + /// The popup root 0x1000034F carries NO direct-state sprite of + /// its own (unlike chat's 0x0600124C panel fill) — each row + /// tiles its own background with no gap between rows (6 rows x 18px = + /// the ListBox's exact 108px height), so + /// is left at its default 0 + /// ('s sprite draw helper no-ops on id 0), matching + /// the absent authored sprite exactly rather than inventing one. + /// + /// + /// The button FACE reuses the same two sprites: 0x060012B3 is + /// literally what vendor's OWN button-face child (0x1000034D) + /// resolves to in the fixture, and 0x060012B4 (the row + /// template's "Highlight" state) is the paired open/pressed look. This + /// is a deliberate, bounded choice: retail's actual closed-button + /// chrome is a separate two-piece label+arrow assembly + /// (0x1000034D label + 0x1000034E a 17x19 arrow cap with + /// its own Normal/Highlight sprites 0x060012B1/0x060012B2), + /// and 's existing single-texture 3-slice + /// DrawButtonFace (end caps sized for chat's 46px LED-arrow + /// texture) does not model a second overlaid arrow icon. Reworking + /// DrawButtonFace into a two-piece label+arrow renderer would + /// have meant redesigning UiMenu's draw model for a purely cosmetic + /// gain while the actual reported bug (nothing renders at all) is + /// fixed by this simpler substitution; the arrow glyph gap is recorded + /// as a residual visual approximation (see the register, AP-161). + /// + /// + private const int TypeMenuRowsPerColumn = 6; + private const float TypeMenuRowHeight = 18f; + private const float TypeMenuColumnWidth = 100f; + private const uint TypeMenuItemNormalSprite = 0x060012B3u; + private const uint TypeMenuItemHighlightSprite = 0x060012B4u; + private const uint TypeMenuNormalSprite = 0x060012B3u; + private const uint TypeMenuPressedSprite = 0x060012B4u; + + /// + /// F2 mask rule (Slice 5.4 review): the split-exempt ItemType + /// mask gmToolbarUI::HandleSelectionChanged applies when seeding + /// GenItemHolder::splitSize for a vendor-owned selection + /// (pc:198779-198790, literal mask at pc:198784). Every + /// row shows IS vendor-owned (its + /// container is unconditionally the open vendor), so the "does this + /// item belong to the open vendor" gate that precedes the mask check in + /// retail's function is always true here and is not reproduced + /// separately — see . + /// + private const uint SplitExemptMask = 0x0DC41CB0u; + /// /// Retail's ordered category table, transcribed verbatim from /// VendorItemsUI::OpenVendor's AddTypeFilter call chain @@ -109,6 +195,8 @@ public sealed class VendorUiController : IRetainedPanelController private readonly VendorState _vendor; private readonly RetailWindowHandle _window; private readonly Func _resolveIcon; + private readonly ClientObjectTable _objects; + private readonly Func _playerGuid; private readonly UiElement _itemsPage; private readonly UiElement _buyingPage; private readonly UiElement _sellingPage; @@ -120,6 +208,8 @@ public sealed class VendorUiController : IRetainedPanelController private readonly UiText _itemNameText; private readonly UiText _itemCostText; private readonly UiButton? _close; + private readonly UiButton? _buyButton; + private readonly UiButton? _addButton; private readonly List<(string Label, ItemType Mask)> _presentCategories = new(); private int _selectedCategoryIndex = -1; @@ -130,6 +220,8 @@ public sealed class VendorUiController : IRetainedPanelController VendorState vendor, RetailWindowHandle window, Func resolveIcon, + ClientObjectTable objects, + Func playerGuid, UiElement itemsPage, UiElement buyingPage, UiElement sellingPage, @@ -141,11 +233,19 @@ public sealed class VendorUiController : IRetainedPanelController UiMenu typeMenu, UiText itemNameText, UiText itemCostText, - UiButton? close) + UiButton? close, + UiButton? buyButton, + UiButton? addButton, + UiDatFont? datFont, + BitmapFont? debugFont, + Func resolveSprite, + uint emptySlotSprite) { _vendor = vendor; _window = window; _resolveIcon = resolveIcon; + _objects = objects; + _playerGuid = playerGuid; _itemsPage = itemsPage; _buyingPage = buyingPage; _sellingPage = sellingPage; @@ -157,18 +257,45 @@ public sealed class VendorUiController : IRetainedPanelController _itemNameText = itemNameText; _itemCostText = itemCostText; _close = close; + _buyButton = buyButton; + _addButton = addButton; _itemList.Columns = 1; _itemList.SingleRow = true; _itemList.HorizontalScroll = true; _itemList.CellWidth = 32f; _itemList.CellHeight = 32f; + // F7b (Slice 5.4 review): fill visible empty slots the same way + // ExternalContainerController.ConfigureList does — an authored, + // mostly-empty single-row strip should show its empty-slot + // background art in the unused cells, not blank space. + _itemList.FillVisibleEmptySlots = true; + if (emptySlotSprite != 0u) + _itemList.CellEmptySprite = emptySlotSprite; + _itemList.EmptySlotFactory = () => new UiItemSlot + { + SpriteResolve = _itemList.SpriteResolve, + }; if (itemScrollbar is not null) { itemScrollbar.Model = _itemList.Scroll; itemScrollbar.Horizontal = true; } + // F1 (Slice 5.4 review): wire the dropdown's font/sprite resolvers + // (UiMenu draws nothing without SpriteResolve — see the popup + // geometry class doc above) and the vendor-authored popup geometry + // instead of UiMenu's chat-shaped class defaults. + _typeMenu.SpriteResolve = resolveSprite; + _typeMenu.DatFont = datFont; + _typeMenu.Font = debugFont; + _typeMenu.NormalSprite = TypeMenuNormalSprite; + _typeMenu.PressedSprite = TypeMenuPressedSprite; + _typeMenu.ItemNormalSprite = TypeMenuItemNormalSprite; + _typeMenu.ItemHighlightSprite = TypeMenuItemHighlightSprite; + _typeMenu.RowsPerColumn = TypeMenuRowsPerColumn; + _typeMenu.RowHeight = TypeMenuRowHeight; + _typeMenu.ColumnWidth = TypeMenuColumnWidth; _typeMenu.OnSelect = payload => { if (payload is uint mask) SelectCategory(mask); @@ -190,16 +317,37 @@ public sealed class VendorUiController : IRetainedPanelController _vendor.Changed += OnVendorChanged; } + /// + /// F2/F3 (Slice 5.4 review): the local player's coin total + /// (PropertyInt.CoinValue) lives here — the same generic + /// ClientObjectTable int-property bundle every other + /// PropertyInt-driven display reads, populated by + /// ObjectTableWiring's PrivateUpdatePropertyInt routing. + /// + /// Resolves the local player's guid to look up in . + /// Retail dat font for the category dropdown's button/row labels. + /// Fallback debug bitmap font (used when is null). + /// Dat RenderSurface id → (GL tex handle, px width, px height). + /// Authored empty-slot background for the item strip, or 0 for none. public static VendorUiController? Bind( ImportedLayout layout, VendorState vendor, RetailWindowHandle window, - Func resolveIcon) + Func resolveIcon, + ClientObjectTable objects, + Func playerGuid, + UiDatFont? datFont, + BitmapFont? debugFont, + Func resolveSprite, + uint emptySlotSprite = 0u) { ArgumentNullException.ThrowIfNull(layout); ArgumentNullException.ThrowIfNull(vendor); ArgumentNullException.ThrowIfNull(window); ArgumentNullException.ThrowIfNull(resolveIcon); + ArgumentNullException.ThrowIfNull(objects); + ArgumentNullException.ThrowIfNull(playerGuid); + ArgumentNullException.ThrowIfNull(resolveSprite); if (layout.FindElement(ItemsPageId) is not { } itemsPage || layout.FindElement(BuyingPageId) is not { } buyingPage @@ -217,11 +365,15 @@ public sealed class VendorUiController : IRetainedPanelController UiButton? close = layout.FindElement(CloseId) as UiButton; UiScrollbar? itemScrollbar = layout.FindElement(ItemScrollbarId) as UiScrollbar; + UiButton? buyButton = layout.FindElement(BuyButtonId) as UiButton; + UiButton? addButton = layout.FindElement(AddButtonId) as UiButton; return new VendorUiController( vendor, window, resolveIcon, + objects, + playerGuid, itemsPage, buyingPage, sellingPage, @@ -233,7 +385,13 @@ public sealed class VendorUiController : IRetainedPanelController typeMenu, itemNameText, itemCostText, - close); + close, + buyButton, + addButton, + datFont, + debugFont, + resolveSprite, + emptySlotSprite); } private enum VendorPanelTab { Items, Buying, Selling } @@ -253,7 +411,25 @@ public sealed class VendorUiController : IRetainedPanelController switch (transition.Kind) { case VendorStateTransitionKind.Opened: + // F6 (Slice 5.4 review): a DIFFERENT vendor than whatever was + // previously open — gmVendorUI::OpenVendor flushes sub-UIs + // when sameVendor==0 (pc:203664-203667), so the previous + // vendor's category selection must not carry over into the + // new vendor's (unrelated) category list. VendorTransition + // already distinguishes this case from same-vendor + // Refreshed (VendorState.Apply's sameVendor check), so + // resetting here is sufficient — RebuildCategories' existing + // clamp (selected<0 -> 0) then lands on the new vendor's + // first present category, matching retail. + _selectedCategoryIndex = -1; + ShowTab(VendorPanelTab.Items); + RebuildCategories(); + _window.Show(); + break; case VendorStateTransitionKind.Refreshed: + // Same vendor re-approached (post-buy/sell refresh, Slice 6) + // — preserve the selection via RebuildCategories' clamp, + // matching retail's sameVendor==1 path. ShowTab(VendorPanelTab.Items); RebuildCategories(); _window.Show(); @@ -334,6 +510,19 @@ public sealed class VendorUiController : IRetainedPanelController /// (no vendor stock at all, or nothing /// selected) shows zero rows, matching retail. /// + /// + /// F4/F7a (Slice 5.4 review): the tail of the SAME retail function + /// (pc:201180-201190) — after every rebuild (fresh open OR an + /// explicit category switch, both funnel through this method) the + /// FIRST item that passed the filter becomes the display selection when + /// the previous one didn't survive it, and the list unconditionally + /// scrolls back to its start (ScrollToShow(m_shopList, 0)). + /// Retail routes the selection through the global + /// ACCWeenieObject::selectedID/SetSelectedObject; this + /// keeps the existing PRIVATE selection + /// instead of wiring that global seam (deferred — see the register, + /// AP-161). + /// private void RebuildItemList() { ItemType activeMask = _selectedCategoryIndex >= 0 && _selectedCategoryIndex < _presentCategories.Count @@ -343,6 +532,7 @@ public sealed class VendorUiController : IRetainedPanelController IReadOnlyList items = _vendor.Items; bool selectionStillPresent = false; + VendorShopItem? firstItem = null; using (_itemList.DeferLayout()) { @@ -353,9 +543,19 @@ public sealed class VendorUiController : IRetainedPanelController { if (((item.ItemType ?? 0u) & maskValue) == 0u) continue; + firstItem ??= item; if (item.ItemGuid == _selectedItemGuid) selectionStillPresent = true; - uint icon = _resolveIcon((ItemType)(item.ItemType ?? 0u), item.IconId, 0u, 0u, 0u); + // F5 (Slice 5.4 review): forward the icon underlay/ + // overlay/effects PublicWeenieDescParser already + // captures, matching ExternalContainerController. + // CreateCell's exact resolve-icon call shape. + uint icon = _resolveIcon( + (ItemType)(item.ItemType ?? 0u), + item.IconId, + item.IconUnderlayId, + item.IconOverlayId, + item.Effects); var cell = new UiItemSlot { SpriteResolve = _itemList.SpriteResolve, @@ -371,17 +571,43 @@ public sealed class VendorUiController : IRetainedPanelController } if (!selectionStillPresent) - ClearSelection(); + { + if (firstItem is { } first) SelectItem(first); + else ClearSelection(); + } + + // F7a: unconditional scroll-to-start on every rebuild (retail only + // guards on the list being non-empty; resetting an already-empty + // list's scroll is harmless). + _itemList.Scroll.SetScrollY(0); } /// - /// Port of retail row selection — updates m_itemNameText/ - /// m_itemCostText with the retail-correct PER-UNIT price - /// (VendorProfile::VendorSellPrice, quantity 1 — the vendor - /// sells this item TO the player at the vendor's own sell rate; - /// 's naming-inversion warning - /// applies, see that type's doc comment). + /// Port of retail row selection display — + /// VendorItemsUI::UpdateItemsUI (0x004C38E0, + /// pc:202539-202820). /// + /// + /// F2/F3 (Slice 5.4 review): the priced/named QUANTITY is retail's + /// ItemHolder::GetObjectSplitSize (0x00586F00, + /// pc:401465-401477) AS SEEDED for a vendor-shop item by + /// gmToolbarUI::HandleSelectionChanged's vendor branch — see + /// for the exact mask citation. Name: + /// ACCWeenieObject::GetObjectName (0x0058E6E0, + /// pc:409056-409132) — NAME_SINGULAR for quantity <= 1, + /// NAME_PLURAL for quantity > 1; when no plural is authored + /// (m_len==1, an empty PString) retail falls back to the + /// SINGULAR name UNCHANGED — read directly off GetObjectName's + /// body (label label_58e76b's m_buffer->m_len != 1 + /// branch falls through to the plain _name read when false), NOT + /// the "likely name+s" guess. Cost text: . + /// Button state: no selection disables Buy/Add + /// (SetState(0xd), pc:202572-202577); a selection enables + /// them (SetState(1), pc:202784-202789) — modeled here as + /// , the project-wide idiom for this + /// retail state pair (see ToolbarController's + /// _useButton.Enabled). + /// private void SelectItem(VendorShopItem item) { _selectedItemGuid = item.ItemGuid; @@ -392,16 +618,84 @@ public sealed class VendorUiController : IRetainedPanelController cell.Selected = cell.ItemId == _selectedItemGuid; } - SetPlainText(_itemNameText, item.Name ?? string.Empty); + int quantity = VendorSplitSize(item); + + string baseName = quantity <= 1 + ? item.Name ?? string.Empty + : (string.IsNullOrEmpty(item.PluralName) ? item.Name : item.PluralName) ?? string.Empty; + string nameText = quantity > 1 ? $"{quantity} {baseName}" : baseName; + SetPlainText(_itemNameText, nameText); VendorShopProfile profile = _vendor.Profile; int rawValue = item.Value ?? 0; int perUnit = VendorPricing.PerUnitValue(rawValue, item.DescStackSize); - int price = VendorPricing.SellPrice(perUnit, item.ItemType ?? 0u, profile.SellPrice, quantity: 1); - string costText = profile.AlternateCurrencyWcid == 0u - ? price.ToString(CultureInfo.InvariantCulture) - : $"{price.ToString(CultureInfo.InvariantCulture)} {profile.AlternateCurrencyPluralName}"; - SetPlainText(_itemCostText, costText); + int price = VendorPricing.SellPrice(perUnit, item.ItemType ?? 0u, profile.SellPrice, quantity); + SetPlainText(_itemCostText, BuildCostText(profile, quantity, price)); + + SetActionButtonsEnabled(true); + } + + /// + /// The quantity retail prices/names a vendor-shop selection at — + /// gmToolbarUI::HandleSelectionChanged's vendor-owned branch + /// (pc:198779-198790, mask literal at pc:198784): 1 unit + /// if the item's type intersects , else its + /// own authored stack size (0/absent treated as 1, matching + /// ItemHolder::GetObjectSplitSize's own stackSize==0 -> 1 + /// floor, pc:401473-401476). + /// + private static int VendorSplitSize(VendorShopItem item) + { + if (((item.ItemType ?? 0u) & SplitExemptMask) != 0u) return 1; + return item.DescStackSize is { } size && size > 0 ? size : 1; + } + + /// + /// Cost sentence — VendorItemsUI::UpdateItemsUI's tail + /// (pc:202641-202790). No-alt-currency: verb agreement ("costs" + /// for quantity <= 1, "cost" for > 1 — pc:202759-202762), + /// comma-grouped price/holdings (retail's InsertCommas, ported as + /// ToString("N0") with ) + /// via the literal format "%s %hsp (you have %hsp)" + /// (pc:202769). Alt-currency: the FULL retail format string, read + /// directly off the data segment rather than the truncated call-site + /// view — "This item costs %d %s. You have %d %s." + /// (acclient_2013_pseudo_c.txt:991314) — this branch uses RAW + /// (not comma-grouped) integers, an asymmetry retail's own literal + /// format establishes, not an oversight here. + /// + /// The player's primary-currency holding is retail's + /// gmVendorUI::m_totalValue, read from PropertyInt.CoinValue + /// (pc:203494-203497) via the SAME + /// generic int-property bundle every other PropertyInt-driven display + /// reads. The alt-currency holding is retail's + /// shopVendorProfile->trade_num - m_last_sale; + /// m_last_sale only changes on a completed Slice-6 purchase, so + /// with no purchase mechanism yet this port uses + /// directly + /// (retail's m_last_sale == 0 case — see the register, AP-161). + /// + /// + private string BuildCostText(VendorShopProfile profile, int quantity, int price) + { + if (profile.AlternateCurrencyWcid != 0u) + { + return string.Format( + CultureInfo.InvariantCulture, + "This item costs {0} {1}. You have {2} {1}.", + price, + profile.AlternateCurrencyPluralName, + (int)profile.AlternateCurrencyAmount); + } + + int playerTotal = _objects.Get(_playerGuid())?.Properties.GetInt((uint)PropertyInt.CoinValue) ?? 0; + string verb = quantity <= 1 ? "costs" : "cost"; + return string.Format( + CultureInfo.InvariantCulture, + "{0} {1} (you have {2})", + verb, + price.ToString("N0", CultureInfo.InvariantCulture), + playerTotal.ToString("N0", CultureInfo.InvariantCulture)); } private void ClearSelection() @@ -414,6 +708,13 @@ public sealed class VendorUiController : IRetainedPanelController } SetPlainText(_itemNameText, string.Empty); SetPlainText(_itemCostText, string.Empty); + SetActionButtonsEnabled(false); + } + + private void SetActionButtonsEnabled(bool enabled) + { + if (_buyButton is not null) _buyButton.Enabled = enabled; + if (_addButton is not null) _addButton.Enabled = enabled; } private void ClearContent() diff --git a/src/AcDream.Core.Net/GameEventWiring.cs b/src/AcDream.Core.Net/GameEventWiring.cs index ab4f9dab..b17de5fe 100644 --- a/src/AcDream.Core.Net/GameEventWiring.cs +++ b/src/AcDream.Core.Net/GameEventWiring.cs @@ -424,7 +424,18 @@ public static class GameEventWiring // supply-count field) -- VendorPricing.PerUnitValue's // divisor for turning Value's stack-total wire number // into a per-unit display price. - item.Desc.StackSize); + item.Desc.StackSize, + // Slice 5.4 review fix F5: forward the icon overlay/ + // underlay/effects PublicWeenieDescParser already + // captures, so a shop item's icon composites the same + // way ExternalContainerController.CreateCell's does. + item.Desc.IconUnderlayId, + item.Desc.IconOverlayId, + item.Desc.UiEffects, + // Slice 5.4 review fix F2/F3: forward the plural name so + // the browse panel can show "100 Arrows" instead of + // fabricating a plural. + item.Desc.PluralName); } vendor?.Apply(p.Value.VendorGuid, profile, shopItems); diff --git a/src/AcDream.Core/Items/VendorState.cs b/src/AcDream.Core/Items/VendorState.cs index a60e04cc..b30fa598 100644 --- a/src/AcDream.Core/Items/VendorState.cs +++ b/src/AcDream.Core/Items/VendorState.cs @@ -57,7 +57,25 @@ public readonly record struct VendorShopItem( // conditionally present (weenieFlags-gated) -- absent maps to null here, // matching retail's own zeroed-struct default of 0 for the same case // (see VendorPricing.PerUnitValue's <= 0 guard). - int? DescStackSize = null); + int? DescStackSize = null, + // Review finding F5 (Slice 5.4 review): PublicWeenieDescBody already + // carries these three (IconOverlayId/IconUnderlayId/UiEffects) — see + // AcDream.Core.Net.Messages.PublicWeenieDescBody. Mirrors + // ClientObjectTable's ClientObject.IconUnderlayId/IconOverlayId/Effects + // naming exactly, so VendorUiController can forward them to + // _resolveIcon the same way ExternalContainerController.CreateCell does + // (item.Type, item.IconId, item.IconUnderlayId, item.IconOverlayId, + // item.Effects). 0 = "not sent", same as ClientObject's convention. + uint IconUnderlayId = 0u, + uint IconOverlayId = 0u, + uint Effects = 0u, + // Review finding F2/F3 (Slice 5.4 review): retail's NAME_PLURAL display + // (ACCWeenieObject::GetObjectName, pc:409056-409132) reads + // PublicWeenieDesc::_plural_name -- wire AcDream.Core.Net.Messages. + // PublicWeenieDescBody.PluralName. Null/empty is a real, retail-modeled + // case (no plural authored): GetObjectName falls back to the singular + // Name unchanged, not an auto-pluralized "Name+s". + string? PluralName = null); public enum VendorStateTransitionKind { @@ -135,10 +153,31 @@ public sealed class VendorState Profile = profile; Items = items; - Changed?.Invoke(new VendorTransition( + var transition = new VendorTransition( sameVendor ? VendorStateTransitionKind.Refreshed : VendorStateTransitionKind.Opened, previous, - vendorGuid)); + vendorGuid); + // Review finding F8 (Slice 5.4 review): match Close()'s per-listener + // isolation + catch-and-log shape instead of a bare Invoke — Apply() + // is called from the SAME per-frame inbound-message-dispatch boundary + // (GameEventDispatcher.Dispatch's ApproachVendor handler) that + // Close()'s doc comment already establishes must survive a broken + // observer, and a bare Invoke lets the FIRST listener's exception + // starve every listener registered after it (e.g. a plugin panel + // wired after the retail VendorUiController). + Action? listeners = Changed; + if (listeners is not null) + { + foreach (Action listener in listeners.GetInvocationList()) + { + try { listener(transition); } + catch (Exception error) + { + Console.Error.WriteLine( + $"[VendorState] Apply() observer threw: {error.Message}"); + } + } + } return true; } diff --git a/tests/AcDream.App.Tests/UI/Layout/VendorUiControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/VendorUiControllerTests.cs index 5c43b654..32c92029 100644 --- a/tests/AcDream.App.Tests/UI/Layout/VendorUiControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/VendorUiControllerTests.cs @@ -3,6 +3,7 @@ using System.Linq; using AcDream.App.UI; using AcDream.App.UI.Layout; using AcDream.Core.Items; +using AcDream.Core.Properties; namespace AcDream.App.Tests.UI.Layout; @@ -44,15 +45,26 @@ public sealed class VendorUiControllerTests layout, new VendorState(), window, - static (_, _, _, _, _) => 0u); + static (_, _, _, _, _) => 0u, + new ClientObjectTable(), + static () => 0u, + datFont: null, + debugFont: null, + static _ => (0u, 0, 0)); Assert.NotNull(controller); } private sealed class Harness { + // F2/F3: a deterministic non-zero player coin total so the cost-text + // "(you have ...)" tail is assertable. + public const int DefaultPlayerCoinValue = 1500; + private const uint PlayerGuid = 0x50000001u; + public readonly VendorState State = new(); public readonly UiRoot Screen = new() { Width = 800f, Height = 600f }; + public readonly ClientObjectTable Objects = new(); public readonly UiItemList ItemList = new(); public readonly UiScrollbar ItemScrollbar = new(); public readonly UiMenu TypeMenu = new(); @@ -65,15 +77,27 @@ public sealed class VendorUiControllerTests public readonly UiElement BuyingPage = new TestElement(); public readonly UiElement SellingPage = new TestElement(); public readonly UiButton CloseButton; + public readonly UiButton BuyButton; + public readonly UiButton AddButton; public readonly RetailWindowHandle Window; public readonly VendorUiController Controller; public Harness() { + var bundle = new PropertyBundle(); + bundle.Ints[(uint)PropertyInt.CoinValue] = DefaultPlayerCoinValue; + Objects.UpsertProperties(PlayerGuid, bundle); + var root = new TestElement { Width = 800f, Height = 110f }; CloseButton = new UiButton( new ElementInfo { Id = VendorUiController.CloseId, Type = 1 }, static _ => (0u, 0, 0)); + BuyButton = new UiButton( + new ElementInfo { Id = VendorUiController.BuyButtonId, Type = 1 }, + static _ => (0u, 0, 0)); + AddButton = new UiButton( + new ElementInfo { Id = VendorUiController.AddButtonId, Type = 1 }, + static _ => (0u, 0, 0)); root.AddChild(CloseButton); root.AddChild(ItemsTab); @@ -87,6 +111,8 @@ public sealed class VendorUiControllerTests ItemsPage.AddChild(TypeMenu); ItemsPage.AddChild(ItemNameText); ItemsPage.AddChild(ItemCostText); + ItemsPage.AddChild(BuyButton); + ItemsPage.AddChild(AddButton); var layout = new ImportedLayout(root, new Dictionary { @@ -102,6 +128,8 @@ public sealed class VendorUiControllerTests [VendorUiController.TypeFilterMenuId] = TypeMenu, [VendorUiController.ItemNameTextId] = ItemNameText, [VendorUiController.ItemCostTextId] = ItemCostText, + [VendorUiController.BuyButtonId] = BuyButton, + [VendorUiController.AddButtonId] = AddButton, }); Window = RetailWindowFrame.Mount( @@ -121,14 +149,22 @@ public sealed class VendorUiControllerTests layout, State, Window, - static (_, iconId, _, _, _) => iconId != 0u ? iconId + 1000u : 0u)!; + // F5: sum every argument so a test can prove underlay/overlay/ + // effects were actually forwarded (iconId-only would silently + // regress to dropping the last three). + static (_, iconId, underlay, overlay, effects) => iconId + underlay + overlay + effects, + Objects, + static () => PlayerGuid, + datFont: null, + debugFont: null, + static _ => (0u, 0, 0))!; Screen.WindowManager.AttachController(WindowNames.Vendor, Controller); } } private static VendorShopProfile Profile( - float sellRate = 1.5f, uint altCurrency = 0u, string altName = "") => - new(0u, 0u, 0u, false, 1.0f, sellRate, altCurrency, 0u, altName); + float sellRate = 1.5f, uint altCurrency = 0u, string altName = "", uint altAmount = 0u) => + new(0u, 0u, 0u, false, 1.0f, sellRate, altCurrency, altAmount, altName); private static string GetText(UiText text) => string.Concat(text.LinesProvider().Select(line => line.Text)); @@ -202,13 +238,18 @@ public sealed class VendorUiControllerTests } [Fact] - public void SelectingItem_ShowsRetailCorrectPerUnitPrice_ForStackedItem() + public void SelectingItem_NotInSplitExemptMask_ShowsWholeStackPriceAndFallbackPluralName() { + // F2/F3 (Slice 5.4 review): MissileWeapon (0x100) does NOT intersect + // the 0xDC41CB0 split-exempt mask, so retail prices/names the WHOLE + // stack, not one unit — the review flagged the OLD test (asserting a + // per-unit price + bare "Arrows") as wrong. var h = new Harness(); h.State.Apply(VendorGuid, Profile(sellRate: 2.0f), new[] { // Value=1000 is the STACK-TOTAL wire value for a 100-unit stack; - // per-unit = 1000/100 = 10. SellPrice = ceil(2.0*10*1 - 0.1) = 20 + // per-unit = 1000/100 = 10. quantity = DescStackSize = 100 (not + // exempt). SellPrice = ceil(2.0*10*100 - 0.1) = 2000 // (VendorPricing.SellPrice, ShopSystem::SellPrice pc:702107). new VendorShopItem( StackedItemGuid, -1, 3u, "Arrows", (uint)ItemType.MissileWeapon, 300u, 1000, @@ -217,17 +258,66 @@ public sealed class VendorUiControllerTests h.ItemList.GetItem(0)!.Clicked?.Invoke(); - Assert.Equal("Arrows", GetText(h.ItemNameText)); - Assert.Equal("20", GetText(h.ItemCostText)); + // No authored PluralName -> ACCWeenieObject::GetObjectName falls back + // to the singular name UNCHANGED (not an invented "Arrows" + "s"). + Assert.Equal("100 Arrows", GetText(h.ItemNameText)); + Assert.Equal( + $"cost {2000:N0} (you have {Harness.DefaultPlayerCoinValue:N0})", + GetText(h.ItemCostText)); } [Fact] - public void SelectingItem_WithAlternateCurrency_AppendsCurrencyName() + public void SelectingItem_InSplitExemptMask_ShowsPerUnitPriceAndSingularName() { + // F2/F3: Food (0x20) DOES intersect the split-exempt mask, so + // quantity is forced to 1 even though the item has a larger + // authored stack — the mask-member case the review asked for. + var h = new Harness(); + h.State.Apply(VendorGuid, Profile(sellRate: 2.0f), new[] + { + // perUnit = 500/50 = 10. quantity forced to 1 (Food is masked). + // SellPrice = ceil(2.0*10*1 - 0.1) = 20. + new VendorShopItem( + FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 500, + DescStackSize: 50), + }); + + h.ItemList.GetItem(0)!.Clicked?.Invoke(); + + Assert.Equal("Bread", GetText(h.ItemNameText)); + Assert.Equal( + $"costs {20:N0} (you have {Harness.DefaultPlayerCoinValue:N0})", + GetText(h.ItemCostText)); + } + + [Fact] + public void SelectingItem_WithAuthoredPluralName_UsesItInsteadOfSingular() + { + var h = new Harness(); + h.State.Apply(VendorGuid, Profile(), new[] + { + new VendorShopItem( + StackedItemGuid, -1, 3u, "Iron Key", (uint)ItemType.Key, 50u, 100, + DescStackSize: 10, PluralName: "Iron Keys"), + }); + + h.ItemList.GetItem(0)!.Clicked?.Invoke(); + + Assert.Equal("10 Iron Keys", GetText(h.ItemNameText)); + } + + [Fact] + public void SelectingItem_WithAlternateCurrency_ShowsFullRetailCostSentence() + { + // F2/F3: the OLD implementation just appended the currency name after + // a bare number ("50 Trade Notes"). Retail's actual alt-currency + // sentence is a full two-clause sentence with a "you have" tail + // (acclient_2013_pseudo_c.txt:991314), using RAW (non-comma-grouped) + // integers — unlike the primary-currency branch. var h = new Harness(); h.State.Apply( VendorGuid, - Profile(sellRate: 1.0f, altCurrency: 0x12345678u, altName: "Trade Notes"), + Profile(sellRate: 1.0f, altCurrency: 0x12345678u, altName: "Trade Notes", altAmount: 12345u), new[] { new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 50), @@ -235,7 +325,52 @@ public sealed class VendorUiControllerTests h.ItemList.GetItem(0)!.Clicked?.Invoke(); - Assert.Equal("50 Trade Notes", GetText(h.ItemCostText)); + // Armor (not in the split-exempt mask) with DescStackSize null -> + // PerUnitValue returns Value unchanged (50); quantity = 1 (no + // authored stack) -> SellPrice(50, Armor, 1.0, 1) = ceil(50-0.1) = 50. + Assert.Equal( + "This item costs 50 Trade Notes. You have 12345 Trade Notes.", + GetText(h.ItemCostText)); + } + + [Fact] + public void NoSelection_DisablesBuyAndAddButtons_SelectionEnablesThem() + { + var h = new Harness(); + Assert.False(h.BuyButton.Enabled); + Assert.False(h.AddButton.Enabled); + + h.State.Apply(VendorGuid, Profile(), new[] + { + new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500), + }); + + // F4 auto-selects the sole item on open, so the buttons are already + // enabled — SetState(1), pc:202784-202789. + Assert.True(h.BuyButton.Enabled); + Assert.True(h.AddButton.Enabled); + + // Closing clears the selection -> SetState(0xd), pc:202572-202577. + h.State.Close(); + Assert.False(h.BuyButton.Enabled); + Assert.False(h.AddButton.Enabled); + } + + [Fact] + public void ShopItem_WithIconUnderlayOverlayEffects_ForwardsThemToResolveIcon() + { + // F5: PublicWeenieDescBody already carries these three; the review + // flagged VendorUiController hardcoding 0u/0u/0u instead of + // forwarding item.IconUnderlayId/IconOverlayId/Effects. + var h = new Harness(); + var item = new VendorShopItem( + ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, IconId: 200u, Value: 500, + IconUnderlayId: 10u, IconOverlayId: 100u, Effects: 1000u); + h.State.Apply(VendorGuid, Profile(), new[] { item }); + + // Harness's resolveIcon sums every argument (see its comment) — this + // regresses to 200u alone if the three are dropped again. + Assert.Equal(200u + 10u + 100u + 1000u, h.ItemList.GetItem(0)!.IconTexture); } [Fact] @@ -254,6 +389,158 @@ public sealed class VendorUiControllerTests Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId); } + [Fact] + public void SelectingDifferentCategory_AutoSelectsFirstFilteredItem() + { + // F4: VendorItemsUI::UpdateItemsList's tail (pc:201180-201190) — + // after a category switch the first item that passed the filter + // becomes the display selection, not a cleared blank. + var h = new Harness(); + var armor = new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500); + var food = new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5); + h.State.Apply(VendorGuid, Profile(), new[] { armor, food }); + + object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload; + h.TypeMenu.OnSelect!.Invoke(foodPayload); + + Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId); + Assert.True(h.ItemList.GetItem(0)!.Selected); + Assert.NotEqual(string.Empty, GetText(h.ItemNameText)); + Assert.NotEqual(string.Empty, GetText(h.ItemCostText)); + Assert.True(h.BuyButton.Enabled); + } + + [Fact] + public void SelectingDifferentCategory_ResetsListScrollToStart() + { + // F7a: VendorItemsUI::UpdateItemsList's tail (pc:201186-201190) — + // ScrollToShow(m_shopList, 0) fires unconditionally on every rebuild. + var h = new Harness(); + var armor = new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500); + var food = new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5); + h.State.Apply(VendorGuid, Profile(), new[] { armor, food }); + + object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload; + h.TypeMenu.OnSelect!.Invoke(foodPayload); + + Assert.Equal(0, h.ItemList.Scroll.ScrollY); + } + + [Fact] + public void ItemList_FillsVisibleEmptySlots() + { + // F7b: mirrors ExternalContainerController.ConfigureList's empty-slot + // fill so the authored 22-slot strip doesn't show blank space. + var h = new Harness(); + + Assert.True(h.ItemList.FillVisibleEmptySlots); + Assert.NotNull(h.ItemList.EmptySlotFactory); + } + + [Fact] + public void Opened_WithDifferentVendor_ResetsToFirstPresentCategory_NotThePreviousVendors() + { + // F6: gmVendorUI::OpenVendor flushes sub-UIs when sameVendor==0 + // (pc:203664-203667) — a different vendor must not inherit whatever + // category index the LAST vendor happened to have selected. + const uint OtherVendorGuid = 0x70000020u; + const uint OtherArmorGuid = 0x60000201u; + const uint OtherFoodGuid = 0x60000202u; + + var h = new Harness(); + h.State.Apply(VendorGuid, Profile(), new[] + { + new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500), + new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5), + }); + object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload; + h.TypeMenu.OnSelect!.Invoke(foodPayload); + Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label); + + // A DIFFERENT vendor, which ALSO has >=2 categories (Armor then + // Food, table order) so a stale numeric index (1) would silently + // "validate" without the fix and land on Food again by coincidence + // rather than by the vendor's own contents. + h.State.Apply(OtherVendorGuid, Profile(), new[] + { + new VendorShopItem(OtherArmorGuid, -1, 4u, "Buckler", (uint)ItemType.Armor, 900u, 40), + new VendorShopItem(OtherFoodGuid, -1, 5u, "Ale", (uint)ItemType.Food, 100u, 3), + }); + + Assert.Equal("Armor", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label); + Assert.Equal(OtherArmorGuid, h.ItemList.GetItem(0)!.ItemId); + } + + [Fact] + public void Refreshed_SameVendor_PreservesCategorySelection() + { + // F6 counterpart: gmVendorUI::OpenVendor's sameVendor==1 path does + // NOT flush sub-UIs, so a same-vendor refresh (Slice 6 post-buy/sell) + // must keep the player's chosen category. + var h = new Harness(); + var items = new[] + { + new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500), + new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5), + }; + h.State.Apply(VendorGuid, Profile(), items); + object? foodPayload = h.TypeMenu.Items.First(i => i.Label == "Food").Payload; + h.TypeMenu.OnSelect!.Invoke(foodPayload); + + // SAME vendor id re-approaches -> VendorStateTransitionKind.Refreshed. + h.State.Apply(VendorGuid, Profile(), items); + + Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label); + } + + [Fact] + public void CategoryMenu_OpensAndSelectsThroughRealHitPath_UsingAuthoredPopupGeometry() + { + // F1: the popup never rendered because SpriteResolve/fonts/sprites + // were never wired, and the geometry was chat's own authored values + // instead of the vendor dropdown's. This drives the SAME UiEvent + // hit-test path UiMenuTests uses (not a direct OnSelect call), and + // reads the geometry back off the live widget instead of hardcoding + // pixel offsets tied to a specific constant set. + var h = new Harness(); + h.State.Apply(VendorGuid, Profile(), new[] + { + new VendorShopItem(ArmorItemGuid, -1, 2u, "Chainmail", (uint)ItemType.Armor, 200u, 500), + new VendorShopItem(FoodItemGuid, -1, 1u, "Bread", (uint)ItemType.Food, 100u, 5), + }); + + // Wiring: without these UiMenu.OnDrawOverlay early-returns (nothing + // renders) and the button face never draws either. + Assert.NotNull(h.TypeMenu.SpriteResolve); + Assert.NotEqual(0u, h.TypeMenu.NormalSprite); + Assert.NotEqual(0u, h.TypeMenu.ItemNormalSprite); + Assert.NotEqual(0u, h.TypeMenu.ItemHighlightSprite); + + // Geometry from the authored template (LayoutDesc 0x21000043's row + // element 0x10000352 and ListBox 0x10000350), NOT chat's 7/17/191. + Assert.Equal(6, h.TypeMenu.RowsPerColumn); + Assert.Equal(18f, h.TypeMenu.RowHeight); + Assert.Equal(100f, h.TypeMenu.ColumnWidth); + + // Open via the real widget event path. + Assert.True(h.TypeMenu.OnEvent(new UiEvent(0, h.TypeMenu, UiEventType.MouseDown, 0, 10, 5))); + + // "Food" is the 2nd present category in table order (Armor, then + // Food) -> row index 1, column 0. Derive the click point from the + // widget's own live geometry (mirrors UiMenu.OnEvent's own math) + // rather than a hardcoded pixel constant. + const int border = 5; // RetailChromeSprites.Border (UiMenu's private bevel thickness) + float outerH = h.TypeMenu.RowsPerColumn * h.TypeMenu.RowHeight + 2 * border; + const int targetRow = 1; + float iy = targetRow * h.TypeMenu.RowHeight + h.TypeMenu.RowHeight / 2f; + float ly = iy - outerH + border; + + Assert.True(h.TypeMenu.OnEvent(new UiEvent(0, h.TypeMenu, UiEventType.MouseDown, 0, 10, (int)ly))); + + Assert.Equal("Food", h.TypeMenu.Items.Single(i => Equals(i.Payload, h.TypeMenu.Selected)).Label); + Assert.Equal(FoodItemGuid, h.ItemList.GetItem(0)!.ItemId); + } + [Fact] public void CloseButton_RoutesThroughVendorStateClose_NotADirectFieldWrite() { diff --git a/tests/AcDream.Core.Tests/Items/VendorStateTests.cs b/tests/AcDream.Core.Tests/Items/VendorStateTests.cs index 629fa1f5..90981f10 100644 --- a/tests/AcDream.Core.Tests/Items/VendorStateTests.cs +++ b/tests/AcDream.Core.Tests/Items/VendorStateTests.cs @@ -140,6 +140,29 @@ public sealed class VendorStateTests Assert.Equal(0u, state.VendorId); } + [Fact] + public void Apply_ThrowingObserver_DoesNotPropagateAndDoesNotStarveOtherListeners() + { + // Slice 5.4 review finding F8: Apply() ran a bare Changed?.Invoke(...) + // where every other production boundary in this class (Close's + // Slice 5.3 review fix 3, matched here) isolates each listener with + // GetInvocationList() + try/catch/log. Apply() runs from the SAME + // per-frame inbound-message-dispatch boundary Close's doc comment + // already establishes must survive a broken observer. + var state = new VendorState(); + + bool secondObserverRan = false; + state.Changed += _ => throw new InvalidOperationException("boom"); + state.Changed += _ => secondObserverRan = true; + + Exception? thrown = Record.Exception( + () => state.Apply(0x40000008u, default, Array.Empty())); + + Assert.Null(thrown); + Assert.True(secondObserverRan); + Assert.Equal(0x40000008u, state.VendorId); + } + [Fact] public void Reset_RetryRepublishesAndOneObserverCannotStarveAnother() {