fix(ui): Campaign CT5 fix round — Normal-state row media, geometry test, padding, doc corrections

Opus review of CT5 (f532f28c) found 0 blockers, 4 SHOULD-FIX, 6 NOTEs; all
applied here.

SHOULD-FIX 1 (visible retail gap): unselected attribute/skill rows now draw
the row template's Normal-state media (0x06004CC2 — pinned by
CharacterPanelLiveDatTests.AttributeRowTemplate_...) instead of drawing
nothing. Independently decoded against the installed DAT: PFID_A8R8G8B8,
48x48, uniform (0,0,0,175) — a ~69%-opaque black tile the native-size
copy-or-tile blit repeats across the row. Wired at all three sites
(HandleRowClick, ApplySkillSelectionVisuals, AddRow). Selected rows keep
0x06000F93 (RowHighlightSprite) unchanged.

SHOULD-FIX 2: added Bind_AttributeRow_/Bind_VitalRow_/Bind_SkillRow_
geometry tests asserting the authored template pixels against BUILT rows
(not DAT pins) — width, height, icon/name/value column positions. The
skill-row case reproduces the real production scrollbar (X=281, per
CharacterPanelLiveDatTests.StatListBox_AuthorsFiveRowTemplatesInSharedLayout)
to prove the documented 281px clamp (scrollbar.Left - list.Left), one pixel
short of the attribute/vital rows' 282px ceiling.

SHOULD-FIX 3: AddRow's name-column Padding corrected from 1f to 0f — the
authored template carries no margin on 0x1000012A; Padding=1f re-created
the X=26 glyph-start bug this slice existed to fix.

SHOULD-FIX 4: reworded both UiPanel.BackgroundSprite doc comments — the
draw is a native-size copy-or-tile blit (UV-repeat), never a stretch.
Decoded 0x06000F93 as exactly 282x20 (matches the row natively, draws as a
plain copy) vs 0x06004CC2's 48x48 tile. Retail's UIRegion::SetImageByDID
(@0x0069F960) decompiles to a pure BlitMode selector switch — param_2==2 ->
Blit_3Alpha, ==3 -> Blit_4Alpha, else Blit_Normal — with no width/height
touched anywhere in the function, answering CT1's open "draw mode 3"
question: it's an alpha-blend selector, not a resize flag.

NOTEs:
  a. AddRow's nameEl now sets OneLine=true so the authored VJustify=Center
     takes the same single-line vertical-centering path the value column
     already uses.
  b. Tempered the "row width is 282" wording in the SkillContentWidth /
     RowContentWidth doc comments — that's a ceiling attribute/vital rows
     land on, not a fact true for skill rows (281, via SkillViewportWidth's
     scrollbar-gutter measurement).
  c. Reworded the section-header (RowPadX) comment — CT1 verified only the
     four header SPRITES; the caption label's own authored margins were
     never checked. Recorded as an open residual, not a cleared divergence.
  d. AttrRows/VitalRows are now internal (InternalsVisibleTo("AcDream.App.Tests")
     already covers AcDream.App.Tests); CharacterPanelLiveDatTests iterates
     them directly instead of a re-typed duplicate array, and now also
     asserts the vitals 2/4/6 current-enum aliasing claim the doc comment
     made but never enforced.
  e. Deleted the stale pre-CT5 0x06001397 narrative in
     CharacterPanelLiveDatTests; the pin's comment now describes the
     post-CT5 state (a regression guard, not an open bug).
  f. Unified ApplySkillSelectionVisuals' selected-branch SpriteResolve
     wrapper closure with HandleRowClick's direct assignment.

Build green; full hermetic solution suite green (Release,
Lane!=InstalledDat&...&Status!=KnownFailure filter); InstalledDat lane
green (ACDREAM_RUN_INSTALLED_DAT_TESTS=1, Lane=InstalledDat&Status!=KnownFailure).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-25 01:47:25 +02:00
parent 657b84c297
commit 0a37a28e76
4 changed files with 303 additions and 75 deletions

View file

