diff --git a/crates/benilla-ui/src/script/button.rs b/crates/benilla-ui/src/script/button.rs index 8a482fd3..c96f2c8d 100644 --- a/crates/benilla-ui/src/script/button.rs +++ b/crates/benilla-ui/src/script/button.rs @@ -1004,12 +1004,21 @@ pub(super) fn click_button(lua: &Lua, id: u32, button: &str, down: bool, scripte Ok(s) => Value::String(s), Err(_) => return, }; - if let Err(e) = event::fire_widget_handler(lua, id, "OnClick", vec![btn, Value::Boolean(down)]) - { - lua.app_data_mut::() - .expect("model app_data") - .record_script_error(e.to_string()); - } + // `PreClick`, the handler, `PostClick` (`script/mod.rs`'s `SCRIPT_KINDS` has why the pair is + // accepted at all). Each is fired on its own, so one that raises is recorded and the next + // still runs: the pair exists to bracket the click, and a broken bracket must not eat it. + let fire = |kind: &str| { + if let Err(e) = + event::fire_widget_handler(lua, id, kind, vec![btn.clone(), Value::Boolean(down)]) + { + lua.app_data_mut::() + .expect("model app_data") + .record_script_error(e.to_string()); + } + }; + fire("PreClick"); + fire("OnClick"); + fire("PostClick"); // ClassicAPI's secure-attribute click, **after** the `OnClick` handler for the same reason the // nameplate selection below is: a frame may have both, and a hook that raises must not eat the // action. A frame with no `type` attribute is untouched. diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 5274278a..dfe7f6fe 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -434,7 +434,7 @@ pub const SCREEN: crate::layout::Handle = 0; /// ScrollFrame's three scroll kinds are `+0x32c`/`+0x334`/`+0x33c` (script-name map `0x786c40`). /// The EditBox's vtable (`0x81c910`) replaces the key and char slots: an EditBox never fires /// `OnKeyDown`, and fires `OnChar` only from `Insert`, with the inserted text (`0x77c13c`). -const SCRIPT_KINDS: [&str; 43] = [ +const SCRIPT_KINDS: [&str; 45] = [ "OnLoad", // Accepted and never fired, deliberately: the realm's own `FloatingChatFrame.xml` sets it on // `ChatFrameEditBox`, so refusing it put a red `SetScript: unsupported script` in front of @@ -451,6 +451,15 @@ const SCRIPT_KINDS: [&str; 43] = [ "OnShow", "OnHide", "OnClick", + // The pair either side of `OnClick`, fired by `button::click_button`. **Not 1.12's** — they + // arrived with the secure template system, so a button could act before and after the + // protected part — but an addon written for a later client registers them at load, and + // refusing the name raises there and takes the rest of the file with it + // (`Cinematic/MenuWheel.lua`'s wheel slots). Nothing protected sits between them here, so the + // order is all they are: `PreClick`, the handler, `PostClick`, each with the same + // `(button, down)` the click carries. + "PreClick", + "PostClick", "OnEnter", "OnLeave", "OnMouseDown", diff --git a/crates/benilla-ui/src/script/object.rs b/crates/benilla-ui/src/script/object.rs index fc1ebf21..abc0c13a 100644 --- a/crates/benilla-ui/src/script/object.rs +++ b/crates/benilla-ui/src/script/object.rs @@ -618,17 +618,41 @@ pub(super) fn create_frame( // at `0x7061ed`, before the node build (`0x706208`) and the name store (`0x70622d`), so it // leaves no partial widget and no global. The raise never returns and `pcall` catches it, as // the widget dispatcher (`0x704f10`) does for every handler. + let mut templates_to_apply: Vec = Vec::new(); if let Some(t) = template.as_deref().filter(|t| !t.is_empty()) { - let known = { + let known = |name: &str| { let model = lua.app_data_ref::().expect("model app_data"); let templates = model.framexml_templates.borrow(); // One name, verbatim, case-folded, as `framexml::expand` resolves it. - templates.contains_key(t) || templates.keys().any(|k| k.eq_ignore_ascii_case(t)) + templates.contains_key(name) || templates.keys().any(|k| k.eq_ignore_ascii_case(name)) }; - if !known { - return Err(mlua::Error::runtime(format!( - "CreateFrame(): Couldn't find inherited node \"{t}\"" - ))); + if known(t) { + templates_to_apply.push(t.to_string()); + } else { + // **A comma-separated list, which 1.12 does not take.** Its `inherits` is one name + // (the lookup above, `0x7061dd`); Era's takes a list and applies them in order, and + // that is what an addon written for Era passes — `Cinematic/MenuWheel.lua:312` asks + // for `"SecureHandlerShowHideTemplate, SecureHandlerAttributeTemplate"`. + // + // **The evidence here is weaker than `compat/`'s other relaxations and that is worth + // saying**: `anchor_args.rs`'s one-argument `SetPoint` rests on the real + // `ClassicAPI.dll` writing that form itself, where this rests only on Era doing it and + // the addon targeting Era — nothing in the DLL's own Lua passes a list, and **no addon + // in the installed corpus does either**, which is also why it can break nothing. + // + // Tried only after the verbatim name misses, so a template whose own name contained a + // comma would still win, and every existing call takes exactly the path it did before. + let parts: Vec = t + .split(',') + .map(|p| p.trim().to_string()) + .filter(|p| !p.is_empty()) + .collect(); + if parts.len() < 2 || !parts.iter().all(|p| known(p)) { + return Err(mlua::Error::runtime(format!( + "CreateFrame(): Couldn't find inherited node \"{t}\"" + ))); + } + templates_to_apply = parts; } } @@ -663,8 +687,10 @@ pub(super) fn create_frame( // The wrapper exists, published under its name, before the template's `OnLoad` runs inside // this call, so the handler can reach the frame by name. let wrapper = frame_wrapper(lua, id)?; - if let Some(template) = template { - let messages = crate::loader::apply_template(lua, &wrapper, &kind, &template); + // In the order written: a later template's `OnLoad` and regions land over an earlier one's, + // which is the order `inherits=` resolves a list in. + for template in &templates_to_apply { + let messages = crate::loader::apply_template(lua, &wrapper, &kind, template); if !messages.is_empty() { let mut model = lua.app_data_mut::().expect("model app_data"); for m in messages { diff --git a/crates/benilla-ui/src/script/tests/create_frame_template.rs b/crates/benilla-ui/src/script/tests/create_frame_template.rs index 15b954e4..730cc214 100644 --- a/crates/benilla-ui/src/script/tests/create_frame_template.rs +++ b/crates/benilla-ui/src/script/tests/create_frame_template.rs @@ -129,10 +129,19 @@ fn a_nested_frames_child_is_named_against_the_caller() { assert!(s.take_warnings().is_empty()); } -/// The template argument is one name: 1.12's `CreateFrame` hands the string to the XML loader's -/// lookup (`0x6ee6f0`, called at `0x7061dd`), which has no splitter; comma lists came later. +/// The template argument is one name **to 1.12**: its `CreateFrame` hands the string to the XML +/// loader's lookup (`0x6ee6f0`, called at `0x7061dd`), which has no splitter; comma lists came +/// later, and Era takes one. +/// +/// `script/object.rs` now accepts a list, as `compat/` does elsewhere, and this test pins the +/// shape of that: the **verbatim name is tried first**, so every single-name call takes exactly +/// the path it did before; a list applies its templates in the order written; and a list naming +/// anything unknown still misses, still reporting the literal it looked up **unsplit** and still +/// leaving no frame behind. That relaxation rests on weaker evidence than the rest of `compat/` +/// (Era does it and the addon targets Era, where the one-argument `SetPoint` rests on the real +/// `ClassicAPI.dll` writing that form itself), which is why the miss path is pinned this closely. #[test] -fn a_comma_list_is_one_name_and_a_single_name_still_chains() { +fn a_comma_list_applies_both_and_an_unknown_one_still_misses_unsplit() { let mut s = script(); register( &s, @@ -162,19 +171,46 @@ fn a_comma_list_is_one_name_and_a_single_name_still_chains() { assert!(s.take_errors().is_empty()); assert!(s.take_warnings().is_empty()); + // A list of known names applies both, in the order written: the size and alpha off TemplA's + // own root, and TemplB's region. + s.run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#) + .unwrap(); + assert_eq!( + s.eval::<(f32, f32)>("return Both:GetWidth(), Both:GetHeight()") + .unwrap(), + (50.0, 60.0), + "the first template applied" + ); + assert_eq!(s.eval::("return Both:GetAlpha()").unwrap(), 0.25); + assert!( + s.eval::(r#"return getglobal("BothBTex") ~= nil"#) + .unwrap(), + "the second template's own region applied too, with $parent resolved against the instance" + ); + assert!(s.take_errors().is_empty()); + assert!(s.take_warnings().is_empty()); + + // One unknown name in the list and the whole call misses, reporting the literal unsplit — + // so a typo reads as the typo it is rather than as a half-built frame. let err = s - .run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#) - .expect_err("a comma list is one name, and it misses"); + .run(r#"Half = CreateFrame("Frame", "Half", nil, "TemplA, NoSuchTemplate")"#) + .expect_err("an unknown name in the list misses"); let text = err.to_string(); assert!( - text.contains("TemplA, TemplB"), + text.contains("TemplA, NoSuchTemplate"), "the miss must name the literal it looked up, unsplit: {text}" ); assert!( - s.eval::(r#"return getglobal("Both") == nil"#) + s.eval::(r#"return getglobal("Half") == nil"#) .unwrap(), - "a missed template must leave no frame behind" + "a missed list must leave no frame behind" ); + + // And a name that is simply unknown, with no comma in it at all, is unchanged. + let err = s + .run(r#"Nope = CreateFrame("Frame", "Nope", nil, "NoSuchTemplate")"#) + .expect_err("an unknown single name misses"); + assert!(err.to_string().contains("NoSuchTemplate")); } /// The lookup folds ASCII case: `SStrHash` (`0x64b3f0`) uppercases before hashing, so a mis-cased