fix(util): preserve nil holes in vararg forwarding; drop no-op unpack ranges

On this Lua 5.0 build, unpack() ignores its explicit i/j arguments entirely
and takes its element count from getn() -- which returns the table's `.n`
field, or border-searches to the first nil when `.n` is absent. Verified in
the binary (luaB_unpack 0x007030a0 -> luaL_getn 0x006f5050) and in-game
(unpack({1, 1, nil, 1}, 1, 4) yields just 1, 1).

- GenerateClosure built its forwarding table `rest` with no `.n`, so
  unpack(rest, 1, restN) border-searched and silently dropped every value
  past the first nil hole in the payload -- despite the comment claiming
  embedded nils survive. It now stamps `all.n` and unpack(all) forwards
  holes intact.

- Every other unpack(x, 1, x.n) / unpack(x, 1, count==x.n) call was a no-op:
  the table already carried `.n`, so unpack(x) is identical. Simplified
  across FunctionUtil, Pools, CallbackRegistry, and TableUtil's SafeUnpack,
  and corrected the header comments that recommended the redundant form.

AddOns/!!!ClassicAPI/Util/AddOnCompat.lua

@@ -43,3 +43,18 @@ end
 if C_AddOns.DoesAddOnExist("SpecialTalentUI") then
     EventUtil.ContinueOnAddOnLoaded("SpecialTalentUI", CAPI_ApplyStandardColorGlobals)
 end
+
+-- ShaguTweaks libpredict register's TBC events that ClassicAPI backports.
+if C_AddOns.DoesAddOnExist("ShaguTweaks") then
+    EventUtil.ContinueOnAddOnLoaded("ShaguTweaks", function()
+        local libp = ShaguTweaks.libpredict
+        if libp then
+            libp.sender:UnregisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
+            libp.sender:UnregisterEvent("UNIT_SPELLCAST_START")
+            libp.sender:UnregisterEvent("UNIT_SPELLCAST_STOP")
+            libp.sender:UnregisterEvent("UNIT_SPELLCAST_FAILED")
+            libp.sender:UnregisterEvent("UNIT_SPELLCAST_INTERRUPTED")
+            libp.sender:UnregisterEvent("UNIT_SPELLCAST_SENT")
+        end
+    end)
+end

AddOns/!!!ClassicAPI/Util/CallbackRegistry.lua

@@ -26,10 +26,17 @@ local function GenerateClosure(func, ...)
     for i = 1, boundN do bound[i] = arg[i] end
     return function(...)
         local callN = arg.n
-        local merged = {}
-        for i = 1, boundN do merged[i] = bound[i] end
-        for i = 1, callN do merged[boundN + i] = arg[i] end
-        return func(unpack(merged, 1, boundN + callN))
+        -- Forward bound + call args faithfully, embedded/leading nils and all.
+        -- The ONLY thing that makes unpack span holes on this Lua 5.0 build is
+        -- a `.n` field: unpack takes its count from getn (which returns t.n)
+        -- and ignores any explicit i/j, so a `.n`-less table stops dead at the
+        -- first hole (verified: unpack({1, 1, nil, 1}, 1, 4) yields just 1, 1).
+        local all = {}
+        local n = 0
+        for i = 1, boundN do n = n + 1; all[n] = bound[i] end
+        for i = 1, callN do n = n + 1; all[n] = arg[i] end
+        all.n = n
+        return func(unpack(all))
     end
 end
 
@@ -145,11 +152,14 @@ function CallbackRegistryMixin:TriggerEvent(event, ...)
     if funcs then
         for owner, func in pairs(funcs) do
             -- Stock 3.3.5 passes the owner as the first arg so callers can
-            -- distinguish multiple registrations of the same function.
-            local n = arg.n
-            local merged = { owner }
-            for i = 1, n do merged[i + 1] = arg[i] end
-            func(unpack(merged, 1, n + 1))
+            -- distinguish multiple registrations of the same function. `arg`
+            -- carries its own `.n`, so unpack(arg) forwards the payload with
+            -- leading/embedded nils intact. Do NOT instead merge into a
+            -- `{ owner, ... }` table and unpack that WITHOUT setting `.n`: this
+            -- build's unpack counts via getn, so a `.n`-less table collapses at
+            -- the first hole (verified in-game: reload's PLAYER_ENTERING_WORLD,
+            -- arg1=nil arg2=1, dropped the payload).
+            func(owner, unpack(arg))
         end
     end
 end

