fix(plugin-ui): Slice B residuals - shelf icon sink without magenta, validated icon bindings, negative ids, memoized DID resolves

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>
This commit is contained in:
Erik 2026-09-06 16:28:13 +02:00
parent 761a7519f1
commit ce05c4fb03
10 changed files with 538 additions and 21 deletions

View file

@ -278,3 +278,34 @@ all four places, list icon column aligned with rows.
- Root `<panel visible>` binding-only literal support — the root element
currently accepts only `{Binding}` for `visible`, not a literal
`visible="true"/"false"` the way every child element does.
- Residual round (2026-09-06, commit following `466272ec5`): shelf icon
resolve re-pointed from `ResolveSprite`/`ResolveChrome` (magenta on miss)
to the shared `IMarkupIconResolver.ResolveDid` (draws nothing on miss) so
the initials fallback can actually fire in production; `icon="{Typo}"`/
`icons="notabinding"` now throw at `Build` on a resolver-less host too;
negative/overflow icon ids (scalar and list) map to `0u` instead of
throwing `OverflowException` at draw or wrapping to `0xFFFFFFFF`;
`RetailMarkupIconResolver.ResolveDid` memoizes per DID (including the
miss) instead of re-probing the DAT every frame; `<icon iconkind="...">`
now throws at `Build` instead of silently ignoring the attribute; stale
`SampleData.cs:64` citations corrected to `:69` (Melee Defense's real
line).
- **Before shipment:** the Smoke plugin's "Icon Smoke" panel
(`src/AcDream.Plugins.Smoke/SmokeIconPanel.cs`) keeps `StartVisible=true`
on purpose, for the owner's connected gate — flip it to
`StartVisible=false` (or drop `AcDream.Plugins.Smoke` from the release
payload entirely) once that gate is done. Checked whether the LAUNCHER
release payload actually ships it: `tools/publish-bin.ps1`'s
`New-PayloadZip` zips App's ENTIRE publish output directory
(`[IO.Compression.ZipFile]::CreateFromDirectory`, no file filtering
beyond checking `$RequiredFiles` exist) into `client-<rid>.zip`, and
`AcDream.App.csproj`'s `CopySmokePluginToPublishOutput` target
(`AfterTargets="Publish"`, gated only on `IsCrossTargetingBuild`, not on
`Configuration`) copies `plugins/AcDream.Plugins.Smoke/` straight into
that same publish directory during `dotnet publish`. **The Smoke plugin
— auto-open panel included — ships in today's release zip.** (The
MossTank plugin has the equivalent `CopyMossTankPluginToPublishOutput`
target and ships the same way; a quick grep shows several of its own
panels also set `StartVisible = true` — worth the same "before shipment"
look, but out of scope for this residual round, which only covers the
Smoke plugin per N9.)

View file

