*Fix single target auras related crash

*Apply aura stacking rules to m_ownedAuras holder.

--HG--
branch : trunk

src/game/DynamicObject.cpp

@@ -120,7 +120,8 @@ void DynamicObject::Update(uint32 p_time)
         if (!m_aura->IsRemoved())
             m_aura->UpdateOwner(p_time, this);
 
-        if (m_aura->IsRemoved() || m_aura->IsExpired())
+        // m_aura may be set to null in Unit::RemoveGameObject call
+        if (m_aura && (m_aura->IsRemoved() || m_aura->IsExpired()))
             expired = true;
     }
     else

src/game/SpellAuras.cpp

@@ -302,22 +302,37 @@ Aura * Aura::Create(SpellEntry const* spellproto, uint8 effMask, WorldObject * o
     assert(owner);
     assert(caster || casterGUID);
     assert(effMask <= MAX_EFFECT_MASK);
+    // try to get caster of aura
+    if (casterGUID)
+    {
+        if (owner->GetGUID() == casterGUID)
+            caster = (Unit *)owner;
+        else
+            caster = ObjectAccessor::GetUnit(*owner, casterGUID);
+    }
+    Aura * aura = NULL;
     switch(owner->GetTypeId())
     {
         case TYPEID_UNIT:
         case TYPEID_PLAYER:
-            return new UnitAura(spellproto,effMask,owner,caster,baseAmount,castItem, casterGUID);
+            aura = new UnitAura(spellproto,effMask,owner,caster,baseAmount,castItem, casterGUID);
+            break;
         case TYPEID_DYNAMICOBJECT:
-            return new DynObjAura(spellproto,effMask,owner,caster,baseAmount,castItem, casterGUID);
+            aura = new DynObjAura(spellproto,effMask,owner,caster,baseAmount,castItem, casterGUID);
+            break;
         default:
             assert(false);
             return NULL;
     }
+    // aura can be removed in Unit::_AddAura call
+    if (aura->IsRemoved())
+        return NULL;
+    return aura;
 }
 
 Aura::Aura(SpellEntry const* spellproto, uint8 effMask, WorldObject * owner, Unit * caster, int32 *baseAmount, Item * castItem, uint64 casterGUID) :
 m_spellProto(spellproto), m_owner(owner), m_casterGuid(casterGUID ? casterGUID : caster->GetGUID()), m_castItemGuid(castItem ? castItem->GetGUID() : 0),
-    m_applyTime(time(NULL)), m_timeCla(0),
+    m_applyTime(time(NULL)), m_timeCla(0), m_isSingleTarget(false),
     m_procCharges(0), m_stackAmount(1), m_isRemoved(false), m_casterLevel(caster ? caster->getLevel() : m_spellProto->spellLevel)
 {
     if(m_spellProto->manaPerSecond || m_spellProto->manaPerSecondPerLevel)
@@ -352,8 +367,6 @@ m_spellProto(spellproto), m_owner(owner), m_casterGuid(casterGUID ? casterGUID :
         else
             m_effects[i] = NULL;
     }
-
-    m_isSingleTarget = IsSingleTargetSpell(GetSpellProto());
 }
 
 Aura::~Aura()
@@ -666,6 +679,14 @@ bool Aura::IsVisible() const
     return !IsPassive() || HasEffectType(SPELL_AURA_ABILITY_IGNORE_AURASTATE); 
 }
 
