Core/Spells: don't allow spells which hit target only with SPELL_EFFECT_DUMMY to execute spell triggers on that targets, also cleanup related code a bit.

5 files changed, 103 insertions, 84 deletions

diff --git a/src/server/game/Miscellaneous/SharedDefines.h b/src/server/game/Miscellaneous/SharedDefines.h
index e84eb565c5b..98fc0869526 100755
--- a/src/server/game/Miscellaneous/SharedDefines.h
+++ b/src/server/game/Miscellaneous/SharedDefines.h
@@ -410,7 +410,7 @@ enum SpellAttr4
     SPELL_ATTR4_UNK0                             = 0x00000001, //  0
     SPELL_ATTR4_UNK1                             = 0x00000002, //  1 proc on finishing move?
     SPELL_ATTR4_UNK2                             = 0x00000004, //  2
-    SPELL_ATTR4_CANT_PROC_FROM_SELFCAST          = 0x00000008, //  3
+    SPELL_ATTR4_UNK3                             = 0x00000008, //  3
     SPELL_ATTR4_UNK4                             = 0x00000010, //  4 This will no longer cause guards to attack on use??
     SPELL_ATTR4_UNK5                             = 0x00000020, //  5
     SPELL_ATTR4_NOT_STEALABLE                    = 0x00000040, //  6 although such auras might be dispellable, they cannot be stolen
@@ -423,8 +423,8 @@ enum SpellAttr4
     SPELL_ATTR4_UNK13                            = 0x00002000, // 13
     SPELL_ATTR4_DAMAGE_DOESNT_BREAK_AURAS        = 0x00004000, // 14 doesn't break auras by damage from these spells
     SPELL_ATTR4_UNK15                            = 0x00008000, // 15
-    SPELL_ATTR4_NOT_USABLE_IN_ARENA              = 0x00010000, // 16 not usable in arena
-    SPELL_ATTR4_USABLE_IN_ARENA                  = 0x00020000, // 17 usable in arena
+    SPELL_ATTR4_NOT_USABLE_IN_ARENA              = 0x00010000, // 16
+    SPELL_ATTR4_USABLE_IN_ARENA                  = 0x00020000, // 17
     SPELL_ATTR4_UNK18                            = 0x00040000, // 18
     SPELL_ATTR4_UNK19                            = 0x00080000, // 19
     SPELL_ATTR4_NOT_CHECK_SELFCAST_POWER         = 0x00100000, // 20 supersedes message "More powerful spell applied" for self casts.
diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index bf444f27e86..f52ce6461b3 100755
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -1166,7 +1166,7 @@ void Spell::DoAllEffectOnTarget(TargetInfo *target)
 
                             //Spells with this flag cannot trigger if effect is casted on self
                             // Slice and Dice, relentless strikes, eviscerate
-    bool canEffectTrigger = unitTarget->CanProc() && (m_spellInfo->AttributesEx4 & (SPELL_ATTR4_CANT_PROC_FROM_SELFCAST) ? m_caster != unitTarget : true);
+    bool canEffectTrigger = unitTarget->CanProc() && CanExecuteTriggersOnHit(mask);
     Unit* spellHitTarget = NULL;
 
     if (missInfo == SPELL_MISS_NONE)                          // In case spell hit target, do all effect on that target
@@ -1355,7 +1355,7 @@ void Spell::DoAllEffectOnTarget(TargetInfo *target)
             m_caster->ToCreature()->AI()->SpellHitTarget(spellHitTarget, m_spellInfo);
 
         // Needs to be called after dealing damage/healing to not remove breaking on damage auras
-        DoTriggersOnSpellHit(spellHitTarget);
+        DoTriggersOnSpellHit(spellHitTarget, mask);
 
         // if target is fallged for pvp also flag caster if a player
         if (unit->IsPvP())
@@ -1525,9 +1525,10 @@ SpellMissInfo Spell::DoSpellHitOnUnit(Unit *unit, const uint32 effectMask, bool
     return SPELL_MISS_NONE;
 }
 
