ui: the override-binding family exists and says what it does not do
`ClearOverrideBindings` was absent outright, so `Cinematic/Camera.lua:470` raised in grom's chat on every camera shot that ended — and a raise mid-handler costs the rest of the handler, not just the line. These are 2.0's secure-binding layer: a second, unsaved binding table a frame owns, outranking the saved one while that frame wants it and dropped whole by `ClearOverrideBindings(owner)`. This client models the saved table only (`script/keybind.rs`), so: - the five setters are no-ops that **warn once each** through the script warning channel. Silence was the alternative and it is the worse one — a setter that quietly binds nothing is the same class of fault as a hook that quietly arms nothing, and the caller cannot find out. - `ClearOverrideBindings` is a plain no-op and does not warn. Dropping a table that was never filled *is* nothing, and warning there would fire on every `ClearRecast()` while the player does nothing unusual. The module header lists what a real layer needs — an override table keyed by owner in `Model`, a generation bump for `bindings.rs`'s `sync_dispatch`, an override arm in `BindingDispatch::map` that wins over the saved chord, and a `Bound::Click` — together with the `SecureHandler*` half in `secure.rs`, which is the same table seen from inside a snippet and no-op for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -72,6 +72,7 @@ mod uimap;
|
||||
// `pub(in …compat)` for its `plate_frame`, which SuperWoW's `UnitNameplate` shares.
|
||||
pub(in crate::script::compat) mod nameplate;
|
||||
mod new_items;
|
||||
mod override_bindings;
|
||||
mod pixel_util;
|
||||
mod player_cache;
|
||||
mod player_info;
|
||||
@@ -137,6 +138,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> {
|
||||
cvar::install_globals(lua)?;
|
||||
g.set("C_NamePlate", nameplate::install(lua)?)?;
|
||||
g.set("C_NewItems", new_items::install(lua)?)?;
|
||||
override_bindings::install(lua)?;
|
||||
g.set("C_Macro", macro_ext::install(lua)?)?;
|
||||
g.set("C_Item", item::install(lua)?)?;
|
||||
g.set("C_AddOns", addons::install(lua)?)?;
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
//! The override-binding globals: `SetOverrideBinding` and its four typed siblings, and
|
||||
//! `ClearOverrideBindings`.
|
||||
//!
|
||||
//! These are 2.0's secure-binding layer, not 1.12's. The reference binding table this client does
|
||||
//! model (`script/keybind.rs`) is the *saved* one — one command per chord, persisted under
|
||||
//! `benilla-config/bindings/` and repainted onto the action bar through `UPDATE_BINDINGS`. An
|
||||
//! override is the other thing entirely: a second, unsaved table a frame owns, which outranks the
|
||||
//! saved one while that frame wants it and is dropped whole by `ClearOverrideBindings(owner)`.
|
||||
//! Nothing here implements that second table, so:
|
||||
//!
|
||||
//! - **The five setters are no-ops that say so**, once each, through the script warning channel.
|
||||
//! Silence was the alternative and it is the worse one: a setter that quietly binds nothing is
|
||||
//! the same class of fault as a hook that quietly arms nothing, and the caller has no way to
|
||||
//! find out. `Cinematic/Camera.lua:461` is the shipped caller — hold a key during a camera shot
|
||||
//! and it should recast — so the warning names a feature that is off, not a mystery.
|
||||
//! - **`ClearOverrideBindings(owner)` is a plain no-op and does not warn.** Dropping a table that
|
||||
//! was never filled *is* nothing; warning there would fire on every `ClearRecast()` the addon
|
||||
//! makes while the player is doing nothing unusual. Before this it was absent outright, and the
|
||||
//! raise it threw (`attempt to call global 'ClearOverrideBindings' (a nil value)`) was the one
|
||||
//! line of this family that reached grom's chat.
|
||||
//!
|
||||
//! What a real layer needs, so the next pass starts from the measurement rather than the idea: an
|
||||
//! override table keyed by owner in `Model`, a generation bump so `bindings.rs`'s `sync_dispatch`
|
||||
//! rebuilds, an override arm in `BindingDispatch::map` that wins over the saved chord, and a
|
||||
//! `Bound::Click` that calls a named button's `Click` — plus the `SecureHandler*` half
|
||||
//! (`compat/classic_api/secure.rs`'s `SetBindingClick` and friends), which is the same table seen
|
||||
//! from inside a snippet and is no-op for exactly the same reason.
|
||||
|
||||
use mlua::{Lua, MultiValue, Value};
|
||||
|
||||
use crate::script::Model;
|
||||
|
||||
/// A setter that does nothing and records why, the first time it is called.
|
||||
///
|
||||
/// Per-function `Cell`, not one shared set: five calls to five different setters are five
|
||||
/// different missing features and each deserves its own line, while five calls to the same one
|
||||
/// are one.
|
||||
fn warned_noop(lua: &Lua, name: &'static str) -> mlua::Result<mlua::Function> {
|
||||
let fired = std::cell::Cell::new(false);
|
||||
lua.create_function(move |lua, _: MultiValue| {
|
||||
if !fired.replace(true) {
|
||||
if let Some(mut model) = lua.app_data_mut::<Model>() {
|
||||
model.record_warning(format!(
|
||||
"{name}: this client has no override-binding layer, so the key is left bound \
|
||||
as it was — the call is ignored (docs/ADDON_COMPAT.md)"
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(Value::Nil)
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
|
||||
let g = lua.globals();
|
||||
for name in [
|
||||
"SetOverrideBinding",
|
||||
"SetOverrideBindingClick",
|
||||
"SetOverrideBindingSpell",
|
||||
"SetOverrideBindingItem",
|
||||
"SetOverrideBindingMacro",
|
||||
] {
|
||||
g.set(name, warned_noop(lua, name)?)?;
|
||||
}
|
||||
// No warning: see the module header.
|
||||
g.set(
|
||||
"ClearOverrideBindings",
|
||||
lua.create_function(|_, _: MultiValue| Ok(Value::Nil))?,
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::script::UiScript;
|
||||
|
||||
#[test]
|
||||
fn a_setter_warns_once_and_the_clear_is_quiet() {
|
||||
let mut s = UiScript::new().unwrap();
|
||||
s.run(r#"SetOverrideBindingClick(UIParent, true, "T", "SomeButton")"#)
|
||||
.unwrap();
|
||||
s.run(r#"SetOverrideBindingClick(UIParent, true, "Y", "SomeButton")"#)
|
||||
.unwrap();
|
||||
s.run(r#"ClearOverrideBindings(UIParent)"#).unwrap();
|
||||
s.run(r#"ClearOverrideBindings(UIParent)"#).unwrap();
|
||||
let warns = s.take_warnings();
|
||||
assert_eq!(warns.len(), 1, "warn-once per setter: {warns:?}");
|
||||
assert!(
|
||||
warns[0].contains("SetOverrideBindingClick"),
|
||||
"the warning names the verb: {warns:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn each_setter_gets_its_own_line() {
|
||||
let mut s = UiScript::new().unwrap();
|
||||
s.run(r#"SetOverrideBinding(UIParent, true, "T", "JUMP")"#)
|
||||
.unwrap();
|
||||
s.run(r#"SetOverrideBindingSpell(UIParent, true, "Y", "Fireball")"#)
|
||||
.unwrap();
|
||||
assert_eq!(s.take_warnings().len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_clear_the_camera_calls_no_longer_raises() {
|
||||
let mut s = UiScript::new().unwrap();
|
||||
// `Cinematic/Camera.lua:470`, which raised `attempt to call global
|
||||
// `ClearOverrideBindings' (a nil value)` on every camera shot that ended.
|
||||
s.run(r#"ClearOverrideBindings(CreateFrame("Button"))"#)
|
||||
.unwrap();
|
||||
assert!(s.take_warnings().is_empty());
|
||||
}
|
||||
}
|
||||
@@ -393,6 +393,20 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> {
|
||||
// modern colour object; 1.12 passes colours as loose r,g,b numbers.
|
||||
"ColorMixin",
|
||||
"CreateColor",
|
||||
// 2.0's override-binding layer (script/compat/classic_api/override_bindings.rs): a
|
||||
// second, unsaved binding table a frame owns, outranking the saved one while it wants it.
|
||||
// This client models the saved table only, so the five setters are no-ops that WARN once
|
||||
// each and the clear is a silent one — dropping a table that was never filled is nothing.
|
||||
// They are here rather than absent because the absence raised: `ClearOverrideBindings` is
|
||||
// `Cinematic/Camera.lua:470`, on every camera shot that ended, and a raise mid-handler
|
||||
// costs the rest of the handler. A feature test on these learns "present", which is wrong
|
||||
// and is why the setters announce themselves in the log instead of binding silently.
|
||||
"ClearOverrideBindings",
|
||||
"SetOverrideBinding",
|
||||
"SetOverrideBindingClick",
|
||||
"SetOverrideBindingItem",
|
||||
"SetOverrideBindingMacro",
|
||||
"SetOverrideBindingSpell",
|
||||
// ClassicAPI's tInvert (script/compat/classic_api/lua_ext.rs): value->key table flip, a
|
||||
// Blizzard utility 1.12 has no counterpart for.
|
||||
"tInvert",
|
||||
|
||||
Reference in New Issue
Block a user