erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix(ui): OP2 rework — dormant UiDatElement subclasses, fixed Panel/CheckboxBitfield64 mechanism

Browse source 
OP2 (df9c7a35) was double-REJECTed: an unconditional Type-8/Type-5 factory
mapping silently re-classed 15 elements across 7 shipped panels (vendor
backdrop lost its fill, character/spellbook roots stopped passing clicks
through, combat gained a phantom import-time tab takeover, ten ListBoxes
gained a spurious hit-testable viewport) because the stale 27 pre-existing
fixtures never exercised the new fields — and the mechanism itself cited a
nonexistent "UIElement_TabControl" class, inverted UiCheckboxBitfield64's
checked-state predicate, and synthesized fake per-row geometry instead of
using the widget's own authored template.

Shape change: UiTabPanel (renamed from UiTabControl) and UiTemplateListBox
now derive from UiDatElement (unsealed) and stay DORMANT by default — an
imported Type-8/Type-5 element gets authored-media drawing, ClickThrough
generic-decoration default, and IUiDatStateful propagation identical to the
pre-OP2 UiDatElement fallback, with zero import-time side effects. The
factory's Type-8/Type-5 arms are unconditional again (no more guard whose
premise the blast-radius sweep proved false), because dormancy makes an
unactivated instance behaviorally indistinguishable from the old fallback.
UiTabPanel.ActivateTabBehavior() and UiTemplateListBox's lazy viewport
creation are the explicit, controller-driven opt-ins Campaign OP slice OP3+
will call; today nothing does, so the four pre-existing shipped Type-8
hosts (character/spellbook/vendor/combat) and ten pre-existing Type-5
ListBoxes keep their pre-OP2 behavior exactly. Filed AD-73 for this
dormant-vs-retail's-unconditional-activation adaptation.

Mechanism fixes (docs/research/2026-08-11-op2-review-mechanism.md):
- UiTabPanel cites UIElement_Panel (Type 8 is UIElement_Panel; no
  UIElement_TabControl exists in the PDB), resolves buttons/pages via a
  GetChildRecursive-equivalent descendant search (not direct-children-only),
  performs no switch when no entry authors 0x32 (deleted the _tabs[0]
  fallback), and surfaces unresolved tab-table entries via UnresolvedEntries
  + a diagnostic line instead of a silent no-op.
- ElementReader.ReadTabTable skips entries missing 0x30/0x31, matching
  retail's SetupTabPageHash @0x0046C2E0 entry filter.
- UiCheckboxBitfield64 now builds every row from its OWN authored template
  (property 0x64 -> {0x2100002B, 0x10000521}) via AddItemFromTemplateList,
  deleting the synthesized ElementInfo + invented RowHeight=14 — matching
  retail's CreateChildren @0x00485DF0, which is itself a UIElement_ListBox
  call. IsSet is now retail's ANY-bit-set predicate (Refresh @0x004859C0),
  not all-bits-set. TS-72 retired: the click-toggle bit math is now fully
  decomp-confirmed (SetBitsOnOrOff via ListenToElementMessage @0x00485AE0).

Regenerated all 32 UI fixtures against real DAT (ACDREAM_REGENERATE_UI_FIXTURES=1)
and committed them — 27 pre-existing fixtures now carry Outline/OutlineColor/
TabTable/TemplateList/ScrollbarElementId; the 5 Options fixtures were already
current. Updated EffectsUiControllerTests' now-correct UiTemplateListBox
class-identity assertion. Added: 6 built-widget behavior pins for all five
pre-existing Type-8 elements + a representative Type-5 element the dormancy
model protects (OP2ReworkBlastRadiusConformanceTests.cs); 5 reader-level
tests driving ReadTabTable/ReadTemplateList/the 0x72 reader from raw
property bags (ElementReaderTests.cs); a multi-bit-mask UiCheckboxBitfield64
test proving the any-bit predicate (the prior single-bit test couldn't
distinguish it from all-bits); an activation-idempotency test and a
before-activation click-is-inert test for UiTabPanel.

Full Release suite: 12,868 passed / 4 skipped / 0 failed (baseline 12,853/4/0
post-OP1-fixes; +15 net new tests, zero regressions).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 01:06:31 +02:00
parent 8a05fda445
commit b236a44279
40 changed files with 6249 additions and 1145 deletions
Download patch file Download diff file Expand all files Collapse all files

143
src/AcDream.App/UI/UiTemplateListBox.cs
View file

