the numerals kit goes stock, and a chat link is its own mouse target

- the status-bar numerals kit (`TextStatusBar.xml` + `.lua`) is the reference's own, and the attempt found a class of silent failure: `TextStatusBar_Initialize()` takes no argument in 1.12 and acts on `this`; ours took an optional bar, our unit frames passed one, and the reference's version ignores it — so the numerals would simply never appear, no error, nothing missing; seventeen of our signatures differ from the reference's that way (Lua drops a surplus argument in silence), written down before their files migrate, and the three "ours narrower" ones — `StaticPopup_Hide(which, data)`, `StaticPopup_FindVisible`, `WorldMapButton_OnClick(button, frame)` — closed now, because that direction bites when a STOCK file arrives and calls our version
- a chat link needs no mouse on its frame: wow-re read the client's ctors as mouse-enabling Button, CheckButton, Slider, ColorSelect and EditBox and NOT the scroll frames; the objection ("something makes a 1.12 chat link clickable") resolved as the engine synthesising one mouse-enabled `CSimpleHyperlinkButton` per link span (`0x7a3240`) — so a link span is a hit-test candidate on its own, the chat window is inert everywhere else, and the wheel is its own predicate rather than a borrowed mouse bit; seven `*ScrollFrame` known divergences retire
- the relic fit rule: `IsValidForSlot`'s slot-17 leg is an EQUALITY — `(InventoryType == RELIC) == hasRelicSlot` — so a relic lights the ranged slot for a druid and a bow does not, on the bag bar and the paper doll alike; closing it half-right would have lit the highlight for a druid dragging a bow
- a chain `.xml` whose `.lua` never loaded: `MoneyInputFrame.xml` had been on the chain without its `.lua`, so the template loaded and all ten `MoneyInputFrame_*` verbs read nil, silently — a gate now fails any chain `.xml` whose `.lua` sibling is neither sourced nor listed; the gap report's LOAD column carries the probe's collision caveat, its fx column stops crying wolf (it follows `<Script file=>`, stops reading `.Field(` as a global, subtracts a file's own locals), and a window is its xml PLUS the lua it sources

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-09-02 17:01:18 +07:00
co-authored by Claude Fable 5.1
parent 112e82d276
commit feca23de3f
30 changed files with 981 additions and 404 deletions
@@ -11,27 +11,19 @@ use benilla_ui::script::{QuadContent, ScriptValue, UiScript, UnitState};
fn exp_bar_harness() -> UiScript {
let mut s = UiScript::new().unwrap();
s.set_screen_size(1024.0, 768.0);
// Through the shared both-stores loader, not a disk read off `assets/ui`: `TextStatusBar.xml`
// is the reference's own since 1751, and a hand-rolled reader here would look for it in the
// wrong store — and would leave its `<Script file="TextStatusBar.lua"/>` unresolved besides.
for file in [
"Fonts.xml",
"UIParent.xml",
"GameTooltip.xml",
"TextStatusBar.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"Cooldown.xml",
"ActionBar.xml",
] {
let text = std::fs::read_to_string(
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
.join("assets/ui")
.join(file),
)
.unwrap();
let doc = benilla_ui::framexml::parse(&text).unwrap();
let report = benilla_ui::loader::load(&s, &doc, &|_| None);
assert!(
report.errors.is_empty(),
"{file}: loader errors: {:?}",
report.errors
);
super::test_ui::load_ui(&s, file);
}
s
}
@@ -309,7 +301,12 @@ fn the_xp_bar_numerals_show_on_hover() {
.eval::<bool>("return MainMenuBarExpText:IsShown()")
.unwrap());
s.run("BenillaExpBar_OnLeave(MainMenuExpBar)").unwrap();
// `this` bound, because the engine binds it: the stock `HideTextStatusBarText` reads
// `this.isZero` on one line where every other line reads `bar` (TextStatusBar.lua:95 — the
// reference's own slip, benign there because `this` is always SOME frame during dispatch, and
// a nil index only from bare Lua). Called from the bar's own `<OnLeave>` in play.
s.run("this = MainMenuExpBar BenillaExpBar_OnLeave(MainMenuExpBar) this = nil")
.unwrap();
assert!(
!s.eval::<bool>("return MainMenuBarExpText:IsShown()")
.unwrap(),
@@ -115,28 +115,19 @@ const MOUSE_HANDLERS: &[&str] = &[
"OnDragStart",
];
/// Widget kinds whose **constructor** enables the mouse, so an element of that tag needs no
/// Whether an element's TAG is a widget kind the constructor mouse-enables — so it needs no
/// `enableMouse` and no handler to be clickable.
///
/// This is deliberately the SAME list [`benilla_ui::widget::WidgetArena::create`] uses, and it has
/// to be: both are one claim about the client's ctors (`CSimpleButton`/`CSimpleEditBox` enable
/// input in theirs; a `Slider` thumb must be draggable; a scroll frame takes the wheel — each
/// carries its own citation there). Reading the engine's list here means a wrong entry is wrong in
/// one place rather than two, and the sweep never reports a divergence that is only the two models
/// disagreeing with each other.
///
/// It is also what answers the chat window: `ChatFrame1` is a `ScrollingMessageFrame`, so the
/// reference's takes the mouse from its ctor even though `FloatingChatFrameTemplate` writes
/// `enableMouse="false"` — which is a no-op, the XML path only ever ENABLES
/// (`scripts-auto-enable.md` §1.3). That is how a 1.12 chat link is clickable at all.
const MOUSE_BY_CTOR: &[&str] = &[
"Button",
"CheckButton",
"EditBox",
"ScrollingMessageFrame",
"ScrollFrame",
"Slider",
];
/// **Answered by the engine, not by a copy of its list.** This used to be a `MOUSE_BY_CTOR` array
/// here whose own comment said it was "deliberately the SAME list `WidgetArena::create` uses… so a
/// wrong entry is wrong in one place rather than two". It was a second list, and it drifted
/// silently the first time the engine's was edited — this sweep then reported six scroll frames as
/// divergences that were only the two models disagreeing with each other, which is precisely what
/// the comment claimed could not happen.
fn tag_mouse_enabled_by_ctor(tag: &str) -> bool {
benilla_ui::script::frame_kind_from_tag(tag)
.is_some_and(benilla_ui::widget::mouse_enabled_by_ctor)
}
/// One accepted difference between benilla and the reference, with the reason it is accepted.
///
@@ -185,6 +176,102 @@ const KNOWN: &[Known] = &[
"the reference bar inherits TextStatusBar, whose OnEnter/OnLeave show the value text; \
ours is a plain StatusBar. Wants the status-text arc, not the flag.",
},
// Seven `*ScrollFrame` mouse entries RETIRED here (1795). They read "our faux scroll pane is a
// Frame with an explicit `<OnMouseWheel>`, not a ScrollFrame" — a divergence that existed only
// because OUR `ScrollFrame` ctor took the mouse and the reference's does not. Correcting the
// ctor list to the client's made both sides agree, and an accepted divergence that has been
// fixed is documentation claiming a defect we do not have.
Known {
frame: "ChatFrame1",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame2",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame3",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame4",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame5",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame6",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "ChatFrame7",
flag: Flag::Mouse,
why: "our chat window takes the mouse and the reference's does not. It was believed a \
`<ScrollingMessageFrame>` takes it BY CONSTRUCTION; wow-re's follow-up round \
(`68987021`) proved that ctor leaves the bit clear, and that a 1.12 chat link is \
clickable through one synthesised `CSimpleHyperlinkButton` child per link span. Our \
hit test now carries that law as a span disjunct, so the LINKS no longer need the \
flag — but ChatFrame.xml still sets it, and a documented chain of choices hangs off \
the old premise (no `SetFrameLevel-1` OnLoad because it would sink the resize grips \
under a mouse-enabled parent; `OnClick` dismisses a held spell, 0843). Unwinding \
that is the chat window's own piece of work, not this flag's.",
},
Known {
frame: "TargetofTargetHealthBar",
flag: Flag::Mouse,
@@ -242,41 +329,6 @@ const KNOWN: &[Known] = &[
//
// The two mail panes are a further step out: ours are flat art with a FontString body, a
// render approximation MailFrame.xml names at the site, so neither side scrolls them.
Known {
frame: "FriendsFrameFriendsScrollFrame",
flag: Flag::Mouse,
why: "our faux scroll pane is a Frame with an explicit <OnMouseWheel>, not a ScrollFrame",
},
Known {
frame: "FriendsFrameIgnoreScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
Known {
frame: "GuildListScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
Known {
frame: "WhoListScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
Known {
frame: "SkillListScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
Known {
frame: "ReputationListScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
Known {
frame: "QuestLogListScrollFrame",
flag: Flag::Mouse,
why: "as FriendsFrameFriendsScrollFrame",
},
// ── id ─────────────────────────────────────────────────────────────────────────────────────
//
// Empty, and it is worth saying why rather than leaving a bare header. The reference side of
@@ -482,10 +534,7 @@ fn reference_takes_mouse(name: &str, frames: &HashMap<String, Element>) -> bool
let Some(el) = frames.get(name) else {
return false;
};
if MOUSE_BY_CTOR
.iter()
.any(|t| t.eq_ignore_ascii_case(&el.tag))
{
if tag_mouse_enabled_by_ctor(&el.tag) {
return true;
}
if resolved_attr(name, frames, "enableMouse", 0).is_some_and(|v| v.eq_ignore_ascii_case("true"))
@@ -71,7 +71,8 @@ fn load_page(s: &UiScript) {
"UiPanels.xml",
"UIParent.xml",
"GameTooltip.xml",
"TextStatusBar.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"ScrollTemplates.xml",
"UIPanelTemplates.xml",
"OptionsFrameTemplates.xml",
@@ -728,7 +728,8 @@ fn interface_harness() -> UiScript {
"GameTooltip.xml",
"UIDropDownMenu.xml",
"UnitPopup.xml",
"TextStatusBar.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"UnitFrames.xml",
"Cooldown.xml",
"ActionBar.xml",
+389 -13
View File
@@ -395,13 +395,13 @@ mod tests {
/// manifest in a fresh VM — the same pass [`chain_readiness_report`] makes, run here so the
/// answer is in one table.
///
/// That last one is 1790, and it is the same mistake as the third column one step later.
/// That last one is 1801, and it is the same mistake as the third column one step later.
/// `<LootButton>` and `<TaxiRouteFrame>` are element TAGS: no census of `Name(` or `:Name(`
/// can reach them, so `LootFrame.xml` sat in this report's "needs NO engine work" list while
/// the readiness probe was printing `4 issue(s)` for it in a different table. Both tables were
/// right. Joining them was left to whoever read them, and I read it wrong.
///
/// Until 1787 the method sites were **silently dropped**: a name in neither the engine nor the
/// Until 1798 the method sites were **silently dropped**: a name in neither the engine nor the
/// FrameXML half of `1.12-globals.tsv` was assumed to be a widget method and skipped, on the
/// reasoning that widget methods are not in `_G`. True, and it meant the report could not see
/// a widget method we had *not built*. `MerchantFrame.xml` read `0 engine` and
@@ -410,10 +410,16 @@ mod tests {
/// file loaded, every check passed, and hovering a vendor row would have raised in play. Both
/// instruments were right about what they measure. Neither measured the window.
///
/// **A remaining `<?>` in the `fx=` column is usually a LoadOnDemand addon**, not something to
/// build. `ClassTrainerFrame_Show`, `CraftFrame_Show`, `MacroFrame_SaveMacro`,
/// `InspectFrame_Show`, `TalentFrame_Toggle` and their siblings live in `Blizzard_*` addons the
/// install ships **packed as `.pub`**, so no amount of scanning the extracted FrameXML finds
/// them. They arrive when that addon does, exactly like an `fx=` name with a home.
///
/// One crudeness remains, stated because it decides how to read the output: neither scan can
/// see a name reached through `getglobal`, so both can under-report.
///
/// They no longer over-report on comments. Both counted commented-out calls until 1789 — 1.12
/// They no longer over-report on comments. Both counted commented-out calls until 1800 — 1.12
/// comments out whole blocks, and `PaperDollFrame.lua:754-756`'s `ShowInventorySellCursor` is
/// three commented lines that this report named as that window's last engine gap. A real
/// binding, never called, blocking a window that was not blocked. [`strip_comments`] takes
@@ -457,7 +463,7 @@ mod tests {
.collect();
// Strip what is not code before either scan. Both `Name(` and `:Name(` counted calls
// inside comments until 1789: the round that built `PickupMerchantItem` also asked wow-re
// inside comments until 1800: the round that built `PickupMerchantItem` also asked wow-re
// for `ShowInventorySellCursor`, which this report had named as `PaperDollFrame.xml`'s
// last engine gap — and the answer was that stock `PaperDollFrame.lua:754-756` has the
// call **commented out**, all three lines. A real binding, never called, blocking a window
@@ -604,7 +610,7 @@ mod tests {
.collect()
};
// The instrument's own tripwire: if the probe stops working, the `method=` column goes
// silently empty — which is precisely the failure mode 1787 exists to end.
// silently empty — which is precisely the failure mode 1798 exists to end.
let control: Vec<String> = ["SetPoint", "SetMerchantItem", "BenillaNotAMethod"]
.iter()
.map(|s| (*s).to_string())
@@ -636,10 +642,48 @@ mod tests {
.map(str::to_string)
.collect();
// The `.lua` files a stock `.xml` SOURCES through `<Script file=>`. A window's own sourced
// code is part of the window: its functions are not gaps, and they are where most of its
// `fx=` names would otherwise be looked for.
let sourced_luas = |xml_leaf: &str| -> Vec<String> {
let Some(b) = super::read(&format!("Interface\\FrameXML\\{xml_leaf}")) else {
return Vec::new();
};
let text = String::from_utf8_lossy(&b).into_owned();
let mut out = Vec::new();
for (i, _) in text.match_indices("<Script") {
let rest = &text[i..];
let Some(end) = rest.find("/>").or_else(|| rest.find('>')) else {
continue;
};
let tag = &rest[..end];
if let Some(fi) = tag.find("file=\"") {
let after = &tag[fi + 6..];
if let Some(q) = after.find('"') {
let leaf = after[..q].rsplit(['\\', '/']).next().unwrap_or("");
if leaf.ends_with(".lua") {
out.push(leaf.to_string());
}
}
}
}
out
};
// `function Name(` across the whole corpus, so an fx gap can name the file that holds it.
//
// Each toc `.xml` is scanned together with **every `.lua` it SOURCES**, not just the
// `X.lua` its own name suggests. Guessing missed the common case: `ActionBarFrame.xml`
// sources `ActionButton.lua`, there is no `ActionBarFrame.lua`, and `ActionButton.lua` is
// not a toc line of its own — so the whole `ActionButton_*` family read `<?>`, which says
// "nothing defines this" about five functions a stock file defines and brings with it.
// The difference matters for how the column is read: a name with a home ARRIVES when that
// file does; a `<?>` is something to build.
let mut home: std::collections::HashMap<String, String> = std::collections::HashMap::new();
for f in &stock {
for cand in [f.clone(), format!("{}.lua", &f[..f.len() - 4])] {
let mut cands = vec![f.clone(), format!("{}.lua", &f[..f.len() - 4])];
cands.extend(sourced_luas(f));
for cand in cands {
let Some(bytes) = super::read(&format!("Interface\\FrameXML\\{cand}")) else {
continue;
};
@@ -662,7 +706,16 @@ mod tests {
let mut out = std::collections::HashSet::new();
let mut i = 0;
while i < b.len() {
if b[i].is_ascii_uppercase() && (i == 0 || !super::is_word(b[i - 1])) {
// A GLOBAL call: uppercase, not inside a word, and **not preceded by `.` or
// `:`**. Without that last test `info.UpdateFunc(` and `dialog.OnAccept(` read as
// globals — `.` is not a word character — and a dozen `StaticPopupDialogs` /
// `MoneyTypeInfo` FIELD names showed up as `<?>` gaps. `:` belongs to the method
// scan below; `.` belongs to nobody, since a field call arrives with its table.
let after_field = i > 0 && (b[i - 1] == '.' || b[i - 1] == ':');
if b[i].is_ascii_uppercase()
&& !after_field
&& (i == 0 || !super::is_word(b[i - 1]))
{
let mut j = i;
while j < b.len() && super::is_word(b[j]) {
j += 1;
@@ -696,13 +749,39 @@ mod tests {
if migrated.contains(f) {
continue;
}
// The window IS its xml plus every `.lua` it sources — `ActionBarFrame.xml` sources
// `ActionButton.lua`, and the whole `ActionButton_*` family is that window's own code,
// not a dependency on somebody else's. Gathering only `X.xml` + `X.lua` reported five
// of its own functions as gaps.
let mut text = String::new();
for cand in [f.clone(), format!("{}.lua", &f[..f.len() - 4])] {
let mut parts = vec![f.clone(), format!("{}.lua", &f[..f.len() - 4])];
parts.extend(sourced_luas(f));
for cand in parts {
if let Some(b) = super::read(&format!("Interface\\FrameXML\\{cand}")) {
text.push_str(&String::from_utf8_lossy(&b));
}
}
let text = strip_comments(&text);
// …and the names the file declares as LOCALS, which a bare call resolves to. The
// idiom that needs it is the reference's own dispatch shape:
//
// local OnAccept = StaticPopupDialogs[dialog.which].OnAccept
// if ( OnAccept ) then dontHide = OnAccept(dialog.data, dialog.data2) end
//
// The call is bare, so the global scan sees `OnAccept(` and reports a gap for a name
// that is a table field one line up. `OnAccept`, `OnCancel`, `OnShow`, `OnHide` and
// the three `EditBoxOn*` all arrived that way. A `local` declaration in the same file
// is proof enough: nothing else could be meant.
let locals: std::collections::HashSet<String> = text
.lines()
.filter_map(|l| l.trim_start().strip_prefix("local "))
.map(|r| {
r.chars()
.take_while(|c| c.is_alphanumeric() || *c == '_')
.collect::<String>()
})
.filter(|n| !n.is_empty())
.collect();
let own: std::collections::HashSet<String> = text
.lines()
.filter_map(|l| l.trim_start().strip_prefix("function "))
@@ -715,7 +794,7 @@ mod tests {
let (mut eng, mut fx) = (Vec::new(), Vec::new());
let mut names: Vec<String> = called(&text)
.into_iter()
.filter(|c| !own.contains(c) && !have.contains(c))
.filter(|c| !own.contains(c) && !locals.contains(c) && !have.contains(c))
.collect();
names.sort();
for c in names {
@@ -737,8 +816,30 @@ mod tests {
// while `chain_readiness_report` was printing `4 issue(s)` for it in another table.
//
// Both tables were right. Joining them was left to whoever read them, and I got it
// wrong (1790). So this one runs the load itself — the same fresh-VM-plus-manifest
// wrong (1801). So this one runs the load itself — the same fresh-VM-plus-manifest
// pass the readiness probe does — and reports it in the same row.
// **A `LOAD:` line for a window we ALSO ship is suspect**, and the reason is the same
// one `chain_readiness_report` carries: `publish_global` is non-overwriting (RF-0023),
// so loading the stock file on top of our identically-named one leaves every colliding
// frame's global pointing at OUR frame while the stock file's handlers run against
// THEIRS. A field set on `this` in an OnLoad is then invisible to `getglobal(name)`,
// and the failure looks like a bug in whatever read it back.
//
// Worked example, because it cost an hour: stock `TradeFrame.xml` reported
// `MoneyFrame.xml:525: attempt to index local 'info'`. Nothing was wrong with our
// MoneyFrame — `MoneyFrame_SetType` had set `this.info` correctly, and
// `getglobal(this:GetName())` answered a DIFFERENT table, because our own
// `TradeFrame.xml` already owned `TradeRecipientMoneyFrame`.
//
// So the column is marked, not trusted. A row whose file we do not ship is a real
// load failure; a row whose file we do ship needs the swap attempted to know.
let ours_too = !migrated.contains(f)
&& std::fs::metadata(
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
.join("assets/ui")
.join(f),
)
.is_ok();
let loads = {
let mut probe = UiScript::new().expect("VM");
probe.set_screen_size(1024.0, 768.0);
@@ -759,8 +860,22 @@ mod tests {
let known = answers(&s, &asked);
let mut meth: Vec<String> = asked.into_iter().filter(|m| !known.contains(m)).collect();
meth.sort();
// A suspect LOAD line does not count as a blocker — it is a question, not an answer.
let load_blockers = if ours_too { 0 } else { loads.len() };
let loads: Vec<String> = loads
.into_iter()
.map(|e| {
let one = e.replace('\n', " ");
let mark = if ours_too {
" (ours too — suspect)"
} else {
""
};
format!("{mark} {}", &one[..one.len().min(150)])
})
.collect();
rows.push((
eng.len() + meth.len() + loads.len(),
eng.len() + meth.len() + load_blockers,
f.clone(),
eng,
fx,
@@ -778,8 +893,7 @@ mod tests {
println!(" method: {}", meth.join(" "));
}
for e in loads {
let one = e.replace('\n', " ");
println!(" LOAD: {}", &one[..one.len().min(150)]);
println!(" LOAD:{e}");
}
if !fx.is_empty() {
println!(" fx: {}", fx.join(" "));
@@ -816,12 +930,103 @@ mod tests {
/// load, which is what this reports: a name ours defines that a chain entry in our own
/// manifest also defines.
///
/// **Two halves, both exact, because both sides DECLARE rather than mention.** The function
/// half is above. The frame half asks the other question a swap has to answer: *can this stock
/// window be added at all* — which is different from "does it load", because a stock window we
/// do not ship under its own name usually has a counterpart of ours under a different one, and
/// both would declare the same frames.
///
/// That half doubles as the map nothing else holds: `ActionBarFrame.xml` is our
/// `ActionBar.xml`, `FloatingChatFrame.xml` is our `ChatFrame.xml`, `PlayerFrame.xml` and
/// `TargetFrame.xml` and `PetFrame.xml` are all our one `UnitFrames.xml`,
/// `MainMenuBarMicroButtons.xml` is our `MicroMenu.xml`, `StaticPopup.xml` is our
/// `UiPanels.xml`. `chain_gap_report` calls several of those unblocked, and they are —
/// individually. They just cannot load beside the file of ours already holding their names.
///
/// Run it before attempting a swap. It predicts which ones will fail without attempting them.
#[test]
#[ignore = "instrument: run by hand before attempting a window swap"]
fn shadowed_reference_functions() {
let _data = benilla_formats::wow_data_or_skip!();
// The FRAME NAMES a document declares — `name="X"` on a widget element, minus the
// `$parent`-relative and `virtual` template forms, which name nothing globally.
//
// The sibling of the function check below and exactly as exact, because both sides
// DECLARE rather than mention. This is the version of the "frame names" idea that works:
// an earlier attempt scanned names a file *referenced* and was pure noise (it found `UI`
// and missed the case it was built for), because a reference can live in any file. A
// declaration cannot.
//
// What it catches: our `UnitFrames.xml` declares `PetFrame`, and so does the stock
// `PetFrame.xml`. Adding that stock file alongside ours would declare the name twice.
// Several stock windows we do not ship under their own name have an equivalent of ours
// under a different one, and `chain_gap_report` calls every one of them unblocked —
// truthfully, because the stock file WOULD load; it just cannot load *beside* ours.
let declares = |text: &str| -> Vec<String> {
let mut out = Vec::new();
for (i, _) in text.match_indices("name=\"") {
let rest = &text[i + 6..];
let Some(end) = rest.find('"') else { continue };
let name = &rest[..end];
if name.starts_with('$') || name.is_empty() {
continue;
}
// A `virtual="true"` element is a template: its name is a registry key, not a
// frame, and two files may legitimately hold the same template name only if one
// replaces the other — which is the same question, so they are reported too.
out.push(name.to_string());
}
out
};
// `function Name(a, b, c)` → the name and its PARAMETER LIST. Arity is the third way our
// files and the reference's can disagree about a name, and the one that fails most
// quietly: `TextStatusBar_Initialize()` takes no argument in 1.12 and acts on `this`, ours
// took an optional bar, and `UnitFrames.xml` calls it with one. Swapping that file stopped
// initialising the unit-frame bars with no error and no missing global — the numerals
// simply never appeared (decision 1793).
//
// **Both directions are silent, and that is the point.** Lua drops extra arguments without
// complaint, so neither an over- nor an under-supplied call raises; they differ only in
// WHO loses information:
//
// * ours WIDER — our callers pass the extra argument and the reference's version drops
// it. Breaks when OUR file is swapped out (the `TextStatusBar` case).
// * ours NARROWER — the reference's callers pass more than ours takes and ours drops it.
// Breaks when a STOCK file is added beside ours and calls our version.
//
// The report names the direction because it says which swap the difference is waiting for,
// not because one of them is safe.
let params_in = |text: &str| -> Vec<(String, usize)> {
let mut out = Vec::new();
for line in text.lines() {
let Some(rest) = line.trim_start().strip_prefix("function ") else {
continue;
};
let name: String = rest
.chars()
.take_while(|c| c.is_alphanumeric() || *c == '_')
.collect();
if name.is_empty() {
continue;
}
let Some(open) = rest.find('(') else { continue };
let Some(close) = rest[open..].find(')') else {
continue;
};
let args = rest[open + 1..open + close].trim();
// `...` is a vararg, which has no fixed arity to compare.
let n = if args.is_empty() || args == "..." {
0
} else {
args.split(',').count()
};
out.push((name, n));
}
out
};
let defined_in = |text: &str| -> Vec<String> {
text.lines()
.filter_map(|l| l.trim_start().strip_prefix("function "))
@@ -841,6 +1046,8 @@ mod tests {
toc.iter().enumerate().map(|(i, f)| (f, i)).collect();
let mut chain_home: std::collections::HashMap<String, (String, usize)> =
std::collections::HashMap::new();
let mut chain_frames: std::collections::HashMap<String, String> =
std::collections::HashMap::new();
for entry in toc.iter().filter(|f| super::is_chain_entry(f)) {
let leaf = entry.rsplit(['\\', '/']).next().unwrap_or(entry);
let mut cands = vec![entry.clone()];
@@ -878,6 +1085,67 @@ mod tests {
}
hits.sort();
hits.dedup();
// The frame names every STOCK window declares — read off the reference's own toc, not off
// our manifest's chain entries. Scanning only what we already load was the first attempt
// and it answered 0 by construction: a stock file we do not load is exactly the one whose
// names could collide, and it was never opened.
let ref_toc = String::from_utf8_lossy(
&super::read("Interface\\FrameXML\\FrameXML.toc").expect("the reference's own toc"),
)
.into_owned();
let mut ref_arity: std::collections::HashMap<String, (usize, String)> =
std::collections::HashMap::new();
for line in ref_toc.lines().map(str::trim) {
if line.is_empty() || line.starts_with('#') || !line.ends_with(".xml") {
continue;
}
for cand in [line.to_string(), format!("{}.lua", &line[..line.len() - 4])] {
let Some(b) = super::read(&format!("Interface/FrameXML/{cand}")) else {
continue;
};
let text = String::from_utf8_lossy(&b).into_owned();
if cand.ends_with(".xml") {
for name in declares(&text) {
chain_frames.entry(name).or_insert_with(|| line.to_string());
}
}
for (name, n) in params_in(&text) {
ref_arity.entry(name).or_insert((n, cand.clone()));
}
}
}
assert!(
chain_frames.contains_key("GameTooltip") && chain_frames.contains_key("PetFrame"),
"the stock frame-name scan found nothing recognisable ({} names) — an empty answer \
here reads as \"no collisions\", which is what the first version of this reported \
for the wrong reason",
chain_frames.len()
);
// …and the frame-name half, over the stock files the manifest does NOT already take off
// the chain. A file of ours whose stock counterpart we already load is a swap that has
// happened; this is about the ones that have not.
let mut frame_hits: Vec<(String, String, String)> = Vec::new();
for entry in toc.iter().filter(|f| !super::is_chain_entry(f)) {
let Ok(text) = std::fs::read_to_string(dir.join(entry)) else {
continue;
};
for name in declares(&text) {
if let Some(home) = chain_frames.get(&name) {
let already = toc
.iter()
.filter(|f| super::is_chain_entry(f))
.any(|f| f.ends_with(home.as_str()));
// A template's name is a registry key rather than a frame, but two files
// holding one is the same question, so it is reported the same way.
if !already {
frame_hits.push((name, entry.clone(), home.clone()));
}
}
}
}
frame_hits.sort();
frame_hits.dedup();
println!("\n=== names ours redefines that a CHAIN entry already defines ===");
println!("{:<36} {:<28} {:<34} winner", "name", "ours", "chain");
@@ -901,6 +1169,114 @@ mod tests {
for (f, n) in &by_file {
println!(" {n:>3} {f}");
}
// The ARITY half: a name we define that the reference also defines, with a different
// parameter count. Not a collision — the two need never both load for this to bite — so it
// is its own section rather than a column.
let mut arity: Vec<(String, String, usize, usize, String)> = Vec::new();
for entry in toc.iter().filter(|f| !super::is_chain_entry(f)) {
let Ok(text) = std::fs::read_to_string(dir.join(entry)) else {
continue;
};
for (name, ours_n) in params_in(&text) {
if let Some((ref_n, home)) = ref_arity.get(&name) {
if *ref_n != ours_n {
arity.push((name, entry.clone(), ours_n, *ref_n, home.clone()));
}
}
}
}
arity.sort();
arity.dedup();
println!(
"\n=== {} signatures of ours differ in ARITY from the reference's ===",
arity.len()
);
println!("{:<34} {:<26} ours ref direction", "name", "ours");
for (name, ours, a, b, home) in &arity {
// Which swap this one is waiting for — see the note above; both are silent.
let dir = if a > b {
"ours WIDER — bites when OUR file goes"
} else {
"ours NARROWER — bites when THEIRS arrives"
};
println!("{name:<34} {ours:<26} {a:>4} {b:>3} {dir} — ref in {home}");
}
// The frame half, grouped the other way — by the STOCK file, because that is the unit of
// the question it answers: "can this stock window be added?"
let mut by_stock: std::collections::BTreeMap<&String, std::collections::BTreeSet<&String>> =
std::collections::BTreeMap::new();
for (_, ours, home) in &frame_hits {
by_stock.entry(home).or_default().insert(ours);
}
println!(
"\n=== {} FRAME-NAME collisions: {} stock windows we do not load already have their \
names declared by a file of ours ===",
frame_hits.len(),
by_stock.len()
);
for (stock, ours) in &by_stock {
let n = frame_hits.iter().filter(|(_, _, h)| h == *stock).count();
let mine: Vec<&str> = ours.iter().map(|s| s.as_str()).collect();
println!(" {n:>3} {stock:<34} vs {}", mine.join(", "));
}
}
/// **A chain `.xml` that does not source its own `.lua` needs TWO manifest lines**, and
/// getting it wrong is silent.
///
/// Most stock windows pull their code in with `<Script file="X.lua"/>`, so naming the `.xml`
/// brings both. A few do not — `TextStatusBar.xml` and `MoneyInputFrame.xml` declare only a
/// template, and the reference's own toc lists their `.lua` on the preceding line (l.32-33,
/// l.11-12). Name the `.xml` alone and the template loads against nothing: every global that
/// file was supposed to define reads nil, every guarded call becomes a no-op, and there is no
/// error anywhere.
///
/// Both were live. `TextStatusBar` was caught by three tests that happened to assert on the
/// numerals; `MoneyInputFrame` was caught only by sweeping for the shape afterwards, and its
/// manifest header had claimed for months to bring "the ten `MoneyInputFrame_*` verbs" while a
/// full-manifest probe answered nil for all of them.
///
/// A gate rather than an instrument, because the answer should always be zero and the failure
/// mode is invisible.
#[test]
fn every_chain_xml_brings_its_own_lua() {
let _data = benilla_formats::wow_data_or_skip!();
let toc = &super::super::addons::Addon::builtin().toc.files;
let listed: std::collections::HashSet<&str> = toc
.iter()
.map(|f| f.rsplit(['\\', '/']).next().unwrap_or(f))
.collect();
let mut orphans = Vec::new();
for entry in toc.iter().filter(|f| super::is_chain_entry(f)) {
let leaf = entry.rsplit(['\\', '/']).next().unwrap_or(entry);
let Some(stem) = leaf.strip_suffix(".xml") else {
continue;
};
let lua = format!("{stem}.lua");
// No sibling in the archive means there is nothing to miss.
if super::read(&format!("Interface/FrameXML/{lua}")).is_none() {
continue;
}
let xml = super::read(&entry.replace('\\', "/"))
.unwrap_or_else(|| panic!("{entry}: not in the chain"));
let text = String::from_utf8_lossy(&xml);
let sourced = text
.to_ascii_lowercase()
.contains(&format!("file=\"{}\"", lua.to_ascii_lowercase()));
if !sourced && !listed.contains(lua.as_str()) {
orphans.push(format!(
"{leaf} does not source {lua}, and {lua} is not a manifest entry"
));
}
}
assert!(
orphans.is_empty(),
"a chain window whose code never loads — silent, every global it defines reads nil:\n {}",
orphans.join("\n ")
);
}
/// A path is a chain entry; a bare name is ours. The one-line rule the manifest rests on.
@@ -87,7 +87,8 @@ fn load_page(s: &UiScript) {
"UiPanels.xml",
"UIParent.xml",
"GameTooltip.xml",
"TextStatusBar.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"Cooldown.xml",
"ActionBar.xml",
"ScrollTemplates.xml",
@@ -191,3 +191,68 @@ fn the_death_countdown_text_rerenders_each_tick() {
);
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
/// **`StaticPopup_Hide`'s and `StaticPopup_FindVisible`'s second argument** (decisions 1793).
///
/// Both took `which` alone until the arity scan found the drift. The reference takes `(which,
/// data)`, and the difference is invisible without a test: Lua discards a surplus argument without
/// complaint, so a stock caller's `StaticPopup_Hide("CONFIRM_BATTLEFIELD_ENTRY", i)` — the shape
/// `BattlefieldFrame.lua:120` uses — would have hidden every instance of that dialog instead of
/// the one it named, and said nothing.
///
/// The `multiple` gate is the reference's own and is why `data` is not a plain equality: a kind
/// declared `multiple` can have several instances up at once and is matched by data; every other
/// kind has at most one, so its data is never consulted.
#[test]
fn hide_and_find_address_one_instance_by_data_only_for_a_multiple_dialog() {
let s = setup();
s.run(
r#"
StaticPopupDialogs["T_MULTI"] = { text = "multi", button1 = "Ok", multiple = 1, timeout = 0 }
StaticPopupDialogs["T_ONE"] = { text = "one", button1 = "Ok", timeout = 0 }
a = StaticPopup_Show("T_MULTI") a.data = "alpha"
b = StaticPopup_Show("T_MULTI") b.data = "beta"
c = StaticPopup_Show("T_ONE") c.data = "gamma"
"#,
)
.unwrap();
assert!(s.errors().is_empty(), "{:?}", s.errors());
// A `multiple` kind is addressed by data — both ways round, so this cannot pass by finding
// whichever instance happens to be first.
for (data, want) in [("alpha", "a"), ("beta", "b")] {
assert!(
s.eval::<bool>(&format!(
"return StaticPopup_FindVisible(\"T_MULTI\", {data:?}) == {want}"
))
.unwrap(),
"FindVisible picks the instance carrying {data:?}"
);
}
// …and a data nothing carries finds nothing, rather than the first instance.
assert!(s
.eval::<bool>(r#"return StaticPopup_FindVisible("T_MULTI", "nobody") == nil"#)
.unwrap());
// Hiding by data takes ONE instance and leaves its sibling up.
s.run(r#"StaticPopup_Hide("T_MULTI", "alpha")"#).unwrap();
assert!(s
.eval::<bool>("return not a:IsShown() and b:IsShown()")
.unwrap());
// No data hides every instance of the kind — which is what every one-argument caller means.
s.run(r#"StaticPopup_Hide("T_MULTI")"#).unwrap();
assert!(s.eval::<bool>("return not b:IsShown()").unwrap());
// A NON-multiple kind ignores data entirely: the reference gates the comparison on
// `info.multiple`, so a wrong data still finds it.
assert!(s
.eval::<bool>(r#"return StaticPopup_FindVisible("T_ONE", "wrong") == c"#)
.unwrap());
// An unknown kind answers nil rather than scanning — the reference's own early out.
assert!(s
.eval::<bool>(r#"return StaticPopup_FindVisible("T_NOPE") == nil"#)
.unwrap());
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
@@ -17,7 +17,8 @@ fn load_unit_frames(s: &UiScript) {
load_xml(s, "UIParent.xml");
// The bars' numerals machinery (decision 1082), which the manifest loads immediately ahead of
// UnitFrames.xml and which every bar's OnLoad wires into since 1143.
load_xml(s, "TextStatusBar.xml");
load_xml(s, "Interface\\FrameXML\\TextStatusBar.lua");
load_xml(s, "Interface\\FrameXML\\TextStatusBar.xml");
load_xml(s, "GameTooltip.xml");
load_xml(s, "UIDropDownMenu.xml");
load_xml(s, "UnitPopup.xml");
@@ -1516,8 +1517,13 @@ fn status_bar_text_paints_the_player_numerals_but_not_the_targets() {
.unwrap();
assert!(shown(&s, "TargetFrameHealthBarText"));
assert_eq!(text(&s, "TargetFrameHealthBarText"), "50 / 100");
s.run("BenillaUnitFrameBar_OnLeave(TargetFrameHealthBar)")
.unwrap();
// `this` bound, as the engine binds it for a bar's own `<OnLeave>`: the stock
// `HideTextStatusBarText` reads `this.isZero` on one line where every other reads `bar`
// (TextStatusBar.lua:95 — the reference's own slip; a nil index only from bare Lua).
s.run(
"this = TargetFrameHealthBar BenillaUnitFrameBar_OnLeave(TargetFrameHealthBar) this = nil",
)
.unwrap();
assert!(
!shown(&s, "TargetFrameHealthBarText"),
"and go away again — the lockShow refcount balances"
@@ -1535,8 +1541,10 @@ fn status_bar_text_paints_the_player_numerals_but_not_the_targets() {
shown(&s, "PlayerFrameHealthBarText"),
"hover shows them even with the option off"
);
s.run("BenillaUnitFrameBar_OnLeave(PlayerFrameHealthBar)")
.unwrap();
s.run(
"this = PlayerFrameHealthBar BenillaUnitFrameBar_OnLeave(PlayerFrameHealthBar) this = nil",
)
.unwrap();
assert!(!shown(&s, "PlayerFrameHealthBarText"));
s.run("SetCVar(\"statusBarText\", \"1\", \"STATUS_BAR_TEXT\")")
.unwrap();