fix(input): scope GetMouseButtonClicked to the click dispatch

It read a button captured from the WH_GETMESSAGE mouse-message hook and
evicted two world ticks later, so it reported a button for as long as one
was held, and for a couple of frames after every click -- useless as the
"am I inside a click handler" test it exists to be.

3.3.5 does none of that. The getter (FUN_0050f950) pushes a frame-manager
field that only the four click dispatches ever write -- button OnClick
FUN_0048fc30, OnDoubleClick FUN_0048fce0, and the two secure click paths
FUN_0096fd70/FUN_0096fdd0 that wrap PreClick -> OnClick -> PostClick --
each saving the old value on the C stack, setting its own button argument,
running the handler, then restoring. That C-stack save is what makes a
nested click shadow the outer one and restore on unwind.

1.12 has no such field but the same bracket point: FUN_00779540 (OnClick)
and FUN_00779650 (OnDoubleClick) fire their handler through the runner
Frame::ClickEvents already intercepts, passing the button name as the
fire's only "%s" argument. Mirror the engine's bracket there and read the
name out of the fire, which also inherits the engine's own naming:
Button:Click (FUN_007826C0) folds its optional string down to a numeric
mask, so a bare Click() arrives as "LeftButton" and an unrecognized name
as "UNKNOWN".

GetMouseButtonClicked moves to Frame::ClickEvents with the dispatch it
describes. IsMouseButtonDown and GLOBAL_MOUSE_* keep the message hook,
which is the right source for a device query. The recursion guard now
covers only the PreClick/PostClick pair, so a nested click still gets its
own button scope.

It now returns nil in OnMouseDown / OnMouseUp / OnDragStart, matching
3.3.5, where those paths never touch the field.

Verified in-game across all four dispatches: real and programmatic clicks,
OnDoubleClick, PreClick/PostClick, the nested inner/outer restore, Click()
argument folding, and an OnUpdate watcher that never saw a value outside a
handler.

docs/API.md

@@ -7915,27 +7915,28 @@ end
 
 ### `GetMouseButtonClicked()`
 
-Returns the name of the button responsible for the mouse handler
-currently running (`"LeftButton"`, `"RightButton"`, `"MiddleButton"`,
-`"Button4"`, `"Button5"`), or `nil` when no click is being handled.
+Returns the name of the mouse button that started the `OnClick` or
+`OnDoubleClick` handler that is now running. At any other time it
+returns `nil`.
 
-Addons call this inside a mouse handler (`OnClick`,
-`OnMouseDown`, `OnMouseUp`, `OnDragStart`, …) to learn which button
-drove it — an alternative to reading `arg1`, and readable
-from nested helper functions where `arg1` isn't in scope. The value
-is captured by the same `WH_GETMESSAGE` hook behind
-`GLOBAL_MOUSE_DOWN` / `GLOBAL_MOUSE_UP`, then evicted a couple of
-frames later so — matching the real API — it reads `nil` outside a
-handler rather than lingering as the last click forever.
+A helper function that the handler calls can also read the name.
+`arg1` is not in scope there.
 