+void Aura::UnregisterSingleTarget()
+{
+    assert(m_isSingleTarget);
+    Unit * caster = GetCaster();
+    caster->GetSingleCastAuras().remove(this);
+    SetIsSingleTarget(false);
+}
+
 void Aura::SetLoadedState(int32 maxduration, int32 duration, int32 charges, uint8 stackamount, uint8 recalculateMask, int32 * amount)
 {
     m_maxDuration = maxduration;
@@ -1302,7 +1323,7 @@ UnitAura::UnitAura(SpellEntry const* spellproto, uint8 effMask, WorldObject * ow
     : Aura(spellproto, effMask, owner, caster, baseAmount, castItem, casterGUID) 
 {
     m_AuraDRGroup = DIMINISHING_NONE;
-    GetUnitOwner()->_AddAura(this);
+    GetUnitOwner()->_AddAura(this, caster);
 };
 
 void UnitAura::_ApplyForTarget(Unit * target, Unit * caster, AuraApplication * aurApp)

src/game/SpellAuras.h

@@ -140,6 +140,7 @@ class TRINITY_DLL_SPEC Aura
         // Single cast aura helpers
         bool IsSingleTarget() const {return m_isSingleTarget;}
         void SetIsSingleTarget(bool val) { m_isSingleTarget = val;}
+        void UnregisterSingleTarget();
 
         void SetLoadedState(int32 maxduration, int32 duration, int32 charges, uint8 stackamount, uint8 recalculateMask, int32 * amount);
 

src/game/Unit.cpp

@@ -3527,9 +3527,54 @@ void Unit::DeMorph()
     SetDisplayId(GetNativeDisplayId());
 }
 
-void Unit::_AddAura(Aura * aura)
+void Unit::_AddAura(UnitAura * aura, Unit * caster)
 {
     m_ownedAuras.insert(AuraMap::value_type(aura->GetId(), aura));
+
+    // passive and Incanter's Absorption and auras with different type can stack with themselves any number of times
+    if (!aura->IsPassive() && aura->GetId() != 44413)
+    {
+        // find current aura from spell and change it's stackamount
+        if (Aura * foundAura = GetOwnedAura(aura->GetId(), aura->GetCasterGUID(), 0, aura))
+        {
+            if(aura->GetSpellProto()->StackAmount)
+                aura->ModStackAmount(foundAura->GetStackAmount());
+
+            // Use the new one to replace the old one
+            // This is the only place where AURA_REMOVE_BY_STACK should be used
+            RemoveOwnedAura(foundAura, AURA_REMOVE_BY_STACK);
+        }
+    }
+    _RemoveNoStackAurasDueToAura(aura);
+
+    if (aura->IsRemoved())
+        return;
+
+    aura->SetIsSingleTarget(caster && IsSingleTargetSpell(aura->GetSpellProto()));
+    if (aura->IsSingleTarget())
+    {
+        // register single target aura
+        caster->GetSingleCastAuras().push_back(aura);
+        // remove other single target auras
+        for (;;)
+        {
+            bool restart = false;
+            Unit::AuraList& scAuras = caster->GetSingleCastAuras();
+            for (Unit::AuraList::iterator itr = scAuras.begin(); itr != scAuras.end(); ++itr)
+            {
+                if( (*itr) != aura &&
+                    IsSingleTargetSpells((*itr)->GetSpellProto(), aura->GetSpellProto()))
+                {
+                    (*itr)->Remove();
+                    restart = true;
+                    break;
+                }
+            }
+
+            if(!restart)
+                break;
+        }
+    }
 }
 
 AuraApplication * Unit::__ApplyAura(Aura * aura)
@@ -3552,10 +3597,6 @@ AuraApplication * Unit::__ApplyAura(Aura * aura)
     AuraApplication * aurApp = new AuraApplication(this, caster, aura);
     m_appliedAuras.insert(AuraApplicationMap::value_type(aurId, aurApp));
 
-    // Register single cast aura
-    if (caster && aura->IsSingleTarget())
-        caster->GetSingleCastAuras().push_back(aurApp);
-
     if(aurSpellInfo->AuraInterruptFlags)
     {
         m_interruptableAuras.push_back(aurApp);
@@ -3568,45 +3609,6 @@ AuraApplication * Unit::__ApplyAura(Aura * aura)
 
     aura->_ApplyForTarget(this, caster, aurApp);
 
-    // passive and Incanter's Absorption and auras with different type can stack with themselves any number of times
-    // auras with type other than TARGET_AURA have CanAuraStack check in their target selection code, so shouldn't go here
-    if (!aura->IsPassive() && aura->GetType() == UNIT_AURA_TYPE && aurId != 44413)
-    {
-        // find current aura from spell and change it's stackamount
-        if (AuraApplication * foundAura = GetAuraApplication(aurId, aura->GetCasterGUID(), 0, aurApp))
-        {
-            if(aurSpellInfo->StackAmount)
-                aura->ModStackAmount(foundAura->GetBase()->GetStackAmount());
-
-            // Use the new one to replace the old one
-            // This is the only place where AURA_REMOVE_BY_STACK should be used
-            RemoveAura(foundAura, AURA_REMOVE_BY_STACK);
-        }
-    }
-
-    // update single target auras list - after aura stack check to allow single target auras to stack
-    if (aura->IsSingleTarget())
-    {
-        for (;;)
-        {
-            bool restart = false;
-            AuraApplicationList& scAuras = caster->GetSingleCastAuras();
-            for (AuraApplicationList::iterator itr = scAuras.begin(); itr != scAuras.end(); ++itr)
-            {
-                if( (*itr)->GetBase() != aura &&
-                    IsSingleTargetSpells((*itr)->GetBase()->GetSpellProto(), aura->GetSpellProto()))
-                {
-                    (*itr)->GetBase()->Remove(AURA_REMOVE_BY_DEFAULT);
-                    restart = true;
-                    break;
-                }
-            }
-
-            if(!restart)
-                break;
-        }
-    }
-
     _RemoveNoStackAurasDueToAura(aura);
 
     // Update target aura state flag
@@ -3641,10 +3643,6 @@ void Unit::__UnapplyAura(AuraApplicationMap::iterator &i)
     // Remove all pointers from lists here to prevent possible pointer invalidation on spellcast/auraapply/auraremove
     m_appliedAuras.erase(i);
 
-    // Unregister single cast aura
-    if (caster && aura->IsSingleTarget())
-        caster->GetSingleCastAuras().remove(aurApp);
-
     if (aura->GetSpellProto()->AuraInterruptFlags)
     {
         m_interruptableAuras.remove(aurApp);
@@ -3755,21 +3753,20 @@ void Unit::_UnapplyAura(AuraApplication * aurApp, AuraRemoveMode removeMode)
     }
 }
 
-void Unit::_RemoveNoStackAurasDueToAura(Aura * aura)
+void Unit::_RemoveNoStackAuraApplicationsDueToAura(Aura * aura)
 {
+    // dynobj auras can stack infinite number of times
     if (aura->GetType() == DYNOBJ_AURA_TYPE)
         return;
 
     SpellEntry const* spellProto = aura->GetSpellProto();
 
-    uint32 spellId = aura->GetId();
+    uint32 spellId = spellProto->Id;
 
     // passive spell special case (only non stackable with ranks)
     if(IsPassiveSpell(spellId) && IsPassiveSpellStackableWithRanks(spellProto))
         return;
 
-    //bool linked = spellmgr.GetSpellCustomAttr(spellId) & SPELL_ATTR_CU_LINK_AURA? true : false;
-
     bool remove = false;
     for (AuraApplicationMap::iterator i = m_appliedAuras.begin(); i != m_appliedAuras.end(); ++i)
     {
@@ -3779,49 +3776,9 @@ void Unit::_RemoveNoStackAurasDueToAura(Aura * aura)
             i = m_appliedAuras.begin();
         }
 
-        // Do not check already applied aura
-        if (i->second->GetBase() == aura)
+        if (!_IsNoStackAuraDueToAura(aura, i->second->GetBase()))
             continue;
 
-        // Do not check already applied aura
-        if (i->second->GetBase()->GetType() != aura->GetType())
-            continue;
-
-        SpellEntry const* i_spellProto = i->second->GetBase()->GetSpellProto();
-        uint32 i_spellId = i_spellProto->Id;
-        bool sameCaster = aura->GetCasterGUID() == (*i).second->GetBase()->GetCasterGUID();
-
-        if(IsPassiveSpell(i_spellId))
-        {
-            // passive non-stackable spells not stackable only for same caster
-            if(!sameCaster)
-                continue;
-
-            // passive non-stackable spells not stackable only with another rank of same spell
-            if (!spellmgr.IsRankSpellDueToSpell(spellProto, i_spellId))
-                continue;
-        }
-
-        bool is_triggered_by_spell = false;
-        // prevent triggering aura of removing aura that triggered it
-        // prevent triggered aura of removing aura that triggering it (triggered effect early some aura of parent spell
-        for (uint8 j = 0; j < MAX_SPELL_EFFECTS; ++j)
-        {
-            if (i_spellProto->EffectTriggerSpell[j] == spellProto->Id
-                || spellProto->EffectTriggerSpell[j] == i_spellProto->Id) // I do not know what is this for
-            {
-                is_triggered_by_spell = true;
-                break;
-            }
-        }
-
-        if (is_triggered_by_spell)
-            continue;
-
-        if(spellmgr.CanAurasStack(spellProto, i_spellProto, sameCaster))
-            continue;
-
-        // Remove all auras by aura caster
         RemoveAura(i, AURA_REMOVE_BY_DEFAULT);
         if(i == m_appliedAuras.end())
             break;
@@ -3829,6 +3786,82 @@ void Unit::_RemoveNoStackAurasDueToAura(Aura * aura)
     }
 }
 
