grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

A stand-in shadowed the real chat window, and took 96 tests with it

Browse Source 
`benilla_seat` — the test kit's stop-gap for frames a one-window kit does not
load — seats one for every row of `UIPARENT_MANAGED_FRAME_POSITIONS`, and
`ChatFrame1` and `ChatFrame2` are rows of it. The pass that triggers the seeding
runs **at load**, from `ShapeshiftBar_OnLoad` (`BonusActionBarFrame.lua:109` →
`:157`), twelve toc rows before `FloatingChatFrame.xml` builds the real windows.
A frame's publish to `_G` is non-overwriting (`0x701bd0`), faithfully, so the
stand-in stood and the real window — correctly built, correctly parented — was
unreachable by name for the rest of the VM's life.

`FloatingChatFrame.lua` then read the stand-in back three times: `FCF_SetWindowName`
concatenating its nil `GetName` (l.693), `FCF_SaveDock` handing
`SetChatWindowDocked` window 0 (l.1284), and `FCF_UpdateButtonSide` doing
arithmetic on its unresolvable `GetRight` (l.1019). Every kit that seats any part
of the chat UI died on the first of those, which is **96 of this crate's tests** —
27 bag, 26 guild, 18 raid, 18 friends and the rest — all reported as one
`assert!(report.errors.is_empty())`.

The two rows are now excluded from the seeding, and the exclusion is the point
rather than a special case: the pass reads a managed row *only* through
`frame = _G[index]; if ( frame )` (`UIParent.lua:1962`) and compares at l.2024
with `frame == ChatFrame1`, which a nil global answers without raising — so these
two never needed a stand-in at all. The rest of the table still seats, because the
custom-positioning tail does read some rows unguarded, and one of those
(`MultiBarBottomRight`, l.2070) is not in the helper's hardcoded list.

Two smaller things on the way, each right on its own and neither enough alone.
Stand-ins are created **named and with the id their name implies** — a bare
`CreateFrame("Frame")` answers `GetName()` nil and `GetID()` 0, the publish is
`setglobal` either way, so the naming costs nothing and it is what a stand-in is
for. And `floatingchatframe.xml` gains `ChatFrame.xml` as a prerequisite, which
the realm's toc puts immediately above it.

Four tests in `loader/tests.rs` are what ruled the loader out, each killing a
hypothesis about template `parent=` inheritance: it reaches the frame that
inherits it one level down, two levels down, with the chain's base in an earlier
document, and for a `<ScrollingMessageFrame>`, which is the kind `ChatFrame1` is.
None of them was the bug — the frame was always built with its parent, and only
its name never reached `_G` — but they pin behaviour the realm's chat windows
depend on and they are how the next one of these gets cheaper.