@ -173,18 +173,35 @@ public static class CharacterStatController
private const float RowValueW = 100f;
// Section-header caption inset ONLY (AddSkillHeader's 0x10000249..0x1000024C
// captions carry their own authored L5/R5 margins property, distinct from
// the data-row template above) — retained at its pre-CT5 value; CT1 found
// no divergence in the header captions, so this constant is out of CT5's
// scope.
// captions carry their own authored geometry, distinct from the data-row
// template above) — retained at its pre-CT5 value. CT5 fix round
// (NOTE c): CT1 verified only the FOUR SPRITES for these header templates
// (SkillSectionHeaderTemplates_MatchExistingSpriteConstants: Width=280,
// Height=20, sprite match) — the caption LABEL's own authored geometry
// (W=280 with L5/R5 margins per the header template, versus our current
// header width, which comes from RowContentWidth/SkillViewportWidth same
// as the data rows, plus this flat 4px inset) was never pinned or
// cross-checked against that. This is a KNOWN, RECORDED residual gap —
// NOT a cleared divergence — left for a future slice to pin the caption
// child's authored margins the same way CT1 pinned the data-row
// template's icon/name/value columns; behavior is unchanged here.
private const float RowPadX = 4f;
private const float SkillHeaderHeight = 20f;
// Authored row-template width (0x10000248 W=282) — CT5: the row's own
// width now comes from THIS authored constant directly rather than the
// ListBox's raw 300px Width (see RowContentWidth below); do not derive
// a "listWidth minus gutter" formula, per the ground-truth doc's
// explicit warning.
// Authored row-template CEILING width (0x10000248 W=282) — CT5: the
// row's own width comes from THIS authored constant as an upper bound,
// never the ListBox's raw 300px Width, and never a derived
// "listWidth minus gutter" formula (the ground-truth doc's explicit
// warning). CT5 fix round (NOTE b, tempered wording): this ceiling is
// NOT the row's width in every context. Attribute/vital rows land on it
// exactly (300px ListBox clamped to 282). The SKILL page instead first
// narrows the viewport to SkillViewportWidth's scrollbar-gutter
// measurement (scrollbar.Left list.Left = 281px in the real
// production layout — CharacterPanelLiveDatTests.
// StatListBox_AuthorsFiveRowTemplatesInSharedLayout pins the scrollbar
// at X=281), and RowContentWidth then clamps skill rows to THAT
// (281 < 282) rather than the raw 282 ceiling — see
// Bind_SkillRow_ClampsToAuthoredScrollbarGutterWidth.
private const float SkillContentWidth = 282f;
private const uint SkillHeaderSpecializedSprite = 0x06000F90u;
@ -204,6 +221,21 @@ public static class CharacterStatController
// — that file and its tests are correct and must NOT be touched.
private const uint RowHighlightSprite = 0x06000F93u;
// CT5 fix round (SHOULD-FIX 1, visible retail gap): the row template's
// NORMAL-state media — StateMedia[Normal] on 0x10000248, pinned byte-
// exact by CharacterPanelLiveDatTests.
// AttributeRowTemplate_IconIsFlushLeftTwentyPixels_NameAndValueAreFixedColumns
// (row.StateMedia["Normal"].File == 0x06004CC2). CT5 itself only wired
// the Highlight-state swap above; UNSELECTED rows drew fully
// transparent instead of this authored background. Independently
// decoded against the installed DAT (2026-08-25): PFID_A8R8G8B8, 48x48,
// a single uniform color (0,0,0,175) — i.e. a ~69%-opaque (175/255)
// flat black tile that retail's native-size copy-or-tile blit repeats
// across the row (see UiPanel.BackgroundSprite's doc comment for the
// tiling mechanism) to produce the dark band under every unselected
// attribute/skill row. Selected rows keep RowHighlightSprite above.
private const uint RowNormalSprite = 0x06004CC2u;
// CT5 (AP-235 unification, gmAttributeUI::PostInit @0x0049DB70 verbatim):
// per-attribute icon DIDs resolve via DBObj::GetDIDByEnum(statEnum,
// category 0x10000002); per-vital (Attribute2ndInfoRegion) icon DIDs via
@ -255,7 +287,13 @@ public static class CharacterStatController
private sealed record SkillRowBinding(UiClickablePanel Panel, CharacterSkill Skill);
// ── Attribute row descriptors — retail display order per spec §1 ─────────
private static readonly (string name, uint iconDid, uint statId)[] AttrRows = new[]
// CT5 fix round (NOTE d): internal (not private) so
// CharacterPanelLiveDatTests.AttributeAndVitalIconDids_MatchTheRetailEnumMapperChain
// can iterate THESE tuples directly instead of a re-typed duplicate
// literal array — a divergence between the two would previously have
// gone undetected. InternalsVisibleTo("AcDream.App.Tests") already
// covers this assembly (AcDream.App.csproj).
internal static readonly (string name, uint iconDid, uint statId)[] AttrRows = new[]
{
("Strength", 0x060002C8u, 1u),
("Endurance", 0x060002C4u, 2u),
@ -265,7 +303,8 @@ public static class CharacterStatController
("Self", 0x060002C7u, 6u),
};
private static readonly (string name, uint iconDid, uint maxStatId)[] VitalRows = new[]
// CT5 fix round (NOTE d): internal for the same reason as AttrRows above.
internal static readonly (string name, uint iconDid, uint maxStatId)[] VitalRows = new[]
{
("Health", 0x06004C3Bu, 1u), // max enum 1; current enum 2
("Stamina", 0x06004C3Cu, 3u), // max enum 3; current enum 4
@ -891,12 +930,20 @@ public static class CharacterStatController
}
/// <summary>
/// CT5: the row's own rendered width is the AUTHORED row-template width
/// (282px, <see cref="SkillContentWidth"/>), never the ListBox's raw
/// container width (300px, <see cref="CharacterStatController.ListBoxId"/>'s
/// own dat rect) — the two numbers do not compose into a "gutter" formula
/// (ground-truth doc §2's explicit warning); a shorter container clamps
/// the row down, but a wider one never stretches it past 282.
/// CT5: the row's own rendered width is <paramref name="list"/>'s own
/// width clamped to the AUTHORED row-template CEILING
/// (282px, <see cref="SkillContentWidth"/>) — the two numbers never
/// compose into a derived "gutter" formula (ground-truth doc §2's
/// explicit warning); a shorter <paramref name="list"/> clamps the row
/// down, but a wider one never stretches it past 282.
/// <para>CT5 fix round (NOTE b, tempered wording): this does NOT mean
/// every row is 282px. Attribute/vital rows pass the raw ListBox
/// (300px, <see cref="CharacterStatController.ListBoxId"/>'s own dat
/// rect) and land on the 282 ceiling exactly. The skill page instead
/// passes the narrower <c>SkillViewportWidth</c>-computed viewport
/// (281px in production — the real scrollbar's authored gutter), so
/// skill rows clamp to 281, one pixel short of the template ceiling.
/// </para>
/// </summary>
private static float RowContentWidth(UiElement list)
=> list.Width > 0f ? MathF.Min(list.Width, SkillContentWidth) : SkillContentWidth;
@ -1231,6 +1278,10 @@ public static class CharacterStatController
// to the template's Highlight-state media (RowHighlightSprite,
// 0x06000F93 — see its own doc comment) when spriteResolve is
// available; otherwise fall back to the translucent gold tint.
// CT5 fix round (SHOULD-FIX 1): the UNSELECTED branch now draws the
// template's Normal-state media (RowNormalSprite, 0x06004CC2 — see
// its own doc comment) the same way, instead of leaving the row
// fully transparent.
for (int i = 0; i < rows.Count; i++)
{
var row = rows[i];
@ -1252,8 +1303,8 @@ public static class CharacterStatController
else
{
row.BackgroundColor = Vector4.Zero;
row.BackgroundSprite = 0u;
row.SpriteResolve = null;
row.BackgroundSprite = spriteResolve is not null ? RowNormalSprite : 0u;
row.SpriteResolve = spriteResolve;
}
}
@ -1296,7 +1347,12 @@ public static class CharacterStatController
{
row.BackgroundColor = Vector4.Zero;
row.BackgroundSprite = RowHighlightSprite;
row.SpriteResolve = id => { var (h, w, ht) = spriteResolve(id); return (h, w, ht); };
// CT5 fix round (NOTE f): unified with HandleRowClick's
// direct assignment — the per-row wrapper closure this
// used to allocate was functionally identical (same
// tuple shape, just differently-named elements, which
// the delegate conversion already accepts without it).
row.SpriteResolve = spriteResolve;
}
else
{
@ -1307,9 +1363,12 @@ public static class CharacterStatController
}
else
{
// CT5 fix round (SHOULD-FIX 1): draw the Normal-state media
// (RowNormalSprite) on unselected rows — see HandleRowClick's
// matching comment for the full citation.
row.BackgroundColor = Vector4.Zero;
row.BackgroundSprite = 0u;
row.SpriteResolve = null;
row.BackgroundSprite = spriteResolve is not null ? RowNormalSprite : 0u;
row.SpriteResolve = spriteResolve;
}
}
}
@ -1754,9 +1813,14 @@ public static class CharacterStatController
Top = top,
Width = width,
Height = height,
BackgroundColor = Vector4.Zero, // transparent until selected
BackgroundSprite = 0u,
SpriteResolve = null,
BackgroundColor = Vector4.Zero,
// CT5 fix round (SHOULD-FIX 1): a freshly-built row starts
// unselected, so it gets the template's Normal-state media
// (RowNormalSprite) up front — matching HandleRowClick's/
// ApplySkillSelectionVisuals' unselected branch exactly, so a
// row never flashes transparent before its first click.
BackgroundSprite = spriteResolve is not null ? RowNormalSprite : 0u,
SpriteResolve = spriteResolve,
BorderColor = Vector4.Zero,
Anchors = AnchorEdges.Left | AnchorEdges.Top,
};
@ -1793,7 +1857,21 @@ public static class CharacterStatController
ClickThrough = true,
Centered = false,
RightAligned = false,
Padding = 1f,
// CT5 fix round (SHOULD-FIX 3): the authored template's
// 0x1000012A name column carries no margin property at all —
// Padding=1f re-created the X=26 glyph start (RowNameX + 1) this
// very slice existed to remove (see AttributeRowTemplate_...'s
// pin: name.X == 25f, no margin). 0f matches the value column's
// own (already-0) Padding.
Padding = 0f,
// CT5 fix round (NOTE a): both the name and value columns author
// VJustify=Center in the template (0x1000012A/0x1000012B share
// the same default UiText.VerticalJustify). OneLine=true routes
// this element through the same single-line vertical-centering
// draw path the value column below already uses instead of the
// multi-line/scroll path, which does not honor VerticalJustify
// the same way.
OneLine = true,
Anchors = AnchorEdges.Left | AnchorEdges.Top,
};
nameEl.LinesProvider = () => new[] { new UiText.Line(capturedName, capturedNameColor) };