-The button stays readable for the whole frame its click lands in (so
-multiple addons hooking the same `OnClick` all see it), and while a
-button is physically held (so `OnDragStart` reads it). It is a
-best-effort replica: WoW dispatches these handlers deferred from the
-OS message, so eviction is time-based (per-frame) rather than exactly
-bracketed around the handler. Values can therefore
-linger ~2 frames past a click — harmless, since it's only meaningful
-inside a mouse handler.
+The name is one of `"LeftButton"`, `"MiddleButton"`, `"RightButton"`,
+`"Button4"`, or `"Button5"`. A `Button:Click()` call with no argument
+gives `"LeftButton"`. A `Button:Click(name)` call with any other name
+gives `"UNKNOWN"`.
+
+`PreClick` and `PostClick` handlers run inside the click, so they read
+the same name.
+
+The other mouse scripts are not clicks. In `OnMouseDown`, `OnMouseUp`,
+and `OnDragStart` this function returns `nil`, and `arg1` holds the
+button.
+
+The name is valid only while the handler runs. If a handler clicks a
+second button, the name of that inner click applies until the inner
+handler ends. Then the outer name comes back.
 
 ```lua
 button:SetScript("OnClick", function()

src/Offsets.h

@@ -255,10 +255,17 @@ enum Offsets {
     // `Frame::ClickEvents` co-hooks this to hand out an external per-button cell
     // for those two names — same technique as FUN_GAMETOOLTIP_SCRIPT_RESOLVER.
     FUN_BUTTON_SCRIPT_RESOLVER = 0x00778C50,
-    // Button OnClick script slot offset (the resolver's OnClick return). Used
-    // by `Frame::ClickEvents` to recognize an OnClick fire at the runner hook
-    // below: the fire passes slotPtr == button + this offset.
+    // Button OnClick / OnDoubleClick script slot offsets (the resolver's
+    // returns for those two names). Used by `Frame::ClickEvents` to recognize a
+    // click fire at the runner hook below: the fire passes slotPtr == button +
+    // one of these. Each has exactly one fire site — the click vmethods
+    // FUN_00779540 (OnClick) and FUN_00779650 (OnDoubleClick), which take a
+    // button BITMASK (1 = Left, 2 = Middle, 4 = Right, 8 = Button4,
+    // 0x10 = Button5), map it through a jump table to the engine's own name
+    // literal (anything else -> "UNKNOWN" at 0x00838044) and fire
+    // FUN_007026F0(button, button + slot, "%s", name).
     OFF_BUTTON_ONCLICK_HANDLER = 0x4CC,
+    OFF_BUTTON_ONDOUBLECLICK_HANDLER = 0x4D4,
     // Frame-script runner WITH exec-context stamping — __cdecl(void *frame,
     // int *slotPtr, const char *fmt, void *vaPtr). Saves DAT_00ceeac0, stamps
     // it from slotPtr[1] (the cell's context), then calls FUN_FRAME_RUN_SCRIPT_ARGS

src/frame/ClickEvents.cpp

@@ -46,6 +46,39 @@
 // fires them regardless; on this client that would require hooking the click
 // vmethod, which SuperWoW owns. Bracketing an existing OnClick is the common
 // case and the one this backport serves.
+//
+// `GetMouseButtonClicked()` — the button string of the innermost click dispatch
+// currently on the stack, nil outside one. It lives in this module because that
+// is precisely what it is. 3.3.5 keeps it in a frame-manager field (the getter
+// FUN_0050f950 pushes `[frameMgr + 0x1234]`, nil when null), and the ONLY code
+// that ever writes that field is the four click dispatches — button OnClick
+// FUN_0048fc30, OnDoubleClick FUN_0048fce0, and the two secure click paths
+// FUN_0096fd70 / FUN_0096fdd0 that wrap PreClick -> OnClick -> PostClick — each
+// with the same bracket around the handler:
+//
+//     saved = mgr->clickedButton;
+//     mgr->clickedButton = buttonName;   // this dispatch's own argument
+//     ... run the handler ...
+//     mgr->clickedButton = saved;
+//
+// Saving on the C stack is what makes a nested click shadow the outer one and
+// restore on unwind, so the innermost dispatch wins. Nothing captures the OS
+// mouse message and nothing evicts on a timer: outside a click dispatch the
+// answer is nil, and it is nil in OnMouseDown / OnMouseUp / OnDragStart too.
+//
+// 1.12 has no such field, but it has the same bracket point. Both click
+// dispatches (FUN_00779540 OnClick, FUN_00779650 OnDoubleClick) fire their
+// handler through the runner this module already intercepts, passing the button
+// name as the fire's only "%s" argument. So we mirror the engine's bracket
+// around that fire and read the name straight out of it — which also inherits
+// the engine's own naming for free: the vmethods take a button BITMASK, and
+// Button:Click (FUN_007826C0) folds its optional string argument down to one, so
+// a bare `Click()` arrives as "LeftButton" and an unrecognized name as
+// "UNKNOWN". (An earlier version captured the button from the WH_GETMESSAGE
+// hook behind GLOBAL_MOUSE_DOWN/UP and evicted it a couple of world ticks later
+// — which reported a button for as long as one was held, and for ~2 frames after
+// every click, making the value useless as the "am I in a click handler" test it
+// exists to be.)
 
 #include "frame/ClickEvents.h"
 
@@ -108,8 +141,59 @@ bool EqualsIgnoreCase(const char *s, const char *literal) {
 // click a button again. While bracketing, further OnClick fires pass straight
 // through (the nested click still runs — it just doesn't get its own nested
 // PreClick/PostClick), which prevents runaway from a self-clicking handler.
+// It guards ONLY the Pre/PostClick pair — a nested click still gets its own
+// clicked-button scope, since that one must track the innermost dispatch.
 int g_firing = 0;
 
+// --- clicked button (GetMouseButtonClicked) --------------------------------
+// The innermost click dispatch's button string, null outside one — and
+// Game::Lua::PushString tail-jumps to pushnil on null, so that surfaces as nil
+// with no extra branch. It always points at one of the engine's own static name
+// literals ("LeftButton" … "UNKNOWN"), which live for the process, so holding
+// the pointer across the dispatch — and across a /reload a click handler
+// triggers — can never dangle.
+const char *g_clickedButton = nullptr;
+
+// The fire's first "%s" argument — the button name. A click fire is always
+// exactly ("%s", name); any other shape isn't a name, so it reads null.
+const char *FirstStringArg(const char *fmt, const void *varargs) {
+    if (fmt == nullptr || varargs == nullptr)
+        return nullptr;
+    const char *p = fmt;
+    while (*p != '\0' && *p != '%')
+        ++p;
+    if (p[0] != '%' || p[1] != 's')
+        return nullptr;
+    return *reinterpret_cast<const char *const *>(varargs);
+}
+
+enum ClickKind { CK_NONE, CK_CLICK, CK_DOUBLECLICK };
+
+// Which click dispatch this fire is, by slot address. Exact: no other fire
+// passes a button's OnClick / OnDoubleClick slot.
+ClickKind ClickKindOf(void *frame, const uint32_t *slotPtr) {
+    const char *base = reinterpret_cast<const char *>(frame);
+    if (slotPtr == reinterpret_cast<const uint32_t *>(
+                       base + Offsets::OFF_BUTTON_ONCLICK_HANDLER))
+        return CK_CLICK;
+    if (slotPtr == reinterpret_cast<const uint32_t *>(
+                       base + Offsets::OFF_BUTTON_ONDOUBLECLICK_HANDLER))
+        return CK_DOUBLECLICK;
+    return CK_NONE;
+}
+
+int __fastcall Script_GetMouseButtonClicked(void *L) {
+    Game::Lua::PushString(L, g_clickedButton);
+    return 1;
+}
+
+void RegisterLuaFunctions() {
+    Game::Lua::RegisterGlobalFunction("GetMouseButtonClicked",
+                                      &Script_GetMouseButtonClicked);
+}
+
+const Game::ModuleAutoRegister _autoreg{&RegisterLuaFunctions};
+
 // --- Resolver co-hook (Button script-name -> slot) ---------------------
 using Resolver_t = int(__fastcall *)(void *self, void *edx, const char *name);
 Resolver_t g_resolverOriginal = nullptr;
@@ -144,19 +228,30 @@ void FireCell(void *frame, int kind, const char *fmt, void *varargs) {
         Game::Lua::SetTop(L, savedTop);
 }
 
-// Falls through (false) for every fire that isn't a top-level OnClick;
-// otherwise runs the whole bracket itself and reports it handled (true).
+// Falls through (false) for every fire that isn't a click dispatch; otherwise
+// runs the dispatch itself, inside the engine's clicked-button bracket, and
+// reports it handled (true). A top-level OnClick additionally gets its
+// PreClick/PostClick pair, which run inside the same bracket so they read the
+// button too — exactly like the secure click path they mirror.
 bool OnRun(void *frame, uint32_t *slotPtr, const char *fmt, void *varargs) {
-    const auto onClickSlot = reinterpret_cast<uint32_t *>(
-        reinterpret_cast<char *>(frame) + Offsets::OFF_BUTTON_ONCLICK_HANDLER);
-    if (slotPtr != onClickSlot || g_firing > 0)
+    const ClickKind kind = ClickKindOf(frame, slotPtr);
+    if (kind == CK_NONE)
         return false;
 
-    ++g_firing;
-    FireCell(frame, SK_PRECLICK, fmt, varargs);
-    Frame::RunnerHook::Original(frame, slotPtr, fmt, varargs); // the button's OnClick
-    FireCell(frame, SK_POSTCLICK, fmt, varargs);
-    --g_firing;
+    const char *saved = g_clickedButton;
+    g_clickedButton = FirstStringArg(fmt, varargs);
+
+    if (kind == CK_CLICK && g_firing == 0) {
+        ++g_firing;
+        FireCell(frame, SK_PRECLICK, fmt, varargs);
+        Frame::RunnerHook::Original(frame, slotPtr, fmt, varargs); // the OnClick
+        FireCell(frame, SK_POSTCLICK, fmt, varargs);
+        --g_firing;
+    } else {
+        Frame::RunnerHook::Original(frame, slotPtr, fmt, varargs);
+    }
+
+    g_clickedButton = saved;
     return true;
 }
 

src/input/GlobalMouse.cpp

@@ -24,25 +24,10 @@
 // as the events (single source of truth — events firing and the
 // state being readable happen at the same point in the pipeline).
 //
-// `GetMouseButtonClicked()` — the button name of the most recent
-// mouse button message (`"LeftButton"` / … / `"Button5"`), or nil
-// when no click is being handled. Modern addons call it inside a
-// mouse handler to learn which button drove it; the value is evicted
-// shortly after, so — like the real API — it reads nil outside a
-// handler rather than returning a stale last-click forever.
-//
-// Modern WoW backs this with a frame-manager field set right before a
-// mouse handler runs and cleared right after (verified in 3.3.5:
-// `GetMouseButtonClicked` pushes `[frameMgr + 0x1234]`, nil when that
-// field is null). We can't bracket the handler the same way here: the
-// `WH_GETMESSAGE` hook sees the button at message-dequeue, but WoW
-// dispatches the frame-script mouse handlers *deferred* (during the
-// frame update, not the message pump). So instead we capture the
-// button on the message and evict it from the per-frame `WorldTick`
-// once a couple of quiet ticks pass with no button held — the value
-// survives the whole frame its click lands in (so every handler,
-// including multiple addons hooking one `OnClick`, sees it) then
-// clears. A held button keeps it set so `OnDragStart` still reads it.
+// `GetMouseButtonClicked()` is NOT here — it is not a mouse-device
+// query at all. The engine scopes it to a click DISPATCH, bracketing
+// the value around the handler rather than tracking the device, so it
+// lives with that dispatch in [Frame::ClickEvents](../frame/ClickEvents.cpp).
 //
 // Same `WH_GETMESSAGE` thread-message hook pattern as
 // [Input::Modifier](Modifier.cpp): we intercept the WM_*BUTTON*
@@ -53,7 +38,6 @@
 
 #include "Game.h"
 #include "event/Custom.h"
-#include "tick/WorldTick.h"
 
 #include <cstdint>
 #include <cstring>
@@ -80,24 +64,6 @@ constexpr uint32_t MASK_ANY_BUTTON = 0x1F;
 
 uint32_t g_mouseButtonMask = 0;
 
-// Name of the button from the most recent mouse-button message, or
-// nullptr when no click is being handled. Points at one of
-// DecodeButton's string literals (stable for the process lifetime),
-// so storing the pointer is safe. `lua_pushstring(L, nullptr)`
-// tail-jumps to pushnil, so a null here surfaces as nil to the Lua
-// caller with no extra branch. Set on each button message; evicted by
-// EvictTick (see the header note on GetMouseButtonClicked).
-const char *g_lastButton = nullptr;
-
-// Set true whenever g_lastButton is (re)assigned, so EvictTick can
-// tell a just-clicked frame from a quiet one. Number of consecutive
-// quiet ticks (no activity, nothing held) required before eviction —
-// >1 so a handler dispatched a frame after its message still reads
-// the button.
-bool g_buttonActivity = false;
-int g_quietTicks = 0;
-constexpr int kQuietTicksToEvict = 2;
-
 HHOOK g_msgHook = nullptr;
 
 // Maps WM_*BUTTON{DOWN,UP} → ("LeftButton"|..., isDown). Returns
@@ -140,13 +106,6 @@ void ProcessMouseMessage(UINT msg, WPARAM wParam) {
     if (name == nullptr)
         return;
 
-    // Capture for GetMouseButtonClicked on both press and release —
-    // during an OnClick / OnMouseUp handler the "clicked" button is the
-    // one just released, same as the press for a normal click.
-    g_lastButton = name;
-    g_buttonActivity = true;
-    g_quietTicks = 0;
-
     const int bitIdx = NameToBitIdx(name);
     if (bitIdx >= 0) {
         const uint32_t bit = 1u << bitIdx;
@@ -182,39 +141,6 @@ int __fastcall Script_IsMouseButtonDown(void *L) {
     return 1;
 }
 
-int __fastcall Script_GetMouseButtonClicked(void *L) {
-    // Null → pushnil (the engine's pushstring tail-jumps on NULL), so
-    // callers outside a mouse handler see nil, matching the modern API.
-    Game::Lua::PushString(L, g_lastButton);
-    return 1;
-}
-
-// Per-frame eviction of g_lastButton. Runs at the tail of each frame's
-// world tick. While a button is held we keep the value (drag handlers
-// still read it) and reset the quiet counter. A frame that saw a fresh
-// button message consumes its activity flag but doesn't yet evict —
-// that one-frame grace lets a handler dispatched the frame after its
-// message still read the button. Only after kQuietTicksToEvict ticks
-// with nothing held and no activity do we clear, so outside a click
-// GetMouseButtonClicked reads nil instead of a stale last-click.
-void EvictTick() {
-    if (g_lastButton == nullptr)
-        return;
-    if (g_mouseButtonMask != 0) {
-        g_quietTicks = 0;
-        return;
-    }
-    if (g_buttonActivity) {
-        g_buttonActivity = false;
-        g_quietTicks = 0;
-        return;
-    }
-    if (++g_quietTicks >= kQuietTicksToEvict) {
-        g_lastButton = nullptr;
-        g_quietTicks = 0;
-    }
-}
-
 LRESULT CALLBACK GetMsgHook(int code, WPARAM wParam, LPARAM lParam) {
     // Only process when the message is being removed from the queue
     // (`PM_REMOVE` / `GetMessage`); `PM_NOREMOVE` peeks would deliver
@@ -236,12 +162,9 @@ void InstallHook() {
 void RegisterLuaFunctions() {
     Game::Lua::RegisterGlobalFunction("IsMouseButtonDown",
                                       &Script_IsMouseButtonDown);
-    Game::Lua::RegisterGlobalFunction("GetMouseButtonClicked",
-                                      &Script_GetMouseButtonClicked);
     InstallHook();
 }
 
-const Tick::WorldTick::AutoSubscribe _evict{&EvictTick};
 const Game::ModuleAutoRegister _autoreg{&RegisterLuaFunctions};
 
 } // namespace