From b7a8e1c406ff2940a8d693898c1bcb6a415ffa2f Mon Sep 17 00:00:00 2001 From: grom Date: Tue, 3 Nov 2026 19:34:52 -0700 Subject: [PATCH] ui: a Slider answers SetEnabled, because OpenMap's options tab called it unguarded Rendering a panel is not the same as it working. OpenMap's options menu opens 500x500 with 104 frames and 49 placed labels, and its Look tab died on click: Map/Panel.lua:1084, attempt to call method SetEnabled (a nil value) -- so a third of its options was unreachable. SetEnabled is already composed here for Button, CheckButton, EditBox and LootButton, and was deliberately withheld from a Slider, because addons duck-type on the enabled family to tell a control from a bar. What they duck-type on is the TRIO (if widget.IsEnabled then), that is what tests/slider.rs pins, and the trio is still absent; SetEnabled is beyond 1.12 by construction, so a Slider answering it invents no 1.12 behaviour. OpenMapClassic calls it unguarded on every options row with a condition, which is what settles it. On a Slider it writes the mouse -- input refused, art not greyed, which 1.12 has no slot for; the caller dims it itself on the next line. The argument goes through GetBoolOrDefault, not Lua truthiness, because DoiteAuras writes SetEnabled(ok and 1 or 0) and 0 is truthy in Lua. Also records a count that was wrong until checked: grep says 39 calls across five addons, but AdvancedTradeSkillWindow2 and SoftSpoken define their own SetEnabled and SoftSpoken's slider row is fully guarded. The real unguarded widget calls are OpenMap's three and DoiteAuras' two -- and SoftSpoken's blank settings page is NOT this, and is still open. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- .../src/script/compat/classic_api/frame.rs | 33 +++++++++++++-- crates/benilla-ui/src/script/slider.rs | 25 +++++++++++ .../src/script/tests/widget_surface.rs | 12 ++++-- docs/ADDON_COMPAT.md | 41 +++++++++++++++++++ 4 files changed, 104 insertions(+), 7 deletions(-) diff --git a/crates/benilla-ui/src/script/compat/classic_api/frame.rs b/crates/benilla-ui/src/script/compat/classic_api/frame.rs index 75138002..a5316e12 100644 --- a/crates/benilla-ui/src/script/compat/classic_api/frame.rs +++ b/crates/benilla-ui/src/script/compat/classic_api/frame.rs @@ -831,13 +831,38 @@ mod tests { s.run("box:SetEnabled(true)").unwrap(); assert_eq!(s.eval::("return box:IsMouseEnabled()").unwrap(), 1); - // A Slider answers neither the trio nor this: addons duck-type on them. + // A Slider answers this one, and nothing else of the enabled family. It used to answer + // none of it, held back because addons duck-type to tell a control from a bar -- but the + // trio is what they duck-type on, and the trio is still absent. An UNGUARDED call is what + // changed it: OpenMapClassic drives `SetEnabled` on every options row that has a + // condition (`Map/Panel.lua:1084`), and the nil method took its whole Look tab down. s.run(r#"sl = CreateFrame("Slider", "CAEnableSlider")"#) .unwrap(); + for name in ["Enable", "Disable", "IsEnabled"] { + assert_eq!( + s.eval::(&format!("return type(sl.{name})")) + .unwrap(), + "nil", + "a Slider is still not a Button: {name} must stay absent" + ); + } + // What it does is refuse the mouse, which is what disabled means for a slider; the caller + // dims the art itself, as OpenMapClassic does on the line below its call. + s.run("sl:EnableMouse(true) sl:SetEnabled(false)").unwrap(); assert_eq!( - s.eval::("return type(sl.SetEnabled)").unwrap(), - "nil", - "a Slider is not a control" + s.eval::>("return sl:IsMouseEnabled()").unwrap(), + None, + "a disabled slider takes no mouse" + ); + s.run("sl:SetEnabled(true)").unwrap(); + assert_eq!(s.eval::("return sl:IsMouseEnabled()").unwrap(), 1); + // `GetBoolOrDefault`, not Lua truthiness: a caller that writes `ok and 1 or 0` means 0 to + // be off, and 0 is truthy in Lua. + s.run("sl:SetEnabled(0)").unwrap(); + assert_eq!( + s.eval::>("return sl:IsMouseEnabled()").unwrap(), + None, + "0 is off, however truthy Lua finds it" ); } diff --git a/crates/benilla-ui/src/script/slider.rs b/crates/benilla-ui/src/script/slider.rs index 40ae6860..1dda68dd 100644 --- a/crates/benilla-ui/src/script/slider.rs +++ b/crates/benilla-ui/src/script/slider.rs @@ -172,6 +172,31 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // A 1.12 Slider has no enabled state: `Enable`/`Disable`/`IsEnabled` exist only in the Button // table (`0x879d00`: `0x77fef0`, `0x77ffd0`, `0x7800b0`), so a Slider must not answer them. + // + // `SetEnabled` is the later-era setter and is a different matter: it is **beyond 1.12** by + // construction, so adding it invents no 1.12 behaviour, and OpenMapClassic calls it on a + // Slider unguarded in every options row that has a condition + // (`Map/Panel.lua:1084`) -- which took its whole "Look" tab down with + // `attempt to call method 'SetEnabled' (a nil value)`. + // + // What it does here is stop the slider taking the mouse, which is what "disabled" means for + // one; the caller dims it itself (`f:SetAlpha(on and 1 or 0.5)` on the line below the call). + // Deviation recorded because the later-era widget also greys its art, and this renderer has + // no disabled state for a Slider to grey. + m.set( + "SetEnabled", + lua.create_function(|lua, (this, flag): (Table, Value)| { + let on = crate::script::binding_abi::bool_or_default(Some(&flag), false); + let h = frame_handle_of(lua, &this)?; + // Still a Slider check, so the method cannot be lifted onto another frame. + with_slider(lua, &this, |_| ())?; + lua.app_data_mut::() + .expect("model") + .arena + .set_mouse_enabled(h, on); + Ok(()) + })?, + )?; // `SetThumbTexture(path [, layer])` or `(r, g, b [, a])`, a region's two `SetTexture` forms, on // the thumb region, created on first use. diff --git a/crates/benilla-ui/src/script/tests/widget_surface.rs b/crates/benilla-ui/src/script/tests/widget_surface.rs index d0f50e35..232d3e72 100644 --- a/crates/benilla-ui/src/script/tests/widget_surface.rs +++ b/crates/benilla-ui/src/script/tests/widget_surface.rs @@ -635,7 +635,7 @@ const ALLOWED_BEYOND: &[(&str, &str, &str)] = &[ ), ( "SetEnabled", - "Button CheckButton EditBox LootButton", + "Button CheckButton EditBox LootButton Slider", "ClassicAPI: retail replaced 1.12's Enable/Disable pair (0x780160, 0x7801b0) with the one \ setter, and later-era addons write it unguarded on their OWN controls — \ Cinematic/Options.lua:4176 is a tick's own Refresh, and the raise there unwound \ @@ -644,8 +644,14 @@ const ALLOWED_BEYOND: &[(&str, &str, &str)] = &[ table, which CheckButton and LootButton inherit. EditBox has no pair to compose (the \ reference gives a box no enabled state at all), so there it writes the mouse and \ keyboard and drops focus — input refused, text NOT greyed, which 1.12 has no slot for. \ - A Slider still answers neither this nor the trio: addons duck-type on it to tell a \ - control from a bar (tests/slider.rs)", + A Slider answers this one too, and used not to. The reason it was held back was that \ + addons duck-type on it to tell a control from a bar -- but the trio is what they \ + duck-type on (`if widget.IsEnabled then`), that is what tests/slider.rs pins, and a \ + Slider still answers none of it. What forced the change is an UNGUARDED call: \ + OpenMapClassic drives it on every options row that has a condition \ + (Map/Panel.lua:1084), and the nil method took its whole Look tab down. On a Slider it \ + writes the mouse -- input refused, art NOT greyed, which 1.12 has no slot for; the \ + caller dims it itself on the next line", ), ( "SetShown", diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 64316be5..7aeb4da8 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -3088,3 +3088,44 @@ number — it swallows the sheet's own torn frame and decorative corner, because busy right out to its border where Orgrimmar's is mostly blank parchment. A 0.58 hull shipped as if it were a trace would be a guess wearing an artist's clothes. The feather claims nothing it cannot show. + +## A third of OpenMap's options raised on open + +"Are the buttons and options actually working?" is a different question from "do they render", +and the answer was no. Rendering them was never enough to tell: `OpenMapMenu` opens 500x500 with +104 frames and 49 placed labels, and the **Look** tab still died on click: + +``` +Map/Panel.lua:1084: attempt to call method `SetEnabled' (a nil value) +``` + +`SetEnabled` is the later-era replacement for 1.12's `Enable`/`Disable` pair and this client +already composes it — on **Button, CheckButton, EditBox and LootButton**. It was deliberately +withheld from a **Slider**, and the reason is written down in `widget_surface.rs`: addons +duck-type on the enabled family to tell a control from a bar. + +The reason doesn't survive contact with the call. What addons duck-type on is the *trio* +(`if widget.IsEnabled then`), that is what `tests/slider.rs` pins, and the trio is still absent — +`SetEnabled` is beyond 1.12 by construction, so putting it on a Slider invents no 1.12 behaviour. +What forces it is that OpenMapClassic calls it **unguarded**, on every options row that carries a +condition, so a nil method took the whole tab with it. + +On a Slider it writes the mouse: input refused, art **not** greyed, which 1.12 has no slot for — +the caller dims it itself on the line below (`f:SetAlpha(on and 1 or 0.5)`). The argument goes +through `GetBoolOrDefault` rather than Lua truthiness, because DoiteAuras writes +`SetEnabled(ok and 1 or 0)` (`Modules/DoiteEdit.lua:6246`) and **0 is truthy in Lua** — read that +way, every control the caller meant to switch off would come back on. + +Both options tabs now open and are complete, sliders and all — Deepest zoom, Icons from, Map size, +Minimap detail from, strength and opacity, each with a thumb, which is the slider-thumb fix +earlier in this file arriving somewhere visible. + +### A count that was wrong until it was checked + +`grep -c "SetEnabled("` over the addon folder answers **39, across five addons**, and that number +is useless: AdvancedTradeSkillWindow2 and SoftSpoken define *their own* `SetEnabled` — a local +function in one, a module method in the other — and SoftSpoken's slider row is fully guarded +(`if slider.SetEnabled then … elseif slider.Enable … elseif slider.EnableMouse`). The real +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.