From eeebb5abf0c45a16682986e33fdfc90524a5b507 Mon Sep 17 00:00:00 2001 From: grom Date: Wed, 28 Oct 2026 21:22:26 -0600 Subject: [PATCH] A stand-in shadowed the real chat window, and took 96 tests with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 ``, 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 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/ui_script/test_ui.rs | 20 +++++- crates/benilla-ui/src/loader/tests.rs | 78 +++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/crates/benilla-app/src/ui_script/test_ui.rs b/crates/benilla-app/src/ui_script/test_ui.rs index fcb42d9c..ee6f5f13 100644 --- a/crates/benilla-app/src/ui_script/test_ui.rs +++ b/crates/benilla-app/src/ui_script/test_ui.rs @@ -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 diff --git a/crates/benilla-ui/src/loader/tests.rs b/crates/benilla-ui/src/loader/tests.rs index a82708c5..93d25767 100644 --- a/crates/benilla-ui/src/loader/tests.rs +++ b/crates/benilla-ui/src/loader/tests.rs @@ -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#" + + + + "#, + ), + &no_files, + ); + assert!(first.errors.is_empty(), "{:?}", first.errors); + let second = load( + &s, + &parse( + r#" + + + + + + "#, + ), + &no_files, + ); + assert!(second.errors.is_empty(), "{:?}", second.errors); + assert_eq!( + s.eval::( + "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 ``, which is the kind `ChatFrame1` actually is. Kept as + /// its own case because the three above all used ``, 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#" + + + + + + + "#, + ); + 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::( + "return tostring(SmfLeaf:GetParent() and SmfLeaf:GetParent():GetName())" + ) + .unwrap(), + "HostBox4" + ); + assert_eq!( + s.eval::("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 /// `` logs as an unknown frame type; `0x51f400` reads the bindings regardless. #[test]