diff --git a/crates/benilla-app/examples/addon_harness.rs b/crates/benilla-app/examples/addon_harness.rs index ce4f752e..5ee13638 100644 --- a/crates/benilla-app/examples/addon_harness.rs +++ b/crates/benilla-app/examples/addon_harness.rs @@ -61,6 +61,30 @@ fn probe_steps(rest: &[String]) -> Vec { "--plates" => Some(addon_harness::probe::Step::Plates( rest.get(i + 1).and_then(|n| n.parse().ok()).unwrap_or(3), )), + "--click" => { + let spec = rest.get(i + 1)?; + let mut it = spec.split(','); + let x: f32 = it.next()?.trim().parse().ok()?; + let y: f32 = it.next()?.trim().parse().ok()?; + let b = it.next().unwrap_or("LeftButton").trim().to_string(); + Some(addon_harness::probe::Step::Click(x, y, b)) + } + "--drag" => { + let spec = rest.get(i + 1)?; + let v: Vec = spec + .split(',') + .filter_map(|p| p.trim().parse().ok()) + .collect(); + (v.len() == 4).then(|| addon_harness::probe::Step::Drag(v[0], v[1], v[2], v[3])) + } + "--wheel" => { + let spec = rest.get(i + 1)?; + let mut it = spec.split(','); + let x: f32 = it.next()?.trim().parse().ok()?; + let y: f32 = it.next()?.trim().parse().ok()?; + let d: f32 = it.next().unwrap_or("-1").trim().parse().ok()?; + Some(addon_harness::probe::Step::Wheel(x, y, d)) + } "--mouse" => { let (x, y) = rest.get(i + 1)?.split_once(',')?; Some(addon_harness::probe::Step::Mouse( diff --git a/crates/benilla-app/src/addon_harness/probe.rs b/crates/benilla-app/src/addon_harness/probe.rs index eac54878..74ff7a40 100644 --- a/crates/benilla-app/src/addon_harness/probe.rs +++ b/crates/benilla-app/src/addon_harness/probe.rs @@ -42,6 +42,21 @@ pub enum Step { /// Move the cursor to `(x, y)` in UI units (y-up from the bottom-left), firing the real /// `OnLeave`/`OnEnter` pair as [`UiScript::mouse_move`] does for the app. Mouse(f32, f32), + /// Press and release a mouse button at `(x, y)`, through the hit-test the app uses. + /// + /// Calling a frame's `OnClick` by hand answers "does this handler raise"; it cannot answer + /// "does a click reach this frame", which is a different question and the one that matters + /// for a map whose whole surface is one button. Drives [`UiScript::mouse_button`] down then + /// up, so capture, `OnMouseDown`/`OnMouseUp` and the click dispatch all run as they do live. + Click(f32, f32, String), + /// Press at `(x1, y1)`, move to `(x2, y2)` in steps, release. A pan is the interaction a + /// stitched map lives on and it cannot be driven by `OnClick`: the drag is a press, a run of + /// moves while captured, and a release, and only the moves carry the delta. + Drag(f32, f32, f32, f32), + /// Turn the wheel at `(x, y)`, through the same hit-test. A canvas addon takes the wheel on + /// whichever frame is topmost under the cursor, so asking a controller to zoom is not the + /// same as turning the wheel over it. + Wheel(f32, f32, f32), /// Advance one `OnUpdate` frame of `secs`; the answer is what the tick raised, or `no errors`. Tick(f32), /// Fire a global event into the VM, as the app's own feeds do. The answer is what the @@ -316,6 +331,64 @@ fn run_steps(script: &mut UiScript, steps: &[Step]) -> Vec<(String, String)> { ), ) } + Step::Click(x, y, button) => { + script.resolve(); + script.mouse_move(*x, *y); + let hit = script + .hit_test_name(*x, *y) + .unwrap_or_else(|| "".into()); + let down = script.mouse_button(*x, *y, button, true); + let up = script.mouse_button(*x, *y, button, false); + let raised = script.take_errors(); + ( + format!("--click {x},{y},{button}"), + if raised.is_empty() { + format!("= {button} on {hit} (down {down}, up {up})") + } else { + format!("= {button} on {hit} RAISED: {}", raised.join(" | ")) + }, + ) + } + Step::Drag(x1, y1, x2, y2) => { + script.resolve(); + script.mouse_move(*x1, *y1); + script.mouse_button(*x1, *y1, "LeftButton", true); + // Several moves, not one: a handler that integrates per-move deltas sees nothing + // from a single jump, and that is how a canvas pans. + const STEPS: u32 = 8; + for i in 1..=STEPS { + let t = i as f32 / STEPS as f32; + script.mouse_move(x1 + (x2 - x1) * t, y1 + (y2 - y1) * t); + // A frame per move, not just a move: a canvas pans from its `OnUpdate`, + // which reads the live cursor. Without the tick the cursor arrives at the + // far end having passed through nothing, and the drag reads as a no-op. + script.tick(1.0 / 60.0); + } + script.mouse_button(*x2, *y2, "LeftButton", false); + let raised = script.take_errors(); + ( + format!("--drag {x1},{y1}->{x2},{y2}"), + if raised.is_empty() { + "= dragged".to_string() + } else { + format!("= dragged RAISED: {}", raised.join(" | ")) + }, + ) + } + Step::Wheel(x, y, delta) => { + script.resolve(); + script.mouse_move(*x, *y); + let took = script.mouse_wheel(*x, *y, *delta); + let raised = script.take_errors(); + ( + format!("--wheel {x},{y},{delta}"), + if raised.is_empty() { + format!("= taken={took}") + } else { + format!("= taken={took} RAISED: {}", raised.join(" | ")) + }, + ) + } Step::Mouse(x, y) => { // Resolve first: the hit-test reads resolved rects, which a new or moved frame // lacks until then. diff --git a/crates/benilla-ui/src/script/stdlib.rs b/crates/benilla-ui/src/script/stdlib.rs index 9c614c86..116bfaa2 100644 --- a/crates/benilla-ui/src/script/stdlib.rs +++ b/crates/benilla-ui/src/script/stdlib.rs @@ -379,6 +379,14 @@ do return unpack(result) end + -- `string.split` is `strsplit` reached as a method, and the delimiter is the subject of the + -- call: Ace3's `AceConfigDialog-3.0` and its `TreeGroup` widget both spell it + -- `("\001"):split(uniquevalue)`, so `self` is the delimiter and the argument is the string. + -- Same function, same argument order, no wrapper. It is here for the same reason as the four + -- names above -- the addon corpus this client loads calls it, and a missing one raises + -- `attempt to call method 'split'` and takes the options window down with it. + string.split = strsplit + -- `...` reads as an EXPRESSION nowhere in 5.0 (`lua50` deletes that `simpleexp` arm), so -- `strjoin` takes its arguments out of `arg`, which holds the VARARGS alone — 5.0's rule, and -- `function(a, ...)` called with three answers `arg.n == 2`. The named `delimiter` is not in it. diff --git a/crates/benilla-ui/src/script/tests/stdlib.rs b/crates/benilla-ui/src/script/tests/stdlib.rs index 3b2dd3fb..d4d124fb 100644 --- a/crates/benilla-ui/src/script/tests/stdlib.rs +++ b/crates/benilla-ui/src/script/tests/stdlib.rs @@ -146,6 +146,37 @@ fn strsplit_splits_plain_and_keeps_empty_fields() { ); } +/// `string.split` is the same function reached as a METHOD, which puts the delimiter on the left +/// of the colon: Ace3's `AceConfigDialog-3.0` and its `TreeGroup` widget both spell it +/// `("\001"):split(uniquevalue)`. It answers one value per field, like `strsplit` -- a table +/// instead raises `invalid value (table) at index 1 in table for 'concat'` inside `Select`, which +/// is how BigWigs' own `function string:split` used to take down the options window of every Ace3 +/// addon in the folder. +#[test] +fn string_split_is_strsplit_reached_as_a_method() { + let s = script(); + assert!( + s.eval::(r#"return string.split == strsplit"#) + .unwrap(), + "the method and the global must be one function, so neither can drift from the other" + ); + // The Ace3 call shape, verbatim: the delimiter is the subject of the call. + assert_eq!( + s.eval::("local a, b, c = (\"\\001\"):split(\"a\\001b\\001c\") return a .. b .. c") + .unwrap(), + "abc" + ); + // One value per field, not a table: read the arity through locals, as 1.12 has no `select`. + assert_eq!( + s.eval::( + r#"local a, b = ("\001"):split("a\001b") return (type(a) == "string" and b == "b") and 1 or 0"# + ) + .unwrap(), + 1, + "a table answer is what broke every Ace3 options window" + ); +} + /// `trim` is TurtleWoW's own name for `strtrim` semantics, and every one of its callers writes it /// identically as `gsub(str, "^%s*(.-)%s*$", "%1")` (aux-addon `util.lua`, ShaguDPS `parser.lua`). /// Both ends go, interior spacing stays — the guild-roster filter relies on the whole of it. diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 7aeb4da8..f04212dd 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -3129,3 +3129,136 @@ function in one, a module method in the other — and SoftSpoken's slider row is unguarded widget calls are OpenMapClassic's three and DoiteAuras' two. It was briefly written down here that this was also the cause of SoftSpoken's blank settings page. It is not, and that page is still open. + +## The blank SoftSpoken window was sitting on the map, and the realm's own LibStub put it there + +The section above ends saying SoftSpoken's blank settings page "is still open". It is closed now, +and it turned out to be the same defect as *"none of the map clicks do anything"* — one frame +causing both. + +### What was actually over the map + +Driving the real pointer at the middle of the map (`--click`, `--drag`) reached nothing: a hook on +`WorldMapButton`'s `OnMouseDown` counted **0** hits, although the button was shown, mouse-enabled, +and its screen rect held the point. A Lua walk of every visible mouse-enabled frame whose screen +rect contains that point answered with exactly **one** frame, and it was not the map: + +``` +mouse-enabled frames over (1143,856): 1 + Frame lvl=100 strata=FULLSCREEN_DIALOG 700x500 parent=UIParent +``` + +700x500 at `FULLSCREEN_DIALOG` is AceGUI-3.0's default `Frame` container. Its regions named it: +a `Close` button, the title `SoftSpoken Stories`, and **empty content**. The blank login popup was +a full-size invisible-looking dialog parked over the centre of the screen, swallowing every click +meant for the map underneath. Both of grom's reports were one frame. + +`hit_test_name` had been printing `` for that point the whole time. That is the probe's +blind spot, not a client fault — it returns `None` for an unnamed frame, and this frame has no +name. **Ask what holds the point, not what it is called.** + +### Why the window was left shown + +`SoftSpoken_Stories/Options.lua:357` builds it inside a `pcall`ed step: + +```lua +RunOptionalStep("AceGUI options frame", function() + self.frame = AceGUI:Create("Frame") -- AceGUI shows a container on acquire + AceConfigDialog:Open("SpokenQuests", self.frame) + self.frame:SetLayout("Fill") + self.frame:Hide() -- the LAST line +end) +``` + +`AceGUI:Create` shows the container, and `Hide()` is the final statement. Anything that raises in +between leaves a shown, empty, screen-covering window and records the reason where nobody looks — +`VoiceOver.Options.initializationErrors`. Reading that table gave the real error in one step: + +``` +AceGUI-3.0.lua:282: attempt to perform arithmetic on local `argc' (a string value) +``` + +### The realm's LibStub hardcodes every library to minor 2 + +`AceGUI-3.0.lua:282` is `WidgetBase.Fire`, and the live copy was **MasterTradeSkills'** — Ace3v, +minor 34 — although OpenMapClassic ships minor **41**. Every library in `LibStub.minors` read `2`. +That is not a bug of ours. `Interface\FrameXML\AIO\Dep_LibWindow-1.1\LibStub.lua`, listed by the +realm's own `FrameXML.toc:162` and therefore loaded before any addon, ships this: + +```lua +function LibStub:NewLibrary(major, minor) + assert(type(major) == "string", "Bad argument #2 to `NewLibrary' (string expected)") + minor = 2 +``` + +Upstream reads `minor = assert(tonumber(strmatch(minor, "%d+")), …)`; 1.12's Lua 5.0 has no +`strmatch`, and whoever packaged the AIO transport "fixed" the line by nailing the value. So +**every library registers as minor 2, and the first copy of each to load keeps it** — later, +higher copies get `oldminor >= minor` and are refused. + +Widget *types* are a separate registry: `RegisterWidgetType(name, ctor, version)` compares real +versions, so OpenMapClassic's modern widgets (`Frame`=30, `TabGroup`=38, `TreeGroup`=49) **do** +replace Ace3v's inside the Ace3v core. The result is an old core hosting modern widgets, and the +two disagree about one argument: + +| | signature | a tab click sends | +|---|---|---| +| Ace3v | `Fire(self, name, argc, a1…a10)` | `Fire("OnGroupSelected", 1, value)` | +| modern | `Fire(self, name, ...)` | `Fire("OnGroupSelected", value)` | + +The modern widget's first payload value lands in `argc`, and `argc+3` raises on a string. + +Finding this took four wrong turns worth recording, because each looked like a client bug: a +verbatim replica of the file registers correctly; `strfind(34, "(%d+)")` coerces the number and +captures `34` exactly as the reference does; the function's `getfenv` *is* `_G` and every global in +it is the right one; and a known-good `NewLibrary` installed on the live table stores the right +value. The function was simply not from the file it looked like it came from. **A Lua function +will name its own file if you make it raise** — `pcall(f, nil, "X", 5)` answered +`Interface\FrameXML\AIO\Dep_LibWindow-1.1\LibStub.lua:26`, and that ended the search. `debug` is +absent here, so forcing a raise is the way to ask. + +### The port edit + +`WidgetBase.Fire` in both Ace3v copies (MasterTradeSkills, SoftSpoken_Stories) now shifts a +non-numeric count into the payload and counts what arrived. Every Ace3v caller passes a number, so +a non-number is unambiguously a modern widget's first argument; the shift gives up the tenth slot, +which no AceGUI widget reaches. Ace3v's own callbacks take `(widget, event, argc, value)` — +`GroupSelected(widget, event, _, uniquevalue)` in its `AceConfigDialog-3.0` — so a computed count +is exactly what they want, and the modern widgets become correct callers rather than tolerated +ones. + +With that in, `initializationErrors` is **0**, the window is `shown=0` at login, nothing covers the +map, and a real `--drag` pans it. The settings window opens with its real tabs — Audio, General, +Voice Packs, Profiles — and clicking them switches pages with no raise. + +### Two more defects it was hiding + +Opening all three registered options tables (`MasterTradeSkills`, `OpenMapClassic`, +`SpokenQuests`) found two failures that had been unreachable behind the first one: + +- **`issecretvalue`** — OpenMapClassic's vendored modern `AceGUIWidget-Heading.lua:33` calls it + bare. It is a recent retail global that neither 1.12 nor this realm has, so it raised on the + first heading drawn. The addon's *own* `Core.lua:159` already guards it + (`issecretvalue ~= nil and issecretvalue(v) or false`); the widget now guards it the same way. + Guarding the caller beats inventing a client global for a feature this client does not have. +- **`string.split` answered a table.** Ace3's `AceConfigDialog-3.0` and its `TreeGroup` both spell + it `("\001"):split(uniquevalue)` — delimiter on the left of the colon — and got a table, so + `tconcat(path, "\001", 1, i)` raised `invalid value (table) at index 1`. The clobber was + `function string:split` in **BigWigs' `Plugins/SpellRequests.lua:172`**, which answers a table + and reads subject and delimiter the other way round. Nothing in BigWigs ever called it; pfUI + names this exact override in `api/api.lua:97` as its reason for not delegating. Removed there, + and `string.split = strsplit` added to the client's beyond-1.12 string family, which is the same + function with the same argument order. + +All three windows now open clean: MasterTradeSkills 4 labels (its real first page), OpenMapClassic +**83**, SpokenQuests 6. Session errors across the whole folder: **0**. + +### The harness was lying about panning + +Before any of this, `--drag` reported that the map would not pan while calling +`ScrollContainer:OnMouseDown("LeftButton")` by hand panned it correctly. `Step::Drag` moved the +cursor eight times and **never advanced a frame**, and a canvas pans from its `OnUpdate`, which +reads the live cursor. With a tick per move the same drag walks `0.5000,0.5000 → +0.7582,0.3073 → 0.5080,0.5161`, and `0.7582,0.3073` is exactly what hand-driving produced. A +gesture instrument that does not tick measures nothing: **the frame between the moves is the +gesture.**