I've got pretty much all of the code/api/event/documentation done at this point, I need to go through and verify it's all 100% and then I'll write the wrapper and it should be ready for testing.
The API looks good. However, for GetHealAmount doesn't seem possible to get the amount of healing incoming to a target from everyone else but yourself before your currently casting heal lands (which is kind of the core functionality used by VisualHeal and the unit frame addons that properly show these as separate, and only show the incoming heals from others that land before your own). For example, if i know that my own heal will land at the time in endTime i will call GetHealAmount like this:
However, there's no way to know if the heal I'm currently casting will be included in that, because the timeframe ends exactly on the point where my heal lands. I guess it depends on both rounding errors and whether you use > or >= for comparison, or other subtle things, so it's not possible to subtract the players heal size from the value returned. Regardless, from an API point of view, it should be possible to get the heal amount without the player contribution. LHC-3.0 never returned the heal from yourself in this call, and relied on the addon to remember that value by itself whenever a heal was active (which is just a matter of saving the amount in a local variable whenever a heal start event is triggered with the player as the healer).
Apart from that, i think everything looks good. Perhaps you want to reconsider exposing the internal tables, because when they are exposed you get locked down in how they are laid out, because changing them is an API change.
The API looks good. However, for GetHealAmount doesn't seem possible to get the amount of healing incoming to a target from everyone else but yourself before your currently casting heal lands (which is kind of the core functionality used by VisualHeal and the unit frame addons that properly show these as separate, and only show the incoming heals from others that land before your own). For example, if i know that my own heal will land at the time in endTime i will call GetHealAmount like this:
However, there's no way to know if the heal I'm currently casting will be included in that, because the timeframe ends exactly on the point where my heal lands. I guess it depends on both rounding errors and whether you use > or >= for comparison, or other subtle things, so it's not possible to subtract the players heal size from the value returned. Regardless, from an API point of view, it should be possible to get the heal amount without the player contribution. LHC-3.0 never returned the heal from yourself in this call, and relied on the addon to remember that value by itself whenever a heal was active (which is just a matter of saving the amount in a local variable whenever a heal start event is triggered with the player as the healer).
Apart from that, i think everything looks good. Perhaps you want to reconsider exposing the internal tables, because when they are exposed you get locked down in how they are laid out, because changing them is an API change.
If you wanted to get your own healing, you'd pass your GUID as casterGUID or you could do what Pitbull and agUF do which is when it sees a players heal start/delayed/stopped through the callbacks it saves when the players heal is going to end and requests all heals within that time period.
Besides that for getting the end time it uses the same method 3.0 did with getting the end time through UnitCastingInfo/UnitChannelInfo.
In HealComm-3.0:
-- Add in the players own heals
local healed = playerHeals[target] or 0
-- Add in any heals from other people
local incoming = select(2, HealComm:UnitIncomingHealGet(target, 0))
if( incoming ) then
healed = healed + incoming
end
-- Apply any healing debuffs
healed = math.floor(healed * HealComm:UnitHealModifierGet(target))
Would be done something like the below in HealComm-4.0
If you needed to get both the total healing incoming and how much the player did.
That's true, the only reason the caster table was exposed is so people can see who has casted a heal or not, but I'll just replace that with a GUIDHasHealed called. The GUID -> Unit table will probably always be there/not changed unless Blizzard decides to let us get units from GUIDs.
Hmm, i'm not sure you understand the problem. I want two quantites:
1) The heal that is incoming *only* from others *before* my heal lands.
2) The heal that is incoming from me.
2 is trivial, but 1 is not with the current API. Calling this:
local restHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS) - playerHealed
Will give me all heals that finish before and after mine, because there is no time frame given. I need those that land before mine only. Therefore, the extra argument is added:
local restHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTimeOfCurrentlyCastingPlayerSpell) - playerHealed
However, as the boundary time is exactly the same time as the player spell lands, it is not possible to know if the currently casting player spell is or is not included in the quantity returned from GetHealAmount.
You should've looked at how LHC4 did it first :p. The way it does it now is
-- Snipped --
It is not doing it the way you described. The new entries in the upgraded version flat out never get added.
Also, AuraHandler should be something like below
if AuraHandler then AuraHandler(...) end
What Xinhuan said is correct, in r1 it will initialize the table and then in r2 onward it will do nothing, if a new entry is added then it's added directly.
Hmm, i'm not sure you understand the problem. I want two quantites:
1) The heal that is incoming *only* from others *before* my heal lands.
2) The heal that is incoming from me.
2 is trivial, but 1 is not with the current API. Calling this:
local restHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS) - playerHealed
Will give me all heals that finish before and after mine, because there is no time frame given. I need those that land before mine only. Therefore, the extra argument is added:
local restHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTimeOfCurrentlyCastingPlayerSpell) - playerHealed
However, as the boundary time is exactly the same time as the player spell lands, it is not possible to know if the currently casting player spell is or is not included in the quantity returned from GetHealAmount.
Note the "Incoming heals from others that will complete before yours"
Perhaps instead an API like:
local healed = HealComm:GetOtherHealAmount(guid, bitFlag, endTime, casterGUID)
that will return all heals within the time band (if any) not counting the players heal, if you wanted to get the players heals + everyone else's before yours land it would be:
local playerHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTime, UnitGUID("player"))
local beforePlayer = HealComm:GetOtherHealAmount(guid, HealComm.ALL_HEALS, endTime)
endTime would be something you kept track of through the heal events to figure out the longest player heal, it would look something like:
function HealComm:GetOtherHealAmount(guid, bitFlag, time)
local amount = 0
for casterGUID, spells in pairs(pendingHeals) do
if( casterGUID ~= playerGUID ) then
amount = amount + filterData(spells, guid, bitFlag, time)
end
end
return amount > 0 and amount or nil
end
You should've looked at how LHC4 did it first :p. The way it does it now is
<snip>
It is not doing it the way you described. The new entries in the upgraded version flat out never get added.
Yes, if it did that then it was flawed upgrading code. This is why for libraries, authors are encouraged to let as many people review the code as possible before letting a beta version out for testing.
What Xinhuan said is correct, in r1 it will initialize the table and then in r2 onward it will do nothing, if a new entry is added then it's added directly.
Oops forgot that it uses UNIT_AURA for classes without data, fixed.
Perhaps instead an API like:
local healed = HealComm:GetOtherHealAmount(guid, bitFlag, endTime, casterGUID)
that will return all heals within the time band (if any) not counting the players heal, if you wanted to get the players heals + everyone else's before yours land it would be:
local playerHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTime, UnitGUID("player"))
local beforePlayer = HealComm:GetOtherHealAmount(guid, HealComm.ALL_HEALS, endTime)
endTime would be something you kept track of through the heal events to figure out the longest player heal, it would look something like:
function HealComm:GetOtherHealAmount(guid, bitFlag, time)
local amount = 0
for casterGUID, spells in pairs(pendingHeals) do
if( casterGUID ~= playerGUID ) then
amount = amount + filterData(spells, guid, bitFlag, time)
end
end
return amount > 0 and amount or nil
end
That'll would work fine. You can also consider just adding a flag to the list of arguments, like this:
Not a bug, but I suggest you check that GetSpellInfo(spellid) exists outside the table constructor to safeguard the lib from breaking on patch days. You never know when blizz will remove a spellid.
Not a bug, but I suggest you check that GetSpellInfo(spellid) exists outside the table constructor to safeguard the lib from breaking on patch days. You never know when blizz will remove a spellid.
It's in as GetOthersHealAmount(guid, bitFlag[, time])
I understand, my suggestions were to avoid adding the extra function, by just providing the extra semantics in an extra parameter instead of in the function name, to avoid cluttering the API.
The wrapper is done and can be found at http://www.wowace.com/addons/libwrapperhealcomm-1-0/ as far as I can tell HealComm-4.0 is ready for beta testing and going to move Shadowed Unit Frames to it to get more testing done.
I understand, my suggestions were to avoid adding the extra function, by just providing the extra semantics in an extra parameter instead of in the function name, to avoid cluttering the API.
It's not that as much as I look at it being clearer to be able to read the function and specifically know what it does, rather than having to remember what a true/false flag does.
2) I noticed the comment "Am I slightly crazy for adding level <40 glyphs? Yes!" there. Since there is no glyph code there, I'm wondering if it's just a missplaced comment or missing code?
3) At the end of the Paladin code, there is a comment for "over 95% crit", but it's coded ">=100".
edit:
4) Infusion of Light talent is not calculated. But I'm not sure if it should be, because the Paladin would need >= 80% crit to make Infusion of Light change the result of the calculation.
5) Same for Sanctified Light talent, but even less likely to push crit over 100.
6) Touched by the Light talent is missing. Will be nerfed soon, but currently many Arena Paladins are using a TbtL spec.
4) Support for buffs/item sets that modify crit is something I'm not sure if I want to do yet, nobody is really going to hit 100% crit without a fight like Loatheb or Divine Favor from normal buff/talents.
4) Support for buffs/item sets that modify crit is something I'm not sure if I want to do yet, nobody is really going to hit 100% crit without a fight like Loatheb or Divine Favor from normal buff/talents.
Shaman have a similar talent/spell called Tidal Force that gives 60/40/20% crit buff to the next three HW/LHW/CH spells.
Raid-buffed, it's likely that the first HW/LHW/CH cast with the 60% buff will be 100% crit, then after that, LHW will likely crit on the remaining 40% and 20% crit buffs.
True, probably only Paladins and Shaman ever hit 100% on a spell for non-Loatheb fights, but it does happen, and it makes a difference in choosing heal targets when you see a heal will land for 15k instead of 10k.
Tidal Force isn't that bad to support since it's one buff, maybe later I'll go through and calculate more of the talents and item sets for crit but I'd rather get hot support/clean up code/etc first before something that doesn't occur normally and only for two of the four healing classes.
Tidal Force isn't that bad to support since it's one buff, maybe later I'll go through and calculate more of the talents and item sets for crit but I'd rather get hot support/clean up code/etc first before something that doesn't occur normally and only for two of the four healing classes.
Totally agreed... HoT support would be much more useful than crit heals in terms of figuring out heal targets.
Speaking of hots, because they are generally 10s-30s long they can't use the normal events that heals do, and the caster of the hot sending a comm message every time each hot ticks would be horribly spammy.
There has to be some sort of callback to indicate a frame needs to redraw due to a heal ticking:
1) Keep track of when each hot/channel is going to tick and do polling in the library that dispatches callbacks every time they would tick
2) Keep track of ticks through CLEU and fire a callback when the library sees a hot tick
3) Add a callback authors can register that indicates a hot/channel has become active and they need to do polling themselves to keep healing up to date.
#1 might have accuracy issues, #2 while more accurate could also result in callbacks being fired at least every 1s maybe 0.50s-0.75s if you had two Druids + misc HoTs running, #3 means a little more work for authors but they can choose the update period they want and I don't think knowing a hot is going to tick with 0.10s-0.20s accuracy is that useful.
I prefer #3, but want to see what others think, keep in mind that when someone casts a direct heal a frame redraw is going to happen regardless.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
Project: http://www.wowace.com/addons/libhealcomm-4-0/
Documentation: http://www.wowace.com/addons/libpendingheals-1-0/pages/api/
I've got pretty much all of the code/api/event/documentation done at this point, I need to go through and verify it's all 100% and then I'll write the wrapper and it should be ready for testing.
HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTime)
However, there's no way to know if the heal I'm currently casting will be included in that, because the timeframe ends exactly on the point where my heal lands. I guess it depends on both rounding errors and whether you use > or >= for comparison, or other subtle things, so it's not possible to subtract the players heal size from the value returned. Regardless, from an API point of view, it should be possible to get the heal amount without the player contribution. LHC-3.0 never returned the heal from yourself in this call, and relied on the addon to remember that value by itself whenever a heal was active (which is just a matter of saving the amount in a local variable whenever a heal start event is triggered with the player as the healer).
Apart from that, i think everything looks good. Perhaps you want to reconsider exposing the internal tables, because when they are exposed you get locked down in how they are laid out, because changing them is an API change.
If you wanted to get your own healing, you'd pass your GUID as casterGUID or you could do what Pitbull and agUF do which is when it sees a players heal start/delayed/stopped through the callbacks it saves when the players heal is going to end and requests all heals within that time period.
Besides that for getting the end time it uses the same method 3.0 did with getting the end time through UnitCastingInfo/UnitChannelInfo.
In HealComm-3.0:
Would be done something like the below in HealComm-4.0
If you needed to get both the total healing incoming and how much the player did.
That's true, the only reason the caster table was exposed is so people can see who has casted a heal or not, but I'll just replace that with a GUIDHasHealed called. The GUID -> Unit table will probably always be there/not changed unless Blizzard decides to let us get units from GUIDs.
1) The heal that is incoming *only* from others *before* my heal lands.
2) The heal that is incoming from me.
2 is trivial, but 1 is not with the current API. Calling this:
Will give me all heals that finish before and after mine, because there is no time frame given. I need those that land before mine only. Therefore, the extra argument is added:
However, as the boundary time is exactly the same time as the player spell lands, it is not possible to know if the currently casting player spell is or is not included in the quantity returned from GetHealAmount.
Maybe this picture explains it better: http://www.wowace.com/addons/visualheal/images/4-heal-bar/
Note the "Incoming heals from others that will complete before yours"
You should've looked at how LHC4 did it first :p. The way it does it now is
r1 will do
and r2 will do
It is not doing it the way you described. The new entries in the upgraded version flat out never get added.
Also, AuraHandler should be something like below
What Xinhuan said is correct, in r1 it will initialize the table and then in r2 onward it will do nothing, if a new entry is added then it's added directly.
So if I add a new spell in r3 it becomes
Oops forgot that it uses UNIT_AURA for classes without data, fixed.
Perhaps instead an API like:
local healed = HealComm:GetOtherHealAmount(guid, bitFlag, endTime, casterGUID)
that will return all heals within the time band (if any) not counting the players heal, if you wanted to get the players heals + everyone else's before yours land it would be:
local playerHealed = HealComm:GetHealAmount(guid, HealComm.ALL_HEALS, endTime, UnitGUID("player"))
local beforePlayer = HealComm:GetOtherHealAmount(guid, HealComm.ALL_HEALS, endTime)
endTime would be something you kept track of through the heal events to figure out the longest player heal, it would look something like:
One more thing, I think you should register CLEU, CHAT_MSG_ADDON, UNIT_AURA, and the rest for healers only when the player is in a group.
Yes, if it did that then it was flawed upgrading code. This is why for libraries, authors are encouraged to let as many people review the code as possible before letting a beta version out for testing.
Especially lib upgrading/reembedding code.
That'll would work fine. You can also consider just adding a flag to the list of arguments, like this:
GetHealAmount(guid, bitFlag[, time[, includePlayer[, casterGuid]]])
Or perhaps a HealComm.PLAYER_HEALS bit ? Dunno if that would create some other consistency problems though. You decide!
Fixed that
CLEU isn't supposed to be registered regradless for non-healers, the other two are no longer registered for non-healers till grouped.
It's in as GetOthersHealAmount(guid, bitFlag[, time])
I understand, my suggestions were to avoid adding the extra function, by just providing the extra semantics in an extra parameter instead of in the function name, to avoid cluttering the API.
It's not that as much as I look at it being clearer to be able to read the function and specifically know what it does, rather than having to remember what a true/false flag does.
1) Missing libram: http://www.wowhead.com/?item=28592
2) I noticed the comment "Am I slightly crazy for adding level <40 glyphs? Yes!" there. Since there is no glyph code there, I'm wondering if it's just a missplaced comment or missing code?
3) At the end of the Paladin code, there is a comment for "over 95% crit", but it's coded ">=100".
edit:
4) Infusion of Light talent is not calculated. But I'm not sure if it should be, because the Paladin would need >= 80% crit to make Infusion of Light change the result of the calculation.
5) Same for Sanctified Light talent, but even less likely to push crit over 100.
6) Touched by the Light talent is missing. Will be nerfed soon, but currently many Arena Paladins are using a TbtL spec.
2) That's just a typo, supposed to say relics
3) Oops missed that one
4) Support for buffs/item sets that modify crit is something I'm not sure if I want to do yet, nobody is really going to hit 100% crit without a fight like Loatheb or Divine Favor from normal buff/talents.
5) Same as above
6) Added
4,5) Ok, I agree.
1,3,6) Thanks!
Shaman have a similar talent/spell called Tidal Force that gives 60/40/20% crit buff to the next three HW/LHW/CH spells.
Raid-buffed, it's likely that the first HW/LHW/CH cast with the 60% buff will be 100% crit, then after that, LHW will likely crit on the remaining 40% and 20% crit buffs.
True, probably only Paladins and Shaman ever hit 100% on a spell for non-Loatheb fights, but it does happen, and it makes a difference in choosing heal targets when you see a heal will land for 15k instead of 10k.
Totally agreed... HoT support would be much more useful than crit heals in terms of figuring out heal targets.
There has to be some sort of callback to indicate a frame needs to redraw due to a heal ticking:
1) Keep track of when each hot/channel is going to tick and do polling in the library that dispatches callbacks every time they would tick
2) Keep track of ticks through CLEU and fire a callback when the library sees a hot tick
3) Add a callback authors can register that indicates a hot/channel has become active and they need to do polling themselves to keep healing up to date.
#1 might have accuracy issues, #2 while more accurate could also result in callbacks being fired at least every 1s maybe 0.50s-0.75s if you had two Druids + misc HoTs running, #3 means a little more work for authors but they can choose the update period they want and I don't think knowing a hot is going to tick with 0.10s-0.20s accuracy is that useful.
I prefer #3, but want to see what others think, keep in mind that when someone casts a direct heal a frame redraw is going to happen regardless.