I was approached by the Death Knight tank in my guild, and was asked to write a stand-alone AddOn that provided death strike heal prediction. At first I wasn't sure I could write such an AddOn because I've never tried to interpret information from the combat log before, but I decided I'd give it a go.

I'd like some feedback and insight on the code thus far, keeping in mind the following notes:

The code is VERY far from complete. I wanted to throw a "proof of concept" together.

Death Strike heals you for the amount of damage you have sustained from non players within the last 5 seconds, with some additional modifiers that I chose to ignore for this test.

The table that holds the key-value pairs of time stamps and damage values populates to an unlimited size. I intend to remove all non-relevant entries.

The code is really sloppy, and not well commented at all, as this was more of a flow-of-thought.

My approach was to isolate all damage(JUST Swing damage for this test) that has an NPC as a source, and the player as the destination. Those values are then added to a table with the timeStamp as the key, and the damage amount as the value. A for-loop sorts through the table and adds together any damage values that have happened within the last 5 seconds.

Just keep in mind that for now, it is quite sloppy and not optimized. I hope this doesn't deter anyone from having a glance over it.

I'd really appreciate insight on the methods and processes, and not an entire written AddOn to replace what I have come up with. This has been a really hands-on learning process for me, and I'd like to continue that.

I just looked over it briefly - one thing I spotted (and I know you said this isn't optimized...) is that you should change this:

Code:

if destGUID == UnitGUID("player") and knownTypes[maskedTypeBit] == "NPC" then
if eventType == "SWING_DAMAGE" then
local amount = select(12, ...)
damageTable[timeStamp] = amount
end -- if event
end -- if destGUID

You can make that even more efficient by storing the player's GUID in a variable, and referring to that, instead of looking up the same value over and over again every time a combat log event occurs. The GUID isn't available before PLAYER_LOGIN on a fresh login, so you'll need to wait until then to populate the variable.

Thanks for the response bsmorgan. I have taken a look at it, but the over 5,000 lines of code make it quite difficult to sift through relevant parts of the code. I may fall back on it if I have some specific concerns further down the line.

Edit: In response to Torhal's suggestion of not using select(), should I just upsource the select() function, or use something similar to:

Lua Code:

local _, _, _, _, _, _, _, _, _, _, _, amount = ...

Edit 2: It is taking ~15 seconds for my loop to decide that the timestamps are too old. Is there a disparity between what time() and timestamp returns? I was under the impression that they were both in seconds, and that timestamp just returns with miliseconds as well, which shouldn't be contributing to this issue. Does "time() - 5" not reflect the last 5 seconds?

Edit 3: I had my SWING_DAMAGE function print the timeStamp, and time(), and there is a 12 second disparity between them. Is this something that will ALWAYS be the same? Is it some kind of bug?

Edit 4: For now, I will just use time() as the key, rather than relying on timeStamp. Seems to be working well.

My code needs to be able to handle SWING_DAMAGE, SPELL_DAMAGE, and SPELL_PERIODIC_DAMAGE. Only SWING_DAMAGE has "amount" seated at the 12th spot of ..., SPELL_DAMAGE and SPELL_PERIODIC_DAMAGE have "amount" seated at the 15th spot of ..., so I think I need to keep it the way it is. Let me know what you think.

The first 11 never change, and swingAmount or spellAmount would be correct so long as they are called for the right eventType. I don't know how much of an impact this would have, just an idea.

This is what I've got now. I've had to make some changes to the way information is stored. Events are stored into the table with an index that increments for each time the event fires, and a nested table that holds the time and amount, that way if two events happen at the EXACT same time, one isn't written in, then overwritten.

Also, the new damageTable format you're using is going to generate a large amount of garbage, since creating new tables is relatively costly in terms of memory. Since there is no advantage in your case to using an indexed table (sequential integer keys) over a hash table (keys can be anything) -- especially since you are nil-ing out keys, which puts holes in the table and prevents the use of any index-related functions like ipairs or table.sort on the table -- you should just use your original damageTable[time()] = amount format instead.

Also, the way you're detecting NPCs seems rather inefficient, with two function calls, a string creation, and a modulus operation. Why not use a single bit.band comparison on the sourceFlags argument instead? It'll be much faster, especially if you upvalue bit.band and the specific flag value constants you're interested in.

Code:

if bit.band(sourceFlags, COMBATLOG_OBJECT_TYPE_NPC) > 0 then
-- source is an NPC
end

The answer to your "why not..." is because I have no idea what bit.band is >.<.

I'll certainly look into every single one of your suggestions!

Also, the reason I had to change the way I am storing information, is because if two evens happen at the exact same time, they'll return the same integer from time(), so the first event will be written in, and then overwritten with the second event, seeing as they have the same key value.

I chose to just increment some integer to use as the key just to ensure that there would always be a unique key, no matter what the values were.

Is that making sense? That's what was happening in my testing at least. I was able to find a pack of wolves in Vale of Eternal Blossoms that ALL cast the SAME spell at the exact same time, and it would print(debugging I had setup) all of their spells, but it would only recognize one in the table(the damage varied by ~3,000). I assumed this was because they key was the same for all of them, and that the value just took on the last one to register.

Thank you so much for your help!

Here is what I have gotten my combat log parsing function to. I've tried to heed all suggestions, seems a lot cleaner than when I first started.

I upsourced bit.band as bit_band (was throwing an error about having a "." when I tried up upsource as local bit.band = bit.band, but I found some post you made where it was bit_band ;])

The answer to your "why not..." is because I have no idea what bit.band is >.<.

