diff --git a/crates/benilla-app/src/addon_harness/mod.rs b/crates/benilla-app/src/addon_harness/mod.rs index 4f081adb..193f11f7 100644 --- a/crates/benilla-app/src/addon_harness/mod.rs +++ b/crates/benilla-app/src/addon_harness/mod.rs @@ -394,6 +394,10 @@ fn manifest_path(root: &Path, name: &str) -> Option { .map(|e| e.path()) } +/// The per-addon VM-instruction bound (see its use in [`survey_one`]). Measured: the heaviest +/// legitimate corpus addon runs 4M, and 214 of 218 run under 1M. +const ADDON_INSTRUCTION_BUDGET: u64 = 200_000_000; + fn survey_one( root: &Path, name: &str, @@ -434,6 +438,20 @@ fn survey_one( } } }; + // **A time bound, so one non-terminating addon cannot take the whole survey with it.** + // + // 1247 met the failure this exists for: `date("*t")` returned a string where Lua returns a + // table, so an addon's `while` never ended and the 218-addon run simply stopped finishing — + // no roster to diff, no column to compare, no error row to read, and the cause found by + // bisecting the corpus BY HAND. A wrong number is recoverable; an instrument that does not + // return is not. + // + // The budget is MEASURED, not guessed. Across the corpus at the time of writing, 214 of 218 + // addons execute fewer than 1M VM instructions in a whole survey (the counter's resolution); + // the heaviest legitimate one is Enchantrix at 4M, then FuBar_CTRaid and BigWigs at 1M. So + // this is ~50x the heaviest real addon — far enough out that a fixture growing new seats + // cannot drift into it, and near enough that a runaway reports in about a second. + script.set_instruction_budget(ADDON_INSTRUCTION_BUDGET); script.set_screen_size(1024.0, 768.0); // Before anything runs: the AddOn API must answer for the whole installed set, not for nothing. // `None` roots because a survey must never read or write the director's real saved variables @@ -510,6 +528,8 @@ 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); @@ -1224,6 +1244,14 @@ fn seat_a_session(script: &mut UiScript) { // empty addon registry: a state the real client cannot be in. script.register_cvars(crate::cvars::REGISTERED.iter().copied()); script.set_realm_name("Harness"); + // THE BIND POINT. `GetBindLocation()` answered `""` in every VM, and a logged-in character with + // no hearth location is not a state one is in — the server sends `SMSG_BINDPOINTUPDATE` at + // login, before any addon runs. Same argument as the purse and the nil faction group. + // + // Three corpus addons read it, in three separate files (FuBar_TransporterFu, Necrosis, + // _LazyPig), and one of them CONCATENATES the result — so an empty seat is the difference + // between exercising their path and not. + script.set_bind_location("Stormwind City"); script.set_unit( "player", Some(benilla_ui::script::UnitState { @@ -3748,6 +3776,13 @@ mod dependency_tests { "one objective outstanding, one done" ); + // The bind point — a plain string, and the reason it is seated is that an addon + // CONCATENATES it (`Necrosis.lua:1089`), where an empty answer reads as no hearth at all. + assert_eq!( + s.eval::("return GetBindLocation()").unwrap(), + "Stormwind City" + ); + // The purse — and asserted as its three coin fields, not as one number, because that is the // whole reason the value is 1234g 56s 78c rather than something round. A seat that reads // back as 12345678 but breaks down wrong is the bug this shape exists to catch. diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index cd832c4a..479fad03 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -88,6 +88,7 @@ impl Plugin for NetPlugin { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -469,6 +470,15 @@ pub(crate) struct Reputations(pub(crate) Vec<(u8, i32)>); #[derive(Resource, Default)] pub(crate) struct HomeBind(pub(crate) Option); +/// The `/played` answer awaiting delivery to the VM (`SMSG_PLAYED_TIME`): total seconds played and +/// seconds since the last level-up. +/// +/// A one-slot mailbox rather than a queue: the reply answers a request, the server sends one per +/// request, and a second landing before the first is drained would mean the first is already stale. +/// `None` whenever there is nothing undelivered. +#[derive(Resource, Default)] +pub(crate) struct PlayedTimeAnswer(pub(crate) Option<(u32, u32)>); + /// The player's equip proficiencies (`SMSG_SET_PROFICIENCY`, at login + on train): item class /// (2 weapons / 4 armor) → allowed-subclass bitmask — the client's `0xc4d4a0[class]` store. /// The item tooltip's slot-line red compares against it. Empty until the packets land. diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 4396982d..6f00dec8 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -190,7 +190,15 @@ pub(super) fn apply_net_updates( ResMut, // The ask-once book-page cache (decision 1105) — every readable's text, keyed by // `PageText` id; the reader session repaints off it. - ResMut, + // Paired into one slot: the group is itself at Bevy's 16-param ceiling, so a 17th + + // entry stops the whole system implementing `SystemParam` — nesting is the same + + // escape this tuple already is. + ( + ResMut, + ResMut, + ), ), ), // One tuple param (the 16-SystemParam ceiling again): the action-bar- + merchant-facing errors @@ -337,7 +345,7 @@ pub(super) fn apply_net_updates( mut mirror_timers, mut pet_bar, mut ui_error_keys, - mut page_texts, + (mut page_texts, mut played_time_answer), ), ) = caches; let ( @@ -742,6 +750,10 @@ pub(super) fn apply_net_updates( chat::area_trigger_message(text, &mut chat_log) } SessionEvent::PlayedTime { total, level } => { + // BOTH halves, and they are not redundant. The chat breakdown is our stand-in for + // the reference's `ChatFrame_DisplayTimePlayed`, which we do not ship; the mailbox + // is what becomes `TIME_PLAYED_MSG(total, level)` for an addon that asked. + played_time_answer.0 = Some((total, level)); chat::played_time(total, level, &mut chat_log) } SessionEvent::RandomRoll { diff --git a/crates/benilla-app/src/ui_chat.rs b/crates/benilla-app/src/ui_chat.rs index fd016ebd..f75c74d5 100644 --- a/crates/benilla-app/src/ui_chat.rs +++ b/crates/benilla-app/src/ui_chat.rs @@ -58,6 +58,10 @@ impl Plugin for UiChatPlugin { // Push before the input pass so a line is on screen the same frame it decodes (mirrors // the loot/merchant feeds). .add_systems(Update, feed::feed_chat.before(UiInput)) + // RequestTimePlayed() -> CMSG_PLAYED_TIME, and SMSG_PLAYED_TIME -> TIME_PLAYED_MSG. + // Beside the chat feed because /played is a chat command and the answer prints there + // too; before the input pass for the same reason feed_chat is. + .add_systems(Update, feed::played_time_bridge.before(UiInput)) // The input: open on ENTER (after the UI input pass has set UiKeyboardCapture, so we // don't reopen the box that's already eating keys), then drain any submitted line. Both // touch the single NonSend VM, so they chain. In-world only (decision 0193): at the diff --git a/crates/benilla-app/src/ui_chat/feed.rs b/crates/benilla-app/src/ui_chat/feed.rs index 2a6441e9..de8906a6 100644 --- a/crates/benilla-app/src/ui_chat/feed.rs +++ b/crates/benilla-app/src/ui_chat/feed.rs @@ -720,3 +720,40 @@ fn needs_name(chat_type: u8) -> bool { | m::CHAT_MSG_BATTLEGROUND_LEADER ) } + +/// **`RequestTimePlayed()` out, `TIME_PLAYED_MSG` back** — the two halves of `/played` for an addon. +/// +/// The request is a count-drain ([`benilla_ui::script::UiScript::take_played_time_asks`], the pvp +/// queue's shape): the packet is empty, so two asks are two sends. The answer is a one-slot mailbox +/// the net apply pass fills, and it is delivered as the event rather than a return value, because +/// that is how the API answers — `RequestTimePlayed()` itself returns nothing. +/// +/// **This does NOT replace the chat breakdown beside it.** `net::apply::chat::played_time` prints +/// the TIME_PLAYED_TOTAL/LEVEL lines because we do not ship `ChatFrame_DisplayTimePlayed`, which is +/// what the reference's own `TIME_PLAYED_MSG` handler does. The two are the reference's two +/// consumers of one packet, not a doubling: an addon that registers the event does its own thing +/// with the numbers, and the player still sees `/played` answer in chat. +pub(crate) fn played_time_bridge( + script: Option>, + commands: Res, + answer: Option>, +) { + let Some(mut script) = script else { + return; + }; + for _ in 0..script.take_played_time_asks() { + let _ = commands.0.send(crate::net::ClientCommand::PlayedTime); + } + let Some(mut answer) = answer else { + return; + }; + if let Some((total, level)) = answer.0.take() { + script.fire_event( + "TIME_PLAYED_MSG", + vec![ + benilla_ui::script::ScriptValue::Int(i64::from(total)), + benilla_ui::script::ScriptValue::Int(i64::from(level)), + ], + ); + } +} diff --git a/crates/benilla-app/src/ui_items/feed.rs b/crates/benilla-app/src/ui_items/feed.rs index 7cc8967a..1898ac57 100644 --- a/crates/benilla-app/src/ui_items/feed.rs +++ b/crates/benilla-app/src/ui_items/feed.rs @@ -310,6 +310,21 @@ pub(super) fn feed_item_stats( let Some(mut script) = script else { return; }; + // **`GetBindLocation()`'s push, and it happens BEFORE the early return below.** + // + // The name resolution lives here because this system already owns it for the hearthstone's + // `$z` token, and a second AreaTable lookup elsewhere is the two-parallel-paths drift that has + // cost this codebase real bugs — the binding and the token must never disagree about where the + // player is bound. But the item feed idles whenever nothing is pending, and the bind point + // arrives long after world entry (`SMSG_BINDPOINTUPDATE` at login and on every re-bind), so the + // push cannot sit behind that gate. + let home_area: Option = home_bind + .as_deref() + .and_then(|b| b.0) + .and_then(|id| area_names.as_deref()?.0.resolve(id as i32)) + .map(str::to_string); + script.set_bind_location(home_area.as_deref().unwrap_or_default()); + pending.extend(items.take_fresh()); pending.extend(script.take_item_stat_asks()); if pending.is_empty() { @@ -317,11 +332,6 @@ pub(super) fn feed_item_stats( } let spell_res = spells.as_deref(); let skill_catalog = skill_lines.as_deref().map(|s| &s.catalog); - let home_area: Option = home_bind - .as_deref() - .and_then(|b| b.0) - .and_then(|id| area_names.as_deref()?.0.resolve(id as i32)) - .map(str::to_string); // A bind-point change re-substitutes every held view: templates pushed before the login's // SMSG_BINDPOINTUPDATE landed carry a raw $z otherwise (the hearthstone's login race). if *last_home != home_area { diff --git a/crates/benilla-ui/src/script/client.rs b/crates/benilla-ui/src/script/client.rs index 919fe8be..5db11e34 100644 --- a/crates/benilla-ui/src/script/client.rs +++ b/crates/benilla-ui/src/script/client.rs @@ -87,6 +87,45 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; + // `RequestTimePlayed()` — ask the server for /played, answered by `TIME_PLAYED_MSG`. + // + // **The request and the answer are two halves and only both are worth having.** The binding + // queues a `CMSG_PLAYED_TIME` the app drains; the app fires `TIME_PLAYED_MSG(total, level)` + // when `SMSG_PLAYED_TIME` lands. Shipping only this half would be 1203 exactly: `QuestHistory` + // calls it and then waits on an event that never comes, which is quieter and worse than the + // `attempt to call global` it gets today. + // + // It returns nothing — the answer arrives as an event, never as a return value. + g.set( + "RequestTimePlayed", + lua.create_function(|lua, ()| { + lua.app_data_mut::() + .expect("model app_data") + .played_time_asks += 1; + Ok(()) + })?, + )?; + + // `GetBindLocation()` — the hearthstone's bind location NAME, as the reference's own hearth + // confirmation prints it (`StaticPopup.lua:1742` formats it straight into the dialog text). + // + // Three corpus addons read it, in three separate files: FuBar_TransporterFu + // (`TransporterFu.lua:456`), Necrosis (`Necrosis.lua:1089`) and _LazyPig (`LazyPig.lua:623`). + // + // **`""` until the app pushes one, never nil — the same choice `GetRealmName` above makes, for + // the same reason.** Necrosis CONCATENATES the result (`..GetBindLocation()`), so a nil is a + // raise rather than a blank; the reference holds a string buffer that is empty before + // `SMSG_BINDPOINTUPDATE` lands, and no logged-in character is ever without a bind point, so the + // empty window is ours alone. Conflating "not yet known" with "bound nowhere" is the price, and + // it is the price the neighbouring verb already pays deliberately. + g.set( + "GetBindLocation", + lua.create_function(|lua, ()| { + let model = lua.app_data_ref::().expect("model app_data"); + lua.create_string(&model.bind_location) + })?, + )?; + // `GetFramerate()` — frames per second, as a number. // // 71 addons in the corpus call it; every performance readout on the ecosystem's FuBar/Titan @@ -105,6 +144,25 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { } impl super::UiScript { + /// Drain the `RequestTimePlayed()` asks queued since the last call — each one is a + /// `CMSG_PLAYED_TIME`. [`super::UiScript::take_pvp_toggles`]'s shape, and a count for the same + /// reason: the request packet is empty, so nothing distinguishes two asks but their number. + pub fn take_played_time_asks(&mut self) -> u32 { + std::mem::take(&mut self.model_mut().played_time_asks) + } + + /// Push the hearthstone bind location's name — the host's half of `GetBindLocation`. + /// + /// The app resolves `SMSG_BINDPOINTUPDATE`'s AreaTable id through the same catalog the + /// hearthstone's `$z` token already goes through, so the two can never disagree about where the + /// player is bound. Idempotent; the empty string means the packet has not landed yet. + pub fn set_bind_location(&mut self, area_name: &str) { + let mut model = self.model_mut(); + if model.bind_location != area_name { + model.bind_location = area_name.to_string(); + } + } + /// Push the realm name — the host's half of `GetRealmName` (decision 1195). /// /// Set at world entry from the realm the session actually connected to. Idempotent, and the diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 0083aae5..66bb763a 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -302,8 +302,20 @@ const SCRIPT_KINDS: [&str; 32] = [ /// [`pointer`]. What stays here is construction and the small host-facing state pushes/queries. pub struct UiScript { lua: Lua, + /// VM instructions executed, counted only while a budget is installed + /// ([`UiScript::set_instruction_budget`]). `Arc` because the hook callback outlives this + /// borrow; `Relaxed` because nothing orders against it — it is a bound, not a clock. + instructions: std::sync::Arc, } +/// How often the instruction hook fires — the resolution of an instruction budget. +/// +/// mlua's own documentation warns that a low value "can incur a very high overhead", and nothing +/// wants a precise bound: the question a budget answers is "is this chunk ever going to stop?", +/// where being out by a million instructions costs nothing and firing every instruction would cost +/// the whole survey's runtime. +pub const INSTRUCTION_HOOK_STEP: u32 = 1_000_000; + /// The chunk name the CLIENT gives an addon's file: `Interface\AddOns\\`. /// /// Backslashes, and the `Interface\AddOns\` prefix, because addons PARSE this. `FuBarPlugin-2.0` @@ -393,7 +405,10 @@ impl UiScript { worldmap::install(&lua)?; net_stats::install(&lua)?; - Ok(UiScript { lua }) + Ok(UiScript { + lua, + instructions: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)), + }) } /// The embedded VM — for the Bevy plugin / TOC-XML loader to add the game-state API bindings @@ -403,6 +418,60 @@ impl UiScript { &self.lua } + /// **Bound how long a chunk may run, so an infinite loop reports instead of hanging.** + /// + /// A missing capability in this engine has always been a silently WRONG ANSWER — a setter that + /// ignores you, a getter that says nil (1203/1205/1211/1230). Decision 1247 met the other kind: + /// `date("*t")` returned a string where Lua returns a table, so `Accountant_WeekStart`'s + /// `while thisDay ~= weekstart` never terminated and the addon spun the VM forever. That is + /// invisible to every instrument we own, because an instrument that never returns produces no + /// roster to diff, no column to compare and no error row to read — the 218-addon survey simply + /// stopped finishing, and the cause was found by bisecting the corpus BY HAND. + /// + /// So a caller that runs untrusted chunks can bound them. Past `budget` VM instructions the + /// hook raises, and the raise propagates like any other Lua error: the addon reports as failed + /// with a distinctive message, and everything after it still runs. + /// + /// **This is opt-in and the app does not use it.** A real session must not kill a player's + /// addon for being slow — the budget exists for harnesses, which run 218 strangers' code in one + /// process and cannot afford one of them to be unbounded. + /// + /// The hook fires every [`INSTRUCTION_HOOK_STEP`] instructions rather than every instruction: + /// mlua's own docs warn that a low value "can incur a very high overhead", and the step is the + /// resolution of the bound, which nothing here needs to be precise. + pub fn set_instruction_budget(&self, budget: u64) { + let used = self.instructions.clone(); + used.store(0, std::sync::atomic::Ordering::Relaxed); + let _ = self.lua.set_hook( + mlua::HookTriggers { + every_nth_instruction: Some(INSTRUCTION_HOOK_STEP), + ..Default::default() + }, + move |_, _| { + let n = used.fetch_add( + u64::from(INSTRUCTION_HOOK_STEP), + std::sync::atomic::Ordering::Relaxed, + ) + u64::from(INSTRUCTION_HOOK_STEP); + if n > budget { + return Err(mlua::Error::runtime(format!( + "benilla: instruction budget exhausted after {n} VM instructions — \ + treating this as a non-terminating loop" + ))); + } + Ok(mlua::VmState::Continue) + }, + ); + } + + /// VM instructions executed since the last [`Self::set_instruction_budget`], to the resolution + /// of [`INSTRUCTION_HOOK_STEP`]. Zero when no budget was ever set — the hook is what counts. + /// + /// Reported rather than merely used, because the budget has to be CHOSEN from the corpus and a + /// number nobody can read is a number nobody can revisit. + pub fn instructions_used(&self) -> u64 { + self.instructions.load(std::sync::atomic::Ordering::Relaxed) + } + /// Set the screen-root rect from a pixel size (`[0,0]` origin, y-up). Top-level frames anchor to /// it; changing it invalidates the next `resolve`. The app calls this every frame with an /// almost-always-identical size — compared before writing so the per-frame idiom doesn't diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 7ed9dc9f..f92b4b80 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -927,7 +927,18 @@ pub(crate) struct Model { /// wire: `LANG_ADDON` in the language field, a four-value distribution set, and a payload the /// binding already composed as `prefix` TAB `message`. pub(crate) addon_sends: Vec, + /// `RequestTimePlayed()` asks queued since the app last drained them — each is one + /// `CMSG_PLAYED_TIME`. A COUNT, not a payload, for [`super::pvp`]'s reason: the packet is + /// empty, so two asks in a frame are two sends rather than one collapsed intent. + pub(crate) played_time_asks: u32, pub(crate) realm_name: String, + /// The hearthstone bind location's NAME, behind `GetBindLocation()` — the app resolves the + /// `SMSG_BINDPOINTUPDATE` area id through the same AreaTable catalog the hearthstone's `$z` + /// token already uses, and pushes the resolved string here. + /// + /// Empty only before that packet has landed — `""`, never nil, matching [`Self::realm_name`] + /// beside it: a consumer concatenates this (`Necrosis.lua:1089`), so nil would be a raise. + pub(crate) bind_location: String, /// Frames per second, behind `GetFramerate()`. Pushed per tick by the app, which owns the /// clock this crate does not have. pub(crate) framerate: f64, @@ -1170,7 +1181,9 @@ impl Model { cursor_pos: (0.0, 0.0), chat_sends: Vec::new(), addon_sends: Vec::new(), + played_time_asks: 0, realm_name: String::new(), + bind_location: String::new(), framerate: 0.0, modifiers: (false, false, false), money: 0, diff --git a/crates/benilla-ui/src/script/party.rs b/crates/benilla-ui/src/script/party.rs index c59b7e2b..3471372c 100644 --- a/crates/benilla-ui/src/script/party.rs +++ b/crates/benilla-ui/src/script/party.rs @@ -772,16 +772,27 @@ mod tests { assert!(s .eval::(r#"return UnitInRaid("mouseover") == nil"#) .unwrap()); - // It NEVER raises: a missing, wrong-typed or unknown token is all nil. - for call in [ - "UnitInRaid()", - "UnitInRaid(nil)", - "UnitInRaid(7)", - r#"UnitInRaid("nosuchtoken")"#, - ] { + // **The input partition, and it is NOT "never raises".** This test used to assert that a + // missing, wrong-typed or unknown token is all nil, on a wow-re finding that published "no + // error path". That claim was refuted by a later §5 cross-check and corrected at the source + // (`raid-roster-bindings.md` §1: *"The grammar is now enumerated and it settles the other + // way"*), which named `UnitInRaid` as one of exactly three verbs carrying the wrong claim. + // + // Quiet nil: `0x6f3690` returns NULL for a missing or uncoercible argument, `0x515970` maps + // NULL/empty to GUID `0:0`, and `0x4baee0` short-circuits `0:0` to false at entry. + for call in ["UnitInRaid()", "UnitInRaid(nil)", r#"UnitInRaid("party3")"#] { assert!( s.eval::(&format!("return {call} == nil")).unwrap(), - "{call} must answer nil, not raise" + "{call} must answer nil" + ); + } + // Raise: a token matching none of the nine prefixes reaches + // `luaL_error("Unknown unit name: %s")` and longjmps — and a NUMBER is coerced to a string + // first, so it raises too. `UnitInRaid("bogus")` never returns on the real client. + for call in ["UnitInRaid(7)", r#"UnitInRaid("nosuchtoken")"#] { + assert!( + s.run(call).is_err(), + "{call} must raise — the token resolves to no prefix at all" ); } } diff --git a/crates/benilla-ui/src/script/stdlib.rs b/crates/benilla-ui/src/script/stdlib.rs index 05f25d35..60a5ca6c 100644 --- a/crates/benilla-ui/src/script/stdlib.rs +++ b/crates/benilla-ui/src/script/stdlib.rs @@ -458,8 +458,7 @@ fn install_time(lua: &Lua) -> mlua::Result<()> { // Against a string return, `dt["wday"]` is nil, `nil ~= 3` forever, and the addon spins // the client — which is exactly what it did to the survey the moment a separate fix let // Accountant reach this function at all. - if false && body == "*t" { - // TOGGLE-A/B + if body == "*t" { let (year, month, day, hour, min, sec, wday, yday) = civil_from_epoch(secs); let t = lua.create_table()?; t.set("year", year)?; diff --git a/crates/benilla-ui/src/script/tests/mod.rs b/crates/benilla-ui/src/script/tests/mod.rs index 8513ff46..9dbbd87a 100644 --- a/crates/benilla-ui/src/script/tests/mod.rs +++ b/crates/benilla-ui/src/script/tests/mod.rs @@ -39,3 +39,128 @@ mod tooltip_unit; mod toplevel; mod visibility; mod worldmap; + +/// **A bounded chunk reports instead of hanging** — the guard decision 1247's hang called for. +/// +/// Without it, a `while true do end` inside an addon takes the whole process with it, and every +/// instrument downstream produces nothing at all: no roster, no columns, no error row. The bound +/// turns that into an ordinary Lua error, which every caller already knows how to report. +#[test] +fn an_instruction_budget_turns_a_runaway_loop_into_an_error() { + let s = crate::script::UiScript::new().unwrap(); + s.set_instruction_budget(5_000_000); + let err = s + .run("local i = 0 while true do i = i + 1 end") + .expect_err("an unbounded loop must raise rather than run forever") + .to_string(); + assert!( + err.contains("instruction budget exhausted"), + "the raise names itself so a report can tell it from an addon's own error: {err}" + ); + assert!( + s.instructions_used() >= 5_000_000, + "the counter reports what was spent: {}", + s.instructions_used() + ); + + // **The VM survives it.** The bound must not poison the interpreter — a survey has 217 more + // addons to run after the one that looped, and each gets its own VM only because this one + // returned at all. + assert_eq!( + s.eval::("return 2 + 3").unwrap(), + 5, + "the VM still evaluates after a budget raise" + ); + + // An ordinary chunk is untouched by a budget it never approaches. + let t = crate::script::UiScript::new().unwrap(); + t.set_instruction_budget(200_000_000); + t.run("BudgetOk = 0 for i = 1, 1000 do BudgetOk = BudgetOk + i end") + .unwrap(); + assert_eq!(t.eval::("return BudgetOk").unwrap(), 500_500); +} + +/// **`GetBindLocation()` — the hearth location's name, `""` before the packet lands.** +/// +/// Three corpus addons read it (FuBar_TransporterFu `TransporterFu.lua:456`, Necrosis +/// `Necrosis.lua:1089`, _LazyPig `LazyPig.lua:623`), and the reference's own hearth confirmation +/// formats it straight into the dialog (`StaticPopup.lua:1742`). +#[test] +fn get_bind_location_answers_the_pushed_name_and_never_nil() { + let mut s = crate::script::UiScript::new().unwrap(); + // Before the app pushes anything: the empty string, NOT nil. Necrosis concatenates the result, + // so nil would be a raise rather than a blank — the same choice `GetRealmName` beside it makes. + assert_eq!(s.eval::("return GetBindLocation()").unwrap(), ""); + assert_eq!( + s.eval::("return type(GetBindLocation())").unwrap(), + "string", + "never nil — a consumer concatenates it" + ); + + s.set_bind_location("Stormwind City"); + assert_eq!( + s.eval::("return GetBindLocation()").unwrap(), + "Stormwind City" + ); + // Concatenation is the idiom that matters, so it is the one asserted. + assert_eq!( + s.eval::(r#"return "Bound: " .. GetBindLocation()"#) + .unwrap(), + "Bound: Stormwind City" + ); + // A re-bind replaces it rather than accumulating. + s.set_bind_location("Ironforge"); + assert_eq!( + s.eval::("return GetBindLocation()").unwrap(), + "Ironforge" + ); +} + +/// **`RequestTimePlayed()` queues the ask; `TIME_PLAYED_MSG` carries the answer.** +/// +/// The two halves are one feature: `QuestHistory` calls the verb and then waits on the event, so +/// shipping the request alone would be 1203 — an addon waiting forever is quieter and worse than +/// the `attempt to call global` it got before. +#[test] +fn request_time_played_queues_an_ask_and_the_answer_arrives_as_an_event() { + let mut s = crate::script::UiScript::new().unwrap(); + + // The verb returns NOTHING — the answer is an event, never a return value. + assert_eq!( + s.eval::("return select('#', RequestTimePlayed())") + .unwrap(), + 0 + ); + // …and that call queued one ask. A COUNT, not a payload: the packet is empty, so two asks in a + // frame are two sends rather than one collapsed intent (the pvp queue's rule). + s.run("RequestTimePlayed() RequestTimePlayed()").unwrap(); + assert_eq!( + s.take_played_time_asks(), + 3, + "the eval above plus two more — each ask is its own CMSG_PLAYED_TIME" + ); + assert_eq!(s.take_played_time_asks(), 0, "the drain empties the queue"); + + // The answer half: the app fires the event with (total, level) in seconds. + s.run( + r#" + TPSeen = nil + local f = CreateFrame("Frame", "TPWatcher") + f:RegisterEvent("TIME_PLAYED_MSG") + f:SetScript("OnEvent", function() TPSeen = { arg1, arg2 } end) + "#, + ) + .unwrap(); + s.fire_event( + "TIME_PLAYED_MSG", + vec![ + crate::script::ScriptValue::Int(360_000), + crate::script::ScriptValue::Int(7_200), + ], + ); + assert_eq!( + s.eval::<(i64, i64)>("return TPSeen[1], TPSeen[2]").unwrap(), + (360_000, 7_200), + "total seconds played and seconds since the last level-up, in that order" + ); +} diff --git a/crates/benilla-ui/src/script/tests/tooltip_unit.rs b/crates/benilla-ui/src/script/tests/tooltip_unit.rs index 0fd5ac0a..e537432d 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_unit.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_unit.rs @@ -43,7 +43,13 @@ fn creature_line_law() { assert(TTTextLeft2:GetText() == "Alpha") assert(TTTextLeft3:GetText() == "Level 10 Beast (Elite)", "got " .. TTTextLeft3:GetText()) assert(TTTextLeft4:GetText() == "Skinnable") - assert(tt:SetUnit("nosuch") == nil, "a missing unit answers nil") + -- A RECOGNISED token naming nothing answers nil... + assert(tt:SetUnit("party4") == nil, "a recognised but absent unit answers nil") + -- ...while an UNRECOGNISED one raises, because SetUnit resolves through the client's one + -- token resolver like every Unit* verb (wow-re raid-roster-bindings.md §1: a token matching + -- none of the nine prefixes reaches `luaL_error("Unknown unit name: %s")` and longjmps). + -- This used to read `SetUnit("nosuch") == nil`, which was the refuted claim. + assert(pcall(tt.SetUnit, tt, "nosuch") == false, "an unrecognised token raises") "#, ) .unwrap(); diff --git a/crates/benilla-ui/src/script/tooltip_unit.rs b/crates/benilla-ui/src/script/tooltip_unit.rs index 896bb9f4..458fa9fa 100644 --- a/crates/benilla-ui/src/script/tooltip_unit.rs +++ b/crates/benilla-ui/src/script/tooltip_unit.rs @@ -577,6 +577,11 @@ pub(super) fn install_methods(lua: &Lua, m: &Table) -> mlua::Result<()> { m.set( "SetUnit", lua.create_function(|lua, (this, token): (Table, String)| { + // Same gate as every `Unit*` verb: `SetUnit` resolves its argument through the client's + // one token resolver, so an unrecognised name raises here too rather than drawing an + // empty tooltip. NOT inside `render_unit` — the app's own push calls that with a + // canonical token and must never be gated against itself. + crate::script::unit::check_unit_token(&Some(token.clone()))?; let ok = render_unit(lua, &this, &token)?; Ok(if ok { mlua::Value::Integer(1) diff --git a/crates/benilla-ui/src/script/unit/bindings.rs b/crates/benilla-ui/src/script/unit/bindings.rs index 1ace0c24..8ffee15b 100644 --- a/crates/benilla-ui/src/script/unit/bindings.rs +++ b/crates/benilla-ui/src/script/unit/bindings.rs @@ -6,7 +6,8 @@ use mlua::{Lua, Value}; use super::super::Model; use super::{ - classification_word, grey_band, level_reads_unknown, pick_unit_token, power_token, with_unit, + check_unit_token, classification_word, grey_band, level_reads_unknown, pick_unit_token, + power_token, with_unit, }; /// The two class ids `GetComboPoints 0x51a190` accepts — the literals `4` and `0xb` it compares @@ -27,14 +28,14 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitExists", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, false, |u| u.exists)) + with_unit(lua, &token, false, |u| u.exists) })?, )?; g.set( "UnitName", lua.create_function(|lua, token: Option| { - let name = with_unit(lua, &token, None, |u| u.name.clone()); + let name = with_unit(lua, &token, None, |u| u.name.clone())?; match name { Some(n) => Ok(Value::String(lua.create_string(&n)?)), None => Ok(Value::Nil), @@ -45,14 +46,14 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitHealth", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, 0i64, |u| i64::from(u.health))) + with_unit(lua, &token, 0i64, |u| i64::from(u.health)) })?, )?; g.set( "UnitHealthMax", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, 0i64, |u| i64::from(u.max_health))) + with_unit(lua, &token, 0i64, |u| i64::from(u.max_health)) })?, )?; @@ -66,6 +67,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitLevel", lua.create_function(|lua, token: Option| { + check_unit_token(&token)?; let model = lua.app_data_ref::().expect("model app_data"); let Some(u) = token.as_ref().and_then(|t| model.unit(t)) else { return Ok(0i64); @@ -89,7 +91,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsCorpse", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.corpse_object) { + Ok(if with_unit(lua, &token, false, |u| u.corpse_object)? { Value::Integer(1) } else { Value::Nil @@ -106,7 +108,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "UnitCanAttack", lua.create_function(|lua, (a, b): (Option, Option)| { let token = pick_unit_token(&a, &b); - Ok(if with_unit(lua, &token, false, |u| u.can_attack) { + Ok(if with_unit(lua, &token, false, |u| u.can_attack)? { Value::Integer(1) } else { Value::Nil @@ -129,7 +131,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsDead", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, false, |u| u.dead)) + with_unit(lua, &token, false, |u| u.dead) })?, )?; @@ -138,13 +140,13 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsGhost", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, false, |u| u.ghost)) + with_unit(lua, &token, false, |u| u.ghost) })?, )?; g.set( "UnitIsDeadOrGhost", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, false, |u| u.dead || u.ghost)) + with_unit(lua, &token, false, |u| u.dead || u.ghost) })?, )?; @@ -156,7 +158,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitReaction", lua.create_function(|lua, (token, _other): (Option, Option)| { - let r = with_unit(lua, &token, 0u8, |u| u.reaction); + let r = with_unit(lua, &token, 0u8, |u| u.reaction)?; Ok(if r == 0 { Value::Nil } else { @@ -177,7 +179,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "UnitIsEnemy", lua.create_function(|lua, (a, b): (Option, Option)| { let token = pick_unit_token(&a, &b); - let r = with_unit(lua, &token, 0u8, |u| u.reaction); + let r = with_unit(lua, &token, 0u8, |u| u.reaction)?; Ok(if (1..=2).contains(&r) { Value::Integer(1) } else { @@ -189,7 +191,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "UnitIsFriend", lua.create_function(|lua, (a, b): (Option, Option)| { let token = pick_unit_token(&a, &b); - let r = with_unit(lua, &token, 0u8, |u| u.reaction); + let r = with_unit(lua, &token, 0u8, |u| u.reaction)?; Ok(if r >= 5 { Value::Integer(1) } else { @@ -207,7 +209,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsPlayer", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_player) { + Ok(if with_unit(lua, &token, false, |u| u.is_player)? { Value::Integer(1) } else { Value::Nil @@ -220,6 +222,9 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsUnit", lua.create_function(|lua, (a, b): (Option, Option)| { + // BOTH arguments go through the resolver, so either being unrecognised raises. + check_unit_token(&a)?; + check_unit_token(&b)?; let model = lua.app_data_ref::().expect("model app_data"); let (Some(a), Some(b)) = (a, b) else { return Ok(Value::Nil); @@ -257,7 +262,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, "Usage: UnitAffectingCombat(\"unit\")", )?; - let hot = with_unit(lua, &Some(token), false, |u| u.exists && u.in_combat); + let hot = with_unit(lua, &Some(token), false, |u| u.exists && u.in_combat)?; Ok(if hot { Value::Integer(1) } else { Value::Nil }) })?, )?; @@ -283,6 +288,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitInRaid", lua.create_function(|lua, token: Option| { + check_unit_token(&token)?; let model = lua.app_data_ref::().expect("model app_data"); let hit = token .as_ref() @@ -304,6 +310,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitInParty", lua.create_function(|lua, token: Option| { + check_unit_token(&token)?; let model = lua.app_data_ref::().expect("model app_data"); let Some(t) = token else { return Ok(Value::Nil); @@ -338,7 +345,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { let token = pick_unit_token(&a, &b); let ok = with_unit(lua, &token, false, |u| { u.exists && u.is_player && u.reaction >= 5 - }); + })?; Ok(if ok { Value::Integer(1) } else { Value::Nil }) })?, )?; @@ -348,7 +355,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "GetRaidTargetIndex", lua.create_function(|lua, token: Option| { - let idx = with_unit(lua, &token, 0u8, |u| u.raid_target); + let idx = with_unit(lua, &token, 0u8, |u| u.raid_target)?; Ok(if idx > 0 { Value::Integer(i64::from(idx)) } else { @@ -363,7 +370,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsConnected", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_connected) { + Ok(if with_unit(lua, &token, false, |u| u.is_connected)? { Value::Integer(1) } else { Value::Nil @@ -373,7 +380,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsAFK", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_afk) { + Ok(if with_unit(lua, &token, false, |u| u.is_afk)? { Value::Integer(1) } else { Value::Nil @@ -383,7 +390,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsDND", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_dnd) { + Ok(if with_unit(lua, &token, false, |u| u.is_dnd)? { Value::Integer(1) } else { Value::Nil @@ -395,7 +402,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsPVP", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.pvp) { + Ok(if with_unit(lua, &token, false, |u| u.pvp)? { Value::Integer(1) } else { Value::Nil @@ -405,7 +412,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsPVPFreeForAll", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_pvp_ffa) { + Ok(if with_unit(lua, &token, false, |u| u.is_pvp_ffa)? { Value::Integer(1) } else { Value::Nil @@ -419,7 +426,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitFactionGroup", lua.create_function(|lua, token: Option| { - match with_unit(lua, &token, None, |u| u.faction_group.clone()) { + match with_unit(lua, &token, None, |u| u.faction_group.clone())? { Some(group) => { let s = Value::String(lua.create_string(&group)?); Ok((s.clone(), s)) @@ -438,7 +445,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { lua.create_function(|lua, token: Option| { let pair = with_unit(lua, &token, None, |u| { u.race.clone().zip(u.race_file.clone()) - }); + })?; match pair { Some((loc, file)) => Ok(( Value::String(lua.create_string(&loc)?), @@ -453,7 +460,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { lua.create_function(|lua, token: Option| { let pair = with_unit(lua, &token, None, |u| { u.class.clone().zip(u.class_file.clone()) - }); + })?; match pair { Some((loc, file)) => Ok(( Value::String(lua.create_string(&loc)?), @@ -468,7 +475,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitSex", lua.create_function(|lua, token: Option| { - let sex = with_unit(lua, &token, 0u8, |u| u.sex); + let sex = with_unit(lua, &token, 0u8, |u| u.sex)?; Ok(if sex == 0 { Value::Nil } else { @@ -528,7 +535,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { .clone() // Stage 3, collapsed: every player race maps to CreatureType 7. .or_else(|| u.is_player.then(|| "Humanoid".to_string())) - }); + })?; match word { Some(w) => Ok(Value::String(lua.create_string(&w)?)), None => Ok(Value::Nil), @@ -544,7 +551,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitClassification", lua.create_function(|lua, token: Option| { - let rank = with_unit(lua, &token, 0u32, |u| u.rank); + let rank = with_unit(lua, &token, 0u32, |u| u.rank)?; Ok(classification_word(rank).to_string()) })?, )?; @@ -554,7 +561,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitPowerType", lua.create_function(|lua, token: Option| { - let ty = with_unit(lua, &token, 0u8, |u| u.power_type); + let ty = with_unit(lua, &token, 0u8, |u| u.power_type)?; Ok((i64::from(ty), power_token(ty).to_string())) })?, )?; @@ -573,13 +580,13 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitMana", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, 0i64, |u| i64::from(u.power))) + with_unit(lua, &token, 0i64, |u| i64::from(u.power)) })?, )?; g.set( "UnitManaMax", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, 0i64, |u| i64::from(u.max_power))) + with_unit(lua, &token, 0i64, |u| i64::from(u.max_power)) })?, )?; @@ -627,13 +634,13 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsCharmed", lua.create_function(|lua, token: Option| { - Ok(with_unit(lua, &token, Value::Nil, |u| { + with_unit(lua, &token, Value::Nil, |u| { if u.charmed { Value::Number(1.0) } else { Value::Nil } - })) + }) })?, )?; diff --git a/crates/benilla-ui/src/script/unit/mod.rs b/crates/benilla-ui/src/script/unit/mod.rs index d5776264..9520fe7e 100644 --- a/crates/benilla-ui/src/script/unit/mod.rs +++ b/crates/benilla-ui/src/script/unit/mod.rs @@ -383,19 +383,80 @@ fn pick_unit_token(a: &Option, b: &Option) -> Option { } } +/// The token prefixes 1.12's resolver `0x515970` recognises, in its own order. +/// +/// The order is the binary's and is preserved even though our recogniser is order-insensitive: +/// `partypet` must be tested before `party` and `raidpet` before `raid` for a *resolver* to pick the +/// right unit, and the day this grows from a boolean into a resolver, the list is already right. +/// +/// **These are PREFIX tests.** The compare length is the literal's, and `_strnicmp` stops at either +/// string's NUL, so `"playerfoo"` matches `player` — which is why it is a quiet nil rather than a +/// raise. `npc` is the sole FULL-STRING compare in the resolver, so `"npctarget"` matches nothing +/// and raises; it is deliberately absent from this list and handled separately below. +const UNIT_TOKEN_PREFIXES: [&str; 8] = [ + "player", + "pet", + "target", + "partypet", + "party", + "raidpet", + "raid", + "mouseover", +]; + +/// Does 1.12's resolver RECOGNISE this token — whether or not it names a live unit? +/// +/// This is the whole raise/nil distinction, so it is worth being exact about what it is not: it does +/// not ask whether a unit exists. `"party5"` on a solo character, `"pet"` with no pet, and +/// `"playerfoo"` are all *recognised* and answer nil; only a token the resolver's nine compares +/// never match raises `Unknown unit name`. +pub(crate) fn token_recognised(token: &str) -> bool { + // `npc` full-string, everything else a prefix — and folded, like every compare in the resolver + // (`SStrCmpI` -> `_strnicmp`, ASCII only; 1247). + token.eq_ignore_ascii_case("npc") + || UNIT_TOKEN_PREFIXES + .iter() + .any(|p| token.len() >= p.len() && token[..p.len()].eq_ignore_ascii_case(p)) +} + +/// The gate every `Unit*` binding puts an addon-supplied token through. +/// +/// **An unrecognised token RAISES and does not return** — `0x515970` falls off the end of its nine +/// compares into `luaL_error(L, "Unknown unit name: %s")`, which longjmps (wow-re +/// `system/ui/scratch/unit-token-grammar.md`). Ours answered nil for everything unknown, which is +/// 1203's shape pointed the other way: a failure the client reports, silently swallowed. +/// +/// The split is THREE-way, not two, and the two quiet legs are as carved: +/// * **absent argument** — quiet nil (the per-binding argument gates are NOT uniform; `UnitName` +/// has its own `lua_isstring` gate and `UnitExists` has none at all, and only those two poles +/// are verified, so this does not invent a gate for the other ~102); +/// * **`""`** — quiet nil; +/// * a **recognised** token naming nothing (`"party5"` solo, `"playerfoo"`) — quiet nil. +pub(crate) fn check_unit_token(token: &Option) -> mlua::Result<()> { + match token { + Some(t) if !t.is_empty() && !token_recognised(t) => { + Err(mlua::Error::runtime(format!("Unknown unit name: {t}"))) + } + _ => Ok(()), + } +} + /// Read a unit token's snapshot under a short model borrow, mapping it through `f`; `default` when the /// token is absent (the "unit doesn't exist" path). +/// +/// Raises for an unrecognised token ([`check_unit_token`]) before it looks anything up. fn with_unit( lua: &Lua, token: &Option, default: T, f: impl FnOnce(&UnitState) -> T, -) -> T { +) -> mlua::Result { + check_unit_token(token)?; let model = lua.app_data_ref::().expect("model app_data"); - match token.as_ref().and_then(|t| model.unit(t)) { + Ok(match token.as_ref().and_then(|t| model.unit(t)) { Some(u) => f(u), None => default, - } + }) } /// The `Unit*`/`GetQuestGreenRange` Lua binding registrations — split from this module's diff --git a/crates/benilla-ui/src/script/unit/tests.rs b/crates/benilla-ui/src/script/unit/tests.rs index bc521efe..c15e9d45 100644 --- a/crates/benilla-ui/src/script/unit/tests.rs +++ b/crates/benilla-ui/src/script/unit/tests.rs @@ -551,9 +551,11 @@ fn faction_group_pair_and_the_pvp_toggle() { assert!(s .eval::(r#"local a, b = UnitFactionGroup("target") return a == nil and b == nil"#) .unwrap()); - // An absent unit answers the same way. + // An absent unit answers the same way. (This read `"focus"` until the unrecognised-token raise + // landed — a token 1.12 does not have at all, so it now raises rather than being "absent". The + // question the line means to ask needs a RECOGNISED token that names nothing.) assert!(s - .eval::(r#"local a = UnitFactionGroup("focus") return a == nil"#) + .eval::(r#"local a = UnitFactionGroup("party4") return a == nil"#) .unwrap()); assert_eq!(s.take_pvp_toggles(), 0, "nothing queued yet"); @@ -640,11 +642,16 @@ fn unit_affecting_combat_is_one_or_nil_and_hides_the_missing_unit() { .unwrap(), "an unresolvable token is the SAME nil a peaceful unit gives" ); - // A number is accepted and stringified (`0x6f3510` is number-OR-string): it resolves the - // token "5", finds nothing, and answers nil — it does not raise. - assert!(s - .eval::("return UnitAffectingCombat(5) == nil") - .unwrap()); + // **A number is accepted and stringified — and that is exactly why it RAISES.** The gate + // `0x6f3510` is number-OR-string, so `5` is coerced via `%.14g` to the token `"5"`; `"5"` then + // matches none of the resolver's nine prefixes and falls into + // `luaL_error("Unknown unit name: %s")`. This assertion read `== nil` and "it does not raise" + // until wow-re's own §5 cross-check refuted the no-error-path claim it rested on + // (`raid-roster-bindings.md` §1) — the acceptance was right, the conclusion was not. + assert!( + s.run("UnitAffectingCombat(5)").is_err(), + "a number is coerced to a token that matches nothing, so it raises" + ); } /// A missing or wrong-typed argument **raises** — `0x6f4940` is `luaL_error` and does not return, @@ -810,8 +817,10 @@ fn a_unit_token_resolves_whatever_its_case() { "the directional pick folds case too" ); - // An unseated token is still absent whatever its case — folding must not invent units. - for spelling in ["bogus", "BOGUS", "Party1"] { + // An unseated but RECOGNISED token is still absent whatever its case — folding must not invent + // units. (`bogus` moved out of this list when the unrecognised-token raise landed: it is not + // "absent", it is not a unit token at all, and the client says so.) + for spelling in ["Party1", "PARTY1", "raid9", "MouseOver"] { assert!( !s.eval::(&format!(r#"return UnitExists("{spelling}")"#)) .unwrap(), @@ -841,3 +850,72 @@ fn seating_a_token_folds_its_key_too() { "removal folds too — no shadowed entry survives" ); } + +/// **An unrecognised token RAISES; a recognised one that names nothing is a quiet nil.** +/// +/// `0x515970` falls off the end of its nine compares into `luaL_error(L, "Unknown unit name: %s")`, +/// which longjmps and never returns (wow-re `system/ui/scratch/unit-token-grammar.md`). Ours +/// answered nil for everything unknown — 1203's shape pointed the other way, a failure the client +/// reports and we swallowed. +/// +/// The split is THREE-way, and the two quiet legs are the ones a "raise on nil" implementation gets +/// wrong. +#[test] +fn an_unrecognised_unit_token_raises_and_a_recognised_empty_one_does_not() { + let mut s = UiScript::new().unwrap(); + s.set_unit("player", Some(player())); + + // RAISE: nothing the resolver's compares match. + for bogus in ["bogus", "focus", "boss1", "arena1"] { + let err = s + .run(&format!(r#"UnitName("{bogus}")"#)) + .expect_err(&format!("{bogus} must raise")) + .to_string(); + assert!( + err.contains(&format!("Unknown unit name: {bogus}")), + "the raise carries the client's own text and the offending token: {err}" + ); + } + // `npc` is the SOLE full-string compare, so a suffix on it matches nothing at all. + assert!(s.run(r#"UnitName("npctarget")"#).is_err()); + // …while `npc` itself is recognised, folded like everything else. + assert!(s.run(r#"UnitName("NPC")"#).is_ok()); + + // QUIET NIL, leg 1: a recognised token naming nothing here. + for absent in [ + "party5", + "raid17", + "pet", + "mouseover", + "partypet2", + "raidpet3", + ] { + assert!( + s.eval::>(&format!(r#"return UnitName("{absent}")"#)) + .unwrap() + .is_none(), + "{absent} is recognised and absent — nil, not a raise" + ); + } + // QUIET NIL, leg 2: a recognised PREFIX with a junk suffix. The compares are prefix tests that + // stop at either NUL, so `playerfoo` matches `player` and never reaches the raise. + for junk in ["playerfoo", "raidx", "petx", "targetish"] { + assert!( + s.eval::>(&format!(r#"return UnitName("{junk}")"#)) + .unwrap() + .is_none(), + "{junk} matches a PREFIX, so it is a quiet nil — not a raise. This is the leg an \ + implementation that raises on \"did not resolve\" gets wrong." + ); + } + // QUIET NIL, leg 3: the empty string and an absent argument. The per-binding argument gates are + // NOT uniform in the client and only two poles are verified, so this asserts the resolver's + // behaviour and does not invent a gate. + assert!(s.run(r#"UnitName("")"#).is_ok()); + assert!(s.run("UnitName()").is_ok()); + + // A two-unit call gates BOTH arguments. + assert!(s.run(r#"UnitIsUnit("player", "bogus")"#).is_err()); + assert!(s.run(r#"UnitIsUnit("bogus", "player")"#).is_err()); + assert!(s.run(r#"UnitIsUnit("player", "party3")"#).is_ok()); +}