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/CallbackRegistry.lua

@@ -26,17 +26,17 @@ local function GenerateClosure(func, ...)
     for i = 1, boundN do bound[i] = arg[i] end
     return function(...)
         local callN = arg.n
-        -- Pass the first bound arg (the owner, always bound[1] here) explicitly
-        -- and everything else as the unpacked tail. Merging bound + call into
-        -- one table and unpacking THAT collapses the vararg count on this Lua
-        -- 5.0 build when a hole lands right after a present element — same trap
-        -- fixed in TriggerEvent's Function branch. Keeping owner out of the
-        -- unpacked table keeps a leading-nil payload inside the vararg tail.
-        local rest = {}
-        local restN = 0
-        for i = 2, boundN do restN = restN + 1; rest[restN] = bound[i] end
-        for i = 1, callN do restN = restN + 1; rest[restN] = arg[i] end
-        return func(bound[1], unpack(rest, 1, restN))
+        -- 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
 
@@ -152,14 +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. Pass it
-            -- explicitly with the payload as the unpacked tail. Do NOT build a
-            -- { owner, ... } table and unpack THAT: on this Lua 5.0 build,
-            -- unpacking a table whose hole lands right after a present element
-            -- (owner) into a `func(first, ...)` collapses the vararg count to 0,
-            -- silently dropping a leading-nil event payload (verified in-game
-            -- with reload's PLAYER_ENTERING_WORLD, arg1=nil arg2=1).
-            func(owner, unpack(arg, 1, arg.n))
+            -- 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