From 47568e42c5c2a733bc6a9780a62fba06f9eb9c24 Mon Sep 17 00:00:00 2001 From: blinkysc <37940565+blinkysc@users.noreply.github.com> Date: Thu, 19 Feb 2026 12:58:46 -0600 Subject: [PATCH] fix(Core/Spells): fix FINISH phase proc targeting for triggered spells (#24757) Co-authored-by: blinkysc Co-authored-by: Mykhailo Redko Co-authored-by: Claude Opus 4.6 --- .../game/Spells/Auras/SpellAuraEffects.cpp | 8 +- src/server/game/Spells/Auras/SpellAuras.cpp | 1 + src/server/game/Spells/Spell.cpp | 2 +- .../Spells/SpellProcTargetResolutionTest.cpp | 204 ++++++++++++++++++ 4 files changed, 211 insertions(+), 4 deletions(-) create mode 100644 src/test/server/game/Spells/SpellProcTargetResolutionTest.cpp diff --git a/src/server/game/Spells/Auras/SpellAuraEffects.cpp b/src/server/game/Spells/Auras/SpellAuraEffects.cpp index 3651fae12..43f9b3adc 100644 --- a/src/server/game/Spells/Auras/SpellAuraEffects.cpp +++ b/src/server/game/Spells/Auras/SpellAuraEffects.cpp @@ -7217,7 +7217,7 @@ void AuraEffect::HandlePeriodicPowerBurnAuraTick(Unit* target, Unit* caster) con void AuraEffect::HandleProcTriggerSpellAuraProc(AuraApplication* aurApp, ProcEventInfo& eventInfo) { Unit* triggerCaster = aurApp->GetTarget(); - Unit* triggerTarget = eventInfo.GetProcTarget(); + Unit* triggerTarget = triggerCaster == eventInfo.GetActor() ? eventInfo.GetActionTarget() : eventInfo.GetActor(); uint32 triggerSpellId = GetSpellInfo()->Effects[GetEffIndex()].TriggerSpell; if (SpellInfo const* triggeredSpellInfo = sSpellMgr->GetSpellInfo(triggerSpellId)) @@ -7234,7 +7234,7 @@ void AuraEffect::HandleProcTriggerSpellAuraProc(AuraApplication* aurApp, ProcEve void AuraEffect::HandleProcTriggerSpellWithValueAuraProc(AuraApplication* aurApp, ProcEventInfo& eventInfo) { Unit* triggerCaster = aurApp->GetTarget(); - Unit* triggerTarget = eventInfo.GetProcTarget(); + Unit* triggerTarget = triggerCaster == eventInfo.GetActor() ? eventInfo.GetActionTarget() : eventInfo.GetActor(); uint32 triggerSpellId = GetSpellInfo()->Effects[m_effIndex].TriggerSpell; if (SpellInfo const* triggeredSpellInfo = sSpellMgr->GetSpellInfo(triggerSpellId)) @@ -7255,7 +7255,9 @@ void AuraEffect::HandleProcTriggerSpellWithValueAuraProc(AuraApplication* aurApp void AuraEffect::HandleProcTriggerDamageAuraProc(AuraApplication* aurApp, ProcEventInfo& eventInfo) { Unit* target = aurApp->GetTarget(); - Unit* triggerTarget = eventInfo.GetProcTarget(); + Unit* triggerTarget = target == eventInfo.GetActor() ? eventInfo.GetActionTarget() : eventInfo.GetActor(); + if (!triggerTarget) + return; if (triggerTarget->HasUnitState(UNIT_STATE_ISOLATED) || triggerTarget->IsImmunedToDamageOrSchool(GetSpellInfo())) { SendTickImmune(triggerTarget, target); diff --git a/src/server/game/Spells/Auras/SpellAuras.cpp b/src/server/game/Spells/Auras/SpellAuras.cpp index 0ea002cff..bdab990fa 100644 --- a/src/server/game/Spells/Auras/SpellAuras.cpp +++ b/src/server/game/Spells/Auras/SpellAuras.cpp @@ -2266,6 +2266,7 @@ uint8 Aura::GetProcEffectMask(AuraApplication* aurApp, ProcEventInfo& eventInfo, } float procChance = CalcProcChance(*procEntry, eventInfo); + if (roll_chance_f(procChance)) return procEffectMask; diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp index 02cea67fd..768b00405 100644 --- a/src/server/game/Spells/Spell.cpp +++ b/src/server/game/Spells/Spell.cpp @@ -4269,7 +4269,7 @@ void Spell::_handle_finish_phase() break; } - Unit::ProcSkillsAndAuras(m_originalCaster, m_originalCaster, procAttacker, PROC_FLAG_NONE, hitMask, 1, BASE_ATTACK, m_spellInfo, m_triggeredByAuraSpell.spellInfo, + Unit::ProcSkillsAndAuras(m_originalCaster, nullptr, procAttacker, PROC_FLAG_NONE, hitMask, 1, BASE_ATTACK, m_spellInfo, m_triggeredByAuraSpell.spellInfo, m_triggeredByAuraSpell.effectIndex, this, nullptr, nullptr, PROC_SPELL_PHASE_FINISH); } } diff --git a/src/test/server/game/Spells/SpellProcTargetResolutionTest.cpp b/src/test/server/game/Spells/SpellProcTargetResolutionTest.cpp new file mode 100644 index 000000000..602f690f9 --- /dev/null +++ b/src/test/server/game/Spells/SpellProcTargetResolutionTest.cpp @@ -0,0 +1,204 @@ +/* + * 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 SpellProcTargetResolutionTest.cpp + * @brief Tests for smart proc trigger target resolution + * + * Verifies the targeting expression used in HandleProcTriggerSpellAuraProc + * and HandleProcTriggerSpellWithValueAuraProc: + * + * triggerTarget = (triggerCaster == actor) ? actionTarget : actor + * + * This expression correctly resolves targets for all proc scenarios: + * - Actor-side HIT phase: triggerCaster==actor, returns enemy (actionTarget) + * - Actor-side FINISH phase: triggerCaster==actor, returns nullptr (actionTarget + * is nullptr because FINISH phase passes no victim) + * - Victim-side HIT phase: triggerCaster!=actor, returns attacker (actor) + */ + +#include "ProcEventInfoHelper.h" +#include "Unit.h" +#include "gtest/gtest.h" + +// Use fake Unit* pointers for testing. The smart targeting expression only +// performs pointer comparison (==), never dereferences, so these are safe. +namespace +{ + Unit* const FAKE_ROGUE = reinterpret_cast(uintptr_t(0x1000)); + Unit* const FAKE_ENEMY = reinterpret_cast(uintptr_t(0x2000)); +} + +/** + * @brief Applies the smart targeting expression from SpellAuraEffects.cpp + * + * This mirrors the logic in HandleProcTriggerSpellAuraProc: + * Unit* triggerCaster = aurApp->GetTarget(); // the aura owner + * Unit* triggerTarget = triggerCaster == eventInfo.GetActor() + * ? eventInfo.GetActionTarget() + * : eventInfo.GetActor(); + */ +static Unit* ResolveProcTriggerTarget(Unit* triggerCaster, ProcEventInfo& eventInfo) +{ + return triggerCaster == eventInfo.GetActor() + ? eventInfo.GetActionTarget() + : eventInfo.GetActor(); +} + +class SpellProcTargetResolutionTest : public ::testing::Test {}; + +// ============================================================================= +// Actor-side proc scenarios (aura owner == event actor) +// ============================================================================= + +TEST_F(SpellProcTargetResolutionTest, ActorSide_HitPhase_TargetsEnemy) +{ + // Scenario: Rogue has Ruthlessness aura, casts Eviscerate on enemy. + // HIT phase: actor=Rogue, actionTarget=Enemy + // triggerCaster is the aura owner (Rogue), which == actor + // Result: returns actionTarget (Enemy) + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(FAKE_ENEMY) + .WithTypeMask(PROC_FLAG_DONE_SPELL_MELEE_DMG_CLASS) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ROGUE; // aura owner + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, FAKE_ENEMY); +} + +TEST_F(SpellProcTargetResolutionTest, ActorSide_FinishPhase_ReturnsNullptr) +{ + // Scenario: Rogue has Ruthlessness aura, finishes Eviscerate. + // FINISH phase: actor=Rogue, actionTarget=nullptr (no victim passed) + // triggerCaster is the aura owner (Rogue), which == actor + // Result: returns nullptr (CastSpell handles this via implicit targeting) + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(nullptr) + .WithTypeMask(PROC_FLAG_DONE_SPELL_MELEE_DMG_CLASS) + .WithSpellPhaseMask(PROC_SPELL_PHASE_FINISH) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ROGUE; // aura owner + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, nullptr); +} + +TEST_F(SpellProcTargetResolutionTest, ActorSide_CastPhase_TargetsEnemy) +{ + // Scenario: Actor-side CAST phase proc (e.g., Nature's Grace). + // actor=Rogue, actionTarget=Enemy + // triggerCaster == actor, returns actionTarget + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(FAKE_ENEMY) + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_NEG) + .WithSpellPhaseMask(PROC_SPELL_PHASE_CAST) + .Build(); + + Unit* triggerCaster = FAKE_ROGUE; + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, FAKE_ENEMY); +} + +// ============================================================================= +// Victim-side proc scenarios (aura owner != event actor) +// ============================================================================= + +TEST_F(SpellProcTargetResolutionTest, VictimSide_HitPhase_TargetsAttacker) +{ + // Scenario: Enemy has a "when hit" proc aura. Rogue hits Enemy. + // HIT phase: actor=Rogue (attacker), actionTarget=Enemy (victim) + // triggerCaster is the aura owner (Enemy), which != actor (Rogue) + // Result: returns actor (Rogue) — the attacker + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(FAKE_ENEMY) + .WithTypeMask(PROC_FLAG_TAKEN_MELEE_AUTO_ATTACK) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ENEMY; // aura owner (victim) + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, FAKE_ROGUE); +} + +// ============================================================================= +// Edge cases +// ============================================================================= + +TEST_F(SpellProcTargetResolutionTest, ActorSide_NullActionTarget_ReturnsNullptr) +{ + // Generic test: when actor-side proc has nullptr actionTarget, + // result is nullptr regardless of phase + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(nullptr) + .WithTypeMask(PROC_FLAG_DONE_MELEE_AUTO_ATTACK) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ROGUE; + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, nullptr); +} + +TEST_F(SpellProcTargetResolutionTest, VictimSide_NullActionTarget_StillReturnsActor) +{ + // When victim-side proc has nullptr actionTarget, the expression + // still returns actor (the attacker) since triggerCaster != actor + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(nullptr) + .WithTypeMask(PROC_FLAG_TAKEN_MELEE_AUTO_ATTACK) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ENEMY; // aura owner (victim), != actor + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, FAKE_ROGUE); +} + +TEST_F(SpellProcTargetResolutionTest, SelfProc_ActorIsActionTarget) +{ + // Edge case: actor == actionTarget (self-damage/self-heal) + // triggerCaster == actor, returns actionTarget (which is also actor) + auto eventInfo = ProcEventInfoBuilder() + .WithActor(FAKE_ROGUE) + .WithActionTarget(FAKE_ROGUE) + .WithTypeMask(PROC_FLAG_DONE_SPELL_MAGIC_DMG_CLASS_POS) + .WithSpellPhaseMask(PROC_SPELL_PHASE_HIT) + .WithHitMask(PROC_HIT_NORMAL) + .Build(); + + Unit* triggerCaster = FAKE_ROGUE; + Unit* result = ResolveProcTriggerTarget(triggerCaster, eventInfo); + + EXPECT_EQ(result, FAKE_ROGUE); +}