View Single Post
Old 11-25-12, 09:16 PM   #2
Phanx
A Pyroguard Emberseer
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 3,703
You could use COMBAT_LOG_EVENT instead of COMBAT_LOG_EVENT_UNFILTERED, but only if your combat log is currently set to display the events you need. This means that if you switch to a combat log filter that only shows your actions, for example, your addon would stop working. You also can't guarantee that other people will have their filters set correctly. For these reasons (mainly the second) you should always use CLEU over CLE for addons you plan to release to the public.

Also, there's no need to use string.sub and tonumber on the GUID; just use bit.band on the whole GUID and compare it to the COMBATLOG_OBJECT_CONTROL_PLAYER global (0x00000100). This will save you 2 function calls and one string creation.

Here's a drycoded example with a few other minor changes:

Code:
-- For general use, I wouldn't recommend obsessively upvaluing every
-- global but it is worth doing for situations like CLEU or OnUpdate
-- where you are accessing them extremely often.
local bit_band = bit.band
local COMBATLOG_OBJECT_CONTROL_PLAYER = COMBATLOG_OBJECT_CONTROL_PLAYER

-- GUIDs are not available when the main chunk executes, so you have to
-- wait and fill this in once the combat log starts running.
local playerGUID

local frame_cleu = CreateFrame("FRAME", "remgankframe_cleu")
frame_cleu:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED")

-- There's no need to define your event handler function separately and
-- then set it as the OnEvent script. You can just define the function
-- right in the SetScript call.
frame_cleu:SetScript("OnEvent", function(self, event, timeStamp, event, hideCaster, sourceGUID, sourceName, sourceFlags, sourceRaidFlags, destGUID, destName, destFlags, destRaidFlags, prefixParam1, prefixParam2, dummyparam, suffixParam1, suffixParam2)
	-- Fill in the playerGUID variable here if needed.
	if not playerGUID then
		playerGUID = UnitGUID("player")
	end

	-- Since these first two conditions apply no matter which event is
	-- firing, check them first.
	if destGUID == playerGUID and bit_band(sourceGUID, COMBATLOG_OBJECT_CONTROL_PLAYER) == COMBATLOG_OBJECT_CONTROL_PLAYER then
		if event == "SWING_DAMAGE" then
			-- Nest checks inside each other instead of using and, so
			-- that if any check matches (eg. it is this event, but not
			-- the right prefix) the code doesn't keep checking other
			-- things that obviously can never match.
			if prefixParam2 > 0 then
				print(string_format("[%s] killed [%s] with %s Melee overkill %s", sourceName, destName, prefixParam1, suffixParam2))
			end
		elseif event == "SPELL_DAMAGE" or event == "SPELL_PERIODIC_DAMAGE" or event == "RANGE_DAMAGE" then
			if suffixParam2 > 0 then
				print(string_format("[%s] killed [%s] with %s damage of %s overkill %s", sourceName, destName, suffixParam1, GetSpellLink(prefixParam1), suffixParam2))
			end
		end
	end

	-- a sample line when an enemy player killed me
	-- [name] killed [me] with 65762 damage of [Combustione dell'Ombra] overkill 48472
end)
I also don't know what you were using the type variable for (since it was always set to an empty string) and it's generally a bad idea to use local variables with the same names as global functions or variables (the type function tells you whether a value is a string, number, table, etc.) so I just deleted it.
__________________
Author/maintainer of Grid, PhanxChat, ShieldsUp, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
Phanx is offline   Reply With Quote