From ad4ed51d6b12a658431191a190fcd9bb8e2f2deb Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 26 Jun 2026 11:35:19 +0200 Subject: [PATCH] =?UTF-8?q?feat(ui):=20importer=20Fix=205=20=E2=80=94=20bu?= =?UTF-8?q?ild=20meter=20text=20children;=20drop=20character=20XP=20inject?= =?UTF-8?q?ion=20hacks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LayoutImporter.BuildWidget gains a UiMeter-specific branch after the ConsumesDatChildren gate: Type-3 slice containers (already consumed by DatWidgetFactory.BuildMeter to extract sprite ids) are skipped, but all other children (Type-12 UIElement_Text overlays) are built normally, registered in byId, and attached as UiElement children of the meter. This fixes the XP meter (0x10000236): its two Type-12 children 0x10000237 "XP for next level:" label 0x10000238 XP-to-next-level value are now real UiText widgets in the tree, findable via FindElement. CharacterStatController.Bind now uses FindElement + LinesProvider binding instead of injecting runtime AddChild nodes. The two previously-injected UiText overlays are removed; the controller binds Padding=0, ClickThrough, RightAligned on the dat-origin widgets instead. New constants XpNextLabelId + XpNextValueId replace the old comment block. TAB GROUPS (SKIPPED — documented): UiText.ConsumesDatChildren flipping is NOT safe globally (risks chat/vitals) and the page-visibility pass in CharacterStatController depends on tab group Children.Count==0. Tab sprite injection onto layout.Root stays and is explicitly commented as deliberate. VITALS SAFE: health/stamina/mana meters have only Type-3 children → new loop finds nothing → zero UiElement children added → byte-identical render. VitalsController.BindMeter AddChild(number) pattern unchanged. Tests: 3 new LayoutImporterTests (slice-only, Fix-5 text children, vitals regression guard) + 3 CharacterStatControllerTests (label bound, value bound, missing children no-throw). 715 tests pass. Co-Authored-By: Claude Sonnet 4.6 --- .../UI/Layout/CharacterStatController.cs | 94 +++++++-------- src/AcDream.App/UI/Layout/LayoutImporter.cs | 26 +++++ .../UI/Layout/CharacterStatControllerTests.cs | 72 ++++++++++++ .../UI/Layout/LayoutImporterTests.cs | 107 ++++++++++++++++-- 4 files changed, 241 insertions(+), 58 deletions(-) diff --git a/src/AcDream.App/UI/Layout/CharacterStatController.cs b/src/AcDream.App/UI/Layout/CharacterStatController.cs index 282d94d6..787622da 100644 --- a/src/AcDream.App/UI/Layout/CharacterStatController.cs +++ b/src/AcDream.App/UI/Layout/CharacterStatController.cs @@ -53,10 +53,12 @@ public static class CharacterStatController public const uint TotalXpLabelId = 0x10000234u; // "Total Experience (XP):" caption left of value public const uint TotalXpId = 0x10000235u; // m_pTotalXPText public const uint XpMeterId = 0x10000236u; // m_pXPToLevelMeter (UiMeter) - // NOTE: 0x10000237 (XP-to-level label) and 0x10000238 (XP-to-level value) are children of - // the UiMeter element 0x10000236 and are consumed by it (UiMeter.ConsumesDatChildren=true). - // FindElement returns NULL for both. They cannot be bound via the element id. - // The XP meter itself renders the track sprite; the controller binds Fill=XpFraction. + // Fix 5: 0x10000237 (XP-to-level label) and 0x10000238 (XP-to-level value) are now + // built by the LayoutImporter as UiText children of the XP meter (non-Type-3 children + // are explicitly built and registered in byId). FindElement DOES return them now. + // The controller binds their LinesProvider instead of injecting new runtime nodes. + public const uint XpNextLabelId = 0x10000237u; // "XP for next level:" label child of XP meter + public const uint XpNextValueId = 0x10000238u; // XP-to-next-level value child of XP meter public const uint ListBoxId = 0x1000023Du; // m_pListBox container // ── Footer STATE-A container id ────────────────────────────────────────── @@ -73,8 +75,18 @@ public static class CharacterStatController // ConsumesDatChildren=true, so the three button children (left-cap, label, right-cap) // are consumed at import time and not present in the widget tree. // - // Fix: the controller adds three sprite-drawing children to each UiText manually, using - // the known RenderSurface ids from the retail UI layout dump (2026-06-25): + // WHY this is NOT fixed in Fix 5 (importer series): + // The Fix 5 meter patch only builds NON-Type-3 children of UiMeter. The tab group children + // are children of UiText (Type 12), not UiMeter (Type 7). Flipping UiText.ConsumesDatChildren + // globally is not safe — the chat transcript UiText and other Type-12 elements also have + // children in some layouts (scroll decorators etc.), and building them as live UiText widgets + // would risk invisible nodes stealing focus/clicks (the exact problem ConsumesDatChildren was + // designed to prevent). Additionally, the page-visibility pass in Bind() depends on + // tab group Children.Count==0 to skip them — adding children to the tab groups would break + // that pass and hide ALL tab content. + // + // Therefore: the controller injects 3 sprite-drawing root children per tab (absolute coords), + // using the known RenderSurface ids from the retail UI layout dump (2026-06-25): // Closed (inactive) state: left=0x06005D93, center=0x06005D95, right=0x06005D97 // Open (active) state: left=0x06005D92, center=0x06005D94, right=0x06005D96 // Source: state_id 11=Closed, 12=Open per gmTabUI::SetActive(bool); sprite ids confirmed @@ -209,57 +221,37 @@ public static class CharacterStatController Label(layout, TotalXpId, null, Body, () => data().TotalXp.ToString("N0")); // XP-to-level meter fill (gmStatManagementUI::UpdateExperience 0x004f0a70). - // NOTE: child elements 0x10000237 (label) and 0x10000238 (value) are consumed by - // the UiMeter and cannot be bound — FindElement returns NULL for both. - // We inject a UiText caption + value ABOVE the meter's parent container instead, - // positioned to the left and right of a thin strip above the red bar. + // Fix 5: child elements 0x10000237 (label) and 0x10000238 (value) are now built by + // the LayoutImporter as UiText children of the XP meter (non-Type-3 meter children + // are explicitly built and registered in byId — see LayoutImporter.BuildWidget). + // FindElement now returns them; the controller binds their LinesProvider. + // The importer builds them as UiText via DatWidgetFactory.BuildText, applying their + // dat-origin HJustify/VJustify/FontDid/FontColor at build time. The controller then + // overrides Padding=0 (to avoid scroll clip in the ~13px-tall bar), ClickThrough=true, + // and provides the LinesProvider for runtime content. if (layout.FindElement(XpMeterId) is UiMeter meter) { meter.Fill = () => data().XpFraction; - // Inject "XP for next level:" label and value ON the meter as children. - // The retail layout puts this caption + value ON TOP of the red bar — the bar - // is behind the text (ref 2026-06-26: "value … with the red fill bar behind it"). - // UiMeter.ConsumesDatChildren=true consumed the original 0x10000237/0x10000238 - // children at import; we re-inject them as runtime UiText overlays at LOCAL (0,0) - // filling the meter's full rect. The caption is left-aligned; the value is right-aligned. + // Bind the dat-origin XP label (0x10000237) and value (0x10000238). + // These are now real UiText children of the meter (built by the importer). + // The retail layout places the caption + value ON TOP of the red bar + // (ref 2026-06-26: "value … with the red fill bar behind it"). // Source: retail spec (2026-06-26-character-window-retail-reference.md §State 1). + if (layout.FindElement(XpNextLabelId) is UiText xpLabel) { - float mW = meter.Width; - float mH = meter.Height; - - var xpNextLabel = new UiText - { - Left = 0f, - Top = 0f, - Width = mW * 0.60f, - Height = mH, - DatFont = datFont, - ClickThrough = true, - Centered = false, - RightAligned = false, - Padding = 0f, // avoid scroll clip — meter bar is ~13px tall - Anchors = AnchorEdges.Left | AnchorEdges.Top, - }; - xpNextLabel.LinesProvider = static () => new[] { new UiText.Line("XP for next level:", Body) }; - - var xpNextValue = new UiText - { - Left = 0f, - Top = 0f, - Width = mW, - Height = mH, - DatFont = datFont, - ClickThrough = true, - Centered = false, - RightAligned = true, - Padding = 0f, // avoid scroll clip - Anchors = AnchorEdges.Left | AnchorEdges.Top, - }; - xpNextValue.LinesProvider = () => new[] { new UiText.Line(data().XpToNextLevel.ToString("N0"), Body) }; - - meter.AddChild(xpNextLabel); - meter.AddChild(xpNextValue); + if (datFont is not null) xpLabel.DatFont = datFont; + xpLabel.ClickThrough = true; + xpLabel.Padding = 0f; // avoid scroll clip — meter bar is ~13px tall + xpLabel.LinesProvider = static () => new[] { new UiText.Line("XP for next level:", Body) }; + } + if (layout.FindElement(XpNextValueId) is UiText xpValue) + { + if (datFont is not null) xpValue.DatFont = datFont; + xpValue.ClickThrough = true; + xpValue.RightAligned = true; + xpValue.Padding = 0f; // avoid scroll clip + xpValue.LinesProvider = () => new[] { new UiText.Line(data().XpToNextLevel.ToString("N0"), Body) }; } } diff --git a/src/AcDream.App/UI/Layout/LayoutImporter.cs b/src/AcDream.App/UI/Layout/LayoutImporter.cs index 24fb30f0..43a60108 100644 --- a/src/AcDream.App/UI/Layout/LayoutImporter.cs +++ b/src/AcDream.App/UI/Layout/LayoutImporter.cs @@ -130,6 +130,32 @@ public static class LayoutImporter if (cw is not null) w.AddChild(cw); } } + else if (w is UiMeter) + { + // Fix 5: UiMeter.ConsumesDatChildren=true swallows ALL children, including text + // label/value overlays that are separate renderable widgets (not part of the bar + // art). BuildMeter in DatWidgetFactory already consumed the Type-3 slice containers + // (reads their grandchild sprite ids to populate Back*/Front* properties). The + // remaining non-Type-3 children (typically Type-12 UIElement_Text overlays such + // as the XP meter's 0x10000237 label + 0x10000238 value) ARE renderable and belong + // in the widget tree. We build them here explicitly, registered in byId so + // FindElement can locate them, and attached as children of the meter so they render + // as overlays at their dat-local coordinates. The controller can then locate these + // widgets via FindElement and bind LinesProvider without injecting new runtime nodes. + // + // Type-3 children are SKIPPED here because BuildMeter already consumed them (they + // carry the 3-slice sprite ids, not text content; building them again would + // double-draw the bar art). All other child types are built normally. + // + // Safe for vitals: the health/stamina/mana meters have ONLY Type-3 slice children + // (no text children). This loop finds nothing for them → no change to vitals. + foreach (var child in info.Children) + { + if (child.Type == 3) continue; // slice containers: already consumed by BuildMeter + var cw = BuildWidget(child, resolve, datFont, fontResolve, byId); + if (cw is not null) w.AddChild(cw); + } + } return w; } diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs index d0607b97..18338f8d 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs @@ -825,6 +825,78 @@ public class CharacterStatControllerTests public void Bind_MissingElements_DoesNotThrow() => CharacterStatController.Bind(Fake(), SampleData.SampleCharacter); + // ── Fix 5: XP meter text children bound via FindElement ────────────────── + + /// + /// Fix 5: the XP label (0x10000237) is a dat-origin UiText child of the XP meter + /// (now built by the importer). After Bind(), FindElement must return a UiText with + /// a LinesProvider that emits "XP for next level:". + /// + [Fact] + public void Bind_XpMeter_XpNextLabel_IsBoundViaFindElement() + { + var meter = new UiMeter(); + var xpLabel = new UiText(); + // Attach the label as a child of the meter so it matches the real importer layout. + meter.AddChild(xpLabel); + var layout = Fake( + (CharacterStatController.XpMeterId, meter), + (CharacterStatController.XpNextLabelId, xpLabel)); + + CharacterStatController.Bind(layout, SampleData.SampleCharacter); + + Assert.NotNull(xpLabel.LinesProvider); + var lines = xpLabel.LinesProvider(); + Assert.Single(lines); + Assert.Equal("XP for next level:", lines[0].Text); + } + + /// + /// Fix 5: the XP value (0x10000238) is bound to the XpToNextLevel string. + /// ClickThrough=true and RightAligned=true must be set by the controller. + /// + [Fact] + public void Bind_XpMeter_XpNextValue_IsBoundViaFindElement() + { + var meter = new UiMeter(); + var xpValue = new UiText(); + meter.AddChild(xpValue); + var layout = Fake( + (CharacterStatController.XpMeterId, meter), + (CharacterStatController.XpNextValueId, xpValue)); + + CharacterStatController.Bind(layout, SampleData.SampleCharacter); + + Assert.NotNull(xpValue.LinesProvider); + Assert.True(xpValue.ClickThrough, "XP value overlay must be ClickThrough"); + Assert.True(xpValue.RightAligned, "XP value overlay must be RightAligned"); + + var lines = xpValue.LinesProvider(); + Assert.Single(lines); + // XpToNextLevel from SampleData = 42_000_000L formatted as "42,000,000" + Assert.Equal((42_000_000L).ToString("N0"), lines[0].Text); + } + + /// + /// Fix 5: if XpNextLabelId / XpNextValueId are absent from the layout (the old + /// ConsumesDatChildren path, or a test layout that doesn't include them), Bind() + /// must not throw — the meter Fill is still bound. + /// + [Fact] + public void Bind_XpMeter_MissingTextChildren_DoesNotThrow() + { + var meter = new UiMeter(); + var layout = Fake((CharacterStatController.XpMeterId, meter)); + + // No XpNextLabelId or XpNextValueId in the layout. + var ex = Record.Exception(() => + CharacterStatController.Bind(layout, SampleData.SampleCharacter)); + + Assert.Null(ex); + // Fill must still be bound. + Assert.NotNull(meter.Fill()); + } + // ── Helpers ────────────────────────────────────────────────────────────── /// Manufacture a minimal UiButton with a fake ElementInfo (no dat sprites). diff --git a/tests/AcDream.App.Tests/UI/Layout/LayoutImporterTests.cs b/tests/AcDream.App.Tests/UI/Layout/LayoutImporterTests.cs index 2025085c..34657064 100644 --- a/tests/AcDream.App.Tests/UI/Layout/LayoutImporterTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/LayoutImporterTests.cs @@ -55,15 +55,15 @@ public class LayoutImporterTests Assert.NotNull(tree.FindElement(0x20000002)); } - // ── Test 3: Meter consumes its children — child ids not in byId ────────── + // ── Test 3: Meter consumes Type-3 slice children — child ids not in byId ──── /// - /// A meter (Type 7) whose children are the 3-slice back/front containers. - /// The meter itself must be findable; its direct children must NOT appear as - /// separate nodes in the tree (meters own their children, not the generic tree). + /// A meter (Type 7) whose children are ONLY the 3-slice back/front containers (Type 3). + /// The meter itself must be findable; its Type-3 children must NOT appear as separate + /// nodes in the tree (Fix 5: only non-Type-3 children are built as separate widgets). /// [Fact] - public void BuildFromInfos_MeterWithChildren_MeterPresent_ChildrenNotInTree() + public void BuildFromInfos_MeterWithSliceChildren_MeterPresent_SliceChildrenNotInTree() { const uint MeterId = 0x100000E6u; const uint BackLayerId = 0x100000E7u; @@ -85,10 +85,103 @@ public class LayoutImporterTests // The meter widget is present. Assert.IsType(tree.FindElement(MeterId)); - // The meter's dat-children are NOT separate UiElement nodes. + // The meter's Type-3 slice children are NOT separate UiElement nodes. Assert.Null(tree.FindElement(BackLayerId)); Assert.Null(tree.FindElement(FrontLayerId)); - // The UiMeter itself has no Ui children (meters consume their children internally). + // The UiMeter itself has no UiElement children (all children were Type-3, consumed). + var uiMeter = (UiMeter)tree.FindElement(MeterId)!; + Assert.Empty(uiMeter.Children); + } + + // ── Test 5: Fix 5 — meter text children (Type-12) are built and registered ─ + + /// + /// Fix 5: a meter (Type 7) with BOTH Type-3 slice containers AND a Type-12 text + /// child. The Type-3 containers must be consumed (not in byId); the Type-12 text + /// child must be built as a UiText, registered in byId, and attached as a UiElement + /// child of the meter (so it renders as an overlay). + /// + [Fact] + public void BuildFromInfos_MeterWithTextChild_TextChildInTreeAsUiTextChildOfMeter() + { + const uint MeterId = 0x10000236u; + const uint BackId = 0x10000237u; // Type-3 slice + const uint FrontId = 0x10000238u; // Type-3 slice + const uint LabelId = 0x10000239u; // Type-12 text child + const uint ValueId = 0x1000023Au; // Type-12 text child + + var backContainer = BuildSliceContainer(BackId, ReadOrder: 0, + l: 0x0600747Eu, t: 0x0600747Fu, r: 0x06007480u); + var frontContainer = BuildSliceContainer(FrontId, ReadOrder: 1, + l: 0x06007481u, t: 0x06007482u, r: 0x06007483u); + + // Two Type-12 text children (the XP "label" and "value" overlay). + var labelInfo = new ElementInfo { Id = LabelId, Type = 12, X = 0, Y = 0, Width = 120, Height = 13 }; + var valueInfo = new ElementInfo { Id = ValueId, Type = 12, X = 0, Y = 0, Width = 200, Height = 13, + HJustify = HJustify.Right }; + + var meter = new ElementInfo { Id = MeterId, Type = 7, Width = 200, Height = 13 }; + meter.Children.Add(backContainer); + meter.Children.Add(frontContainer); + meter.Children.Add(labelInfo); + meter.Children.Add(valueInfo); + + var root = new ElementInfo { Id = 0x100005F9, Type = 3, Width = 210, Height = 20 }; + + var tree = LayoutImporter.BuildFromInfos(root, new[] { meter }, NoTex, null); + + // Meter is present. + Assert.IsType(tree.FindElement(MeterId)); + + // Type-3 slice containers are NOT in byId (consumed by BuildMeter). + Assert.Null(tree.FindElement(BackId)); + Assert.Null(tree.FindElement(FrontId)); + + // Type-12 text children ARE in byId as UiText. + Assert.IsType(tree.FindElement(LabelId)); + Assert.IsType(tree.FindElement(ValueId)); + + // The UiMeter has exactly 2 UiElement children (the two text overlays). + var uiMeter = (UiMeter)tree.FindElement(MeterId)!; + Assert.Equal(2, uiMeter.Children.Count); + Assert.All(uiMeter.Children, c => Assert.IsType(c)); + + // The value child should have RightAligned=true (HJustify.Right in the dat). + var valueWidget = (UiText)tree.FindElement(ValueId)!; + Assert.True(valueWidget.RightAligned, "Type-12 text child with HJustify.Right must build as RightAligned=true"); + } + + // ── Test 6: Fix 5 — vitals meters (Type-3 only) are unaffected ──────────── + + /// + /// Fix 5 regression guard: vitals health/stamina/mana meters have ONLY Type-3 slice + /// children (no Type-12 text children). The meter must have zero UiElement children + /// after build — VitalsController injects its number overlay at runtime, not the importer. + /// + [Fact] + public void BuildFromInfos_VitalsMeter_NoTextChildren_MeterHasNoUiChildren() + { + const uint MeterId = 0x100000E6u; // vitals health meter + const uint BackId = 0x100000E7u; + const uint FrontId = 0x100000E8u; + + var backContainer = BuildSliceContainer(BackId, ReadOrder: 0, + l: 0x0600747Eu, t: 0x0600747Fu, r: 0x06007480u); + var frontContainer = BuildSliceContainer(FrontId, ReadOrder: 1, + l: 0x06007481u, t: 0x06007482u, r: 0x06007483u); + + var meter = new ElementInfo { Id = MeterId, Type = 7, Width = 150, Height = 16 }; + meter.Children.Add(backContainer); + meter.Children.Add(frontContainer); + + var root = new ElementInfo { Id = 0x100005F9, Type = 3, Width = 160, Height = 58 }; + + var tree = LayoutImporter.BuildFromInfos(root, new[] { meter }, NoTex, null); + + Assert.IsType(tree.FindElement(MeterId)); + + // Vitals meter has no UiElement children — only Type-3 children were present, + // all consumed, none built as UiText overlays. var uiMeter = (UiMeter)tree.FindElement(MeterId)!; Assert.Empty(uiMeter.Children); }