diff --git a/data/sql/updates/pending_db_world/rev_1772310582393914812.sql b/data/sql/updates/pending_db_world/rev_1772310582393914812.sql new file mode 100644 index 000000000..f2fa773ff --- /dev/null +++ b/data/sql/updates/pending_db_world/rev_1772310582393914812.sql @@ -0,0 +1,4 @@ +-- Fingers of Frost buff: change SpellPhaseMask from 3 (CAST|HIT) to 1 (CAST only). +-- With !IsTriggered() removed from CAST proc blocks, triggered spells now fire +-- CAST procs, so HIT phase is no longer needed for triggered spell consumption. +UPDATE `spell_proc` SET `SpellPhaseMask` = 1 WHERE `SpellId` = 74396; diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp index 4306c1efb..298ff0635 100644 --- a/src/server/game/Spells/Spell.cpp +++ b/src/server/game/Spells/Spell.cpp @@ -3956,6 +3956,44 @@ void Spell::_cast(bool skipCheck) } else { + // CAST phase procs for non-channeled immediate spells + // (channeled spells handle this below to preserve spell mods) + // Note: triggered spells are allowed here; per-aura filtering via + // PROC_ATTR_TRIGGERED_CAN_PROC in SpellAuras.cpp handles rejection. + if (!m_spellInfo->IsChanneled() && m_originalCaster) + { + uint32 procAttacker = m_procAttacker; + if (!procAttacker) + { + bool IsPositive = m_spellInfo->IsPositive(); + if (m_spellInfo->DmgClass == SPELL_DAMAGE_CLASS_MAGIC) + { + procAttacker = IsPositive ? PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_POS : PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG; + } + else + { + procAttacker = IsPositive ? PROC_FLAG_DONE_SPELL_NONE_DMG_CLASS_POS : PROC_FLAG_DONE_SPELL_NONE_DMG_CLASS_NEG; + } + } + + uint32 hitMask = PROC_HIT_NORMAL; + + for (std::list::iterator ihit = m_UniqueTargetInfo.begin(); ihit != m_UniqueTargetInfo.end(); ++ihit) + { + if (ihit->missCondition != SPELL_MISS_NONE) + continue; + + if (!ihit->crit) + continue; + + hitMask |= PROC_HIT_CRITICAL; + break; + } + + Unit::ProcSkillsAndAuras(m_originalCaster, m_originalCaster, procAttacker, PROC_FLAG_NONE, hitMask, 1, BASE_ATTACK, m_spellInfo, m_triggeredByAuraSpell.spellInfo, + m_triggeredByAuraSpell.effectIndex, this, nullptr, nullptr, PROC_SPELL_PHASE_CAST); + } + // Immediate spell, no big deal handle_immediate(); } @@ -3985,13 +4023,11 @@ void Spell::_cast(bool skipCheck) if (modOwner) modOwner->SetSpellModTakingSpell(this, false); - // Handle procs on cast - only for non-triggered spells - // Triggered spells (from auras, items, etc.) should not fire CAST phase procs - // as they are not player-initiated casts. This prevents issues like Arcane Potency - // charges being consumed by periodic damage effects (e.g., Blizzard ticks). - // Must be called AFTER handle_immediate() so spell mods (like Missile Barrage's - // duration reduction) are applied before the aura is consumed by the proc. - if (m_originalCaster && !IsTriggered()) + // CAST phase procs for delayed and channeled spells + // Note: triggered spells are allowed here; per-aura filtering via + // PROC_ATTR_TRIGGERED_CAN_PROC in SpellAuras.cpp handles rejection. + if ((m_spellState == SPELL_STATE_DELAYED || m_spellInfo->IsChanneled()) + && m_originalCaster) { uint32 procAttacker = m_procAttacker; if (!procAttacker) diff --git a/src/server/scripts/Spells/spell_mage.cpp b/src/server/scripts/Spells/spell_mage.cpp index 67cfc1bf1..c4b6865c3 100644 --- a/src/server/scripts/Spells/spell_mage.cpp +++ b/src/server/scripts/Spells/spell_mage.cpp @@ -958,23 +958,11 @@ class spell_mage_fingers_of_frost : public AuraScript void PrepareProc(ProcEventInfo& eventInfo) { + // Block channeled spells (e.g. Blizzard channel start) from consuming charges. + // All other filtering is handled by SpellPhaseMask=1 (CAST only) in spell_proc. if (Spell const* spell = eventInfo.GetProcSpell()) - { - bool isTriggered = spell->IsTriggered(); - bool isCastPhase = (eventInfo.GetSpellPhaseMask() & PROC_SPELL_PHASE_CAST) != 0; - bool isChanneled = spell->GetSpellInfo()->IsChanneled(); - bool prevent = false; - - if (isTriggered) - prevent = false; - else if (isChanneled) - prevent = true; - else if (!isCastPhase) - prevent = true; - - if (prevent) + if (spell->GetSpellInfo()->IsChanneled()) PreventDefaultAction(); - } } void OnRemove(AuraEffect const* /*aurEff*/, AuraEffectHandleModes /*mode*/) diff --git a/src/test/server/game/Spells/SpellProcPhaseOrderingTest.cpp b/src/test/server/game/Spells/SpellProcPhaseOrderingTest.cpp new file mode 100644 index 000000000..7a5108ee7 --- /dev/null +++ b/src/test/server/game/Spells/SpellProcPhaseOrderingTest.cpp @@ -0,0 +1,301 @@ +/* + * This file is part of the AzerothCore Project. See AUTHORS file for Copyright information + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +/** + * @file SpellProcPhaseOrderingTest.cpp + * @brief Tests for proc phase ordering: CAST -> HIT -> FINISH + * + * Validates that CAST-phase procs are isolated from HIT-phase events + * and vice versa. This is critical for correct behavior of spells like + * Arcane Potency (consumed on CAST) not being affected by HIT events + * from the same spell cast. + * + * Related fix: Non-channeled immediate spells fire CAST procs before + * handle_immediate() to ensure CAST -> HIT -> FINISH ordering. + */ + +#include "ProcChanceTestHelper.h" +#include "ProcEventInfoHelper.h" +#include "AuraStub.h" +#include "SpellInfoTestHelper.h" +#include "gtest/gtest.h" + +using namespace testing; + +class SpellProcPhaseOrderingTest : public ::testing::Test +{ +protected: + void SetUp() override {} + + void TearDown() override + { + for (auto* si : _spellInfos) + delete si; + _spellInfos.clear(); + } + + SpellInfo* CreateSpellInfo(uint32 id) + { + auto* si = SpellInfoBuilder().WithId(id).Build(); + _spellInfos.push_back(si); + return si; + } + + std::vector _spellInfos; +}; + +// ============================================================================= +// Phase Isolation: CAST-only procs must not trigger on HIT events +// ============================================================================= + +TEST_F(SpellProcPhaseOrderingTest, CastPhaseProc_TriggersOnCastEvent) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .Build(); + + auto castEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); +} + +TEST_F(SpellProcPhaseOrderingTest, CastPhaseProc_DoesNotTriggerOnHitEvent) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .Build(); + + auto hitEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + EXPECT_FALSE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, hitEvent)); +} + +TEST_F(SpellProcPhaseOrderingTest, CastPhaseProc_DoesNotTriggerOnFinishEvent) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .Build(); + + auto finishEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_FINISH) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + EXPECT_FALSE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, finishEvent)); +} + +// ============================================================================= +// Phase Isolation: HIT-only procs must not trigger on CAST events +// ============================================================================= + +TEST_F(SpellProcPhaseOrderingTest, HitPhaseProc_DoesNotTriggerOnCastEvent) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .Build(); + + auto castEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + EXPECT_FALSE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); +} + +// ============================================================================= +// Charge consumption respects phase isolation +// Simulates the Arcane Potency scenario: charges consumed on CAST phase +// should not be consumed again when HIT phase fires later. +// ============================================================================= + +TEST_F(SpellProcPhaseOrderingTest, ChargeConsumedOnCast_NotConsumedAgainOnHit) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + // Proc entry configured for CAST phase only (like Arcane Potency) + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithChance(100.0f) + .Build(); + + auto aura = AuraStubBuilder() + .WithId(12345) + .WithCharges(2) + .Build(); + + // CAST phase event fires first (correct ordering) + auto castEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + // CAST phase matches -> proc triggers, charge consumed + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); + ProcChanceTestHelper::SimulateConsumeProcCharges(aura.get(), procEntry); + EXPECT_EQ(aura->GetCharges(), 1); + + // HIT phase event fires second + auto hitEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + // HIT phase does NOT match CAST-only proc -> no trigger, no charge consumed + EXPECT_FALSE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, hitEvent)); + EXPECT_EQ(aura->GetCharges(), 1); // Still 1, not consumed +} + +TEST_F(SpellProcPhaseOrderingTest, ChargeConsumedOnCast_AvailableForNextSpell) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithChance(100.0f) + .Build(); + + auto aura = AuraStubBuilder() + .WithId(12345) + .WithCharges(2) + .Build(); + + auto castEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + // First spell cast consumes one charge + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); + ProcChanceTestHelper::SimulateConsumeProcCharges(aura.get(), procEntry); + EXPECT_EQ(aura->GetCharges(), 1); + EXPECT_FALSE(aura->IsRemoved()); + + // Second spell cast consumes last charge + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); + ProcChanceTestHelper::SimulateConsumeProcCharges(aura.get(), procEntry); + EXPECT_EQ(aura->GetCharges(), 0); + EXPECT_TRUE(aura->IsRemoved()); +} + +// ============================================================================= +// Multi-phase procs (CAST | HIT) trigger on both phases +// ============================================================================= + +TEST_F(SpellProcPhaseOrderingTest, MultiPhaseProc_TriggersOnBothCastAndHit) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST | PROC_SPELL_PHASE_HIT) + .Build(); + + auto castEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + auto hitEvent = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, castEvent)); + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, hitEvent)); +} + +// ============================================================================= +// All three phases are distinct +// ============================================================================= + +TEST_F(SpellProcPhaseOrderingTest, AllThreePhases_MutuallyExclusive) +{ + auto* spellInfo = CreateSpellInfo(1); + DamageInfo damageInfo(nullptr, nullptr, 100, spellInfo, SPELL_SCHOOL_MASK_ARCANE, SPELL_DIRECT_DAMAGE); + + uint32 phases[] = { PROC_SPELL_PHASE_CAST, PROC_SPELL_PHASE_HIT, PROC_SPELL_PHASE_FINISH }; + + for (uint32 procPhase : phases) + { + auto procEntry = SpellProcEntryBuilder() + .WithProcFlags(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(procPhase) + .Build(); + + for (uint32 eventPhase : phases) + { + auto event = ProcEventInfoBuilder() + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(eventPhase) + .WithHitMask(PROC_HIT_NORMAL) + .WithDamageInfo(&damageInfo) + .Build(); + + if (procPhase == eventPhase) + EXPECT_TRUE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, event)) + << "Phase " << procPhase << " should match itself"; + else + EXPECT_FALSE(sSpellMgr->CanSpellTriggerProcOnEvent(procEntry, event)) + << "Phase " << procPhase << " should not match phase " << eventPhase; + } + } +}