I am both happy and suspicious there have been no complaints/bugs in the past few days, provided no bug reports I'm aiming for a stable tag next Friday.
Er... you're right to be suspicious. Here are some more feedback about minor 16:
1) I respecced my druid to full heal yesterday (had a 24/0/47 pvp heal spec before that). Now that I have Wild Growth again, I'm noticing HealComm-4.0 does not support it at all. Is it intended ?
2) I noticed an issue with the final tick of HoTs. I verified it with Rejuvenation, Regrowth and Lifebloom (ignoring bomb_heals) but I think it applies to other HoTs as well.
I used to update my display on HC callbacks using a 3.0 seconds time band (GetTime() + 3.0). The final tick was very often missing, e.g. GetHealAmount returns nil 3 seconds before the end of the HoT.
Suspecting some time boundary issue, I increased the time band to 3.2 seconds. Now it sometimes counted 2 ticks at once (e.g. returning 3k instead of 1,5k) which makes sense tough I do not clearly understand why it didn't do it consistently. Anyway, the final tick was still missing.
I finally changed my code to poll incoming heals every 0.5 seconds, using both 3.0 seconds and 3.2 seconds time bands. I obtained exactly the same results as above: the final tick is missing.
Given my tests, I think the final tick is expected a short time after HoT expiration so HC4 cuts it off.
3) Just noticed this in the code:
local ALL_DATA = 0x0f
local DIRECT_HEALS = 0x01
local CHANNEL_HEALS = 0x02
local HOT_HEALS = 0x04
local ABSORB_SHIELDS = 0x08
local BOMB_HEALS = 0x16
Shouldn't this be :
local ALL_DATA = 0x1f -- instead of 0x0f
local DIRECT_HEALS = 0x01
local CHANNEL_HEALS = 0x02
local HOT_HEALS = 0x04
local ABSORB_SHIELDS = 0x08
local BOMB_HEALS = 0x10 -- instead of 0x16
ALL_DATA and ABSORB_SHIELDS shouldn't be in yet, I commented those back out but it does look like it could be missing the final tick so I'll look into it more and get it fixed. The math to figure out ticks is a little bit annoying and probably needs to be changed to use fuzzy math a little so instead of it having to be exactly in 3 seconds it's 3 seconds +/- 0.10 seconds kind of thing to account for lag.
I still think BOMB_HEALS shouldn't be 0x16 but either 16 or 0x10. It makes more sense with regard to bit masks.
For the final tick, I have to make clear that if Rejuvenation ends at GetTime()+2, both :GetHealAmount(guid, ALL_HEALS, GetTime()+3) and :GetHealAmount(guid,ALL_HEALS) returns nil. That's not related to the time band. I'll try to hack filterData a bit to see if I could find something.
I obtain correct results with the following code for calculating HoT amount:
local tickInterval = pending.tickInterval
local secondsLeft = endTime - currentTime
if not time then
healAmount = healAmount + amount * math.ceil(secondsLeft / tickInterval)
else
local bandSeconds = math.min(time - currentTime, secondsLeft)
-- Count every full intervals included in the time band
healAmount = healAmount + amount * math.floor(bandSeconds / tickInterval)
-- Fuzzy math here
local nextTickIn = secondsLeft % tickInterval
local fractionalBandSeconds = bandSeconds % tickInterval
if nextTickIn > 0 and nextTickIn <= fractionalBandSeconds then
healAmount = healAmount + amount
end
end
Basically, the idea is that if your time band is 4 seconds and the tick interval is 3 seconds, the next tick should be counted if it occurs within 1 seconds (4 modulo 1). This also fix the finaly tick issue.
Hrm... I didn't test it with not time though. Edit: I'll write unit tests to verify it.
Wild Growth is something I'll add after I get this working correctly, as I'm probably going to have to go with some table recycling code and I'll want to redo some other parts to take advantage of recycling if I am. As well as the fact that I need to look into the Wild Growth formula a bit more for calculating each tick.
The code that is currently being used should work fine it just needs to account for the final tick which is a quick fix. But the problem with how you are testing it is you're assuming it's going to tick in exact intervals which it only does if you have no other hots or heals being cast. Just pulling the example from the comments:
This is necessary to make sure we get the correct amount of ticks.
Here's what would happen if we assume that Lifebloom will tick exactly every 1 second from the time the function is called:
1s = tick
2s = tick
Hit time band, only ticking twice in 2.30 seconds.
When in reality Lifebloom is ticking 0.20s ahead of the call, so it's really:
0.20s = tick
1.20s = tick
2.20s = tick
Hit time band, ticked three times in 2.30 seconds.
This comes up when you have multiple hots triggering healing updates, Rejuvenation can fire an update then 0.50s later Lifebloom ticks and so on.
Assuming your time band is 2.30 seconds and the lifebloom ends in 4.20 seconds (hence the 0.20 seconds shift), my code would give:
* 2 full ticks using math.floor(bandSeconds / tickInterval) = math.floor(2.3/2) = 2
* 1 additional tick because:
Is it intentional that BOMB_HEALS is 0x16 instead of 0x10? Currently, BOMB_HEALS is equal to bit.bor(0x10, HOT_HEALS, CHANNEL_HEALS), so if you specify BOMB_HEALS, you also pull in the HOT_HEALS and CHANNEL_HEALS. I think it would be wise to make sure the individual categories are orthogonal...
It assumes the HoT starts at time 0 and displays the number of ticks at pseudo-random times, as calculated by each function.
The one problem with your math.ceil on line #27 is that you will have the same issue I had that made me force it to use only the first two decimal places, where a hot can tick ahead of what the client thinks it should. For example Rejuvenation has 3 seconds left, so two ticks. It ticks at 3s left but due to lag and such the game thinks the HoT has 3.1 seconds left, ceiling it then makes it think it has 2 ticks left when it's actually 1 left.
I'll look at adding the code in the morning.
I don't see the advantage of doing floor(x/y)+1 instead of ceil(x/y). In the first case, if x == y you get 2 where you get only 1 on the second case. Are you wanting to counts 2 ticks when the next tick should happen "right now" ? (which actually is a corner case as it is quite unlikely to happen due to lag and all that fun stuff).
I could add the rounding to my code anyway, that wouldn't change the result so much.
BTW, you should look at the results of the test code. It appears that your "next tick" calculation sometimes returns a negative value, leading to missing tick.
You are reading what I said wrong, this is what I mean can happen over 15 seconds of Rejuvenation at 3 seconds per tick/5 ticks. Even if it's a corner case I was able to get it to occur a few times while randomly testing at 100 MS latency.
#4 already happened, but due to client lag it the library will think it's actually going to happen in 0.07 seconds, without subtracting 0.10 and rounding it will think that it has 3,000 incoming because it thinks there are two ticks left on the cast not one.
Flooring + 1 the results is so if you have a 6 second time band, and 5.95 seconds left it will see that there is tick #5 and #4 left, not #3 - #5.
The next tick is below zero sometimes because your debug code is ahead of where it stops it from being at line #13 :p it never actually uses the nextTickIn variable beyond changing it to local seconds. I'll look into it in a bit need to get food first.
Latest push has your version, there was a bug I got where it was still managing to double up ticks but I can't seem to replicate it, but it happens for the entire duration from start to end at a 4s time band on Rejuvenation.
This could happen if the display or tick occured 1 second before the next theoretical tick (e.g. ticking at 2s when the next tick should be at 3s) but that seems highly unlikely to happen.
Hrm, probably have to come up with a better way of doing this somehow, but I am not sure what yet, but both methods don't seem to be 100% which is what I want.
Small thing, the toc file has the wrong path and spelling for CallbackHandler-1.0, so it can't load unless something else has pulled in CallbackHandler-1.0.
Doing some small-scale testing and found a small issue. auto self cast is off.
1) target an NPC
2) clear the target
3) cast heal
4) hold mouse over another NPC
5) /targetlasttarget
It thinks you're casting on the other NPC in this case. Trying the same with Renew and nothing happens (works fine when casted on self though).
This was on a level 17 priest, and i also found it a bit odd that heal rank 1 is reported to heal for 307, when its heal range is 297-344 (no talents and no spell power). The same was true for the other spells.
Why not updating HoT timing data on SPELL_PERIODIC_HEAL to fix time drift ?
Edit: BTW, I think you neglected Druids' Nature's Splendor in CalculateHotHealing().
I just made some test on a laggy server (Dalaran just before raiding), I had ticks up to 5 seconds *after* the buffs should have faded out.
That's what I'm starting to think might be best, something perhaps like:
When the hot is initially cached and the library pulls stack/end time info it will also pull duration and figure out how many times it's going to tick.
Each tick will set a last tick variable to an exact number, so something like:
Doing some small-scale testing and found a small issue. auto self cast is off.
1) target an NPC
2) clear the target
3) cast heal
4) hold mouse over another NPC
5) /targetlasttarget
It thinks you're casting on the other NPC in this case. Trying the same with Renew and nothing happens (works fine when casted on self though).
This was on a level 17 priest, and i also found it a bit odd that heal rank 1 is reported to heal for 307, when its heal range is 297-344 (no talents and no spell power). The same was true for the other spells.
Looks like it might have been a priority issue which I pushed a fix for, but I won't be able to test it for another few hours. Heal (Rank 1) is listed as 295 - 314 rather than 295 - 341 internally so that will be fixed in next push.
TOC typo is also fixed.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
1) I respecced my druid to full heal yesterday (had a 24/0/47 pvp heal spec before that). Now that I have Wild Growth again, I'm noticing HealComm-4.0 does not support it at all. Is it intended ?
2) I noticed an issue with the final tick of HoTs. I verified it with Rejuvenation, Regrowth and Lifebloom (ignoring bomb_heals) but I think it applies to other HoTs as well.
I used to update my display on HC callbacks using a 3.0 seconds time band (GetTime() + 3.0). The final tick was very often missing, e.g. GetHealAmount returns nil 3 seconds before the end of the HoT.
Suspecting some time boundary issue, I increased the time band to 3.2 seconds. Now it sometimes counted 2 ticks at once (e.g. returning 3k instead of 1,5k) which makes sense tough I do not clearly understand why it didn't do it consistently. Anyway, the final tick was still missing.
I finally changed my code to poll incoming heals every 0.5 seconds, using both 3.0 seconds and 3.2 seconds time bands. I obtained exactly the same results as above: the final tick is missing.
Given my tests, I think the final tick is expected a short time after HoT expiration so HC4 cuts it off.
3) Just noticed this in the code:
Shouldn't this be :
For the final tick, I have to make clear that if Rejuvenation ends at GetTime()+2, both :GetHealAmount(guid, ALL_HEALS, GetTime()+3) and :GetHealAmount(guid,ALL_HEALS) returns nil. That's not related to the time band. I'll try to hack filterData a bit to see if I could find something.
Basically, the idea is that if your time band is 4 seconds and the tick interval is 3 seconds, the next tick should be counted if it occurs within 1 seconds (4 modulo 1). This also fix the finaly tick issue.
Hrm... I didn't test it with not time though. Edit: I'll write unit tests to verify it.
What about Wild Growth ?
The code that is currently being used should work fine it just needs to account for the final tick which is a quick fix. But the problem with how you are testing it is you're assuming it's going to tick in exact intervals which it only does if you have no other hots or heals being cast. Just pulling the example from the comments:
* 2 full ticks using math.floor(bandSeconds / tickInterval) = math.floor(2.3/2) = 2
* 1 additional tick because:
So it properly counts 3 ticks in this case without involving a loop.
Actually the same code would work when no time band is provided by assuming bandSeconds = secondsLeft.
I didn't pull that code out of my magic hat and I considered several cases, not just the obvious ones.
http://paste.wowace.com/1133/
It assumes the HoT starts at time 0 and displays the number of ticks at pseudo-random times, as calculated by each function.
The one problem with your math.ceil on line #27 is that you will have the same issue I had that made me force it to use only the first two decimal places, where a hot can tick ahead of what the client thinks it should. For example Rejuvenation has 3 seconds left, so two ticks. It ticks at 3s left but due to lag and such the game thinks the HoT has 3.1 seconds left, ceiling it then makes it think it has 2 ticks left when it's actually 1 left.
I'll look at adding the code in the morning.
I could add the rounding to my code anyway, that wouldn't change the result so much.
BTW, you should look at the results of the test code. It appears that your "next tick" calculation sometimes returns a negative value, leading to missing tick.
15s - Initial application (1.5k incoming)
#1 - 11.98s - tick (1.5k)
#2 - 8.90 - tick (1.5k)
#3 - 5.95 - tick (1.5k)
#4 - 3.07 - tick (3k)
#5 - 0s - tick (0)
#4 already happened, but due to client lag it the library will think it's actually going to happen in 0.07 seconds, without subtracting 0.10 and rounding it will think that it has 3,000 incoming because it thinks there are two ticks left on the cast not one.
Flooring + 1 the results is so if you have a 6 second time band, and 5.95 seconds left it will see that there is tick #5 and #4 left, not #3 - #5.
The next tick is below zero sometimes because your debug code is ahead of where it stops it from being at line #13 :p it never actually uses the nextTickIn variable beyond changing it to local seconds. I'll look into it in a bit need to get food first.
Edit:
BTW, I think you neglected Druids' Nature's Splendor in CalculateHotHealing().I just made some test on a laggy server (Dalaran just before raiding), I had ticks up to 5 seconds *after* the buffs should have faded out.
1) target an NPC
2) clear the target
3) cast heal
4) hold mouse over another NPC
5) /targetlasttarget
It thinks you're casting on the other NPC in this case. Trying the same with Renew and nothing happens (works fine when casted on self though).
This was on a level 17 priest, and i also found it a bit odd that heal rank 1 is reported to heal for 307, when its heal range is 297-344 (no talents and no spell power). The same was true for the other spells.
That's what I'm starting to think might be best, something perhaps like:
When the hot is initially cached and the library pulls stack/end time info it will also pull duration and figure out how many times it's going to tick.
Each tick will set a last tick variable to an exact number, so something like:
totalTicks = duration / tickInterval
startTime = endTime - duration
lastTick = startTime + tickInterval
Then all calculations for when the last tick/when next tick is coming would be based off of the lastTick number which would fix the time drifting.
Looks like it might have been a priority issue which I pushed a fix for, but I won't be able to test it for another few hours. Heal (Rank 1) is listed as 295 - 314 rather than 295 - 341 internally so that will be fixed in next push.
TOC typo is also fixed.