diff --git a/crates/benilla-ui/src/script/saved.rs b/crates/benilla-ui/src/script/saved.rs index f19409f4..f052b1ee 100644 --- a/crates/benilla-ui/src/script/saved.rs +++ b/crates/benilla-ui/src/script/saved.rs @@ -56,6 +56,16 @@ impl super::UiScript { pub fn saved_variables_bytes_for(&self, names: &[String]) -> Vec { let mut out = Vec::new(); let mut unwritable = Vec::new(); + // A widget writes as its global name, and only the model can turn a `T[0]` id into one. + let name_of = |id: u32| -> Option { + let model = self.model_ref(); + model + .id_to_frame + .get(&id) + .and_then(|h| model.arena.frame(*h)) + .and_then(|f| f.name.clone()) + .or_else(|| super::region::region_name_of(&model, id)) + }; for name in names { let value: Value = match self.lua().globals().get(name.as_str()) { Ok(v) => v, @@ -65,7 +75,7 @@ impl super::UiScript { } }; let mut seen = HashSet::new(); - match serialize(&value, 1, &mut seen) { + match serialize(&value, 1, &mut seen, &name_of) { Some(text) => { out.extend_from_slice(name.as_bytes()); out.extend_from_slice(b" = "); @@ -88,14 +98,20 @@ impl super::UiScript { /// One Lua value as Lua expression text, or `None` when it has none. `depth` is the indent of a /// table's contents here (1 at top level); `seen` holds the tables on the current path, so a repeat /// is a cycle and drops, while a shared subtable is written each time it appears. -fn serialize(v: &Value, depth: usize, seen: &mut HashSet<*const c_void>) -> Option> { +fn serialize( + v: &Value, + depth: usize, + seen: &mut HashSet<*const c_void>, + name_of: &dyn Fn(u32) -> Option, +) -> Option> { match v { Value::Nil => Some(b"nil".to_vec()), Value::Boolean(b) => Some(b.to_string().into_bytes()), Value::Integer(i) => Some(i.to_string().into_bytes()), Value::Number(n) => number(*n).map(String::into_bytes), Value::String(s) => Some(quote(&s.as_bytes())), - Value::Table(t) => table(t, depth, seen), + // A widget is tested before its fields: it writes as a name, never as a table. + Value::Table(t) => widget(t, name_of).or_else(|| table(t, depth, seen, name_of)), // Functions, threads and userdata have no literal; a frame's `[0]` handle is a // lightuserdata (`0x701bd0`). _ => None, @@ -133,11 +149,54 @@ fn quote(s: &[u8]) -> Vec { out } +/// A widget wrapper's value text, or `None` for a plain table. +/// +/// The reference asks this first and never walks a widget's fields: the table branch of its value +/// writer does `lua_rawgeti(v, 0)` and `lua_touserdata` (`0x704613`-`0x704628`, the identity +/// `0x701bd0` wrote), and on a hit writes the object's **own global name, bare** — a Lua +/// expression, so the reloaded chunk's lookup hands the live object back — or +/// `nil --[[ unnamed object ]]` (`0x872e0c`) for an anonymous one. +/// +/// His own real-client file is the proof: `["Dcr_OutputWindow"] = ChatFrame1,`. Serializing the +/// frame's *fields* instead reloads a data table with no methods, which is past the owning addon's +/// guard (`Decursive.lua:1245` accepts any non-nil) and raises on the first call — the +/// `attempt to call method 'AddMessage' (a nil value)` in his log. +/// +/// Deviation: a name that is no Lua identifier writes as an anonymous object, where the reference +/// writes it raw and would leave a file that will not parse. +fn widget(t: &Table, name_of: &dyn Fn(u32) -> Option) -> Option> { + let Ok(Value::LightUserData(l)) = t.raw_get::(0) else { + return None; + }; + let id = l.0 as usize as u32; + Some(match name_of(id).filter(|n| is_lua_name(n)) { + Some(name) => name.into_bytes(), + None => b"nil --[[ unnamed object ]]".to_vec(), + }) +} + +/// Whether a name can be written bare and read back as the same global. +fn is_lua_name(s: &str) -> bool { + const KEYWORDS: [&str; 21] = [ + "and", "break", "do", "else", "elseif", "end", "false", "for", "function", "if", "in", + "local", "nil", "not", "or", "repeat", "return", "then", "true", "until", "while", + ]; + !s.is_empty() + && !s.starts_with(|c: char| c.is_ascii_digit()) + && s.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_') + && !KEYWORDS.contains(&s) +} + /// A table constructor, `{\n[key] = value,\n}`: every key bracketed, as the /// reference writes for every table shape (`[0x704607, 0x704989)`), integer keys ascending, then /// strings. It reloads a list as a list through the 1.12 parser: `recfield` gives a `[k] = v` field /// no size hint, so dense integer keys land in the array part, which `next` walks in order. -fn table(t: &Table, depth: usize, seen: &mut HashSet<*const c_void>) -> Option> { +fn table( + t: &Table, + depth: usize, + seen: &mut HashSet<*const c_void>, + name_of: &dyn Fn(u32) -> Option, +) -> Option> { if depth > MAX_DEPTH || !seen.insert(t.to_pointer()) { return None; } @@ -145,7 +204,7 @@ fn table(t: &Table, depth: usize, seen: &mut HashSet<*const c_void>) -> Option, Vec)> = Vec::new(); for pair in t.pairs::() { let Ok((k, v)) = pair else { continue }; - let Some(value) = serialize(&v, depth + 1, seen) else { + let Some(value) = serialize(&v, depth + 1, seen, name_of) else { continue; // an unserializable entry drops, like the reference's }; match k { @@ -365,4 +424,76 @@ mod tests { .eval::("return KEYS[string.char(200)] == 1 and KEYS[string.char(201)] == 2") .unwrap()); } + + /// Decursive's shape, and his log's `attempt to call method 'AddMessage' (a nil value)`: it + /// stashes `DEFAULT_CHAT_FRAME` in its saved table and calls a method on whatever comes back + /// (`Decursive.lua:1393`), past a guard that accepts any non-nil (`:1245`). The reference + /// writes a named widget as its bare global name — `["Dcr_OutputWindow"] = ChatFrame1,` in his + /// own real-client file — so the reload hands back the live frame, methods and all. + #[test] + fn a_saved_frame_comes_back_as_the_frame_not_a_copy_of_its_fields() { + let s = UiScript::new().unwrap(); + s.run( + r#" + local f = CreateFrame("Frame", "MyOutputWindow") + f.buttonSide = "left" + SAVED = { window = f } + RegisterForSave("SAVED") + "#, + ) + .unwrap(); + let bytes = s.saved_variables_bytes(); + let text = String::from_utf8_lossy(&bytes).to_string(); + assert!( + text.contains("[\"window\"] = MyOutputWindow,"), + "a named frame writes as its global name; wrote:\n{text}" + ); + assert!( + !text.contains("buttonSide"), + "a widget's own fields are never walked; wrote:\n{text}" + ); + + // The restart: FrameXML builds the frame, then the saved chunk runs. + let fresh = UiScript::new().unwrap(); + fresh + .run(r#"CreateFrame("Frame", "MyOutputWindow")"#) + .unwrap(); + fresh.run_chunk(&bytes).unwrap(); + assert!( + fresh + .eval::("return SAVED.window == MyOutputWindow") + .unwrap(), + "the saved value is the live frame again" + ); + assert_eq!( + fresh + .eval::("return SAVED.window:GetName()") + .unwrap(), + "MyOutputWindow", + "so a method call on it still works" + ); + } + + /// An anonymous widget has no expression to write, so it takes the reference's own sentinel + /// (`0x872e0c`) and reloads as nil — which is what the addon's own nil guard is for. + #[test] + fn an_anonymous_frame_saves_as_the_references_unnamed_object() { + let s = UiScript::new().unwrap(); + s.run( + r#" + SAVED = { window = CreateFrame("Frame") } + RegisterForSave("SAVED") + "#, + ) + .unwrap(); + let bytes = s.saved_variables_bytes(); + let text = String::from_utf8_lossy(&bytes).to_string(); + assert!( + text.contains("[\"window\"] = nil --[[ unnamed object ]],"), + "wrote:\n{text}" + ); + let fresh = UiScript::new().unwrap(); + fresh.run_chunk(&bytes).unwrap(); + assert!(fresh.eval::("return SAVED.window == nil").unwrap()); + } } diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 52f020c3..eb681167 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -415,3 +415,4 @@ with the load checked after each. | `PlayerModel:SetUnit` tracked the token instead of holding the unit | **fixed** | unitscan's alert reads `SetUnit'target'` on the rare it just found by name, and the addon restores the player's own target on the next line — so a pane that re-resolved `"target"` per paint showed the card for one frame and then emptied. Reported the moment the by-name targeting stopped leaving the target on the rare: *"unitscan is not targetting anymore, very nice, but now it only shows the 3d model when I do target"*. The reference resolves the token in `SetUnit`'s own rebuild worker (`0x505b50`, the one `RefreshUnit` also runs) and nothing re-resolves it per paint — which is why FrameXML re-issues `SetUnit` from `PaperDollFrame_OnShow` and on `UNIT_MODEL_CHANGED`, and why pfUI can detect a portrait change with **no event at all** by pinning a scratch pane and comparing `GetModel` against the shown one (`api/unitframes.lua:2069`), a test that only says anything if a pane holds what it was given. `ModelState::unit_guid` is that pin, resolved at the call and read by one helper (`Model::unit_pane_subject`) for both the paint gate and the extract. A pane seated **before** its unit streamed in keeps re-reading its token, since our subject arrives asynchronously where the reference's is already in the object manager; `RefreshUnit` re-pins, as its worker does. Test: `a_unit_pane_keeps_the_unit_it_was_given_when_the_token_moves_on`. Still true and still not built: `GetModel()` answers nil on a unit pane here, so pfUI's eventless detection leg can never fire — its event leg does, and that is what draws its portraits | | An item tooltip's rebuild was silent, so an addon's lines lasted one refresh | **fixed** | Reported from play with a video: hovering a bag item draws the enhanced tooltip — MasterTradeSkills' recipe block, AdvancedTradeSkillWindow2's `Bags/Bank/Total` — for about nine frames of sixty, and then the plate reverts to the bare one and stays bare while the cursor sits still. The loop behind it is stock: `ContainerFrameItemButton_OnUpdate` re-enters `..._OnEnter` every `TOOLTIP_UPDATE_TIME` (`ContainerFrame.lua:693` in the player's own `patch-4.mpq`, and pfUI's bag slots are `ContainerFrameItemButtonTemplate`, so they run it too), and each turn is `SetOwner` + `SetBagItem` — a clear that wipes every line an addon appended. What puts them back in the reference is a **hide/show pair**: the `SetOwner` binding hides the plate before it even reads the owner argument (`0x531179` zeroes the content gate at `+0xd0`, `0x531183` calls `[vtable+0x84]` = `0x530a60`, which is the `SetOwner(NULL)` core plus the base hide `0x76ad50`), and the content `Set*` re-shows it through the tooltip's own Show override (`0x530a80`), so every rebuild raises `OnHide` then `OnShow`. Ours cleared in place, and the base show early-outs on an already-visible frame (`0x76ae3b`), so the plate never hid and the `OnShow` never came. `LibItemTooltip-1.0` — the tooltip contract of MasterTradeSkills and Gromissingcrafts — hangs entirely on a scratch child frame of `GameTooltip` listening for `OnShow`, which is why exactly those two addons' lines vanished while pfUI's `ItemID` and sell-price lines, hooked on the `Set*` methods themselves, survived every refresh. All addresses read off the player's own `WoW.exe` | | The object sparkle was armed but buried inside the object | **fixed** — the placement is ours | *"ore vein is still not sparkling"*, after the switch fix. Reproduced offline in the `fxview` GameObject lane (`WOW_CAPTURE=fxview WOW_FX_GO=310 WOW_FX_GO_TYPE=3`, a copper mining node): the gate fires, `attach_spell_fx` spawns the instance (`kit spawn … Particles\LootFX.mdl`), and `WOW_PARTICLE_CENSUS` lists all four of `LootFX`'s emitters at the node's own position, live and ungated — while the captured frame is **byte-identical** to one taken with the sparkle gated off. The attach cascade's last rung is the host's origin, which for a unit is right (`0x13` is attachment 19, Base, the ground point under its feet, and this is a ground twinkle); a 1.12 **GameObject** model authors no attachment points at all (`copper_Miningnode_01.m2`: 0), so the effect sat at the node's pivot, inside the rock — that model's own vertices run y ∈ [-3.83, +2.73]. A host with no attachment points now seats the effect at the top centre of its own box, and the capture differs; the same sparkle on a peacebloom, whose model is small, already showed at the pivot. The placement is ours and no more: SuperWoW publishes no source and the reference has no object arm to copy. It is also **subtle by design** — `SpellVisualEffectName` row 14 scales 1.0 and `Particles\LootFX.m2` is four emitters of 0.06–0.14 yd particles with ~0.9 yd of reach — so it reads as a few twinkles over the node rather than a beacon | +| A frame left in saved variables came back as a bag of its fields | **fixed** | `attempt to call method 'AddMessage' (a nil value)` at `Decursive.lua:1393`, every login. Decursive keeps its output window in its own saved table (`Dcr_OutputWindow = DEFAULT_CHAT_FRAME`) and guards the reload with `if Dcr_Saved.Dcr_OutputWindow == nil or not ...` (`:1245`) — which a non-nil **data** table passes. Our writer walked the frame like any other table and wrote ChatFrame1's own fields (`buttonSide`, `channelList`, `editBox`, …), doubling the file (2217 bytes against his real client's 1108) and losing every method. The reference tests the value's `T[0]` identity first — `lua_rawgeti(v, 0)` then `lua_touserdata` at `0x704613`-`0x704628`, the lightuserdata `0x701bd0` wrote — and never walks a widget: a **named** one writes as its bare global name, a Lua expression, so the reloaded chunk's own lookup hands the live object back, and an anonymous one as `nil --[[ unnamed object ]]` (`0x872e0c`). His own real-client file is the proof: `["Dcr_OutputWindow"] = ChatFrame1,`. Deviation: a name that is no Lua identifier writes as an anonymous object, where the reference writes it raw (`%s`) and can leave a file that will not parse. Tests: `a_saved_frame_comes_back_as_the_frame_not_a_copy_of_its_fields`, `an_anonymous_frame_saves_as_the_references_unnamed_object` |