From e0e78883084890b1d4f60a50f3ee3b43c86ef5ae Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 9 Aug 2026 18:14:08 +0200 Subject: [PATCH] =?UTF-8?q?fix(chat):=20CH2=20rework=20=E2=80=94=20SpewBox?= =?UTF-8?q?=20tick-driven=20visibility=20+=20binary-derived=20error=20tabl?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks Campaign CH slice CH2 per the REJECT-review findings doc (docs/research/2026-08-09-ch2-review-findings.md). BLOCKER 1 — SpewBoxController never rendered a line and leaked its pending queue. LinesProvider only ran through UiText.OnDraw, which gates on Visible — and the box started invisible, so the provider (the sole caller of SpewBoxState.Tick) never ran. Gave the controller an explicit per-frame Tick(now) driven by UiRoot's global-message-3 broadcast (a zero-size GlobalTimeSink child, the same pattern VendorUiController.DragOverGlobalTimeSink already uses), matching retail's gmSpewBoxUI::Update. LinesProvider now only returns the cache. Tests rewritten to drive root.Tick(...) instead of calling the provider directly, plus new coverage for visibility-without-a-draw, queue-drain-without-a-draw, and bounded-queue-across-many-ticks. BLOCKER 2 — re-derived the HandleFailureEvent routing table from the PDB-paired binary instead of the pseudo-C's ~33-char string previews. tools/pdb-extract/sweep_weenie_strings.py sweeps every push imm32 in VA 0x571990-0x575480, dereferences into .rdata/.data, and decodes the full UTF-16LE literal. Added the 5 ids dispatched via else-if (missed by case-label enumeration), resolved 0x4F8 (previously excluded), fixed 18 wrong strings (16 the review flagged + 2 more — 0x4E9 and 0x518 — an automated diff between every swept literal and the landed table found). Every changed row cross-checked against ACE's WeenieError/WeenieErrorWithString enum doc comments; both oracles agreed on every row, including a case where the review's own proposed text for the new 0x4E8 row was itself wrong (it was 0x4E9's text) — corrected via the else-if block's own instruction address plus the ACE cross-check. Pinned table count: 344 (338 + 5 + 0x4F8). SHOULD-FIX 1 — RuntimeCommunicationState.ResetSpewBox was dead code; folded into the ChatIdentity generation-reset stage (same lifetime boundary), with a reset assertion added to the existing populated-reset test. SHOULD-FIX 2 — AddText trimmed only the trailing end and invented an empty-string early return; retail's AddTextToScroll trims both ends (trim(&str, 1, 1, ws)) and has no empty guard. Both retired. SHOULD-FIX 3 — ShowWeenieError bypassed the AddText chokepoint via ChatLog.OnWeenieError (hardcoded LogTextType 0x00); routed through Communication.AddText(Resolve(code, param)) instead, and ChatLog.OnWeenieError is deleted — GameEventWiring's legacy no-router fallback now resolves + calls OnSystemMessage directly. SHOULD-FIX 4 — retail's HandleFailureEvent switch has no default case; an unmapped id now resolves to a null Text (silence toward the player) instead of the invented "WeenieError 0xNNNN" hex fallback, with a diagnostics-only console log line for the id. NITs — AP-TBD placeholders corrected to their real register rows (AP-178, not the unrelated AP-177 lifetime row); filed AP-180 for the windowId dual-destination gap and corrected three stale "lands with CH2" comments; extended SpewBoxLayoutDumpDiagnostic from dats.Portal to dats.Local and found the SpewBox element for real — LayoutDesc 0x21000011, element 0x10000048, size 450x72, MaxConcurrentItems (ListBox property 0x10000028) = 4, not retail's code default of 1. AP-178 narrowed accordingly; SpewBoxState.MaxConcurrentItems and SpewBoxController's extent/anchor/OneLine are now authored rather than placeholder (absolute screen position and colour remain open); fixed the "19 ids... lists 18" miscount by retiring the stale paragraph in the class doc rewrite; aligned the UseDone handler's silent-status check with the other two WeenieError handlers. Full Release suite: 11,914 passed / 4 skipped / 0 failed (build 0 errors). Co-Authored-By: Claude Opus 5 --- .../retail-divergence-register.md | 7 +- docs/plans/2026-08-09-chat-parity-campaign.md | 9 +- .../Net/LiveSessionRuntimeFactory.cs | 29 +- src/AcDream.App/UI/SpewBoxController.cs | 177 +++++++++--- src/AcDream.Core.Net/GameEventWiring.cs | 64 +++-- src/AcDream.Core/Chat/ChatLog.cs | 53 +--- src/AcDream.Core/Chat/SpewBoxState.cs | 36 ++- src/AcDream.Core/Chat/WeenieErrorMessages.cs | 260 +++++++++++++----- .../Hosting/HeadlessGameplayOperations.cs | 14 +- .../Gameplay/RuntimeCommunicationState.cs | 29 +- src/AcDream.Runtime/RuntimeGenerationReset.cs | 19 +- .../Panels/Chat/ChatVM.cs | 21 +- .../UI/SpewBoxControllerTests.cs | 104 ++++++- .../UI/SpewBoxLayoutDumpDiagnostic.cs | 146 +++++++++- .../GameEventWiringTests.cs | 64 ++++- tests/AcDream.Core.Tests/Chat/ChatLogTests.cs | 48 +--- .../Chat/SpewBoxStateTests.cs | 55 ++-- .../Chat/WeenieErrorMessagesTests.cs | 140 +++++++--- .../RuntimeCommunicationStateTests.cs | 21 +- .../RuntimeGenerationResetTests.cs | 8 + .../Panels/SpewBox/SpewBoxVMTests.cs | 12 +- tools/pdb-extract/sweep_weenie_strings.py | 184 +++++++++++++ 22 files changed, 1164 insertions(+), 336 deletions(-) create mode 100644 tools/pdb-extract/sweep_weenie_strings.py diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index ebaabd49..b402baf3 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) — 127 active rows (AP-177/AP-178/AP-179 filed 2026-08-09, Campaign CH slice CH2 (interface text / SpewBox) — AP-177 records the invented 5-second SpewBox line lifetime (retail's real timeout is keystone-owned and unmeasured); AP-178 records the invented SpewBox screen position/extent/font/colour/MaxConcurrentItems, filed only after `SpewBoxLayoutDumpDiagnostic`'s exhaustive sweep of every installed LayoutDesc found zero elements of class 0x10000016 (gmSpewBoxUI is code-mounted, not dat-authored); AP-179 is the OnCombatLine half of the RETIRED AP-176 split out to its own row. AP-176 RETIRED the same day — the WeenieErrorMessages full 338-row `HandleFailureEvent` port (`WeenieErrorMessages.Resolve`) replaces the single-stand-in-`LogTextType` approximation that row recorded for `ChatLog.OnWeenieError`. AP-175 filed 2026-08-09, Campaign CH slice CH1 — PopUpString renders as a chat-log line instead of retail's modal dialog; AP-39 updated the same day — chat coloring is now retail's exact 34-value `LogTextType` table, not a synthetic per-`ChatKind` approximation of it. AP-173 and AP-174 filed 2026-08-08, Campaign A slice A2 — AP-173 expresses retail's ±15 dB DirectSound pan as an OpenAL azimuth by inverting the constant-power pan law, since AL exposes no per-channel gain for a mono source; AP-174 records acdream's extra master volume knob on top of retail's three, folded into retail's single master multiply so the −50 dB cutoff and dB quantisation move with it. AP-172 and AP-171 filed 2026-08-08, #354 spell-bar drag-reorder fix — the favorite-bar reorder gesture defers its own list rebuild for the drag's duration so `UiRoot`'s drag-cancel safety net cannot destroy the in-flight cell, compensating the drop-time target index for the resulting stale sibling numbering; final positions and the wire pair are retail-exact, only the mid-drag visual reflow timing differs. AP-170 filed 2026-08-08, grand-gate finding G3 — an out-of-range vendor Use now arms on arrival instead of sending immediately, because the user's local ACE server polls for the player to actually reach use range before opening the shop panel and a too-early Use is silently lost; AP-169 filed 2026-08-08, grand-gate finding G2 — the vendor toolbar split-slider resolver falls back to the packed shop-supply-count field when the item's own `PublicWeenieDesc._stackSize` is absent, because the user's local ACE server never populates the latter for a browse-list item; AP-167/AP-168 filed 2026-08-09 at the Opus review of `92ea3977` (findings F1/F6) — Buy All's container-vs-item slot classification approximates retail's bitfield/capacity test with `ItemType.Container` [AP-168], and SellSingleItem's non-empty-container refusal branch is not ported [AP-167]; AP-164 RETIRED the same review (finding F4) — BF_RETAINED is now checked end to end; AP-162 NARROWED the same review (finding F1) — Buy All's four client-side pre-send guards are now ported, leaving only the single-item TryBuy path without one; AP-161 gains a REVIEW CORRECTIONS paragraph the same review (findings F1-F13) summarizing the rest as bug fixes to already-claimed behavior, not new divergences. AP-164/AP-165/AP-166 filed 2026-08-09 at Slice 6b/6c (staging+sell arc) — InqAcceptability's non-sellable bitfield is unmodeled [AP-164], the Buy-side stackable-removal-amount test substitutes DescStackSize for retail's _maxStackSize [AP-165], and the Buying/Selling tabs' own purse/count text plus the cross-panel pending-sell inventory highlight are unwired [AP-166]; AP-161 NARROWED the same day — the row's last vendor-specific residual (Buying/Selling tabs render but carry no data binding) CLOSES now that both tabs are fully wired (staging, drag-to-sell, InqAcceptability gating, Sell 0x0060, the X-close confirmation), leaving only the two long-standing PRE-EXISTING residuals (dropdown arrow-cap glyph, alt-currency m_last_sale simplification) plus the three new AP-164/165/166 residuals just filed; AP-162 EXTENDED the same day — the same no-client-pre-check omission now also covers the batched "Buy All" path (TryBuyAll), not just the single-item TryBuy. AP-162/AP-163 filed 2026-08-09 at Slice 6.3 (buy arc) — no client-side Buy affordability/capacity pre-check [AP-162] and the shop-item guid-collision skip-not-clobber policy [AP-163]; AP-161 NARROWED the same day — the private-selection and unwired-examine residuals CLOSE at Slice 6.1/6.2, leaving only the dropdown arrow-cap glyph and the alt-currency `m_last_sale` simplification, plus a confirmed-absent-from-retail note on double-click-to-buy. 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) +## 3. Documented approximation (AP) — 128 active rows (AP-178 NARROWED 2026-08-09 at the CH2 REJECT-review rework NIT 3 — extending `SpewBoxLayoutDumpDiagnostic`'s sweep from `dats.Portal` (zero hits) to `dats.Local` FOUND the SpewBox element; extent (`450×72`) and `MaxConcurrentItems` (`4`, not the code-default `1`) are now AUTHORED, leaving only absolute screen position and colour open. AP-180 filed 2026-08-09 at the CH2 REJECT-review rework — `RuntimeCommunicationState.AddText`'s `windowId` parameter is accepted but not consumed, so retail's dual-destination echo (a `0x1A` message with a non-zero `windowId` lands in both the SpewBox and its originating chat window) is unimplemented; latent today since every production caller passes `windowId = 0`. AP-177/AP-178/AP-179 filed 2026-08-09, Campaign CH slice CH2 (interface text / SpewBox) — AP-177 records the invented 5-second SpewBox line lifetime (retail's real timeout is keystone-owned and unmeasured); AP-178's original filing recorded the invented SpewBox screen position/extent/font/colour/MaxConcurrentItems after `SpewBoxLayoutDumpDiagnostic`'s Portal-only sweep found zero elements of class 0x10000016 — see the NARROWED note above for the corrected finding; AP-179 is the OnCombatLine half of the RETIRED AP-176 split out to its own row. AP-176 RETIRED the same day — the WeenieErrorMessages full 344-row `HandleFailureEvent` port (`WeenieErrorMessages.Resolve`) replaces the single-stand-in-`LogTextType` approximation that row recorded for `ChatLog.OnWeenieError`. AP-175 filed 2026-08-09, Campaign CH slice CH1 — PopUpString renders as a chat-log line instead of retail's modal dialog; AP-39 updated the same day — chat coloring is now retail's exact 34-value `LogTextType` table, not a synthetic per-`ChatKind` approximation of it. AP-173 and AP-174 filed 2026-08-08, Campaign A slice A2 — AP-173 expresses retail's ±15 dB DirectSound pan as an OpenAL azimuth by inverting the constant-power pan law, since AL exposes no per-channel gain for a mono source; AP-174 records acdream's extra master volume knob on top of retail's three, folded into retail's single master multiply so the −50 dB cutoff and dB quantisation move with it. AP-172 and AP-171 filed 2026-08-08, #354 spell-bar drag-reorder fix — the favorite-bar reorder gesture defers its own list rebuild for the drag's duration so `UiRoot`'s drag-cancel safety net cannot destroy the in-flight cell, compensating the drop-time target index for the resulting stale sibling numbering; final positions and the wire pair are retail-exact, only the mid-drag visual reflow timing differs. AP-170 filed 2026-08-08, grand-gate finding G3 — an out-of-range vendor Use now arms on arrival instead of sending immediately, because the user's local ACE server polls for the player to actually reach use range before opening the shop panel and a too-early Use is silently lost; AP-169 filed 2026-08-08, grand-gate finding G2 — the vendor toolbar split-slider resolver falls back to the packed shop-supply-count field when the item's own `PublicWeenieDesc._stackSize` is absent, because the user's local ACE server never populates the latter for a browse-list item; AP-167/AP-168 filed 2026-08-09 at the Opus review of `92ea3977` (findings F1/F6) — Buy All's container-vs-item slot classification approximates retail's bitfield/capacity test with `ItemType.Container` [AP-168], and SellSingleItem's non-empty-container refusal branch is not ported [AP-167]; AP-164 RETIRED the same review (finding F4) — BF_RETAINED is now checked end to end; AP-162 NARROWED the same review (finding F1) — Buy All's four client-side pre-send guards are now ported, leaving only the single-item TryBuy path without one; AP-161 gains a REVIEW CORRECTIONS paragraph the same review (findings F1-F13) summarizing the rest as bug fixes to already-claimed behavior, not new divergences. AP-164/AP-165/AP-166 filed 2026-08-09 at Slice 6b/6c (staging+sell arc) — InqAcceptability's non-sellable bitfield is unmodeled [AP-164], the Buy-side stackable-removal-amount test substitutes DescStackSize for retail's _maxStackSize [AP-165], and the Buying/Selling tabs' own purse/count text plus the cross-panel pending-sell inventory highlight are unwired [AP-166]; AP-161 NARROWED the same day — the row's last vendor-specific residual (Buying/Selling tabs render but carry no data binding) CLOSES now that both tabs are fully wired (staging, drag-to-sell, InqAcceptability gating, Sell 0x0060, the X-close confirmation), leaving only the two long-standing PRE-EXISTING residuals (dropdown arrow-cap glyph, alt-currency m_last_sale simplification) plus the three new AP-164/165/166 residuals just filed; AP-162 EXTENDED the same day — the same no-client-pre-check omission now also covers the batched "Buy All" path (TryBuyAll), not just the single-item TryBuy. AP-162/AP-163 filed 2026-08-09 at Slice 6.3 (buy arc) — no client-side Buy affordability/capacity pre-check [AP-162] and the shop-item guid-collision skip-not-clobber policy [AP-163]; AP-161 NARROWED the same day — the private-selection and unwired-examine residuals CLOSE at Slice 6.1/6.2, leaving only the dropdown arrow-cap glyph and the alt-currency `m_last_sale` simplification, plus a confirmed-absent-from-retail note on double-click-to-buy. 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 @@ -234,8 +234,9 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-40 | Chat uses one fixed `0.75` outer opacity and has no descendant-focus-driven active/default opacity transition | `src/AcDream.App/Rendering/GameWindow.cs` chat mount; `ChatWindowController.cs` | Font resolution is now live and per-element; only the opacity behavior remains deferred to the shared window/focus runtime | Focused chat remains too translucent and idle chat never restores the configured default alpha | `ChatInterface::SetOpacity @ 0x004F3120`; `SetDefaultOpacity @ 0x004F3BC0`; `SetActiveOpacity @ 0x004F3C40` | | AP-175 | PopUpString (`GameEvent 0x0004`) renders as an ordinary chat-log line (`ChatKind.Popup`) instead of retail's MODAL DIALOG. Filed 2026-08-09, Campaign CH slice CH1 (color table) — the color-table work routes this entry through the new 34-value `LogTextType` table (fixed at `0x00` Default/green, unchanged from the entry's pre-existing color) but does not change WHERE it renders; a modal-dialog port is out of this slice's scope | `src/AcDream.Core/Chat/ChatLog.cs` (`OnPopup`); `src/AcDream.Core.Net/GameEventWiring.cs:126` | Informational popup text still reaches the player via the chat transcript; a full modal-dialog port is deferred work, not a color-table concern | Any retail-specific PopUpString behavior contingent on being a blocking modal (e.g. must-acknowledge) is not reproduced; acdream's chat-log line can be missed or scrolled past instead | `ClientCommunicationSystem::Handle_Communication__PopUpString @0x0057FE80`; `docs/research/2026-08-09-chat-retail-color-table.md` §5.1 | | AP-177 | SpewBox line lifetime is an INVENTED 5-second placeholder. Retail's `gmSpewBoxUI` never raises the expiry element message (`0x10000003`) anywhere in its own compiled Sept 2013 EoR code — the real per-line timeout/fade curve is owned by keystone.dll's authored behaviour for layout `0x10000012` element `0x1000004A`, which this slice did not measure (a live cdb capture on `gmSpewBoxUI::ListenToElementMessage @0x004D57C0` against a real retail client would resolve it). Filed 2026-08-09, Campaign CH slice CH2 | `src/AcDream.Core/Chat/SpewBoxState.cs` (`DefaultLifetime`) | A round, conservative placeholder was chosen over guessing a retail-matching curve; no fade is modeled at all (the line pops on and off) | SpewBox lines may linger noticeably longer or shorter than retail's actual timing, and pop instead of fading | `docs/research/2026-08-09-chat-retail-interface-text.md` §3.2.1 | -| AP-178 | SpewBox screen position (centered top-of-viewport, `y=60px`), extent, font, and colour (a warm-yellow placeholder), plus `SpewBoxState.MaxConcurrentItems` (fixed at retail's CODE default of 1, not any authored value), are all INVENTED. `SpewBoxLayoutDumpDiagnostic` swept the installed `client_portal.dat`'s ENTIRE LayoutDesc id range (`0x21000000`-`0x21000075`, 101 of 118 possible ids populated, sanity-checked against 3 independently-known ids — 0x2100002E character window, 0x21000023 inventory, 0x21000016 toolbar prototype) and found ZERO elements of class `0x10000016` (`gmSpewBoxUI`) anywhere — the SpewBox is mounted directly from C++ code in `gmClient`'s HUD registration block, not resolved from any authored LayoutDesc tree, so a dat dump cannot recover these values. Filed 2026-08-09, Campaign CH slice CH2 | `src/AcDream.App/UI/SpewBoxController.cs`; `src/AcDream.Core/Chat/SpewBoxState.cs` (`MaxConcurrentItems`) | The colour placeholder follows the user's own (unconfirmed) recollection that retail's SpewBox is yellow rather than an arbitrary pick; a live cdb capture of `gmSpewBoxUI`'s runtime rect/state would be the next resolution path, matching how `reference_retail_chat_colors.md` was built | SpewBox text may render in the wrong screen location, size, font, or colour versus retail; bursts of refusals collapse to one visible line where retail's authored `ListBox` may show more | `docs/research/2026-08-09-chat-retail-interface-text.md` §3.2.2-§3.2.4; `tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs` | -| AP-179 | `ChatLog.OnCombatLine`'s generic `0x06` Combat fallback types combat-feedback lines with a single stand-in `LogTextType` for callers with no more specific hit/miss/evade classification in hand, instead of retail's per-message dispatch. Split out of AP-176 (RETIRED 2026-08-09, Campaign CH slice CH2 — the WeenieError half of that bundled row is now the full 338-row `HandleFailureEvent` port, `WeenieErrorMessages.Resolve`); this combat-line half was never in CH2's scope and keeps its own row so the divergence is not silently dropped | `src/AcDream.Core/Chat/ChatLog.cs` (`OnCombatLine`) | `0x06` matches the switch's majority combat-line behavior and is a safe baseline; a full per-combat-message dispatch port is out of Campaign CH's scope | Wrong chat color for the combat-line kinds retail types distinctly (hit/miss/evade variants) | `ClientCommunicationSystem::HandleFailureEvent @0x00571990`; originally filed at the CH1 Opus review 2026-08-09 as part of AP-176, split out at CH2 | +| AP-178 | **NARROWED 2026-08-09 at the CH2 REJECT-review rework (NIT 3, `docs/research/2026-08-09-ch2-review-findings.md`):** the original filing swept only `dats.Portal` and found ZERO elements of class `0x10000016` (`gmSpewBoxUI`), so it recorded position, extent, font, colour, AND `MaxConcurrentItems` as all-invented. Extending the identical sweep to `dats.Local` (`client_local_English.dat`) FOUND it: LayoutDesc `0x21000011`, element `0x10000048`, whose sole child (ListBox `0x10000049`, matching `gmSpewBoxUI::PostInit`'s `GetChildRecursive(0x10000049)` verbatim) carries ListBox property `0x10000028` = the integer `4`. Two sub-claims RETIRE: extent is now AUTHORED (`450×72`, not a placeholder size) and `MaxConcurrentItems` is now AUTHORED (`4`, not retail's code-default `1`). Two sub-claims REMAIN open: (1) absolute screen position — the recovered position is `(0,0)` RELATIVE TO A PARENT this sweep could not identify (the element is presumably still mounted via the C++ `gmClient` HUD registration block the research doc's §1.1 describes, just parented under something dat-authored rather than the root view directly), so `TopOffset=60px` + a one-time-computed centered `Left` remain acdream's own placeholder, not a resolved retail value; (2) colour — the element/ListBox's direct-state `StateDesc.Properties` dump found only a bool at `0x3B` and the `MaxConcurrentItems` integer, no colour property, and the per-`UIStateId` `States` dictionary (hover/pressed/etc. variants) was not walked this pass. Retail's edge codes (`leftEdge=3`/`rightEdge=3`, "centered" per `ElementReader.ToAnchors`'s own doc comment; `topEdge=1`, top-anchored) confirm the box is a fixed-width centered block, not a full-viewport stretch — `SpewBoxController`'s anchor shape was corrected to match (`AnchorEdges.Top` only + computed centered `Left`, `OneLine=false` since 4 concurrent lines can now actually be visible instead of collapsing to 1) | `src/AcDream.App/UI/SpewBoxController.cs`; `src/AcDream.Core/Chat/SpewBoxState.cs` (`MaxConcurrentItems`) | The colour placeholder follows the user's own (unconfirmed) recollection that retail's SpewBox is yellow rather than an arbitrary pick; a live cdb capture of `gmSpewBoxUI`'s runtime rect/state (or walking the `States` dictionary this pass skipped, or identifying the C++-assigned parent) would be the next resolution path for the two still-open sub-claims | SpewBox text may render in the wrong absolute screen location or colour versus retail; the size/max-items risk this row originally recorded ("bursts of refusals collapse to one visible line where retail's authored ListBox may show more") is RETIRED — up to 4 now render, matching the authored value | `docs/research/2026-08-09-chat-retail-interface-text.md` §3.2.2-§3.2.4; `tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs`; `src/AcDream.App/UI/Layout/ElementReader.cs` (`ToAnchors`) | +| AP-179 | `ChatLog.OnCombatLine`'s generic `0x06` Combat fallback types combat-feedback lines with a single stand-in `LogTextType` for callers with no more specific hit/miss/evade classification in hand, instead of retail's per-message dispatch. Split out of AP-176 (RETIRED 2026-08-09, Campaign CH slice CH2 — the WeenieError half of that bundled row is now the full 344-row `HandleFailureEvent` port, `WeenieErrorMessages.Resolve`); this combat-line half was never in CH2's scope and keeps its own row so the divergence is not silently dropped | `src/AcDream.Core/Chat/ChatLog.cs` (`OnCombatLine`) | `0x06` matches the switch's majority combat-line behavior and is a safe baseline; a full per-combat-message dispatch port is out of Campaign CH's scope | Wrong chat color for the combat-line kinds retail types distinctly (hit/miss/evade variants) | `ClientCommunicationSystem::HandleFailureEvent @0x00571990`; originally filed at the CH1 Opus review 2026-08-09 as part of AP-176, split out at CH2 | +| AP-180 | `RuntimeCommunicationState.AddText`'s `windowId` parameter is accepted but not consumed — retail's `ClientSystem::AddTextToScroll(text, type, allowPluginFilter, windowId)` delivers a `type == 0x1A` message with a non-zero `windowId` to BOTH the SpewBox and that specific chat window (research doc §2.3), the shape ~40 slash-command-output sites depend on. acdream's chokepoint routes on `type` alone; every current production caller passes `windowId = 0`, so the gap is latent, not yet visibly wrong. Filed 2026-08-09 at the CH2 REJECT-review rework (NIT 2, `docs/research/2026-08-09-ch2-review-findings.md`) | `src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs` (`AddText`) | No production caller passes a non-zero `windowId` yet, so nothing observably diverges today; implementing the dual-destination echo is CH4/CH5 scope at the earliest | A future slash-command-output caller that passes a non-zero `windowId` expecting it to echo into its originating chat window (matching retail) will silently land in the SpewBox only | `ClientSystem::AddTextToScroll @0x00563C50`; `docs/research/2026-08-09-chat-retail-interface-text.md` §2.3 | | AP-41 | Scrollbar thumb 3-slice cap fallback only: single-tile draw (`0x06004C63`) used only when `ThumbTopSprite`/`ThumbBotSprite` are unset; the chat controller passes all three cap ids so the 3-slice path is drawn in practice | `src/AcDream.App/UI/UiScrollbar.cs:35` | The fallback single-tile path is unreachable when caps are bound (chat controller always sets them); the 3-slice path is the active code path | Only if a future caller omits the cap ids will the fallback fire — no visual regression in the chat window | `UIElement_Scrollbar::UpdateLayout @0x4710d0`; cap sprites `0x06004C60` (top) + `0x06004C66` (bottom) from base layout `0x2100003E` | | AP-42 | `UiMenu` item model is flat (label + opaque payload, single-level popup); retail `UIElement_Menu::MakePopup @0x46d310` supports hierarchical nested submenus via recursive popup chain | `src/AcDream.App/UI/UiMenu.cs` | The chat talk-focus menu is single-level (14 rows, 2 columns, no submenu); hierarchy is latent and unreachable through the chat window — no behavioral difference in the current usage | A future menu with nested submenus would render flat (only the top-level items drawn, no drill-down) | `UIElement_Menu::MakePopup` @0x46d310 | | AP-45 | `PublicUpdatePropertyInt (0x02CE)` sequence byte parsed-past but not honored; last update wins (no freshness check against sequence number) | `src/AcDream.Core.Net/Messages/PublicUpdatePropertyInt.cs` | Loopback ACE rarely reorders; this property stream has not yet joined the per-object freshness owner introduced for physics messages. | A reordered 0x02CE on a real network could apply a stale UiEffects value — item icon temporarily shows the wrong effect state, corrected on next update | `PublicUpdatePropertyInt` sequence byte (ACE GameMessagePublicUpdatePropertyInt) | diff --git a/docs/plans/2026-08-09-chat-parity-campaign.md b/docs/plans/2026-08-09-chat-parity-campaign.md index 62cc88bb..5dc87a98 100644 --- a/docs/plans/2026-08-09-chat-parity-campaign.md +++ b/docs/plans/2026-08-09-chat-parity-campaign.md @@ -1,9 +1,10 @@ # Campaign CH — chat & interface-text retail parity **Status:** ACTIVE 2026-08-09 — CH1 complete (implementation `172c6f9a` + -review fixes `34d8a3c0`); CH2 code-complete (`77c8296e`), pending the -in-client user gate (jump-in-air / jump-loaded refusals showing on-screen, -not in chat). +review fixes `34d8a3c0`); CH2 code-complete (`77c8296e`), REJECT-reviewed +(`docs/research/2026-08-09-ch2-review-findings.md`) and reworked +(`9a6fee6e`), pending the in-client user gate (jump-in-air / jump-loaded +refusals showing on-screen, not in chat). **Why now:** first track of the alpha-release program (chat is the most visible daily surface for the friend-alpha). User-directed 2026-08-09. @@ -102,7 +103,7 @@ implementer per slice against a pinned contract (per |---|---|---|---|---| | R1–R4 research | `see docs/research/2026-08-09-chat-retail-*` | — | — | — | | CH1 colors | `172c6f9a` | 11,835 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `34d8a3c0` | pending | -| CH2 interface text | `77c8296e` | 11,890 passed / 4 skipped / 0 failed | REJECT (`docs/research/2026-08-09-ch2-review-findings.md`); rework in progress | pending | +| CH2 interface text | `77c8296e`, reworked `9a6fee6e` | 11,914 passed / 4 skipped / 0 failed | REJECT; reworked `9a6fee6e` (`docs/research/2026-08-09-ch2-review-findings.md`) | pending | | CH3 side channels | — | — | — | — | | CH4 commands | — | — | — | — | | CH5 closeout | — | — | — | — | diff --git a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs index c04250a0..70914fec 100644 --- a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs +++ b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs @@ -332,13 +332,34 @@ internal sealed class LiveSessionRuntimeFactory // Default, NOT 0x1A (corrected 2026-08-09, Opus review of // 172c6f9a). Retail types command output like @version/@loc // green; 0x1A (bright red) is reserved for genuine refusals. - // The refusal-vs-informational split lands with CH2's producer - // rewiring (SpewBox routing) — see - // docs/research/2026-08-09-chat-retail-interface-text.md §7.2. + // + // Comment corrected 2026-08-09, CH2 REJECT-review rework + // (NIT 2, docs/research/2026-08-09-ch2-review-findings.md): + // CH2's SpewBox routing covers WeenieError/WeenieErrorWithString + // ids (see ShowWeenieError below), which carry their own + // resolved RetailLogTextType — it did NOT reach this sink, + // which takes plain pre-formatted text with no error code + // attached. Per-call-site refusal-vs-informational + // classification of this sink's callers, plus retail's + // windowId dual-destination echo (register row AP-180), remain + // unstarted — CH4/CH5 scope at the earliest, not CH2. ShowSystemMessage: text => _domain.Communication.Chat.OnSystemMessage(text, 0x00u), + // SHOULD-FIX 3 (docs/research/2026-08-09-ch2-review-findings.md): + // route through the AddText chokepoint instead of the deleted + // ChatLog.OnWeenieError, which hardcoded LogTextType 0x00 — + // several ShowWeenieError call sites (e.g. 0x0561, the friends- + // list-full refusal) resolve to ClientLocal and belong in the + // SpewBox, not green in chat. An id WeenieErrorMessages has no + // row for resolves to a null Text — retail's switch has no + // default case, so it produces no player-facing text. ShowWeenieError: - code => _domain.Communication.Chat.OnWeenieError(code, null), + code => + { + (string? text, RetailLogTextType type) = WeenieErrorMessages.Resolve(code, null); + if (text is not null) + _domain.Communication.AddText(text, type); + }, PlayerPublicWeenieBitfield: () => _domain.EntityObjects.Objects.Get(_player.Identity.ServerGuid)? .PublicWeenieBitfield, diff --git a/src/AcDream.App/UI/SpewBoxController.cs b/src/AcDream.App/UI/SpewBoxController.cs index 068c7f89..b5fc6250 100644 --- a/src/AcDream.App/UI/SpewBoxController.cs +++ b/src/AcDream.App/UI/SpewBoxController.cs @@ -10,37 +10,98 @@ namespace AcDream.App.UI; /// ClickThrough block at a high /// . Unlike that controller's single /// overwrite-only slot, this reads 's bounded, -/// newest-on-top, per-entry-expiring queue every frame — the retained UI -/// tree has no separate per-frame "Update(dt)" hook, so -/// (already polled once per render pass) -/// doubles as this controller's tick source. +/// newest-on-top, per-entry-expiring queue every frame. /// /// -/// Position / font / colour / max-items are PLACEHOLDERS. The task -/// C.7 LayoutDesc dump (SpewBoxLayoutDumpDiagnostic) was attempted -/// and completed EXHAUSTIVELY against the installed DAT's entire LayoutDesc -/// id range (0x21000000-0x21000075, 101 of 118 possible ids -/// populated, sanity-checked against 3 independently-known ids) and found -/// ZERO elements of class 0x10000016 anywhere — gmSpewBoxUI is -/// mounted directly from C++ code, not resolved from any authored -/// LayoutDesc tree, so its screen position/extent/font/colour and the -/// authored MaxConcurrentItems ListBox property are simply not -/// recoverable this way. See the divergence register rows this class cites -/// for each specific placeholder. +/// CH2 REJECT-review rework, BLOCKER 1 +/// (docs/research/2026-08-09-ch2-review-findings.md): the +/// original landing drove the queue drain from +/// , which UiText.OnDraw only +/// calls when the element is ALREADY Visible — and the element +/// starts invisible, so the provider was never invoked, no line ever drew, +/// and 's pending queue never drained (an +/// unbounded per-session leak). Retail's own gmSpewBoxUI::Update +/// drains off the UI tick (global message 3, +/// UIElementManager::UseTime @0x0045CFD0), not off drawing — +/// reproduces that: it is a zero-size child +/// mounted alongside purely so 's +/// per-frame BroadcastGlobalUiTime walk reaches it (the same +/// pattern VendorUiController.DragOverGlobalTimeSink uses for +/// gmVendorUI::ListenToGlobalMessage). pulls +/// , caches the resulting lines, and sets +/// 's Visible flag; +/// now only ever returns the cache — it is polled by drawing, but no +/// longer double-duties as the tick source, so lines become visible and +/// the queue drains even across a frame where nothing gets drawn (headless, +/// a hidden window, or simply before the first render pass). +/// +/// +/// Position / font / colour are still PLACEHOLDERS; extent and +/// max-items are now AUTHORED. CH2 REJECT-review rework, NIT 3 +/// (docs/research/2026-08-09-ch2-review-findings.md): the task C.7 +/// LayoutDesc dump (SpewBoxLayoutDumpDiagnostic) originally searched +/// only dats.Portal — EXHAUSTIVELY, against the entire installed +/// LayoutDesc id range (0x21000000-0x21000075, 101 of 118 +/// possible ids populated, sanity-checked against 3 independently-known +/// ids) — and found ZERO elements of class 0x10000016 there. +/// Extending the identical sweep to dats.Local +/// (client_local_English.dat) found it: LayoutDesc +/// 0x21000011, element 0x10000048 (gmSpewBoxUI), +/// position (0,0) RELATIVE TO ITS PARENT (edge codes +/// leftEdge=3/rightEdge=3 — ElementReader.ToAnchors's own doc +/// comment names 3 as "centered", a mode that projection cannot represent; +/// topEdge=1 — top-anchored per that same helper), size +/// 450×72, one child (ListBox 0x10000049, matching +/// gmSpewBoxUI::PostInit's GetChildRecursive(0x10000049) +/// verbatim) carrying MaxConcurrentItems (property +/// 0x10000028) = 4, not retail's code-default 1. The +/// PARENT this element mounts under (and therefore the ABSOLUTE screen +/// position) is still unresolved — (0,0) is parent-relative, and the +/// parent is presumably assigned by the same C++ code the research doc's +/// §1.1 describes, not by another LayoutDesc this sweep can walk to. See +/// the divergence register rows this class cites for each remaining +/// placeholder. /// internal sealed class SpewBoxController : IDisposable { /// - /// Register row AP-TBD (position/extent): retail's authored screen - /// position for the SpewBox host is unknown (see class remarks); this - /// centered-top placement is acdream's own choice, not a retail value. + /// Register row AP-178 (screen position): retail's authored ABSOLUTE + /// screen position is still unknown — the LayoutDesc dump (see class + /// remarks) recovered the element's position as (0,0) relative + /// to a PARENT this sweep could not identify, so this centered-top + /// placement remains acdream's own choice, not a resolved retail value. + /// (The SIBLING row AP-177 — the invented line-lifetime timeout — lives + /// in 's own doc comment, not + /// here; this controller does not own that concern.) /// private const float TopOffset = 60f; - private const float BoxHeight = 40f; /// - /// Register row AP-TBD (colour): the chat colour table's 0x1A - /// entry (colorBrightRed) is explicitly NOT this — retail's own + /// Register row AP-178 (extent): AUTHORED, not a placeholder — the + /// LayoutDesc dump (see class remarks) found the SpewBox element sized + /// 450×72 in dats.Local. Retail's own edge codes + /// (leftEdge=3/rightEdge=3, "centered" per + /// ElementReader.ToAnchors's doc comment) mean the box is a + /// FIXED-width block horizontally centered in its parent, not a + /// full-viewport stretch — the constructor below anchors it that way + /// (a one-time centered computed against + /// , only) since + /// has no "centered, fixed-width" flag + /// combination to express retail's mode 3 directly. + /// + private const float SpewBoxWidth = 450f; + private const float SpewBoxHeight = 72f; + + /// + /// Register row AP-178 (colour): retail's authored colour for THIS + /// element remains unresolved — the LayoutDesc dump (see class remarks) + /// found only two direct-state properties on the SpewBox element/ListBox + /// (a bool at 0x3B and the MaxConcurrentItems integer at + /// 0x10000028); no colour property surfaced in that direct-state + /// dump, and the per-UIStateId States dictionary (hover/ + /// pressed/etc. variants, which could carry it) was not walked this + /// pass. The chat colour table's 0x1A entry + /// (colorBrightRed) is explicitly NOT this — retail's own /// BuildChatColorLookupTable writes to ChatInterface::m_chatLog, /// a completely different element tree the SpewBox never touches /// (research doc §3.2.3). This warm-yellow placeholder follows the @@ -53,6 +114,8 @@ internal sealed class SpewBoxController : IDisposable private readonly UiRoot _root; private readonly UiText _text; private readonly SpewBoxVM _vm; + private readonly GlobalTimeSink _timeSink; + private UiText.Line[] _lines = Array.Empty(); private bool _disposed; public SpewBoxController(UiRoot root, SpewBoxVM vm) @@ -62,43 +125,63 @@ internal sealed class SpewBoxController : IDisposable _text = new UiText { Name = "SpewBox", - Left = 0f, + // Centered fixed-width block (retail's "mode 3" edge code on + // both left and right) — see the AP-178 extent comment above. + Left = (root.Width - SpewBoxWidth) / 2f, Top = TopOffset, - Width = root.Width, - Height = BoxHeight, - Anchors = AnchorEdges.Left | AnchorEdges.Top | AnchorEdges.Right, + Width = SpewBoxWidth, + Height = SpewBoxHeight, + Anchors = AnchorEdges.Top, Centered = true, - OneLine = true, + // AUTHORED MaxConcurrentItems is 4, not retail's code-default 1 + // (see SpewBoxState.MaxConcurrentItems) — OneLine=true would + // silently collapse the box back down to showing only the + // newest of up to 4 concurrent lines. + OneLine = false, ClickThrough = true, ZOrder = int.MaxValue, DefaultColor = SpewBoxColor, Visible = false, }; - _text.LinesProvider = ComputeLines; + _text.LinesProvider = () => _lines; _root.AddChild(_text); + + _timeSink = new GlobalTimeSink(Tick); + _root.AddChild(_timeSink); } /// - /// Polled once per render pass by — this IS the - /// SpewBox's per-frame tick (drains SpewBoxState's pending queue - /// and prunes expired entries; see ). - /// returns newest-first, matching retail's - /// InsertItem(item, 0); OneLine mode only ever draws - /// index 0, so with retail's code-default - /// MaxConcurrentItems == 1 this always shows the current line. + /// The SpewBox's per-frame tick, driven by 's + /// global-message-3 broadcast via — the + /// direct analogue of gmSpewBoxUI::Update. Drains + /// 's pending queue and prunes expired + /// entries (see ), caches the resulting + /// display lines, and sets 's visibility. Runs + /// whether or not a draw pass follows. /// - private IReadOnlyList ComputeLines() + /// + /// 's own per-frame clock — not + /// Environment.TickCount64 — matching every other + /// consumer's time source. + /// + private void Tick(double nowSeconds) { - double nowSeconds = Environment.TickCount64 / 1000.0; + // SpewBoxVM.Lines returns newest-first, matching retail's + // InsertItem(item, 0) — with OneLine now false and the AUTHORED + // MaxConcurrentItems == 4 (see SpewBoxState.MaxConcurrentItems), + // up to 4 lines render, newest on top. IReadOnlyList lines = _vm.Lines(nowSeconds); _text.Visible = lines.Count > 0; if (lines.Count == 0) - return Array.Empty(); + { + _lines = Array.Empty(); + return; + } var result = new UiText.Line[lines.Count]; for (int i = 0; i < lines.Count; i++) result[i] = new UiText.Line(lines[i].Text, SpewBoxColor); - return result; + _lines = result; } public void Dispose() @@ -107,6 +190,24 @@ internal sealed class SpewBoxController : IDisposable return; _root.RemoveChild(_text); + _root.RemoveChild(_timeSink); _disposed = true; } + + /// + /// A runtime-only, zero-size, always-invisible-to-hit-testing helper + /// that opts this controller into retail's global UI message 3 — see + /// the class remarks and VendorUiController.DragOverGlobalTimeSink + /// for the identical pattern. is not + /// itself a (it wraps one), so it cannot + /// directly implement the way + /// does — 's broadcast + /// walks the ELEMENT tree, not arbitrary controllers. + /// + private sealed class GlobalTimeSink : UiElement, IUiGlobalTimeListener + { + private readonly Action _onGlobalUiTime; + public GlobalTimeSink(Action onGlobalUiTime) => _onGlobalUiTime = onGlobalUiTime; + public void OnGlobalUiTime(double nowSeconds) => _onGlobalUiTime(nowSeconds); + } } diff --git a/src/AcDream.Core.Net/GameEventWiring.cs b/src/AcDream.Core.Net/GameEventWiring.cs index 254ebe9a..2fb38f6e 100644 --- a/src/AcDream.Core.Net/GameEventWiring.cs +++ b/src/AcDream.Core.Net/GameEventWiring.cs @@ -248,39 +248,53 @@ public static class GameEventWiring // Campaign CH slice CH2: retail resolves BOTH the display text and // the AddTextToScroll destination type from the SAME per-id switch // (ClientCommunicationSystem::HandleFailureEvent @0x00571990 — see - // WeenieErrorMessages' full 338-row port). When a router is wired + // WeenieErrorMessages' full 344-row port). When a router is wired // (the production path), the resolved type decides chat vs SpewBox; - // otherwise this falls back to the legacy chat-only path so callers - // that don't wire the router (older tests) keep their prior shape. + // otherwise this falls back to a direct chat append so callers that + // don't wire the router (older tests) keep a working, if + // SpewBox-less, path. + // + // REJECT-review rework (SHOULD-FIX 3/4, + // docs/research/2026-08-09-ch2-review-findings.md): the legacy + // fallback no longer routes through the deleted ChatLog.OnWeenieError + // (SHOULD-FIX 3 — that chokepoint bypass is retired everywhere, not + // just at the ShowWeenieError call site) and an unmapped id resolves + // to a null Text — retail's switch has no default case, so it + // produces NO text toward the player (SHOULD-FIX 4). Both branches + // below skip display for a null Text and log the raw id instead, so + // an unmapped code stays visible to US without ever reaching chat. registrar.Register(GameEventType.WeenieError, e => { var code = GameEvents.ParseWeenieError(e.Payload.Span); if (code is null) return; + if (WeenieErrorMessages.IsSilentClientControlStatus(code.Value)) return; + var (text, type) = WeenieErrorMessages.Resolve(code.Value, null); + if (text is null) + { + Console.WriteLine($"[weenie-error] unmapped code=0x{code.Value:X4}"); + return; + } if (onInterfaceText is not null) - { - if (WeenieErrorMessages.IsSilentClientControlStatus(code.Value)) return; - var (text, type) = WeenieErrorMessages.Resolve(code.Value, null); onInterfaceText(text, type); - } else - { - chat.OnWeenieError(code.Value, param: null); - } + chat.OnSystemMessage(text, chatType: (uint)type); }); registrar.Register(GameEventType.WeenieErrorWithString, e => { var p = GameEvents.ParseWeenieErrorWithString(e.Payload.Span); if (p is null) return; + if (WeenieErrorMessages.IsSilentClientControlStatus(p.Value.ErrorCode)) return; + var (text, type) = WeenieErrorMessages.Resolve(p.Value.ErrorCode, p.Value.Interpolation); + if (text is null) + { + Console.WriteLine( + $"[weenie-error] unmapped code=0x{p.Value.ErrorCode:X4} param={p.Value.Interpolation}"); + return; + } if (onInterfaceText is not null) - { - if (WeenieErrorMessages.IsSilentClientControlStatus(p.Value.ErrorCode)) return; - var (text, type) = WeenieErrorMessages.Resolve(p.Value.ErrorCode, p.Value.Interpolation); onInterfaceText(text, type); - } else - { - chat.OnWeenieError(p.Value.ErrorCode, p.Value.Interpolation); - } + chat.OnSystemMessage(text, chatType: (uint)type); }); // ── Combat ──────────────────────────────────────────────── @@ -590,11 +604,27 @@ public static class GameEventWiring + $"err={(err is null ? "n/a" : $"0x{err.Value:X4}")}"); } if (err is null) return; + // Already the diagnostics-only log line SHOULD-FIX 4 asks for — + // it fires unconditionally, so an unmapped code below stays + // visible to US even though it produces no player-facing text. Console.WriteLine($"[use-done] err=0x{err.Value:X4}"); onUseDone?.Invoke(err.Value); if (err.Value == 0) return; + // NIT 6 (docs/research/2026-08-09-ch2-review-findings.md): + // aligned with the WeenieError/WeenieErrorWithString handlers + // above, which check this before resolving. Harmless either + // way today — 0x3B/0x3C have no HandleFailureEvent case, so + // WeenieErrorMessages.Resolve already returns a null Text for + // them — but an explicit early-out here is more direct than + // relying on that coincidence, and guards against a future + // table addition accidentally making one of these two + // resolvable when retail's own switch genuinely has no case + // for either. + if (WeenieErrorMessages.IsSilentClientControlStatus(err.Value)) return; + var (text, type) = WeenieErrorMessages.Resolve(err.Value, null); + if (text is null) return; if (onInterfaceText is not null) onInterfaceText(text, type); else diff --git a/src/AcDream.Core/Chat/ChatLog.cs b/src/AcDream.Core/Chat/ChatLog.cs index 0d09a3fb..574e79c5 100644 --- a/src/AcDream.Core/Chat/ChatLog.cs +++ b/src/AcDream.Core/Chat/ChatLog.cs @@ -183,45 +183,20 @@ public sealed class ChatLog }); } - /// WeenieError (0x028A) / WeenieErrorWithString (0x028B). - /// - /// Phase I.5: previously-orphaned parser. The server fires this when a - /// game-logic action fails (e.g. "you don't have enough mana", "you - /// can't pick that up"). Routed as ; the - /// ChannelId field carries the WeenieError code so plugins can - /// filter or react. is the interpolated - /// substring (null for plain WeenieError, set for WeenieErrorWithString). - /// - public void OnWeenieError(uint errorId, string? param) - { - if (WeenieErrorMessages.IsSilentClientControlStatus(errorId)) - return; - - // Phase I (post-launch fix): translate the wire code into the - // retail-faithful template via WeenieErrorMessages. Many codes - // are *informational* (e.g. 0x051B "You have entered the X - // channel.", 0x051D "Turbine Chat is enabled.") not errors; - // the old "WeenieError 0xNNNN" framing was misleading. Unknown - // codes still fall back to the raw "WeenieError 0xNNNN[: param]" - // form so nothing is silently lost. See - // WeenieErrorMessages.Format for the templates + lookup table. - string text = WeenieErrorMessages.Format(errorId, param); - Append(new ChatEntry( - Kind: ChatKind.System, - Sender: "", - Text: text, - SenderGuid: 0, - ChannelId: errorId) - { - // Retail's HandleFailureEvent @0x00571990 dispatches per ERROR - // CODE across an ~87-case switch, mostly AddTextToScroll(..., - // 0, ...) with a scattered handful at 0x1a (client-local red). - // A full per-code port is future work (register row AP-176); - // 0x00 (Default) matches the switch's majority behavior and is - // the safe baseline. - LogTextType = 0x00u, - }); - } + // WeenieError (0x028A) / WeenieErrorWithString (0x028B) used to have a + // dedicated OnWeenieError entry point here (Phase I.5, hardcoded at + // LogTextType 0x00 pending register row AP-176). REJECT-review rework + // (SHOULD-FIX 3, docs/research/2026-08-09-ch2-review-findings.md): + // AP-176 retired at Campaign CH slice CH2 — WeenieErrorMessages.Resolve + // now resolves BOTH the display text AND the real per-code retail + // RetailLogTextType (chat vs SpewBox) from the full 344-row + // HandleFailureEvent port. Every producer of WeenieError text — the + // inbound GameEventWiring handlers AND the client-command + // ShowWeenieError sink — now resolves through WeenieErrorMessages and + // calls the AddText chokepoint (RuntimeCommunicationState.AddText / + // ChatLog.OnSystemMessage) directly instead of through a dedicated + // ChatLog method, so the single-fixed-color OnWeenieError entry point + // is deleted rather than kept as a second, narrower routing path. /// /// Channel broadcast — legacy ChatChannel (0x0147) or the diff --git a/src/AcDream.Core/Chat/SpewBoxState.cs b/src/AcDream.Core/Chat/SpewBoxState.cs index 9d13f264..5c90067f 100644 --- a/src/AcDream.Core/Chat/SpewBoxState.cs +++ b/src/AcDream.Core/Chat/SpewBoxState.cs @@ -35,21 +35,39 @@ public readonly record struct SpewBoxEntry(string Text, double ExpiresAtSeconds) /// Retail decouples enqueue (RecvNotice_DisplayFinalStringInfo /// @0x004D60A0, type-filtered to 0x1A only) from display /// (Update @0x004D5DF0, driven once per UI tick by global message -/// 3) by exactly one frame. reproduces that: it -/// drains whatever is pending into the visible list (applying retail's -/// dedupe-against-index-0 and MaxConcurrentItems overflow rules) and -/// prunes expired entries, all in the caller's own per-frame cadence. +/// 3) — NOT by exactly one frame (fixed 2026-08-09, CH2 REJECT-review +/// rework NIT 5: the earlier wording overstated this). A message that +/// arrives just before the tick fires waits ~0 frames; one that arrives +/// just after waits nearly a full frame — retail's own gap is 0-1 frames, +/// bounded by tick cadence, not a fixed one-frame delay. +/// reproduces the SAME-CALL shape: it drains whatever is pending into the +/// visible list (applying retail's dedupe-against-index-0 and +/// MaxConcurrentItems overflow rules) and prunes expired entries in +/// one call, so a caller invoking then immediately +/// and in the same frame sees the +/// line SAME-frame — the decoupling only shows up when the caller's own +/// tick cadence spans multiple frames, exactly like retail's. /// /// public sealed class SpewBoxState { /// - /// Retail's own code default (gmSpewBoxUI::PostInit @0x004D5AB0) - /// when ListBox property 0x10000028 is absent or unreadable. The - /// shipped LayoutDesc's authored value was not resolved in this slice — - /// see the divergence register. + /// The shipped LayoutDesc's AUTHORED value — no longer a placeholder. + /// CH2 REJECT-review rework, NIT 3 + /// (docs/research/2026-08-09-ch2-review-findings.md): the + /// original C.7 sweep only searched dats.Portal, which has no + /// 0x10000016 (gmSpewBoxUI) element anywhere; extending the same + /// sweep to dats.Local (client_local_English.dat) found + /// it — LayoutDesc 0x21000011, element 0x10000048, whose + /// sole child (ListBox 0x10000049, matching + /// gmSpewBoxUI::PostInit's GetChildRecursive(0x10000049) + /// call verbatim) carries ListBox property 0x10000028 = the + /// integer 4. Retail's own code default + /// (gmSpewBoxUI::PostInit @0x004D5AB0), used only when this + /// property is absent or unreadable, was 1 — the shipped layout + /// overrides it with 4. /// - public const int MaxConcurrentItems = 1; + public const int MaxConcurrentItems = 4; /// /// Retail's own client never raises the expiry element message diff --git a/src/AcDream.Core/Chat/WeenieErrorMessages.cs b/src/AcDream.Core/Chat/WeenieErrorMessages.cs index 1f262bcc..bc1abb22 100644 --- a/src/AcDream.Core/Chat/WeenieErrorMessages.cs +++ b/src/AcDream.Core/Chat/WeenieErrorMessages.cs @@ -13,58 +13,97 @@ namespace AcDream.Core.Chat; /// (Sept 2013 EoR build), the 339-case switch that decides BOTH the display /// string and the AddTextToScroll type argument for every /// WeenieError/WeenieErrorWithString id retail's client knows -/// about. Transcribed from +/// about. Originally transcribed from /// docs/research/2026-08-09-chat-retail-interface-text.md Appendix A -/// (itself read off the named retail decomp), with the following -/// corrections made directly against -/// docs/research/named-retail/acclient_2013_pseudo_c.txt rather than -/// trusting the appendix's markdown transcription wholesale: +/// (itself read off the named retail decomp's ~33-char inline string +/// previews). +/// +/// +/// +/// REJECT-review rework (2026-08-09, docs/research/2026-08-09-ch2-review-findings.md +/// BLOCKER 2): the Appendix A transcription — "enumerate case +/// labels, read the truncated preview" — turned out to be structurally +/// unsound: it missed 5 ids dispatched via else if (arg2 == N) rather +/// than a case label, and it silently trusted several truncated +/// previews whose full text differs materially from the 33-char prefix. +/// This rework re-derives the table MECHANICALLY: a Python sweep +/// (tools/pdb-extract/sweep_weenie_strings.py) walks the PE section +/// table of the PDB-paired binary (C:\Users\erikn\Downloads\acclient.exe, +/// verified via check_exe_pdb.py) and every push imm32 +/// (opcode 0x68) operand in VA 0x571990-0x575480 that +/// dereferences into .rdata/.data is read as a UTF-16LE +/// literal to its NUL terminator — the FULL string, never the pseudo-C's +/// truncated preview. Every changed row was additionally cross-checked +/// against ACE's own WeenieError.cs/WeenieErrorWithString.cs +/// enum doc comments (references/ACE/Source/ACE.Entity/Enum/); the +/// two independent oracles agreed on every row. /// /// /// -/// 7 ids the appendix marked "no literal — shared string global" +/// 5 ids added that Appendix A missed entirely because they +/// dispatch via else if (arg2 == N) chains ABOVE the main switch, +/// not a case label: 0x04F (Magic), 0x3EE +/// (ClientLocal), 0x408 (ClientLocal), 0x48A (Default), +/// 0x4E8 (Default). 0x43 remains correctly absent — its +/// only retail effect is ClientCombatSystem::AbortAutomaticAttack, +/// no display text. +/// 18 existing rows corrected — the swept binary literal +/// disagreed with the previously-landed text for +/// 0x051/0x053/0x054/0x466/0x4A3/ +/// 0x4B5/0x4E0/0x4E9/0x4F7/0x518/ +/// 0x544/0x54E/0x552/0x553/0x554/ +/// 0x555/0x57F/0x582. Two of these (0x4E9, +/// 0x518) were NOT in the review's own flagged list — they surfaced +/// from an automated diff between every swept literal and the landed +/// table, confirming the review's own "sweep may find more" prediction. +/// 0x4E9 is the sharpest case: the review's own findings doc +/// proposed the wrong text for the NEW 0x4E8 row (attributing +/// 0x4E9's genuine text to it) — the corrected mechanical anchor +/// (the else if (arg2 == 0x4e8) block's OWN instruction address, +/// not proximity to a neighboring case label in the printed +/// listing) plus the ACE cross-check together overrule that proposal: +/// 0x4E8 = "...and only the owner may open the hook.", 0x4E9 +/// = "...use the '@house hooks on' command to make the hook openable." — +/// the previously-landed table had them reversed (0x4E9 held 0x4E8's +/// text; 0x4E8 did not exist as a row at all). +/// 0x4F8 now resolves for real (see below) — no id is +/// deliberately excluded any more. New pinned count: 344 rows +/// (338 landed + 5 added + 0x4F8). +/// 7 ids Appendix A marked "no literal — shared string global" /// (0x024, 0x048, 0x049, 0x4DE, 0x4DF, -/// 0x55A, 0x55E) were resolved by reading the case bodies -/// directly: 0x024/0x048/0x049 reuse the same +/// 0x55A, 0x55E) remain resolved exactly as the prior pass +/// found: 0x024/0x048/0x049 reuse the same /// process-lifetime globals as the local jump-refusal family (see /// ); 0x4DE/0x4DF are /// arg3 + "\n"; 0x55A is sprintf("%s\n", arg3); /// 0x55E passes arg3 straight through with no format string /// at all. -/// Appendix A's markdown table trims leading whitespace from every -/// cell, which silently ate the leading " " retail's -/// arg3 + literal CONCATENATION sites (as opposed to a real -/// sprintf("%s...") site) depend on. 19 ids -/// (0x02B, 0x3EF, 0x46A, 0x4CE, 0x4CF, -/// 0x4F7, 0x4F9, 0x4FA, 0x4FF, 0x509, -/// 0x50B, 0x50C, 0x50D, 0x517, 0x518, -/// 0x51E, 0x521, 0x522) were fixed by re-reading their -/// case bodies and prepending the %s the concatenation implies. -/// Several of these (and the %s-prefixed but truncated -/// 0x4F4-0x4F6, 0x530, 0x534, 0x53E, -/// 0x541, 0x543, 0x54B, 0x562, 0x56D, -/// 0x57A, 0x57B, 0x580) were also truncated by the -/// pseudo-C's ~33-char inline preview; the full text was recovered from a -/// SECOND, non-truncated data_XXXXXXXX dump elsewhere in the same -/// oracle file — reading the oracle again, not guessing. /// 0x4F4's retail literal is /// "%s fails to affect you because $s cannot affect anyone!" — /// note $s, not %s, for the second placeholder. That is a /// genuine retail typo/bug (only the first %s substitutes; the /// literal $s prints as-is) and is preserved verbatim rather than -/// corrected, per the "the client is probably right" rule. -/// 0x4F7's retail string is itself incomplete — it -/// concatenates arg3 with a literal that dangles on -/// "...as " with no closing word. Confirmed via exact byte-count -/// against the declared array size (not a display artifact); preserved -/// verbatim. -/// 0x4F8 could not be resolved with confidence — its case -/// body is a tangled multi-operator+ concatenation chain full of -/// decompiler self-referential artifacts (see -/// claude-memory/feedback_bn_decomp_field_names.md). Deliberately -/// EXCLUDED from the table rather than guessed; falls back to the generic -/// WeenieError 0xNNNN[: param] form like any other unmapped id. -/// +/// corrected, per the "the client is probably right" rule. 0x04F +/// preserves the same $s typo pattern. +/// 0x4F7's retail string is a normal, COMPLETE +/// arg3 + literal concatenation — "%s fails to affect you because +/// you are not a player killer!" — with no dangling text. The prior +/// pass's class-doc claim that 0x4F7's literal "dangles" was a +/// misattribution of 0x4F8's FIRST operand (which does end +/// mid-clause, on "...as ", by design — see 0x4F8 below), not a +/// genuine truncation in 0x4F7 itself. +/// 0x4F8 resolves cleanly once traced correctly: its case +/// body concatenates arg3 + " fails to affect you because you are not +/// the same sort of player killer as " + arg3 + "!\n" across three +/// operator+ calls whose decompiled operand names are BN +/// self-referential artifacts (see +/// claude-memory/feedback_bn_decomp_field_names.md) — the sweep's +/// direct dereference of both literal data pointers +/// (data_7d2ee8 = " fails to affect you because you are not the +/// same sort of player killer as ", data_7d2f80 = +/// "!\n") resolves the ambiguity without needing to trust the +/// confusing decompiler naming. Both %s placeholders take the SAME +/// parameter (retail only has one arg3 to substitute twice). /// /// /// @@ -106,16 +145,31 @@ public static class WeenieErrorMessages /// The wire error code. /// The interpolated substring (null for plain /// WeenieError, set for WeenieErrorWithString). - public static string Format(uint errorCode, string? param) => Resolve(errorCode, param).Text; + /// + /// The retail display text, or for an id + /// HandleFailureEvent's switch has no case for — see + /// . + /// + public static string? Format(uint errorCode, string? param) => Resolve(errorCode, param).Text; /// /// Resolve a WeenieError / WeenieErrorWithString code into its retail /// display text AND the retail it routes - /// to. Unmapped codes fall back to the raw - /// WeenieError 0xNNNN[: param] form at - /// so nothing is silently lost. + /// to. /// - public static (string Text, RetailLogTextType Type) Resolve(uint errorCode, string? param) + /// + /// CH2 REJECT-review rework, SHOULD-FIX 4 + /// (docs/research/2026-08-09-ch2-review-findings.md): retail's + /// HandleFailureEvent switch has NO default: case — an id + /// it does not recognize produces NO text at all, silently, toward the + /// player. The prior pass's WeenieError 0xNNNN[: param] hex + /// fallback was acdream's own invention with no retail counterpart (an + /// unregistered divergence). Callers must treat a + /// Text as "retail shows nothing here" and skip display entirely; + /// they should still log the raw id to a diagnostics-only sink so an + /// unmapped code is not silently invisible to US, only to the player. + /// + public static (string? Text, RetailLogTextType Type) Resolve(uint errorCode, string? param) { if (Table.TryGetValue(errorCode, out Entry entry)) { @@ -125,17 +179,17 @@ public static class WeenieErrorMessages return (text, entry.Type); } - string fallback = string.IsNullOrEmpty(param) - ? $"WeenieError 0x{errorCode:X4}" - : $"WeenieError 0x{errorCode:X4}: {param}"; - return (fallback, RetailLogTextType.Default); + return (null, RetailLogTextType.Default); } /// /// The full retail routing table, transcribed from - /// ClientCommunicationSystem::HandleFailureEvent @0x00571990 (338 - /// of its 339 cases — see the class doc comment for the one deliberate - /// exclusion and every correction made against the raw decomp). + /// ClientCommunicationSystem::HandleFailureEvent @0x00571990 — 344 + /// rows (338 landed at the prior pass + 5 ids the prior pass's + /// case-label enumeration missed + 0x4F8, which now resolves for + /// real; every id the switch dispatches has a row here). See the class + /// doc comment for the binary-sweep + ACE cross-check methodology and + /// every correction made against it. /// private static readonly Dictionary Table = new() { @@ -162,11 +216,20 @@ public static class WeenieErrorMessages [0x04Au] = new("Ack! You killed yourself!", RetailLogTextType.Default), [0x04Du] = new("Invalid PK status!", RetailLogTextType.ClientLocal), [0x04Eu] = new("You fail to affect %s because you cannot affect anyone!", RetailLogTextType.Magic), + // 0x04F added — BLOCKER2: else-if dispatch (arg2 == 0x4f), missed by + // the prior case-label enumeration. Preserves retail's own $s typo, + // same pattern as 0x4F4. + [0x04Fu] = new("You fail to affect %s because $s cannot be harmed!", RetailLogTextType.Magic), [0x050u] = new("You fail to affect %s because beneficial spells do not affect %s!", RetailLogTextType.Magic), - [0x051u] = new("You fail to affect %s because you cannot affect anyone!", RetailLogTextType.Magic), + // 0x051/0x053/0x054 corrected — BLOCKER2: the prior pass copied + // 0x04E's/0x053's template across all four "You fail to affect" + // Magic-family ids; each is in fact a DISTINCT retail literal. + [0x051u] = new("You fail to affect %s because you are not a player killer!", RetailLogTextType.Magic), [0x052u] = new("You fail to affect %s because %s is not a player killer!", RetailLogTextType.Magic), - [0x053u] = new("You fail to affect %s because you cannot affect anyone!", RetailLogTextType.Magic), - [0x054u] = new("You fail to affect %s because you cannot affect anyone!", RetailLogTextType.Magic), + [0x053u] = new("You fail to affect %s because you are not the same sort of player killer as %s!", RetailLogTextType.Magic), + [0x054u] = new("You fail to affect %s because you are acting across a house boundary!", RetailLogTextType.Magic), + // 0x3EE added — BLOCKER2: else-if dispatch (arg2 == 0x3ee). + [0x3EEu] = new("The container is closed!", RetailLogTextType.ClientLocal), [0x3EFu] = new("%s is not accepting gifts right now.", RetailLogTextType.Default), [0x3F1u] = new("You failed to go to non-combat mode.", RetailLogTextType.ClientLocal), [0x3F7u] = new("You are too fatigued to attack!", RetailLogTextType.ClientLocal), @@ -181,6 +244,10 @@ public static class WeenieErrorMessages [0x403u] = new("Your spell's target is missing!", RetailLogTextType.ClientLocal), [0x404u] = new("Your projectile spell mislaunched!", RetailLogTextType.ClientLocal), [0x407u] = new("Your spell cannot be cast outside", RetailLogTextType.ClientLocal), + // 0x408 added — BLOCKER2: else-if dispatch (arg2 == 0x408), sibling + // of 0x407 above (both lack trailing punctuation in retail's own + // literal — not a display artifact). + [0x408u] = new("Your spell cannot be cast inside", RetailLogTextType.ClientLocal), [0x40Au] = new("You are unprepared to cast a spell", RetailLogTextType.ClientLocal), [0x40Bu] = new("You've already sworn your Allegiance", RetailLogTextType.ClientLocal), [0x40Cu] = new("You don't have enough experience available to swear Allegiance", RetailLogTextType.ClientLocal), @@ -215,7 +282,10 @@ public static class WeenieErrorMessages [0x45Du] = new("Non-player killers may not interact with that portal!", RetailLogTextType.Magic), [0x45Eu] = new("You do not own a house!", RetailLogTextType.ClientLocal), [0x45Fu] = new("You do not own a house!", RetailLogTextType.ClientLocal), - [0x466u] = new("You must purchase Asheron's Call -- Dark Majesty to use this function.", RetailLogTextType.Magic), + // 0x466 corrected — BLOCKER2: retail's colon-separated "Asheron's + // Call: Dark Majesty..." wording, distinct from the "--"-separated + // Throne of Destiny family at 0x552-0x555 below. + [0x466u] = new("You must purchase Asheron's Call: Dark Majesty to interact with that portal.", RetailLogTextType.Magic), [0x469u] = new("You have used all the hooks you are allowed to use for this house.", RetailLogTextType.Default), [0x46Au] = new("%s doesn't know what to do with that.", RetailLogTextType.Default), [0x474u] = new("You must complete a quest to interact with that portal.", RetailLogTextType.Magic), @@ -225,6 +295,8 @@ public static class WeenieErrorMessages [0x482u] = new("Your monarch has closed the mansion to the Allegiance.", RetailLogTextType.ClientLocal), [0x488u] = new("You must be above level %s to purchase this dwelling.", RetailLogTextType.Default), [0x489u] = new("You must be at or below level %s to purchase this dwelling.", RetailLogTextType.Default), + // 0x48A added — BLOCKER2: else-if dispatch (arg2 == 0x48a). + [0x48Au] = new("You must be a monarch to purchase this dwelling.", RetailLogTextType.Default), [0x48Bu] = new("You must be above allegiance rank %s to purchase this dwelling.", RetailLogTextType.Default), [0x48Cu] = new("You must be at or below allegiance rank %s to purchase this dwelling.", RetailLogTextType.Default), [0x48Eu] = new("Your offer of Allegiance has been ignored.", RetailLogTextType.ClientLocal), @@ -248,7 +320,9 @@ public static class WeenieErrorMessages [0x4A0u] = new("You fail to link with the portal!", RetailLogTextType.Magic), [0x4A1u] = new("You successfully link with the portal!", RetailLogTextType.Magic), [0x4A2u] = new("You fail to recall to the portal!", RetailLogTextType.Magic), - [0x4A3u] = new("You must have linked with a portal in order to summon it!", RetailLogTextType.Magic), + // 0x4A3 corrected — BLOCKER2: retail's real text is about RECALL, + // not summon (0x4A5 below is the genuine "summon it" sibling). + [0x4A3u] = new("You must have linked with a portal in order to recall to it!", RetailLogTextType.Magic), [0x4A4u] = new("You fail to summon the portal!", RetailLogTextType.Magic), [0x4A5u] = new("You must have linked with a portal in order to summon it!", RetailLogTextType.Magic), [0x4A6u] = new("You fail to teleport!", RetailLogTextType.Magic), @@ -265,7 +339,9 @@ public static class WeenieErrorMessages [0x4B2u] = new("The key doesn't fit this lock.", RetailLogTextType.Default), [0x4B3u] = new("The lock has been used too recently.", RetailLogTextType.ClientLocal), [0x4B4u] = new("You aren't trained in lockpicking!", RetailLogTextType.ClientLocal), - [0x4B5u] = new("You must specify a character to boot.", RetailLogTextType.ClientLocal), + // 0x4B5 corrected — BLOCKER2: distinct from 0x491's genuine "boot" + // wording (0x4B5 is the allegiance-query variant). + [0x4B5u] = new("You must specify a character to query.", RetailLogTextType.ClientLocal), [0x4B6u] = new("Please use the allegiance panel to view your own information.", RetailLogTextType.ClientLocal), [0x4B7u] = new("You have used that command too recently.", RetailLogTextType.ClientLocal), [0x4B8u] = new("You do not own that salvage tool!", RetailLogTextType.Default), @@ -303,7 +379,12 @@ public static class WeenieErrorMessages [0x4DDu] = new("You have failed to alter your attributes.", RetailLogTextType.Default), [0x4DEu] = new("%s", RetailLogTextType.Default), [0x4DFu] = new("%s", RetailLogTextType.Default), - [0x4E0u] = new("You are currently wielding items which require a certain level of %s. Your %s skill cannot be lowered while you are wielding these items. Please remove these items and try again.", RetailLogTextType.Default), + // 0x4E0 corrected — BLOCKER2: the prior pass copied 0x4D5's + // "skill cannot be lowered" template; retail's genuine 0x4E0 text is + // about ATTRIBUTES not transferring, a distinct sibling to 0x4DD + // ("failed to alter your attributes") and 0x4E1 ("succeeded in + // transferring your attributes!"). + [0x4E0u] = new("You are currently wielding items which require a certain level of skill. Your attributes cannot be transferred while you are wielding these items. Please remove these items and try again.", RetailLogTextType.Default), [0x4E1u] = new("You have succeeded in transferring your attributes!", RetailLogTextType.Default), [0x4E2u] = new("This hook is a duplicated housing object. You may not add items to a duplicated housing object. Please empty the hook and allow it to reset.", RetailLogTextType.Default), [0x4E3u] = new("That item is of the wrong type to be placed on this hook.", RetailLogTextType.Default), @@ -311,7 +392,15 @@ public static class WeenieErrorMessages [0x4E5u] = new("This hook was a duplicated housing object. Since it is now empty, it will be deleted momentarily. Once it is gone, it is safe to use the other, non-duplicated hook that is here.", RetailLogTextType.Default), [0x4E6u] = new("This chest was a duplicated housing object. Since it is now empty, it will be deleted momentarily. Once it is gone, it is safe to use the other, non-duplicated chest that is here.", RetailLogTextType.Default), [0x4E7u] = new("You cannot swear allegiance to anyone because you own a monarch-only house. Please abandon your house and try again.", RetailLogTextType.Default), - [0x4E9u] = new("The %s cannot be used while on a hook and only the owner may open the hook.", RetailLogTextType.Default), + // 0x4E8 added — BLOCKER2: else-if dispatch (arg2 == 0x4e8), anchored + // on that branch's OWN instruction address (not proximity to a + // neighboring case label — see the class doc comment's 0x4E9 note). + [0x4E8u] = new("The %s cannot be used while on a hook and only the owner may open the hook.", RetailLogTextType.Default), + // 0x4E9 corrected — BLOCKER2: the prior pass gave this row 0x4E8's + // text (the two are easy to conflate — near-duplicate wording for + // adjacent ids). Confirmed against ACE's ItemUnusableOnHook_CanOpen + // (0x04E9) doc comment. + [0x4E9u] = new("The %s cannot be used while on a hook, use the '@house hooks on' command to make the hook openable.", RetailLogTextType.Default), [0x4EAu] = new("The %s can only be used while on a hook.", RetailLogTextType.Default), [0x4EBu] = new("You can't do that while in the air!", RetailLogTextType.ClientLocal), [0x4ECu] = new("You cannot modify your player killer status while you are recovering from a PK death.", RetailLogTextType.Default), @@ -325,8 +414,17 @@ public static class WeenieErrorMessages [0x4F4u] = new("%s fails to affect you because $s cannot affect anyone!", RetailLogTextType.Magic), [0x4F5u] = new("%s fails to affect you because you cannot be harmed!", RetailLogTextType.Magic), [0x4F6u] = new("%s fails to affect you because %s is not a player killer!", RetailLogTextType.Magic), - [0x4F7u] = new("%s fails to affect you because you are not the same sort of player killer as", RetailLogTextType.Magic), - // 0x4F8 deliberately excluded — see the class doc comment. + // 0x4F7 corrected — BLOCKER2: this is a COMPLETE, ordinary + // concatenation, not the dangling literal the prior pass's class + // doc misattributed to it (that "..as " dangle belongs to 0x4F8, + // its first operand — see the class doc comment). + [0x4F7u] = new("%s fails to affect you because you are not a player killer!", RetailLogTextType.Magic), + // 0x4F8 added — BLOCKER2: previously deliberately excluded pending + // a confident trace of its 3-operator+ concatenation chain; the + // binary sweep's direct dereference of both literal data pointers + // (data_7d2ee8 + data_7d2f80) resolves it. Both %s placeholders + // substitute the SAME parameter (retail only has one arg3). + [0x4F8u] = new("%s fails to affect you because you are not the same sort of player killer as %s!", RetailLogTextType.Magic), [0x4F9u] = new("%s fails to affect you across a house boundary!", RetailLogTextType.Magic), [0x4FAu] = new("%s is an invalid target.", RetailLogTextType.Magic), [0x4FBu] = new("You are an invalid target for the spell of %s.", RetailLogTextType.Magic), @@ -357,7 +455,14 @@ public static class WeenieErrorMessages [0x515u] = new("You no longer have the maximum number of %s hooked. You may hook additional %s.", RetailLogTextType.Default), [0x516u] = new("You are not permitted to use that hook.", RetailLogTextType.Default), [0x517u] = new("%s is not close enough to your level.", RetailLogTextType.Default), - [0x518u] = new("%s cannot be recruited into the fellowship.", RetailLogTextType.Default), + // 0x518 corrected — BLOCKER2: not in the review's flagged list; + // surfaced from an automated diff between the swept binary + // literals and the landed table. Retail's real text is a 3-part + // concatenation ("This fellowship is locked; " + arg3 + " cannot be + // recruited into the fellowship."), not the arg3-only fragment + // previously landed. Confirmed against ACE's + // LockedFellowshipCannotRecruit_ (0x0518) doc comment. + [0x518u] = new("This fellowship is locked; %s cannot be recruited into the fellowship.", RetailLogTextType.Default), [0x519u] = new("The fellowship is locked, you were not added to the fellowship.", RetailLogTextType.Default), [0x51Au] = new("Only the original owner may use that item's magic.", RetailLogTextType.ClientLocal), [0x51Bu] = new("You have entered the %s channel.", RetailLogTextType.Default), @@ -398,7 +503,10 @@ public static class WeenieErrorMessages [0x541u] = new("%s is now an allegiance officer.", RetailLogTextType.Default), [0x542u] = new("An unspecified error occurred while attempting to set %s as an allegiance officer.", RetailLogTextType.Default), [0x543u] = new("%s is no longer an allegiance officer.", RetailLogTextType.Default), - [0x544u] = new("An unspecified error occurred while attempting to set %s as an allegiance officer.", RetailLogTextType.Default), + // 0x544 corrected — BLOCKER2: retail's genuine text says REMOVE, not + // SET (0x542 above is the genuine "set" sibling); the prior pass + // duplicated 0x542's template here. + [0x544u] = new("An unspecified error occurred while attempting to remove %s as an allegiance officer.", RetailLogTextType.Default), [0x545u] = new("You already have the maximum number of allegiance officers. You must remove some before you add any more.", RetailLogTextType.Default), [0x546u] = new("Your allegiance officers have been cleared.", RetailLogTextType.Default), [0x547u] = new("You must wait %s before communicating again!", RetailLogTextType.Default), @@ -408,14 +516,24 @@ public static class WeenieErrorMessages [0x54Bu] = new("%s is already an allegiance officer of that level.", RetailLogTextType.Default), [0x54Cu] = new("Your allegiance does not have a hometown.", RetailLogTextType.Default), [0x54Du] = new("The %s is currently in use.", RetailLogTextType.ClientLocal), - [0x54Eu] = new("The hook does not contain a usable item. Use the '@house hooks on'command to make the hook openable.", RetailLogTextType.Default), + // 0x54E corrected — BLOCKER2: retail's genuine 0x54E text is the + // "you do not own the house" variant, distinct from 0x54F's + // "@house hooks on" variant the prior pass duplicated onto both ids. + [0x54Eu] = new("The hook does not contain a usable item. You cannot open the hook because you do not own the house to which it belongs.", RetailLogTextType.Default), [0x54Fu] = new("The hook does not contain a usable item. Use the '@house hooks on'command to make the hook openable.", RetailLogTextType.Default), [0x550u] = new("Out of Range!", RetailLogTextType.ClientLocal), [0x551u] = new("You are not listening to the %s channel!", RetailLogTextType.Default), - [0x552u] = new("You must purchase Asheron's Call -- Dark Majesty to use this function.", RetailLogTextType.ClientLocal), - [0x553u] = new("You must purchase Asheron's Call -- Dark Majesty to use this function.", RetailLogTextType.ClientLocal), - [0x554u] = new("You must purchase Asheron's Call -- Dark Majesty to use this function.", RetailLogTextType.ClientLocal), - [0x555u] = new("You must purchase Asheron's Call -- Dark Majesty to use this function.", RetailLogTextType.ClientLocal), + // 0x552-0x555 corrected — BLOCKER2: the prior pass copied 0x49A's + // "Dark Majesty...to use this function." template across all four + // ids. Retail's genuine family is the "--"-separated Throne of + // Destiny expansion, each with its OWN distinct ending + // (function/item/portal/quest) — not four identical strings. + // Confirmed against ACE's MustPurchaseThroneOfDestinyToUseFunction/ + // ToUseItem/ToUsePortal/ToAccessQuest (0x0552-0x0555) doc comments. + [0x552u] = new("You must purchase Asheron's Call -- Throne of Destiny to use this function.", RetailLogTextType.ClientLocal), + [0x553u] = new("You must purchase Asheron's Call -- Throne of Destiny to use this item.", RetailLogTextType.ClientLocal), + [0x554u] = new("You must purchase Asheron's Call -- Throne of Destiny to use this portal.", RetailLogTextType.ClientLocal), + [0x555u] = new("You must purchase Asheron's Call -- Throne of Destiny to access this quest.", RetailLogTextType.ClientLocal), [0x556u] = new("You have failed to complete the augmentation.", RetailLogTextType.Default), [0x557u] = new("You have used this augmentation too many times already.", RetailLogTextType.Default), [0x558u] = new("You have used augmentations of this type too many times already.", RetailLogTextType.Default), @@ -457,10 +575,16 @@ public static class WeenieErrorMessages [0x57Cu] = new("You have cleared the pre-approved vassal for your allegiance.", RetailLogTextType.Default), [0x57Du] = new("That character is already gagged!", RetailLogTextType.Default), [0x57Eu] = new("That character is not currently gagged!", RetailLogTextType.Default), - [0x57Fu] = new("Your allegiance chat privileges have been restored.", RetailLogTextType.Default), + // 0x57F corrected — BLOCKER2: 0x581's "restored" text was + // incorrectly duplicated here; 0x57F is the "removed" notice, the + // sibling that fires when privileges are taken away, not restored. + [0x57Fu] = new("Your allegiance chat privileges have been temporarily removed by %s. Until they are restored, you may not view or speak in the allegiance chat channel.", RetailLogTextType.Default), [0x580u] = new("%s is now temporarily unable to view or speak in allegiance chat. The gag will run out in 5 minutes, or %s may be explicitly ungagged before then.", RetailLogTextType.Default), [0x581u] = new("Your allegiance chat privileges have been restored.", RetailLogTextType.Default), - [0x582u] = new("Your allegiance chat privileges have been restored.", RetailLogTextType.Default), + // 0x582 corrected — BLOCKER2: 0x581's unparameterized text was + // incorrectly duplicated here; 0x582 is the %s-parameterized + // sibling ("...restored BY %s."). + [0x582u] = new("Your allegiance chat privileges have been restored by %s.", RetailLogTextType.Default), [0x583u] = new("You have restored allegiance chat privileges to %s.", RetailLogTextType.Default), [0x584u] = new("You cannot pick up more of that item!", RetailLogTextType.ClientLocal), [0x585u] = new("You are restricted to clothes and armor created for your race.", RetailLogTextType.ClientLocal), diff --git a/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs b/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs index b5ca0cc9..99bada0b 100644 --- a/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs +++ b/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs @@ -227,9 +227,17 @@ internal sealed class HeadlessGameplayOperations // Opus review of 172c6f9a). Same ClientCommandController-output // category as ChatVM.ShowSystemMessage; retail types the great // majority of that output green, reserving 0x1A (bright red) for - // genuine refusals. The refusal-vs-informational split lands with - // CH2's producer rewiring (SpewBox routing) — see - // docs/research/2026-08-09-chat-retail-interface-text.md §7.2. + // genuine refusals. + // + // Comment corrected 2026-08-09, CH2 REJECT-review rework (NIT 2, + // docs/research/2026-08-09-ch2-review-findings.md): CH2's SpewBox + // routing covers WeenieError/WeenieErrorWithString ids, which carry + // their own resolved RetailLogTextType — it did NOT reach this sink, + // which takes plain pre-formatted text with no error code attached. + // Per-call-site refusal-vs-informational classification of this + // sink's callers, plus retail's windowId dual-destination echo + // (register row AP-180), remain unstarted — CH4/CH5 scope at the + // earliest, not CH2. public void DisplayMessage(string message) => RequireRuntime().CommunicationOwner.Chat.OnSystemMessage( message, diff --git a/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs b/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs index 01372dab..188a33f6 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs @@ -158,20 +158,31 @@ public sealed class RuntimeCommunicationState : IDisposable /// with exactly as before. /// /// is accepted for future parity with - /// retail's per-window echo (a non-zero window ID lands in both the - /// SpewBox AND that specific chat window — research doc §2.3) but is - /// not yet consumed; every current production caller passes the - /// default 0. + /// retail's per-window echo (a non-zero window ID lands in BOTH the + /// SpewBox AND that specific chat window, ~40 slash-command-output + /// sites — research doc §2.3) but is not yet consumed; every current + /// production caller passes the default 0. This dual-destination + /// gap is filed as register row AP-180 — implementing it is CH4/CH5 + /// scope at the earliest. /// public void AddText(string text, RetailLogTextType type, uint windowId = 0) { ArgumentNullException.ThrowIfNull(text); - // Retail's own first step (0x00563C50): trim trailing whitespace - // before anything else, regardless of destination. - text = text.TrimEnd(); - if (text.Length == 0) - return; + // CH2 REJECT-review rework (SHOULD-FIX 2, + // docs/research/2026-08-09-ch2-review-findings.md): retail's own + // first step (ClientSystem::AddTextToScroll @0x00563C50) is + // trim(&str, 1, 1, ws) — BOTH ends, not trailing-only (the + // trailing-only trim in research doc §3.1 belongs to + // gmSpewBoxUI::Update, a SEPARATE later call on the SpewBox's own + // display path, not this chokepoint). Retail also has no empty- + // string guard here — AddTextToScroll broadcasts empty strings + // deliberately (the type-7/Magic s_NullBuffer sites reuse a shared + // buffer that can legitimately be empty between calls); inventing + // an early-return for empty text was an unregistered acdream-only + // divergence, now retired rather than kept as a guessed + // approximation. + text = text.Trim(); if (type == RetailLogTextType.ClientLocal) { diff --git a/src/AcDream.Runtime/RuntimeGenerationReset.cs b/src/AcDream.Runtime/RuntimeGenerationReset.cs index 6bbdff8a..705b5776 100644 --- a/src/AcDream.Runtime/RuntimeGenerationReset.cs +++ b/src/AcDream.Runtime/RuntimeGenerationReset.cs @@ -315,7 +315,24 @@ public sealed class RuntimeGenerationReset state.Stage = RuntimeGenerationResetStage.ChatIdentity; break; case RuntimeGenerationResetStage.ChatIdentity: - Advance(state, _communication.ResetChatIdentity); + // CH2 REJECT-review rework (SHOULD-FIX 1, + // docs/research/2026-08-09-ch2-review-findings.md): + // RuntimeCommunicationState.ResetSpewBox was dead code — + // no caller reset the transient SpewBox queue at + // generation boundaries even though ResetChatIdentity + // (the chat transcript's identity/dedup reset) already + // ran here every generation. They share this stage + // because they're the same lifetime boundary — a fresh + // generation must not resurrect a stale refusal line — + // even though they differ in WHAT they reset: + // ResetChatIdentity preserves the visible transcript, + // ResetSpewBox clears it (see RuntimeCommunicationState's + // own doc comments on each). + Advance(state, () => + { + _communication.ResetChatIdentity(); + _communication.ResetSpewBox(); + }); break; case RuntimeGenerationResetStage.PlayerSnapshots: Advance(state, _inventory.ResetPlayerSnapshots); diff --git a/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs b/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs index 9ee54b3c..669aab1c 100644 --- a/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs +++ b/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs @@ -118,9 +118,24 @@ public sealed class ChatVM : IDisposable /// general-purpose output — @version, /loc, friends list, usage lines — /// and retail types the great majority of that informational command /// output 0x00, reserving 0x1A (bright red) for genuine - /// refusals/errors. The refusal-vs-informational split lands with CH2's - /// producer rewiring (SpewBox routing) — see - /// docs/research/2026-08-09-chat-retail-interface-text.md §7.2. + /// refusals/errors. + /// + /// + /// Comment corrected 2026-08-09, CH2 REJECT-review rework (NIT 2, + /// docs/research/2026-08-09-ch2-review-findings.md): the earlier + /// wording claimed the refusal-vs-informational split "lands with CH2's + /// producer rewiring" — it did not. CH2's SpewBox routing covers + /// WeenieError/WeenieErrorWithString ids, which carry their + /// own resolved RetailLogTextType; this sink takes plain + /// pre-formatted TEXT with no error code attached, so + /// WeenieErrorMessages has nothing to classify here. Per-call-site + /// classification of THIS sink's callers (which specific + /// ClientCommandController lines are genuine refusals retail + /// would type 0x1A) remains unstarted, and even a classified + /// caller would still need retail's windowId dual-destination + /// echo (see register row AP-180) to land in both the SpewBox and the + /// command's originating chat window — out of scope for CH4/CH5, not + /// CH2. /// public void ShowSystemMessage(string text) => _log.OnSystemMessage(text, chatType: 0x00u); diff --git a/tests/AcDream.App.Tests/UI/SpewBoxControllerTests.cs b/tests/AcDream.App.Tests/UI/SpewBoxControllerTests.cs index c1f35559..6e4bb7a9 100644 --- a/tests/AcDream.App.Tests/UI/SpewBoxControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/SpewBoxControllerTests.cs @@ -4,6 +4,17 @@ using AcDream.UI.Abstractions.Panels.SpewBox; namespace AcDream.App.Tests.UI; +/// +/// CH2 REJECT-review rework, BLOCKER 1 +/// (docs/research/2026-08-09-ch2-review-findings.md): these tests +/// drive the controller's frame hook (UiRoot.Tick, the global-message-3 +/// broadcast) rather than calling UiText.LinesProvider directly. The +/// original tests invoked the provider straight through, bypassing the +/// Visible gate that UiText.OnDraw checks before ever polling +/// it — that is exactly why the original bug (start-invisible → provider +/// never called by drawing → no line ever visible, pending queue never +/// drains) was invisible to the original test suite. +/// public sealed class SpewBoxControllerTests { [Fact] @@ -18,13 +29,18 @@ public sealed class SpewBoxControllerTests using (var controller = new SpewBoxController(root, new SpewBoxVM(state))) { - UiText text = Assert.IsType(Assert.Single(root.Children)); + UiText text = Assert.IsType(root.Children.OfType().Single()); Assert.False(text.Visible); Assert.True(text.ClickThrough); Assert.Equal(int.MaxValue, text.ZOrder); state.Enqueue("You can't jump while in the air"); + // Drive the frame hook, NOT the provider — this is the tick + // that drains the pending queue in production, wired through + // UiRoot.Tick's IUiGlobalTimeListener broadcast. + root.Tick(dt: 0d, nowMs: 1000L); + UiText.Line line = Assert.Single(text.LinesProvider!()); Assert.Equal("You can't jump while in the air", line.Text); Assert.True(text.Visible); @@ -39,8 +55,92 @@ public sealed class SpewBoxControllerTests var root = new UiRoot { Width = 1280f, Height = 720f }; using var controller = new SpewBoxController(root, new SpewBoxVM(new SpewBoxState())); - UiText text = Assert.IsType(Assert.Single(root.Children)); + root.Tick(dt: 0d, nowMs: 1000L); + + UiText text = Assert.IsType(root.Children.OfType().Single()); Assert.Empty(text.LinesProvider!()); Assert.False(text.Visible); } + + [Fact] + public void Controller_BecomesVisible_WithoutAnyDrawHavingHappenedFirst() + { + // BLOCKER 1 acceptance (a): the original bug was start-invisible → + // LinesProvider only reachable through UiElement.DrawSelfAndChildren + // (which returns early on !Visible) → provider never called → never + // visible. This test never calls anything draw-shaped — no OnDraw, + // no DrawSelfAndChildren, no UiRenderContext — only the tick. + var root = new UiRoot { Width = 1280f, Height = 720f }; + var state = new SpewBoxState(); + using var controller = new SpewBoxController(root, new SpewBoxVM(state)); + + UiText text = Assert.IsType(root.Children.OfType().Single()); + Assert.False(text.Visible); + + state.Enqueue("Out of Range!"); + root.Tick(dt: 0d, nowMs: 500L); + + Assert.True(text.Visible); + Assert.Equal("Out of Range!", Assert.Single(text.LinesProvider!()).Text); + } + + [Fact] + public void Controller_PendingQueueDrains_WithoutAnyDrawPass() + { + // BLOCKER 1 acceptance (b): SpewBoxState.Tick is the only caller + // that drains _pending (SpewBoxState.cs's Queue). Before the + // fix, that call only happened inside LinesProvider, which drawing + // gated behind Visible — so the pending queue was an unbounded + // per-session leak. Enqueue, tick the frame hook, and assert the + // underlying state actually drained (Count reflects the visible + // set, not the still-pending queue) — never touching drawing. + var root = new UiRoot { Width = 1280f, Height = 720f }; + var state = new SpewBoxState(); + using var controller = new SpewBoxController(root, new SpewBoxVM(state)); + + state.Enqueue("first"); + Assert.Equal(0, state.Count); // still pending, not yet drained + + root.Tick(dt: 0d, nowMs: 100L); + + Assert.Equal(1, state.Count); // drained into the visible list + Assert.Equal("first", state.Snapshot()[0].Text); + } + + [Fact] + public void Controller_QueueStaysBounded_AcrossManyTicksWithoutADrawPass() + { + // BLOCKER 1 acceptance (c): repeated enqueue+tick cycles must never + // let the visible set grow past SpewBoxState.MaxConcurrentItems + // (retail's code default, 1) — this is what "unbounded per-session + // leak" would have looked like if the drain never ran at all. + var root = new UiRoot { Width = 1280f, Height = 720f }; + var state = new SpewBoxState(); + using var controller = new SpewBoxController(root, new SpewBoxVM(state)); + + for (int i = 0; i < 50; i++) + { + state.Enqueue($"line {i}"); + root.Tick(dt: 0d, nowMs: 1000L + i); + Assert.True(state.Count <= SpewBoxState.MaxConcurrentItems); + } + + UiText text = Assert.IsType(root.Children.OfType().Single()); + Assert.True(text.LinesProvider!().Count <= SpewBoxState.MaxConcurrentItems); + // Newest-at-top, matching retail's InsertItem(item, 0). + Assert.Equal("line 49", text.LinesProvider!()[0].Text); + } + + [Fact] + public void Dispose_RemovesBothTheTextAndTheGlobalTimeSinkFromTheRoot() + { + var root = new UiRoot { Width = 1280f, Height = 720f }; + var controller = new SpewBoxController(root, new SpewBoxVM(new SpewBoxState())); + + Assert.Equal(2, root.Children.Count); // UiText + the tick sink + + controller.Dispose(); + + Assert.Empty(root.Children); + } } diff --git a/tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs b/tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs index 6970fc9b..061fea95 100644 --- a/tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs +++ b/tests/AcDream.App.Tests/UI/SpewBoxLayoutDumpDiagnostic.cs @@ -33,14 +33,32 @@ namespace AcDream.App.Tests.UI; /// appears to reference — exists but has zero top-level elements (it is /// not the per-line template catalog; that id must resolve through a /// different mechanism than a direct dats.Get<LayoutDesc> hit, -/// which this slice did not crack). Conclusion: gmSpewBoxUI is -/// mounted directly from C++ code in gmClient's HUD registration -/// block (research doc §1.1) rather than resolved from any authored -/// LayoutDesc tree — its position/extent/font/color/max-items are NOT -/// recoverable via this dump technique. Every value the SpewBox -/// presentation uses below is therefore an invented placeholder with its -/// own divergence-register row, exactly as the research doc's §3.2 -/// PRESENTATION-UNKNOWN section predicted. +/// which this slice did not crack). +/// +/// +/// +/// RESULT 2, CH2 REJECT-review rework NIT 3 +/// (2026-08-09, docs/research/2026-08-09-ch2-review-findings.md): the +/// FIRST result above only ever swept dats.Portal. Extending the +/// IDENTICAL sweep to dats.Local (client_local_English.dat) +/// FINDS it: LayoutDesc 0x21000011, element 0x10000048 +/// (class 0x10000016), position (0,0) relative to its parent, +/// size 450×72, one child — ListBox 0x10000049 (matching +/// gmSpewBoxUI::PostInit's GetChildRecursive(0x10000049) call +/// verbatim, though its own widget-class Type is 0x00000005, +/// NOT the id 0x10000049 — GetChildRecursive searches by +/// ElementId, not Type) — carrying MaxConcurrentItems +/// (property 0x10000028) = 4. gmSpewBoxUI IS +/// dat-authored after all; the earlier "mounted directly from C++ code... +/// not resolved from any authored LayoutDesc tree" conclusion was an +/// artifact of only having checked one of the two locale-bearing dats. +/// Extent and MaxConcurrentItems are now AUTHORED, not invented — +/// see and +/// SpewBoxController's own doc comments, and register row AP-178. +/// Absolute screen position (the element's parent, hence its true screen +/// offset, is still unidentified) and colour (no colour property surfaced +/// in this element's direct-state dump; the per-UIStateId +/// States dictionary was not walked) remain open. /// /// public sealed class SpewBoxLayoutDumpDiagnostic @@ -118,7 +136,7 @@ public sealed class SpewBoxLayoutDumpDiagnostic _out.WriteLine( $" ListBox child 0x{listBox.ElementId:X8} " + $"pos=({listBox.X},{listBox.Y}) size=({listBox.Width}x{listBox.Height})"); - DumpProperties(listBox, " "); + DumpProperties(listBox, " ", _out.WriteLine); } else { @@ -139,7 +157,7 @@ public sealed class SpewBoxLayoutDumpDiagnostic _out.WriteLine( $" Line template 0x{LineTemplateElementId:X8}: type=0x{lineTemplate.Type:X8} " + $"pos=({lineTemplate.X},{lineTemplate.Y}) size=({lineTemplate.Width}x{lineTemplate.Height})"); - DumpProperties(lineTemplate, " "); + DumpProperties(lineTemplate, " ", _out.WriteLine); } else { @@ -151,6 +169,95 @@ public sealed class SpewBoxLayoutDumpDiagnostic _out.WriteLine($"LayoutDesc 0x{LineTemplateLayoutEnum:X8} does not exist in the installed DAT."); } + // CH2 REJECT-review rework, NIT 3 + // (docs/research/2026-08-09-ch2-review-findings.md): the original + // sweep above never consulted dats.Local (client_local_English.dat + // — research doc §8.1 names this file, not client_portal.dat, as + // the one that might carry locale-specific UI text/layout + // resources). Repeat the exact same class-0x10000016 sweep against + // Local's own LayoutDesc id space before trusting AP-178's + // "not dat-authored, no LayoutDesc dump can recover this" wording. + var localHits = new List<(uint LayoutId, ElementDesc Element)>(); + int localScanned = 0; + List localIds = dats.Local.GetAllIdsOfType().ToList(); + _out.WriteLine($"Local.GetAllIdsOfType count: {localIds.Count}"); + if (localIds.Count > 0) + _out.WriteLine($"Local min id: 0x{localIds.Min():X8} Local max id: 0x{localIds.Max():X8}"); + + foreach (uint layoutId in localIds) + { + localScanned++; + if (!dats.Local.TryGet(layoutId, out LayoutDesc? ld) || ld is null) + continue; + + foreach (var kv in ld.Elements) + { + var found = FindByType(kv.Value, SpewBoxElementClass); + if (found is not null) + localHits.Add((layoutId, found)); + } + } + + _out.WriteLine($"Scanned {localScanned} dats.Local LayoutDescs."); + _out.WriteLine( + $"dats.Local elements of class 0x{SpewBoxElementClass:X8} (gmSpewBoxUI): {localHits.Count}"); + + foreach (var (layoutId, element) in localHits) + { + _out.WriteLine( + $" [Local] LayoutDesc 0x{layoutId:X8} -> element 0x{element.ElementId:X8} " + + $"pos=({element.X},{element.Y}) size=({element.Width}x{element.Height}) " + + $"zLevel={element.ZLevel} readOrder={element.ReadOrder} " + + $"leftEdge={element.LeftEdge} topEdge={element.TopEdge} " + + $"rightEdge={element.RightEdge} bottomEdge={element.BottomEdge} " + + $"baseElement=0x{element.BaseElement:X8} baseLayoutId=0x{element.BaseLayoutId:X8} " + + $"children={element.Children.Count}"); + DumpProperties(element, " ", _out.WriteLine); + foreach (var (childId, child) in element.Children) + { + _out.WriteLine( + $" child 0x{childId:X8}: type=0x{child.Type:X8} " + + $"pos=({child.X},{child.Y}) size=({child.Width}x{child.Height})"); + DumpProperties(child, " ", _out.WriteLine); + } + + // NIT 3 finding: gmSpewBoxUI::PostInit's GetChildRecursive(0x10000049) + // searches by ELEMENT ID, not by the widget CLASS (Type) — + // ListBoxElementClass below was the wrong axis to search on + // (it collided with the decomp's 0x10000049 constant, which is + // actually this instance's authored ElementId; the ListBox + // widget's own Type turned out to be 0x00000005). Look the + // child up directly by the ElementId the decomp names. + if (element.Children.TryGetValue(ListBoxElementClass, out ElementDesc? listBoxById)) + { + _out.WriteLine( + $" [Local] ListBox-by-ElementId 0x{listBoxById.ElementId:X8} " + + $"(Type=0x{listBoxById.Type:X8}) " + + $"pos=({listBoxById.X},{listBoxById.Y}) size=({listBoxById.Width}x{listBoxById.Height})"); + } + + var listBox = FindByType(element, ListBoxElementClass); + if (listBox is not null) + { + _out.WriteLine( + $" [Local] ListBox-by-Type child 0x{listBox.ElementId:X8} " + + $"pos=({listBox.X},{listBox.Y}) size=({listBox.Width}x{listBox.Height})"); + DumpProperties(listBox, " ", _out.WriteLine); + } + } + + if (dats.Local.TryGet(LineTemplateLayoutEnum, out LayoutDesc? localTemplateLd) + && localTemplateLd is not null) + { + _out.WriteLine( + $"[Local] LayoutDesc 0x{LineTemplateLayoutEnum:X8} exists " + + $"({localTemplateLd.Elements.Count} top-level elements)."); + } + else + { + _out.WriteLine($"[Local] LayoutDesc 0x{LineTemplateLayoutEnum:X8} does not exist."); + } + // Informational only — this is a discovery sweep, not a pass/fail gate. // The findings are transcribed into WeenieErrorMessages/SpewBoxController // doc comments and the divergence register by hand after reading this @@ -170,17 +277,24 @@ public sealed class SpewBoxLayoutDumpDiagnostic return null; } - private static void DumpProperties(ElementDesc d, string indent) + private static void DumpProperties(ElementDesc d, string indent, Action write) { + // NIT 3 fix (docs/research/2026-08-09-ch2-review-findings.md): this + // used to write to Console.WriteLine unconditionally, which xUnit's + // "Standard Output Messages" capture does NOT show — a latent bug + // that only mattered once a hit with an actual property to dump + // existed (the original Portal-only sweep found none). Routed + // through the caller's ITestOutputHelper.WriteLine so a future run + // actually surfaces this. if (d.StateDesc?.Properties is null) { - System.Console.WriteLine($"{indent}(no direct-state properties)"); + write($"{indent}(no direct-state properties)"); return; } foreach (var (propertyId, property) in d.StateDesc.Properties) { - System.Console.WriteLine($"{indent}property 0x{propertyId:X8} = {Describe(property)}" + write($"{indent}property 0x{propertyId:X8} = {Describe(property)}" + (propertyId == ListBoxMaxItemsProperty ? " <-- MaxConcurrentItems" : "")); } } @@ -190,6 +304,12 @@ public sealed class SpewBoxLayoutDumpDiagnostic DatReaderWriter.Types.EnumBaseProperty e => $"Enum({e.Value})", DatReaderWriter.Types.DataIdBaseProperty did => $"DataId(0x{did.Value:X8})", DatReaderWriter.Types.ArrayBaseProperty arr => $"Array[{arr.Value.Count}]({string.Join(", ", arr.Value.Select(Describe))})", + // NIT 3 fix: the original switch had no case for these two — + // Integer is exactly the type MaxConcurrentItems (property + // 0x10000028) uses, so without this case the diagnostic could + // find the property but never print its actual authored value. + DatReaderWriter.Types.IntegerBaseProperty i => $"Integer({i.Value})", + DatReaderWriter.Types.BoolBaseProperty b => $"Bool({b.Value})", _ => property.ToString() ?? "?", }; } diff --git a/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs b/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs index ecbe753a..4df16ee6 100644 --- a/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs +++ b/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs @@ -704,11 +704,15 @@ public sealed class GameEventWiringTests // Phase I.5: 0x028A previously had a parser // (GameEvents.ParseWeenieError) but no dispatcher registration. The // server fires this for plain game-logic failures (e.g. "you can't - // pick that up"). Now wired → ChatLog.OnWeenieError. + // pick that up"). Now wired → WeenieErrorMessages.Resolve + + // ChatLog.OnSystemMessage (the legacy no-router fallback; + // REJECT-review rework SHOULD-FIX 3 deleted the dedicated + // ChatLog.OnWeenieError chokepoint-bypass — see + // docs/research/2026-08-09-ch2-review-findings.md). var (d, _, _, _, chat) = MakeAll(); byte[] payload = new byte[4]; - BinaryPrimitives.WriteUInt32LittleEndian(payload, 0x9C); // arbitrary error code + BinaryPrimitives.WriteUInt32LittleEndian(payload, 0x04Au); // Ack! You killed yourself! (Default) var env = GameEventEnvelope.TryParse(WrapEnvelope(GameEventType.WeenieError, payload)); d.Dispatch(env!.Value); @@ -716,8 +720,29 @@ public sealed class GameEventWiringTests Assert.Equal(1, chat.Count); var e = chat.Snapshot()[0]; Assert.Equal(ChatKind.System, e.Kind); - Assert.Equal(0x9Cu, e.ChannelId); - Assert.Contains("0x009C", e.Text); + Assert.Equal("Ack! You killed yourself!", e.Text); + Assert.Equal(0x00u, e.LogTextType); + } + + [Fact] + public void WireAll_WeenieError_UnmappedCode_DoesNotReachChat() + { + // REJECT-review rework (SHOULD-FIX 4, + // docs/research/2026-08-09-ch2-review-findings.md): retail's + // HandleFailureEvent switch has no default case — an id it does not + // recognize produces NO text at all, silently, toward the player. + // The prior "WeenieError 0xNNNN" hex-fallback framing this test + // used to pin was acdream's own invention with no retail + // counterpart — an unregistered divergence, now retired. + var (d, _, _, _, chat) = MakeAll(); + + byte[] payload = new byte[4]; + BinaryPrimitives.WriteUInt32LittleEndian(payload, 0x9C); // arbitrary unmapped error code + + var env = GameEventEnvelope.TryParse(WrapEnvelope(GameEventType.WeenieError, payload)); + d.Dispatch(env!.Value); + + Assert.Equal(0, chat.Count); } [Theory] @@ -739,8 +764,29 @@ public sealed class GameEventWiringTests public void WireAll_WeenieErrorWithString_RoutesToChatLogWithInterpolation() { // Phase I.5: 0x028B carries an interpolated substring (e.g. the - // target's name in "you can't pick up the {Mana Stone}"). Now - // wired → ChatLog.OnWeenieError with the param. + // target's name). Now wired → WeenieErrorMessages.Resolve + + // ChatLog.OnSystemMessage (the legacy no-router fallback). + var (d, _, _, _, chat) = MakeAll(); + + byte[] interpBytes = MakeString16L("Caith"); + byte[] payload = new byte[4 + interpBytes.Length]; + BinaryPrimitives.WriteUInt32LittleEndian(payload, 0x0521u); // "%s has been added to the list of people you can hear." + Array.Copy(interpBytes, 0, payload, 4, interpBytes.Length); + + var env = GameEventEnvelope.TryParse(WrapEnvelope(GameEventType.WeenieErrorWithString, payload)); + d.Dispatch(env!.Value); + + Assert.Equal(1, chat.Count); + var e = chat.Snapshot()[0]; + Assert.Equal(ChatKind.System, e.Kind); + Assert.Equal("Caith has been added to the list of people you can hear.", e.Text); + } + + [Fact] + public void WireAll_WeenieErrorWithString_UnmappedCode_DoesNotReachChat() + { + // REJECT-review rework (SHOULD-FIX 4): same retail-faithful silence + // as the plain WeenieError case above. var (d, _, _, _, chat) = MakeAll(); byte[] interpBytes = MakeString16L("Mana Stone"); @@ -751,11 +797,7 @@ public sealed class GameEventWiringTests var env = GameEventEnvelope.TryParse(WrapEnvelope(GameEventType.WeenieErrorWithString, payload)); d.Dispatch(env!.Value); - Assert.Equal(1, chat.Count); - var e = chat.Snapshot()[0]; - Assert.Equal(ChatKind.System, e.Kind); - Assert.Equal(0x42u, e.ChannelId); - Assert.Contains("Mana Stone", e.Text); + Assert.Equal(0, chat.Count); } // ── Campaign CH slice CH2: onInterfaceText routing ─────────────────── diff --git a/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs b/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs index 485c36a0..d2281292 100644 --- a/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs +++ b/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs @@ -131,38 +131,14 @@ public sealed class ChatLogTests Assert.Equal(0x90ABCDEFu, e.ChannelId); // killer guid stashed here } - [Fact] - public void OnWeenieError_PlainCode_AppendsSystemEntry() - { - var log = new ChatLog(); - log.OnWeenieError(errorId: 0x1234, param: null); - var e = log.Snapshot()[0]; - Assert.Equal(ChatKind.System, e.Kind); - Assert.Contains("0x1234", e.Text); - Assert.Equal(0x1234u, e.ChannelId); - } - - [Fact] - public void OnWeenieError_WithString_AppendsInterpolation() - { - var log = new ChatLog(); - log.OnWeenieError(errorId: 0x5678, param: "Mana Stone"); - var e = log.Snapshot()[0]; - Assert.Equal(ChatKind.System, e.Kind); - Assert.Contains("Mana Stone", e.Text); - } - - [Theory] - [InlineData(0x003Bu)] // ILeftTheWorld - [InlineData(0x003Cu)] // ITeleported - public void OnWeenieError_RetailSilentClientControlStatus_DoesNotAppend(uint code) - { - var log = new ChatLog(); - - log.OnWeenieError(code, param: null); - - Assert.Empty(log.Snapshot()); - } + // OnWeenieError-specific tests (plain code, interpolation, silent + // client-control statuses) were removed here — REJECT-review rework + // (SHOULD-FIX 3, docs/research/2026-08-09-ch2-review-findings.md) + // deletes ChatLog.OnWeenieError itself; every producer now resolves via + // WeenieErrorMessages and calls the AddText chokepoint / OnSystemMessage + // directly. Equivalent coverage lives in GameEventWiringTests.cs (the + // inbound wire path) and WeenieErrorMessagesTests.cs (the resolve + // table), including the silent-client-control-status behavior. [Fact] public void OnLocalSpeech_EmptySender_SubstitutesYou() @@ -256,14 +232,6 @@ public sealed class ChatLogTests Assert.Equal(0x00u, log.Snapshot()[0].LogTextType); } - [Fact] - public void OnWeenieError_LogTextType_IsDefault() - { - var log = new ChatLog(); - log.OnWeenieError(errorId: 0x1234, param: null); - Assert.Equal(0x00u, log.Snapshot()[0].LogTextType); - } - [Fact] public void OnPopup_LogTextType_IsDefault() { diff --git a/tests/AcDream.Core.Tests/Chat/SpewBoxStateTests.cs b/tests/AcDream.Core.Tests/Chat/SpewBoxStateTests.cs index 81951791..7d91ff71 100644 --- a/tests/AcDream.Core.Tests/Chat/SpewBoxStateTests.cs +++ b/tests/AcDream.Core.Tests/Chat/SpewBoxStateTests.cs @@ -37,23 +37,22 @@ public sealed class SpewBoxStateTests [Fact] public void Tick_InsertsNewestAtIndexZero() { - // Retail: InsertItem(item, 0) — with MaxConcurrentItems raised past - // the code default of 1, newer entries must lead the visible list. + // Retail: InsertItem(item, 0) — CH2 REJECT-review rework NIT 3 + // raised MaxConcurrentItems from the code default (1) to the + // AUTHORED LayoutDesc value (4, see SpewBoxState.MaxConcurrentItems's + // own doc comment), so two entries now comfortably coexist without + // triggering the overflow rule — this test can assert the ordering + // guarantee directly instead of relying on eviction as a side effect. var state = new SpewBoxState(); state.Enqueue("first"); state.Tick(0d); state.Enqueue("second"); state.Tick(0d); - // MaxConcurrentItems == 1 (retail code default) means "first" was - // already evicted by the overflow rule — assert directly on the - // ordering guarantee instead by forcing a raised cap via reflection - // is out of scope; the dedupe/overflow tests below cover that - // interaction precisely. Here we only need the single surviving - // entry to be "second" (the newest), proving insert-at-front beat - // whatever eviction order a stack (insert-at-back) would produce. - SpewBoxEntry entry = Assert.Single(state.Snapshot()); - Assert.Equal("second", entry.Text); + Assert.Equal(2, state.Count); + SpewBoxEntry[] snapshot = state.Snapshot(); + Assert.Equal("second", snapshot[0].Text); + Assert.Equal("first", snapshot[1].Text); } [Fact] @@ -78,33 +77,43 @@ public sealed class SpewBoxStateTests [Fact] public void Tick_DifferentText_DoesNotDedupe() { + // CH2 REJECT-review rework NIT 3: with the AUTHORED + // MaxConcurrentItems == 4, two distinct messages both fit without + // any eviction — dedupe (index-0-only) is the only thing that could + // collapse them, and it correctly does not apply to different text. var state = new SpewBoxState(); state.Enqueue("first message"); state.Tick(0d); state.Enqueue("second message"); state.Tick(0d); - // With MaxConcurrentItems == 1, "second message" evicts "first - // message" via overflow, not dedupe — either way only one survives, - // and it must be the newest. - SpewBoxEntry entry = Assert.Single(state.Snapshot()); - Assert.Equal("second message", entry.Text); + Assert.Equal(2, state.Count); + SpewBoxEntry[] snapshot = state.Snapshot(); + Assert.Equal("second message", snapshot[0].Text); + Assert.Equal("first message", snapshot[1].Text); } [Fact] public void Tick_Overflow_DropsOldest_RespectingMaxConcurrentItems() { + // CH2 REJECT-review rework NIT 3: MaxConcurrentItems is the + // AUTHORED LayoutDesc value (4), not retail's code default (1) — + // enqueue past the cap to actually exercise the overflow rule. var state = new SpewBoxState(); - Assert.Equal(1, SpewBoxState.MaxConcurrentItems); + Assert.Equal(4, SpewBoxState.MaxConcurrentItems); - state.Enqueue("oldest"); - state.Tick(0d); - state.Enqueue("newer"); - state.Tick(0d); + for (int i = 0; i < SpewBoxState.MaxConcurrentItems + 1; i++) + { + state.Enqueue($"line {i}"); + state.Tick(0d); + } Assert.Equal(SpewBoxState.MaxConcurrentItems, state.Count); - SpewBoxEntry entry = Assert.Single(state.Snapshot()); - Assert.Equal("newer", entry.Text); + SpewBoxEntry[] snapshot = state.Snapshot(); + // Newest at index 0; "line 0" (the oldest) dropped by overflow. + Assert.Equal("line 4", snapshot[0].Text); + Assert.Equal("line 1", snapshot[3].Text); + Assert.DoesNotContain(snapshot, e => e.Text == "line 0"); } [Fact] diff --git a/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs b/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs index dd47c07c..f24b98ca 100644 --- a/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs +++ b/tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs @@ -42,18 +42,22 @@ public sealed class WeenieErrorMessagesTests // ── known codes — informational, no parameter ──────────────────── [Fact] - public void Format_0x051D_FallsBackToHex_NoRetailCaseExists() + public void Format_0x051D_ReturnsNull_NoRetailCaseExists() { // Campaign CH slice CH2: the pre-CH2 "Turbine Chat is enabled." // text for 0x051D was an ACE-derived guess, never decomp-confirmed. // The full HandleFailureEvent port found NO case for 0x51D anywhere // in the switch (only 0x51C has one — case 0x51c: at raw line // 383115-383118 of acclient_2013_pseudo_c.txt) — retail's own - // client simply has no display text for this id. Falling back to - // the generic form is now the retail-faithful answer, not a gap. - Assert.Equal( - "WeenieError 0x051D", - WeenieErrorMessages.Format(0x051D, param: null)); + // client simply has no display text for this id. + // + // REJECT-review rework (SHOULD-FIX 4, + // docs/research/2026-08-09-ch2-review-findings.md): retail's switch + // has no default case — an unhandled id produces NO text, silently, + // toward the player. Format now returns null rather than inventing + // a "WeenieError 0xNNNN" hex fallback that has no retail + // counterpart. + Assert.Null(WeenieErrorMessages.Format(0x051D, param: null)); } // ── known codes — error-level ──────────────────────────────────── @@ -167,27 +171,31 @@ public sealed class WeenieErrorMessagesTests WeenieErrorMessages.Format(0x04EE, null)); } - // ── unknown codes — graceful fallback preserves debug info ─────── + // ── unknown codes — retail-faithful silence (SHOULD-FIX 4) ─────── + // + // docs/research/2026-08-09-ch2-review-findings.md SHOULD-FIX 4: retail's + // HandleFailureEvent switch has no default case — an id it does not + // recognize produces NO text at all, toward the player. The prior + // "WeenieError 0xNNNN[: param]" hex fallback was acdream's own + // invention with no retail counterpart. These three tests are flipped + // (not deleted) to pin the new null-means-silence contract. [Fact] - public void Format_UnknownCode_NoParam_FallsBackToHexForm() + public void Format_UnknownCode_NoParam_ReturnsNull() { - Assert.Equal("WeenieError 0xABCD", WeenieErrorMessages.Format(0xABCD, null)); + Assert.Null(WeenieErrorMessages.Format(0xABCD, null)); } [Fact] - public void Format_UnknownCode_WithParam_FallsBackToColonForm() + public void Format_UnknownCode_WithParam_ReturnsNull() { - Assert.Equal( - "WeenieError 0xDEAD: Mana Stone", - WeenieErrorMessages.Format(0xDEAD, "Mana Stone")); + Assert.Null(WeenieErrorMessages.Format(0xDEAD, "Mana Stone")); } [Fact] - public void Format_UnknownCode_EmptyParam_StaysAsHexOnly() + public void Format_UnknownCode_EmptyParam_ReturnsNull() { - // Empty string param shouldn't add a stray colon. - Assert.Equal("WeenieError 0xCAFE", WeenieErrorMessages.Format(0xCAFE, "")); + Assert.Null(WeenieErrorMessages.Format(0xCAFE, "")); } // ── parameterised templates with non-trivial params ────────────── @@ -201,17 +209,18 @@ public sealed class WeenieErrorMessagesTests } [Fact] - public void Format_0x004F_FallsBackToHex_NoRetailCaseExists() + public void Format_0x004F_ResolvesToRetailText() { - // Campaign CH slice CH2: the pre-CH2 "You fail to affect _ because - // they cannot be harmed!" text for 0x004F was an ACE-derived guess. - // Direct decomp verification (grepping every "case 0x4f:" in - // ClientCommunicationSystem::HandleFailureEvent's whole body) found - // NONE — only 0x4E, 0x50, 0x51, 0x52, 0x53, 0x54 have cases; 0x4F is - // skipped entirely, same as the many other gaps in that switch's - // sparse jump table. Retail has no display text for this id. + // REJECT-review rework (BLOCKER 2, + // docs/research/2026-08-09-ch2-review-findings.md): the prior + // "grep for a case label" transcription missed 0x04F because it + // dispatches via `else if (arg2 == 0x4f)`, not a switch case label. + // The binary sweep found its sprintf format string directly + // (VA 0x00571e23, in ClientCommunicationSystem::HandleFailureEvent's + // else-if chain). Retail preserves its own $s typo (only the first + // %s substitutes), same pattern as 0x4F4. Assert.Equal( - "WeenieError 0x004F: Drudge", + "You fail to affect Drudge because $s cannot be harmed!", WeenieErrorMessages.Format(0x004F, "Drudge")); } @@ -226,34 +235,87 @@ public sealed class WeenieErrorMessagesTests // ── Campaign CH slice CH2: the full HandleFailureEvent table port ──── /// - /// Pins the table's size: 338 rows (Appendix A's 339 minus the one - /// deliberately-excluded 0x4F8, see the class doc comment on - /// ). A change to this number without - /// a matching research/commit citation is a red flag, not a routine - /// edit. + /// Pins the table's size: 344 rows. REJECT-review rework (BLOCKER 2, + /// docs/research/2026-08-09-ch2-review-findings.md) added the 5 ids the + /// prior pass's case-label enumeration missed (dispatched via + /// else if chains, not switch cases) plus 0x4F8, which now + /// resolves for real instead of being deliberately excluded: 338 + 5 + 1 + /// = 344. A change to this number without a matching research/commit + /// citation is a red flag, not a routine edit. /// [Fact] - public void Resolve_FullTable_HasExactly338Rows() + public void Resolve_FullTable_HasExactly344Rows() { int count = 0; for (uint id = 0; id <= 0x600u; id++) { var (text, _) = WeenieErrorMessages.Resolve(id, null); - if (!text.StartsWith("WeenieError 0x", StringComparison.Ordinal)) + if (text is not null) count++; } - Assert.Equal(338, count); + Assert.Equal(344, count); } [Fact] - public void Resolve_0x4F8_IsDeliberatelyExcluded_FallsBackToHex() + public void Resolve_0x4F8_NowResolvesForReal() { - // See the class doc comment: 0x4F8's case body is a tangled - // multi-operator+ decompiler artifact that could not be resolved - // with confidence — excluded rather than guessed. + // REJECT-review rework (BLOCKER 2): the prior pass excluded 0x4F8 + // because its case body's 3-operator+ concatenation chain has + // confusing BN-generated self-referential operand names. The binary + // sweep dereferenced both literal data pointers directly + // (data_7d2ee8, data_7d2f80), sidestepping the naming confusion. + // Both %s placeholders substitute the SAME parameter (retail only + // has one arg3 to concatenate twice). var (text, type) = WeenieErrorMessages.Resolve(0x4F8, "Someone"); - Assert.Equal("WeenieError 0x04F8: Someone", text); - Assert.Equal(RetailLogTextType.Default, type); + Assert.Equal( + "Someone fails to affect you because you are not the same sort of player killer as Someone!", + text); + Assert.Equal(RetailLogTextType.Magic, type); + } + + // ── REJECT-review rework (BLOCKER 2): every corrected/added row ────── + // + // docs/research/2026-08-09-ch2-review-findings.md — pins the exact text + // for every id the binary sweep + ACE cross-check corrected or added + // this pass, so a future regression to the wrong (previously-landed) + // text fails loudly instead of silently. + + [Theory] + // 5 ids added — missed by the prior case-label enumeration because + // they dispatch via `else if (arg2 == N)`, not a switch case. + [InlineData(0x04Fu, "You fail to affect %s because $s cannot be harmed!", RetailLogTextType.Magic)] + [InlineData(0x3EEu, "The container is closed!", RetailLogTextType.ClientLocal)] + [InlineData(0x408u, "Your spell cannot be cast inside", RetailLogTextType.ClientLocal)] + [InlineData(0x48Au, "You must be a monarch to purchase this dwelling.", RetailLogTextType.Default)] + [InlineData(0x4E8u, "The %s cannot be used while on a hook and only the owner may open the hook.", RetailLogTextType.Default)] + // 16 ids corrected per the review's own flagged list. + [InlineData(0x051u, "You fail to affect %s because you are not a player killer!", RetailLogTextType.Magic)] + [InlineData(0x053u, "You fail to affect %s because you are not the same sort of player killer as %s!", RetailLogTextType.Magic)] + [InlineData(0x054u, "You fail to affect %s because you are acting across a house boundary!", RetailLogTextType.Magic)] + [InlineData(0x466u, "You must purchase Asheron's Call: Dark Majesty to interact with that portal.", RetailLogTextType.Magic)] + [InlineData(0x4A3u, "You must have linked with a portal in order to recall to it!", RetailLogTextType.Magic)] + [InlineData(0x4B5u, "You must specify a character to query.", RetailLogTextType.ClientLocal)] + [InlineData(0x4E0u, "You are currently wielding items which require a certain level of skill. Your attributes cannot be transferred while you are wielding these items. Please remove these items and try again.", RetailLogTextType.Default)] + [InlineData(0x4F7u, "%s fails to affect you because you are not a player killer!", RetailLogTextType.Magic)] + [InlineData(0x544u, "An unspecified error occurred while attempting to remove %s as an allegiance officer.", RetailLogTextType.Default)] + [InlineData(0x54Eu, "The hook does not contain a usable item. You cannot open the hook because you do not own the house to which it belongs.", RetailLogTextType.Default)] + [InlineData(0x552u, "You must purchase Asheron's Call -- Throne of Destiny to use this function.", RetailLogTextType.ClientLocal)] + [InlineData(0x553u, "You must purchase Asheron's Call -- Throne of Destiny to use this item.", RetailLogTextType.ClientLocal)] + [InlineData(0x554u, "You must purchase Asheron's Call -- Throne of Destiny to use this portal.", RetailLogTextType.ClientLocal)] + [InlineData(0x555u, "You must purchase Asheron's Call -- Throne of Destiny to access this quest.", RetailLogTextType.ClientLocal)] + [InlineData(0x57Fu, "Your allegiance chat privileges have been temporarily removed by %s. Until they are restored, you may not view or speak in the allegiance chat channel.", RetailLogTextType.Default)] + [InlineData(0x582u, "Your allegiance chat privileges have been restored by %s.", RetailLogTextType.Default)] + // 2 ids corrected that were NOT in the review's flagged list — found by + // an automated diff between the swept binary literals and the landed + // table (the review's own "sweep may find more" prediction). + [InlineData(0x4E9u, "The %s cannot be used while on a hook, use the '@house hooks on' command to make the hook openable.", RetailLogTextType.Default)] + [InlineData(0x518u, "This fellowship is locked; %s cannot be recruited into the fellowship.", RetailLogTextType.Default)] + public void Resolve_Blocker2CorrectedRows_MatchTheSweptBinaryLiteral( + uint id, string expectedTemplate, RetailLogTextType expectedType) + { + var (text, type) = WeenieErrorMessages.Resolve(id, param: null); + Assert.Equal(expectedTemplate, text); + Assert.Equal(expectedType, type); } // ── spot pins across all three retail routing destinations ────────── diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs index 96dad77c..273af9d0 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs @@ -207,27 +207,38 @@ public sealed class RuntimeCommunicationStateTests } [Fact] - public void AddText_TrimsTrailingWhitespace_LikeRetailAddTextToScroll() + public void AddText_TrimsBothEnds_LikeRetailAddTextToScroll() { + // CH2 REJECT-review rework (SHOULD-FIX 2, + // docs/research/2026-08-09-ch2-review-findings.md): retail's + // AddTextToScroll @0x00563C50 calls trim(&str, 1, 1, ws) — BOTH + // ends, not trailing-only. using var state = new RuntimeCommunicationState(); - state.AddText("Out of Range! ", RetailLogTextType.ClientLocal); + state.AddText(" Out of Range! ", RetailLogTextType.ClientLocal); state.SpewBox.Tick(0d); Assert.Equal("Out of Range!", state.SpewBox.Snapshot()[0].Text); } [Fact] - public void AddText_EmptyAfterTrim_IsDropped() + public void AddText_EmptyAfterTrim_StillBroadcasts_LikeRetail() { + // CH2 REJECT-review rework (SHOULD-FIX 2): retail's + // AddTextToScroll has no empty-string guard — the previous + // early-return was an unregistered acdream-only divergence, now + // retired. An all-whitespace message still reaches its destination + // as an empty string. using var state = new RuntimeCommunicationState(); state.AddText(" ", RetailLogTextType.ClientLocal); state.AddText(" ", RetailLogTextType.Default); state.SpewBox.Tick(0d); - Assert.Equal(0, state.SpewBox.Count); - Assert.Equal(0, state.Chat.Count); + Assert.Equal(1, state.SpewBox.Count); + Assert.Equal("", state.SpewBox.Snapshot()[0].Text); + Assert.Equal(1, state.Chat.Count); + Assert.Equal("", state.Chat.Snapshot()[0].Text); } [Fact] diff --git a/tests/AcDream.Runtime.Tests/RuntimeGenerationResetTests.cs b/tests/AcDream.Runtime.Tests/RuntimeGenerationResetTests.cs index 468f2100..e66ae29c 100644 --- a/tests/AcDream.Runtime.Tests/RuntimeGenerationResetTests.cs +++ b/tests/AcDream.Runtime.Tests/RuntimeGenerationResetTests.cs @@ -46,6 +46,7 @@ public sealed class RuntimeGenerationResetTests runtime.ActionOwner.Combat.SetCombatMode(CombatMode.Missile); runtime.CommunicationOwner.Chat.SetLocalPlayerGuid(player); runtime.CommunicationOwner.Chat.OnSystemMessage("retained", 1u); + runtime.CommunicationOwner.SpewBox.Enqueue("about to be torn down"); _ = runtime.MovementOwner.Execute( RuntimeMovementCommand.ToggleRunLock); var observer = new RecordingObserver(); @@ -81,6 +82,13 @@ public sealed class RuntimeGenerationResetTests Assert.Equal(CombatMode.NonCombat, runtime.Actions.Snapshot.CombatMode); Assert.False(runtime.MovementOwner.AutoRunActive); Assert.Equal(1, runtime.CommunicationOwner.Chat.Count); + // SHOULD-FIX 1 (docs/research/2026-08-09-ch2-review-findings.md): + // ResetSpewBox was dead code — a fresh generation must not + // resurrect a stale refusal line. Assert BOTH that the pending + // enqueue never surfaces (no leftover Tick drains it into + // visibility) and that Reset itself converges Count to zero. + runtime.CommunicationOwner.SpewBox.Tick(0d); + Assert.Equal(0, runtime.CommunicationOwner.SpewBox.Count); Assert.Null( runtime.CommunicationOwner.CommandTargets.LastIncomingTellSender); Assert.False(runtime.GenerationReset.CaptureSnapshot().IsActive); diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/SpewBox/SpewBoxVMTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/SpewBox/SpewBoxVMTests.cs index 216765bf..0490c69b 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/SpewBox/SpewBoxVMTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/SpewBox/SpewBoxVMTests.cs @@ -48,6 +48,10 @@ public sealed class SpewBoxVMTests [Fact] public void Lines_NewestFirst_MatchesRetailInsertAtZero() { + // CH2 REJECT-review rework NIT 3: MaxConcurrentItems is the + // AUTHORED LayoutDesc value (4, see SpewBoxState.MaxConcurrentItems's + // own doc comment), not retail's code default (1) — both entries + // now survive, so assert the ordering directly. var state = new SpewBoxState(); var vm = new SpewBoxVM(state); state.Enqueue("older"); @@ -56,11 +60,9 @@ public sealed class SpewBoxVMTests IReadOnlyList lines = vm.Lines(0d); - // MaxConcurrentItems == 1 means only the newest survives, which is - // itself proof insertion happens at the front (retail's overflow - // rule drops the OLDEST / highest index, not the newest). - SpewBoxLine line = Assert.Single(lines); - Assert.Equal("newer", line.Text); + Assert.Equal(2, lines.Count); + Assert.Equal("newer", lines[0].Text); + Assert.Equal("older", lines[1].Text); } [Fact] diff --git a/tools/pdb-extract/sweep_weenie_strings.py b/tools/pdb-extract/sweep_weenie_strings.py new file mode 100644 index 00000000..b75bb0a6 --- /dev/null +++ b/tools/pdb-extract/sweep_weenie_strings.py @@ -0,0 +1,184 @@ +"""Sweep `push imm32` (0x68) operands inside a VA range of a PE binary, +dereference each into a data section (.rdata/.data), and decode any that +resolve to a printable UTF-16LE literal. + +Built for the CH2 REJECT-review rework (BLOCKER 2, +docs/research/2026-08-09-ch2-review-findings.md) to re-derive +ClientCommunicationSystem::HandleFailureEvent (@0x00571990)'s 344-row +display-string table from ground truth instead of the Binary Ninja +pseudo-C's ~33-char inline preview -- the same class of problem +check_exe_pdb.py and dump_pdb_info.py solve for PDB metadata, applied to +wide-string literal recovery. Not tied to WeenieError specifically: any VA +range in any PDB-paired PE binary works. + +ALWAYS run check_exe_pdb.py first to confirm the candidate .exe pairs with +the PDB you're cross-referencing addresses against -- a mismatched binary +will produce confident-looking garbage. + +Usage: + py tools/pdb-extract/sweep_weenie_strings.py --range 0x571990 0x575480 [--min-len 4] + py tools/pdb-extract/sweep_weenie_strings.py --anchor 0x005750a5 [--window 64] + py tools/pdb-extract/sweep_weenie_strings.py --deref 0x007d2ee8 + +--range LO HI sweep every string-valued push imm32 in [LO, HI) +--anchor VA search backward `--window` bytes from a case-body/call-site + VA (taken from the pseudo-C) for the nearest string-valued + push -- use when you already know roughly where a specific + case lives and just need its untruncated literal +--deref VA dereference one known data pointer directly (e.g. a + `data_XXXXXXXX` symbol name from the pseudo-C, which + directly encodes its own VA in hex) +--min-len N minimum decoded string length to report (default 3); + raise this to cut noise from short accidental hits +""" +import argparse +import struct + + +class PeImage: + def __init__(self, path): + with open(path, "rb") as f: + self.data = f.read() + if self.data[0:2] != b"MZ": + raise ValueError("not a PE file (no MZ)") + e_lfanew = struct.unpack_from("= len(self.data): + return None + return off + + def read_bytes(self, va, n): + off = self.va_to_off(va) + if off is None: + return None + return self.data[off:off + n] + + def read_utf16_cstr(self, va, max_chars=400): + off = self.va_to_off(va) + if off is None: + return None + out = [] + for i in range(max_chars): + chunk = self.data[off + i * 2: off + i * 2 + 2] + if len(chunk) < 2: + break + code = struct.unpack(" 0x2FFF: + return None + out.append(chr(code)) + return None # ran off the end without a NUL -- not a bounded literal + + def is_data_section(self, va): + s = self.section_for_va(va) + return s is not None and s["name"] in (".rdata", ".data") + + def sweep_push_imm32(self, lo, hi, min_len=3): + """Scan [lo, hi) for `push imm32` (opcode 0x68) whose operand VA + dereferences to a UTF-16LE string in .rdata/.data. Returns a list + of (instr_va, target_va, text).""" + hits = [] + off_lo = self.va_to_off(lo) + off_hi = self.va_to_off(hi) + if off_lo is None or off_hi is None: + raise ValueError("range not mapped") + i = off_lo + while i < off_hi - 4: + if self.data[i] == 0x68: + operand = struct.unpack_from("= min_len: + instr_va = lo + (i - off_lo) + hits.append((instr_va, operand, text)) + i += 1 + return hits + + def find_push_before(self, anchor_va, window=64, min_len=3): + """Search backward from anchor_va (a call-site VA taken from the + pseudo-C) for the nearest preceding `push imm32` whose operand + dereferences to a printable UTF-16LE string.""" + lo = anchor_va - window + return self.sweep_push_imm32(lo, anchor_va + 2, min_len=min_len) + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("exe") + ap.add_argument("--range", nargs=2, metavar=("LO", "HI")) + ap.add_argument("--anchor", action="append", default=[]) + ap.add_argument("--window", type=int, default=64) + ap.add_argument("--deref", action="append", default=[]) + ap.add_argument("--min-len", type=int, default=3) + args = ap.parse_args() + + pe = PeImage(args.exe) + print(f"ImageBase=0x{pe.image_base:08x} sections:") + for s in pe.sections: + print(f" {s['name']:<9} VA=0x{s['va']:08x} vsize=0x{s['vsize']:06x} " + f"rawptr=0x{s['raw_ptr']:08x} rawsize=0x{s['raw_size']:06x}") + print() + + if args.range: + lo = int(args.range[0], 16) + hi = int(args.range[1], 16) + hits = pe.sweep_push_imm32(lo, hi, min_len=args.min_len) + print(f"# sweep 0x{lo:08x}-0x{hi:08x}: {len(hits)} string-valued push imm32 sites") + for instr_va, target_va, text in hits: + print(f"0x{instr_va:08x} -> data_0x{target_va:08x} {text!r}") + + for a in args.anchor: + anchor = int(a, 16) + hits = pe.find_push_before(anchor, window=args.window, min_len=args.min_len) + print(f"\n# anchor 0x{anchor:08x} (window={args.window}): {len(hits)} hits") + for instr_va, target_va, text in hits: + print(f"0x{instr_va:08x} -> data_0x{target_va:08x} {text!r}") + + for d in args.deref: + target = int(d, 16) + text = pe.read_utf16_cstr(target) + print(f"\n# deref 0x{target:08x}: {text!r}") + + +if __name__ == "__main__": + main()