AddOns/!!!ClassicAPI/Util/FunctionUtil.lua

@@ -2,7 +2,9 @@
 --
 -- Implementation differences from the modern source:
 --   - Lua 5.0 has no select() and no `...` as expression; use the
---     implicit `arg` table (arg.n + arg[i]) and unpack(arg, 1, arg.n).
+--     implicit `arg` table (arg.n + arg[i]). unpack(arg) already spans
+--     arg[1..arg.n] via getn — this build's unpack ignores explicit i/j
+--     args, so the count must live in the table's `.n`.
 --   - Lua 5.0 has no `#` operator; use table.getn().
 --   - 5.0's xpcall is strictly (f, handler) — no trailing args. We wrap
 --     the script call in a local closure that captures frame + arg.
@@ -32,7 +34,7 @@ function ExecuteFrameScript(frame, scriptName, ...)
 	if script then
 		local args = arg;
 		local function invoke()
-			return script(frame, unpack(args, 1, args.n));
+			return script(frame, unpack(args));
 		end
 		xpcall(invoke, CallErrorHandler);
 	end
@@ -45,7 +47,7 @@ function CallMethodOnNearestAncestor(self, methodName, ...)
 	end
 
 	if ancestor then
-		return true, ancestor[methodName](ancestor, unpack(arg, 1, arg.n));
+		return true, ancestor[methodName](ancestor, unpack(arg));
 	end
 
 	return false;
@@ -58,7 +60,7 @@ function GetValueOrCallFunction(tbl, key, ...)
 
 	local value = tbl[key];
 	if type(value) == "function" then
-		return value(unpack(arg, 1, arg.n));
+		return value(unpack(arg));
 	else
 		return value;
 	end
@@ -117,12 +119,12 @@ function IterateTables(iteratorFunction, ...)
 end
 
 local s_passThroughClosureGenerators = {
-	function(f) return function(...) return f(unpack(arg, 1, arg.n)); end; end,
-	function(f, a) return function(...) return f(a, unpack(arg, 1, arg.n)); end; end,
-	function(f, a, b) return function(...) return f(a, b, unpack(arg, 1, arg.n)); end; end,
-	function(f, a, b, c) return function(...) return f(a, b, c, unpack(arg, 1, arg.n)); end; end,
-	function(f, a, b, c, d) return function(...) return f(a, b, c, d, unpack(arg, 1, arg.n)); end; end,
-	function(f, a, b, c, d, e) return function(...) return f(a, b, c, d, e, unpack(arg, 1, arg.n)); end; end,
+	function(f) return function(...) return f(unpack(arg)); end; end,
+	function(f, a) return function(...) return f(a, unpack(arg)); end; end,
+	function(f, a, b) return function(...) return f(a, b, unpack(arg)); end; end,
+	function(f, a, b, c) return function(...) return f(a, b, c, unpack(arg)); end; end,
+	function(f, a, b, c, d) return function(...) return f(a, b, c, d, unpack(arg)); end; end,
+	function(f, a, b, c, d, e) return function(...) return f(a, b, c, d, e, unpack(arg)); end; end,
 };
 
 local s_flatClosureGenerators = {
@@ -138,7 +140,7 @@ local function GenerateClosureInternal(generatorArray, f, ...)
 	local count = arg.n;
 	local generator = generatorArray[count + 1];
 	if generator then
-		return generator(f, unpack(arg, 1, count));
+		return generator(f, unpack(arg));
 	end
 
 	assertsafe(false, "Closure generation does not support more than " .. (table.getn(generatorArray) - 1) .. " parameters");
@@ -147,14 +149,14 @@ end
 
 -- Syntactic sugar for function(...) return f(a, b, c, ...); end
 function GenerateClosure(f, ...)
-	return GenerateClosureInternal(s_passThroughClosureGenerators, f, unpack(arg, 1, arg.n));
+	return GenerateClosureInternal(s_passThroughClosureGenerators, f, unpack(arg));
 end
 
 -- Generates a closure with the specified arguments that will ignore extra arguments when called later. Useful for passing
 -- through callbacks to APIs where we don't want extra arguments to be passed through, i.e. simple OnClick scripts.
 -- This is equivalent to: function() return f(a, b, c); end INSTEAD OF function(...) return f(a, b, c, ...); end
 function GenerateFlatClosure(f, ...)
-	return GenerateClosureInternal(s_flatClosureGenerators, f, unpack(arg, 1, arg.n));
+	return GenerateClosureInternal(s_flatClosureGenerators, f, unpack(arg));
 end
 
 function RunNextFrame(callback)
@@ -166,7 +168,7 @@ FunctionUtil = {};
 function FunctionUtil.SafeInvokeMethod(table, methodName, ...)
 	local method = table[methodName];
 	if method then
-		return method(table, unpack(arg, 1, arg.n));
+		return method(table, unpack(arg));
 	end
 
 	return nil;

AddOns/!!!ClassicAPI/Util/GlobalCallbackRegistry.lua

@@ -22,6 +22,18 @@ function EventRegistry:OnLoad()
         -- Lua-5.0 varargs, so we forward through a small wrapper.
         self:DispatchGameEvent(event, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9)
     end)