+void Unit::_RemoveNoStackAurasDueToAura(Aura * aura)
+{
+    SpellEntry const* spellProto = aura->GetSpellProto();
+
+    uint32 spellId = spellProto->Id;
+
+    // passive spell special case (only non stackable with ranks)
+    if(IsPassiveSpell(spellId) && IsPassiveSpellStackableWithRanks(spellProto))
+        return;
+
+    bool remove = false;
+    for (AuraMap::iterator i = m_ownedAuras.begin(); i != m_ownedAuras.end(); ++i)
+    {
+        if(remove)
+        {
+            remove = false;
+            i = m_ownedAuras.begin();
+        }
+
+        if (!_IsNoStackAuraDueToAura(aura, i->second))
+            continue;
+
+        RemoveOwnedAura(i, AURA_REMOVE_BY_DEFAULT);
+        if(i == m_ownedAuras.end())
+            break;
+        remove = true;
+    }
+}
+
+bool Unit::_IsNoStackAuraDueToAura(Aura * appliedAura, Aura * existingAura) const
+{
+    SpellEntry const* spellProto = appliedAura->GetSpellProto();
+    // Do not check already applied aura
+    if (existingAura == appliedAura)
+        return false;
+
+    // Do not check dynobj auras for stacking
+    if (existingAura->GetType() != UNIT_AURA_TYPE)
+        return false;
+
+    SpellEntry const* i_spellProto = existingAura->GetSpellProto();
+    uint32 i_spellId = i_spellProto->Id;
+    bool sameCaster = appliedAura->GetCasterGUID() == existingAura->GetCasterGUID();
+
+    if(IsPassiveSpell(i_spellId))
+    {
+        // passive non-stackable spells not stackable only for same caster
+        if(!sameCaster)
+            return false;
+
+        // passive non-stackable spells not stackable only with another rank of same spell
+        if (!spellmgr.IsRankSpellDueToSpell(spellProto, i_spellId))
+            return false;
+    }
+
+    bool is_triggered_by_spell = false;
+    // prevent triggering aura of removing aura that triggered it
+    // prevent triggered aura of removing aura that triggering it (triggered effect early some aura of parent spell
+    for (uint8 j = 0; j < MAX_SPELL_EFFECTS; ++j)
+    {
+        if (i_spellProto->EffectTriggerSpell[j] == spellProto->Id
+            || spellProto->EffectTriggerSpell[j] == i_spellProto->Id) // I do not know what is this for
+        {
+            is_triggered_by_spell = true;
+            break;
+        }
+    }
+
+    if (is_triggered_by_spell)
+        return false;
+
+    if(spellmgr.CanAurasStack(spellProto, i_spellProto, sameCaster))
+        return false;
+    return true;
+}
+
 void Unit::_HandleAuraEffect(AuraEffect * aurEff, bool apply)
 {
     if (apply)
@@ -3849,6 +3882,10 @@ void Unit::RemoveOwnedAura(AuraMap::iterator &i, AuraRemoveMode removeMode)
 
     m_ownedAuras.erase(i);
 
+    // Unregister single target aura
+    if (aura->IsSingleTarget())
+        aura->UnregisterSingleTarget();
+
     aura->_Remove(removeMode);
 
     i = m_ownedAuras.begin();
@@ -3883,10 +3920,10 @@ void Unit::RemoveOwnedAura(Aura * aura, AuraRemoveMode removeMode)
     assert(false);
 }
 