View file

@ -26,10 +26,35 @@ public class UiPanel : UiElement
/// <summary>Optional dat RenderSurface id for the panel background sprite, drawn
/// in place of (or alongside) <see cref="BackgroundColor"/>. 0 = none.
/// When set, the sprite is stretched to fill the panel rect — the same
/// full-swap semantics retail's <c>InfoRegion::SetState</c> uses. Used by
/// the character-panel attribute/skill row's selected-row highlight
/// (sprite 0x06000F93, template 0x10000248's Highlight-state media).</summary>
/// <para>CT5 fix round (SHOULD-FIX 4): drawn at NATIVE SIZE — retail's
/// copy-or-tile blit, never scaled (see <c>UiDatElement.OnDraw</c>'s own
/// doc comment, ~lines 273-360, for the full ground truth). The UV
/// rectangle below (<c>Width / tw, Height / th</c>) is deliberately
/// UV-REPEAT, not a fixed 0..1 a stretch would use — GL_REPEAT-wrapped
/// UI textures tile past their native pixel size rather than scaling.
/// Retail's generic sprite blit (<c>Graphic::Draw</c> 0x00693b20 /
/// <c>Graphic::PutImage</c> 0x00693a30) has exactly two behaviors, copy
/// or tile, and can never scale a source image up to fill a larger
/// destination.</para>
/// <para>Used by the character-panel attribute/skill row's Highlight-state
/// swap (sprite 0x06000F93, template 0x10000248's Highlight-state
/// media) and CT5's Normal-state row background (0x06004CC2). The two
/// sprites behave differently under this same tile formula: 0x06000F93
/// is authored at exactly the row's own 282x20 native size (decoded
/// against the installed DAT, 2026-08-25: PFID_R8G8B8, 282x20), so it
/// draws as a plain COPY with no visible seam; 0x06004CC2 is a 48x48
/// uniform-color tile (PFID_A8R8G8B8, single color (0,0,0,175)) that
/// visibly TILES across the wider row — both are the same code path,
/// just different source-vs-destination ratios.</para>
/// <para>Retail's own <c>UIRegion::SetImageByDID</c> (@0x0069F960)
/// confirms its third parameter is a <c>BlitMode</c> COLOR-BLEND
/// selector, not a resize flag — the decompiled body switches purely on
/// that value (<c>param_2 == 2</c> -&gt; <c>Blit_3Alpha</c>,
/// <c>== 3</c> -&gt; <c>Blit_4Alpha</c>, else <c>Blit_Normal</c>) and
/// never touches width/height at all. This answers CT1's open "icon
/// draw mode 3" question (ground-truth doc §2): mode 3 selects
/// <c>Blit_4Alpha</c>, an alpha-blend variant, not a resize.</para>
/// </summary>
public uint BackgroundSprite { get; set; }
/// <summary>Resolves a dat RenderSurface id to (GL tex handle, pixel width, pixel height).
@ -195,14 +220,17 @@ public class UiSimpleButton : UiPanel
///
/// <para>Campaign CT slice CT5 (2026-08-25): the selected-row highlight draws through the
/// inherited <see cref="UiPanel.OnDraw"/> full-panel <see cref="UiPanel.BackgroundSprite"/>
/// stretch — no override here. This class previously had its own "selection bars" draw
/// mode (a thin top/bottom-bar rendering tuned to look like sprite 0x06001397's dark
/// bars); CT1's ground-truth research found that sprite belongs to a DIFFERENT retail
/// mechanism entirely (the spellbook row's overlay child), and the row's actual retail
/// Highlight state (<c>InfoRegion::SetState</c>, media 0x06000F93) is a plain full-row
/// background SWAP — exactly what the inherited <see cref="UiPanel.OnDraw"/> already
/// draws. The bars mode was therefore retired rather than reconfigured to a wrong sprite's
/// geometry; no consumer outside <c>CharacterStatController</c> ever set it.</para>
/// draw (native-size copy-or-tile — see that property's own doc comment for the CT5 fix
/// round's correction of the mechanism; NOT a stretch) — no override here. This class
/// previously had its own "selection bars" draw mode (a thin top/bottom-bar rendering
/// tuned to look like sprite 0x06001397's dark bars); CT1's ground-truth research found
/// that sprite belongs to a DIFFERENT retail mechanism entirely (the spellbook row's
/// overlay child), and the row's actual retail Highlight state (<c>InfoRegion::SetState</c>,
/// media 0x06000F93) is a plain full-row background SWAP — exactly what the inherited
/// <see cref="UiPanel.OnDraw"/> already draws (0x06000F93 is authored at exactly the row's
/// own 282x20 native size, so the swap needs no scaling to look right). The bars mode was
/// therefore retired rather than reconfigured to a wrong sprite's geometry; no consumer
/// outside <c>CharacterStatController</c> ever set it.</para>
/// </summary>
public class UiClickablePanel : UiPanel
{