+
+    -- Stop delivering events once logout begins: a callback firing during the
+    -- engine's teardown can touch state that's already gone. A dedicated frame
+    -- owns PLAYER_LOGOUT so this fires regardless of what consumers registered
+    -- — folding the check into frameEventFrame's OnEvent would depend on the
+    -- ref-counted event set still holding PLAYER_LOGOUT at logout time.
+    self.logoutFrame = CreateFrame("Frame")
+    self.logoutFrame:RegisterEvent("PLAYER_LOGOUT")
+    self.logoutFrame:SetScript("OnEvent", function()
+        self.frameEventFrame:UnregisterAllEvents()
+        self.frameEventFrame:SetScript("OnEvent", nil)
+    end)
 end
 
 -- Helper so we can forward up to nine event args (the most any vanilla

AddOns/!!!ClassicAPI/Util/ItemUtil.lua

@@ -216,8 +216,7 @@ function ItemMixin:GetItemLink() -- requires item data to be loaded
     end
 
     if self.itemID then
-        local _, link = GetItemInfo(self.itemID)
-        return link
+        return (select(2, C_Item.GetItemInfo(self.itemID)));
     end
 
     if not self:IsItemEmpty() then
@@ -299,9 +298,7 @@ end
 
 function ItemMixin:GetInventoryTypeName()
     if not self:IsItemEmpty() then
-        -- GetItemInfoInstant returns: itemID, itemType, itemSubType, equipLoc, ...
-        local _, _, _, equipLoc = C_Item.GetItemInfoInstant(self:GetItemID())
-        return equipLoc
+        return select(4, C_Item.GetItemInfoInstant(self:GetItemID()));
     end
 end
 

AddOns/!!!ClassicAPI/Util/Pools.lua

@@ -14,7 +14,8 @@
 --     consumer addons doesn't fail; they'll error at Acquire() time
 --     if anything actually invokes them.
 --   - Lua 5.0 has no `...` as expression; the converter mixins use
---     `unpack(arg, 1, arg.n)` to forward varargs.
+--     `unpack(arg)` to forward varargs (spans arg[1..arg.n] via getn;
+--     this build's unpack ignores explicit i/j — the count lives in `.n`).
 --   - assertsafe / nop aren't globals in stock 1.12; defined as local
 --     fallbacks (same pattern as FunctionUtil.lua).
 
@@ -381,12 +382,12 @@ function FramePoolCollectionConverterMixin:CreatePoolKeyFromPoolArgs(args)
 end
 
 function FramePoolCollectionConverterMixin:GetOrCreatePool(...)
-	local args = FramePoolCollection_ArgsToTable(unpack(arg, 1, arg.n));
+	local args = FramePoolCollection_ArgsToTable(unpack(arg));
 	return self:GetOrCreatePoolWithArgs(args);
 end
 
 function FramePoolCollectionConverterMixin:CreatePool(...)
-	local args = FramePoolCollection_ArgsToTable(unpack(arg, 1, arg.n));
+	local args = FramePoolCollection_ArgsToTable(unpack(arg));
 	return self:CreatePoolWithArgs(args);
 end
 
@@ -432,12 +433,12 @@ local function FontStringPoolCollection_ArgsToTable(parent, layer, subLayer, tem
 end
 
 function FontStringPoolCollectionMixin:GetOrCreatePool(...)
-	local args = FontStringPoolCollection_ArgsToTable(unpack(arg, 1, arg.n));
+	local args = FontStringPoolCollection_ArgsToTable(unpack(arg));
 	return self:GetOrCreatePoolWithArgs(args);
 end
 
 function FontStringPoolCollectionMixin:CreatePool(...)
-	local args = FontStringPoolCollection_ArgsToTable(unpack(arg, 1, arg.n));
+	local args = FontStringPoolCollection_ArgsToTable(unpack(arg));
 	return self:CreatePoolWithArgs(args);
 end
 

AddOns/!!!ClassicAPI/Util/TableUtil.lua

@@ -431,7 +431,7 @@ function SafePack(...)
 end
 
 function SafeUnpack(tbl)
-    return unpack(tbl, 1, tbl.n)
+    return unpack(tbl)
 end
 
 -- Returns the length of a table, accounting for the possibility of a table constructed using SafePack.

AddOns/!!!ClassicAPI/Util/TradeSkillLink.lua

@@ -448,8 +448,8 @@ local function CreateFrame_TradeSkillLink()
 
 	prodIcon:SetScript("OnEnter", function()
 		GameTooltip:SetOwner(this, "ANCHOR_RIGHT");
-		if this.itemLink then
-			GameTooltip:SetHyperlink(this.itemLink);
+		if this.itemID then
+			GameTooltip:SetHyperlink("item:"..this.itemID);
 		elseif this.spellID then
 			GameTooltip:SetSpellByID(this.spellID);
 		end
@@ -490,12 +490,17 @@ local function CreateFrame_TradeSkillLink()
 		nm:SetJustifyV("MIDDLE");
 		btn.nameText = nm;
 		btn:SetScript("OnEnter", function()
-			if this.itemLink then
+			if this.itemID then
 				GameTooltip:SetOwner(this, "ANCHOR_RIGHT");
-				GameTooltip:SetHyperlink(this.itemLink);
+				GameTooltip:SetHyperlink("item:"..this.itemID);
 				GameTooltip:Show();
 			end
 		end);
+		btn:SetScript("OnClick", function()
+			if this.itemLink and IsShiftKeyDown() and ChatFrameEditBox:IsVisible() then
+				ChatFrameEditBox:Insert(this.itemLink);
+			end
+		end);
 		btn:SetScript("OnLeave", GameTooltip_Hide);
 		f.reagents[i] = btn;
 		return btn;
@@ -564,6 +569,7 @@ local function CreateFrame_TradeSkillLink()
 			local name, tex, link = itemInfo(reagent.itemID);
 			btn:SetNormalTexture(tex or "Interface\\Icons\\INV_Misc_QuestionMark");
 			btn.itemLink = link;
+			btn.itemID = reagent.itemID
 			btn.nameText:SetText(name or ("item:" .. reagent.itemID));
 			local need = reagent.count or 1;
 			local have = C_Item.GetItemCount(reagent.itemID) or 0;
@@ -582,6 +588,7 @@ local function CreateFrame_TradeSkillLink()
 		end
 		for i = total + 1, table.getn(self.reagents) do
 			self.reagents[i].itemLink = nil;
+			self.reagents[i].itemID = nil;
 			self.reagents[i]:Hide();
 		end
 
@@ -630,6 +637,7 @@ local function CreateFrame_TradeSkillLink()
 		self.prodDesc:SetText("");
 		for i = 1, table.getn(self.reagents) do
 			self.reagents[i].itemLink = nil;
+			self.reagents[i].itemID = nil;
 			self.reagents[i]:Hide();
 		end
 		self.detailChild:SetHeight(LO.detail[4]);
@@ -726,7 +734,7 @@ local function ShowTradeSkillLink(link, text)
 		local r = recipes[i];
 		if r.isKnown then
 			r.band = DifficultyBand(cur or 0, r.trivialLevel, r.greenLevel);
-			r.name = GetSpellInfo(r.spellID) or ("spell:" .. r.spellID);
+			r.name = C_Spell.GetSpellName(r.spellID) or ("spell:" .. r.spellID);
 			table.insert(knownList, r);
 		end
 	end

README.md

@@ -35,7 +35,7 @@ Full per-function reference: **[docs/API.md](docs/API.md)**.
 | [Expansion](docs/API.md#expansion) | `ClassicExpansionAtLeast`, `ClassicExpansionAtMost`, `GetClassicExpansionLevel` |
 | [Faction](docs/API.md#faction) | `C_Reputation.GetFactionDataByIndex`, `C_Reputation.GetFactionStandings`, `C_Reputation.GetLastStandingChange`, `C_Reputation.GetWatchedFactionData`, `C_Reputation.SetWatchedFactionByID`, `GetFactionIDByIndex`, `GetFactionInfoByID`, `GetFactionParentID` |
 | [Focus](docs/API.md#focus) | `ClearFocus`, `FocusUnit` |
-| [Frame](docs/API.md#frame) | `region:SetPoint("point")` (one-arg form), `region:SetSize`, `region:GetSize`, `region:IsMouseOver`, `region:GetRect`, `region:IsDragging`, `frame:SetShown`, `frame:SetResizeBounds`, `frame:HookScript`, `frame:IsEventRegistered`, `frame:GetEffectiveAlpha` |
+| [Frame](docs/API.md#frame) | `region:SetPoint("point")` (one-arg form), `region:SetSize`, `region:GetSize`, `region:IsMouseOver`, `region:GetRect`, `region:IsDragging`, `frame:SetShown`, `frame:SetResizeBounds`, `frame:HookScript`, `frame:IsEventRegistered`, `frame:GetEffectiveAlpha`, `frame:SetAttribute`, `frame:SetAttributeNoHandler`, `frame:ClearAttribute`, `frame:GetAttribute`, `OnAttributeChanged` (script), `SetModernScriptArgs`, `GetModernScriptArgs` |
 | [FriendList](docs/API.md#friendlist) | `C_FriendList.IsWhoQueryPending`, `C_FriendList.SendWhoQueryByName` |
 | [GameObject](docs/API.md#gameobject) | `C_GameObjectInfo.GetGameObjectInfoByID`, `C_GameObjectInfo.RequestLoadGameObjectByID`, `ClosestGameObjectPosition` |
 | [Glue](docs/API.md#glue) | `C_Glue.IsOnGlueScreen` |

docs/API.md

@@ -165,6 +165,7 @@ build instructions.
   - [`frame:IsEventRegistered(event)`](#frameiseventregisteredevent)
   - [`frame:GetEffectiveAlpha()`](#framegeteffectivealpha)
   - [`frame:SetAttribute` / `SetAttributeNoHandler` / `ClearAttribute` / `GetAttribute` (+ unit-frame mouseover)](#framesetattributename-value--framesetattributenohandlername-value--frameclearattributename--framegetattribute)
+  - [`SetModernScriptArgs(enable)` / `GetModernScriptArgs()`](#setmodernscriptargsenable--getmodernscriptargs)
 
 - [FriendList](#friendlist)
   - [`C_FriendList.SendWhoQueryByName(name)`](#c_friendlistsendwhoquerybynamename)
@@ -3923,6 +3924,40 @@ per-frame handler slot for this one name (1.12 frames are never destroyed, so
 the slot never goes stale). Recursion-guarded, so a handler may itself call
 `SetAttribute`.
 
+### `SetModernScriptArgs(enable)` / `GetModernScriptArgs()`
+
+Global toggle (not a frame method) for **modern positional script arguments**.
+Vanilla always invokes a frame-script handler with **zero** Lua arguments — the
+handler reads `this` / `arg1..argN` / `event` as globals. With this enabled, every
+handler additionally receives its values as real positional arguments, so modern
+addon ports written as `function(self, delta) … end` work unmodified:
+
+- most scripts: `(self, arg1..argN)` — e.g. `OnMouseWheel(self, delta)`,
+  `OnClick(self, button)`, `OnValueChanged(self, value)`, `OnUpdate(self, elapsed)`.
+- `OnEvent`: `(self, event, arg1..argN)`.
+
+It's purely **additive** — the `this` / `arg1` / `event` globals are still set, so
+vanilla-style handlers keep working; a handler declaring no parameters just ignores
+the extras. `SetModernScriptArgs(enable)` sets the state and returns it;
+`GetModernScriptArgs()` returns the current state.
+
+**Default OFF (opt-in).** It reimplements the tail of the engine's hottest Lua
+path (the runner that fires for every `OnUpdate`, every frame), so it stays off
+unless you ask for it; while off the dispatch path is exactly vanilla. Enable it
+once at load if your addon relies on the modern handler signature.
+
+```lua
+SetModernScriptArgs(true)   -- returns true
+
+local f = CreateFrame("Frame")
+f:EnableMouseWheel(true)
+f:SetScript("OnMouseWheel", function(self, delta)
+    -- `self` and `delta` are bound; `this` / `arg1` still work too
+end)
+
+GetModernScriptArgs()       -- true
+```
+
 ## FriendList
 
 ### `C_FriendList.SendWhoQueryByName(name)`