saved: a widget writes as its name, so a saved frame comes back a frame

Decursive raised `attempt to call method 'AddMessage' (a nil value)` on every
login (`Decursive.lua:1393`). It keeps its output window in its own saved table
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: his `Decursive.lua` came back
holding ChatFrame1's own fields (`buttonSide`, `channelList`, `editBox`, …),
twice the size of the real client's file and with no methods on it at all.

The reference tests the value's `T[0]` identity before its fields —
`lua_rawgeti(v, 0)` then `lua_touserdata`, `0x704613`-`0x704628`, reading 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; 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 and would leave a file that will not parse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn

crates/benilla-ui/src/script/saved.rs

@@ -56,6 +56,16 @@ impl super::UiScript {
     pub fn saved_variables_bytes_for(&self, names: &[String]) -> Vec<u8> {
         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<String> {
+            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<Vec<u8>> {
+fn serialize(
+    v: &Value,
+    depth: usize,
+    seen: &mut HashSet<*const c_void>,
+    name_of: &dyn Fn(u32) -> Option<String>,
+) -> Option<Vec<u8>> {
     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<u8> {
     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<String>) -> Option<Vec<u8>> {
+    let Ok(Value::LightUserData(l)) = t.raw_get::<Value>(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<tabs>[key] = value,\n<tabs-1>}`: 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<Vec<u8>> {
+fn table(
+    t: &Table,
+    depth: usize,
+    seen: &mut HashSet<*const c_void>,
+    name_of: &dyn Fn(u32) -> Option<String>,
+) -> Option<Vec<u8>> {
     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<V
     let mut strs: Vec<(Vec<u8>, Vec<u8>)> = Vec::new();
     for pair in t.pairs::<Value, Value>() {
         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::<bool>("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::<bool>("return SAVED.window == MyOutputWindow")
+                .unwrap(),
+            "the saved value is the live frame again"
+        );
+        assert_eq!(
+            fresh
+                .eval::<String>("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::<bool>("return SAVED.window == nil").unwrap());
+    }
 }

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` |