diff --git a/crates/benilla-ui/src/script/lua50/table_size.rs b/crates/benilla-ui/src/script/lua50/table_size.rs index 6772a558..a271e3bf 100644 --- a/crates/benilla-ui/src/script/lua50/table_size.rs +++ b/crates/benilla-ui/src/script/lua50/table_size.rs @@ -25,11 +25,42 @@ //! in every frame, for the whole session. The same mismatch silently *loses* data the other way: //! `insert` after a `#` append writes over the appended slot instead of after it. //! -//! So a size this library **derived** is treated as a cache: if `t[n + 1]` is not nil the table has -//! grown since, and the count wins. A size an addon **declared** with `table.setn` is still -//! honoured exactly as 5.0 honours it — that is the whole point of `setn(t, 0)` as a logical wipe -//! (`AceLibrary`, Mik's capture tables), and it is also why the two cases cannot share one slot. -//! An explicit numeric `t.n` field is a declaration too, and stays untouched. +//! So a size this library **derived** is treated as a cache: it is the truth only while the table +//! still looks the way the `insert`/`remove` that wrote it left it — nothing in `t[n + 1]`, and +//! something in `t[n]`. Otherwise the count wins. A size an addon **declared** with `table.setn` +//! is still honoured exactly as 5.0 honours it — that is the whole point of `setn(t, 0)` as a +//! logical wipe (`AceLibrary`, Mik's capture tables), and it is also why the two cases cannot +//! share one slot. An explicit numeric `t.n` field is a declaration too, and stays untouched. +//! +//! ## The other half: a table emptied behind the library's back +//! +//! The same mismatch runs the other way, and it is the more common one. Ace2 decides which Lua it +//! is on by compiling a *grammar* probe: +//! +//! ```lua +//! local lua51 = loadstring("return function(...) return ... end") and true or false +//! local table_setn = lua51 and function() end or table.setn +//! ``` +//! +//! That is `Dewdrop-2.0:22` and `Tablet-2.0`'s head, and it is sound on either real client: 5.0 +//! cannot compile `function(...)` so it keeps calling `setn`, and a real 5.1 answers `getn` from +//! `#`, where a cleared table is 0 either way. This client is the one combination neither library +//! was written for — the realm's parser takes 5.1's grammar while its table library is still +//! 5.0's `n` — so the probe says 5.1, `table_setn` becomes a no-op, and the only reset of the +//! remembered size is gone. +//! +//! `Dewdrop:FeedAceOptionsTable` recycles one `values` table across calls: it clears it with +//! `for k in pairs(values) do values[k] = nil end`, which this layer never sees, then reads +//! `table.getn(values)` and gets the size the last call's inserts left. On the second menu it +//! walks that stale count with `table.remove`, descends `options.args[nil]`, and returns having +//! added no lines: **every dropdown after the first in a session opens as an empty box.** It is +//! not one addon — the folder this was found in has seven copies of that head across BigWigs, +//! AuldLangSyne, SuperAPI, NampowerSettings and shootyepgp, and whichever wins AceLibrary's +//! arbitration serves all of them. +//! +//! Treating a derived size as stale once `t[n]` is gone fixes all seven without touching a +//! library, and refuses to guess for the holed case: a table whose *last* slot is still there +//! keeps its remembered size, hole or no hole, exactly as 5.0 would. use mlua::{Function, Lua, MultiValue, Table, Value}; @@ -51,15 +82,22 @@ struct Sizes { impl Sizes { /// `luaL_getn` (`0x6f5050`); its count to the first nil is not the `#` border on a holed - /// table. A size this library derived is dropped when the table has grown past it. + /// table. A size this library derived is dropped once the table has grown past it or lost the + /// slot it ended on; see the module doc for both. fn get(&self, t: &Table) -> mlua::Result { if let Some(n) = as_size(&t.raw_get::("n")?) { return Ok(n); } if let Some(n) = as_size(&self.store.raw_get::(t.clone())?) { - let declared = self.declared.raw_get::(t.clone())?; + let declared = matches!( + self.declared.raw_get::(t.clone())?, + Value::Boolean(true) + ); let grown = !matches!(t.raw_get::(n + 1)?, Value::Nil); - if matches!(declared, Value::Boolean(true)) || !grown { + // Shrunk behind our back: only the last remembered slot is checked, so a hole in the + // middle still keeps the size, which is what 5.0 does. + let shrunk = n >= 1 && matches!(t.raw_get::(n)?, Value::Nil); + if declared || (!grown && !shrunk) { return Ok(n); } } @@ -377,6 +415,95 @@ mod tests { ); } + /// Dewdrop-2.0's recycled `values` table: insert, clear with `pairs`, read the size again. + /// On this client `table_setn` is the no-op branch, so nothing resets the remembered size and + /// the stale count is what made every dropdown after the first open empty. + #[test] + fn a_derived_size_is_stale_once_the_table_has_been_emptied() { + let s = UiScript::new().unwrap(); + assert_eq!( + s.eval::( + r#" + local values = {} + table.insert(values, "a") + table.insert(values, "b") + table.insert(values, "c") + for k in pairs(values) do values[k] = nil end + return table.getn(values) + "# + ) + .unwrap(), + 0, + "every slot the remembered 3 covered is gone, so the count is 0" + ); + // And the loop that count feeds: Dewdrop drains `values` with `remove` before deciding + // whether it is looking at a group, and a stale drain hands it `options.args[nil]`. + assert_eq!( + s.eval::( + r#" + local t = {} + table.insert(t, "a") + for k in pairs(t) do t[k] = nil end + local drained = 0 + while table.getn(t) > 0 do + table.remove(t) + drained = drained + 1 + if drained > 8 then break end + end + return drained + "# + ) + .unwrap(), + 0, + "an emptied table must not hand out removes that answer nil" + ); + } + + /// The line the shrink check is drawn on: a hole in the middle leaves the size alone, because + /// 5.0 leaves it alone and an addon may well be about to fill the hole back in. + #[test] + fn a_hole_in_the_middle_keeps_the_remembered_size() { + let s = UiScript::new().unwrap(); + assert_eq!( + s.eval::( + "local t = {} for i = 1, 5 do table.insert(t, i) end \ + t[2] = nil return table.getn(t)" + ) + .unwrap(), + 5, + "slot 5 is still there, so the remembered 5 still describes the table" + ); + assert_eq!( + s.eval::( + "local t = {} for i = 1, 5 do table.insert(t, i) end \ + t[2] = nil table.insert(t, 9) return t[6]" + ) + .unwrap(), + 9, + "and insert still appends after it rather than filling the hole" + ); + } + + /// A declaration survives the clear: `setn` is the addon stating the size, and this is the + /// shape `AceLibrary` relies on. + #[test] + fn a_declared_size_survives_an_emptied_table() { + let s = UiScript::new().unwrap(); + assert_eq!( + s.eval::( + r#" + local t = { "a", "b", "c" } + table.setn(t, 3) + for k in pairs(t) do t[k] = nil end + return table.getn(t) + "# + ) + .unwrap(), + 3, + "the addon declared 3; only a derived size is a cache" + ); + } + /// Mik's Scrolling Battle Text 4.43's capture tables: wipe, `setn(t, 0)`, then insert. #[test] fn a_wipe_then_setn_zero_then_insert_is_countable_again() {