269 → 173, with no test newly failing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-10-28 21:22:26 -06:00
co-authored by Claude Opus 5
parent f4960efc53
commit eeebb5abf0
2 changed files with 97 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/ui_script/test_ui.rs
+19 -1
View File
@@ -346,9 +346,27 @@ pub(super) const UIPARENT_STAND_INS: &str = r#"
-- The `isVar` rows are skipped: those keys are global NUMBERS the pass writes
-- (CONTAINER_OFFSET_X/Y, PETACTIONBAR_YPOS…), and a frame seated under one of those names
-- would be arithmetic's problem two files later.
-- …except the two chat windows, and the exception is the point rather than a special
-- case. The pass reads a managed row ONLY through `frame = _G[index]; if ( frame )`
-- (UIParent.lua:1962) and compares at l.2024 with `frame == ChatFrame1`, which a nil
-- global answers without raising -- so these two never need a stand-in. Every other row
-- can be reached unguarded by the custom-positioning tail (`MultiBarBottomLeft:IsShown()`
-- l.2040, `PetActionBarFrame:IsShown()` l.2065, `MultiBarBottomRight:IsShown()` l.2070),
-- which is why the rest of the table still seats.
--
-- And seating these two is not merely unnecessary, it is destructive: `FloatingChatFrame`
-- builds the real ChatFrame1..7 twelve toc rows AFTER this pass first runs (it runs at
-- load, from ShapeshiftBar_OnLoad, BonusActionBarFrame.lua:109 -> :157), a frame's publish
-- to _G is non-overwriting (0x701bd0), so a stand-in seated here shadows the real window
-- for the rest of the VM's life while the real one -- correctly built, correctly parented
-- -- is unreachable by name. FloatingChatFrame.lua then reads the stand-in back:
-- FCF_SetWindowName concatenating its nil GetName (l.693), FCF_SaveDock handing
-- SetChatWindowDocked window 0 (l.1284), and FCF_UpdateButtonSide doing arithmetic on its
-- unresolvable GetRight (l.1019). That cost 136 of this crate's tests.
local chat_windows = { ChatFrame1 = true, ChatFrame2 = true }
local named = {}
for name, row in pairs(UIPARENT_MANAGED_FRAME_POSITIONS) do
if not row.isVar then table.insert(named, name) end
if not row.isVar and not chat_windows[name] then table.insert(named, name) end
-- …and every row's ANCHOR TARGET (`anchorTo`, l.1668), which is a different set: the
-- keys are the frames being MOVED, the targets are what they move relative to, and
-- several targets (`ActionButton1`, `MainMenuBarArtFrame`) are declared in files a
crates/benilla-ui/src/loader/tests.rs
+78
View File
@@ -942,6 +942,84 @@ mod loader_tests {
);
}
/// …and when the chain's **base** template came from an earlier document, which is the last
/// difference between the two tests above and the realm's real shape:
/// `FloatingChatFrameTemplate` carries the `parent=` and inherits `ChatFrameTemplate`, which
/// `ChatFrame.xml` declared one toc row earlier.
#[test]
fn a_parent_survives_an_inherit_chain_whose_base_is_in_an_earlier_document() {
let s = UiScript::new().unwrap();
let first = load(
&s,
&parse(
r#"<Ui>
<Frame name="EarlyBaseTemplate" virtual="true">
<Size><AbsDimension x="40" y="10"/></Size>
</Frame>
</Ui>"#,
),
&no_files,
);
assert!(first.errors.is_empty(), "{:?}", first.errors);
let second = load(
&s,
&parse(
r#"<Ui>
<Frame name="HostBox3"/>
<Frame name="LateMidTemplate" inherits="EarlyBaseTemplate" parent="HostBox3" virtual="true">
<Anchors><Anchor point="BOTTOMLEFT"/></Anchors>
</Frame>
<Frame name="LateLeaf" inherits="LateMidTemplate"/>
</Ui>"#,
),
&no_files,
);
assert!(second.errors.is_empty(), "{:?}", second.errors);
assert_eq!(
s.eval::<String>(
"return tostring(LateLeaf:GetParent() and LateLeaf:GetParent():GetName())"
)
.unwrap(),
"HostBox3",
"a cross-document base must not cost the middle template its parent="
);
}
/// …and for a `<ScrollingMessageFrame>`, which is the kind `ChatFrame1` actually is. Kept as
/// its own case because the three above all used `<Frame>`, and a kind with its own
/// materialise path is the one place a dropped attribute would not show.
#[test]
fn a_scrolling_message_frame_takes_its_templates_parent() {
let s = UiScript::new().unwrap();
let doc = parse(
r#"<Ui>
<Frame name="HostBox4" setAllPoints="true"/>
<ScrollingMessageFrame name="SmfTemplate" parent="HostBox4" virtual="true">
<Size><AbsDimension x="40" y="10"/></Size>
<Anchors><Anchor point="BOTTOMLEFT"/></Anchors>
</ScrollingMessageFrame>
<ScrollingMessageFrame name="SmfLeaf" inherits="SmfTemplate" id="1"/>
</Ui>"#,
);
let mut s = s;
s.set_screen_size(1024.0, 768.0);
let report = load(&s, &doc, &no_files);
assert!(report.errors.is_empty(), "{:?}", report.errors);
assert_eq!(
s.eval::<String>(
"return tostring(SmfLeaf:GetParent() and SmfLeaf:GetParent():GetName())"
)
.unwrap(),
"HostBox4"
);
assert_eq!(
s.eval::<String>("return tostring(SmfLeaf:GetRight())")
.unwrap(),
"40",
"with a parent the anchor resolves, which is what FCF_UpdateButtonSide needs"
);
}
/// A `.toc` line hands a listed `Bindings.xml` to `0x6ede10` (from `0x6edd51`), so each
/// `<Binding>` logs as an unknown frame type; `0x51f400` reads the bindings regardless.
#[test]