UnitFlag Filter or GUID?

[Numba1stunna]

Hello,

So I have made a frame to test the number of times a specific event is triggered and the sub-catagories of those events. I only put the ones I want to use.

if not TestEvent then
		TestEvent = CreateFrame("FRAME","OurFrame")
		TestEvent:Hide()
	end
	local function TestEvent_OnEvent(self, event, ...)
		if event == "COMBAT_LOG_EVENT_UNFILTERED" then
			CombatLogEventUnfiltered = CombatLogEventUnfiltered + 1
			if type == "SPELL_DAMAGE" then
				SpellDamage = SpellDamage + 1
				return
			end
			if type == "SPELL_PERIODIC_DAMAGE" then
				SpellPeriodicDamage = SpellPeriodicDamage + 1
				return
			end
			if type == "SPELL_CAST_SUCCESS" then
				SpellCastSuccess = SpellCastSuccess + 1
				return
			end
			if type == "SPELL_CAST_START" then
				SpellCastStart = SpellCastStart + 1
				return
			end
			if type == "SPELL_AURA_APPLIED" then
				SpellAuraApplied = SpellAuraApplied + 1
				return
			end
			if type == "SPELL_CAST_FAILED" then
				SpellCastFailed = SpellCastFailed + 1
				return
			end
			if type == "SWING_DAMAGE" then
				SwingDamage = SwingDamage + 1
				return
			end
			if type == "SWING_MISSED" then -- different parameters from swing_dmg
				SwingMissed = SwingMissed + 1
				return
			end
			if type == "UNIT_DIED" then
				UnitDied = UnitDied + 1
				return
			end
			if type == "SPELL_PERIODIC_AURA_REMOVED" then
				SpellPeriodicAuraRemoved = SpellPeriodicAuraRemoved + 1
				return
			end
			if type == "SPELL_AURA_REMOVED" then
				SpellAuraRemoved = SpellAuraRemoved + 1
				return
			end
			return
		end

	    if event == "UNIT_COMBO_POINTS" then
	    	UnitComboPoints = UnitComboPoints + 1
	    	return
	    end
	    if event == "UI_ERROR_MESSAGE" then
	    	UIErrorMessage = UIErrorMessage + 1
	    	return
	    end
	    if event == "PLAYER_TARGET_CHANGED" then 
	    	PlayerTargetChanged = PlayerTargetChanged + 1
	    	return
	    end
		if event == "UNIT_SPELLCAST_SENT" then
			UnitSpellcastSent = UnitSpellcastSent + 1
			return
		end
		if event == "UNIT_SPELLCAST_SUCCEEDED" then
			UnitSpellcastSucceeded = UnitSpellcastSucceeded + 1
			return
		end
		return
	end

	TestEvent:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
	TestEvent:RegisterEvent("UNIT_COMBO_POINTS")
	TestEvent:RegisterEvent("PLAYER_TARGET_CHANGED")
	TestEvent:RegisterEvent("PLAYER_FOCUS_CHANGED")
	TestEvent:RegisterEvent("UI_ERROR_MESSAGE")
	TestEvent:RegisterEvent("UNIT_SPELLCAST_SENT")
	TestEvent:RegisterEvent("UNIT_SPELLCAST_SUCCEEDED")
	TestEvent:SetScript("OnEvent", TestEvent_OnEvent)

The results of the events after a battle grounds was...

COMBAT_LOG_EVENT_UNFILTERED : 89946
--> SPELL_DAMAGE: 9482
--> SPELL_PERIODIC_DAMAGE: 5208
--> SPELL_CAST_SUCCESS: 7498
--> SPELL_CAST_START: 1053
--> SPELL_AURA_APPLIED: 13431
--> SPELL_CAST_FAILED: 12452
--> SWING_DAMAGE: 3132
--> SWING_MISSED: 1091
--> UNIT_DIED: 244
--> SPELL_PERIODIC_AURA_REMOVED: 0 -- not sure why
--> SPELL_AURA_REMOVED: 12946 -- I tested this for hots/dots and it uses this one
UNIT_COMBO_POINTS: 1118
UI_ERROR_MESSAGE: 12626
PLAYER_TARGET_CHANGED: 227
UNIT_SPELLCAST_SENT: 1751 -- my understanding is this only applies to the player
UNIT_SPELLCAST_SUCCEEDED: 7188 -- this seems to apply to player and party/raid members.

