Simple interrupt/dispel/grounding script help

[ramzax]

Hello guys, I've been working on a simple interrupt, dispel, ground and reflect announce script using some code I found on a forum as base.
The code is probably really inefficient, but so far it does announce interrupts correctly, but I can't get groundings to work. I want it to only say what it did if in a pvp setting or in combat, but so far I can't get it to work except by using instanceType and setting it to pvp and arena, but I don't know how to check if in combat outside an instance.

Here's the code so far:

local _,instanceType = IsInInstance()
    if instanceType == "arena" or instanceType == "pvp" then
        local f = CreateFrame("Frame")
        local function Update(self, event, ...)
            local pvpType = GetZonePVPInfo()    
                f:UnregisterEvent("ZONE_CHANGED_NEW_AREA")  
            if event == "COMBAT_LOG_EVENT_UNFILTERED" then      
                local timestamp, event, eventType, _, sourceGUID, sourceName, sourceFlags, sourceRaidFlags, destGUID, destName, destFlags, _, spellID, spellName, _, extraskillID, extraSkillName = ...
                    if eventType == "SPELL_INTERRUPT" and sourceName == UnitName("player") then
                            SendChatMessage("Interrupted "..destName.."'s"..GetSpellLink(extraskillID), "PARTY", nil, nil)
                        elseif eventType == "SPELL_DISPEL" and sourceName == UnitName("player") then
                            SendChatMessage("Dispelled "..destName.."'s"..GetSpellLink(extraskillID), "PARTY", nil, nil)
                        elseif eventType == "SPELL_STOLEN" and sourceName == UnitName("player") then
                            SendChatMessage("Stole "..destName.."'s"..GetSpellLink(extraskillID), "PARTY", nil, nil)
                        elseif eventType == "SPELL_MISSED" then
                            if (bit.band(destFlags, COMBATLOG_OBJECT_AFFILIATION_MINE) > 0) then
                                local _,missType = select(13,...)
                                    if missType == "REFLECT" then
                                        SendChatMessage("Reflected "..GetSpellLink(extraskillID)..".", "PARTY", nil, nil)
                                        SendChatMessage("Reflected "..GetSpellLink(extraskillID)..".", "SAY", nil, nil)
                                    elseif destName == "Grounding Totem" and (bit.band(destFlags, COMBATLOG_OBJECT_AFFILIATION_MINE) > 0) then
                                        SendChatMessage("Grounded "..GetSpellLink(extraskillID)..".", "PARTY", nil, nil)
                                        SendChatMessage("Grounded "..GetSpellLink(extraskillID)..".", "SAY", nil, nil)
                                    end
                            end
                        elseif eventType== "SPELL_DAMAGE" then
                            if (bit.band(destFlags, COMBATLOG_OBJECT_AFFILIATION_MINE) > 0) then
                                local spellName = select(13,...)
                                    if destName == "Grounding Totem" then
                                        SendChatMessage("Grounded "..GetSpellLink(extraskillID)..".", "PARTY", nil, nil)
                                        SendChatMessage("Grounded "..GetSpellLink(extraskillID)..".", "SAY", nil, nil)
                                    end
                            end
                    end
                end
            end
        f:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
        f:RegisterEvent("ZONE_CHANGED_NEW_AREA")
        f:SetScript("OnEvent", Update)
    end

Any help would be appreciated

[Seerah]

http://wowprogramming.com/docs/api/InCombatLockdown

[Phanx]

#1 - You should check with your group before using an addon like this. Many players consider this sort of automated announcement to be spam, and will not appreciate it, especially in a random group. Personally I would /ignore anyone using this kind of thing in a random group; outside of a coordinated arena team or RBG group, absolutely nobody cares what spell your shaman's Grounding Totem is absorbing or what spell your mage is stealing.

#2 - Please use consistent indenation. Your code is an unreadable mess because nothing is indented consistently.

#3 - Your code as written will only do load if you are already in an arena or battleground when you log in, since you have wrapped everything in an instance type check. Get rid of that.

#4 - Even if you are in an arena/bg on login, your code will not stop working once you leave those areas, since you stop listening for zone changes the first time any event fires, and you don't actually check to see what kind of zone you're in before announcing things.