-void Spell::DoTriggersOnSpellHit(Unit *unit)
+void Spell::DoTriggersOnSpellHit(Unit *unit, uint8 effMask)
 {
     // Apply additional spell effects to target
+    // TODO: move this code to scripts
     if (m_preCastSpell)
     {
         // Paladin immunity shields
@@ -1549,38 +1550,44 @@ void Spell::DoTriggersOnSpellHit(Unit *unit)
             m_caster->AddAura(m_preCastSpell, unit);
     }
 
-    // spells with this flag can trigger only if not selfcast (eviscerate for example)
-    if (m_ChanceTriggerSpells.size() && (!((m_spellInfo->AttributesEx4 & SPELL_ATTR4_CANT_PROC_FROM_SELFCAST) && unit == m_caster)))
+    // handle SPELL_AURA_ADD_TARGET_TRIGGER auras
+    // this is executed after spell proc spells on target hit
+    // spells are triggered for each hit spell target
+    // info confirmed with retail sniffs of permafrost and shadow weaving
+    if (!m_hitTriggerSpells.empty() && CanExecuteTriggersOnHit(effMask))
     {
-        int _duration=0;
-        for (ChanceTriggerSpells::const_iterator i = m_ChanceTriggerSpells.begin(); i != m_ChanceTriggerSpells.end(); ++i)
+        int _duration = 0;
+        for (HitTriggerSpells::const_iterator i = m_hitTriggerSpells.begin(); i != m_hitTriggerSpells.end(); ++i)
         {
-            // SPELL_AURA_ADD_TARGET_TRIGGER auras shouldn't trigger auras without duration
-            // set duration equal to triggering spell
             if (roll_chance_i(i->second))
             {
                 m_caster->CastSpell(unit, i->first, true);
                 sLog->outDebug(LOG_FILTER_SPELLS_AURAS, "Spell %d triggered spell %d by SPELL_AURA_ADD_TARGET_TRIGGER aura", m_spellInfo->Id, i->first->Id);
-            }
-            if (GetSpellDuration(i->first) == -1)
-            {
-                if (Aura * triggeredAur = unit->GetAura(i->first->Id, m_caster->GetGUID()))
+
+                // SPELL_AURA_ADD_TARGET_TRIGGER auras shouldn't trigger auras without duration
+                // set duration of current aura to the triggered spell
+                if (GetSpellDuration(i->first) == -1)
                 {
-                    // get duration from aura-only once
-                    if (!_duration)
+                    if (Aura * triggeredAur = unit->GetAura(i->first->Id, m_caster->GetGUID()))
                     {
-                        Aura * aur = unit->GetAura(m_spellInfo->Id, m_caster->GetGUID());
-                        _duration = aur ? aur->GetDuration() : -1;
+                        // get duration from aura-only once
+                        if (!_duration)
+                        {
+                            Aura * aur = unit->GetAura(m_spellInfo->Id, m_caster->GetGUID());
+                            _duration = aur ? aur->GetDuration() : -1;
+                        }
+                        triggeredAur->SetDuration(_duration);
                     }
-                    triggeredAur->SetDuration(_duration);
                 }
             }
         }
     }
 
+    // trigger linked auras remove/apply
+    // TODO: remove/cleanup this, as this table is not documented and people are doing stupid things with it
     if (m_customAttr & SPELL_ATTR0_CU_LINK_HIT)
-        if (const std::vector<int32> *spell_triggered = sSpellMgr->GetSpellLinked(m_spellInfo->Id + SPELL_LINK_HIT))
-            for (std::vector<int32>::const_iterator i = spell_triggered->begin(); i != spell_triggered->end(); ++i)
+        if (std::vector<int32> const* spellTriggered = sSpellMgr->GetSpellLinked(m_spellInfo->Id + SPELL_LINK_HIT))
+            for (std::vector<int32>::const_iterator i = spellTriggered->begin(); i != spellTriggered->end(); ++i)
                 if (*i < 0)
                     unit->RemoveAurasDueToSpell(-(*i));
                 else
@@ -3135,30 +3142,7 @@ void Spell::cast(bool skipCheck)
         return;
     }
 