So with these results, I plan to make an addon starting from highest to lowest number of triggers.

Now my question is with the bit.bands. Should I used a bit.band filter or a GUID filter? or both?

Example:

local function TestEvent_OnEvent(self, event, ...)
	if event == "COMBAT_LOG_EVENT_UNFILTERED" then
		local timestamp, type, _, sourceGUID, sourceName, sourceFlags, _, destGUID, destName, destFlags, destFlags2 = select(1, ...)
		local COMBATLOG_OBJECT_AFFILIATION_MINE = COMBATLOG_OBJECT_AFFILIATION_MINE
		local COMBATLOG_OBJECT_TARGET = COMBATLOG_OBJECT_TARGET
		if type == "SPELL_CAST_SUCCESS" then
			if bit.band(destFlags,COMBATLOG_OBJECT_AFFILIATION_MINE) == COMBATLOG_OBJECT_AFFILIATION_MINE then
				DoStuff()
				return
			end
			if bit.band(sourceFlags,COMBATLOG_OBJECT_TARGET) == COMBATLOG_OBJECT_TARGET then
				DoStuff()
				return
			end
		end
	end
end

OR

local function TestEvent_OnEvent(self, event, ...)
	if event == "COMBAT_LOG_EVENT_UNFILTERED" then
		local timestamp, type, _, sourceGUID, sourceName, sourceFlags, _, destGUID, destName, destFlags, destFlags2 = select(1, ...)
		local playerGUID = UnitGUID("player")
		local targetGUID = UnitGUID("target")
		if type == "SPELL_CAST_SUCCESS" then
			if playerGUID == destGUID then
				DoStuff()
				return
			end
			if targetGUID == sourcGUID then
				DoStuff()
				return
			end
		end
	end
end

[SDPhantom]

The difference is between whether or not a function call is added to the evaluation of a condition. When it comes to CLEU events, it's best to use as little time processing as possible. Something to do is cache the player and target GUIDs outside the event handle and refer to the cache rather than calling their functions each time.

local PlayerGUID = UnitGUID("player");
local TargetGUID;
local function TestEvent_OnEvent(self, event, ...)
    if event == "PLAYER_TARGET_CHANGED" then
        TargetGUID = UnitGUID("target");
    elseif event == "COMBAT_LOG_EVENT_UNFILTERED" then
        local timestamp, type, _, sourceGUID, sourceName, sourceFlags, _, destGUID, destName, destFlags, destFlags2 = ...;
        if type == "SPELL_CAST_SUCCESS" then
            if PlayerGUID == destGUID then
                DoStuff()
                return
            end
            if TargetGUID == sourcGUID then
                DoStuff()
                return
            end
        end
    end
end

PS: Don't ever call select(1,...) it's a waste of CPU power and does absolutely nothing. select() is an arg shifting function that shifts the output to the index given. Calling select(1,...) is telling it to shift the args to return the list starting at position 1. This essentially spits out the exact list you gave it, wasting the CPU's time.

Also, if you're storing copies of globals or other values that don't change often, you should consider moving those locals outside of the function. As-is, you're not regaining any performance with them where they are. The code above is an example of how to efficiently use them

[Numba1stunna]

Thank you for the reply! I have another problem. It seems that sometimes the function DoStuff() is called twice, why is that?

[SDPhantom]

In the worst case scenario, CLEU can fire hundreds of times even between frame renderings. Depending if the two instances of DoStuff() are doing the same thing, you can consolidate the two if blocks.

