diff --git a/src/AcDream.App/UI/UiCheckLamp.cs b/src/AcDream.App/UI/UiCheckLamp.cs new file mode 100644 index 000000000..4f78e8f2b --- /dev/null +++ b/src/AcDream.App/UI/UiCheckLamp.cs @@ -0,0 +1,41 @@ +using System.Numerics; + +namespace AcDream.App.UI; + +/// +/// Fix round item 8: the five-band lamp glyph checkbox primitive, promoted +/// out of into its own shared static so BOTH +/// (the standalone <toggle> +/// element) and 's <column type="check"> +/// cell draw the IDENTICAL glyph from ONE definition rather than two +/// independently-maintained copies of the same five DrawFill calls +/// and four colors. +/// +internal static class UiCheckLamp +{ + /// The lamp glyph's fixed on-screen size in px (both axes). + 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); + + /// + /// Draws the lamp with its top-left at , + /// — a x + /// footprint. Five bands form the small circular indicator without + /// introducing a plugin bitmap or a new renderer primitive. + /// + 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); + } +} diff --git a/src/AcDream.App/UI/UiMarkupList.cs b/src/AcDream.App/UI/UiMarkupList.cs index ae28b1949..09dffb6f0 100644 --- a/src/AcDream.App/UI/UiMarkupList.cs +++ b/src/AcDream.App/UI/UiMarkupList.cs @@ -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 } /// - /// Draws the same five-band lamp glyph 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 uses + /// (both now share '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 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 ). /// /// /// Fix round item 7: unlike text/icon cells (which draw nothing past @@ -398,13 +403,13 @@ public sealed class UiMarkupList : UiElement /// /// private void DrawCheckCell( - UiRenderContext context, IReadOnlyList? flags, int index, float cellX, float y) + UiRenderContext context, IReadOnlyList? 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( diff --git a/src/AcDream.App/UI/UiMarkupListColumn.cs b/src/AcDream.App/UI/UiMarkupListColumn.cs index 62864deab..0ba418bf7 100644 --- a/src/AcDream.App/UI/UiMarkupListColumn.cs +++ b/src/AcDream.App/UI/UiMarkupListColumn.cs @@ -32,29 +32,45 @@ public enum UiMarkupListColumnKind /// column-layout helper, which recomputes this dynamically off the list's /// live Width rather than baking it in at Build. /// +/// +/// +/// Fix round item 8: every settable member is internal init — this +/// type is constructible only through its / +/// / factories. A plugin (an external +/// assembly with no InternalsVisibleTo grant) can never assemble an +/// inconsistent instance (e.g. Text with +/// set) via object-initializer syntax; only the +/// three factories, which each set exactly the fields their own kind uses, +/// can construct one. +/// /// 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; } /// /// Fix round item 2: width="*" — 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 the class doc above). /// - public bool IsAutoWidth { get; init; } + public bool IsAutoWidth { get; internal init; } // ── text ────────────────────────────────────────────────────────────── /// <column type="text" items="{IReadOnlyList<string>}">. - public Func>? TextSource { get; init; } + public Func>? TextSource { get; internal init; } /// /// Optional per-row text color override, mirroring the single-column /// list's own colors attribute. Null (the default, when the /// column has no colors attribute at all) means every row in this /// column draws with the list's . /// - public Func>? ColorsSource { get; init; } + public Func>? ColorsSource { get; internal init; } /// /// Fix round finding 1: optional onclick="{Action<int>}" 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. /// - public Action? TextClicked { get; init; } + public Action? TextClicked { get; internal init; } // ── check ───────────────────────────────────────────────────────────── /// <column type="check" values="{IReadOnlyList<bool>}">. - public Func>? CheckSource { get; init; } + public Func>? CheckSource { get; internal init; } /// /// Fired with the ROW INDEX on a click anywhere in this cell — the /// plugin flips its own bool; the column never mutates /// 's backing collection itself. Required (a /// check column with no onchange throws at Build). /// - public Action? CheckChanged { get; init; } + public Action? CheckChanged { get; internal init; } // ── icon ────────────────────────────────────────────────────────────── /// <column type="icon" values="{IReadOnlyList<uint>}"> — one icon id per row. - public Func>? IconValuesSource { get; init; } + public Func>? IconValuesSource { get; internal init; } /// /// Resolves one entry to a drawable icon, /// dispatched by the column's own iconkind — 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). /// - public Func? IconResolve { get; init; } + public Func? IconResolve { get; internal init; } /// /// Fired with the ROW INDEX on a click anywhere in this cell. Required /// (an icon column with no onclick throws at Build). /// - public Action? IconClicked { get; init; } + public Action? IconClicked { get; internal init; } public static UiMarkupListColumn Text( float width, diff --git a/src/AcDream.App/UI/UiMarkupToggle.cs b/src/AcDream.App/UI/UiMarkupToggle.cs index 3f3a2dd96..ebfc4ad90 100644 --- a/src/AcDream.App/UI/UiMarkupToggle.cs +++ b/src/AcDream.App/UI/UiMarkupToggle.cs @@ -8,19 +8,6 @@ namespace AcDream.App.UI; /// public sealed class UiMarkupToggle : UiElement { - // Internal (not private): Campaign VT slice 1 Part B's 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? TextSource { get; set; } public Func? 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 + // 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 - // 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); - } } diff --git a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs index 3c5e9b61d..f372d2545 100644 --- a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs +++ b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs @@ -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() {