[patch] Apply haste rating to channeled spells

[Auntie Mangos]

What bug does the patch fix? What features does the patch add?

Apply haste to channeled spells

For which repository revision was the patch created?

9798

Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

http://getmangos.eu/community/showthread.php?11665-Bug-haste-on-channelled-spells for example

Who has been writing this patch? Please include either forum user names or email addresses.

Me

Diff:

V1d: http://pastebin.com/k7LsAHZ2

V1c: http://pastebin.ca/1795313

V1b: http://pastebin.ca/1770596

V1a: http://pastebin.ca/1769835

V1: http://pastebin.ca/1769325

I didnt see any patch for this, so I wrote one by myself, feel free to comment.

[Lynx3d]

Hm i think you risk to lose the last tick with your calculations...integer math will lose the fractional parts on divisions, so be careful.

I think it's better to either recalculate periodic time by the amount of ticks and the new channeling time, or recalculate the channeling time by calculating the new periodic time with haste and multiplying it by the number of ticks.

Like you did it, periodictime*ticks can be greater than duration, which leads to loss of the last tick.

[Wojta]

I tried it with many different haste ratings and I never lost last tick. Yes, its possible, but time is in ms, so even if its by one ms higher, I think its ok.

EDIT: hmm but it can be 1ms/tick higher....so for example blizzard (16 ticks) can really lost one tick...damn your right, I'll try to rewrite this.

[yavi]

Ok...

duration/periodictime = x

x - tick count

newduration=duration*haste

now...

(newduration)/(periodictime*haste) = (duration*haste)/(periodictime*haste) = duration/periodictime = x

So... why exactly do some magical calculactions instead of multiplying both duration and periodictime by haste?

[Wojta]

If you apply full haste to periodic time, ticks will be too short, now its like in tooltip, isnt it?

EDIT:

SPELL: 42846, m_maxduration: 4880, periodic: 976, ticks: 5.000000 and in tooltip is "..every 0.98 for 4.88". I will try just apply haste to periodic time...

EDIT2:

with just haste apply on periodic time its...

SPELL: 42846, m_maxduration: 4880, periodic: 975, ticks: 5.000000

I will try it with blizzard, there should be bigger difference....

EDIT3:

Blizzard

SPELL: 27085, m_maxduration: 7808, periodic: 976, ticks: 8.000000 - just with haste

SPELL: 27085, m_maxduration: 7808, periodic: 975, ticks: 8.000000 - my calculation

Penance

(Tooltip says ".. every 0.98 for 1.95") but thats crazy..penance does not get 3 ticks, only two... what the..? Oh...maybe because penance does not get first instant shot, and...thats caused by my patch...i will fix it

SPELL: 52988, m_maxduration: 1950, periodic: 975, ticks: 2.000000 - my calculation

SPELL: 52988, m_maxduration: 1950, periodic: 975, ticks: 2.000000 - just apply haste

hmm its almost same, but it looks like haste/ticks idea is correct...

EDIT4:

wait, priest on my server says that first tick of penance was never instant....

[ascent]

2>..\\..\\src\\game\\SpellAuras.cpp(447) : error C2146 : ')'is 'm_maxduration' Identifier is not in front.

