[11740] spellmodifier - reset at spell cancel

[rowman]

i tried to understand the "spell modifier system"

I think there's a bug:

after canceling a spell the spellmods (changed at the start in applyspellmods) are never reseted

due to this failure, started and later on canceled casts use buffs like "clearcasting" although they're never finished

so the buff is "used" but not removed

dont know whether i understood this correctly

may this fix help ?

[== c++ ==]
diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index 4ea97f9..818e079 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -17158,6 +17158,25 @@ void Player::RemoveSpellMods(Spell const* spell)
    }
}

+void Player::ResetSpellModsDueToCanceledSpell (Spell const* spell)
+{
+    for(int i=0;i<MAX_SPELLMOD;++i)
+    {
+        for (SpellModList::const_iterator itr = m_spellMods[i].begin(); itr != m_spellMods[i].end()
+        {
+            SpellModifier *mod = *itr;
+            ++itr;
+            if (mod && mod->charges == -1 && (mod->lastAffected == spell))
+            {
+                mod->charges += 2;
+                mod->lastAffected = NULL;
+                if (m_SpellModRemoveCount > 0)
+                    --m_SpellModRemoveCount;
+            }
+        }
+    }
+}
+
// send Proficiency
void Player::SendProficiency(ItemClass itemClass, uint32 itemSubclassMask)
{
diff --git a/src/game/Player.h b/src/game/Player.h
index d384a69..f1bcce4 100644
--- a/src/game/Player.h
+++ b/src/game/Player.h
@@ -1465,6 +1465,7 @@ class MANGOS_DLL_SPEC Player : public Unit
        template <class T> T ApplySpellMod(uint32 spellId, SpellModOp op, T &basevalue, Spell const* spell = NULL);
        SpellModifier* GetSpellMod(SpellModOp op, uint32 spellId) const;
        void RemoveSpellMods(Spell const* spell);
+        void ResetSpellModsDueToCanceledSpell (Spell const* spell);

        static uint32 const infinityCooldownDelay = MONTH;  // used for set "infinity cooldowns" for spells and check
        static uint32 const infinityCooldownDelayCheck = MONTH/2;
diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index 74000ef..aaff825 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -2639,6 +2641,8 @@ void Spell::cancel()
        {
        } break;
    }
+    if (m_caster->GetTypeId() == TYPEID_PLAYER && !IsChanneledSpell(m_spellInfo))
+        ((Player*)m_caster)->ResetSpellModsDueToCanceledSpell (this);

    finish(false);
    m_caster->RemoveDynObject(m_spellInfo->Id);

[rowman]

1 mistake fixed

(

Patch also fixes the "instant cast" Bug

(instant never used)

[rowman]

anyone out there ?

[VladimirMangos]

Nice catch of problem. Fix make sense with 2 change:

1) need call for any finish(false), so in Spell::finish call

2) i think same way restore must be applied to >1 charges spellmod case

when charge must restored.

I currently test version with this changes for mangos master.

[VladimirMangos]

In [11740] with described and some other changes. Thank you

It will be backported to One at some moment.

[rowman]

isnt lastAffected only set if charge == 0 ?

so you never get the chance apply restore in charges > 1 cases ?

(in order to your second change)

[VladimirMangos]

i modify code in applied version for set at any spelmod charge use, this not affect original way use lastAffected. I only not sure at 100% that impossible cases when while one normal spell castes another also use this spellmod charge. In this cases one from 2 used charges can be lost at cancel. Ofc, possible create array with slot per charge for store specific charge spell casts, but i don't want adding more structures if not real cases for this need.

[rowman]

oh i've overlooked your changes in applyspellmods