From 55239575e6320a1d8f43a289dadfbea99bfdc339 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 15 Jun 2026 13:25:44 +0200 Subject: [PATCH] =?UTF-8?q?refactor(D.2b):=20ElementReader=20review=20fixe?= =?UTF-8?q?s=20=E2=80=94=20defensive=20Children=20copy=20+=20sentinel=20do?= =?UTF-8?q?c?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Merge: defensive copy `new List(derived.Children)` so a later mutation of the merged result or the input can't corrupt the other - Merge: add comment on Width/Height 0-sentinel (Plan-1 safe; Plan-2 limitation and float?-upgrade path documented inline) - Test: replace mid-sentence "Wait —" authoring trace in EdgeFlagsToAnchors_ValueThree_FallsBackToTopLeft with a clean conclusion-first summary of the value-3 mapping rule 9/9 ElementReaderTests pass; 0 build errors. Co-Authored-By: Claude Sonnet 4.6 --- src/AcDream.App/UI/Layout/ElementReader.cs | 10 +++++++++- .../UI/Layout/ElementReaderTests.cs | 12 +++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/AcDream.App/UI/Layout/ElementReader.cs b/src/AcDream.App/UI/Layout/ElementReader.cs index e1a5272e..c5087b99 100644 --- a/src/AcDream.App/UI/Layout/ElementReader.cs +++ b/src/AcDream.App/UI/Layout/ElementReader.cs @@ -145,6 +145,11 @@ public static class ElementReader Type = derived.Type != 0 ? derived.Type : base_.Type, X = derived.X, Y = derived.Y, + // NOTE: 0 is the "not set, inherit from base" sentinel for Width/Height. This + // diverges from the format doc §12 rule 2 ("derived W/H win even if zero") but is + // indistinguishable for Plan 1 (all base elements are zero-size Type-12 prototypes). + // If a real zero-size derived element ever needs to override a non-zero base in + // Plan 2, switch Width/Height to float? + null-coalescing (and update Tasks 3-5). Width = derived.Width != 0 ? derived.Width : base_.Width, Height = derived.Height != 0 ? derived.Height : base_.Height, Left = derived.Left, @@ -154,7 +159,10 @@ public static class ElementReader ReadOrder = derived.ReadOrder, FontDid = derived.FontDid != 0 ? derived.FontDid : base_.FontDid, // Children come from the derived element's own tree, not the base prototype's. - Children = derived.Children, + // Defensive copy: prevent a later mutation of either the merged result or the input + // from corrupting the other. Safe for the Task-5 flow (derived.Children is fully + // populated by the recursive importer BEFORE Merge is called and never mutated after). + Children = new List(derived.Children), }; // Start with base StateMedia as defaults, then let derived entries override. m.StateMedia = new Dictionary(base_.StateMedia); diff --git a/tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs b/tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs index 90b1a995..c489f88c 100644 --- a/tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs @@ -66,13 +66,11 @@ public class ElementReaderTests } /// - /// Value 3 = floating/centered between both far edges on that axis. - /// Both LeftEdge=3 and RightEdge=3 → neither Left nor Right are set by the - /// near/stretch rules. The result is only Right+Bottom (the "far" semantics). - /// Specifically: left=3 → not Left (3 is not 1 or 4); right=3 → Right (3 is not 2 or 4, skip). - /// Wait — value 3 means "pinned to BOTH far edges" per format doc §4. Re-check the - /// mapping rule: Right anchor fires on right==2 || right==4, NOT on right==3. - /// So value 3 on LeftEdge, TopEdge, RightEdge, BottomEdge → no flags set → default Left|Top. + /// Value 3 = floating/centered between both far edges on that axis (format doc §4). + /// The anchor mapping fires on near-pin (1) and stretch (4) for Left/Top, and on + /// far-pin (2) and stretch (4) for Right/Bottom — value 3 matches none of these rules. + /// Therefore all-3 edge flags contribute no anchor bits and fall through to the + /// Left|Top default (pin top-left, fixed size). /// This test covers that corner case (element 0x100004A9 — expand-detail overlay). /// [Fact]