if(m_spellProto->AttributesEx & (SPELL_ATTR_EX_CHANNELED_1 | SPELL_ATTR_EX_CHANNELED_2) && GetCaster()

m_maxduration = int32(m_maxduration * GetCaster()->GetFloatValue(UNIT_MOD_CAST_SPEED));

problem but..

if(m_spellProto->AttributesEx & (SPELL_ATTR_EX_CHANNELED_1 | SPELL_ATTR_EX_CHANNELED_2) && GetCaster())

m_maxduration = int32(m_maxduration * GetCaster()->GetFloatValue(UNIT_MOD_CAST_SPEED));

change and compile is fine. what's the problem?

[Lynx3d]

Arbitrarily crafted example:

Player has UNIT_MOD_CAST_SPEED value of 0.7294 (that's about 37% haste)

Let's take Blizzard (8000 duration, 1000 periodic time) and your calculations.

duration = int32(8000*0.7294) = 5835

diff = 8000 - 5835 = 2165

diff = int32(2165/8.0) = 270

periodictime = 1000-270 = 730

Now 8*730 is 5840 which is larger than 5835, the last tick would happen 5ms after the aura expired...

[Wojta]

What about that:

V1a: http://pastebin.ca/1769835

[Lynx3d]

Just noticed, this line doesn't make sense to me:

if(m_origDuration - duration != duration)

Don't you just want to check if haste changes the aura duration? Then it should be simply:

if(m_origDuration != duration)

Calculation is still a bit complicated, but it should be numerically stable. I also wonder if "ticks" is ever non-integer...

In any case, since periodictime is what limits the time resolution of the whole haste calculation, so I'd start with something like:

int32 new_periodictime = periodictime * GetCaster()->GetFloatValue(UNIT_MOD_CAST_SPEED)

And i think that if you do what yavi suggested (multiply both with haste factor), new duration can only be slightly larger than periodictime*ticks, never smaller, because in C++ float to int conversion uses truncation.

At least unless people start activating unsafe math optimizations that affect rounding mode...so if you want to be absolutely sure, multiply periodictime with the amount of ticks, but i still have some doubts with using a float to calulate the ticks...

Oh, i noticed another thing. When you do:

float ticks = duration / periodic;

then the division is actually done as integer, since both variables are integer, and after that it is converted to float...so this line will never give fractional results anyway.

[Wojta]

As you suggested V1b: http://pastebin.ca/1770596.

I used

int32 ticks = int32(duration / periodic);

because I did not know if compiler get over this without int32 - result can be float.

[pasdVn]

Thank you for Patch. Nice feature! :-)

PS.:

because I did not know if compiler get over this without int32 - result can be float.

Diving and Integer by anything else does not change it's type, so it still stays an integer and you won't need this typecast, as Lynx3d said. But that are only peanuts^^

[Wojta]

One bug - visual effect (not damage) of blizzard (and Rain of Fire I guess) does not end when caster stop channeling - with my 10% haste (which is not very much), visual effect continue about 1s after end of spell. Not serious bug, but does anybody know how to fix it?

[Wojta]

Another one - when you have for example http://www.wowhead.com/?spell=32182, casting time is too short, for example arcane missiles have 7 ticks instead of 5. I thing its caused by

 
   // Apply periodic time mod
    if(modOwner && m_modifier.periodictime)
        modOwner->ApplySpellMod(GetId(), SPELLMOD_ACTIVATION_TIME, m_modifier.periodictime);

How can I apply it to aura duraion? Simply by " modOwner->ApplySpellMod(GetId(), SPELLMOD_ACTIVATION_TIME, m_modifier.duration);? Or not use this spell mod at all?

EDIT:

Fixed in V1c: http://pastebin.ca/1795313

[Sarjuuk]

i'll take the liberty to bump this. It's pretty important after all^^

0.12 -> http://paste2.org/p/691307

master -> http://paste2.org/p/691345

used V1c as basis, but

- removed unused GetAuraOrigDuration() and SetAuraOrigDuration()

- removed m_origDuration and replaced the one place it got used by a direct call to Unit::CalculateSpellDuration()

- kept ApplyHasteToPeriodic() private

[lu1gui]

Bump this.

Import change.

[Wojta]

V1d: http://pastebin.com/k7LsAHZ2

- updated to 9605

- changes by Sarjuuk, thx

- move

m_duration = m_maxduration;

behind ApplyHasteToPeriodic() in aura constructor, thx reeshack for notice this

[lu1gui]

This is working fine.

[Unkle Nuke]

Link for patch version 1d is broken. Can you please update and maintain?

[Wojta]

Updated to 9798: http://pastebin.com/k7LsAHZ2 (why diff syntax highlight is not working on pastebin.com? )

[Sarjuuk]

just being curious here:

would adding

"if(Player* modOwner = spell->GetCaster()->GetSpellModOwner())

modOwner->ApplySpellMod(spellInfo->Id, SPELLMOD_DURATION, dur, spell);"

to ApplyHasteToChannelSpell() fix spells like this one ? :>

[morderek]

First of all thanks for the patch.

I´m using it in my serverand i found one bug:

http://es.wowhead.com/spell=54490

haste is applied fine but the mage doesn´t cast 5 missiles, they can cast 10-11 missiles(modified by haste)

How to repeat:

Pick an arcane mage

Learn missile barrage

Cast icy veins, arcane power, haste trinkets, etc

cast arcane missiles with missile barrage, the mage cast 11 missiles instead of five

I hope you can understand me (i can upload images)

[Sarjuuk]

well, my change worked (with 0.12)

and since Missle Barrage also uses SPELLMOD_DURATION, there is a good chance, that it will work on master, for this case too.

(note: Pictures of "the Client" aren't allowed here )

[zergtmn]

just being curious here:

would adding

"if(Player* modOwner = spell->GetCaster()->GetSpellModOwner())

modOwner->ApplySpellMod(spellInfo->Id, SPELLMOD_DURATION, dur, spell);"

to ApplyHasteToChannelSpell() fix spells like this one ? :>

Don't work for me =\\