Bitwise operators allow lots of information to be stored in a single bitflag. For example, the same flag (it's a hexadecimal number, and looks like 0x12A3B4C5D6E7F89 or something) shown in the combat log for a unit can tell you whether the unit is a player, pet, totem, guardian, NPC, or object; whether it's controlled by a player or not; how it reacts to the player; whether it's in the player's group or not; etc.

In simple terms, bit.bor combines bitflags (for example, you'd use it to create a bitflag describing a server-controlled NPC that's hostile to you) and bit.band lets you determine whether a bitflag includes another bitflag (in your case, the NPC-controlled flag).

Blizzard uses these operators in their combat log code.

Originally Posted by Clamsoda

... if two evens happen at the exact same time, they'll return the same integer from time(), so the first event will be written in, and then overwritten with the second event, seeing as they have the same key value.

You could save the value in ms instead, and just add 1 if it's a duplicate:

Code:

-- Add an event:
local now = time() * 1000
if damageTable[now] then
now = now + 1
end
damageTable[now] = amount
-- Loop over the events:
local earliest = (time() - 5) * 1000
...

Alternatively, you could use two flat indexed tables:

Code:

local damageTimes, damageAmounts = {}, {}
-- Add an event:
damageTimes[#damageTimes+1] = time()
damageAmounts[#damageAmounts+1] = amount
-- Loop over the times, add up relevant values, remove irrelevant ones:
local i, total, earliest = 1, 0, time() - 5
while i <= #damageTimes do
if damageTimes[i] >= earliest then
-- It's within 5 seconds; add it to the total.
total = total + damageAmounts[i]
-- Move on to the next record.
i = i + 1
else
-- It's old; remove it from both tables.
tremove(damageTimes, i)
tremove(damageAmounts, i)
-- Don't increment the index; the next record just moved up into the current index.
end
end

As a third alternative, you could use subtables as you are now, but recycle them so you're not constantly creating new tables:

Code:

local GetTable, ReleaseTable
do
local pool = {}
function newTable()
local t = next(pool)
if t then
pool[t] = nil
return t
end
return {}
end
function delTable(t)
-- This next line is specifically tailored to your code; if you're reusing
-- this function in some other code, change it to wipe(t) or something.
t[1], t[2] = nil, nil
pool[t] = true
end
end
-- Add an event:
local t = newTable()
t[1] = time()
t[2] = amount
damageTable[#damageTable+1] = t
-- Loop over the events, add up relevant values, remove irrelevant ones:
local total, earliest = 0, time() - 5
for i = #damageTable, 1, -1 do
local t = damageTable[i]
if t[1] >= earliest then
total = total + t[2]
else
-- Remove it from the event table and put it in the pool:
delTable(tremove(damageTable, i))
end
end

Originally Posted by Clamsoda

I upsourced bit.band as bit_band (was throwing an error about having a "." when I tried up upsource as local bit.band = bit.band, but I found some post you made where it was bit_band ;])

Yep; variable names can only contain letters, numbers, and underscores. Replacing spaces and periods with underscores is generally the common practice, like using an underscore to represent a "junk" variable in a list of returns.

Originally Posted by Clamsoda

Here is what I have gotten my combat log parsing function to.

I'd suggest registering CLEU only after PLAYER_LOGIN fires. I'm not sure when CLEU starts firing, but if it does fire before PLAYER_LOGIN it's just wasting CPU cycles since you don't know the player's GUID yet and the player isn't doing anything at that point either.

I wanted to inquire as to whether or not there is already a library that I can pass an amount to, and it will calculate any healing modification relevant to said player (Vampiric Blood, Mortal Strike, Guardian Spirit etc.), and it will return the value once any spell has modified it.

I'd rather not re-invent the wheel here. I can, and already have accounted for Scent of Blood, so that isn't necessary. Also, I was curious if anyone is aware as to what the priority for healing modifications is.

Edit: Just thinking, perhaps there is some way to spoof UnitGetIncomingHeals(), and let WoW do the work for me? Or possibly the UNIT_HEAL_PREDICTION event?

No, because UNIT_HEAL_PREDICTION and UnitGetIncomingHeals only tell you about heals that are already in progress -- eg. spells in the process of being cast, or active HoT effects that are ticking. The healing from Death Strike happens instantly when you use the ability. There's no cast time and no ongoing effect, and no way for the game to predict when you're about to use Death Strike.

I'm also not aware of any current library for calculating the healing someone will receive... LibResComm did this prior to Cataclysm when Blizzard added the incoming heals API, but at this point it's so out of date it would be more work to update than just to write your own code tailored to your own specific needs (tracking the player's own self-healing).

If you want a list of current debuffs that reduce/prevent healing received, check my addon GridStatusHealingReduced. I can also send you a spreadsheet with more detailed info about the debuffs it tracks, such as the actual amount of the reduction, if you want. I don't have a list of buffs that increase healing received, though.

On the other hand, I'm not really sure it's worthwhile to spend all this extra time tracking all these auras and doing all this math for such a simple addon.

On the other hand, I'm not really sure it's worthwhile to spend all this extra time tracking all these auras and doing all this math for such a simple addon.

You may have a point there, and the AddOn's relevance only reflects that of Throne of Thunder, with our raid comp (for now that is). I was more so interested in a library to do the heavy hauling, rather than writing my own or something like it.

For now, Scent of Blood and Vampiric Blood (glyphed) are factored in. I'll come up with something for Hard Stare and Horridon's debuff, but other than those the AddOn works perfectly, and consistently predicts the amount of healing with very low occurrences of error, and those errors tend to be 1,000 or 2,000.

I ended up going with two indexed tables, by the way. The "milisecond + 1" approach only worked if there were two events at the same time, if there was a third event, it would over write the second event and so forth. I am pretty happy with it for now, it was an awesome learning process.