#5 - You're using an incorrect list of arguments for CLEU -- the sub-event ("eventType") is the second argument, but you've inserted an extra "event" argument in that position, causing your variable names to not match their actual values -- so your code isn't actually checking what you think it's checking. You're also checking the wrong variables in several other places, which looks like the result of indiscriminiate copying and pasting.

#6 - CLEU is a very frequent event. Rather than checking the zone type every time it fires, you should simply check the zone type when the zone type changes, and unregister CLEU if the zone type doesn't match what you want. You should also check the most frequent cases (eg. SPELL_DAMAGE) first so as little code has to execute as possible each time it fires.

#7 - Rather than calling UnitName over and over every time a combat event occurs, you should call it once when your addon loads, store the result in a variable, and check against that. Better yet, check the GUID instead of the name, to avoid false positives if a cross-realm player has the same name as you.

#8 - If you want your addon to work for non-English players, you should be using NPC IDs rather than names (eg. "Grounding Totem").

#9 - When passing arguments to a function, eg. SendChatMessage, you don't need to pass a bunch of nil values at the end of the list. Just stop the list with the last non-nil argument.

Not tested, but here's a much-cleaned-up version:

local COMBATLOG_OBJECT_AFFILIATION_MINE = COMBATLOG_OBJECT_AFFILIATION_MINE
local bit_band, strsub, tonumber = bit.band, strsub, tonumber

local playerGUID

local f = CreateFrame("Frame")
f:RegisterEvent("PLAYER_LOGIN")
f:SetScript("OnEvent", function(self, event, timestamp, eventType, hideCaster, sourceGUID, sourceName, sourceFlags, sourceRaidFlags, destGUID, destName, destFlags, destRaidFlags, ...)
	if event == "COMBAT_LOG_EVENT_UNFILTERED" then
		if eventType == "SPELL_DAMAGE" then
			if bit_band(destFlags, COMBATLOG_OBJECT_AFFILIATION_MINE) > 0 and tonumber(strsub(destGUID, 6, 10), 16) == 5925 then
				local spellID, spellName, spellSchool, amount, overkill, school, resisted, blocked, absorbed, critical, glancing, crushing = ...
				SendChatMessage(format("Grounded %s", GetSpellLink(spellID)), "PARTY")
				SendChatMessage(format("Grounded %s", GetSpellLink(spellID)), "SAY")
			end
			return
		end

		if eventType == "SPELL_MISSED" then
			if bit_band(destFlags, COMBATLOG_OBJECT_AFFILIATION_MINE) > 0 then
				local spellID, spellName, spellSchool, missType, isOffHand, missAmount = ...
				if missType == "REFLECT" then
					SendChatMessage(format("Reflected %s", GetSpellLink(extraSpellID)), "PARTY")
					SendChatMessage(format("Reflected %s", GetSpellLink(extraSpellID)), "SAY")
				elseif tonumber(strsub(destGUID, 6, 10), 16) == 5925 then
					SendChatMessage(format("Grounded %s", GetSpellLink(extraSpellID)), "PARTY")
					SendChatMessage(format("Grounded %s", GetSpellLink(extraSpellID)), "SAY")
				end
			end
			return
		end

		local action = eventType == "SPELL_DISPEL" and "Dispelled" or eventType == "SPELL_INTERRUPT" and "Interrupted" or eventType == "SPELL_STOLEN" and "Stole"
		if action then
			if sourceGUID == playerGUID then
				local spellID, spellName, spellSchool, extraSpellID, extraSpellName, extraSchool, auraType = ...
				SendChatMessage(format("%s %s's %s", action, destName, GetSpellLink(extraSpellID)), "PARTY")
			end
		end
	else
		if event == "PLAYER_LOGIN" then
			playerGUID = UnitGUID("player")
			self:RegisterEvent("PLAYER_REGEN_ENABLED")
			self:RegisterEvent("PLAYER_REGEN_DISABLED")
			self:RegisterEvent("ZONE_CHANGED_NEW_AREA")
		end

		local _, instanceType == IsInInstance()
		if IsInGroup() and (instanceType == "arena" or instanceType == "pvp" or (instanceType == "none" and GetZonePVPInfo() == "combat") or UnitAffectingCombat("player")) then
			self:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
		else
			self:UnregisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
		end
	end
end)