diff --git a/crates/benilla-app/src/ui_chat/input/mod.rs b/crates/benilla-app/src/ui_chat/input/mod.rs index a17923d5..71888e21 100644 --- a/crates/benilla-app/src/ui_chat/input/mod.rs +++ b/crates/benilla-app/src/ui_chat/input/mod.rs @@ -14,7 +14,7 @@ use bevy::prelude::*; mod parse; #[cfg(test)] -pub(super) use parse::lua_long_string; +pub(super) use parse::lua_quoted_string; pub(super) use parse::{console_command, parse_line, ParsedChat}; use crate::creature_anim::{move_flags, MovementState}; diff --git a/crates/benilla-app/src/ui_chat/input/parse.rs b/crates/benilla-app/src/ui_chat/input/parse.rs index c8a2393d..b07557d6 100644 --- a/crates/benilla-app/src/ui_chat/input/parse.rs +++ b/crates/benilla-app/src/ui_chat/input/parse.rs @@ -234,15 +234,46 @@ pub(in crate::ui_chat) fn parse_line(table: &SlashCommands, line: &str) -> Parse } } -/// `s` as a Lua long-bracket string literal (`[[…]]`), at a bracket level the text cannot close -/// early — the one quoting that needs no escaping of what a player typed. -pub(in crate::ui_chat) fn lua_long_string(s: &str) -> String { - let mut level = 0; - while s.contains(&format!("]{}]", "=".repeat(level))) { - level += 1; +/// `s` as a Lua **short** string literal, escaped — the only quoting 1.12's lexer can read for +/// arbitrary text (decision 2136). +/// +/// This used to build a long-bracket literal and step its `=` level past anything the payload +/// could close early: `[=[a]]b]=]`. That works on a Lua 5.1-family lexer and **is a syntax error +/// on the 1.12 client**, whose lexer has no long-string levels at all — `read_long_string +/// 0x700010` routes `=` to its default arm as ordinary content, the `[` arm at `0x6ff771` makes +/// exactly one comparison (`cmp eax,0x5b`), and a `[=` therefore leaves the lexer holding the +/// single-character token `[`, which `prefixexp 0x6fde40` refuses with ``unexpected symbol near +/// `['`` (wow-5875-re `lua-dialect.md` §9.2, verified and executed). +/// +/// 2101 removed the three 5.1 grammar additions the corpus probes and left the levelled long +/// bracket standing, on a measurement of **zero occurrences** across FrameXML, GlueXML, +/// Blizzard's addons, the director's own folder and the 219-addon corpus. That measurement was +/// right and incomplete in one specific way: it scanned Lua *trees*, and this is Lua *generated +/// by Rust at runtime*, which no source scan could see and the fork's parser cannot reject at +/// compile time either. It is the one live producer of a construct the reference cannot parse, +/// and it fires whenever a `/console` argument contains `]]`. +/// +/// Escaping is what 5.0 leaves: `\` and `"` are self-escaped, the three whitespace controls take +/// their named forms, and every other control byte takes a **three-digit** `\ddd` — padded so a +/// following digit cannot be swallowed into the escape. +pub(in crate::ui_chat) fn lua_quoted_string(s: &str) -> String { + let mut out = String::with_capacity(s.len() + 2); + out.push('"'); + for c in s.chars() { + match c { + '\\' => out.push_str("\\\\"), + '"' => out.push_str("\\\""), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + c if (c as u32) < 0x20 || c as u32 == 0x7f => { + out.push_str(&format!("\\{:03}", c as u32)); + } + c => out.push(c), + } } - let eq = "=".repeat(level); - format!("[{eq}[{s}]{eq}]") + out.push('"'); + out } /// The per-command argument grammar. Each arm is the reference handler's own body reduced to what @@ -436,7 +467,7 @@ fn slash_command(index: SlashIndex, args: &str) -> ParsedChat { // this arm knew `reloadui` and answered everything else "not implemented" — which is // what a probe's `/console fpsJournal 1` got, while the same line typed worked.) S::Console => ParsedChat::Lua { - body: format!("ConsoleExec({})", lua_long_string(args)), + body: format!("ConsoleExec({})", lua_quoted_string(args)), }, // `/script` = the ref's `RunScript(msg)`: the typed text IS the chunk, un-escaped. S::Script => { diff --git a/crates/benilla-app/src/ui_chat/tests.rs b/crates/benilla-app/src/ui_chat/tests.rs index 0b5e6b46..666e368b 100644 --- a/crates/benilla-app/src/ui_chat/tests.rs +++ b/crates/benilla-app/src/ui_chat/tests.rs @@ -1652,20 +1652,43 @@ fn real_alias_table_resolves_the_shipped_commands() { assert_eq!( parse_line("/console fpsJournal 1"), ParsedChat::Lua { - body: "ConsoleExec([[fpsJournal 1]])".into() + body: "ConsoleExec(\"fpsJournal 1\")".into() } ); assert_eq!( parse_line("/console reloadUI"), ParsedChat::Lua { - body: "ConsoleExec([[reloadUI]])".into() + body: "ConsoleExec(\"reloadUI\")".into() } ); - assert_eq!(super::input::lua_long_string("a]]b"), "[=[a]]b]=]"); + // **The quoting is a SHORT string, because 1.12's lexer has no long-string levels** (2136). + // The old long-bracket form stepped its `=` level past whatever the payload could close, which + // is a construct the reference cannot compile at all — see `lua_quoted_string`. + assert_eq!(super::input::lua_quoted_string("a]]b"), "\"a]]b\""); + assert_eq!(super::input::lua_quoted_string("a]]b]=]c"), "\"a]]b]=]c\""); assert_eq!( - super::input::lua_long_string("a]]b]=]c"), - "[==[a]]b]=]c]==]" + super::input::lua_quoted_string("say \"hi\"\\n"), + "\"say \\\"hi\\\"\\\\n\"" ); + // …and the generated literal ROUND-TRIPS through a real VM, which is the assertion that + // actually pins the grammar: the old form compiled here and would not have on the reference, + // so a string check alone could not have caught it. + { + let vm = benilla_ui::script::UiScript::new().expect("VM"); + for payload in [ + "fpsJournal 1", + "a]]b", + "a]]b]=]c", + "quote \" and backslash \\", + "tab\there", + ] { + let lit = super::input::lua_quoted_string(payload); + let got: String = vm + .eval(&format!("return {lit}")) + .unwrap_or_else(|e| panic!("{lit} must compile on a 1.12-grammar VM: {e}")); + assert_eq!(got, payload, "and must carry the text unchanged"); + } + } // The whole shipped surface, so a table that half-loaded fails loudly: **225 distinct emote // commands** over the 169 `EmotesText` names (the strings repeat — `EMOTE87_CMD1` and `_CMD2` // are both "/sit" — and EMOTE27 "UNUSED" has no row, so it contributes none), and **68 distinct diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index b55e26e6..25a0152f 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -640,11 +640,21 @@ fn disabled_set(identity: Option<&(String, String)>) -> HashSet { .unwrap_or_default() } -/// Write a character's enable state from the AddOns screen (decision 1197). +/// Write a character's enable state — the AddOns screen's write (decision 1197) and, since 2139, +/// the in-world logout write too. /// -/// **Merges rather than replaces**, for the reason [`parse_enable_state`] documents: a name in the -/// file that is not installed right now belongs to an addon that will be again, and rewriting the -/// file from the installed list alone would forget the player's choice on every uninstall. +/// **Merges rather than replaces, because that is what the reference's writer structurally IS.** +/// `0x51ef20` walks the in-memory enable **hash** and emits one `"%s: %s\r\n"` line per entry +/// (`0x853968`), and that hash is built by the reader `0x51ebe0` from `AddOns.txt` itself — which +/// "creates entries for every line, both states", strdup'ing each name as written. So a row for an +/// addon that is not installed right now was loaded, is never removed, and is written straight back +/// out; and the spelling that survives is the FILE's, not any folder's, because the folder list +/// never enters the hash at all (wow-5875-re `system/ui/scratch/addon-enable-store.md` §5/§6). +/// +/// Both halves matter and both were divergent on the logout path, which used to rebuild the file +/// from `addon_enable_states()` — the installed-folder registry. One logout erased +/// `Uninstalled: disabled` outright and rewrote `myaddon:` as `MyAddon:`. The uninstall case is the +/// one with teeth: a player who removes a folder for a week loses the choice they made about it. pub(crate) fn write_enable_state(identity: Option<&(String, String)>, states: &[(String, bool)]) { let Some(path) = enable_state_path(identity) else { return; // no character picked, or no state folder — nothing to write to @@ -668,24 +678,21 @@ pub(crate) fn write_enable_state(identity: Option<&(String, String)>, states: &[ } } -/// Write the enable state back — the reference's own last shutdown step (`0x490bd0`'s tail, -/// after the saved-variables files), so a `DisableAddOn` from Lua survives the session. +/// Write the enable state back — the reference's own last shutdown step (`0x490c88`, after the +/// saved-variables files), so a `DisableAddOn` from Lua survives the session. +/// +/// **The same writer the AddOns screen uses** (2139). This used to render +/// `addon_enable_states()` — the installed-folder registry — over the whole file, which is not the +/// shape of `0x51ef20`: the reference emits the enable *hash*, and that hash is the file's own +/// contents plus this session's toggles. Two things followed from the difference, both reproduced +/// before the change: a row for an addon not installed this session was dropped, and every name +/// was rewritten into its folder's case rather than the spelling the file already had. pub(super) fn save_enable_state(script: &UiScript, identity: Option<&(String, String)>) { let states = script.addon_enable_states(); if states.is_empty() { - return; // nothing was ever registered — a glue-only run or a capture; an empty write is a wipe - } - let Some(path) = enable_state_path(identity) else { - return; - }; - match crate::local_state::write_atomic(&path, &render_enable_state(&states)) { - Ok(()) => info!( - "ui_script: wrote {} ({} addons)", - path.display(), - states.len() - ), - Err(e) => warn!("ui_script: cannot write {}: {e}", path.display()), + return; // nothing was ever registered — a glue-only run or a capture; nothing to merge in } + write_enable_state(identity, &states); } /// Write every loaded addon's declared saved variables — the reference's own shutdown step @@ -2439,4 +2446,162 @@ mod tests { ); let _ = std::fs::remove_dir_all(home.parent().unwrap()); } + + /// **The logout write MERGES, and it keeps the file's own spelling** (decision 2139). + /// + /// The reference's writer `0x51ef20` emits the in-memory enable hash, and the reader + /// `0x51ebe0` builds that hash from `AddOns.txt` — an entry per line, both states, each name + /// strdup'd as written. The installed-folder list never enters it. So a row survives an + /// uninstall, and the spelling that survives is the file's. + /// + /// Ours rebuilt the file from `addon_enable_states()` instead, and one logout erased + /// `Uninstalled: disabled` and rewrote `myaddon:` as `MyAddon:`. + #[test] + fn the_logout_write_keeps_rows_it_did_not_put_there() { + let _l = crate::local_state::test_env::ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE"); + let (home, _h) = hermetic_root("logout-merge"); + write_addon( + &home, + "MyAddon", + "## Interface: 11200\nmain.lua\n", + &[("main.lua", "MyAddonRan = true")], + ); + let id = ("Realm".to_string(), "Char".to_string()); + let p = crate::local_state::addons_state_path("Realm", "Char").unwrap(); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, "Uninstalled: disabled\nmyaddon: enabled\n").unwrap(); + + let mut script = UiScript::new().unwrap(); + script.set_screen_size(1024.0, 768.0); + let _ = load_third_party(&mut script, Some(&id), true); + script.eval::<()>("DisableAddOn('MyAddon')").unwrap(); + save_enable_state(&script, Some(&id)); + + let after = std::fs::read_to_string(&p).unwrap(); + let rows = parse_enable_state(&after); + // The uninstalled addon's row is still there, still disabled — the half with teeth. + assert_eq!( + rows.iter() + .find(|(n, _)| n.eq_ignore_ascii_case("Uninstalled")) + .map(|(_, on)| *on), + Some(false), + "a row for an addon not installed this session must survive the write: {after}" + ); + // The file's own spelling survives; the folder's case does not overwrite it. + assert!( + after.contains("myaddon:"), + "the file's spelling is what the reference writes back: {after}" + ); + // …and this session's toggle still landed on it. + assert_eq!( + rows.iter() + .find(|(n, _)| n.eq_ignore_ascii_case("MyAddon")) + .map(|(_, on)| *on), + Some(false), + "the DisableAddOn must still be persisted: {after}" + ); + let _ = std::fs::remove_dir_all(home.parent().unwrap()); + } + + /// **A LoadOnDemand dependency cycle LOADS BOTH and answers `(1, nil)`** (decision 2139) — + /// and before the fix this call took the whole process down with SIGABRT. + /// + /// `AddOn_Load 0x51f240` has no re-entrancy guard: every live site of the visiting byte + /// `[UIADDON+0x2d]` is inside `AddOn_CanLoad 0x51e780`, none in the loader. What bounds the + /// recursion is the *loaded* byte, stamped **before** the dependency loops — + /// `0x51f313 mov byte [rec+0x18],1`, a flag-neutral store between `0x51f311 test eax,eax` and + /// `0x51f317 jbe`, therefore unconditional — so the re-entered frame takes the already-loaded + /// early-out at `0x51f2d6`/`0x51f2db` and returns 1. We stamped it after the recursion, so + /// nothing terminated: Rust-level recursion, stack overflow, `signal 6`, uncatchable by + /// `pcall`. + /// + /// The three assertions below are the reference's own observable consequences, and the last + /// two are the ones that would betray a guard bolted on somewhere else instead: + /// **no reason token** (a cycle produces none — the reference has no such token), + /// `ADDON_LOADED` fires **B then A**, and `IsAddOnLoaded('Ping')` answers 1 throughout Pong's + /// execution, because Ping is flagged loaded before its own files run. + #[test] + fn a_load_on_demand_dependency_cycle_loads_both_sides() { + let _l = crate::local_state::test_env::ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE"); + let (home, _h) = hermetic_root("lod-cycle"); + write_addon( + &home, + "Ping", + "## Interface: 11200\n## LoadOnDemand: 1\n## Dependencies: Pong\nping.lua\n", + &[( + "ping.lua", + "PingRan = true Order = (Order or '') .. 'Ping,'", + )], + ); + write_addon( + &home, + "Pong", + "## Interface: 11200\n## LoadOnDemand: 1\n## Dependencies: Ping\npong.lua\n", + &[( + "pong.lua", + "PongRan = true Order = (Order or '') .. 'Pong,' \ + PingSeenLoaded = IsAddOnLoaded('Ping')", + )], + ); + let mut script = UiScript::new().unwrap(); + script.set_screen_size(1024.0, 768.0); + script + .eval::<()>( + "AddonOrder = '' \ + CycleWatch = CreateFrame('Frame') \ + CycleWatch:RegisterEvent('ADDON_LOADED') \ + CycleWatch:SetScript('OnEvent', function() \ + AddonOrder = AddonOrder .. arg1 .. ',' end)", + ) + .unwrap(); + let _ = load_third_party(&mut script, None, true); + + // Neither ran at startup — the walk skips LoadOnDemand, which is why a cycle among them + // is never observed until something demand-loads one. + assert_eq!( + script + .eval::("return PingRan == nil and PongRan == nil") + .ok(), + Some(true) + ); + + // The call that used to abort the process. + assert_eq!( + script + .eval::>( + "local loaded, reason = LoadAddOn('Ping') \ + return { tostring(loaded), tostring(reason) }" + ) + .ok(), + Some(vec!["1".into(), "nil".into()]), + "a cycle loads and answers (1, nil) — the reference has no cycle reason token" + ); + + // Both sides ran, each `.toc` list exactly once. + assert_eq!( + script.eval::("return Order").ok().as_deref(), + Some("Pong,Ping,"), + "the dependency's files run first, and neither list runs twice" + ); + // `ADDON_LOADED` fires B then A. + assert_eq!( + script.eval::("return AddonOrder").ok().as_deref(), + Some("Pong,Ping,") + ); + // The stamp is ahead of the files, so the cycle inverts the declared order: Ping reads as + // loaded for the whole of Pong's execution. This is the reference's behaviour and it is + // the mechanism, not a side effect. + assert_eq!( + script.eval::("return PingSeenLoaded").ok(), + Some(1), + "inside a cycle an addon is flagged loaded before its own files run" + ); + let _ = std::fs::remove_dir_all(home.parent().unwrap()); + } } diff --git a/crates/benilla-app/src/ui_script/shape_gate.rs b/crates/benilla-app/src/ui_script/shape_gate.rs index ddcaffe7..9f98cdc9 100644 --- a/crates/benilla-app/src/ui_script/shape_gate.rs +++ b/crates/benilla-app/src/ui_script/shape_gate.rs @@ -104,6 +104,12 @@ struct Row { } fn rows() -> Vec { + rows_of_kind("global") +} + +/// The same read, for one `table_kind` — `global` is the registrar surface, `baselib` the +/// 36-entry Lua base array looped straight into `_G` at `0x811e28`. +fn rows_of_kind(kind: &str) -> Vec { let tsv = concat!( env!("CARGO_MANIFEST_DIR"), "/../../reference/1.12-shapes.tsv" @@ -114,7 +120,7 @@ fn rows() -> Vec { .filter_map(|l| { let f: Vec<&str> = l.split('\t').collect(); // name fn pair_va table_va table_kind argc argc_conf arity arity_conf kinds … - if f.len() < 9 || f[4] != "global" || f[8] != "exact" { + if f.len() < 9 || f[4] != kind || f[8] != "exact" { return None; } Some(Row { @@ -261,6 +267,136 @@ fn every_query_binding_answers_the_reference_s_return_arity() { ); } +/// **The base-library arm of the gate** (decision 2136) — the 36 entries at `0x811e28`, which the +/// other three arms never looked at. +/// +/// [`rows`] filters to `table_kind = global`, so every `baselib` row in the vendored table sat +/// outside all of the arity, kinds and widget halves. That is not a small corner: it is `gcinfo`, +/// `collectgarbage`, `loadstring`, `pairs`, `type`, `tostring` — the functions every addon calls +/// most — and it is where two divergences sat with their correct arities recorded beside them the +/// whole time. `gcinfo … 2 exact (number,number) agree` and `collectgarbage … 0 exact () agree` +/// were both in this file before anyone answered either one wrong. +/// +/// **Only the no-argument-safe names are probed, by an explicit list rather than a prefix rule.** +/// The global arm can filter to query verbs because `Get*`/`Is*` names them; the base library has +/// no such convention, and calling it blind means calling `error`, `pcall` and `setfenv` for their +/// arities. So the list below is what is safe to call with nothing, and everything else is +/// uncovered — a loss of coverage, never a wrong assertion, exactly as the global arm's own filter +/// is. `seterrorhandler`/`setfenv`/`setglobal` are excluded for mutating the VM, not for raising. +const BASELIB_NO_ARG_PROBES: &[&str] = &[ + "collectgarbage", + "date", + // The five shipped stubs — `xor eax,eax; ret` in the image (`lua-dialect.md` §3a), so calling + // one is as safe here as it is there, and their `0 exact ()` rows are still worth holding. + "debugbreak", + "debugdump", + "debuginfo", + "debugload", + "debugprint", + "debugprofilestart", + "debugprofilestop", + "debugstack", + "debugtimestamp", + "gcinfo", + "geterrorhandler", + "getfenv", + "time", +]; + +/// The base library's own shrinking list — same rules as [`NOT_YET_ASSERTED`]. **Empty.** +const BASELIB_NOT_YET_ASSERTED: &[(&str, usize, &str)] = &[]; + +#[test] +fn the_base_library_answers_the_reference_s_return_arity_and_kinds() { + let rows = rows_of_kind("baselib"); + assert!( + rows.len() > 20, + "the vendored table's baselib rows look wrong: {}", + rows.len() + ); + let s = benilla_ui::script::UiScript::new().expect("VM"); + let mut checked = 0usize; + let mut mismatches: Vec = Vec::new(); + + for r in &rows { + if !BASELIB_NO_ARG_PROBES.contains(&r.name.as_str()) { + continue; + } + let name = &r.name; + let probe = format!( + "if type({name}) ~= 'function' then return -1 end \ + local ok, n = pcall(function() return select('#', {name}()) end) \ + if not ok then return -1 end return n" + ); + let got: i64 = match s.eval(&probe) { + Ok(n) => n, + Err(_) => continue, + }; + if got < 0 { + continue; + } + checked += 1; + if got as usize != r.arity { + if let Some((_, known, _)) = BASELIB_NOT_YET_ASSERTED.iter().find(|(n, ..)| n == name) { + assert_eq!(*known, r.arity, "{name} is listed at a stale arity"); + continue; + } + mismatches.push(format!( + "{name}: answers {got}, reference states {}", + r.arity + )); + continue; + } + // Kinds, wherever the table calls them trustworthy — the same `kinds_conf = agree` rule + // the global arm uses, and the column that types `gcinfo` as `(number,number)`. + if r.kinds.is_empty() { + continue; + } + let Ok(kinds) = s.eval::(&format!( + "local t = {{ {name}() }} local out = '' \ + for i = 1, {} do out = out .. (i > 1 and ',' or '') .. type(t[i]) end return out", + r.arity + )) else { + continue; + }; + let got_tuple = format!("({kinds})"); + let acceptable = r.kinds.split('|').map(str::trim).any(|alt| { + alt == got_tuple + || alt + .trim_matches(|c| c == '(' || c == ')') + .split(',') + .map(str::trim) + .zip(kinds.split(',').map(str::trim)) + // `string?` is string-or-nil; `any`/`value` accept anything. + .all(|(want, got)| { + want == got + || (want == "string?" && (got == "string" || got == "nil")) + || want == "any" + || want == "value" + }) + }); + if !acceptable { + mismatches.push(format!( + "{name}: answers kinds {got_tuple}, reference states {}", + r.kinds + )); + } + } + + // A floor, so a change that stops the probe measuring anything fails loudly. + // A floor, not a target: raise it when coverage rises, never lower it to fit a change. + assert!( + checked >= 15, + "the base-library gate measured only {checked} bindings" + ); + assert!( + mismatches.is_empty(), + "{} of {checked} probed base-library bindings diverge:\n {}", + mismatches.len(), + mismatches.join("\n ") + ); +} + /// The widget registrar tables, each mapped to a Lua expression that produces one instance. /// /// The table address IS the widget class — the reference registers one `{name, fn}` table per diff --git a/crates/benilla-ui/src/script/addon.rs b/crates/benilla-ui/src/script/addon.rs index 2dedf808..aaf1ce35 100644 --- a/crates/benilla-ui/src/script/addon.rs +++ b/crates/benilla-ui/src/script/addon.rs @@ -453,6 +453,35 @@ fn load_addon(lua: &Lua, i: usize) -> Result<(), String> { Box::new(move |req: &str| read_under(&root, req)) }; + // ── The *loaded* stamp goes HERE, before the dependency walk — and it is the whole reason a + // dependency cycle terminates (decision 2139). + // + // `AddOn_Load 0x51f240` carries **no re-entrancy guard of its own**: an image-wide census of + // the visiting byte `[UIADDON+0x2d]` puts every live site inside `AddOn_CanLoad 0x51e780` + // (writers `0x5205a8`/`0x51e8ae`/`0x51e8f1`, reader `0x51e812`) and none in the loader. What + // bounds the recursion is this byte: `0x51f313 mov byte [rec+0x18],1` sits between + // `0x51f311 test eax,eax` and `0x51f317 jbe` — a flag-neutral, therefore **unconditional** + // store — in the dependency loop's PREAMBLE, ahead of both the OptionalDeps body + // (`0x51f320`) and the required-dep body (`0x51f343`). So a re-entered frame hits the + // already-loaded early-out at `0x51f2d6`/`0x51f2db` and returns 1. + // + // We stamped it *after* the recursion instead, which is why `LoadAddOn` on two mutually + // dependent LoadOnDemand addons overflowed the stack and took the process down with SIGABRT — + // not a Lua error, so nothing `pcall` or the error handler could reach. + // + // **The gate strictly dominates this store** (`0x51f2fa`/`0x51f301` precede `0x51f313`), so a + // refusal leaves the byte clear and the load stays retryable. That ordering is why the stamp + // is here and not at the top of the function. + // + // **The consequence is real and is the reference's own**: inside a cycle an addon is flagged + // loaded before its own files run, so `IsAddOnLoaded("A")` answers 1 for the whole of B's + // execution, and `ADDON_LOADED` fires B before A. That is not a wart we are copying blindly — + // it is what makes the recursion finite, and wow-re executed it against the real bytes. + { + let mut model = lua.app_data_mut::().expect("model"); + model.addons[i].loaded = true; + } + // The gate said yes, so a dependency failing HERE is a load-time failure (its files errored, // it was uninstalled mid-session) — still mapped to the DEP_ mirror, applied once. for dep in &deps { @@ -487,11 +516,6 @@ fn load_addon(lua: &Lua, i: usize) -> Result<(), String> { // `Bindings.xml` (1188 phase 4) attaches here, between the files and the saved variables. load_bindings(lua, &name, &read); load_saved_variables(lua, i); - - { - let mut model = lua.app_data_mut::().expect("model"); - model.addons[i].loaded = true; - } // The verified position: after the files, at the end of this addon's load (`0x51f5ad`). super::event::fire_global(lua, "ADDON_LOADED", &[super::ScriptValue::Str(name)]); Ok(()) diff --git a/crates/benilla-ui/src/script/client.rs b/crates/benilla-ui/src/script/client.rs index f52de90a..ab431662 100644 --- a/crates/benilla-ui/src/script/client.rs +++ b/crates/benilla-ui/src/script/client.rs @@ -80,10 +80,21 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // `RunScript(text)` — compile and run a chunk in the shared global state. // - // The reference's is `RunScript 0x7044c0`, and it is how a macro body, a `/script` slash - // command, and every addon's "evaluate this snippet" helper reach Lua. It is the same - // `loadstring`+call our own chunk loader does, so it inherits the same sandbox: there is no - // `setfenv` here and none in the reference either — a script runs with full API access. + // The reference's is `RunScript 0x48b980` (pair `0x83e288`, name `0x83ea60`), and it is how a + // macro body, a `/script` slash command, and every addon's "evaluate this snippet" helper + // reach Lua. It inherits the same sandbox as our chunk loader: there is no `setfenv` here and + // none in the reference either — a script runs with full API access. + // + // **The chunk NAME here is ours, not the image's, and that is an open question** (2136's + // "left open"). wow-re's chunk-source census (`lowhealth-playerstatus-onupdate-dead.md` §3) + // places this binding's feed precisely — `0x48b9cb` into `0x704cd0`, the source-string arm of + // `FS_DoBuffer 0x704ae0` — but records the name argument only for its sibling arms: an XML + // `