Remove two silently-shadowed code paths

Both were invisible: no error, no warning, just logic that never ran.

Console.lua no longer defines the global RunMacro. Core.lua installs a hook of
the same name and loads first, so this file's name-only wrapper replaced it. What
that cost, whenever SuperMacro was absent: macro *index* arguments stopped
working (Blizzard's RunMacro accepts an index or a name, and ExecuteMacroByName
takes only a name), the delegation to SuperMacro went away, so did the fallback
to the saved Blizzard original when a macro would not resolve, and so did
clearing the stop/skip flags at the start of a top-level run -- which is what
lets /stopmacro, /skipmacro and /firstaction work across parent/child macro
boundaries. With SuperMacro loaded, Compatibility/SuperMacro.lua reassigns the
global afterwards anyway, so this definition only ever did harm.
SlashCmdList.RUNMACRO keeps using ExecuteMacroByName, which is correct there.

Utility.lua drops ClearReactiveProcOnCast and the wrappers it existed for, which
tried to clear a reactive proc when its spell was cast. Three independent bugs
meant it never worked: the wrappers guarded on CleveRoids.Frame, which Core.lua
creates after this file loads, so the guard was always false and neither wrapper
installed; the UNIT_CASTEVENT wrapper tested arg2 == "START" when that signature
is (caster, target, action, spell_id, cast_time), so it compared the target; and
both passed a spell ID to a lookup keyed by spell name. They also allocated an
arg table per call and unpack()'d it. Deleting is behaviour-preserving.

Swept for both patterns: no other duplicate global definitions, and no remaining
file-scope use of CleveRoids.Frame in a file that loads before Core.lua.
This commit is contained in:
Brues
2026-09-10 14:40:16 -05:00
parent f296425c94
commit 828acc0c4b
2 changed files with 23 additions and 54 deletions
+9 -8
View File
@@ -358,14 +358,15 @@ SlashCmdList.RUNMACRO = function(msg)
return CleveRoids.ExecuteMacroByName(CleveRoids.Trim(msg))
end
-- Global RunMacro wrapper for user convenience (delegates to namespaced internal function)
-- This pattern ensures internal logic uses CleveRoids.ExecuteMacroByName and won't break
-- if another addon overwrites the global RunMacro
-- NOTE: When SuperMacro is also loaded, Compatibility/SuperMacro.lua redirects this to
-- SuperMacro_RunMacro so macros go through RunLine (where CRM commands are intercepted)
function RunMacro(name)
return CleveRoids.ExecuteMacroByName(name)
end
-- The global RunMacro is installed by Core.lua, which loads before this file. Do not
-- redefine it here: Core's hook accepts a macro index as well as a name (Blizzard's
-- RunMacro takes either), delegates to SuperMacro when that is driving execution,
-- falls back to the saved Blizzard original when a macro will not resolve, and clears
-- the stop/skip flags at the start of a top-level run, which is what lets /stopmacro,
-- /skipmacro and /firstaction work across parent/child macro boundaries. A plain
-- name-only wrapper here silently replaced all of that.
-- When SuperMacro is loaded, Compatibility/SuperMacro.lua redirects the global to
-- SuperMacro_RunMacro so macros go through RunLine (where CRM commands are intercepted).
SLASH_RETARGET1 = "/retarget"
SlashCmdList.RETARGET = function(msg)
+14 -46
View File
@@ -9246,52 +9246,20 @@ function CleveRoids.ParseReactiveCombatLog(lowerMsg)
end
end
-- Clear reactive proc when spell is cast
function CleveRoids.ClearReactiveProcOnCast(spellName)
if not spellName then return end
-- Check if this is a reactive spell
if CleveRoids.reactiveSpells and CleveRoids.reactiveSpells[spellName] then
CleveRoids.ClearReactiveProc(spellName)
CleveRoids.QueueActionUpdate()
end
end
-- Hook UNIT_CASTEVENT to clear reactive procs
local originalUnitCastEvent = CleveRoids.Frame and CleveRoids.Frame.UNIT_CASTEVENT
if originalUnitCastEvent then
CleveRoids.Frame.UNIT_CASTEVENT = function(...)
-- Call original handler first
if type(originalUnitCastEvent) == "function" then
originalUnitCastEvent(unpack(arg))
end
-- Clear reactive proc and resist state on spell cast start
if arg1 == "player" and arg2 == "START" and arg4 then
CleveRoids.ClearReactiveProcOnCast(arg4)
CleveRoids.ClearResistState()
end
end
end
-- Hook SPELL_START_SELF to clear reactive procs (Nampower fallback when SuperWoW not available)
local originalSpellStartSelf = CleveRoids.Frame and CleveRoids.Frame.SPELL_START_SELF
if originalSpellStartSelf and not CleveRoids.hasSuperwow then
CleveRoids.Frame.SPELL_START_SELF = function(...)
-- Call original handler first
if type(originalSpellStartSelf) == "function" then
originalSpellStartSelf(unpack(arg))
end
-- Clear reactive proc and resist state on spell cast start
-- SPELL_START_SELF args: casterGuid, targetGuid, spellId, ...
local spellId = arg[3]
if spellId then
CleveRoids.ClearReactiveProcOnCast(spellId)
CleveRoids.ClearResistState()
end
end
end
-- Removed: ClearReactiveProcOnCast plus wrappers around Frame.UNIT_CASTEVENT and
-- Frame.SPELL_START_SELF that tried to clear a reactive proc when its spell was cast.
-- The feature never worked, for three independent reasons:
-- 1. Both wrappers guarded on `CleveRoids.Frame and CleveRoids.Frame.<handler>`,
-- but CleveRoids.Frame is created in Core.lua, which loads AFTER this file, so
-- the guard was always false and neither wrapper was ever installed.
-- 2. The UNIT_CASTEVENT wrapper tested arg2 == "START", but that signature is
-- (caster, target, action, spell_id, cast_time) -- the action is arg3, so it was
-- comparing the target.
-- 3. Both passed a spell *ID* to a function that looks the name up in
-- CleveRoids.reactiveSpells, which is keyed by name, so nothing could match.
-- They also each allocated an `arg` table per call and unpack()'d it. Deleting is
-- behaviour-preserving; if clearing a reactive proc on cast is still wanted it needs
-- to be written fresh against the real handler signatures.
-- NAMPOWER v2.24+ AUTO_ATTACK EVENT HANDLER FOR REACTIVE ABILITIES
-- Uses native events for dodge/parry/block detection when available.