-    if (m_spellInfo->SpellFamilyName)
-    {
-        if (m_spellInfo->excludeCasterAuraSpell && !IsPositiveSpell(m_spellInfo->excludeCasterAuraSpell))
-            m_preCastSpell = m_spellInfo->excludeCasterAuraSpell;
-        else if (m_spellInfo->excludeTargetAuraSpell && !IsPositiveSpell(m_spellInfo->excludeTargetAuraSpell))
-            m_preCastSpell = m_spellInfo->excludeTargetAuraSpell;
-    }
-
-    switch (m_spellInfo->SpellFamilyName)
-    {
-        case SPELLFAMILY_GENERIC:
-        {
-            if (m_spellInfo->Mechanic == MECHANIC_BANDAGE) // Bandages
-                m_preCastSpell = 11196;                                // Recently Bandaged
-            break;
-        }
-        case SPELLFAMILY_MAGE:
-        {
-             // Permafrost
-             if (m_spellInfo->SpellFamilyFlags[1] & 0x00001000 ||  m_spellInfo->SpellFamilyFlags[0] & 0x00100220)
-              m_preCastSpell = 68391;
-             break;
-        }
-    }
+    PrepareTriggersExecutedOnHit();
 
     // traded items have trade slot instead of guid in m_itemTargetGUID
     // set to real guid to be sent later to the client
@@ -3188,23 +3172,6 @@ void Spell::cast(bool skipCheck)
             TakeReagents();
     }
 
-    // are there any spells need to be triggered after hit?
-    // handle SPELL_AURA_ADD_TARGET_TRIGGER auras
-    Unit::AuraEffectList const& targetTriggers = m_caster->GetAuraEffectsByType(SPELL_AURA_ADD_TARGET_TRIGGER);
-    for (Unit::AuraEffectList::const_iterator i = targetTriggers.begin(); i != targetTriggers.end(); ++i)
-    {
-        if (!(*i)->IsAffectedOnSpell(m_spellInfo))
-            continue;
-        SpellEntry const *auraSpellInfo = (*i)->GetSpellProto();
-        uint32 auraSpellIdx = (*i)->GetEffIndex();
-        if (SpellEntry const *spellInfo = sSpellStore.LookupEntry(auraSpellInfo->EffectTriggerSpell[auraSpellIdx]))
-        {
-            int32 auraBaseAmount = (*i)->GetBaseAmount();
-            int32 chance = m_caster->CalculateSpellDamage(NULL, auraSpellInfo, auraSpellIdx, &auraBaseAmount);
-            m_ChanceTriggerSpells.push_back(std::make_pair(spellInfo, chance * (*i)->GetBase()->GetStackAmount()));
-        }
-    }
-
     if (m_customAttr & SPELL_ATTR0_CU_DIRECT_DAMAGE)
         CalculateDamageDoneForAllTargets();
 
@@ -3482,6 +3449,7 @@ void Spell::_handle_finish_phase()
         if (m_comboPointGain)
             m_caster->m_movedPlayer->GainSpellComboPoints(m_comboPointGain);
     }
+    // TODO: trigger proc phase finish here
 }
 
 void Spell::SendSpellCooldown()
@@ -7302,6 +7270,70 @@ void Spell::CallScriptAfterUnitTargetSelectHandlers(std::list<Unit*>& unitTarget
     }
 }
 
