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

fix(vt): list column fix round 8/11 — shared UiCheckLamp + factory-only column

Browse source 
New internal static UiCheckLamp (LampSize=11f, the four checked/
unchecked colors, Draw) is the ONE definition of the five-band lamp
glyph, promoted out of UiMarkupToggle's private/internal fields —
UiMarkupToggle.OnDraw and UiMarkupList.DrawCheckCell both call
UiCheckLamp.Draw now instead of each carrying (or one exposing to the
other) its own copy of the same five DrawFill calls and four colors.

DrawCheckCell also centers the lamp horizontally in its cell instead of
drawing it flush left at cellX+1 — matching DrawIconCell, which already
centers its sprite. A check column declared wider than the glyph itself
(routine under fix item 11's PITCH-based authoring convention) no longer
strands the glyph in the cell's left edge.

UiMarkupListColumn's settable members are now internal init (Kind/Width
lost their `required` modifier — C# forbids `required` pairing with a
setter less visible than the containing public type, CS9032 — every
factory already sets both unconditionally, so this is a compiler-level
demotion, not a behavior change) — the type is constructible only
through its Text/Check/Icon factories from any external assembly (a
plugin) with no InternalsVisibleTo grant, so it can never assemble an
inconsistent instance via object-initializer syntax.

Replaced the inert draw-offset assertion in
Columns_CheckThenIcon_EachCellDrawsInsideItsOwnColumnBounds (check
column FIRST, so its own-cell assertion held trivially even with
completely broken column offsets) with
Columns_IconThenCheck_EachCellDrawsInsideItsOwnColumnBounds — check
column now SECOND, so the assertion can only pass if the glyph actually
moved into its own [20,100) cell.

New test: a single 50px-wide check column's glyph lands near the cell's
midpoint (~22.5) rather than the old flush-left x=4 — shown to fail
first before the centering change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-06 21:16:00 +02:00
parent f8e5edb848
commit 8fb4909cc8
5 changed files with 140 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

41
src/AcDream.App/UI/UiCheckLamp.cs Normal file
View file

@ -0,0 +1,41 @@
using System.Numerics;
namespace AcDream.App.UI;
/// <summary>
/// Fix round item 8: the five-band lamp glyph checkbox primitive, promoted
/// out of <see cref="UiMarkupToggle"/> into its own shared static so BOTH
/// <see cref="UiMarkupToggle"/> (the standalone <c>&lt;toggle&gt;</c>
/// element) and <see cref="UiMarkupList"/>'s <c>&lt;column type="check"&gt;</c>
/// cell draw the IDENTICAL glyph from ONE definition rather than two
/// independently-maintained copies of the same five <c>DrawFill</c> calls
/// and four colors.
/// </summary>
internal static class UiCheckLamp
{
/// <summary>The lamp glyph's fixed on-screen size in px (both axes).</summary>
public const float LampSize = 11f;
public static readonly Vector4 CheckedOuter = new(0.36f, 0.58f, 0.12f, 1f);
public static readonly Vector4 CheckedInner = new(0.52f, 1f, 0.08f, 1f);
public static readonly Vector4 UncheckedOuter = new(0.26f, 0.22f, 0.13f, 1f);
public static readonly Vector4 UncheckedInner = new(0.38f, 0.34f, 0.23f, 1f);
/// <summary>
/// Draws the lamp with its top-left at <paramref name="x"/>,
/// <paramref name="y"/> — a <see cref="LampSize"/> x <see cref="LampSize"/>
/// footprint. Five bands form the small circular indicator without
/// introducing a plugin bitmap or a new renderer primitive.
/// </summary>
public static void Draw(UiRenderContext ctx, float x, float y, bool isChecked)
{
Vector4 outer = isChecked ? CheckedOuter : UncheckedOuter;
Vector4 inner = isChecked ? CheckedInner : UncheckedInner;
ctx.DrawFill(x + 3f, y, 5f, 1f, outer);
ctx.DrawFill(x + 1f, y + 1f, 9f, 2f, outer);
ctx.DrawFill(x, y + 3f, 11f, 5f, outer);
ctx.DrawFill(x + 1f, y + 8f, 9f, 2f, outer);
ctx.DrawFill(x + 3f, y + 10f, 5f, 1f, outer);
ctx.DrawFill(x + 3f, y + 3f, 5f, 5f, inner);
}
}

25
src/AcDream.App/UI/UiMarkupList.cs
View file