@ -69,9 +69,9 @@ check those four against the markup by eye.
| `meter cur`, `meter max` | **Silent**`BindUint` returns `null` (the meter shows no cur/max) | `uint?` (accepts any integral type) |
| `meter fill`, `slider value` | **Silent**`BindFloat` returns `0` | `float?`/`float` |
| `list items`, `menu items` | Throws | `IEnumerable<string>` |
| `list colors` | **Silent** if omitted (no color override); throws if present but mistyped | `IEnumerable<uint>` |
| `list icons` (Slice B) | Throws if present but mistyped; omitting it entirely means no icon column at all | `IEnumerable<uint>` **or** `IEnumerable<int>` |
| `<icon>`/`<button icon>` `did`/`spell`/`item` bindings (Slice B) | Throws | any integral type (`uint`, `int`, `long`, `ushort`, a nullable of one, …) via `Convert.ToUInt32` |
| `list colors` | **Silent** if omitted (no color override); throws if present but mistyped | `IEnumerable<uint>` **or** `IEnumerable<int>` (shared `BindUintList`) |
| `list icons` (Slice B) | Throws if present but mistyped; omitting it entirely means no icon column at all. A negative `int` element is **silent**: it maps to `0u` (no icon for that row), matching the scalar `did`/`spell`/`item` row above | `IEnumerable<uint>` **or** `IEnumerable<int>` |
| `<icon>`/`<button icon>` `did`/`spell`/`item` bindings (Slice B) | Throws (missing/mistyped property) — but a resolved value that is negative or above `uint.MaxValue` is **silent**: it maps to `0u` (draws nothing) rather than throwing `OverflowException` at draw time | any integral type (`uint`, `int`, `long`, `ushort`, a nullable of one, …) via `Convert.ToUInt32` |
| `list selected` | Throws (required int reader) | `int` |
| `tab selected`, `toggle checked` | Throws (required bool reader) | `bool` |
| root `panel visible` | Throws (required bool reader; see the root-only note below) | `bool` |
@ -168,7 +168,7 @@ in particular comes straight from `SkillBase.IconId`, and retail's own
`UIRegion::SetImageByDID(SkillBase._iconID)` (`@0x004f150e`) draws that field
directly as a DID with no `+0x06000000` step of its own — adding the block
prefix again would double-normalize it and resolve nothing (see
`src/AcDream.App/UI/Layout/SampleData.cs:64-82` for the real values: Melee
`src/AcDream.App/UI/Layout/SampleData.cs:69-83` for the real values: Melee
Defense is `0x06000165`, never `7735`/`0x165`, in that field).
**API-v1 note:** `IconId` is a positional/`init` member on each of the four
@ -216,6 +216,12 @@ never stretches. A `tooltip` attribute makes the icon a real hit-test target
(it is click-through otherwise, so it never steals clicks meant for something
underneath it).
`<icon>` derives its kind from WHICH of `did`/`spell`/`item` is set — unlike
`<button>`/`<list>`, it has no per-element `iconkind` to disambiguate.
Putting `iconkind` on an `<icon>` throws `FormatException` at `Build`
("`iconkind applies to button and list; icon derives its kind from
did/spell/item`") rather than silently ignoring it.
### `<button icon="..." iconkind="did|spell|item">`
```xml

View file

@ -245,6 +245,16 @@ public sealed class TextureCache
/// </summary>
public uint GetOrUploadRenderSurface(uint renderSurfaceId, out int width, out int height, bool nearest = false)
{
// Residual round finding N5 (nit): keying by (renderSurfaceId, nearest)
// means a DID requested BOTH nearest (plugin markup icons,
// RetailMarkupIconResolver.ResolveDid) and non-nearest (chrome art via
// ResolveChrome) decodes and uploads the same source bytes twice —
// GetOrCreateLinearUiTwin below exists to share one already-uploaded
// texture across samplers, but only in the nearest-registered-first
// direction, and no caller wires this method through it. Left
// unaddressed (nit, not required for this round): the rewiring would
// need a lookup keyed by id alone before falling back to per-sampler
// upload, which this method does not have today.
var cacheKey = (renderSurfaceId, nearest);
if (_renderSurfaceGpuTextures.TryGetValue(cacheKey, out GpuUiTextureEntry existing))
{

View file

@ -60,6 +60,25 @@ public sealed class RetailMarkupIconResolver : IMarkupIconResolver
private readonly IconComposer _icons;
private readonly ClientObjectTable _objects;
/// <summary>
/// Residual round finding N4 (perf): memoizes <see cref="ResolveDid"/>'s
/// result per DID, including the <c>(0, 0, 0)</c> miss. Without this, an
/// unresolvable id (e.g. the shelf's initials-fallback path, or a
/// standalone <c>&lt;icon&gt;</c> whose id never resolves) re-probed
/// Portal AND HighRes — two cache misses plus two B-tree lookups under
/// <c>DatDatabaseWrapper</c>'s database lock — on EVERY frame, forever;
/// a resolvable id re-entered the DAT lock via
/// <see cref="TextureCache.GetOrUploadRenderSurface"/> every frame too,
/// even though that call's own cache already made the SECOND upload
/// free — the wasted cost was the per-frame database-lock re-entry, not
/// a duplicate GPU upload. A plain <see cref="Dictionary{TKey,TValue}"/>
/// is fine (unbounded, no eviction): the set of distinct plugin icon
/// DIDs one process ever asks for is small (per-plugin descriptor icons
/// plus whatever a plugin's own markup binds), nothing like the world's
/// full RenderSurface population.
/// </summary>
private readonly Dictionary<uint, (uint tex, int w, int h)> _resolvedDidCache = new();
public RetailMarkupIconResolver(
IDatReaderWriter dats,
TextureCache textureCache,
@ -96,13 +115,23 @@ public sealed class RetailMarkupIconResolver : IMarkupIconResolver
{
if (did == 0u)
return (0u, 0, 0);
if (_resolvedDidCache.TryGetValue(did, out (uint tex, int w, int h) cached))
return cached;
(uint tex, int w, int h) result;
if (!_dats.Portal.TryGet<RenderSurface>(did, out _)
&& !_dats.HighRes.TryGet<RenderSurface>(did, out _))
{
return (0u, 0, 0);
result = (0u, 0, 0);
}
uint tex = _textureCache.GetOrUploadRenderSurface(did, out int w, out int h, nearest: true);
return (tex, w, h);
else
{
uint tex = _textureCache.GetOrUploadRenderSurface(did, out int w, out int h, nearest: true);
result = (tex, w, h);
}
_resolvedDidCache[did] = result;
return result;
}
public (uint tex, int w, int h) ResolveSpell(uint spellId)

View file

@ -228,10 +228,19 @@ public static class MarkupDocument
{
string? buttonIconKind = (string?)el.Attribute("iconkind");
ValidateIconKind(buttonIconKind);
// Residual round finding N2: BindUintLiteralOrBinding
// must run UNCONDITIONALLY — same rule as
// ValidateIconKind just above — so a malformed
// icon="{Typo}" throws FormatException at Build on
// every host, not only ones with icons wired. Only
// the ASSIGNMENT onto button.IconSource stays gated
// on icons is not null (finding 7: an always-empty
// IconSource would permanently reserve the icon
// column on a resolver-less host).
Func<uint> buttonIconReader =
BindUintLiteralOrBinding(buttonIcon, binding, "button icon");
if (icons is not null)
{
Func<uint> buttonIconReader =
BindUintLiteralOrBinding(buttonIcon, binding, "button icon");
button.IconSource = BuildIconSource(
buttonIconKind,
buttonIconReader,
@ -246,6 +255,21 @@ public static class MarkupDocument
case "icon":
{
// Residual round finding N7: unlike <button>/<list>,
// <icon> derives its kind from WHICH of did/spell/item is
// set (below) — iconkind is meaningless here and was
// previously silently ignored (a plugin author's
// iconkind="spell" typo on an <icon did="..."> would
// never do what it looked like it did). Reject it
// loudly instead, same "malformed markup throws at
// Build" rule every other attribute in this grammar
// follows.
if (el.Attribute("iconkind") is not null)
{
throw new FormatException(
"iconkind applies to button and list; icon derives its kind from did/spell/item");
}
string? didAttr = (string?)el.Attribute("did");
string? spellAttr = (string?)el.Attribute("spell");
string? itemAttr = (string?)el.Attribute("item");
@ -518,9 +542,17 @@ public static class MarkupDocument
{
string? listIconKind = (string?)el.Attribute("iconkind");
ValidateIconKind(listIconKind);
// Residual round finding N2: same rule as the button's
// icon reader above — BindUintList must run
// UNCONDITIONALLY so icons="notabinding" (a malformed,
// non-{Binding} literal — list icons has no literal
// grammar) throws FormatException at Build even with no
// resolver wired. Only the assignment stays gated.
Func<IReadOnlyList<uint>> listIconIdsReader =
BindUintList(listIcons, binding, "list icons");
if (icons is not null)
{
list.IconIdsSource = BindUintList(listIcons, binding, "list icons");
list.IconIdsSource = listIconIdsReader;
list.IconResolve = BuildRowIconResolve(listIconKind, icons);
}
}
@ -628,6 +660,20 @@ public static class MarkupDocument
/// at read time rather than a type check at Build, the same tradeoff
/// <see cref="BindUint"/> already makes.
/// </remarks>
/// <remarks>
/// Residual round finding N3: a NEGATIVE bound value (Decal's own
/// convention for "no icon" — e.g. <c>HudPictureBox.Image = -1</c>) used
/// to throw <see cref="OverflowException"/> straight out of
/// <see cref="Convert.ToUInt32(object, IFormatProvider)"/> — every frame,
/// from inside <c>UiSimpleButton.OnDraw</c>. DECIDED: any value that does
/// not fit in a <see cref="uint"/> — negative, or above
/// <see cref="uint.MaxValue"/> — maps to <c>0u</c> (draws nothing) rather
/// than throwing at draw time. <see cref="Convert.ToUInt32(object, IFormatProvider)"/>
/// already throws exactly <see cref="OverflowException"/> for both of
/// those cases (never for a non-numeric type, which still throws
/// <see cref="InvalidCastException"/>/<see cref="FormatException"/> as
/// before), so catching only that one exception type is sufficient.
/// </remarks>
private static Func<uint> BindUintLiteralOrBinding(
string expression, object binding, string context)
{
@ -647,10 +693,28 @@ public static class MarkupDocument
{
uint u => u,
null => 0u,
var v => Convert.ToUInt32(v, CultureInfo.InvariantCulture),
var v => ToUintOrZero(v),
};
}
/// <summary>
/// <see cref="Convert.ToUInt32(object, IFormatProvider)"/>, mapping an
/// out-of-range value (negative, or above <see cref="uint.MaxValue"/>) to
/// <c>0u</c> instead of propagating <see cref="OverflowException"/>. See
/// <see cref="BindUintLiteralOrBinding"/>'s finding N3 remark.
/// </summary>
private static uint ToUintOrZero(object value)
{
try
{
return Convert.ToUInt32(value, CultureInfo.InvariantCulture);
}
catch (OverflowException)
{
return 0u;
}
}
private static uint ParseUintLiteral(string text, string context)
{
string trimmed = text.Trim();
@ -776,9 +840,16 @@ public static class MarkupDocument
/// (MosswartMassacre's <c>FlagTrackerView.cs</c> feeds
/// <c>HudPictureBox.Image</c> from <c>int</c> ids), so a plugin porting
/// that convention hands the host <c>IEnumerable&lt;int&gt;</c>, not
/// <c>IEnumerable&lt;uint&gt;</c>. Icon ids never go negative in
/// practice, so the per-element conversion is an unchecked reinterpret
/// rather than a throwing checked cast.
/// <c>IEnumerable&lt;uint&gt;</c>.
/// </remarks>
/// <remarks>
/// Residual round finding N3: the per-element conversion used to be an
/// UNCHECKED reinterpret (<c>-1</c> silently wrapped to <c>0xFFFFFFFF</c>
/// — a bogus, almost-certainly-unresolvable id drawn as if it were a real
/// one, rather than the "no icon" <c>0u</c> the same negative value maps
/// to on the scalar path — see <see cref="ToUintOrZero"/>). DECIDED: a
/// negative element now maps to <c>0u</c> here too, matching the scalar
/// path's contract exactly (both "no icon" conventions agree).
/// </remarks>
private static Func<IReadOnlyList<uint>> BindUintList(
string? expression,
@ -805,7 +876,7 @@ public static class MarkupDocument
if (typeof(IEnumerable<int>).IsAssignableFrom(property.PropertyType))
{
return () => property.GetValue(binding) is IEnumerable<int> values
? values.Select(static v => unchecked((uint)v)).ToArray()
? values.Select(static v => v < 0 ? 0u : (uint)v).ToArray()
: Array.Empty<uint>();
}
throw new FormatException(

View file

@ -4781,9 +4781,24 @@ public sealed class RetailUiRuntime : IDisposable
{
if (_pluginSidePanel is null)
{
// Residual round finding N1 (2026-09-06): the shelf's
// icon resolve must go through the SAME
// IMarkupIconResolver.ResolveDid every markup icon
// sink uses, never _bindings.Assets.ResolveSprite
// (= ResolveChrome = TextureCache.GetOrUploadRenderSurface,
// which returns the 1x1 magenta placeholder for a
// missing id — see that method's own doc comment).
// ResolveDid returns (0,0,0) for an id that does not
// resolve to a real installed RenderSurface, which is
// what lets PluginShelfButton's initials fallback
// (_iconAvailable) actually fire for a bad descriptor
// id in production. iconResolver.ResolveDid already
// expects a normalized did (PluginShelfButton's own
// ctor normalizes via PluginIcons.Normalize before
// calling _resolve), so no double-normalize here.
_pluginSidePanel = new PluginSidePanel(
Host.WindowManager,
_bindings.Assets.ResolveSprite,
iconResolver.ResolveDid,
_bindings.Assets.DefaultFont);
Host.Root.AddChild(_pluginSidePanel);
// Slice A: the shelf is itself a retained window (stable

View file

@ -15,7 +15,7 @@ namespace AcDream.Plugins.Smoke;
/// <remarks>
/// Review fix round (2026-09-06): <see cref="BareIndexIconId"/> was
/// <c>7735</c> (an unverified literal) and is now retail's Melee Defense
/// skill icon index <c>0x165</c> — <c>SampleData.cs:64</c> attests
/// skill icon index <c>0x165</c> — <c>SampleData.cs:69</c> attests
/// <c>0x06000165</c> is a real installed-DAT RenderSurface, so the
/// descriptor and the bare-index <c>&lt;icon&gt;</c> both draw ART a tester
/// can actually verify against retail, not a guess. The list's
@ -31,7 +31,7 @@ internal static class SmokeIconPanel
/// <summary>
/// A Decal/VirindiViewService-style bare portal.dat index for retail's
/// Melee Defense skill icon (<c>0x06000165</c> — attested in
/// <c>src/AcDream.App/UI/Layout/SampleData.cs:64</c>) rather than an
/// <c>src/AcDream.App/UI/Layout/SampleData.cs:69</c>) rather than an
/// unverified literal. Deliberately used on BOTH the descriptor and the
/// first <c>&lt;icon&gt;</c> below to prove the host normalizes it
/// identically at each sink.

View file

@ -207,6 +207,23 @@ public sealed class MarkupIconTests
() => MarkupDocument.Build(xml, new object(), Sprite));
}
[Fact]
public void Icon_IconKindAttribute_ThrowsAtBuild()
{
// Residual round finding N7: unlike <button>/<list>, <icon> derives
// its kind from WHICH of did/spell/item is set — iconkind was
// previously silently ignored here (a plugin author's
// iconkind="spell" typo on an <icon did="..."> would never do what
// it looked like it did). Now it throws.
const string xml =
"<panel x=\"0\" y=\"0\" w=\"100\" h=\"60\">" +
"<icon x=\"0\" y=\"0\" did=\"1\" iconkind=\"did\"/>" +
"</panel>";
Assert.Throws<FormatException>(
() => MarkupDocument.Build(xml, new object(), Sprite));
}
[Fact]
public void Icon_WidthHeightDefaultTo32WhenOmitted()
{
@ -290,6 +307,23 @@ public sealed class MarkupIconTests
() => MarkupDocument.Build(xml, new object(), Sprite));
}
[Fact]
public void ButtonIcon_MalformedBinding_ThrowsAtBuild_EvenWithNoResolverWired()
{
// Residual round finding N2: BindUintLiteralOrBinding used to run
// ONLY when icons was non-null, so icon="{Typo}" (no such property
// on the binding object) silently loaded on a resolver-less host
// instead of throwing FormatException at Build the way every other
// malformed markup attribute (including iconkind just above) does.
const string xml =
"<panel x=\"0\" y=\"0\" w=\"100\" h=\"60\">" +
"<button x=\"0\" y=\"0\" w=\"60\" h=\"20\" text=\"Go\" icon=\"{Typo}\"/>" +
"</panel>";
Assert.Throws<FormatException>(
() => MarkupDocument.Build(xml, new object(), Sprite));
}
[Fact]
public void ButtonIcon_NoResolverWired_IconSourceStaysNull()
{
@ -335,6 +369,35 @@ public sealed class MarkupIconTests
Assert.Equal(("did", PluginIcons.Normalize(42u)), resolver.Calls[^1]);
}
private sealed class NegativeIntIconBinding
{
public int IconIdInt { get; set; } = -1;
}
[Fact]
public void ButtonIcon_NegativeIntProperty_MapsToZero_NotOverflowException()
{
// Residual round finding N3: Convert.ToUInt32(-1) used to throw
// OverflowException straight out of BindUintLiteralOrBinding's
// reader — every frame, from inside UiSimpleButton.OnDraw, since
// IconSource is invoked at draw time. DECIDED: a negative bound
// value (Decal's own "no icon" convention, e.g.
// HudPictureBox.Image = -1) maps to 0u instead of throwing.
const string xml =
"<panel x=\"0\" y=\"0\" w=\"100\" h=\"60\">" +
"<button x=\"0\" y=\"0\" w=\"60\" h=\"20\" text=\"Go\" icon=\"{IconIdInt}\"/>" +
"</panel>";
var resolver = new FakeIconResolver();
var binding = new NegativeIntIconBinding();
var panel = MarkupDocument.Build(xml, binding, Sprite, icons: resolver);
var button = Assert.IsType<UiSimpleButton>(panel.Children[0]);
(uint tex, _, _) = button.IconSource!();
Assert.Equal(0u, tex);
Assert.Equal(("did", 0u), resolver.Calls[^1]);
}
// ── <list icons>: resolver dispatch + column reservation ──────────────────
private sealed class ListIconBinding
@ -419,6 +482,24 @@ public sealed class MarkupIconTests
Assert.Null(list.IconResolve);
}
[Fact]
public void ListIcons_MalformedBinding_ThrowsAtBuild_EvenWithNoResolverWired()
{
// Residual round finding N2: BindUintList used to run ONLY when
// icons was non-null, so icons="notabinding" (list icons has no
// literal grammar at all — every non-empty value must be a
// {Binding}) silently loaded on a resolver-less host instead of
// throwing FormatException at Build.
const string xml =
"<panel x=\"0\" y=\"0\" w=\"200\" h=\"100\">" +
"<list x=\"0\" y=\"0\" w=\"180\" h=\"60\" items=\"{Items}\" " +
"icons=\"notabinding\" selected=\"{Selected}\"/>" +
"</panel>";
Assert.Throws<FormatException>(
() => MarkupDocument.Build(xml, new ListIconBinding(), Sprite));
}
private sealed class IntListIconBinding
{
public IReadOnlyList<string> Items { get; } = new[] { "First", "Second" };
@ -426,6 +507,36 @@ public sealed class MarkupIconTests
public int Selected { get; set; } = -1;
}
private sealed class NegativeIntListIconBinding
{
public IReadOnlyList<string> Items { get; } = new[] { "First" };
public IEnumerable<int> IconIds { get; } = new[] { -1 };
public int Selected { get; set; } = -1;
}
[Fact]
public void ListIcons_NegativeIntElement_MapsToZero_NotWraparound()
{
// Residual round finding N3: the per-element conversion used to be
// an unchecked reinterpret — -1 silently wrapped to 0xFFFFFFFF, a
// bogus id that could coincidentally resolve to something, rather
// than the "no icon" 0u the scalar (button/icon) path already used
// for the same negative value. DECIDED: both paths now agree.
const string xml =
"<panel x=\"0\" y=\"0\" w=\"200\" h=\"100\">" +
"<list x=\"0\" y=\"0\" w=\"180\" h=\"60\" items=\"{Items}\" " +
"icons=\"{IconIds}\" iconkind=\"did\" selected=\"{Selected}\"/>" +
"</panel>";
var resolver = new FakeIconResolver();
var binding = new NegativeIntListIconBinding();
var panel = MarkupDocument.Build(xml, binding, Sprite, icons: resolver);
var list = Assert.IsType<UiMarkupList>(panel.Children[0]);
Assert.NotNull(list.IconIdsSource);
Assert.Equal(new uint[] { 0u }, list.IconIdsSource!());
}
[Fact]
public void ListIcons_BindsToAnIEnumerableOfInt_NotOnlyIEnumerableOfUint()
{

View file

@ -982,7 +982,7 @@ public sealed class PluginSidePanelTests
// Slice B): the descriptor's bare Decal-style index must resolve
// through PluginIcons.Normalize at the shelf button, same as every
// markup did sink — 0x165 (Melee Defense's real installed-DAT index,
// SampleData.cs:64) becomes 0x06000165.
// SampleData.cs:69) becomes 0x06000165.
var resolvedIds = new List<uint>();
var root = new UiRoot { Width = 800f, Height = 600f };
using var shelf = new PluginSidePanel(
@ -1010,6 +1010,30 @@ public sealed class PluginSidePanelTests
Assert.Contains(0x06000165u, resolvedIds);
}
/// <summary>
/// Residual round finding N1: this fake's shape matters, not just its
/// return value. Production wires <see cref="PluginSidePanel"/> to
/// <c>IMarkupIconResolver.ResolveDid</c> (the same seam every other
/// markup icon sink shares) — NEVER to
/// <c>RetailUiAssets.ResolveSprite</c> (= ResolveChrome =
/// <c>TextureCache.GetOrUploadRenderSurface</c>), which returns a
/// non-zero 1x1 magenta placeholder handle for a missing id rather than
/// <c>(0, 0, 0)</c>. A bare <c>_ => (0u, 0, 0)</c> lambda proves the
/// button's OWN fallback logic works, but says nothing about whether the
/// resolver actually WIRED IN would ever produce that input — a
/// resolver that instead mirrored ResolveChrome's magenta-on-miss
/// contract would have passed a test built that way while still
/// leaving the fallback dead in production. This type exists so the
/// test reads as "given a resolver that behaves like
/// RetailMarkupIconResolver.ResolveDid", matching
/// <c>RetailMarkupIconResolverInstalledDatTests.ShelfButton_BogusDescriptorId_OnTheRealResolver_FallsBackToInitials</c>,
/// which runs the same shape against the REAL resolver and a real DAT.
/// </summary>
private sealed class FakeResolveDidResolver
{
public (uint tex, int w, int h) ResolveDid(uint did) => (0u, 0, 0);
}
[Fact]
public void ShelfButton_FallsBackToInitials_WhenTheResolveNeverYieldsATexture()
{
@ -1018,9 +1042,10 @@ public sealed class PluginSidePanelTests
// different installation) must fall back to the initials text
// rather than rendering a blank button forever.
var root = new UiRoot { Width = 800f, Height = 600f };
var resolver = new FakeResolveDidResolver();
using var shelf = new PluginSidePanel(
root.WindowManager,
_ => (0u, 0, 0),
resolver.ResolveDid,
font: null);
root.AddChild(shelf);

View file

@ -1,9 +1,16 @@
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Numerics;
using AcDream.App.Rendering;
using AcDream.App.Rendering.Gpu;
using AcDream.App.Tests.Rendering;
using AcDream.App.Tests.Rendering.Gpu;
using AcDream.App.UI;
using AcDream.Content;
using AcDream.Core.Items;
using AcDream.Plugin.Abstractions;
using DatReaderWriter;
using DatReaderWriter.Lib.IO;
using DatReaderWriter.Options;
using Xunit;
@ -27,7 +34,7 @@ public sealed class RetailMarkupIconResolverInstalledDatTests
/// <summary>
/// A Decal-habit "add the block prefix again" mistake applied to an
/// already-full DID: <c>0x06000165</c> (Melee Defense's real installed
/// icon, <c>SampleData.cs:64</c>) plus another <c>0x06000000</c> lands at
/// icon, <c>SampleData.cs:69</c>) plus another <c>0x06000000</c> lands at
/// <c>0x0C000165</c> — a value almost certainly absent from both Portal
/// and HighRes.
/// </summary>
@ -62,4 +69,216 @@ public sealed class RetailMarkupIconResolverInstalledDatTests
Assert.NotEqual(0u, realTex);
Assert.True(realW > 0 && realH > 0);
}
/// <summary>
/// Residual round finding N1: proves the production wiring fix end to
/// end against the REAL resolver — <see cref="RetailUiRuntime.MountPlugins"/>
/// now passes <c>iconResolver.ResolveDid</c> (this class) to
/// <see cref="PluginSidePanel"/>, not
/// <c>_bindings.Assets.ResolveSprite</c> (= ResolveChrome =
/// <see cref="TextureCache.GetOrUploadRenderSurface"/>, which returns a
/// non-zero 1x1 magenta placeholder for a missing id and would have kept
/// <see cref="PluginSidePanel.PluginShelfButton"/>'s initials fallback
/// from ever firing). A bogus descriptor icon id run through the real
/// resolver must resolve to nothing, and the shelf button must fall
/// back to its initials text — exactly the shape that would have caught
/// the pre-fix wiring bug (ResolveSprite would have returned a non-zero
/// magenta handle here instead of (0,0,0)).
/// </summary>
[Fact]
public void ShelfButton_BogusDescriptorId_OnTheRealResolver_FallsBackToInitials()
{
string? datDir = InstalledDatTestPath.Resolve();
if (datDir is null)
{
Assert.Fail(
"Lane=InstalledDat requires an installed retail DAT directory; see docs/release-gate.md.");
return;
}
using var dats = new DatCollection(datDir, DatAccessType.Read);
using var adapter = new DatCollectionAdapter(dats);
var device = new RecordingGpuDevice();
using var cache = new TextureCache(device, adapter);
var icons = new IconComposer(adapter, cache);
var objects = new ClientObjectTable();
var resolver = new RetailMarkupIconResolver(adapter, cache, icons, objects);
var root = new UiRoot { Width = 800f, Height = 600f };
using var shelf = new PluginSidePanel(
root.WindowManager,
resolver.ResolveDid,
font: null);
root.AddChild(shelf);
var frame = new UiPanel { Width = 200f, Height = 100f };
root.AddChild(frame);
RetailWindowHandle handle = root.WindowManager.Register(
"plugin:acdream.test:main", frame);
shelf.Add(
new PluginUiOwner("acdream.test", "Test Plugin"),
new PluginPanelDescriptor("main", "Test Plugin")
{
IconText = "TP",
// Already >= the Normalize boundary (0x01000000), so
// PluginShelfButton's ctor leaves it unchanged — a real DID
// shape that is nonetheless absent from both Portal and
// HighRes (see DecalHabitDoubleNormalizedId's own doc above).
IconSurfaceId = DecalHabitDoubleNormalizedId,
},
handle);
PluginSidePanel.PluginShelfButton button = Assert.Single(
shelf.Children.OfType<PluginSidePanel.PluginShelfButton>());
Assert.Equal(string.Empty, button.Text);
var textRenderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
textRenderer.Begin(new Vector2(200f, 200f));
var ctx = new UiRenderContext(textRenderer, new Vector2(200f, 200f));
button.DrawSelfAndChildren(ctx);
Assert.Equal("TP", button.Text);
}
private sealed class NullGpuFrameSource : ICurrentGpuFrameSource
{
public IGpuFrame? CurrentFrame => null;
}
/// <summary>
/// Residual round finding N4 (perf): <see cref="RetailMarkupIconResolver.ResolveDid"/>
/// used to probe the DAT (two cache misses + two B-tree lookups under
/// <c>DatDatabaseWrapper</c>'s database lock) on EVERY call for an
/// unresolvable id — including once per frame from a draw-time icon
/// source, forever. These two tests use a bare-bones fake
/// <see cref="IDatReaderWriter"/>/<see cref="IDatDatabase"/> that counts
/// <c>TryGet</c> calls directly, rather than the InstalledDat lane
/// above — the property under test is call COUNT, not decode
/// correctness, so no real DAT files are needed here.
/// </summary>
[Fact]
public void ResolveDid_RepeatedUnresolvableId_ProbesTheDatExactlyOnce()
{
var dats = new CountingDatReaderWriter();
var device = new RecordingGpuDevice();
using var cache = new TextureCache(device, dats);
var icons = new IconComposer(dats, cache);
var objects = new ClientObjectTable();
var resolver = new RetailMarkupIconResolver(dats, cache, icons, objects);
(uint tex1, int w1, int h1) = resolver.ResolveDid(DecalHabitDoubleNormalizedId);
(uint tex2, int w2, int h2) = resolver.ResolveDid(DecalHabitDoubleNormalizedId);
(uint tex3, int w3, int h3) = resolver.ResolveDid(DecalHabitDoubleNormalizedId);
Assert.Equal((0u, 0, 0), (tex1, w1, h1));
Assert.Equal((0u, 0, 0), (tex2, w2, h2));
Assert.Equal((0u, 0, 0), (tex3, w3, h3));
// Without memoization this would be 3 (one probe pair per call);
// with it, the miss is cached after the first resolve.
Assert.Equal(1, dats.Portal.TryGetCallCount);
Assert.Equal(1, dats.HighRes.TryGetCallCount);
}
[Fact]
public void ResolveDid_DifferentIds_ProbeIndependently()
{
// A per-id cache must not collapse distinct ids into one entry.
var dats = new CountingDatReaderWriter();
var device = new RecordingGpuDevice();
using var cache = new TextureCache(device, dats);
var icons = new IconComposer(dats, cache);
var objects = new ClientObjectTable();
var resolver = new RetailMarkupIconResolver(dats, cache, icons, objects);
resolver.ResolveDid(0x06000001u);
resolver.ResolveDid(0x06000002u);
resolver.ResolveDid(0x06000001u);
Assert.Equal(2, dats.Portal.TryGetCallCount);
Assert.Equal(2, dats.HighRes.TryGetCallCount);
}
/// <summary>Always misses (<c>TryGet</c> returns <see langword="false"/>),
/// counting how many times it was asked.</summary>
private sealed class CountingDatDatabase : IDatDatabase
{
public int TryGetCallCount { get; private set; }
public DatDatabase Db => throw new NotImplementedException();
public int Iteration => 0;
public IEnumerable<uint> GetAllIdsOfType<T>() where T : IDBObj =>
throw new NotImplementedException();
public bool TryGet<T>(uint fileId, [MaybeNullWhen(false)] out T value)
where T : IDBObj
{
TryGetCallCount++;
value = default;
return false;
}
public bool TryGetFileBytes(uint fileId, [MaybeNullWhen(false)] out byte[] value) =>
throw new NotImplementedException();
public bool TryGetFileBytes(uint fileId, ref byte[] bytes, out int bytesRead) =>
throw new NotImplementedException();
public bool TrySave<T>(T obj, int iteration = 0) where T : IDBObj =>
throw new NotImplementedException();
public void Dispose() { }
}
/// <summary>
/// Only <see cref="Portal"/>/<see cref="HighRes"/> are exercised by
/// <see cref="RetailMarkupIconResolver.ResolveDid"/>'s miss path — every
/// other member throws if a future change starts touching it, so this
/// fake fails loudly rather than silently returning nonsense.
/// </summary>
private sealed class CountingDatReaderWriter : IDatReaderWriter
{
public CountingDatDatabase Portal { get; } = new();
public CountingDatDatabase HighRes { get; } = new();
IDatDatabase IDatReaderWriter.Portal => Portal;
IDatDatabase IDatReaderWriter.HighRes => HighRes;
public string SourceDirectory => string.Empty;
public IDatDatabase Cell => throw new NotImplementedException();
public ReadOnlyDictionary<uint, IDatDatabase> CellRegions => throw new NotImplementedException();
public IDatDatabase Language => throw new NotImplementedException();
public IDatDatabase Local => throw new NotImplementedException();
public ReadOnlyDictionary<uint, uint> RegionFileMap => throw new NotImplementedException();
public int PortalIteration => 0;
public int CellIteration => 0;
public int HighResIteration => 0;
public int LanguageIteration => 0;
public bool TryGetFileBytes(uint regionId, uint fileId, ref byte[] bytes, out int bytesRead) =>
throw new NotImplementedException();
public IEnumerable<uint> GetAllIdsOfType<T>() where T : IDBObj =>
throw new NotImplementedException();
public bool TrySave<T>(T obj, int iteration = 0) where T : IDBObj =>
throw new NotImplementedException();
public bool TrySave<T>(uint regionId, T obj, int iteration = 0) where T : IDBObj =>
throw new NotImplementedException();
public IEnumerable<IDatReaderWriter.IdResolution> ResolveId(uint id) =>
throw new NotImplementedException();
[return: MaybeNull]
public T Get<T>(uint fileId) where T : IDBObj =>
throw new NotImplementedException();
public bool TryGet<T>(uint fileId, [MaybeNullWhen(false)] out T value)
where T : IDBObj =>
throw new NotImplementedException();
public void Dispose() { }
}
}