+bool Spell::CanExecuteTriggersOnHit(uint8 effMask) const
+{
+    // check which effects can trigger proc
+    // don't allow to proc for dummy-only spell target hits
+    // prevents triggering/procing effects twice from spells like Eviscerate
+    for (uint8 i = 0;effMask && i < MAX_SPELL_EFFECTS; ++i)
+    {
+        if (m_spellInfo->Effect[i] == SPELL_EFFECT_DUMMY)
+            effMask &= ~(1<<i);
+    }
+    return effMask;
+}
+
+void Spell::PrepareTriggersExecutedOnHit()
+{
+    // todo: move this to scripts
+    if (m_spellInfo->SpellFamilyName)
+    {
+        if (m_spellInfo->excludeCasterAuraSpell && !IsPositiveSpell(m_spellInfo->excludeCasterAuraSpell))
+            m_preCastSpell = m_spellInfo->excludeCasterAuraSpell;
+        else if (m_spellInfo->excludeTargetAuraSpell && !IsPositiveSpell(m_spellInfo->excludeTargetAuraSpell))
+            m_preCastSpell = m_spellInfo->excludeTargetAuraSpell;
+    }
+
+    // todo: move this to scripts
+    switch (m_spellInfo->SpellFamilyName)
+    {
+        case SPELLFAMILY_GENERIC:
+        {
+            if (m_spellInfo->Mechanic == MECHANIC_BANDAGE) // Bandages
+                m_preCastSpell = 11196;  // Recently Bandaged
+            break;
+        }
+        case SPELLFAMILY_MAGE:
+        {
+             // Permafrost
+             if (m_spellInfo->SpellFamilyFlags[1] & 0x00001000 ||  m_spellInfo->SpellFamilyFlags[0] & 0x00100220)
+                 m_preCastSpell = 68391;
+             break;
+        }
+    }
+
+    // handle SPELL_AURA_ADD_TARGET_TRIGGER auras:
+    // save auras which were present on spell caster on cast, to prevent triggered auras from affecting caster
+    // and to correctly calculate proc chance when combopoints are present
+    Unit::AuraEffectList const& targetTriggers = m_caster->GetAuraEffectsByType(SPELL_AURA_ADD_TARGET_TRIGGER);
+    for (Unit::AuraEffectList::const_iterator i = targetTriggers.begin(); i != targetTriggers.end(); ++i)
+    {
+        if (!(*i)->IsAffectedOnSpell(m_spellInfo))
+            continue;
+        SpellEntry const *auraSpellInfo = (*i)->GetSpellProto();
+        uint32 auraSpellIdx = (*i)->GetEffIndex();
+        if (SpellEntry const *spellInfo = sSpellStore.LookupEntry(auraSpellInfo->EffectTriggerSpell[auraSpellIdx]))
+        {
+            // calculate the chance using spell base amount, because aura amount is not updated on combo-points change
+            // this possibly needs fixing
+            int32 auraBaseAmount = (*i)->GetBaseAmount();
+            int32 chance = m_caster->CalculateSpellDamage(NULL, auraSpellInfo, auraSpellIdx, &auraBaseAmount);
+            // proc chance is stored in effect amount
+            m_hitTriggerSpells.push_back(std::make_pair(spellInfo, chance * (*i)->GetBase()->GetStackAmount()));
+        }
+    }
+}
+
 // Global cooldowns management
 enum GCDLimits
 {
diff --git a/src/server/game/Spells/Spell.h b/src/server/game/Spells/Spell.h
index ddb700aa8ab..a05ad331776 100755
--- a/src/server/game/Spells/Spell.h
+++ b/src/server/game/Spells/Spell.h
@@ -631,7 +631,7 @@ class Spell
         void AddItemTarget(Item* target, uint32 effIndex);
         void DoAllEffectOnTarget(TargetInfo *target);
         SpellMissInfo DoSpellHitOnUnit(Unit *unit, uint32 effectMask, bool scaleAura);
-        void DoTriggersOnSpellHit(Unit *unit);
+        void DoTriggersOnSpellHit(Unit *unit, uint8 effMask);
         void DoAllEffectOnTarget(GOTargetInfo *target);
         void DoAllEffectOnTarget(ItemTargetInfo *target);
         bool UpdateChanneledTargetList();
@@ -667,6 +667,11 @@ class Spell
         void CallScriptAfterUnitTargetSelectHandlers(std::list<Unit*>& unitTargets, SpellEffIndex effIndex);
         std::list<SpellScript *> m_loadedScripts;
 
+        bool CanExecuteTriggersOnHit(uint8 effMask) const;
+        void PrepareTriggersExecutedOnHit();
+        typedef std::list< std::pair<SpellEntry const*, int32> > HitTriggerSpells;
+        HitTriggerSpells m_hitTriggerSpells;
+
         // effect helpers
         void GetSummonPosition(uint32 i, Position &pos, float radius = 0.0f, uint32 count = 0);
         void SummonGuardian(uint32 i, uint32 entry, SummonPropertiesEntry const *properties);
@@ -675,10 +680,6 @@ class Spell
         SpellCastResult CanOpenLock(uint32 effIndex, uint32 lockid, SkillType& skillid, int32& reqSkillValue, int32& skillValue);
         // -------------------------------------------
 
-        //List For Triggered Spells
-        typedef std::vector< std::pair<SpellEntry const*, int32> > ChanceTriggerSpells;
-        ChanceTriggerSpells m_ChanceTriggerSpells;
-
         uint32 m_spellState;
         uint32 m_timer;
 
diff --git a/src/server/game/Spells/SpellEffects.cpp b/src/server/game/Spells/SpellEffects.cpp
index 6d2a47bdce4..b9cd951d479 100755
--- a/src/server/game/Spells/SpellEffects.cpp
+++ b/src/server/game/Spells/SpellEffects.cpp
@@ -1248,7 +1248,7 @@ void Spell::EffectDummy(SpellEffIndex effIndex)
                     m_caster->CastSpell(m_caster, 65156, true);
                 return;
             }
