Narrow re-review of the FA3 fix round (9afa05b5/35c40a9b/ae772709/ a5553904/c5f73744) against this doc's 2 MUST-FIX and 9 SHOULD-FIX. Every disposition re-derived from the diffs, not from the commit messages. All 11 correctly applied; nothing reopened; no new MUST-FIX or SHOULD-FIX. Dispositions worth naming: MF-1/MF-2 were fixed as script REWRITES that state the verified truth (Allegiance is both the default and the left-most tab; @allegiance info prints chat data AND the blocks stay hidden, report neither) rather than deletions, and MF-2's report-bullet is correctly inverted to "blocks becoming VISIBLE is the anomaly". SF-3's assertions pin more than asked (which page is visible, not just that one is). SF-7 landed as a binding FA5 acceptance line plus the forward hazard about Tick()'s unconditional LinesProvider reassignment, the right altitude for a shell slice. Claims verified rather than accepted: SF-1's no-register-row precedent (grepped -- no row exists for any Toggle*Panel close-on-second-press, so the precedent is real); the #383 timestamp correction (b4edee972026-08-11 09:19,e71e5a9606:25,74c3d85d2026-08-12 02:58 = ~17h39m and ~20h33m, previous day); OnShown/OnHidden really are driven by RetailWindowHandle.NotifyVisibility and correctly do NOT fire for a window mounted Visible=false; UiTemplateListBox.Scroll forces the extent-seeded viewport so pre-row scrollbar wiring is sound; and the new template cache is safe because Build is the pure builder -- only BuildFromInfos (tests-only) mutates the ElementInfo it is handed. Gates re-run on the post-fix Release binaries: live-mount probe passes against the installed DATs with the promoted assertions showing Allegiance Visible=True (other three False) and 0x10000492 count = 2; AcDream.App.Tests 4,876 passed / 3 skipped / 0 failed, exactly +5 over the pre-fix 4,871 and exactly the ledger's App figure, so the 13,238/4/0 +5 reconciles at the only project this round touched. Blast radius is 14 files, all FA3's own plus doc-only edits to UiTemplateListBox and MountSocialPanel. Carry-forwards (non-blocking): the Flush scroll-position reset will bite harder in FA4's per-vitals-tick roster rebuild; the production template cache has no test (both long-roster tests use the fake resolver); the scrollbar element ids are literals where ScrollbarElementId carries the authored value (cohort-wide nit); and the allRowsResolved retry rebuilds per frame on a permanently unresolvable template (bounded and cheap). FA3's remaining obligation is unchanged: the user's connected gate, against a script that no longer contains two instructions guaranteed to produce false defect reports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
40 KiB
Campaign FA slice FA3 — MECHANISM-FAITHFULNESS review
Target: 0a9ca2f1 (feature), 74c3d85d (fixture + tests), b6a25110
(gate script + ledger), d7e1cffd (research addendum), reviewed against
docs/plans/2026-08-11-fellowship-allegiance-campaign.md slice FA3 + D1 + §6
and docs/research/2026-08-11-fa-panel-structure.md (incl. §10 + the FA3
correction addendum).
Verdict: APPROVE-WITH-FIXES — 2 MUST-FIX (both in the user's connected-gate script, which is the artifact the user actually reads while gating), 9 SHOULD-FIX. The load-bearing tab-table correction is CONFIRMED CORRECT by re-derivation from the authored bytes AND independently from the installed DATs. No code defect found in the mount path.
Method note: every claim below was re-derived, not taken from the commit
messages. The fixture was parsed directly; the env-gated live-mount probe was
run against the installed DATs; the retail anchors were re-read in
acclient_2013_pseudo_c.txt; and one BN string operand was byte-verified in
the PDB-paired binary (check_exe_pdb.py → === MATCH ===, GUID
9e847e2f-777c-4bd9-886c-22256bb87f32).
1. The tab-table correction — VERIFIED CORRECT (item 1)
This was the load-bearing fact. It holds, three independent ways.
(a) The authored 0x2E array, read straight out of the committed fixture
(tests/AcDream.App.Tests/UI/Layout/fixtures/social_panel_2100006E_1000018F.json,
root 0x1000018F, property 46 = 0x2E, four Kind 8 struct entries with
sub-properties 48=0x30 button, 49=0x31 page, 50=0x32 IsDefault):
| entry | P0x30 (button) | P0x31 (page) | P0x32 |
|---|---|---|---|
| 0 | 268436108 = 0x1000028C |
268436113 = 0x10000291 |
true |
| 1 | 268436110 = 0x1000028E |
268436114 = 0x10000292 |
absent |
| 2 | 268436754 = 0x10000512 |
268436755 = 0x10000513 |
absent |
| 3 | 268436795 = 0x1000053B |
268436810 = 0x1000054A |
absent |
This is exactly the pairing recorded in SocialPanelController.cs:34-37,
and Allegiance (0x1000028C→0x10000291) is the sole IsDefault entry.
(b) Each button's own resolved caption — from the live-mount probe run against the installed DATs during this review:
[socialprobe] tab button 0x1000028C (UiText) caption='Allegiance'
[socialprobe] tab button 0x1000028E (UiText) caption='Fellowship'
[socialprobe] tab button 0x10000512 (UiText) caption='Friends'
[socialprobe] tab button 0x1000053B (UiText) caption='Squelch'
(c) Each page's own P0x57 + its RegisterElementClass type, both from the
fixture and re-confirmed by the probe:
| page | authored Type |
retail class (verified in pseudo-C) | P0x57 |
|---|---|---|---|
0x10000291 |
0x1000002C |
RegisterElementClass(0x1000002c, gmAllegianceUI::Create) @004911fa |
0x1000000E = ToggleAllegiancePanel (F3) |
0x10000292 |
0x1000002D |
RegisterElementClass(0x1000002d, gmFellowshipUI::Create) @0048e6aa |
0x1000000F = ToggleFellowshipPanel (F4) |
0x10000513 |
0x10000045 |
RegisterElementClass(0x10000045, gmFriendsUI::Create) @0048e21a |
0x10000118 |
0x1000054A |
0x10000047 |
RegisterElementClass(0x10000047, gmSquelchUI::Create) @0048d04a |
0x10000124 |
The class doc's further claim that 0x10000118/0x10000124 carry no
retail-default keybind is also correct: neither literal appears in
keymap-default.txt or retail-default.keymap.txt.
A fourth corroboration the addendum did not state, and should have. With
the corrected pairing, the authored x-order becomes Allegiance (x=0),
Fellowship (x=72), Friends (x=144), Squelch (x=206) — i.e. the authored
default tab is also the left-most tab, which is what one would expect and
which the old (refuted) guess did not produce. Button widths track caption
lengths under the corrected mapping too (Friends, the shortest caption, is
the narrowest button at 62px). This same x-order is the basis of MUST-FIX 1
below.
Also verified: root P0x10000029 = 12 (the RetailPanelCatalog.SocialPanel
byte-verification claim), root P0x57 = 0x1000000D, close button 0x10000290
P0x12 = 0x1000000D, and 0x1000000D really is an InputMap context id
(ID_InputMap_ToggleChatEntry, src/AcDream.Core/Input/RetailActionMap.cs:161;
it appears as Context 0x1000000D in keymap-default.txt) — so the class
doc's "authored but UNCONSUMED" reading is accurate, not a hand-wave.
MUST-FIX
MF-1 — the gate script carries the REFUTED x-order into the user's contract
docs/research/2026-08-12-campaign-fa-test-script.md:59-64 (step 9):
"left-to-right authored order is Friends, Allegiance, Fellowship, Squelch"
and :20-25 (step 1):
"the authored default tab — NOT Friends, despite Friends being drawn left-most on the tab strip in some sort orders"
Both are wrong. The authored geometry (fixture, corroborated by the probe's
root-child dump) is:
0x1000028C "Allegiance" x=0 72x25
0x1000028E "Fellowship" x=72 72x25
0x10000512 "Friends" x=144 62x25
0x1000053B "Squelch" x=206 70x25
0x10000290 (close X) x=276 24x25
Friends is drawn third, not left-most. This is the same x-order guess
d7e1cffd corrects in the research doc — it just never got corrected in the
script. Step 10's caption list (:65-68) repeats the wrong ordering.
Why this is MUST-FIX and not a nit: the script is the user's contract for the connected gate, and step 1 explicitly primes the user to expect Friends at the left edge next to a tab that opens by default. The user will see Allegiance at x=0 and has been told that is the wrong-looking case. Fix step 9, step 1's parenthetical, and step 10 to the real order — and take the opportunity to say "the default tab is also the left-most tab", which is the most reassuring form of the correction.
MF-2 — the gate script's step 14 sends the user to a trigger that CANNOT fire, and tells them to report it if it doesn't
docs/research/2026-08-12-campaign-fa-test-script.md:100-107 (step 14):
"If some other client action causes an allegiance push to land server-side during this session (e.g. a
@allegiance infochat command, which already works independently of this panel), the monarch/patron blocks should then become VISIBLE — report if they do not."
and the matching "What to report" bullet at :153-155:
"The Allegiance page's monarch/patron blocks staying hidden after a real allegiance push has visibly landed (e.g. after
@allegiance infoshows non-empty data in chat)."
RuntimeAllegianceState._hasProfile is set in exactly one place —
ApplyUpdate (src/AcDream.Runtime/Gameplay/RuntimeAllegianceState.cs:126-142),
whose own doc comment reads "0x0020 AllegianceUpdate — … the ONLY
inbound writer of this owner's profile (see the class doc's MUST-FIX 2
correction)". FA2's own fix round deliberately stopped 0x027C (the
@allegiance info response family) from seeding this owner
(4272ad0e, "0x027C stops seeding"; GameEventWiring.cs:231 documents it),
and the plan's FA5 row states the 0x001F subscription that provokes 0x0020
is FA5 scope. FA3 sends no 0x001F.
So the script names a trigger the campaign deliberately disconnected, and then
instructs the user to file a defect when it fails to work. Either drop the
trigger entirely and state plainly that the blocks are expected to stay hidden
for the whole of FA3 (the honest shell-slice expectation), or replace it with
the only real path (a server-side allegiance change that makes ACE push
0x0020 unsolicited) and say so. The "What to report" bullet must go with it.
SHOULD-FIX
SF-1 — the F3/F4 "retail Toggle-action semantics" claim has no retail anchor (item 2)
src/AcDream.App/UI/Layout/SocialPanelController.cs:200-204:
"lets
RetailUiRuntime.HandleInputActionimplement retail's Toggle-action close-on-second-press semantics (same shape asOpenSpellbook's own page-aware toggle)"
I went looking for the retail handler and could not find one. Re-derived:
- No
P0x57consumer exists in the binary. The only two0x57reads anywhere inacclient_2013_pseudo_c.txtareAppraisalProfile::InqInt(arg2, 0x57, …)@004b2954andAppraisalProfile::InqFloat@004b3981— unrelated. NoGetAttribute_Enum(this, 0x57, …)site exists. This matches lane A §6.1's UNVERIFIED flag; the FA3 class doc correctly restates it for the root'sP0x57, then quietly relies on a different retail claim for the toggle. ClientUISystem::OnAction @0x00564B90does not handle these actions. Form_InputAction > 0x7cit handles exactly0x1000001E,0x10000025and0x1000002Band returns0for everything else —0x1000000E/0x1000000Ffall through.gmPanelUInever sees the action.gmPanelUI::PostInit @0x004BD010doesRegisterForGlobalMessage(this, 1), butgmPanelUI::ListenToGlobalMessageis COMDAT-folded ontoNoticeHandler::RecvNotice_DisplayWeenieError @0x004F5860, whose body isreturn;.- No other
OnActionimplementation's range covers0x1000000E/0x1000000F(ChatInterface0x10000020..0x10000119, magic0x10000060..0x10000105, combat0x1000005B..,CPlayerSystem0x1000002A..).
So retail's actual F3/F4 behaviour — and in particular what a repeat press does
when the panel is already open on the other tab — is not established from
this decomp. The shipped rule ("close only when already on the target tab")
is acdream's own OpenSpellbook precedent, which is a perfectly defensible
choice; it just must not be labelled as retail's. Re-word the comment to name
it as precedent-following, and add the unknown to the research doc's §8 list
(next to U7, which is the same gap) so FA7's closeout can decide whether it
wants a register row. The gate script's "same toggle-closes-on-repeat-press
shape as every other Toggle*Panel action" (:26-29) is fine as-is — it
claims acdream consistency, not retail fidelity.
SF-2 — per-frame closure allocation in the allegiance page tick
src/AcDream.App/UI/Layout/SocialAllegiancePageController.cs:108-110:
IReadOnlyList<UiText.Line> lines = hasProfile ? NoLines : BlankLine;
if (_monarchName is not null) _monarchName.LinesProvider = () => lines;
if (_patronName is not null) _patronName.LinesProvider = () => lines;
lines is captured, so this allocates a display class plus a delegate,
twice, on every frame, whether or not the panel is visible
(RetailUiRuntime.cs:559 ticks it unconditionally). At the profile's measured
~520 FPS that is ~2,000 allocations/second of pure garbage for a value that
changes at most once per session. The Modern Runtime slices spent whole commits
driving per-frame paths to 0 B; this is a new, avoidable regression of that
discipline in the graphical hot loop.
Fix: hoist two static readonly Func<IReadOnlyList<UiText.Line>> fields (one
returning BlankLine, one returning NoLines) and assign the cached delegate,
and/or only write LinesProvider when the gate actually flips. The fellowship
page's tick is already allocation-free — this is the only offender.
SF-3 — the live-mount probe prints two of its headline findings without asserting them (item 7)
tests/AcDream.App.Tests/UI/Layout/SocialPanelLiveMountProbeTests.cs:117-118
computes passupCount and prints it, but never asserts it. The commit message
nevertheless claims "0x10000492 is confirmed authored twice under the
allegiance page". A print is not a gate: this cannot fail if a future importer
change collapses or drops one of the two instances — which is exactly the
regression the scoped-lookup rule exists to survive. (For the record I did
confirm the fact independently: the fixture authors 0x10000492 twice, once
under 0x10000255/0x10000490 and once under 0x1000025A/0x10000490, and the
probe printed = 2.) Add Assert.Equal(2, passupCount).
Likewise, the probe activates the tab behavior (:67) and asserts
UnresolvedEntries is empty, but never asserts page exclusivity — that
after ActivateTabBehavior() exactly one of the four page elements is
Visible, and that it is the Allegiance page. That is the #372 class
("the panel mounts but the pages are wrong/blank") stated as an assertion
rather than a hope, and it is one line.
Everything else the probe asserts is real and does cover the classes it claims: non-empty tab captions (the #375 resolver class — I re-ran it and it passes), the four pages resolving, the fellowship frame pair, and the six allegiance signature elements. All pass against the installed DATs.
SF-4 — "all three visible option checkboxes" is four
src/AcDream.App/UI/Layout/SocialFellowshipPageController.cs:19. Frame
0x1000026B authors four Type 0x10000035 children —
0x10000270 (IgnoreFellowshipRequests), 0x10000271
(FellowshipAutoAcceptRequests), 0x10000272 (FellowshipShareXP),
0x10000273 (FellowshipShareLoot) — per lane A §3.1 and confirmed in the
fixture. The gate script step 11 (:73-82) hedges with "a fourth, Share Loot,
may also be present" when both the decomp and the committed fixture already
settle it. Fix the count in both places; a gate script that is unsure about
what the user will see invites noise.
SF-5 — AD-79's pinning test does not cover the control AD-79 itself enumerates
The register row names seven controls, including the Friends page's "Appear
Offline"-shaped checkbox 0x1000052C, and cites
SocialPanelControllerTests.FriendsAndSquelchActionButtons_AreClickable_ButHaveNoHandler
as what pins the INERT contract. That test
(SocialPanelControllerTests.cs:272-291) covers only the six buttons.
0x1000052C builds through DatWidgetFactory.BuildCheckbox as a UiButton
(DatWidgetFactory.cs:785-816), so it can be added to the existing loop
verbatim. Seven enumerated, six pinned is the kind of drift AD-78's own
"Risk" column warns about.
SF-6 — the two page controllers do not cite AD-79 by id
SocialFriendsPageController.cs:23-25 and SocialSquelchPageController.cs:21-22
both say "See this commit's single register row…". AD-79's own Where column
asserts "both classes' own doc comments cite this row". Literally true,
practically not: a future reader grepping AD-79 will not find the code. Name
the row.
SF-7 — the allegiance empty state is coarser than the contract's own source (item 4)
FA3's contract line is "all four pages' empty states", and lane A §4.5 is what defines the allegiance one. I re-read and byte-verified that section's claims because it is what the slice implements against:
gmAllegianceUI::UpdateMonarchData @0x00491B40branches on(monarchId == player_id || monarchId == 0)— retail hides the monarch block when there is no monarch or the monarch is you, then sets bothm_pMonarchNameandm_pMonarchFollowersto the literal atdata_794358.data_794358byte-verified in the PDB-paired binary (C:\Users\erikn\Downloads\acclient.exe, image base0x400000, RVA0x394358→ file offset0x394358): bytes20 00 00 00= UTF-16LE" ", one character. So lane A'sL" "reading is correct and FA3'sBlankLine = [new UiText.Line(" ", …)](SocialAllegiancePageController.cs:40-41) matches it exactly — a single space, not an empty string, as asserted atSocialPanelControllerTests.cs:177-178. This half is clean.
What is coarser: FA3 gates both blocks on Snapshot.HasProfile rather than
per-relationship. The consequence is not hypothetical — the moment any
0x0020 AllegianceUpdate lands, a monarch character sees an empty, visible
monarch block that retail hides, and anyone whose patron is the monarch sees an
empty patron block instead of retail's relabelled-monarch presentation. FA2
already ships the data needed to do it properly:
IRuntimeAllegianceView.TryGetMonarch / TryGetPatron
(GameRuntimeGameplayViews.cs:200-204) plus Snapshot.MonarchGuid; the FA3
Callbacks record deliberately narrows the input to
Func<RuntimeAllegianceSnapshot> (SocialPanelController.cs:90-96), which is
what forecloses it.
I am not calling this a MUST-FIX: FA3 is a shell slice, the class doc is
honest about the simplification, and MF-2's finding means HasProfile is
effectively always false for the whole FA3 gate, so nothing wrong is visible
during this gate. But it is a contract gap that will be invisible by the time
FA5 lands unless it is written down. Either widen the callback to the view and
implement the real per-relationship gate now, or add an explicit FA5 acceptance
line ("the monarch block hides when the monarch is you; the patron block hides
when the patron is the monarch") to the plan's FA5 row so it cannot be lost.
Related, and worth a line in the class doc: Tick() reassigns
LinesProvider unconditionally every frame, so any FA5 code that sets a
real monarch/patron name on 0x10000257/0x1000025C will be overwritten on
the next frame unless this method is changed at the same time.
SF-8 — Tick() ignores _disposed
SocialPanelController.Dispose() (:224-228) sets _disposed and nothing
reads it. The handle passes Controller = controller to
RetailWindowFrame.Mount, and RetailUiRuntime.SocialPanelController is never
nulled, so RetailUiRuntime.Tick's SocialPanelController?.Tick()
(RetailUiRuntime.cs:559) will keep ticking the four page controllers after
disposal. Nothing throws today (the Runtime owners outlive the panel and the
J-owner snapshot getters are disposed-guarded), so this is low severity — but
"tick a disposed controller" is precisely the class the J-slices spent commits
eliminating. One if (_disposed) return; closes it.
SF-9 — SocialPanelRowText.FindDeepest documents a stronger guarantee than it implements
SocialPanelRowText.cs:19-31 promises "the deepest UiText descendant
(pre-order, last match wins)". The implementation returns the last match in
traversal order, which equals the deepest only when the subtree is a single
chain. Both real templates are single chains — I dumped them from the
installed DATs:
FriendsRow 0x10000519 Type 3 (270x24) -> 0x1000051A Type 0xC -> 0x1000051F Type 0xC
SquelchRow 0x10000541 Type 3 (270x24) -> 0x10000542 Type 0xC -> 0x1000054D Type 0xC
so the behaviour is correct today and the class doc's honesty about
gmFriendsUI/gmSquelchUI being outside the campaign's decompiled scope is
exactly the right posture. Just make the doc describe what the code does, or
make the code track depth.
2. What verified clean
Stated plainly, because most of this slice is right.
The tab-table correction (item 1). Confirmed three independent ways
(§1 above). The addendum in d7e1cffd is accurate, and correcting in place
while leaving the original text visible follows the FA1/FA2 convention.
§6 Campaign-OP lessons compliance (item 3). Each one re-derived:
- String resolver on every
Build.MountSocialPanelpassesresolver.Resolveto the mainLayoutImporter.Build(RetailUiRuntime.cs:2611-2617) and its nestedTemplateResolverbuilds each row template with its ownstrings.Resolve(:2626-2645). Both covered — the #375 class. The probe's non-empty-caption assertion is the live check, and it passes. - Scoped
FindDescendantfor every repeating id. I enumerated all duplicate element ids in the panel subtree rather than trusting the stated one. Duplicates exist at0x10000439/0x100000E9/0x10000215(×4, tab button art),0x100002CE–0x100002D0(×16, button 3-slice art),0x10000328(×6, checkbox art),0x00000001/0x10000071/0x10000072/0x10000364–0x10000366(×4, scrollbar parts),0x10000490/0x10000491/0x10000492(×2, the two allegiance blocks), and — not previously called out anywhere —0x10000254,0x1000025Dand0x1000051Erepeat ACROSS pages (0x10000254under both the Friends page and the allegiance monarch block;0x1000025Dunder the Friends page and the patron block;0x1000051Eunder both Friends and Squelch). Every FA3 lookup is safe: the four page ids, the two fellowship frames, both allegiance field containers, both name texts and both ListBox ids are each unique, the per-page controllers scope to their page root anyway, and the single flatlayout.FindElement(0x10000290)for the close button targets a unique id.0x10000492is not looked up at all in FA3, so the intra-panel duplication cannot bite yet — but the three cross-page duplicates above are worth carrying into FA5's notes, since FA5 will be resolving allegiance-page ids. - Tab activation performed.
controller.ActivateTabs()atRetailUiRuntime.cs:2662, afterBind— the OP3 ordering constraint. Verified live:UnresolvedEntriesis empty andIsShowingAllegianceis true after activation. - No lazily-created fill-anchored 0×0 children (the #372 class). The new
controllers create no elements at all. The only lazy child is
UiTemplateListBox's viewport, which already seedsWidth/Heightfrom the ListBox's authored extent (UiTemplateListBox.cs:152-182, the #372 fix), and both social ListBoxes carry real authored extents (Friends270x400, Squelch270x430) atBindtime. - Same-layout template-prototype skip. Checked for this layout specifically:
all four
P0x64arrays point at other LayoutDescs — Friends0x2100005D/0x10000519, Squelch0x21000060/0x10000541, fellows0x21000030/0x10000281, vassals0x2100002F/0x10000266. The importer's skip is same-LAYOUT only (LayoutImporter.cs:233-249), and in any caseMountSocialPaneluses the single-rootImportInfos(dats, layoutId, rootElementId)overload, which never enumerates top-level siblings. No parked prototypes are possible here. - No hand-rolled scroll viewport. Both lists ride the existing
UiScrollablePanelthroughUiTemplateListBox, so the #371 straddling-row clip comes for free. Popup/pointer-priority rules are not applicable.
UiTemplateListBox.Flush(). The doc claim "resetting ContentHeight to 0"
is accurate: UiScrollablePanel.ClearContent() (:54-61) removes every child,
clears _baseTops, sets ContentHeight = 0 and resets the scroll offset. The
dormancy guard (_viewport?.) is genuine — a never-populated box still never
allocates its viewport. The shrink case the Friends/Squelch rebuild needs
therefore works.
Fellowship empty-state frame swap (item 4). Retail anchor re-read at
gmFellowshipUI::Update @0x0048F440: when
ClientFellowshipSystem::GetFellowshipSystem()->m_pFellowship == 0 it calls
m_pInAFellowshipFrame->SetVisible(0) then
m_pNotInAFellowshipFrame->SetVisible(1) — exactly the two writes
SocialFellowshipPageController.Tick makes, driven off
RuntimeFellowshipSnapshot.IsInFellowship. Lane-A unknown U6 is genuinely
closed: page 0x10000292 has exactly two children, the two frames, so a
single Visible toggle per frame really is the whole swap. (Frame contents:
0x1000026B = name box 0x1000026F, Create 0x10000274, four checkboxes;
0x10000275 = roster ListBox 0x10000279, scrollbar, six buttons
0x1000027B–0x10000280.)
Friends/Squelch read-only binding (item 5). Bound to the J4.1 owners by
reference — RuntimeCommunicationState.Friends/.Squelch are get-only
properties constructed once (RuntimeCommunicationState.cs:67-68,132-133) and
cleared in place at reset (:250-251), so holding the instance across a
generation reset is correct, and FriendsState.Clear() bumps Revision
(FriendsState.cs:67-74) so the panel rebuilds to empty on its own. No new
state copies; Snapshot()'s array copy only runs on a revision change. The
per-frame cost when nothing changed is one Interlocked.Read per list. The
D1 INERT contract holds: checkboxes build as UiButton with a null OnClick
(no latching visual state without a handler), and the six buttons are pinned
null by test.
Panel exclusivity + catalog (item 6). SocialPanel = 12 is byte-verified
(root P0x10000029 = 12). Registered in Mounted only. The toolbar seam
genuinely tolerates it: SyncToolbarWindowButtons iterates ToolbarPanels
only (RetailUiRuntime.cs:809-814), so no phantom button; BindPanelButtons
binds by authored toolbar element, and the toolbar authors no 0x10000029 = 12
button; and the one path that does reach the toolbar with a non-toolbar id —
OnWindowVisibilityChanged → ToolbarController.SetPanelOpen(12, …)
(:831-832) — walks _panelButtons, finds nothing, and returns
(ToolbarController.cs:394-406). This is the same path PositiveEffects (4),
LinkStatus (8), MiniGame (9) and Vitae (15) already take. No crash, no
greyed phantom. Both open paths keep RetailPanelUiController state coherent:
ToggleWindow and CloseWindow both route through TryGetPanelId →
_panelUi (:708-711, :801-807).
Window-frame policy. RetailWindowFrame.Options for the social panel is
byte-for-byte the Options panel's post-review policy (ResizeX=false,
bottom-edge-only, both constrain flags, full ContentAnchors,
DrawChromeCenter = !AuthorsFullPanelCenter(rootInfo)), differing only in
Left/Top. No invented divergence in the shared main-panel cohort. The
root/page height mismatch (root 300x362, pages 300x575) is identical to
the already-gated Options panel's own authored shape, so it is not an FA3
property.
The live-mount probe run (item 7). Executed with
ACDREAM_PROBE_LIVE_MOUNT=1 against the installed DATs: 1 passed, 0
failed. Every assertion holds. Given #383 (installed-DAT vs committed-fixture
drift), I also cross-checked the committed social-panel fixture against the
probe's live dump on root extent, child count and order, the tab table, all
four page P0x57 values, and both allegiance blocks' geometry (including the
two differently sized 0x10000490 instances, monarch (210,0 90x36) vs
patron (220,0 80x36)). No drift — this fixture is faithful to the
installed DAT.
Tests. SocialPanelControllerTests + RetailPanelCatalogTests: 22 passed.
Full AcDream.App.Tests on the committed Release binaries: 4,871 passed / 3
skipped / 0 failed — no collateral regressions. The ledger's +18 arithmetic
checks out exactly (13 SocialPanelControllerTests + 4 RetailPanelCatalogTests
- 1 probe). I did not re-run the full 13,233-test solution suite (no builds permitted in this review), so that figure is unverified but arithmetically consistent.
Register bookkeeping. AD-79's header count is right — the table contains
exactly 59 AD- rows, matching "59 active rows", up one from 58. The row's
element enumeration is accurate against the fixture (Friends 0x10000514/
0x10000515/0x10000516 + 0x1000052C; Squelch 0x10000547/0x1000054B/
0x1000054C), and its honesty about gmFriendsUI/gmSquelchUI being outside
the campaign's researched scope is the right framing. Only SF-5/SF-6 above.
Gate-script spot checks (item 8). Steps 1–3 (F3 opens on Allegiance; repeat
F3 closes; F4 opens directly on Fellowship), step 4 (F3 while open on
Fellowship switches rather than closes) and step 5 (the authored X at
0x10000290, a direct root child at (276,0) 24x25, always visible because it
is not inside any page, wired to ToggleWindow) all match
RetailUiRuntime.OpenSocialPanel (:603-620) and
SocialPanelController.Bind's close wiring (:145-150) exactly. Steps 7–8's
exclusivity claims match RetailPanelUiController.SetPanelVisibility's
one-active-panel logic. Step 6's geometry claims match the frame options. The
two claims that do not hold are MF-1 and MF-2. One wording nit for whoever
edits the script: step 5 calls the X "top-right of the window chrome" when it
is an authored panel element, not chrome — harmless, but a user hunting the
chrome's own close affordance may be briefly confused.
3. Disposition
APPROVE-WITH-FIXES. MF-1 and MF-2 must land before the user runs the gate —
both are in docs/research/2026-08-12-campaign-fa-test-script.md and neither
touches code. SF-1 through SF-9 are code/doc/test corrections that can ride the
same fix round. Nothing here warrants a re-review of the mount path itself: the
tab table, the frame swap, the string resolver, the scoped lookups, the tab
activation, the catalog registration and the toolbar seam are all correct, and
the live-mount probe passes against the installed DATs.
4. NARROW RE-REVIEW — 2026-08-12 — CLOSED, no reopen
Fix round 9afa05b5 / 35c40a9b / ae772709 / a5553904 / c5f73744.
Every disposition below was re-derived from the actual diffs, not from the
commit messages. All 2 MUST-FIX and all 9 SHOULD-FIX are correctly applied.
No finding is reopened. No new finding rises to MUST-FIX or SHOULD-FIX.
4.1 Mechanism dispositions
| # | Verdict | Evidence |
|---|---|---|
| MF-1 tab x-order in the gate script | FIXED | Steps 1, 9, 10 and the "What to report" bullet now all read Allegiance, Fellowship, Friends, Squelch, and step 1 states the point I asked for — "the default tab and the left-most tab are the SAME tab". Each correction carries an inline [FA3 fix-round correction, mechanism MF-1] note naming the earlier draft's error, matching the campaign's correct-in-place convention. |
MF-2 @allegiance info false-defect route |
FIXED, and inverted correctly | Step 14 now states the true FA3 expectation in full: @allegiance info still prints real chat data, the panel blocks stay hidden all gate, "report NEITHER half as a bug". The "What to report" bullet is inverted from "blocks staying hidden" to "blocks becoming VISIBLE at ANY point … there is no code path in FA3 that can make that happen". That is the right anomaly to point the user at, and it cites the FA2 SHA (4272ad0e) for why. |
| SF-1 unsupported retail claim on F3/F4 | FIXED | SocialPanelController.cs:200-215 relabels it as acdream's own OpenSpellbook-precedent convention and reproduces all three negative findings (no GetAttribute_Enum(this, 0x57, …) site; ClientUISystem::OnAction's >0x7c branch handles three unrelated ids; gmPanelUI::ListenToGlobalMessage COMDAT-folded onto a no-op). U11 filed in fa-panel-structure.md §8 with the same evidence and an honest "cdb trace is the only remaining way" recipe. I also checked the no-register-row argument rather than taking it: grepping the register for Toggle*Panel / close-on-second-press returns nothing — no row exists for the spellbook or any other non-toolbar toggle, so "following the existing no-row precedent" is factually true, not a convenient claim. |
| SF-2 per-frame closure allocation | FIXED | Two static readonly Func<…> fields (BlankLineProvider/NoLinesProvider, SocialAllegiancePageController.cs:59-67); Tick now selects one and does a plain field write. Both lambdas close over static readonly fields only, so each is a single cached instance — the tick is genuinely zero-allocation now. |
| SF-3 printed-but-unasserted probe findings | FIXED, and better than asked | Assert.Equal(2, passupCount) added; page exclusivity promoted to a real assertion that also pins which page (Assert.Equal("Allegiance", visiblePageName)) and fails loudly with both offending page names if two are visible. Re-ran live (§4.3): Allegiance Visible=True, other three False, 0x10000492 occurrences = 2. |
| SF-4 "three checkboxes" is four | FIXED in both places | SocialFellowshipPageController.cs:19-21 now says FOUR and names 0x10000270–0x10000273; gate-script step 11 drops the "a fourth may also be present" hedge. |
| SF-5 AD-79 pins six of its seven controls | FIXED | 0x1000052C added to the existing loop in FriendsAndSquelchActionButtons_AreClickable_ButHaveNoHandler, with a doc comment explaining why the checkbox slots into the button loop. Seven enumerated, seven pinned. |
| SF-6 controllers don't cite AD-79 by id | FIXED | Both classes now read "See register row AD-79 (docs/architecture/retail-divergence-register.md)". Greppable in both directions. |
| SF-7 allegiance gate coarser than retail's | FIXED via the documented alternative | Taken as the plan-line option I offered rather than a code change — correct call for a shell slice. The FA5 row now carries an explicit MUST: widen Callbacks.AllegianceSnapshot to reach TryGetMonarch/TryGetPatron, "a monarch character must NOT see an empty, visible monarch block". The class doc (:37-49) repeats it and carries the forward hazard I flagged — that FA5's name population must change Tick's unconditional LinesProvider reassignment in the same commit. |
SF-8 Tick() ignores _disposed |
FIXED | if (_disposed) return; is the first line of SocialPanelController.Tick. |
SF-9 FindDeepest doc overstates |
FIXED | Rewritten to "the LAST UiText found by a pre-order walk", with the single-chain caveat and the live-dumped shape of both real templates. Describes what the code does. |
4.2 Blast dispositions — spot-verified
- Scrollbar wiring (blast MF-1). Both controllers resolve their authored
sibling scrollbar scoped to the page root and assign
scrollbar.Model = listBox.Scroll, with a log line on miss. Verified the seam rather than assuming it:UiTemplateListBox.Scroll => Viewport.Scroll(:107) forces the lazily-created viewport, which is already extent-seeded by the #372 fix and returns one stableUiScrollable, so wiring before the first row exists is sound. Type0xBreally does build asUiScrollbar(DatWidgetFactory.cs:122), the ids0x10000518/0x10000543match each ListBox's own authoredScrollbarElementId(probe-confirmed), andscrollbar.Model = listBox.Scrollis verbatim the shapeCharacterOptionsPageController:323/ChatOptionsPageController:293/ConfigOptionsPageController:397already use. The four new tests pin both the wiring (Assert.Same) and real reachability (40 rows exceeding the authored extent,HasOverflow, scroll actually advances). - Template cache (blast SF-2).
rowTemplateCachememoizes the per-templateElementInfo?(nulls included, so a permanent miss cannot re-walk the DAT) while stillBuild-ing a fresh widget per row under the DAT lock. I checked the soundness question this introduces — whether re-Building from one sharedElementInfois safe:Buildis the documented pure builder and mutates nothing; the only mutating entry point isBuildFromInfos(LayoutImporter.cs:68-78, a tests-only convenience that overwritesrootInfo.Children), which this path does not call. Safe. - Visibility-gated rebuild (blast SF-2).
OnShown/OnHiddenare realIRetainedPanelControllermembers (IRetainedPanelController.cs:12-16) and are genuinely driven —RetailWindowHandle.NotifyVisibility(:92-108) fires them on every transition, andAttachController/NotifyInitialStatecorrectly do NOT fireOnShownfor a window mountedVisible = false. So_visiblestarts false in production exactly as the new test simulates. Fellowship/Allegiance deliberately stay ungated (no DAT access) — right split.Friends_RevisionBumpWhileHidden_DoesNotRebuild_ButRebuildsOnShowpins both halves, including that the bump is deferred rather than lost. - Revision-latch ordering (blast SF-3).
_lastRevisionnow advances only when every row resolved, so a transient resolver miss retries instead of latching an empty list until the next server-side change. Checked the empty -roster case: the loop body never runs,allRowsResolvedstays true, the revision latches — no spurious rebuild loop on an empty friends list. Flushdoc (blast SF-5). Corrected to include the scroll reset, and it goes further than the finding asked by naming the resulting UX cost (a scrolled user yanked to the top on every rebuild), explaining why it was NOT silently fixed, and flagging it forward. That is the right disposition for an unasked behavior change.- #383 narrowing (blast SF-6). I re-derived the timestamps rather than
trusting them:
keyboard_config_21000009.jsonlast written atb4edee97(2026-08-11 09:19),options_2100002B.jsonate71e5a96(2026-08-11 06:25), FA3's fixture commit74c3d85dat 2026-08-12 02:58 — ~17h39m and ~20h33m earlier, the previous day. "~18h and ~21h, same day" is accurate, and it does materially tighten the investigation window. The addendum correctly attributes the new fixture's no-drift confirmation and narrows the issue to the two OP-era fixtures. - Two new gate steps (blast N-8). Step 19 (long-roster scroll) targets
exactly where the scrollbar fix bites and is honestly marked soft when the
roster is too short; its arithmetic is right (Friends box
270x400inside a300x362panel). Step 21 (restore-open across relaunch) is stated as expected-not-a-bug, and its premise checks out —stateManagedVisibilityWindowsis still[Combat, JumpPowerbar, ExternalContainer, Vendor](RetailUiRuntime.cs:440-446), so the social panel restores like every sibling main panel.
4.3 Gates re-run on the post-fix binaries
Release binaries timestamp 2026-08-12 03:44, i.e. at/after the last code
commit (ae772709, 03:44) — the runs below are against the fixed code.
- Live-mount probe (
ACDREAM_PROBE_LIVE_MOUNT=1, installed DATs): 1 passed / 0 failed. The two promoted assertions pass with observable values —page Allegiance … Visible=True, Friends/Fellowship/SquelchVisible=False,0x10000492 occurrences under allegiance page = 2— and all four captions still resolve (Allegiance/Fellowship/Friends/Squelch). AcDream.App.Tests, Release,--no-build: 4,876 passed / 3 skipped / 0 failed, exactly+5over the 4,871 I measured pre-fix-round and exactly the ledger's own per-project App figure. The+5reconciles: five genuinely new[Fact]s (two scrollbar-wiring, two long-roster reachability, one hidden-vs-shown rebuild), with the two extended-in-place tests and the two new probe assertions correctly counted as net zero. The 13,238/4/0 solution total is therefore consistent at the only project this fix round touched; I did not re-run the other eight (no builds permitted).- Blast radius:
git diff --stat 9e622f56..c5f73744touches 14 files, all FA3's own plus doc-only edits to the sharedUiTemplateListBoxand theMountSocialPanelmethod. No collateral edits.
4.4 Carry-forward (non-blocking, for FA4/FA5 — not reopened findings)
- Scroll position resets on every rebuild. Correctly deferred and
documented in
UiTemplateListBox.Flush. It bites harder in FA4, whose fellow roster rebuilds on every vitals tick rather than on a rare roster change — worth resolving there rather than deferring again. - The production template cache is untested. Both long-roster tests use
FakeRowTemplateResolver, sorowTemplateCacheitself has no coverage. I verified its soundness by inspection (§4.2), but a rebuild-twice assertion in the live-mount probe would cost one line and cover the real path. - Hardcoded scrollbar ids.
0x10000518/0x10000543are literals whenUiTemplateListBox.ScrollbarElementIdalready carries the authored value. Both match today (probe-confirmed) and this mirrors the existing Options-tab consumers, so it is a cohort-wide nit, not an FA3 defect. allRowsResolvedretry loop. A permanently unresolvable template makes the list rebuild every frame while the panel is open. Bounded and cheap (the cache stores the null, so no DAT walk;Buildis skipped entirely) — recorded so it is not rediscovered as a mystery.
4.5 Verdict
CLOSED — no reopen. Every mechanism finding is applied, and each was applied at the right altitude: the two MUST-FIX as script rewrites that state the verified truth rather than merely deleting the wrong claim, SF-7 as a binding FA5 acceptance line rather than scope creep into a shell slice, and SF-3 as assertions that pin more than the finding asked for. The fix round also left the two things it chose not to change (the Flush scroll reset, the coarser allegiance gate) documented and forward-flagged rather than silent, which is the disposition this project's register rules ask for. FA3's remaining obligation is unchanged: the user's connected gate, now against a script that no longer contains two instructions guaranteed to produce false defect reports.