@ -1,32 +1,68 @@
using System;
using System.Collections.Generic;
using System.Numerics;
using AcDream.App.UI.Layout;
namespace AcDream.App.UI;
/// <summary>
/// Retail <c>UIElement_ListBox</c> (Type 5) with an authored row-template list (dat
/// property <c>0x64</c>). Port of <c>AddItemFromTemplateList(index)</c>: instantiates
/// row <paramref name="index"/>'s template — a <see cref="UiTemplateListEntry"/>
/// Retail <c>UIElement_ListBox</c> (dat class Type <c>5</c>) with an authored row-template
/// list (dat property <c>0x64</c>). Port of <c>AddItemFromTemplateList(index)</c>:
/// instantiates row <paramref name="index"/>'s template — a <see cref="UiTemplateListEntry"/>
/// naming a cross-layout <c>{LayoutDesc DID, element id}</c> pair, per
/// <c>docs/research/2026-08-10-options-panel-structure.md</c> §1.5 — through the SAME
/// import machinery every other retained window uses, and appends it as one
/// scrollable row.
/// import machinery every other retained window uses, and appends it as one row.
///
/// <para>
/// Wraps a <see cref="UiScrollablePanel"/> as its single child rather than
/// duplicating its scroll logic — <see cref="UiScrollablePanel"/> is sealed, and this
/// is the SAME "controller-built row list" viewport pattern
/// <c>CharacterStatController.RebuildActiveList</c> already uses for the skill list
/// (a <see cref="UiScrollablePanel"/> child sized to its host, rows added through it).
/// <b>OP2 rework (2026-08-11):</b> derives from <see cref="UiDatElement"/> and stays
/// DORMANT until a controller adds its first row. The OP2 REJECT-review blast-radius
/// finding (`docs/research/2026-08-11-op2-review-blast.md`) established that EVERY
/// pre-existing Type-5 element reaching <see cref="DatWidgetFactory"/> already authors a
/// non-empty <c>0x64</c> template array (character `0x1000023D`/`0x10000532`, effects
/// positive/negative `0x10000123`, examine `0x10000149`/`0x10000335`/`0x1000032D`,
/// mini-game `0x10000174`, spellbook `0x10000464`) — so the original Type-5 factory
/// guard's premise ("none currently reach this factory") was false, and the old
/// unconditional viewport injection in the constructor (<c>base.AddChild(_viewport)</c>)
/// gave every one of those ten elements a spurious hit-testable
/// <see cref="UiScrollablePanel"/> child plus lost their authored media (the old class
/// derived from bare <see cref="UiPanel"/>, zeroed its background/border, and drew
/// nothing dat-authored). None of the ten pre-existing elements author real dat
/// children of their own (verified against every regenerated fixture — their row
/// content comes ONLY from the cross-layout <c>0x64</c> array, never from a literal
/// child in their own LayoutDesc subtree), so making this class dormant-by-default is a
/// pure behavior restoration: an un-activated instance renders and hit-tests EXACTLY
/// like the pre-OP2 <see cref="UiDatElement"/> fallback. The Options panel's three
/// ListBoxes (Character/Config/Chat, Campaign OP slice OP4+) are the only elements a
/// controller will ever call <see cref="AddItemFromTemplateList"/> against.
/// </para>
///
/// <para>
/// <see cref="ConsumesDatChildren"/> is <c>true</c>: retail ListBox rows come ONLY from
/// <c>AddItemFromTemplateList</c>, never from static dat children of the ListBox element
/// itself, so this widget owns its own child construction the same way
/// Meter/Menu/Button/Scrollbar/Text/Field do — <c>LayoutImporter</c> does not attempt to
/// recurse into any literal dat children this element might (incorrectly) author.
/// </para>
///
/// <para>
/// Row content is stacked inside a lazily-created <see cref="UiScrollablePanel"/> —
/// created on the FIRST successful <see cref="AddItemFromTemplateList"/> call, not in the
/// constructor. This is what makes dormancy free: an instance that never gets a row
/// (every pre-existing panel today) never allocates or attaches the viewport, so there is
/// no spurious hit-testable child competing with whatever the controller separately adds
/// via <see cref="UiElement.AddChild"/> (the same "resolve → not a UiItemList → build one
/// and attach it directly to the host" pattern <c>EffectsUiController</c>/
/// <c>AppraisalUiController</c>/<c>SpellbookWindowController</c>/
/// <c>CharacterStatController</c> already use for these exact ten elements).
/// <see cref="UiScrollablePanel"/> is sealed, hence composition rather than inheritance —
/// the SAME "controller-built row list" viewport pattern
/// <c>CharacterStatController.RebuildActiveList</c> already uses for the skill list.
/// The wrapped panel is anchored to fill this box, so scrolling, per-row visibility
/// clipping, and the pixel scroll model (<see cref="Scroll"/>) all come from the
/// SAME code CH6/the character sheet already exercise — no new scroll model. Rows
/// stack in call order: each new row's <see cref="UiElement.Top"/> is set to the
/// viewport's current <see cref="UiScrollablePanel.ContentHeight"/> before it is
/// added, exactly retail's own ListBox layout (each row is authored at its own
/// template-local Y=0; the box stacks instances).
/// clipping, and the pixel scroll model (<see cref="Scroll"/>) all come from the SAME
/// code CH6/the character sheet already exercise — no new scroll model. Rows stack in
/// call order: each new row's <see cref="UiElement.Top"/> is set to the viewport's
/// current <see cref="UiScrollablePanel.ContentHeight"/> before it is added, exactly
/// retail's own ListBox layout (each row is authored at its own template-local Y=0; the
/// box stacks instances).
/// </para>
///
/// <para>
@ -42,12 +78,15 @@ namespace AcDream.App.UI;
/// <c>IDatReaderWriter</c>.
/// </para>
/// </summary>
public sealed class UiTemplateListBox : UiPanel
public sealed class UiTemplateListBox : UiDatElement
{
private readonly UiScrollablePanel _viewport = new()
{
Anchors = AnchorEdges.Left | AnchorEdges.Top | AnchorEdges.Right | AnchorEdges.Bottom,
};
/// <summary>Retail element id this class was ported for: Type 5 = 5.</summary>
public const uint RetailTypeId = 5u;
private const int DefaultLineHeight = 16;
private UiScrollablePanel? _viewport;
private int _pendingLineHeight = DefaultLineHeight;
/// <summary>The authored row-template list (dat property 0x64), in authored array order.</summary>
public IReadOnlyList<UiTemplateListEntry> Templates { get; }
@ -63,19 +102,25 @@ public sealed class UiTemplateListBox : UiPanel
public uint ScrollbarElementId { get; }
/// <summary>The wrapped viewport's pixel scroll model — link a page controller's
/// resolved scrollbar (<see cref="ScrollbarElementId"/>) to this.</summary>
public UiScrollable Scroll => _viewport.Scroll;
/// resolved scrollbar (<see cref="ScrollbarElementId"/>) to this. Creates the
/// viewport on first access (see class doc — dormancy).</summary>
public UiScrollable Scroll => Viewport.Scroll;
/// <summary>Total stacked row height in px — the same value <see cref="Scroll"/>'s
/// content extent uses.</summary>
public int ContentHeight => _viewport.ContentHeight;
/// content extent uses. 0 while dormant (no viewport created yet).</summary>
public int ContentHeight => _viewport?.ContentHeight ?? 0;
/// <summary>Row height for the scroll model's line-scroll quantum. Set once template
/// heights are known; defaults to the viewport's own default (16px).</summary>
/// heights are known; defaults to the viewport's own default (16px). Safe to set
/// before the viewport exists — the value is applied once it's created.</summary>
public int LineHeight
{
get => _viewport.LineHeight;
set => _viewport.LineHeight = value;
get => _viewport?.LineHeight ?? _pendingLineHeight;
set
{
_pendingLineHeight = value;
if (_viewport is not null) _viewport.LineHeight = value;
}
}
/// <summary>
@ -85,21 +130,44 @@ public sealed class UiTemplateListBox : UiPanel
/// </summary>
public Func<uint, uint, UiElement?>? TemplateResolver { get; set; }
public UiTemplateListBox(IReadOnlyList<UiTemplateListEntry> templates, uint scrollbarElementId)
public UiTemplateListBox(
ElementInfo info,
Func<uint, (uint tex, int w, int h)> resolve,
IReadOnlyList<UiTemplateListEntry> templates,
uint scrollbarElementId)
: base(info, resolve)
{
Templates = templates;
ScrollbarElementId = scrollbarElementId;
BackgroundColor = Vector4.Zero;
BorderColor = Vector4.Zero;
base.AddChild(_viewport);
}
/// <summary>Retail ListBox rows never come from static dat children — see class doc.</summary>
public override bool ConsumesDatChildren => true;
private UiScrollablePanel Viewport
{
get
{
if (_viewport is null)
{
_viewport = new UiScrollablePanel
{
Anchors = AnchorEdges.Left | AnchorEdges.Top | AnchorEdges.Right | AnchorEdges.Bottom,
LineHeight = _pendingLineHeight,
};
base.AddChild(_viewport);
}
return _viewport;
}
}
/// <summary>
/// Retail <c>UIElement_ListBox::AddItemFromTemplateList(m_pOptionBox, index,
/// nullptr)</c>: resolves <c>Templates[index]</c> through <see cref="TemplateResolver"/>
/// and appends the built subtree as the next row, stacked below the previous one.
/// Returns the built row widget, or null when <paramref name="index"/> is out of
/// range, no resolver is wired, or the resolver produced nothing.
/// Lazily creates the internal viewport on the FIRST successful call (see class doc —
/// dormancy). Returns the built row widget, or null when <paramref name="index"/> is
/// out of range, no resolver is wired, or the resolver produced nothing.
/// </summary>
public UiElement? AddItemFromTemplateList(int index)
{
@ -111,9 +179,10 @@ public sealed class UiTemplateListBox : UiPanel
UiElement? row = resolver(entry.TemplateLayoutId, entry.TemplateElementId);
if (row is null) return null;
UiScrollablePanel viewport = Viewport;
row.Left = 0f;
row.Top = _viewport.ContentHeight;
_viewport.AddChild(row);
row.Top = viewport.ContentHeight;
viewport.AddChild(row);
return row;
}
}