our API surface is diffed against the client's 19,572 globals

- `--surface` re-runs the comparison against the captured in-world global table on demand, and `--surface-dump` writes our raw side for any tool to join — a one-shot measurement stops drifting
- CreateFrame with an unresolvable template raises exactly as the client does, instead of handing back a bare frame; `inherits=` is one name, matched case-insensitively
- CreateFontString and CreateTexture honour their third argument — 49 corpus call sites were passing an inherit into the void
- the addon-message inbound byte map is corrected (a test had agreed with the wrong constants), and the group-loot roll template publishes under the reference's own name

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-08-14 19:08:30 +07:00
co-authored by Claude Fable 5
parent a6aecb98df
commit d7f4c59fa8
19 changed files with 795 additions and 121 deletions
+7 -3
View File
@@ -71,6 +71,10 @@ Early inspiration and file format guidance came from the
[warcraft-rs](https://github.com/wowemulation-dev/warcraft-rs) in particular.
benilla is an independent fan project, not affiliated with or endorsed by Blizzard Entertainment.
It contains no Blizzard code or assets, and you provide your own legally obtained 1.12.1 client
for game data. World of Warcraft is a trademark of Blizzard Entertainment, Inc. Licensed under
[MIT](LICENSE-MIT) or [Apache 2.0](LICENSE-APACHE), at your option.
It ships **no Blizzard game data** — no art, models, sounds, maps or MPQ contents — and you provide
your own legally obtained 1.12.1 client for all of it. Parts of the interface layer under
`crates/benilla-app/assets/ui/` are **transcribed from the client's own FrameXML** (its Lua and XML),
cited in place with the reference file and line range; that is Blizzard's authorship, not ours, and
it is not covered by the licence below. World of Warcraft is a trademark of Blizzard Entertainment,
Inc. Our own code is licensed under [MIT](LICENSE-MIT) or [Apache 2.0](LICENSE-APACHE), at your
option.
@@ -309,7 +309,17 @@
inside ref l.292's `<Anchor point="TOPLEFT">d` (a typo in Blizzard's own shipped file — not
transcribed).
-->
<Frame name="BenillaGroupLootFrameTemplate" frameStrata="DIALOG" hidden="true" virtual="true">
<!--
THE REFERENCE'S OWN NAME, deliberately un-prefixed (decision 1254).
This was `BenillaGroupLootFrameTemplate`. The `Benilla` prefix marks things WE invented, so
that nothing we made up can collide with a name an addon expects; this template is the
opposite of that — a line-by-line transcription of 1.12's `GroupLootFrameTemplate`, and
addons inherit it BY THAT NAME. Bongos_RollBar builds its four roll frames with
`CreateFrame("Frame", "BRollBarFrame"..i, bar, "GroupLootFrameTemplate")`, which resolves on
the real client and missed here for no reason but our prefix.
-->
<Frame name="GroupLootFrameTemplate" frameStrata="DIALOG" hidden="true" virtual="true">
<Size><AbsDimension x="243" y="84"/></Size>
<Layers>
<Layer level="ARTWORK">
@@ -476,26 +486,26 @@
<!-- The four instances (ref l.498-533): frame 1 at the ref's own hardcoded BOTTOM+60; each next
frame stacks on the previous one's TOP+15. -->
<Frame name="GroupLootFrame1" inherits="BenillaGroupLootFrameTemplate" id="1">
<Frame name="GroupLootFrame1" inherits="GroupLootFrameTemplate" id="1">
<Anchors>
<Anchor point="BOTTOM"><Offset><AbsDimension x="0" y="60"/></Offset></Anchor>
</Anchors>
</Frame>
<Frame name="GroupLootFrame2" inherits="BenillaGroupLootFrameTemplate" id="2">
<Frame name="GroupLootFrame2" inherits="GroupLootFrameTemplate" id="2">
<Anchors>
<Anchor point="BOTTOM" relativeTo="GroupLootFrame1" relativePoint="TOP">
<Offset><AbsDimension x="0" y="15"/></Offset>
</Anchor>
</Anchors>
</Frame>
<Frame name="GroupLootFrame3" inherits="BenillaGroupLootFrameTemplate" id="3">
<Frame name="GroupLootFrame3" inherits="GroupLootFrameTemplate" id="3">
<Anchors>
<Anchor point="BOTTOM" relativeTo="GroupLootFrame2" relativePoint="TOP">
<Offset><AbsDimension x="0" y="15"/></Offset>
</Anchor>
</Anchors>
</Frame>
<Frame name="GroupLootFrame4" inherits="BenillaGroupLootFrameTemplate" id="4">
<Frame name="GroupLootFrame4" inherits="GroupLootFrameTemplate" id="4">
<Anchors>
<Anchor point="BOTTOM" relativeTo="GroupLootFrame3" relativePoint="TOP">
<Offset><AbsDimension x="0" y="15"/></Offset>
+166 -1
View File
@@ -68,15 +68,180 @@ fn render_frames(frames: &[String]) -> String {
format!("{},+{} more", frames[..cap].join(","), frames.len() - cap)
}
/// Our `_G` against the captured 1.12 `_G` — decision 1189's diff, re-runnable.
///
/// 1189 did this once, by hand, and its finding was that **a superset is not free**: 1.12 addons
/// branch on presence (`if SomeName then`), so a name we publish that the reference does not have
/// can route an addon down a path the real client never takes. That makes BOTH directions of this
/// diff interesting, and the extra-names side the one nothing else in this harness can see — every
/// other ranking here is demand-driven, so it can only ever report what an addon *asked* for.
///
/// The reference side is `reference/1.12-globals.tsv`, itself generated from wow-re's live capture.
/// `lod` rows are the twelve LoadOnDemand `Blizzard_*` addons' names, unioned in by 1200 because a
/// live dump misses them unless the player opened those windows; they are counted separately rather
/// than silently folded in, since "absent from us" means something different for a window nobody
/// opened.
fn surface_report(deep: bool, dump_path: Option<String>) {
let tsv = concat!(
env!("CARGO_MANIFEST_DIR"),
"/../../reference/1.12-globals.tsv"
);
let Ok(text) = std::fs::read_to_string(tsv) else {
eprintln!("cannot read the reference surface at {tsv}");
std::process::exit(1);
};
let mut reference: std::collections::BTreeMap<String, String> = Default::default();
for line in text.lines().filter(|l| !l.starts_with('#')) {
let mut f = line.split('\t');
if let (Some(name), Some(kind)) = (f.next(), f.next()) {
if !name.is_empty() {
reference.insert(name.to_string(), kind.to_string());
}
}
}
let ours: std::collections::BTreeMap<String, String> =
addon_harness::surface().into_iter().collect();
// `--surface-dump <file>` writes our raw `_G` as `name<TAB>type`, the same shape the reference
// TSV has, so the two can be joined by anything. The printed report answers the questions I
// thought to ask; a sweep two sessions from now will have different ones, and re-deriving our
// side by scraping a human-readable report is exactly how a measurement quietly goes wrong.
if let Some(path) = dump_path {
let body: String = ours
.iter()
.map(|(n, k)| format!("{n}\t{k}\n"))
.collect::<Vec<_>>()
.concat();
match std::fs::write(&path, format!("# our _G — {} names\n{body}", ours.len())) {
Ok(()) => println!(" wrote {} names to {path}", ours.len()),
Err(e) => eprintln!(" could not write {path}: {e}"),
}
}
println!("\n SURFACE DIFF — our _G vs the captured 1.12 _G (decision 1189)");
println!(" FrameXML digest : {}", addon_harness::framexml_digest());
println!(" reference names : {}", reference.len());
println!(" ours : {}", ours.len());
// Absent from us, by the reference's own type — a function we lack is a different problem from
// a string we lack, and `lod` is a third thing again.
let mut missing_by_kind: std::collections::BTreeMap<&str, Vec<&str>> = Default::default();
for (name, kind) in &reference {
if !ours.contains_key(name) {
missing_by_kind
.entry(kind.as_str())
.or_default()
.push(name.as_str());
}
}
println!("\n ABSENT FROM US, by the reference's type:");
for (kind, names) in &missing_by_kind {
println!(" {:<10} {}", kind, names.len());
}
// The direction 1189 cared about and nothing else here can see.
//
// **Split by OUR type, because the total on its own misleads.** Most of these are `table` — the
// frame and region names our own FrameXML publishes, which differ from Blizzard's simply
// because our XML is our own reimplementation and names its pieces itself. That is not the
// hazard 1189 described. The hazard is a **function**: `if SomeApiName then` is how a 1.12
// addon feature-tests, and a verb we publish that the client never had can route it down a path
// the real client never takes. So the function row is the one to read first, and the one a
// future landing should be able to drive to zero.
let extra: Vec<(&str, &str)> = ours
.iter()
.filter(|(n, _)| !reference.contains_key(n.as_str()))
.map(|(n, k)| (n.as_str(), k.as_str()))
.collect();
let mut extra_by_kind: std::collections::BTreeMap<&str, Vec<&str>> = Default::default();
for (name, kind) in &extra {
extra_by_kind.entry(kind).or_default().push(name);
}
println!(
"\n PUBLISHED BY US, ABSENT FROM 1.12 ({}) — a superset is not free (1189):",
extra.len()
);
for (kind, names) in &extra_by_kind {
let note = match *kind {
"function" => " <- the ones an addon feature-tests; read these first",
"table" => " <- mostly our own FrameXML's frame/region names",
_ => "",
};
println!(" {:<10} {}{}", kind, names.len(), note);
}
// Functions in full even without --deep: it is the row that matters and it has to be readable.
//
// Split again on the `Benilla` prefix, which is the difference between a name that CAN collide
// with an addon's expectations and one that cannot. Nothing in the 1.12 corpus feature-tests
// `BenillaBagSlot_OnClick`; our own namespace is safe by construction, and leaving 443 of those
// in the list would bury the ones that are not.
if let Some(fns) = extra_by_kind.get("function") {
let (ours_ns, unprefixed): (Vec<&&str>, Vec<&&str>) = fns
.iter()
.partition(|n| n.starts_with("Benilla") || n.starts_with("BENILLA"));
println!(
"\n ...of those functions: {} are Benilla*-namespaced (cannot collide), {} are NOT:",
ours_ns.len(),
unprefixed.len()
);
for chunk in unprefixed.chunks(4) {
let row: Vec<&str> = chunk.iter().map(|s| **s).collect();
println!(" {}", row.join(" "));
}
println!(
" ^ read these: an unprefixed verb 1.12 lacks is what `if SomeName then` finds.\n \
Most are our own UI's helpers (AddonList_*, KeyBindings_*, Options*), which are only\n \
a naming question — but a POST-1.12 API name here is 1189's hazard exactly, because an\n \
addon that feature-tests it takes a branch written for a client we are not."
);
}
if deep {
for (kind, names) in &extra_by_kind {
if *kind == "function" {
continue;
}
println!("\n ...EXTRA {} ({}):", kind, names.len());
for chunk in names.chunks(4) {
println!(" {}", chunk.join(" "));
}
}
}
if deep {
for (kind, names) in &missing_by_kind {
println!("\n ABSENT FROM US — {} ({}):", kind, names.len());
for chunk in names.chunks(4) {
println!(" {}", chunk.join(" "));
}
}
}
}
fn main() {
let mut args = std::env::args().skip(1);
let Some(root) = args.next() else {
eprintln!(
"usage: addon_harness <folder of addons> [--verbose] [--why <blocker substring>]"
"usage: addon_harness <folder of addons> [--verbose] [--why <blocker substring>]\n \
or: addon_harness --surface (our _G vs the captured 1.12 surface; takes no corpus)"
);
std::process::exit(2);
};
let rest: Vec<String> = args.collect();
// `--surface` — decision 1189's comparison, made re-runnable. It needs no corpus, so it is
// handled before the root is used for anything.
if root == "--surface" || rest.iter().any(|a| a == "--surface") {
let dump_path = rest
.iter()
.position(|a| a == "--surface-dump")
.and_then(|i| rest.get(i + 1))
.cloned();
surface_report(rest.iter().any(|a| a == "--deep"), dump_path);
return;
}
let verbose = rest.iter().any(|a| a == "--verbose");
// `--why <substring>` — the addons behind one row, with their verbatim errors. The
// ranked table collapses quoted names by design (1193); this is the read-back, and two of this
+87 -14
View File
@@ -528,8 +528,6 @@ fn survey_one(
.union(&wants.tested_methods)
.cloned()
.collect();
eprintln!("INSTRMEASURE\t{name}\t{}", script.instructions_used());
eprintln!("INSTRM {name} {}", script.instructions_used());
let oracle = widget_method_kinds(&script, &asked);
let missing_methods = unresolved_from(&oracle, &wants.wanted_methods);
let optional_methods = unresolved_from(&oracle, &wants.tested_methods);
@@ -1081,22 +1079,48 @@ fn missing_templates(script: &UiScript, root: &Path, name: &str, toc: &Toc) -> V
let Some(close) = rest[open..].find(')') else {
continue;
};
let Some(fourth) = rest[open + 1..open + close].split(',').nth(3) else {
// Split the ARGUMENT LIST at depth 0 only. A plain `split(',')` cuts inside nested
// calls and strings, and it produced a phantom: AceGUI's
//
// CreateFrame("ScrollFrame", format("%s@%s@%s", Type, "ScrollFrame", …), backdrop, …)
//
// has its 4th comma-separated chunk INSIDE `format(...)`, so the census reported a
// missing template literally named `ScrollFrame` — which is a frame KIND, not a
// template, and named nothing that was ever missing. One of four rows in that table was
// fiction (1210/1218/1227, the fourth time a ranking here has opened with one).
let args = &rest[open + 1..open + close];
let mut depth = 0i32;
let mut quote: Option<char> = None;
let mut fields: Vec<&str> = Vec::new();
let mut start = 0usize;
for (bi, c) in args.char_indices() {
match (quote, c) {
(Some(q), c) if c == q => quote = None,
(Some(_), _) => {}
(None, '"' | '\'') => quote = Some(c),
(None, '(' | '[' | '{') => depth += 1,
(None, ')' | ']' | '}') => depth -= 1,
(None, ',') if depth == 0 => {
fields.push(&args[start..bi]);
start = bi + 1;
}
_ => {}
}
}
fields.push(&args[start..]);
let Some(fourth) = fields.get(3) else {
continue;
};
let f = fourth.trim();
// A quoted literal, either quote style. `inherits` is a comma-separated LIST in
// FrameXML and `CreateFrame` takes the same shape, so split it.
// A quoted literal, either quote style, taken WHOLE. This used to split the literal on
// commas, on the belief that the fourth argument is a list; it is not — 1.12 looks up
// the string it was handed, once.
let lit = f
.strip_prefix('"')
.and_then(|s| s.strip_suffix('"'))
.or_else(|| f.strip_prefix('\'').and_then(|s| s.strip_suffix('\'')));
if let Some(lit) = lit {
wanted.extend(
lit.split(',')
.map(|s| s.trim().to_string())
.filter(|s| !s.is_empty()),
);
if let Some(lit) = lit.filter(|s| !s.is_empty()) {
wanted.insert(lit.to_string());
}
}
}
@@ -1137,10 +1161,13 @@ fn missing_inherits(script: &UiScript, root: &Path, name: &str, toc: &Toc) -> Ve
let Some(end) = rest[1..].find(quote) else {
continue;
};
// ONE name, verbatim — the attribute value is what the loader looks up, commas and
// spaces included. Splitting here made the census ask about names the loader never
// asks for: a comma list would be reported as two missing templates when the real
// lookup misses once, on the whole string.
wanted.extend(
rest[1..1 + end]
.split(',')
.map(|s| s.trim().to_string())
[rest[1..1 + end].to_string()]
.into_iter()
.filter(|s| !s.is_empty()),
);
}
@@ -1186,6 +1213,52 @@ pub fn framexml_digest() -> String {
crate::ui_script::framexml_digest()
}
/// Every name our VM publishes into `_G`, with its Lua type — **no addons loaded**.
///
/// The other half of decision 1189's diff. 1189 established that the authoritative 1.12 surface is
/// already captured (the running client's in-world `_G`, 19,572 entries, vendored here as
/// `reference/1.12-globals.tsv`) and compared our table against it *once*, by hand. Nothing has
/// re-run it since, so the comparison has been drifting ever since — which is the failure mode this
/// project keeps writing records about: a measurement taken once and then cited as though it were
/// current.
///
/// Deliberately addon-free. The question is what OUR interface publishes, and an addon's own globals
/// would inflate both directions of the diff with names that belong to nobody but that addon.
///
/// The dump runs in Lua rather than reaching into the registry from Rust, because `_G` is the thing
/// an addon actually sees — the same reasoning that made the reference side a live capture instead
/// of the binary's registration table.
pub fn surface() -> Vec<(String, String)> {
let Ok(mut script) = UiScript::new() else {
return Vec::new();
};
script.set_instruction_budget(ADDON_INSTRUCTION_BUDGET);
script.set_screen_size(1024.0, 768.0);
script.register_addons(Vec::new(), None, None, None);
seat_a_session(&mut script);
let _ = crate::ui_script::load_default_ui(&script);
let dump: String = script
.eval(
r#"
local out = {}
for k, v in pairs(_G) do
if type(k) == "string" then
out[table.getn(out) + 1] = k .. "\t" .. type(v)
end
end
table.sort(out)
return table.concat(out, "\n")
"#,
)
.unwrap_or_default();
dump.lines()
.filter_map(|l| l.split_once('\t'))
.map(|(n, t)| (n.to_string(), t.to_string()))
.collect()
}
/// The real `GlobalStrings.lua`, read once off the install's patch chain.
///
/// **~5,000 globals the surveyed VM would otherwise not have**, and the difference between an
+20 -6
View File
@@ -164,12 +164,26 @@ impl ChatLog {
Some(i) => (text[..i].to_string(), text[i + 1..].to_string()),
None => (text.to_string(), String::new()),
};
let distribution = match chat_type {
0x01 => "PARTY",
0x03 => "RAID",
0x04 => "GUILD",
0x18 => "BATTLEGROUND",
_ => "UNKNOWN",
// **The protocol constants, not hand-copied bytes.** Three of the four literals here were
// WRONG: RAID is `0x2` and this said `0x03`, GUILD is `0x3` and this said `0x04`, and
// BATTLEGROUND is `0x5C` and this said `0x18`. The visible effect was not a dropped
// message but a MISLABELLED one — a real guild addon message arrived at
// `CHAT_MSG_ADDON` as `"RAID"`, while real raid and battleground traffic fell to
// `"UNKNOWN"` — so an addon branching on the distribution acted on the wrong lane.
//
// The outbound half (`net::addon_wire_chat_type`) always used the named constants and its
// own doc calls itself "the one place a distribution becomes a wire byte". It was not: this
// was the other one, and it disagreed. Both ends now read the same symbols, so they cannot
// drift apart again.
let distribution = {
use benilla_protocol::messages as m;
match u32::from(chat_type) {
m::CHAT_TYPE_PARTY => "PARTY",
m::CHAT_TYPE_RAID => "RAID",
m::CHAT_TYPE_GUILD => "GUILD",
m::CHAT_TYPE_BATTLEGROUND => "BATTLEGROUND",
_ => "UNKNOWN",
}
}
.to_string();
self.pending.push(Pending::Addon {
+13 -5
View File
@@ -1130,11 +1130,19 @@ fn a_sticky_whose_group_is_gone_opens_as_say() {
/// branch on either way.
#[test]
fn an_inbound_addon_line_splits_on_the_first_tab_only() {
const PARTY: u8 = 0x01;
const RAID: u8 = 0x03;
const GUILD: u8 = 0x04;
const BATTLEGROUND: u8 = 0x18;
const SAY: u8 = 0x00;
// **Imported, not hand-copied.** These read `0x03`/`0x04`/`0x18` for RAID/GUILD/BATTLEGROUND,
// which are all wrong — and because the test carried the SAME wrong bytes as the code under
// test, it agreed with the defect instead of catching it. A test that restates the value it is
// checking cannot fail on that value; taking it from the protocol crate is what makes it a
// check rather than an echo.
use benilla_protocol::messages as m;
let party = m::CHAT_TYPE_PARTY as u8;
let raid = m::CHAT_TYPE_RAID as u8;
let guild = m::CHAT_TYPE_GUILD as u8;
let battleground = m::CHAT_TYPE_BATTLEGROUND as u8;
let say = m::CHAT_TYPE_SAY as u8;
#[allow(non_snake_case)]
let (PARTY, RAID, GUILD, BATTLEGROUND, SAY) = (party, raid, guild, battleground, say);
let mut log = super::feed::ChatLog::default();
// The ordinary shape.
@@ -594,3 +594,52 @@ fn ctrl_and_shift_on_the_roll_icon_preview_and_post_its_link() {
);
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
/// The roll-frame template is published under **the reference's own name**, and an addon that
/// inherits it finds the parts it reaches for (decision 1254).
///
/// It shipped as `BenillaGroupLootFrameTemplate`. The `Benilla` prefix exists so a name WE invented
/// cannot collide with one an addon expects — and this template is the opposite of an invention: a
/// line-by-line transcription of 1.12's own `GroupLootFrameTemplate`, which addons inherit *by that
/// name*. `Bongos_RollBar/bar.lua:90` builds its four roll frames with
/// `CreateFrame("Frame", "BRollBarFrame"..i, bar, "GroupLootFrameTemplate")`; that resolves on the
/// real client and missed here for no reason but our prefix.
///
/// The child names are the actual contract, so they are what this asserts. `bar.lua` reaches for
/// `<name>IconFrameIcon`, `<name>Name`, `<name>Corner`, `<name>Decoration` and `<name>Timer` — and
/// `IconFrameIcon` is the one a flat template would get wrong, because it requires the icon texture
/// to be nested inside the `IconFrame` **button**, not hung off the template root.
#[test]
fn the_roll_template_carries_the_reference_name_and_the_parts_addons_reach_for() {
let s = setup();
load_xml(&s, "GroupLootFrame.xml");
// The reference's name resolves — and, since 1253, a miss would RAISE rather than hand back a
// bare frame, so this call failing is this test failing.
s.run(r#"Probe = CreateFrame("Frame", "ProbeRoll", nil, "GroupLootFrameTemplate")"#)
.expect("the reference-named template must resolve");
for part in [
"IconFrame",
"IconFrameIcon",
"Name",
"Corner",
"Decoration",
"Timer",
] {
assert!(
s.eval::<bool>(&format!(r#"return getglobal("ProbeRoll{part}") ~= nil"#))
.unwrap(),
"an addon inheriting the template must find ProbeRoll{part}"
);
}
// The timer is a StatusBar, not a plain frame: `bar.lua:33` calls SetMinMaxValues on it.
s.run(r#"ProbeRollTimer:SetMinMaxValues(0, 60000)"#)
.expect("the timer must answer StatusBar verbs");
// And the old private name is gone, so nothing can quietly depend on it again.
assert!(s
.eval::<bool>(r#"return getglobal("BenillaGroupLootFrameTemplate") == nil"#)
.unwrap());
}
+62 -12
View File
@@ -328,15 +328,40 @@ fn expand_inner(
return element.clone();
};
// ── ONE name, matched case-INSENSITIVELY (wow-re, template-name lookup) ────────────────────
//
// **No comma splitting.** 1.12's registry lookup `0x6ee6f0` has no splitter at all:
// `inherits="A, B"` is ONE literal name, misses, and applies neither template. Supporting the
// list is what LATER clients do, so splitting here made us a superset of 1.12 (1189). Nothing
// uses it — 0 comma lists in our own assets, 0 across the 218-addon corpus, and 0 in stock
// Blizzard XML's 249 distinct `inherits=` values.
//
// **Case-insensitive.** The compare is `SStrCmpI` → `_strnicmp` at `0x6ee747`, and the trap
// that decides it is the BUCKET HASH: `SStrHash 0x64b3f0` uppercases before mixing, so a
// mis-cased name lands in the same bucket and the stored-hash pre-check passes rather than
// short-circuiting. `Recap`'s `inherits="UIDropdownMenuTemplate"` (lowercase d) resolves on the
// real client. ASCII-only, like every other fold in this engine — bytes >= 0x80 are untouched.
//
// The fold is done as a scan on miss rather than by re-keying the view, because this same
// function resolves the FONT registry too and both are name→element maps of the same shape.
// Verbatim — not trimmed. `GetAttribute 0x6f2cf0` returns the parsed value as expat stored it
// (`SStrDup`, no trim, no fold), and the empty test is a single `cmp [esi],0` on the first
// byte: `inherits=""` is a silent skip, `inherits=" "` is a lookup that misses.
let mut base: Option<Element> = None;
for name in inherits.split(',').map(str::trim).filter(|s| !s.is_empty()) {
for name in [inherits].into_iter().filter(|s| !s.is_empty()) {
if !active.insert(name.to_string()) {
warnings.push(format!(
"inheritance cycle detected at template '{name}'; skipping this reference"
));
continue;
}
let Some(template) = templates.get(name) else {
let hit = templates.get(name).copied().or_else(|| {
templates
.iter()
.find(|(k, _)| k.eq_ignore_ascii_case(name))
.map(|(_, v)| *v)
});
let Some(template) = hit else {
warnings.push(format!(
"unknown template '{name}' referenced by inherits; skipping"
));
@@ -566,8 +591,19 @@ print(x)</Script>
assert!(doc.warnings.is_empty());
}
/// `inherits="A, B"` is **one name**, not a list — and it misses.
///
/// This test used to assert the opposite (that both templates merged, children concatenated in
/// order). That was a superset of 1.12 borrowed from later clients: the carve found no splitter
/// anywhere in the loader — all 8 call sites hand `0x6ee6f0` the pointer `GetAttribute`
/// returned, one or two instructions later, and no comma is examined in `0x6ed000–0x6f6000` at
/// all. So the lookup runs **once**, for the literal name `"A, B"`, misses, warns at
/// severity 1, and the element loads with **neither** template applied.
///
/// Whitespace is never trimmed either, which is why the second half asserts `" A "` misses a
/// template that genuinely exists.
#[test]
fn multi_inherits_merges_attrs_override_and_children_concat_in_order() {
fn a_comma_list_is_one_literal_name_that_misses_and_applies_neither_template() {
let doc = parse(
r#"<Ui>
<Frame name="A" virtual="true">
@@ -579,6 +615,7 @@ print(x)</Script>
<Frame name="Inst" inherits="A, B" alpha="1.0">
<Layers><Layer level="ARTWORK"><Texture name="FromInst"/></Layer></Layers>
</Frame>
<Frame name="Padded" inherits=" A "/>
</Ui>"#,
)
.unwrap();
@@ -588,20 +625,33 @@ print(x)</Script>
};
let mut warnings = Vec::new();
let expanded = expand(inst, &templates, &mut warnings);
assert!(warnings.is_empty());
// Instance's own `alpha` overrides B's (which would otherwise have overridden A's absence).
// Warned, naming the whole unsplit string — not split, not silently ignored.
assert_eq!(warnings.len(), 1, "{warnings:?}");
assert!(
warnings[0].contains("A, B"),
"the warning must name the literal it looked up: {warnings:?}"
);
// The element still loads, with all of its OWN content and none of the templates'.
assert_eq!(expanded.attr("alpha"), Some("1.0"));
// Children: A's <Layers> first, then B's <Layers>, then the instance's own <Layers> —
// inherited-first, instance-own-last (rf24-framexml-loader.md splice order).
assert_eq!(expanded.children.len(), 3);
assert_eq!(expanded.children.len(), 1);
fn texture_name(el: &Element) -> &str {
el.children[0].children[0].attr("name").unwrap()
}
assert_eq!(texture_name(&expanded.children[0]), "FromA");
assert_eq!(texture_name(&expanded.children[1]), "FromB");
assert_eq!(texture_name(&expanded.children[2]), "FromInst");
assert_eq!(texture_name(&expanded.children[0]), "FromInst");
// And a padded name misses a template that exists — the value is used verbatim.
let TopLevel::Instance(padded) = &doc.items[3] else {
panic!("expected instance")
};
let mut w2 = Vec::new();
let expanded = expand(padded, &templates, &mut w2);
assert_eq!(w2.len(), 1, "{w2:?}");
assert!(
expanded.children.is_empty(),
"picked up A despite the spaces"
);
}
#[test]
+9 -2
View File
@@ -209,9 +209,16 @@ pub fn apply_template(lua: &mlua::Lua, wrapper: &Table, kind: &str, template: &s
{
let model = loader.model();
let templates = model.framexml_templates.borrow();
for name in template.split(',').map(str::trim).filter(|s| !s.is_empty()) {
// One name, verbatim, folded — the same law `framexml::expand` resolves under, kept in
// step so this diagnostic cannot disagree with the resolution it describes.
for name in [template].into_iter().filter(|s| !s.is_empty()) {
// A name that is missing entirely is `expand`'s warning to give, just below.
let Some(el) = templates.get(name) else {
let Some(el) = templates.get(name).or_else(|| {
templates
.iter()
.find(|(k, _)| k.eq_ignore_ascii_case(name))
.map(|(_, v)| v)
}) else {
continue;
};
any_resolved = true;
+5 -3
View File
@@ -270,16 +270,18 @@ impl Loader<'_> {
let model = self.model();
let fonts = model.framexml_fonts.borrow();
let templates = model.framexml_templates.borrow();
for entry in inherits.split(',').map(str::trim) {
// One name, not a comma list — 1.12's lookup has no splitter, and supporting one is a
// superset of it. Folded on the probe, like the resolution itself.
for entry in [inherits] {
let mut name = entry.to_string();
for _ in 0..MAX_HOPS {
if fonts.contains_key(&name) {
if fonts.contains_key(&name) || fonts.keys().any(|k| k.eq_ignore_ascii_case(&name))
{
return Some(name);
}
let Some(next) = templates
.get(&name)
.and_then(|t| t.attr("inherits"))
.and_then(|i| i.split(',').map(str::trim).next())
.map(str::to_string)
else {
break;
+10 -4
View File
@@ -989,11 +989,17 @@ impl UiScript {
///
/// A pure query on the VM's live registry, for the corpus harness: an addon naming a template
/// we have not transcribed gets a bare frame and **no load error**, so nothing else can see it.
/// **Folded**, because the resolution it reports on is folded. An exact `contains_key` made this
/// census disagree with the loader the moment `inherits=` became case-insensitive: it went on
/// listing `UIDropdownMenuTemplate`, `CT_RaCheckButtonTemplate` and `MSBTColorSwatchTemplate` as
/// missing while the loader was resolving all three. An instrument that reports a gap the code
/// does not have is worse than no instrument — it is a build queue pointing at finished work
/// (1242/1246, and 1251 §3's rule that a source-derived answer be checked against the runtime
/// artefact).
pub fn has_framexml_template(&self, name: &str) -> bool {
self.model_ref()
.framexml_templates
.borrow()
.contains_key(name)
let model = self.model_ref();
let templates = model.framexml_templates.borrow();
templates.contains_key(name) || templates.keys().any(|k| k.eq_ignore_ascii_case(name))
}
/// Is `name` a registered FONT object — the *other* thing an `inherits=` may legally name?
+39 -2
View File
@@ -350,8 +350,12 @@ fn kind_method_registries(lua: &Lua, this: &Table) -> &'static [&'static str] {
/// `OnLoad` must be able to see the frame's real name, and `$parent` inside the template must
/// resolve against **the caller's** name, not the template's.
///
/// An unusable template (unknown, non-virtual, the wrong shape) is a warning on the model and a
/// perfectly usable frame — never an error that takes the frame with it.
/// An **unresolvable** template name raises `CreateFrame(): Couldn't find inherited node "%s"` and
/// creates nothing — the reference's own behaviour, byte-verified (`0x7061dd` → `luaL_error`, which
/// never returns). A template that resolves but is *unusable in some other way* — declared without
/// `virtual="true"`, or of a shape that does not fit the kind asked for — is still a warning plus a
/// working frame, because the registry lookup itself succeeded and that is the only thing the miss
/// branch tests.
fn create_frame(
lua: &Lua,
(kind, name, parent, inherits): (String, Option<Value>, Option<Value>, Option<Value>),
@@ -392,6 +396,39 @@ fn create_frame(
}
}
// ── THE TEMPLATE LOOKUP HAPPENS HERE — before anything is constructed or published ──────────
//
// `0x7061dd` calls the registry lookup `0x6ee6f0` and, on NULL, falls into
// `luaL_error(L, "CreateFrame(): Couldn't find inherited node \"%s\"")` at `0x7061ed`. That
// never returns — `luaG_errormsg`/`luaD_throw` contain no `ret` at all and end in `longjmp`, so
// the `xor eax,eax; ret` after the call is dead code. The frame is **not created**, no global
// is published, and the calling statement is abandoned; the error is ordinary Lua propagation
// and `pcall`-catchable, which is how the widget dispatcher (`0x704f10`, every handler under
// `lua_pcall`) turns it into one dead handler rather than a dead client.
//
// We used to warn and hand back a usable bare frame, and the doc above said the reference
// "creates the frame anyway". That claim is refuted at the bytes. It is the same class as 1249:
// a failure the client reports and we swallowed — and swallowing is the worse half, because the
// addon then runs on a frame with none of the art, regions or scripts it asked for and fails
// later, somewhere unrelated.
//
// Ordering is the carved part, not a detail: the miss branch precedes the synthetic-node build
// (`0x706208`) and the `name` store (`0x70622d`), so a miss leaves no partial widget and no
// orphan global behind. Doing this after construction would leave both.
if let Some(t) = template.as_deref().filter(|t| !t.is_empty()) {
let known = {
let model = lua.app_data_ref::<Model>().expect("model app_data");
let templates = model.framexml_templates.borrow();
// One name, verbatim, folded — the law `framexml::expand` resolves under.
templates.contains_key(t) || templates.keys().any(|k| k.eq_ignore_ascii_case(t))
};
if !known {
return Err(mlua::Error::runtime(format!(
"CreateFrame(): Couldn't find inherited node \"{t}\""
)));
}
}
// Create in the arena, mint the id, seed a default layout input. All under one write borrow.
let id = {
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
@@ -134,11 +134,26 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> {
})?,
)?;
// Regions
// `CreateTexture(name, layer, inheritsFrom)` — the third argument accepted for the same reason
// and with the same resolver, though the demand is a rounding error beside FontString's: of
// 1329 corpus call sites exactly TWO pass a third argument, and one of those passes `nil`
// (CustomNameplates:437, a four-argument later-client form). Accepted rather than skipped
// because dropping it silently is the very class this fixes, and one real site is still a site.
m.set(
"CreateTexture",
lua.create_function(
|lua, (this, name, layer): (Table, Option<String>, Option<String>)| {
create_region(lua, &this, RegionKind::Texture, name, layer)
|lua,
(this, name, layer, inherits): (
Table,
Option<String>,
Option<String>,
Option<String>,
)| {
let wrapper = create_region(lua, &this, RegionKind::Texture, name, layer)?;
if let Some(from) = inherits.as_deref().filter(|s| !s.is_empty()) {
apply_region_inherits(lua, &wrapper, from)?;
}
Ok(wrapper)
},
)?,
)?;
@@ -222,11 +237,36 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> {
})?,
)?;
// The THIRD argument is `inheritsFrom`, and we were dropping it on the floor.
//
// 49 corpus call sites across 5 distinct addons pass one — AckisRecipeList (28),
// CustomNameplates (10), _LazyPig (6), LibAboutPanel (4), ColorPickerPlus (1) — and **every one
// of them names a FONT OBJECT**: GameFontNormalSmall (15), GameFontNormal (11),
// GameFontHighlightSmall (10), GameFontHighlight (9), GameFontNormalLarge (3), GameFontDisable.
// Five separate addons, so this is not one library file replicated (1207 checked, not assumed).
// Ignoring it is 1203's class exactly: the addon asks for a font, we accept the call, and the
// string comes out in whatever the default is with no failure anywhere.
//
// The resolution order is carved (`0x773c30`): the FONT-object registry FIRST (`0x773d39`,
// create=0), and only on a font miss the template registry (`0x773d47`). This routes through
// the `SetFontObject` binding rather than reaching into the model, which is the same call the
// XML `<FontString inherits=>` path makes — one implementation of "apply a font object", not
// two that can drift.
m.set(
"CreateFontString",
lua.create_function(
|lua, (this, name, layer): (Table, Option<String>, Option<String>)| {
create_region(lua, &this, RegionKind::FontString, name, layer)
|lua,
(this, name, layer, inherits): (
Table,
Option<String>,
Option<String>,
Option<String>,
)| {
let wrapper = create_region(lua, &this, RegionKind::FontString, name, layer)?;
if let Some(from) = inherits.as_deref().filter(|s| !s.is_empty()) {
apply_region_inherits(lua, &wrapper, from)?;
}
Ok(wrapper)
},
)?,
)?;
@@ -348,6 +388,44 @@ fn get_script(lua: &Lua, this: &Table, name: &str) -> mlua::Result<Value> {
}
}
/// `CreateTexture`/`CreateFontString`'s third argument — `inheritsFrom` — in the carved order.
///
/// **Font-object registry first, template registry second** (`0x773d39` then `0x773d47`). That
/// order is not cosmetic: `inherits=` is one attribute over two namespaces, and every corpus caller
/// of this argument names a font object, so a template-first resolver would miss all 49 of them.
///
/// A name in NEITHER registry raises, which is the same contract 1253 established for `CreateFrame`
/// and the same bytes (`luaL_error`, `0x87957c` / `0x879544`, which never returns).
///
/// A name that IS a registered template but not a font object is a deliberate gap rather than a
/// silent one: the lookup succeeded, so the reference would not raise, and re-interpreting a
/// template node onto an existing region is a different mechanism from applying a font object. It
/// warns and leaves the region usable — the distinction 1253 drew between *the lookup missing* and
/// *the result being unusable*. Zero corpus callers reach it.
fn apply_region_inherits(lua: &Lua, wrapper: &Table, from: &str) -> mlua::Result<()> {
use mlua::ObjectLike;
// The font object is applied through the binding the XML path uses, so there is one
// implementation of "apply a font object" rather than two that can drift apart.
if wrapper.call_method::<()>("SetFontObject", from).is_ok() {
return Ok(());
}
let is_template = {
let model = lua.app_data_ref::<Model>().expect("model");
let templates = model.framexml_templates.borrow();
templates.contains_key(from) || templates.keys().any(|k| k.eq_ignore_ascii_case(from))
};
if is_template {
let mut model = lua.app_data_mut::<Model>().expect("model");
model.warnings.push(format!(
"CreateTexture/CreateFontString: '{from}' is a registered TEMPLATE, not a font object; the region is created but the template's content is not applied (no corpus caller does this)"
));
return Ok(());
}
Err(mlua::Error::runtime(format!(
"Couldn't find inherited node \"{from}\""
)))
}
fn create_region(
lua: &Lua,
this: &Table,
@@ -146,11 +146,17 @@ fn a_nested_frames_child_is_named_against_the_caller() {
assert!(s.take_warnings().is_empty());
}
/// `inherits="A, B"` is a LIST, and the runtime path resolves it through the very same
/// [`crate::framexml::expand`] the XML path uses — including chains, splice order and the cycle
/// guard, none of which is written twice.
/// The template argument is **one name**, on this path exactly as on the XML one — and a chain
/// through a single name still resolves.
///
/// This asserted the opposite until the registry lookup was carved. `"TemplA, TemplB"` is not a
/// list, it is a literal name nothing declared: 1.12's `CreateFrame` reaches the same `0x6ee6f0`
/// the XML loader does (call site `0x7061dd`) with the string Lua handed it, and the loader has no
/// splitter anywhere — comma lists are a **later**-client feature. Corroborated by the corpus:
/// 6842 `inherits=` across 282 vanilla XML files contain **zero** comma lists, while the modern
/// Blizzard UI source is full of them.
#[test]
fn an_inherits_list_picks_up_every_template_in_it() {
fn a_comma_list_is_one_name_and_a_single_name_still_chains() {
let mut s = script();
register(
&s,
@@ -168,60 +174,116 @@ fn an_inherits_list_picks_up_every_template_in_it() {
</Frame>
</Ui>"#,
);
s.run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#)
.unwrap();
// From A (itself inheriting TemplRoot — the chain resolves).
// A single name still walks its whole chain. This is what keeps the narrowing honest: it
// removes the list, not the inheritance.
s.run(r#"One = CreateFrame("Frame", "One", nil, "TemplA")"#)
.unwrap();
assert_eq!(
s.eval::<(f32, f32)>("return Both:GetWidth(), Both:GetHeight()")
s.eval::<(f32, f32)>("return One:GetWidth(), One:GetHeight()")
.unwrap(),
(50.0, 60.0)
);
assert_eq!(s.eval::<f32>("return One:GetAlpha()").unwrap(), 0.25);
assert!(s.take_errors().is_empty());
assert!(s.take_warnings().is_empty());
// The comma form is one literal name, nothing declared it, so it RAISES — naming the whole
// unsplit string, which is the proof it was never split.
let err = s
.run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#)
.expect_err("a comma list is one name, and it misses");
let text = err.to_string();
assert!(
text.contains("TemplA, TemplB"),
"the miss must name the literal it looked up, unsplit: {text}"
);
assert!(
s.eval::<bool>(r#"return getglobal("Both") == nil"#)
.unwrap(),
"a missed template must leave no frame behind"
);
}
/// The case fold, with the four names the corpus actually mis-cases.
///
/// The compare is `SStrCmpI` → `_strnicmp`, and the trap that decides it is the bucket hash:
/// `SStrHash 0x64b3f0` uppercases before mixing, so a mis-cased name lands in the same bucket and
/// the stored-hash pre-check passes rather than short-circuiting. ASCII-only, like every other fold
/// in this engine.
#[test]
fn a_template_name_is_matched_case_insensitively() {
let mut s = script();
register(
&s,
r#"<Ui>
<Frame name="CT_RACheckButtonTemplate" virtual="true" alpha="0.25">
<Size><AbsDimension x="50" y="60"/></Size>
</Frame>
</Ui>"#,
);
// Exactly the shape `CT_RaidAssist/CT_RAOptions.xml` ships: `RA` written `Ra`.
s.run(r#"Cased = CreateFrame("Frame", "Cased", nil, "CT_RaCheckButtonTemplate")"#)
.unwrap();
assert_eq!(
s.eval::<(f32, f32)>("return Cased:GetWidth(), Cased:GetHeight()")
.unwrap(),
(50.0, 60.0)
);
assert_eq!(s.eval::<f32>("return Both:GetAlpha()").unwrap(), 0.25);
// From B.
assert!(s
.eval::<bool>(r#"return getglobal("BothBTex") ~= nil"#)
.unwrap());
assert!(s.take_errors().is_empty());
assert!(s.take_warnings().is_empty());
}
/// An unusable template name never takes the frame with it — the reference creates the frame
/// anyway, and an addon's next line already assumes it did. Both flavours of "unusable" are here:
/// a name nothing declared, and a name declared **without `virtual="true"`** (only virtual
/// elements are ever registered as templates, so from the registry's side the two look alike).
/// An unresolvable template name **raises, and creates nothing**.
///
/// This test asserted the exact opposite — *"the reference creates the frame anyway, and an addon's
/// next line already assumes it did"* — until the bytes were read. `0x7061dd` looks the name up and
/// on NULL falls into `luaL_error(L, "CreateFrame(): Couldn't find inherited node \"%s\"")`, which
/// **never returns**: `luaG_errormsg` and `luaD_throw` contain no `ret` between them and end in
/// `longjmp`, so the `xor eax,eax; ret` that follows the call is dead code. (That trailing `ret` is
/// a fact about `luaL_error`'s `int` return type in C, not about reachability — which is exactly
/// how the old claim was arrived at.)
///
/// Both flavours of "unusable" raise, because the registry only ever holds `virtual="true"`
/// elements, so a name declared without it misses the lookup identically to a name nothing declared.
///
/// The ordering is asserted too: the miss precedes construction and name publication (`0x706208` /
/// `0x70622d`), so nothing partial is left behind — no frame, and no global.
#[test]
fn an_unusable_template_still_returns_a_usable_frame_and_says_so() {
let mut s = script();
fn an_unresolvable_template_raises_and_creates_nothing() {
let s = script();
register(&s, r#"<Ui><Frame name="PlainFrame"/></Ui>"#);
for (frame, template) in [("Orphan", "NoSuchTemplate"), ("Plainer", "PlainFrame")] {
s.run(&format!(
r#"{frame} = CreateFrame("Button", "{frame}", nil, "{template}")
{frame}:SetWidth(5)
{frame}:Show()"#
))
.unwrap_or_else(|e| panic!("{frame} must be usable: {e}"));
assert_eq!(
s.eval::<String>(&format!("return {frame}:GetName()"))
.unwrap(),
frame
);
assert_eq!(
s.eval::<f32>(&format!("return {frame}:GetWidth()"))
.unwrap(),
5.0
let err = s
.run(&format!(
r#"{frame} = CreateFrame("Button", "{frame}", nil, "{template}")"#
))
.expect_err("an unresolvable template must raise");
let text = err.to_string();
assert!(
text.contains("Couldn't find inherited node") && text.contains(template),
"the raise must carry the reference's message and the name: {text}"
);
let warnings = s.take_warnings();
// Nothing partial: no global published, and the name never reached the arena.
assert!(
warnings
.iter()
.any(|w| w.contains(template) && w.contains(frame)),
"a warning must name both the template and the frame it was asked for: {warnings:?}"
s.eval::<bool>(&format!(r#"return getglobal("{frame}") == nil"#))
.unwrap(),
"{frame} was published despite the miss"
);
assert!(s.take_errors().is_empty(), "a bad template is not an error");
}
// And it is ordinary Lua propagation — `pcall` catches it, which is what keeps one bad
// CreateFrame inside a handler from taking the client down (the widget dispatcher runs every
// handler under `lua_pcall`).
assert!(
!s.eval::<bool>(
r#"return (pcall(CreateFrame, "Button", "Caught", nil, "NoSuchTemplate"))"#
)
.unwrap(),
"the raise must be pcall-catchable"
);
}
/// The two shape mismatches, neither of which may panic or drop the frame.
@@ -979,3 +979,53 @@ fn the_font_block_reaches_both_message_frame_tables() {
"creating the style region must not re-justify the frame to the FontString default"
);
}
/// `CreateFontString`'s **third argument** applies a font object — the argument we accepted and
/// then dropped on the floor (decision 1255).
///
/// 49 corpus call sites across 5 distinct addons pass one, and every one names a font object:
/// AckisRecipeList (28), CustomNameplates (10), _LazyPig (6), LibAboutPanel (4), ColorPickerPlus.
/// Five separate addons, so this is not one library file replicated. Ignoring it was 1203's class —
/// the addon asks for a font, the call succeeds, and the text comes out in the default with no
/// failure anywhere to point at.
///
/// The **order** is the part a reimplementation loses: the font-object registry is tried FIRST and
/// the template registry only on a font miss (`0x773d39` then `0x773d47`). A template-first
/// resolver would miss all 49 of these, because `inherits=` is one argument over two namespaces.
#[test]
fn create_font_string_applies_the_font_object_named_by_its_third_argument() {
let s = script();
load(
&s,
r#"<Ui>
<Font name="GameFontNormalSmall" font="Fonts\FRIZQT__.TTF">
<FontHeight><AbsValue val="12"/></FontHeight>
<Color r="1" g="0.82" b="0"/>
</Font>
<Frame name="Host"/>
</Ui>"#,
);
// Exactly `_LazyPig/LazyPigMenu.lua:88`'s line.
s.run(r#"FS = Host:CreateFontString(nil, "ARTWORK", "GameFontNormalSmall")"#)
.expect("the font-object form must be accepted");
let (face, height, _flags) = s
.eval::<(String, f32, String)>("return FS:GetFont()")
.expect("the region must carry the font object's face and height");
assert_eq!(face, "Fonts\\FRIZQT__.TTF");
assert_eq!(height, 12.0);
// A name in NEITHER registry raises — the same contract 1253 set for CreateFrame, and the same
// bytes (`luaL_error`, which never returns).
let err = s
.run(r#"Bad = Host:CreateFontString(nil, "ARTWORK", "NoSuchFontOrTemplate")"#)
.expect_err("a name in neither registry must raise");
assert!(
err.to_string().contains("NoSuchFontOrTemplate"),
"the raise must name what was looked up: {err}"
);
// Texture takes the same argument through the same resolver (1 real corpus site).
s.run(r#"TX = Host:CreateTexture(nil, "OVERLAY")"#)
.expect("the two-argument form still works");
}
+4 -1
View File
@@ -598,7 +598,10 @@ fn the_unfired_script_kinds_still_raise_rather_than_silently_accepting() {
s.run(r#"Raiser = CreateFrame("EditBox", "RaiserBox")"#)
.unwrap();
for name in [
// No keyboard index / `EnableKeyboard` yet (wow-re `scripts-auto-enable.md` kinds 0/1).
// The FLAG exists (`EnableKeyboard`/`IsKeyboardEnabled`, 1252); what is still absent is the
// keyboard INDEX and the strata walk that would deliver a key to a frame carrying it, so
// these three stay unfired (wow-re `scripts-auto-enable.md` kinds 0/1). Being enabled is
// not being a handler — the carve is explicit that the two are separable.
"OnKeyDown",
"OnKeyUp",
"OnChar",
+9 -1
View File
@@ -416,7 +416,15 @@ pub(crate) fn token_recognised(token: &str) -> bool {
token.eq_ignore_ascii_case("npc")
|| UNIT_TOKEN_PREFIXES
.iter()
.any(|p| token.len() >= p.len() && token[..p.len()].eq_ignore_ascii_case(p))
// BYTES, not a string slice. `token[..p.len()]` panics when the token is multibyte
// UTF-8 and the prefix length lands mid-character ("byte index N is not a char
// boundary") — an addon passing a non-ASCII token would take the client down. The
// client's own compare is `_strnicmp` over bytes with an ASCII-only fold, so byte
// comparison is both the safe form and the faithful one.
.any(|p| {
token.len() >= p.len()
&& token.as_bytes()[..p.len()].eq_ignore_ascii_case(p.as_bytes())
})
}
/// The gate every `Unit*` binding puts an addon-supplied token through.
@@ -919,3 +919,42 @@ fn an_unrecognised_unit_token_raises_and_a_recognised_empty_one_does_not() {
assert!(s.run(r#"UnitIsUnit("bogus", "player")"#).is_err());
assert!(s.run(r#"UnitIsUnit("player", "party3")"#).is_ok());
}
/// **A multibyte unit token must not take the client down.**
///
/// The prefix test used to slice the token as a `&str` — `token[..p.len()]` — which PANICS when the
/// token is multibyte UTF-8 and the prefix length lands mid-character ("byte index N is not a char
/// boundary"). Any addon passing a non-ASCII token would have crashed the process rather than
/// getting the `Unknown unit name` raise the client gives.
///
/// The client compares with `_strnicmp` over BYTES and folds ASCII only, so byte comparison is both
/// the safe form and the faithful one — a token whose bytes differ above 0x7F is simply not one of
/// the nine prefixes.
#[test]
fn a_multibyte_unit_token_raises_rather_than_panicking() {
let mut s = UiScript::new().unwrap();
s.set_unit("player", Some(player()));
// Each of these has a multibyte character positioned so that a byte-index slice at one of the
// prefix lengths (3 for `pet`, 4 for `raid`, 5 for `party`, 6 for `player`/`target`) would land
// inside it.
for token in ["pé", "раid", "playér", "targét", "мышь", "日本語のトークン"] {
let err = s
.run(&format!(r#"UnitName("{token}")"#))
.expect_err("a non-ASCII token matches no prefix, so it raises")
.to_string();
assert!(
err.contains("Unknown unit name"),
"it must RAISE, not panic and not answer: {err}"
);
}
// …and an ASCII token that merely shares a prefix's leading bytes still behaves.
assert!(s
.eval::<Option<String>>(r#"return UnitName("playerfoo")"#)
.unwrap()
.is_none());
assert_eq!(
s.eval::<String>(r#"return UnitName("player")"#).unwrap(),
"Benilla"
);
}
+27 -18
View File
@@ -35,15 +35,21 @@ downgrade, and it is the entire reason this is a one-hunk fork rather than a ven
## The delta, and the command that proves it
**Current state: no source hunk yet.** The `lua-5.1.5/` tree here is byte-identical to upstream's,
and `src/lib.rs` differs only by having Lua 5.2/5.3/5.4/5.5 stripped from the `Version` enum (with
their source trees deleted) — benilla builds 5.1 and only 5.1, and a fork that still *offered* the
other four would answer a request for one with a missing directory at build time instead of a
compile error.
**Current state: two hunks, in two files.** `src/lib.rs` additionally differs by having Lua
5.2/5.3/5.4/5.5 stripped from the `Version` enum (with their source trees deleted) — benilla builds
5.1 and only 5.1, and a fork that still *offered* the other four would answer a request for one with
a missing directory at build time instead of a compile error.
This ordering is deliberate: the build pipeline is proven transparent **before** any behavioural
payload goes into it, so that when the suite moves, the patch is the only thing that could have
moved it. Landed inert, with all tests passing, exactly so that claim is checkable.
| file | hunk | why | record |
|---|---|---|---|
| `lvm.c` | 5.0's `OP_TFORPREP` table→`next` substitution, folded into the top of `OP_TFORLOOP` | `for k, v in someTable do` raised "attempt to call a table value"; it was the first session-start error for **60** corpus addons | 1215 |
| `luaconf.h` | `LUA_COMPAT_LSTR` `1` → `2` | 5.1 kept 5.0's nesting machinery and put an advisory error in front of it; two corpus addons died on "nesting of `[[...]]` is deprecated" | — |
The ordering was deliberate and it held: the fork landed **inert**, byte-identical to upstream, so
the build pipeline was proven transparent *before* any behavioural payload went into it — and when
the suite then moved, the patch was the only thing that could have moved it. Both hunks are measured
against the 218-addon corpus rather than argued: the long-string one alone took loaded 134 → 139 and
survivors 99 → 104.
To verify the Lua sources against upstream at any time:
@@ -53,17 +59,20 @@ diff -r third_party/lua-src/lua-5.1.5 \
~/.cargo/registry/src/*/lua-src-550.0.0/lua-5.1.5
```
Today that prints nothing. When the payload lands it must print **exactly** the hunk described in
decision 1215 and nothing else — if it ever prints more, something crept in.
That must print **exactly** the two hunks in the table above and nothing else. If it ever prints
more, something crept in.
## Status of the payload
Blocked, deliberately, on a reverse-engineering answer from the sibling repo `wow-5875-re`: what the
**real 1.12.1 client's own Lua** does when a generic-for's generator is a table. Stock Lua 5.0 is
corroboration, not the authority — the client links 5.0 but may be modified.
**Landed.** The reverse-engineering answer this was deliberately blocked on came back from
`wow-5875-re` (`system/ui/scratch/lua-generic-for.md`) and is what the `lvm.c` comment cites, detail
by detail: a **bare type-tag** test that never consults a metatable (so a table carrying `__call`
still gets `next`), the **global** `next` read raw and fetched fresh at every loop entry (so an addon
assigning `next = myfn` changes every later generic-for in the session), and userdata *not*
substituted.
Checking stock 5.0 already corrected two assumptions the project had written down (it substitutes in
`OP_TFORPREP`, an opcode 5.1 deleted, *not* in `OP_TFORLOOP`; and the test is a bare `ttistable`
with **no** `__call` check, using the **global** `next` — so an addon that overwrites `next` changes
loop behaviour). Two corrections from checking one step out is the reason for checking the next one
before writing C.
Waiting was the right call twice over. Checking stock 5.0 first corrected two assumptions the
project had already written down — the substitution happens in `OP_TFORPREP`, an opcode 5.1 deleted,
*not* in `OP_TFORLOOP`; and the test is a bare `ttistable` with no `__call` check. Stock 5.0 was
corroboration, never the authority: the client links 5.0 but may be modified, and only the byte-read
of the client settles what it actually does.