Skip to content

fix(Core/Spells): Fire CAST proc before HIT/FINISH for instant spells#25647

Open
blinkysc wants to merge 2 commits intoazerothcore:masterfrom
blinkysc:fix/proc-phase-order-cast-before-finish
Open

fix(Core/Spells): Fire CAST proc before HIT/FINISH for instant spells#25647
blinkysc wants to merge 2 commits intoazerothcore:masterfrom
blinkysc:fix/proc-phase-order-cast-before-finish

Conversation

@blinkysc
Copy link
Copy Markdown
Contributor

@blinkysc blinkysc commented Apr 29, 2026

Changes Proposed:

This PR proposes changes to:

  • Core (units, players, creatures, game systems).
  • Scripts (bosses, spell scripts, creature scripts).
  • Database (SAI, creatures, etc).

For instant spells, HIT and FINISH happen inside handle_immediate(), so CAST procs were firing last - the actual order was HIT -> FINISH -> CAST. An aura applied during HIT (e.g. Arcane Potency from a Clearcasting that procs mid-AoE on Arcane Explosion) was then consumed by the same cast's trailing CAST proc, never benefiting the player.

This moves the CAST proc block to before the immediate/delayed branch in Spell::_cast() so the spec-correct order CAST -> HIT -> FINISH is restored. m_appliedMods is already populated by HandleLaunchPhase (via ApplyModToSpell and SpellDoneCritChance), so PROC_ATTR_REQ_SPELLMOD-based consumption (Hot Streak, Brain Freeze, Arcane Potency, etc.) still drops charges correctly.

Missile Barrage (44401) is the one outlier: its three modifiers (SPELLMOD_DURATION effect 0, SPELLMOD_ACTIVATION_TIME effect 1, SPELLMOD_COST effect 2) are all applied inside handle_immediate (channel start and periodic-aura setup), so consumption on CAST would fail. The accompanying SQL update moves its SpellPhaseMask from CAST (1) to FINISH (4) so the charge drops once m_appliedMods is fully populated.

I audited every other spell_proc row with SpellPhaseMask = CAST and AttributesMask & PROC_ATTR_REQ_SPELLMOD (12043, 12536, 16166, 16246, 16870, 17116, 17941, 18708, 34936, 47383, 48108, 53817, 54274/76/77, 57529/31, 57761, 71162/65) - all use SPELLMOD_CASTING_TIME, SPELLMOD_COST, SPELLMOD_GLOBAL_COOLDOWN, SPELLMOD_DAMAGE, or SPELLMOD_CRITICAL_CHANCE, all of which are applied either in Spell::prepare() or in HandleLaunchPhase (i.e. before the new CAST proc location). Missile Barrage is the only entry that needed a phase change.

AI-assisted Pull Requests

Important

While the use of AI tools when preparing pull requests is not prohibited, contributors must clearly disclose when such tools have been used and specify the model involved.

Contributors are also expected to fully understand the changes they are submitting and must be able to explain and justify those changes when requested by maintainers.

  • AI tools (e.g. ChatGPT, Claude, or similar) were used entirely or partially in preparing this pull request. Claude Code with azerothMCP.

Issues Addressed:

SOURCE:

The changes have been validated through:

  • Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
  • Sniffs (remember to share them with the open source community!)
  • Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
  • The changes promoted by this pull request come partially or entirely from another project (cherry-pick). The Core change is cherry-picked from TrinityCore commit 1de6f59ffc by Shauren (credited as co-author). The commit is in TC master but has not yet been cherry-picked to TC's 3.3.5 branch.

Tests Performed:

This PR has been:

  • Tested in-game by the author.
  • Tested in-game by other community members/someone else other than the author/has been live on production servers.
  • This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

  • This pull request can be tested by following the reproduction steps provided in the linked issue.
  1. Create a Mage with Arcane Concentration (5/5) and Arcane Potency (2/2) talents.
  2. Cast Arcane Explosion on a group of 4+ target dummies repeatedly.
  3. When Clearcasting procs mid-AoE, verify Arcane Potency appears and persists into the next cast (previously it was eaten by the same cast that triggered it).
  4. Verify the next eligible spell (Arcane Blast, Fireball, Frostbolt, Arcane Explosion) consumes the Arcane Potency charge.
  5. Cast Frostbolt/Fireball/etc. until Missile Barrage procs, then cast Arcane Missiles - verify channel duration is shortened (~2.5s), missiles fire every 0.5s, mana cost is reduced, and the buff disappears after that one Arcane Missiles cast.
  6. Verify Hot Streak, Brain Freeze, Maelstrom Weapon, Backlash, Backdraft, Presence of Mind, Nature's Swiftness still consume on cast as before.
  7. Verify periodic AoE spells (Blizzard, Hurricane, Rain of Fire, Volley) do not burn cast-charge buffs on each tick.

Full regression checklist

A complete list of every CAST-phase spell_proc entry (78 rows: class talents, set bonuses, trinkets, encounter procs) is available here for testers to work through:

https://gist.github.com/blinkysc/5e6c0db909734dfc795e167eaa8cfabb

Known Issues and TODO List:

  • Arcane Blast (36032) consumption timing on Arcane Missiles cast - currently removed at end of channel via spell_linked_spell, would ideally consume at channel start.

blinkysc and others added 2 commits April 28, 2026 21:02
Instant spells fired CAST procs after handle_immediate, so an aura
applied during HIT (e.g. Arcane Potency from a mid-AoE Clearcasting)
was consumed by the same cast's trailing CAST proc.

Cherry-picked from TrinityCore commit 1de6f59ffc.

Co-authored-by: Shauren <shauren.trinity@gmail.com>
Its three modifiers (DURATION, ACTIVATION_TIME, COST) are applied
inside handle_immediate, after CAST fires. Consumption on FINISH
catches them once m_appliedMods is fully populated.
@github-actions github-actions Bot added DB related to the SQL database CORE Related to the core file-cpp Used to trigger the matrix build labels Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CORE Related to the core DB related to the SQL database file-cpp Used to trigger the matrix build Ready to be Reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mage] Arcane Potency interaction with arcane explosion [Mage] Arcane Potency interaction with arcane explosion

1 participant