Fix summon/target buttons silently doing nothing (Classic Era)

RaidSummon/RaidSummon.lua

@@ -1,5 +1,8 @@
 RaidSummon = LibStub("AceAddon-3.0"):NewAddon("RaidSummon", "AceConsole-3.0", "AceEvent-3.0", "AceTimer-3.0", "AceComm-3.0")
 local L = LibStub("AceLocale-3.0"):GetLocale("RaidSummon", true)
+-- Classic Era's latest patch moved GetAddOnMetadata to C_AddOns; fall
+-- back to the old global for clients that haven't made the switch.
+local GetAddOnMetadata = (C_AddOns and C_AddOns.GetAddOnMetadata) or GetAddOnMetadata
 
 --set options
 local options = {
@@ -449,27 +452,13 @@ function RaidSummon:NameListButton_PreClick(source, button)
 
 	RaidSummon:getRaidMembers()
 
-	if RaidSummonRaidMembersDB then
-		for i, v in ipairs (RaidSummonRaidMembersDB) do
-			if v.rName == name then
-				raidIndex = "raid"..v.rIndex
-			end
-		end
-
-		if raidIndex then
-			--set target when not in combat (securetemplate)
-			if not InCombatLockdown() then
-				if RaidSummonRaidMembersDB then
-					source:SetAttribute("type1", "target")
-					source:SetAttribute("unit", raidIndex)
-				end
-				source:SetAttribute("type2", "spell")
-				source:SetAttribute("spell", "698") --698 - Ritual of Summoning
-			else
-				print(L["Lockdown"])
-			end
-		end
-	end
+	-- Secure attributes (type1/macrotext1/type2/macrotext2) are staged
+	-- once in UpdateList() when the button's name/text is set, not here.
+	-- Re-staging in PreClick on every click did not reliably take effect
+	-- in time for the same click to use. Attributes staged before combat
+	-- remain valid through combat, so clicking still works mid-fight
+	-- even though UpdateList() itself can't restage anything new while
+	-- InCombatLockdown() is true.
 
 	if buttonName == "RightButton" and targetname ~= nil and not InCombatLockdown() then
 
@@ -567,14 +556,33 @@ function RaidSummon:UpdateList()
 			end
 
 			if not InCombatLockdown() then
-				_G["RaidSummon_NameList"..i]:Show()
+				-- Stage secure attributes here, once, when the list is
+				-- built, rather than inside a PreClick handler on every
+				-- click. Re-staging in PreClick (the old approach) did not
+				-- reliably take effect in time for the same click to use.
+				local btn = _G["RaidSummon_NameList"..i]
+				local rName = RaidSummonBrowseDB[i].rName
+				local ritualSpellName = GetSpellInfo(698) --698 - Ritual of Summoning
+				btn:SetAttribute("unit", nil)
+				btn:SetAttribute("type1", "macro")
+				btn:SetAttribute("macrotext1", "/target "..rName)
+				if ritualSpellName then
+					btn:SetAttribute("type2", "macro")
+					btn:SetAttribute("macrotext2", "/target "..rName.."\n/cast "..ritualSpellName)
+				end
+				btn:Show()
 			else
 				RaidSummon:UpdateListCombatCheck()
 			end
 		else
 			if not InCombatLockdown() then
 				_G["RaidSummon_NameList"..i.."TextName"]:SetText("")
-				_G["RaidSummon_NameList"..i]:Hide()
+				local btn = _G["RaidSummon_NameList"..i]
+				btn:SetAttribute("type1", nil)
+				btn:SetAttribute("macrotext1", nil)
+				btn:SetAttribute("type2", nil)
+				btn:SetAttribute("macrotext2", nil)
+				btn:Hide()
 			else
 				RaidSummon:UpdateListCombatCheck()
 			end
@@ -605,15 +613,37 @@ function RaidSummon:getRaidMembers()
 		if (members > 0) then
 		RaidSummonRaidMembersDB = {}
 
+			-- GetRaidRosterInfo(i)'s index is the roster LIST position,
+			-- which is not guaranteed to match the "raidN" unit token
+			-- (actual subgroup slot). They only coincidentally line up
+			-- for some raid compositions. Resolve rIndex from the actual
+			-- unit token instead of trusting the roster-list position.
 			for i = 1, members do
 				local rName, rRank, rSubgroup, rLevel, rClass, rfileName = GetRaidRosterInfo(i)
 
 				if rName and rClass and rfileName then
-					RaidSummonRaidMembersDB[i] = {}
-					RaidSummonRaidMembersDB[i].rIndex = i
-					RaidSummonRaidMembersDB[i].rName = rName
-					RaidSummonRaidMembersDB[i].rClass = rClass
-					RaidSummonRaidMembersDB[i].rfileName = rfileName
+					-- Prefer the verified "raidN" unit token, but fall
+					-- back to the roster-list position if it can't be
+					-- cross-matched (e.g. a name mismatch between
+					-- GetRaidRosterInfo and UnitName for names with
+					-- accented/special characters) -- dropping the
+					-- member entirely instead left a hole in this
+					-- array, which silently truncated every ipairs()
+					-- loop over it after the first unmatched member.
+					local unitToken
+					for u = 1, MAX_RAID_MEMBERS do
+						if UnitName("raid"..u) == rName then
+							unitToken = u
+							break
+						end
+					end
+
+					table.insert(RaidSummonRaidMembersDB, {
+						rIndex = unitToken or i,
+						rName = rName,
+						rClass = rClass,
+						rfileName = rfileName,
+					})
 				end
 			end
 		end

RaidSummon/RaidSummon.xml

@@ -1,5 +1,9 @@
 <Ui xmlns="http://www.blizzard.com/wow/ui/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.blizzard.com/wow/ui/ ..\FrameXML\UI_shared.xsd">
-	<Button name="RaidSummon_NameListButton" inherits="SecureActionButtonTemplate" virtual="true">
+	<!-- Was SecureActionButtonTemplate. Isolated testing showed that
+	     template's OnClick dispatch silently fails to execute type1/
+	     type2 macro actions on the current client, while an otherwise
+	     identical SecureUnitButtonTemplate button works correctly. -->
+	<Button name="RaidSummon_NameListButton" inherits="SecureUnitButtonTemplate" virtual="true">
 		<Size>
 			<AbsDimension x="100" y="16"/>
 		</Size>