ui: hooksecurefunc over a widget method has to read through the metatable
The table form read the field with `raw_get`, on the reasoning that a field
reached only through a metatable is not that table's to replace. A widget
carries none of its own methods — they live on the shared class table its
metatable's `__index` names (`object.rs`'s `frame_meta_for`) — so
`hooksecurefunc(frame, "SetAlpha", cb)` found nothing there and armed nothing,
silently. Measured: **0 fires across 2 `SetAlpha` calls**, while the plain-table
and global forms fired correctly.
That is not a corner of the form, it is what the form is for. 29 files in grom's
corpus hook a widget method, and the one that shows: `Cinematic/Frames.lua:459`
hooks each pfUI frame's own `SetAlpha` so that Cinematic can **follow** pfUI's
out-of-combat fade rather than fight it ("their frames have SetAlpha followed
straight away by ours, taking what it set as the new base"). With the hook
unarmed the two wrote a different alpha to the same frame on alternating frames
— which is grom's "still flashing my unitframe when cinematic is on".
The reference reads `table[name]` plainly and assigns the wrapper back, which
*shadows* the class method for that one object and leaves its siblings alone.
This now does the same: read through `__index`, write raw onto the host, which
is the same thing a plain assignment would do since frame metatables carry no
`__newindex`. A test pins both halves — the chain fires on the hooked frame, and
a sibling of the same kind is untouched, with nothing written to the class table.
Corpus roster byte-identical (87 ok / 1 fail) with all 29 files' hooks armed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -60,8 +60,25 @@ fn hooksecurefunc(lua: &Lua, args: MultiValue) -> mlua::Result<()> {
|
||||
return Ok(()); // not the documented shape; nothing to arm
|
||||
};
|
||||
let name = name.to_str()?.to_string();
|
||||
// `raw_get`, not `get`: a field reached only through a metatable is not this table's to replace.
|
||||
let Value::Function(orig) = host.raw_get::<Value>(name.as_str())? else {
|
||||
// Read **through** the metatable, write onto the host itself — which is the reference's own
|
||||
// `local orig = table[name]` followed by `table[name] = wrapper`, and the only reading of
|
||||
// this form that works on a widget. A frame carries none of its methods: they live on the
|
||||
// shared class table its metatable's `__index` names (`object.rs`'s `frame_meta_for`), so a
|
||||
// raw read of `frame.SetAlpha` finds nothing and the hook silently never arms. That is not
|
||||
// the rare case, it is the *common* one — the two-argument form exists for widgets.
|
||||
//
|
||||
// Measured 2026-11-02, before this: `hooksecurefunc(f, "SetAlpha", cb)` fired **0 times**
|
||||
// across two `SetAlpha` calls, while the plain-table and global forms fired correctly. The
|
||||
// frame-by-frame symptom was grom's unit frame strobing whenever Cinematic was on:
|
||||
// `Cinematic/Frames.lua:459` hooks each pfUI frame's own `SetAlpha` precisely so it can
|
||||
// *follow* pfUI's out-of-combat fade instead of fighting it, and with the hook unarmed the
|
||||
// two wrote a different alpha to the same frame on alternating frames. 29 files in the
|
||||
// corpus use this form, openmap's canvas, pin and data-provider hooks among them.
|
||||
//
|
||||
// The write stays raw: it shadows the class method for this one object, which is what the
|
||||
// reference does and what makes a per-frame hook possible at all. Frame metatables carry no
|
||||
// `__newindex`, so raw and plain assignment agree here anyway.
|
||||
let Value::Function(orig) = host.get::<Value>(name.as_str())? else {
|
||||
return Ok(());
|
||||
};
|
||||
host.raw_set(name, chained(lua, orig, hook)?)
|
||||
@@ -104,6 +121,51 @@ mod tests {
|
||||
assert_eq!(s.eval::<String>("return seen").unwrap(), "2/3");
|
||||
}
|
||||
|
||||
/// The two-argument form over a **widget method**, which is what that form is for: a frame
|
||||
/// carries none of its own methods, so the original has to be read through the metatable and
|
||||
/// the wrapper written onto the frame, shadowing the class method for this one object.
|
||||
///
|
||||
/// Read raw instead, the hook silently never armed — measured as 0 fires for 2 calls. The
|
||||
/// symptom was a strobing unit frame: `Cinematic/Frames.lua:459` hooks each pfUI frame's
|
||||
/// `SetAlpha` so it can follow pfUI's own fade, and an unarmed hook left the two writing a
|
||||
/// different alpha to the same frame on alternating frames.
|
||||
#[test]
|
||||
fn the_two_argument_form_arms_on_a_widget_method_and_leaves_its_siblings_alone() {
|
||||
let s = UiScript::new().unwrap();
|
||||
s.run(
|
||||
r#"
|
||||
f = CreateFrame("Frame", "HookedFrame", UIParent)
|
||||
other = CreateFrame("Frame", "UnhookedFrame", UIParent)
|
||||
seen, others = 0, 0
|
||||
hooksecurefunc(f, "SetAlpha", function(self, a) seen = seen + 1 end)
|
||||
hooksecurefunc(f, "SetAlpha", function(self, a) seen = seen + 10 end)
|
||||
f:SetAlpha(0.5)
|
||||
other:SetAlpha(0.5)
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
s.eval::<i64>("return seen").unwrap(),
|
||||
11,
|
||||
"both hooks run, chained, on one call"
|
||||
);
|
||||
assert_eq!(
|
||||
s.eval::<f64>("return f:GetAlpha()").unwrap(),
|
||||
0.5,
|
||||
"and the original still did its work"
|
||||
);
|
||||
// The shadow is the frame's own: a sibling of the same kind still calls the class method.
|
||||
assert_eq!(s.eval::<i64>("return others").unwrap(), 0);
|
||||
assert_eq!(s.eval::<f64>("return other:GetAlpha()").unwrap(), 0.5);
|
||||
assert_eq!(
|
||||
s.eval::<String>("return type(rawget(other, 'SetAlpha'))")
|
||||
.unwrap(),
|
||||
"nil",
|
||||
"hooking one frame must not write onto the shared class table"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hooking_twice_chains_in_install_order_rather_than_replacing() {
|
||||
let s = UiScript::new().unwrap();
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user