@ -349,7 +349,7 @@ public sealed class UiMarkupList : UiElement
DrawTextCell(context, _cachedTextRows[c], _cachedColorRows[c], index, cellX, y);
break;
case UiMarkupListColumnKind.Check:
DrawCheckCell(context, _cachedCheckRows[c], index, cellX, y);
DrawCheckCell(context, _cachedCheckRows[c], index, cellX, cellW, y);
break;
case UiMarkupListColumnKind.Icon:
DrawIconCell(context, columns[c], _cachedIconRows[c], index, cellX, cellW, y);
@ -381,10 +381,15 @@ public sealed class UiMarkupList : UiElement
}
/// <summary>
/// Draws the same five-band lamp glyph <see cref="UiMarkupToggle"/> uses,
/// so a check column reads exactly like every other checkbox in the
/// client (contract requirement: reuse the toggle's own primitives rather
/// than a bespoke box-and-tick).
/// Draws the same five-band lamp glyph <see cref="UiMarkupToggle"/> uses
/// (both now share <see cref="UiCheckLamp"/>'s one definition — fix round
/// item 8), so a check column reads exactly like every other checkbox in
/// the client (contract requirement: reuse the toggle's own primitive
/// rather than a bespoke box-and-tick). Centered horizontally in its
/// cell, matching how <see cref="DrawIconCell"/> already centers its
/// sprite — a check cell is not always as narrow as the glyph itself
/// (fix round item 11's PITCH-based authoring convention routinely
/// declares check columns wider than <see cref="UiCheckLamp.LampSize"/>).
///
/// <para>
/// Fix round item 7: unlike text/icon cells (which draw nothing past
@ -398,13 +403,13 @@ public sealed class UiMarkupList : UiElement
/// </para>
/// </summary>
private void DrawCheckCell(
UiRenderContext context, IReadOnlyList<bool>? flags, int index, float cellX, float y)
UiRenderContext context, IReadOnlyList<bool>? flags, int index, float cellX, float cellW, float y)
{
bool isChecked = flags is not null && index < flags.Count && flags[index];
Vector4 outer = isChecked ? UiMarkupToggle.CheckedOuter : UiMarkupToggle.UncheckedOuter;
Vector4 inner = isChecked ? UiMarkupToggle.CheckedInner : UiMarkupToggle.UncheckedInner;
UiMarkupToggle.DrawLamp(
context, cellX + 1f, y + MathF.Max(1f, (RowHeight - 11f) * 0.5f), outer, inner);
float extent = MathF.Max(0f, cellW - 2f);
float lampX = cellX + 1f + MathF.Max(0f, extent - UiCheckLamp.LampSize) * 0.5f;
float lampY = y + MathF.Max(1f, (RowHeight - UiCheckLamp.LampSize) * 0.5f);
UiCheckLamp.Draw(context, lampX, lampY, isChecked);
}
private void DrawIconCell(

38
src/AcDream.App/UI/UiMarkupListColumn.cs
View file

@ -32,29 +32,45 @@ public enum UiMarkupListColumnKind
/// column-layout helper, which recomputes this dynamically off the list's
/// live <c>Width</c> rather than baking it in at Build.
/// </para>
///
/// <para>
/// Fix round item 8: every settable member is <c>internal init</c> — this
/// type is constructible only through its <see cref="Text"/>/
/// <see cref="Check"/>/<see cref="Icon"/> factories. A plugin (an external
/// assembly with no <c>InternalsVisibleTo</c> grant) can never assemble an
/// inconsistent instance (e.g. <see cref="Kind"/> Text with
/// <see cref="CheckChanged"/> set) via object-initializer syntax; only the
/// three factories, which each set exactly the fields their own kind uses,
/// can construct one.
/// </para>
/// </summary>
public sealed class UiMarkupListColumn
{
public required UiMarkupListColumnKind Kind { get; init; }
public required float Width { get; init; }
// Fix round item 8: `required` cannot pair with a setter less visible
// than the type itself (CS9032) — since Kind/Width are now internal
// init, every factory sets both unconditionally instead (compiler
// enforcement moves from "required" to "the only three call sites all
// do it").
public UiMarkupListColumnKind Kind { get; internal init; }
public float Width { get; internal init; }
/// <summary>
/// Fix round item 2: <c>width="*"</c> — this column shares the list's
/// remaining width equally with every other auto column (the last
/// column in the list is ALWAYS treated as auto regardless of this flag
/// or its own declared <see cref="Width"/> — see the class doc above).
/// </summary>
public bool IsAutoWidth { get; init; }
public bool IsAutoWidth { get; internal init; }
// ── text ──────────────────────────────────────────────────────────────
/// <summary><c>&lt;column type="text" items="{IReadOnlyList&lt;string&gt;}"&gt;</c>.</summary>
public Func<IReadOnlyList<string>>? TextSource { get; init; }
public Func<IReadOnlyList<string>>? TextSource { get; internal init; }
/// <summary>
/// Optional per-row text color override, mirroring the single-column
/// list's own <c>colors</c> attribute. Null (the default, when the
/// column has no <c>colors</c> attribute at all) means every row in this
/// column draws with the list's <see cref="UiMarkupList.TextColor"/>.
/// </summary>
public Func<IReadOnlyList<uint>>? ColorsSource { get; init; }
public Func<IReadOnlyList<uint>>? ColorsSource { get; internal init; }
/// <summary>
/// Fix round finding 1: optional <c>onclick="{Action&lt;int&gt;}"</c> on a
/// text column. Fired with the ROW INDEX on a click anywhere in the cell
@ -66,22 +82,22 @@ public sealed class UiMarkupListColumn
/// target — but the select-on-click default stays for any acdream markup
/// that already relies on it.
/// </summary>
public Action<int>? TextClicked { get; init; }
public Action<int>? TextClicked { get; internal init; }
// ── check ─────────────────────────────────────────────────────────────
/// <summary><c>&lt;column type="check" values="{IReadOnlyList&lt;bool&gt;}"&gt;</c>.</summary>
public Func<IReadOnlyList<bool>>? CheckSource { get; init; }
public Func<IReadOnlyList<bool>>? CheckSource { get; internal init; }
/// <summary>
/// Fired with the ROW INDEX on a click anywhere in this cell — the
/// plugin flips its own bool; the column never mutates
/// <see cref="CheckSource"/>'s backing collection itself. Required (a
/// check column with no <c>onchange</c> throws at Build).
/// </summary>
public Action<int>? CheckChanged { get; init; }
public Action<int>? CheckChanged { get; internal init; }
// ── icon ──────────────────────────────────────────────────────────────
/// <summary><c>&lt;column type="icon" values="{IReadOnlyList&lt;uint&gt;}"&gt;</c> — one icon id per row.</summary>
public Func<IReadOnlyList<uint>>? IconValuesSource { get; init; }
public Func<IReadOnlyList<uint>>? IconValuesSource { get; internal init; }
/// <summary>
/// Resolves one <see cref="IconValuesSource"/> entry to a drawable icon,
/// dispatched by the column's own <c>iconkind</c> — built by
@ -90,12 +106,12 @@ public sealed class UiMarkupListColumn
/// the host (the column then draws no icons, matching every other
/// Slice-B icon sink's "no resolver → draws nothing" rule).
/// </summary>
public Func<uint, (uint tex, int w, int h)>? IconResolve { get; init; }
public Func<uint, (uint tex, int w, int h)>? IconResolve { get; internal init; }
/// <summary>
/// Fired with the ROW INDEX on a click anywhere in this cell. Required
/// (an icon column with no <c>onclick</c> throws at Build).
/// </summary>
public Action<int>? IconClicked { get; init; }
public Action<int>? IconClicked { get; internal init; }
public static UiMarkupListColumn Text(
float width,

39
src/AcDream.App/UI/UiMarkupToggle.cs
View file

@ -8,19 +8,6 @@ namespace AcDream.App.UI;
/// </summary>
public sealed class UiMarkupToggle : UiElement
{
// Internal (not private): Campaign VT slice 1 Part B's <list><column
// type="check"> cell draws the exact same five-band lamp glyph so a
// column checkbox looks like every other checkbox in the client — see
// UiMarkupList.DrawCheckCell.
internal static readonly Vector4 CheckedOuter =
new(0.36f, 0.58f, 0.12f, 1f);
internal static readonly Vector4 CheckedInner =
new(0.52f, 1f, 0.08f, 1f);
internal static readonly Vector4 UncheckedOuter =
new(0.26f, 0.22f, 0.13f, 1f);
internal static readonly Vector4 UncheckedInner =
new(0.38f, 0.34f, 0.23f, 1f);
public string Text { get; set; } = string.Empty;
public Func<string?>? TextSource { get; set; }
public Func<bool>? CheckedSource { get; set; }
@ -43,9 +30,10 @@ public sealed class UiMarkupToggle : UiElement
protected override void OnDraw(UiRenderContext ctx)
{
Vector4 outer = IsChecked ? CheckedOuter : UncheckedOuter;
Vector4 inner = IsChecked ? CheckedInner : UncheckedInner;
DrawLamp(ctx, 1f, MathF.Max(1f, (Height - 11f) * 0.5f), outer, inner);
// Fix round item 8: the lamp glyph is now the SHARED
// UiCheckLamp.Draw primitive — UiMarkupList's <column type="check">
// cell draws the exact same glyph from the same one definition.
UiCheckLamp.Draw(ctx, 1f, MathF.Max(1f, (Height - UiCheckLamp.LampSize) * 0.5f), IsChecked);
string caption = TextSource?.Invoke() ?? Text;
Vector4 color = Enabled
@ -59,23 +47,4 @@ public sealed class UiMarkupToggle : UiElement
else
ctx.DrawString(caption, 17f, y, color);
}
// Internal (not private): shared with UiMarkupList's <column type="check">
// cell draw — see the field comments above.
internal static void DrawLamp(
UiRenderContext ctx,
float x,
float y,
Vector4 outer,
Vector4 inner)
{
// Five bands form the small circular indicator without introducing a
// plugin bitmap or a new renderer primitive.
ctx.DrawFill(x + 3f, y, 5f, 1f, outer);
ctx.DrawFill(x + 1f, y + 1f, 9f, 2f, outer);
ctx.DrawFill(x, y + 3f, 11f, 5f, outer);
ctx.DrawFill(x + 1f, y + 8f, 9f, 2f, outer);
ctx.DrawFill(x + 3f, y + 10f, 5f, 1f, outer);
ctx.DrawFill(x + 3f, y + 3f, 5f, 5f, inner);
}
}

72
tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs
View file

@ -601,14 +601,16 @@ public sealed class MarkupListColumnsTests
}
[Fact]
public void Columns_CheckThenIcon_EachCellDrawsInsideItsOwnColumnBounds()
public void Columns_IconThenCheck_EachCellDrawsInsideItsOwnColumnBounds()
{
// col0 = check, width 20; col1 (last) = icon, absorbs the remainder
// of a 100-wide list (80px). Both columns render row 0; the check
// glyph's inner-lamp fill (a distinct, deliberately non-default color)
// and the icon sprite must each stay within their own [cellX, cellX+w)
// window — proving the column offsets actually took effect, not just
// that both draw somewhere.
// col0 = icon, width 20; col1 (last) = check, absorbs the remainder
// of a 100-wide list (80px). Fix round item 8: the check column is
// now SECOND (not first, as the original draft had it) — a check
// glyph in col0's [0,20) position would satisfy a ">=0 and <20"
// assertion trivially even if the column-offset math were completely
// broken (e.g. everything drawing at x=0); putting it second makes
// the assertion a REAL pin — it can only pass if the glyph actually
// moved to col1's [20,100) cell.
var list = new UiMarkupList
{
Width = 100f, Height = 40f, RowHeight = 18f,
@ -616,9 +618,9 @@ public sealed class MarkupListColumnsTests
BackgroundColor = default, BorderColor = default,
Columns = new[]
{
UiMarkupListColumn.Check(20f, () => new[] { true }, _ => { }),
UiMarkupListColumn.Icon(
20f, () => new uint[] { 9u }, id => (id, 16, 16), _ => { }),
UiMarkupListColumn.Check(20f, () => new[] { true }, _ => { }),
},
};
var (renderer, ctx) = MakeContext(200f, 200f);
@ -632,15 +634,15 @@ public sealed class MarkupListColumnsTests
// exactly one match.
var checkVertex = renderer.DebugSpriteSegmentVerts
.SelectMany(s => Chunk(s.Verts))
.First(v => ColorMatches(v, UiMarkupToggle.CheckedInner));
Assert.True(checkVertex[0] >= 0f && checkVertex[0] < 20f,
$"expected the check glyph inside [0,20), got x={checkVertex[0]}");
.First(v => ColorMatches(v, UiCheckLamp.CheckedInner));
Assert.True(checkVertex[0] >= 20f - 0.01f && checkVertex[0] < 100f,
$"expected the check glyph inside [20,100), got x={checkVertex[0]}");
// The icon column drew a real (non-zero-width) sprite quad on its
// own resolved texture (9u), entirely at or past x=20 (col1's start).
// own resolved texture (9u), entirely inside col0's [0,20) cell.
var iconQuad = Assert.Single(renderer.DebugSpriteSegmentVerts, s => s.Texture == 9u);
Assert.True(iconQuad.Verts[0] >= 20f - 0.01f,
$"expected the icon column's sprite at or past x=20, got x={iconQuad.Verts[0]}");
Assert.True(iconQuad.Verts[0] < 20f,
$"expected the icon column's sprite inside [0,20), got x={iconQuad.Verts[0]}");
float iconWidth = iconQuad.Verts[8] - iconQuad.Verts[0];
Assert.True(iconWidth > 0f, $"expected a non-zero icon quad width, got {iconWidth}");
}
@ -663,11 +665,11 @@ public sealed class MarkupListColumnsTests
list.DrawSelfAndChildren(ctx);
var quads = renderer.DebugSpriteSegmentVerts.SelectMany(s => Chunk(s.Verts)).ToList();
Assert.Contains(quads, v => ColorMatches(v, UiMarkupToggle.CheckedInner));
Assert.Contains(quads, v => ColorMatches(v, UiMarkupToggle.UncheckedInner));
Assert.Contains(quads, v => ColorMatches(v, UiCheckLamp.CheckedInner));
Assert.Contains(quads, v => ColorMatches(v, UiCheckLamp.UncheckedInner));
// Row 0's checked glyph sits above row 1's unchecked glyph.
var checkedY = quads.First(v => ColorMatches(v, UiMarkupToggle.CheckedInner))[1];
var uncheckedY = quads.First(v => ColorMatches(v, UiMarkupToggle.UncheckedInner))[1];
var checkedY = quads.First(v => ColorMatches(v, UiCheckLamp.CheckedInner))[1];
var uncheckedY = quads.First(v => ColorMatches(v, UiCheckLamp.UncheckedInner))[1];
Assert.True(uncheckedY > checkedY,
$"expected row 1's glyph ({uncheckedY}) below row 0's ({checkedY})");
}
@ -699,7 +701,7 @@ public sealed class MarkupListColumnsTests
var uncheckedQuads = renderer.DebugSpriteSegmentVerts
.SelectMany(s => Chunk(s.Verts))
.Where(v => ColorMatches(v, UiMarkupToggle.UncheckedInner))
.Where(v => ColorMatches(v, UiCheckLamp.UncheckedInner))
.ToList();
// One unchecked lamp for row 1 (bool value false — not this test's
// case) never applies here since col1's only row (0) is TRUE; rows 1
@ -709,6 +711,38 @@ public sealed class MarkupListColumnsTests
+ $"1-row data), got {uncheckedQuads.Count} unchecked lamp quad(s)");
}
// ── Fix round: shared UiCheckLamp, centered check glyph (fix item 8) ────
[Fact]
public void CheckColumn_GlyphIsHorizontallyCenteredInItsCell_LikeTheIconCellAlreadyIs()
{
// A single (last) check column absorbing a 50px-wide list: the old
// flush-left placement put the lamp's inner fill at x=4 regardless
// of cell width; centered (like DrawIconCell already centers its
// sprite) it must sit near the cell's midpoint instead.
var list = new UiMarkupList
{
Width = 50f, Height = 20f, RowHeight = 18f,
SelectedIndexSource = () => -1,
BackgroundColor = default, BorderColor = default,
Columns = new[]
{
UiMarkupListColumn.Check(50f, () => new[] { true }, _ => { }),
},
};
var (renderer, ctx) = MakeContext(200f, 200f);
list.DrawSelfAndChildren(ctx);
var innerVertex = renderer.DebugSpriteSegmentVerts
.SelectMany(s => Chunk(s.Verts))
.First(v => ColorMatches(v, UiCheckLamp.CheckedInner));
// Flush-left would put this at x=4; centered in a 50px cell (extent
// 48, lamp size 11) it lands at 1 + (48-11)/2 + 3 = 22.5.
Assert.True(innerVertex[0] > 15f,
$"expected the check glyph centered in its 50px cell (~22.5), got x={innerVertex[0]}");
}
[Fact]
public void IconColumn_DrawsResolvedRowIcon_AndSkipsAMissingOne()
{