if PlayerGUID == destGUID or TargetGUID == sourcGUID then
	DoStuff()
end

Otherwise, you can try forming them into an if ... elseif block.

if PlayerGUID == destGUID then
	DoStuffA()
elseif TargetGUID == sourcGUID then
	DoStuffB()
end

[Phanx]

Originally Posted by SDPhantom Something to do is cache the player and target GUIDs outside the event handle and refer to the cache rather than calling their functions each time. local PlayerGUID = UnitGUID("player"); local TargetGUID; local function TestEvent_OnEvent(self, event, ...)     if event == "PLAYER_TARGET_CHANGED" then         TargetGUID = UnitGUID("target");     elseif event == "COMBAT_LOG_EVENT_UNFILTERED" then

You should be aware that the player's GUID is not available in the main chunk on a fresh login. The code you posted will work when reloading the UI, but won't work on a fresh login. The PlayerGUID variable's value will be nil, and any check against it will not have the desired effect -- and may in fact have an undesired effect; for example, if you compare the destGUID against it, then your check will pass when the spell that was cast doesn't have a target, which is not what you were intending to detect!

To solve this problem, you should also listen for the PLAYER_LOGIN event, and update the PlayerGUID variable when it fires:

local PlayerGUID, TargetGUID;
local function TestEvent_OnEvent(self, event, ...)
    if event == "PLAYER_LOGIN" then
        PlayerGUID = UnitGUID("player");
    elseif event == "PLAYER_TARGET_CHANGED" then
        TargetGUID = UnitGUID("target");
    elseif event == "COMBAT_LOG_EVENT_UNFILTERED" then
        -- do stuff here

Also, for efficiency, you should try to order the if/else chain in your event handler so that the most frequent events are checked first, and the least frequent last. For the above three events, your if/else chain should look like this:

local function TestEvent_OnEvent(self, event, ...)
    if event == "COMBAT_LOG_EVENT_UNFILTERED" then
        -- do stuff here
    elseif event == "PLAYER_TARGET_CHANGED" then
        TargetGUID = UnitGUID("target");
    elseif event == "PLAYER_LOGIN" then
        PlayerGUID = UnitGUID("player");

Anyway, your DoStuff function shouldn't be called twice for any given CLEU event, based on the code you posted, because you're returning after calling it. (And actually you can/should just do return DoStuff() instead of DoStuff() and then return separately.) Did you change your code? Why do you think DoStuff is being called twice? Have you tried printing the arguments to get more information? (It's possible CLEU could be firing twice for some particular spell.)

Finally, in the interest of making your code easier to read and maintain, you might consider replacing your giant nested if/else chain for handling events with table lookups and named methods, eg:

local playerGUID, targetGUID
 
local addon = CreateFrame("Frame")
addon:SetScript("OnEvent", function(self, event, ...)
    return self[event](self, ...)
end)
addon:RegisterEvent("PLAYER_LOGIN")
 
function addon:PLAYER_LOGIN()
    playerGUID = UnitGUID("player")
 
    self:RegisterEvent("PLAYER_TARGET_CHANGED")
    self:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED")
    -- Register more events here
end
 
function addon:PLAYER_TARGET_CHANGED()
    targetGUID = UnitGUID("target")
end
 
function addon:COMBAT_LOG_EVENT_UNFILTERED(timestamp, event, ...)
    return self[event] and self[event](self, ...)
end
 
function addon:SPELL_CAST_SUCCESS(_, sourceGUID, sourceName, sourceFlags, sourceRaidFlags, destGUID, destName, destFlags, destRaidFlags, spellID, spellName, spellSchool)
    if playerGUID == destGUID then
        print(spellName, "was cast on you!")
    elseif targetGUID and targetGUID == sourceGUID then
        -- ^ targetGUID == sourceGUID shouldn't match if both are nil,
        --   since that means you don't have a target right now.
        print(spellName, "was cast on your target.")
    end
end