-            //Slam
+            // Slam
             if (m_spellInfo->SpellFamilyFlags[0] & SPELLFAMILYFLAG_WARRIOR_SLAM && m_spellInfo->SpellIconID == 559)
             {
                 int32 bp0 = damage;
diff --git a/src/server/game/Spells/SpellMgr.cpp b/src/server/game/Spells/SpellMgr.cpp
index 28dce44e2c6..4167b9e1516 100755
--- a/src/server/game/Spells/SpellMgr.cpp
+++ b/src/server/game/Spells/SpellMgr.cpp
@@ -3753,8 +3753,6 @@ void SpellMgr::LoadSpellCustomAttr()
             ++count;
             break;
         case 27820: // Mana Detonation
-        //case 28062: case 39090:             // Positive/Negative Charge
-        //case 28085: case 39093:
         case 69782: // Ooze Flood
         case 69796: // Ooze Flood
         case 69798: // Ooze Flood
@@ -3893,10 +3891,6 @@ void SpellMgr::LoadSpellCustomAttr()
             spellInfo->EffectMiscValue[0] |= 1;
             ++count;
             break;
-        case 52025: // Cleansing Totem Effect
-            spellInfo->EffectDieSides[1] = 1;
-            ++count;
-            break;
         case 51904: // Summon Ghouls On Scarlet Crusade (core does not know the triggered spell is summon spell)
             spellInfo->EffectImplicitTargetA[0] = TARGET_UNIT_CASTER;
             ++count;
@@ -4243,14 +4237,6 @@ void SpellMgr::LoadSpellCustomAttr()
                     break;
                 ++count;
                 break;
-                // Do not allow Deadly throw and Slice and Dice to proc twice
-            case SPELLFAMILY_ROGUE:
-                if (spellInfo->SpellFamilyFlags[1] & 0x1 || spellInfo->SpellFamilyFlags[0] & 0x40000)
-                    spellInfo->AttributesEx4 |= SPELL_ATTR4_CANT_PROC_FROM_SELFCAST;
-                else
-                    break;
-                ++count;
-                break;
             case SPELLFAMILY_PALADIN:
                 // Seals of the Pure should affect Seal of Righteousness
                 if (spellInfo->SpellIconID == 25 && spellInfo->Attributes & SPELL_ATTR0_PASSIVE)