-Aura * Unit::GetOwnedAura(uint32 spellId, uint64 caster, uint8 reqEffMask) const
+Aura * Unit::GetOwnedAura(uint32 spellId, uint64 caster, uint8 reqEffMask, Aura * except) const
 {
     for (AuraMap::const_iterator itr = m_ownedAuras.lower_bound(spellId); itr != m_ownedAuras.upper_bound(spellId); ++itr)
-        if(((itr->second->GetEffectMask() & reqEffMask) == reqEffMask) && (!caster || itr->second->GetCasterGUID() == caster))
+        if(((itr->second->GetEffectMask() & reqEffMask) == reqEffMask) && (!caster || itr->second->GetCasterGUID() == caster) && (!except || except != itr->second))
             return itr->second;
     return NULL;
 }
@@ -4096,10 +4133,12 @@ void Unit::RemoveAurasDueToSpellBySteal(uint32 spellId, uint64 casterGUID, Unit
                 int32 dur = 2*MINUTE*IN_MILISECONDS < aura->GetDuration() ? 2*MINUTE*IN_MILISECONDS : aura->GetDuration();
 
                 newAura = Aura::TryCreate(aura->GetSpellProto(), effMask, stealer, NULL, &baseDamage[0], NULL, aura->GetCasterGUID());
-                assert(newAura);
+                if (!newAura)
+                    return;
                 newAura->SetLoadedState(dur, dur, stealCharge ? 1 : aura->GetCharges(), aura->GetStackAmount(), recalculateMask, &damage[0]);
                 // strange but intended behaviour: Stolen single target auras won't be treated as single targeted
-                newAura->SetIsSingleTarget(false);
+                if (newAura->IsSingleTarget())
+                    newAura->UnregisterSingleTarget();
                 newAura->ApplyForTargets();
             }
             return;
