Owner report 2026-09-06: MossTank's shelf icon (0x06002C41) drew with a white
ring. DAT icon art reserves pure-white-opaque pixels as the recolor key that
retail IconData::RenderIcons (0x0058d180) replaces per pixel through
SurfaceWindow::ReplaceColor (0x004415b0) from the effect tile — the solid-black
0x21 tile when there are no effects. The inventory already does this through
IconComposer; the plugin did sink (markup <icon did>, <button icon>, <list icons>
and the shelf button) blitted the art raw.
RetailMarkupIconResolver.ResolveDid now hands out IconComposer.GetKeyedIcon —
the drag-icon composite (base art + effects==0 recolor, no overlay, no
underlay), sharing that cache — so did icons look like a mundane inventory item
does. The resolver no longer needs a TextureCache. KeyedIconInstalledDatTests
pins both halves against the real DAT: the raw art carries the key, the
composite carries none, and ResolveDid returns exactly the keyed composite.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Split the two hermetic RetailMarkupIconResolver memoization tests (and
their counting fakes) out of the Lane=InstalledDat class into a new
untagged RetailMarkupIconResolverMemoizationTests.cs so CI's portable
filter (Lane!=InstalledDat) actually runs them.
- PluginSidePanel: move the entry button's Anchors = AnchorEdges.None from
the Add() call site into PluginShelfButton's own constructor (same
comment carried over) so a second construction path cannot miss it.
- UiRectOutlinePainterOrderTests: assert the back panel's border segment
carries exactly 4 quads (24 vertices, FloatsPerVertex each) so a partial
outline cannot pass the painter-order check.
- RetailMarkupIconResolver: document the type as UI-thread-only (every
caller is a draw-time icon source) and bound the MISS cache to 256
entries with FIFO eviction — HIT entries stay unbounded (bounded by the
DAT's own surface count already). New test proves the 257th distinct
miss evicts the first (re-probe count rises); verified failing first
against the un-bounded code (Expected 258, Actual 257) before restoring
the fix.
- docs/plugin-ui-markup.md: split the icon-binding row's failure mode into
Build-time (missing property only — the binder never checks CLR type)
vs. draw-time (a resolved value that cannot convert to a number throws
from the draw, not from Build).
- docs/ISSUES.md: filed #486 (credits picture scroll frozen by the
per-draw anchor pass) and #487 (radar compass tokens candidate, same
mechanism, unconfirmed); corrected #461's causality — the graceful
logout/reveal-cancel log lines are printed by LiveSessionController.Tick's
catch -> StopAfterFailure -> StopCore AFTER the motion-update exception,
then it rethrows, so the logout is a consequence of the crash, not its
cause; real chain is the #462 stalled login-reveal materialization
leaving PlayerMovementController in RuntimeOwnedDormant outside its
SetPosition ground phase when an inbound 0xF74C arrives.
- Plan doc: recorded the three fix-round commits' verdicts (all PASS) and
the Smoke-plugin cleanup commit SHA in the Review ledger, plus a pointer
to the two newly filed issues.
Verified: dotnet build AcDream.slnx -c Release (0/0), targeted filter
85/0/0, full App suite 7364 passed / 97 skipped / 36 failed (36 pre-existing
InstalledDat/Manual/Linux-only failures, unchanged by name from baseline;
net +1 passed test from the new eviction test).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bounded residual round on the Slice B review fix commit (466272ec5),
docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md.
N1 (BLOCKING): PluginSidePanel's shelf entries resolved icons through
_bindings.Assets.ResolveSprite (= ResolveChrome =
TextureCache.GetOrUploadRenderSurface), which returns a non-zero 1x1
magenta placeholder for a missing id - so PluginShelfButton's initials
fallback could never fire in production. RetailUiRuntime.MountPlugins
now passes the same iconResolver.ResolveDid every markup icon sink
already uses, which returns (0,0,0) for an unresolvable id. Re-pointed
the existing initials-fallback unit test at a resolver matching
ResolveDid's real contract, and added an InstalledDat-lane test
(ShelfButton_BogusDescriptorId_OnTheRealResolver_FallsBackToInitials)
proving a bogus descriptor id on the REAL resolver yields initials.
N2: MarkupDocument's <button icon>/<list icons> handling only built the
uint reader (BindUintLiteralOrBinding/BindUintList) when an
IMarkupIconResolver was wired, so a malformed icon="{Typo}" or
icons="notabinding" silently loaded instead of throwing at Build on a
resolver-less host. Both readers now build unconditionally (same rule
ValidateIconKind already followed); only the IconSource/IconIdsSource
assignment stays gated on icons is not null.
N3: a negative bound icon id threw OverflowException out of
Convert.ToUInt32 every frame from inside UiSimpleButton.OnDraw (scalar
path), while the list's IEnumerable<int> path silently wrapped -1 to
0xFFFFFFFF (unchecked reinterpret). Both paths now map any
out-of-range value (negative, or above uint.MaxValue) to 0u instead -
Decal's own "no icon" convention - via a shared ToUintOrZero helper
that catches exactly the OverflowException Convert.ToUInt32 already
throws for both cases.
N4 (perf): RetailMarkupIconResolver.ResolveDid probed Portal/HighRes
(two cache misses + two B-tree lookups under the database lock) on
EVERY call for an unresolvable id, and re-entered the DAT lock on
every resolve of a hit too. Memoizes the resolved (tex,w,h) tuple per
DID, including the (0,0,0) miss, in a plain Dictionary.
N5 (nit): documented in TextureCache.GetOrUploadRenderSurface that the
(id, nearest) cache key uploads the same RenderSurface twice when both
samplers are wanted (chrome via ResolveChrome, plugin icons via
ResolveDid's nearest:true) - GetOrCreateLinearUiTwin exists but only
shares in the nearest-registered-first direction, so wiring it through
here is left as a documented nit rather than a behavior change.
N6 (nit): corrected stale SampleData.cs:64 citations to :69 (Melee
Defense's real line after the file grew) across SmokeIconPanel.cs,
PluginSidePanelTests.cs, RetailMarkupIconResolverInstalledDatTests.cs,
and docs/plugin-ui-markup.md (including the 64-82 range, now 69-83).
N7 (nit): <icon iconkind="..."> was silently ignored (icon derives its
kind from which of did/spell/item is set, unlike button/list). Now
throws FormatException at Build with a message naming the correct
surfaces; documented in plugin-ui-markup.md.
N8 (nit): the truth-table's `list colors` row now says IEnumerable<uint>
or IEnumerable<int> (shared BindUintList), matching `list icons`.
N9 (ship check): added a "Before shipment" line to the plan's Review
ledger. Confirmed the Smoke plugin (including its auto-open Icon Smoke
panel) IS included in the launcher's client-<rid>.zip release payload:
tools/publish-bin.ps1's New-PayloadZip zips App's entire publish
directory unfiltered, and AcDream.App.csproj's
CopySmokePluginToPublishOutput target runs unconditionally
AfterTargets="Publish". No behavior changed per instruction - flagged
for follow-up after the owner's connected gate.
Verification: dotnet build AcDream.slnx -c Release green; filtered
test command 108/108 passed (0 skipped) including the InstalledDat
lane; full AcDream.App.Tests suite 7362 passed / 97 skipped / 36
failed (all 36 pre-existing, same names, none touching
Markup/PluginSidePanel/RetailMarkupIconResolver/TextureCache/PluginIcons);
AcDream.Plugins.MossTank.Tests 337/337 passed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two Opus reviews of commit 8217a349e (Slice B: DAT icons in plugin
markup) found one BLOCKING defect and 14 SHOULD-FIX findings. All 15
fixed here in one commit per the review contract.
BLOCKING (finding 1): an unresolvable did painted a magenta square.
TextureCache.GetOrUploadRenderSurface's 1x1 magenta placeholder for a
missing RenderSurface is load-bearing for authored chrome, but
RetailMarkupIconResolver.ResolveDid only short-circuited did==0, so any
other unresolvable id fell through to that placeholder and got scaled
up by UiMarkupIcon/UiMarkupList/UiSimpleButton -- the classic
resolve(0)-style footgun (claude-memory/feedback_ui_resolve_zero_magenta.md),
just triggered by a missing id instead of a literal 0. Fixed by probing
Portal/HighRes existence via IDatReaderWriter.TryGet<RenderSurface>
BEFORE ever calling GetOrUploadRenderSurface -- that TryGet already
serializes concurrent DAT access internally (DatDatabaseWrapper's own
_databaseLock), the same synchronization IconComposer.TryDecode relies
on, so no additional lock was needed. RetailMarkupIconResolver now
takes IDatReaderWriter + TextureCache directly (RetailUiAssets gained a
TextureCache field, its one construction site in
InteractionRetainedUiComposition.cs updated) instead of the old
resolveSprite delegate, since it also needs the nearest-sampled upload
path for finding 6 below.
Finding 2 -- Smoke panel wiring bugs: its list fed iconkind="spell"
raw art DIDs (PluginSpellInfo.IconId) instead of spell ids, so
IMarkupIconResolver.ResolveSpell composited the wrong (or no) badge
every row. SmokeIconPanel.Binding.SpellIds now yields SpellId (the
printed text still shows IconId alongside). The bare-index demo and the
descriptor both moved from the unverified literal 7735 to 0x165 --
retail's real Melee Defense skill icon (SampleData.cs:64,
0x06000165) -- so the owner's visual gate proves real art, not a guess.
StartVisible flipped true, and a character with no self-buffs known
falls back to spell 1's real catalog entry (or an honest "no spells
known" row with icon 0 if even that fails) rather than fabricating art.
Finding 3 -- PluginIcons.Normalize's threshold was silently rewritten
from the contract's 0x01000000 to 0x06000000 during Slice B. Restored
to 0x01000000; the class/method XML docs now state the number directly
(no cref to the private const); the test table adds 0x02000000 (a value
that only distinguishes the two thresholds) and 0x01000000 itself
(passes through unchanged).
Finding 4 -- an unknown iconkind (e.g. "spel") only threw when a
resolver happened to be wired, because BuildIconSource/
BuildRowIconResolve validated inside their own null-icons early return.
A new ValidateIconKind helper runs UNCONDITIONALLY before that branch,
so a malformed iconkind is a Build-time author error on every host.
Finding 5 -- BindUintLiteralOrBinding required an exact uint property
type, rejecting the int-typed bindings Decal-facing code commonly uses
(MosswartMassacre's HudPictureBox.Image is int end to end). It now
matches BindUint's existing leniency: any property, converted via
Convert.ToUInt32 at read time. BindUintList likewise now accepts
IEnumerable<int> alongside IEnumerable<uint> (unchecked per-element
reinterpret -- icon ids never go negative in practice).
Finding 6 -- TextureCache._renderSurfaceGpuTextures was keyed by id
alone, so whichever caller asked for a given RenderSurface id FIRST won
the sampler for every later caller of the same id -- UiDatFont's glyph
atlases already request nearest:true while ResolveChrome's background
art requests nearest:false, so this was a real, reachable collision,
not hypothetical. Rekeyed to (id, nearest); RetailMarkupIconResolver.
ResolveDid now requests nearest:true (pixel-exact 32x32 icon art);
ResolveChrome is untouched (still nearest:false/linear). Audited every
other _renderSurfaceGpuTextures use site (TryGetValue/set/Dispose
iteration+Clear) plus the separate _nearestUiTextureSources/
_linearUiTwinHandles/_uploadMetadata dictionaries (all keyed by handle
or accounting name, unaffected) -- no other eviction/accounting path
assumed id-only keying.
Finding 7 -- column-reservation semantics, per the DECIDED shape:
MarkupDocument now sets button.IconSource / list.IconIdsSource +
IconResolve ONLY when a resolver (icons parameter) is actually wired --
previously button.IconSource was always assigned (even to an
always-empty func on an icons:null host); combined with this finding's
other half -- UiSimpleButton.OnDraw now reserves its icon column
whenever IconSource is non-null, regardless of a per-frame resolve miss,
so a bound id that goes briefly to 0 no longer slides the caption back
and forth -- would have permanently reserved a blank column on such a
host. UiMarkupList already reserved its column whenever IconIdsSource
was set; no draw-side change needed there.
Finding 8 -- added a with/without-icons comparison test for
UiMarkupList (mirroring the existing UiSimpleButton one): asserts the
row text quad's x is strictly greater with an icon column present, and
the icon quad itself has non-zero width.
Finding 9 -- <icon tooltip=""> (empty string) was still treated as
"has a tooltip" by a bare attribute-presence check, making the icon
swallow clicks with no visible tooltip ever appearing. Now uses
!string.IsNullOrWhiteSpace, matching ApplyCommon's own predicate for
every other element's tooltip.
Finding 10 -- PluginShelfButton.OnDraw drew nothing when a non-zero
descriptor icon id resolved to no texture (a bad Decal index, a DAT id
from a different install), rather than falling back to Initials the
way a zero id already did. Now decides once, on the first draw
(memoized, so Initials' string work never repeats every frame): a
failed resolve permanently switches Text to the initials fallback,
computed and assigned BEFORE base.OnDraw actually paints the caption.
Finding 11 -- MarkupDocument.AddElement's switch had no default arm, so
an unknown or miscased element name (<Icon>, <butotn>) silently
vanished from the built tree instead of failing loudly like every
other malformed-markup case. Added a default arm that throws
FormatException. Ran AcDream.Plugins.MossTank.Tests (337/337,
unchanged) and the full App markup suite to confirm no existing markup
relies on an unknown element.
Finding 12 -- PluginPanelDescriptor.IconSurfaceId's XML doc now states
that a bare Decal index is accepted and normalized, citing
PluginIcons.Normalize.
Finding 13 -- docs/plugin-ui-markup.md: replaced the blanket "wrong
type/missing property throws at Build" sentence with the per-attribute
truth table the review produced (which attributes are silent at
runtime vs. throw at Build, and each one's bound CLR/delegate type);
restated the icon-id boundary as 0x01000000; added the "do NOT add
0x06000000 to the four already-full IconId records" warning (citing
SkillBase._iconID / UIRegion::SetImageByDID @0x004f150e); documented
that 0x-prefixed hex is required (an unprefixed all-digit literal
parses as decimal); noted unknown element names now throw; called out
list colors (0xRRGGBB) vs. color=/background=/border= (#AARRGGBB) as
non-interchangeable grammars; documented the root <panel visible>
binding-only exception; corrected the shelf's collapse toggle glyphs
(</>, not the old doc's arrows) and the 28px collapsed-tab size; added
the IconId record-equality API-v1 note; and called out iconkind as
per-<list> (mixed id spaces need pre-normalized DIDs; the composited
spell badge has no did-space escape hatch) plus the existing
one-text-column LIMITATION being deferred to MossTank.
Coverage added for finding 14: a PluginSidePanelTests case proving the
shelf button normalizes a bare descriptor index before resolving, and a
reflection-based unit on AppAutomationSurface.ProjectWorldObject (its
public callers gate on IsAvailable, which needs a fully connected
session heavier than this mapping needs -- the plan's own documented
fallback) proving PluginWorldObject.IconId carries ClientObject.IconId
through unchanged; PluginInventoryItem.IconId uses the identical
one-line pattern inline in CaptureOwnedItems, reviewed by inspection.
Finding 15: recorded a "Review ledger" section in the plan doc with
both slices' commits, both review verdicts, and the two items
explicitly deferred to the MossTank plugin work (multi-column list,
root literal visible).
Verification: full solution builds green. Targeted filter
(Markup|PluginSidePanel|PluginIcons|AppAutomation|TextureCache|
UiDatFont) passes 131/131, including the two InstalledDat-lane tests
(RetailMarkupIconResolverInstalledDatTests,
AppAutomationSurfaceIconInstalledDatTests) actually resolving against
the real installed DAT, not skipping. AcDream.Plugins.MossTank.Tests
passes 337/337 unchanged. Full AcDream.App.Tests suite: 7351 passed /
97 skipped / 36 failed -- identical failure set/count to the
7334/97/36 baseline (the +17 passes are exactly the new/expanded
tests: 2 new PluginIconsTests.Normalize theory rows, 10 new
MarkupIconTests cases, 2 new PluginSidePanelTests cases, 1 new
AppAutomationSurfaceTests case, and the 2 new standalone test files).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Erik