From cd34ab77d23fa68f38530e9534f8c3841927a65a Mon Sep 17 00:00:00 2001
From: Sam Sung <51686767+samwhosung@users.noreply.github.com>
Date: Wed, 30 Sep 2026 15:31:35 +0700
Subject: [PATCH] /errors: put Clear and Close in the panel art's sockets
 (#720)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `/errors` window wears the ClassTrainer panel art, but its Clear and Close buttons sat at y −455, below the art, and its detail pane ran to −442, into the button sockets.

On that art the class trainer lays out (`Blizzard_TrainerUI.xml`, `ClassTrainer_SetToClassTrainer`):
- a 296×184 list at (−67, −96) from the top right, with 11 rows;
- a 119-tall detail pane 8 below the list, ending at −407;
- Train and Exit, 80×22, centred at (224, −420) and (305, −420).

The log now uses those frames: Clear and Close sit in the sockets, the detail ends above them, and the list shows 11 rows, not 13.

The new test lays out the stock trainer and the log in one UI and checks that each log frame equals the trainer's, with the buttons inside the window. It fails on the old layout.

Fixes #698

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 .../benilla-app/assets/ui/ScriptLogFrame.xml  |  50 ++++---
 .../benilla-app/src/ui_script/layer_tests.rs  | 128 +++++++++++++++++-
 .../src/ui_script/trainer_tests.rs            |   4 +-
 3 files changed, 158 insertions(+), 24 deletions(-)

diff --git a/crates/benilla-app/assets/ui/ScriptLogFrame.xml b/crates/benilla-app/assets/ui/ScriptLogFrame.xml
index ef460f43..d12319a8 100644
--- a/crates/benilla-app/assets/ui/ScriptLogFrame.xml
+++ b/crates/benilla-app/assets/ui/ScriptLogFrame.xml
@@ -1,16 +1,20 @@
 <Ui>
     <!--
         The script error log, which 1.12 does not have, so every name carries the `Benilla` prefix;
-        the stock `ScriptErrors` dialog shows one message, this keeps the log. Shaped like the
-        reference's CraftFrame. It collects as a 1.12 error-log addon does: it wraps the handler
-        `geterrorhandler` holds, the stock `_ERRORMESSAGE` (BasicControls.xml:16), which still runs
-        after it. An addon that later calls `seterrorhandler` replaces the chain, as in 1.12.
+        the stock `ScriptErrors` dialog shows one message, this keeps the log. Laid out on the
+        ClassTrainer panel art as the class trainer lays itself out (Blizzard_TrainerUI.xml:214-356
+        and :481-513, Blizzard_TrainerUI.lua:469-474): its rows, its list, its detail under the
+        list and its two buttons in the art's sockets. It collects as a 1.12 error-log addon does:
+        it wraps the handler `geterrorhandler` holds, the stock `_ERRORMESSAGE`
+        (BasicControls.xml:16), which still runs after it. An addon that later calls
+        `seterrorhandler` replaces the chain, as in 1.12.
     -->
     <Script><![CDATA[
         -- A pushable above every stock panel's, so a game window opening pushes the log aside.
         UIPanelWindows["BenillaScriptLogFrame"] = { area = "left", pushable = 9, whileDead = 1 }
 
-        BENILLA_SCRIPTLOG_ROWS = 13
+        -- The class trainer's rows (`CLASS_TRAINER_SKILLS_DISPLAYED`, Blizzard_TrainerUI.lua:470).
+        BENILLA_SCRIPTLOG_ROWS = 11
         BENILLA_SCRIPTLOG_ROW_HEIGHT = 16
         -- A wrapped row would break the faux kit's fixed rows.
         BENILLA_SCRIPTLOG_ROW_CHARS = 52
@@ -254,8 +258,9 @@
                 </Layer></Layers>
             </Frame>
 
+            <!-- The class trainer's first row (ClassTrainerSkill1, Blizzard_TrainerUI.xml:214-222). -->
             <Button name="BenillaScriptLogRow1" inherits="BenillaScriptLogRowTemplate">
-                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="22" y="-94"/></Offset></Anchor></Anchors>
+                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="22" y="-100"/></Offset></Anchor></Anchors>
             </Button>
             <Button name="BenillaScriptLogRow2" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow1" relativePoint="BOTTOMLEFT"/></Anchors></Button>
             <Button name="BenillaScriptLogRow3" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow2" relativePoint="BOTTOMLEFT"/></Anchors></Button>
@@ -267,23 +272,24 @@
             <Button name="BenillaScriptLogRow9" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow8" relativePoint="BOTTOMLEFT"/></Anchors></Button>
             <Button name="BenillaScriptLogRow10" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow9" relativePoint="BOTTOMLEFT"/></Anchors></Button>
             <Button name="BenillaScriptLogRow11" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow10" relativePoint="BOTTOMLEFT"/></Anchors></Button>
-            <Button name="BenillaScriptLogRow12" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow11" relativePoint="BOTTOMLEFT"/></Anchors></Button>
-            <Button name="BenillaScriptLogRow13" inherits="BenillaScriptLogRowTemplate"><Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogRow12" relativePoint="BOTTOMLEFT"/></Anchors></Button>
 
-            <!-- Only the faux kit's right-hand Slider renders. -->
+            <!-- The class trainer's list frame (Blizzard_TrainerUI.xml:313-322, 184 tall as its .lua:471
+                 sets it). Only the faux kit's right-hand Slider renders. -->
             <ScrollFrame name="BenillaScriptLogListScrollFrame" inherits="FauxScrollFrameTemplate">
-                <Size><AbsDimension x="293" y="208"/></Size>
-                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="22" y="-94"/></Offset></Anchor></Anchors>
-                            <Scripts>
+                <Size><AbsDimension x="296" y="184"/></Size>
+                <Anchors><Anchor point="TOPRIGHT" relativePoint="TOPRIGHT"><Offset><AbsDimension x="-67" y="-96"/></Offset></Anchor></Anchors>
+                <Scripts>
                     <!-- The stock kit has no `updateFunc`, so the repaint is wired here. -->
                     <OnVerticalScroll>FauxScrollFrame_OnVerticalScroll(BENILLA_SCRIPTLOG_ROW_HEIGHT, BenillaScriptLog_Update)</OnVerticalScroll>
                 </Scripts>
-</ScrollFrame>
+            </ScrollFrame>
 
-            <!-- The real-scroll kit: a message with its traceback has variable height. -->
+            <!-- The class trainer's detail pane, 8 under its list and 119 tall (Blizzard_TrainerUI.xml:
+                 346-356, .lua:472), so it ends above the buttons' sockets. The real-scroll kit: a
+                 message with its traceback has variable height. -->
             <ScrollFrame name="BenillaScriptLogDetailScroll">
-                <Size><AbsDimension x="293" y="128"/></Size>
-                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="22" y="-314"/></Offset></Anchor></Anchors>
+                <Size><AbsDimension x="296" y="119"/></Size>
+                <Anchors><Anchor point="TOPLEFT" relativeTo="BenillaScriptLogListScrollFrame" relativePoint="BOTTOMLEFT"><Offset><AbsDimension x="0" y="-8"/></Offset></Anchor></Anchors>
                 <Scripts>
                     <OnLoad>this:SetScrollChild(getglobal(this:GetName() .. "Child"))</OnLoad>
                     <OnMouseWheel>BenillaScroll_OnMouseWheel(this, arg1)</OnMouseWheel>
@@ -299,7 +305,7 @@
                         <Scripts><OnValueChanged>BenillaScrollBar_OnValueChanged(this, arg1)</OnValueChanged></Scripts>
                     </Slider>
                     <Frame name="$parentChild">
-                        <Size><AbsDimension x="293" y="128"/></Size>
+                        <Size><AbsDimension x="296" y="119"/></Size>
                         <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"/></Anchors>
                         <Layers>
                             <Layer level="ARTWORK">
@@ -314,14 +320,16 @@
                 </Frames>
             </ScrollFrame>
 
+            <!-- The art's two button sockets, where the class trainer's Train and Exit sit
+                 (Blizzard_TrainerUI.xml:481-513). -->
             <Button name="BenillaScriptLogClearButton" inherits="UIPanelButtonTemplate" text="Clear">
-                <Size><AbsDimension x="96" y="22"/></Size>
-                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="22" y="-455"/></Offset></Anchor></Anchors>
+                <Size><AbsDimension x="80" y="22"/></Size>
+                <Anchors><Anchor point="CENTER" relativePoint="TOPLEFT"><Offset><AbsDimension x="224" y="-420"/></Offset></Anchor></Anchors>
                 <Scripts><OnClick>BenillaScriptLog_Clear()</OnClick></Scripts>
             </Button>
             <Button name="BenillaScriptLogCloseBottomButton" inherits="UIPanelButtonTemplate" text="Close">
-                <Size><AbsDimension x="96" y="22"/></Size>
-                <Anchors><Anchor point="TOPLEFT" relativePoint="TOPLEFT"><Offset><AbsDimension x="219" y="-455"/></Offset></Anchor></Anchors>
+                <Size><AbsDimension x="80" y="22"/></Size>
+                <Anchors><Anchor point="CENTER" relativePoint="TOPLEFT"><Offset><AbsDimension x="305" y="-420"/></Offset></Anchor></Anchors>
                 <Scripts><OnClick>HideUIPanel(getglobal("BenillaScriptLogFrame"))</OnClick></Scripts>
             </Button>
         </Frames>
diff --git a/crates/benilla-app/src/ui_script/layer_tests.rs b/crates/benilla-app/src/ui_script/layer_tests.rs
index edf6805b..ba67daba 100644
--- a/crates/benilla-app/src/ui_script/layer_tests.rs
+++ b/crates/benilla-app/src/ui_script/layer_tests.rs
@@ -3,7 +3,7 @@
 //! row, as 1.12.1 loads FrameXML (`UI_Init 0x48fbf0`: `FrameXML.toc` at `0x48ffed`, the addons at
 //! `0x4900a3`, only the latter through `AddOn_Load`); and a dev build boots without it.
 
-use benilla_ui::script::UiScript;
+use benilla_ui::script::{ScriptValue, UiScript};
 
 use crate::local_state::test_env::{EnvGuard, ENV_LOCK};
 
@@ -612,3 +612,129 @@ fn with_the_layer_plates_turned_on_stay_on_after_entry_and_reload() {
     assert_eq!(plate_globals(&app), (Some(1), Some(1)));
     let _ = std::fs::remove_dir_all(&home);
 }
+
+/// A frame's rect from its window's top-left corner, y down: `(left, top, right, bottom)`.
+fn window_rect(s: &UiScript, window: &str, frame: &str) -> (f32, f32, f32, f32) {
+    s.eval(&format!(
+        "local w, f = {window}, {frame} \
+         return f:GetLeft() - w:GetLeft(), w:GetTop() - f:GetTop(), \
+                f:GetRight() - w:GetLeft(), w:GetTop() - f:GetBottom()"
+    ))
+    .unwrap_or_else(|e| panic!("{frame} in {window}: {e}"))
+}
+
+/// The `/errors` window wears the ClassTrainer art, so its frames sit where the class trainer's own
+/// do on it (`Blizzard_TrainerUI.xml`, resized by `ClassTrainer_SetToClassTrainer`,
+/// `Blizzard_TrainerUI.lua:469-475`): the rows, the list, the detail under it, and the two buttons
+/// in the art's sockets (`ClassTrainerTrainButton` and `ClassTrainerCancelButton`, 80x22 centred
+/// at 224 and 305, -420). The detail ends above the buttons, as the trainer's does at -407.
+#[test]
+fn the_error_log_lays_out_on_the_trainer_art_as_the_class_trainer_does() {
+    benilla_formats::wow_data_or_skip!();
+    let mut s = super::trainer_tests::trainer_script();
+    // The file registers its commands in the stock table, which `ChatFrame.lua` declares and this
+    // kit stops short of.
+    s.run("SlashCmdList = {}").unwrap();
+    super::test_ui::load_ui_strict(&s, "ScriptLogFrame.xml");
+
+    // The class trainer's layout, read off its own frames once it has laid itself out.
+    s.set_money(50);
+    s.set_trainer(Some(super::trainer_tests::menu()));
+    s.fire_event(
+        "TRAINER_SHOW",
+        vec![ScriptValue::Str("Sana Winterhoof".into())],
+    );
+    s.resolve();
+    let trainer = |s: &UiScript, frame: &str| window_rect(s, "ClassTrainerFrame", frame);
+    let ref_rows = s
+        .eval::<i64>("return CLASS_TRAINER_SKILLS_DISPLAYED")
+        .unwrap();
+    let ref_list = trainer(&s, "ClassTrainerListScrollFrame");
+    let ref_detail = trainer(&s, "ClassTrainerDetailScrollFrame");
+    let ref_buttons = [
+        trainer(&s, "ClassTrainerTrainButton"),
+        trainer(&s, "ClassTrainerCancelButton"),
+    ];
+    let ref_rows_rects: Vec<_> = (1..=ref_rows)
+        .map(|i| trainer(&s, &format!("ClassTrainerSkill{i}")))
+        .collect();
+    // The reference's own numbers, so a drifted harness cannot pass this against itself.
+    assert_eq!(ref_rows, 11);
+    assert_eq!(ref_list, (21.0, 96.0, 317.0, 280.0));
+    assert_eq!(ref_detail, (21.0, 288.0, 317.0, 407.0));
+    assert_eq!(ref_buttons[0], (184.0, 409.0, 264.0, 431.0));
+    assert_eq!(ref_buttons[1], (265.0, 409.0, 345.0, 431.0));
+
+    s.set_trainer(None);
+    s.fire_event("TRAINER_CLOSED", vec![]);
+    s.run("ShowUIPanel(BenillaScriptLogFrame)").unwrap();
+    // Enough rows to fill the list, so every row shows.
+    s.run("for i = 1, 30 do BenillaScriptLog_Record('error ' .. i) end")
+        .unwrap();
+    for _ in 0..4 {
+        s.resolve();
+        s.tick(0.016);
+    }
+    s.resolve();
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+    let ours = |s: &UiScript, frame: &str| window_rect(s, "BenillaScriptLogFrame", frame);
+
+    let window = ours(&s, "BenillaScriptLogFrame");
+    assert_eq!(window, (0.0, 0.0, 384.0, 512.0), "the standard panel size");
+    let clear = ours(&s, "BenillaScriptLogClearButton");
+    let close = ours(&s, "BenillaScriptLogCloseBottomButton");
+    let detail = ours(&s, "BenillaScriptLogDetailScroll");
+    let list = ours(&s, "BenillaScriptLogListScrollFrame");
+
+    // The buttons take the trainer's two sockets, at its size, inside the window's rect.
+    assert_eq!(clear, ref_buttons[0], "Clear sits where Train does");
+    assert_eq!(close, ref_buttons[1], "Close sits where Exit does");
+    for (name, b) in [("Clear", clear), ("Close", close)] {
+        assert!(
+            b.0 >= window.0 && b.1 >= window.1 && b.2 <= window.2 && b.3 <= window.3,
+            "{name} {b:?} lies inside the {window:?} window"
+        );
+    }
+    // The detail pane is the trainer's, and its bottom clears both buttons' tops.
+    assert_eq!(
+        detail, ref_detail,
+        "the detail sits where the trainer's does"
+    );
+    assert!(
+        detail.3 < clear.1 && detail.3 < close.1,
+        "the detail's bottom {} is above the buttons' top {}",
+        detail.3,
+        clear.1.min(close.1)
+    );
+    // The list is the trainer's, with its rows.
+    assert_eq!(list, ref_list, "the list sits where the trainer's does");
+    assert_eq!(
+        s.eval::<i64>("return BENILLA_SCRIPTLOG_ROWS").unwrap(),
+        ref_rows,
+        "as many rows as the trainer's list"
+    );
+    assert!(
+        s.eval::<bool>(&format!(
+            "return getglobal('BenillaScriptLogRow{}') == nil",
+            ref_rows + 1
+        ))
+        .unwrap(),
+        "no row beyond the list"
+    );
+    for (i, want) in ref_rows_rects.iter().enumerate() {
+        let row = ours(&s, &format!("BenillaScriptLogRow{}", i + 1));
+        // The trainer widens a row to 323 while its scroll bar is hidden and keeps the template's
+        // 293 beside it (`Blizzard_TrainerUI.lua:133-137`); the log's rows are the 293.
+        assert_eq!(
+            (row.0, row.1, row.2, row.3),
+            (want.0, want.1, want.0 + 293.0, want.3),
+            "row {} sits where the trainer's does",
+            i + 1
+        );
+        assert!(
+            row.1 >= list.1 && row.3 <= list.3,
+            "row {} {row:?} lies inside the list {list:?}",
+            i + 1
+        );
+    }
+}
diff --git a/crates/benilla-app/src/ui_script/trainer_tests.rs b/crates/benilla-app/src/ui_script/trainer_tests.rs
index 06ea37a9..7af7d278 100644
--- a/crates/benilla-app/src/ui_script/trainer_tests.rs
+++ b/crates/benilla-app/src/ui_script/trainer_tests.rs
@@ -9,7 +9,7 @@ use benilla_ui::script::{
 use super::test_ui::load_ui as load_xml;
 
 /// The trainer window with every state filter on, for the full tree at fixed indices.
-fn trainer_script() -> UiScript {
+pub(super) fn trainer_script() -> UiScript {
     let mut s = trainer_script_base();
     load_xml(
         &s,
@@ -148,7 +148,7 @@ fn service(
 /// A two-line warrior menu. Groups sort by name, services by level then name, so the tree is:
 ///   1 H:Arms · 2 Heroic Strike(avail,10c,l1) · 3 Cleave(unavail,l20,skill+ability) ·
 ///   4 H:Fury · 5 Rend(used,30c) · 6 Thunder Clap(avail,500c)
-fn menu() -> TrainerState {
+pub(super) fn menu() -> TrainerState {
     TrainerState {
         greeting: "Well met. Let me show you the way of the warrior.".into(),
         trainer_type: 0,