@@ -4145,18 +4184,20 @@ void Unit::RemoveAurasByType(AuraType auraType, uint64 casterGUID, Aura * except
 void Unit::RemoveNotOwnSingleTargetAuras(uint32 newPhase)
 {
     // single target auras from other casters
-    for (AuraMap::iterator iter = m_ownedAuras.begin(); iter != m_ownedAuras.end();)
+    for (AuraApplicationMap::iterator iter = m_appliedAuras.begin(); iter != m_appliedAuras.end();)
     {
-        Aura const * aura = iter->second;
+        AuraApplication const * aurApp = iter->second;
+        Aura const * aura = aurApp->GetBase();
+
         if (aura->GetCasterGUID() !=GetGUID() && IsSingleTargetSpell(aura->GetSpellProto()))
         {
             if (!newPhase)
-                RemoveOwnedAura(iter);
+                RemoveAura(iter);
             else
             {
                 Unit* caster = aura->GetCaster();
                 if (!caster || !caster->InSamePhase(newPhase))
-                    RemoveOwnedAura(iter);
+                    RemoveAura(iter);
                 else
                     ++iter;
             }
@@ -4166,15 +4207,15 @@ void Unit::RemoveNotOwnSingleTargetAuras(uint32 newPhase)
     }
 
     // single target auras at other targets
-    AuraApplicationList& scAuras = GetSingleCastAuras();
-    for (AuraApplicationList::iterator iter = scAuras.begin(); iter != scAuras.end();)
+    AuraList& scAuras = GetSingleCastAuras();
+    for (AuraList::iterator iter = scAuras.begin(); iter != scAuras.end();)
     {
-        AuraApplication * aurApp= *iter;
+        Aura * aura = *iter;
         ++iter;
-        if (aurApp->GetTarget() != this && !aurApp->GetTarget()->InSamePhase(newPhase))
+        if (aura->GetUnitOwner() != this && !aura->GetUnitOwner()->InSamePhase(newPhase))
         {
             uint32 removedAuras = m_removedAurasCount;
-            aurApp->GetBase()->Remove();
+            aura->Remove();
             if (m_removedAurasCount > removedAuras + 1)
                 iter = scAuras.begin();
         }

src/game/Unit.h

@@ -320,6 +320,7 @@ struct SpellValue;
 
 class AuraApplication;
 class Aura;
+class UnitAura;
 class AuraEffect;
 class Creature;
 class Spell;
@@ -1531,14 +1532,16 @@ class TRINITY_DLL_SPEC Unit : public WorldObject
         Pet* CreateTamedPetFrom(Creature* creatureTarget,uint32 spell_id = 0);
 
         // aura apply/remove helpers - you should better not use these
-        void _AddAura(Aura * aura);
+        void _AddAura(UnitAura * aura, Unit * caster);
         AuraApplication * __ApplyAura(Aura * aura);
         void __UnapplyAura(AuraApplicationMap::iterator &i);
         bool _ApplyAuraEffect(Aura * aura, uint8 effIndex);
         void _UnapplyAuraEffect(AuraApplication * aurApp, uint8 effIndex, AuraRemoveMode removeMode);
         void _UnapplyAura(AuraApplicationMap::iterator &i, AuraRemoveMode removeMode);
         void _UnapplyAura(AuraApplication * aurApp, AuraRemoveMode removeMode);
+        void _RemoveNoStackAuraApplicationsDueToAura(Aura * aura);
         void _RemoveNoStackAurasDueToAura(Aura * aura);
+        bool _IsNoStackAuraDueToAura(Aura * appliedAura, Aura * existingAura) const;
         void _HandleAuraEffect(AuraEffect * aurEff, bool apply);
 
         // m_ownedAuras container management
@@ -1549,7 +1552,7 @@ class TRINITY_DLL_SPEC Unit : public WorldObject
         void RemoveOwnedAura(uint32 spellId, uint64 caster = 0, uint8 reqEffMask = 0, AuraRemoveMode removeMode = AURA_REMOVE_BY_DEFAULT);
         void RemoveOwnedAura(Aura * aura, AuraRemoveMode removeMode = AURA_REMOVE_BY_DEFAULT);
 
-        Aura * GetOwnedAura(uint32 spellId, uint64 casterGUID = 0, uint8 reqEffMask = 0) const;
+        Aura * GetOwnedAura(uint32 spellId, uint64 casterGUID = 0, uint8 reqEffMask = 0, Aura * except = NULL) const;
 
         // m_appliedAuras container management
         AuraApplicationMap      & GetAppliedAuras()       { return m_appliedAuras; }
@@ -1583,8 +1586,8 @@ class TRINITY_DLL_SPEC Unit : public WorldObject
         void _ApplyAllAuraStatMods();
 
         AuraEffectList const& GetAuraEffectsByType(AuraType type) const { return m_modAuras[type]; }
-        AuraApplicationList      & GetSingleCastAuras()       { return m_scAuras; }
-        AuraApplicationList const& GetSingleCastAuras() const { return m_scAuras; }
+        AuraList      & GetSingleCastAuras()       { return m_scAuras; }
+        AuraList const& GetSingleCastAuras() const { return m_scAuras; }
 
         AuraEffect * GetAuraEffect(uint32 spellId, uint8 effIndex, uint64 casterGUID = 0) const;
         AuraEffect * GetAuraEffectOfRankedSpell(uint32 spellId, uint8 effIndex, uint64 casterGUID = 0) const;
@@ -1994,7 +1997,7 @@ class TRINITY_DLL_SPEC Unit : public WorldObject
         uint32 m_removedAurasCount;
 
         AuraEffectList m_modAuras[TOTAL_AURAS];
-        AuraApplicationList m_scAuras;                        // casted singlecast auras
+        AuraList m_scAuras;                        // casted singlecast auras
         AuraApplicationList m_interruptableAuras;             // auras which have interrupt mask applied on unit
         AuraStateAurasMap m_auraStateAuras;        // Used for improve performance of aura state checks on aura apply/remove
         uint32 m_interruptMask;