[Havoc] Use spell data ICD for Essence Break proc#11072
Draft
taherbert wants to merge 1 commit intosimulationcraft:midnightfrom
Draft
[Havoc] Use spell data ICD for Essence Break proc#11072taherbert wants to merge 1 commit intosimulationcraft:midnightfrom
taherbert wants to merge 1 commit intosimulationcraft:midnightfrom
Conversation
Spell 320338 has internal_cooldown=1000 (1s per-target ICD) but the code was using structural gates instead: first_attack for BD/DS and \!may_refund for CS/Anni. These approximate the ICD at low haste but allow sub-1s consecutive procs when GCD drops below 1.0s. Replace with target_specific_cooldown_t using the spell data ICD, matching the pattern used by Rogue Weaponmaster and Evoker Bombardments.
f1ed8c8 to
d0a957c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Essence Break's debuff (spell 320338) has
Internal Cooldown: 1 secondsin the spell data, but the code ignores it. Instead, it uses structural gates to limit procs:first_attackboolean (only the first of 4 damage events procs)!may_refund(only the main-hand hit procs)These gates approximate the ICD at low haste, but break down when the GCD drops below 1.0s. In that case, the old code allows consecutive procs less than 1 second apart, which does not match in-game behavior.
Fix
Replace the structural gates with a
target_specific_cooldown_tusing the spell data's 1s ICD. This is the same pattern used by Rogue's Weaponmaster and Evoker's Bombardments.Now any damage event from BD/DS/CS/Anni can trigger the proc, as long as the per-target ICD is ready. This lets a later hit in the sequence proc if the first hit lands too early (during ICD), which is what happens in-game.
Evidence
Spell data
In-game log (WCL)
Procs on a single target during one ESSB window, from @Aezeor's testing:
All proc gaps are > 1.0s. DS hit 4 at 0.933s after the last proc is blocked by ICD.
At the same timestamp (7.755), a different target that had not been procced yet does get its ESSB proc, confirming per-target tracking.
Sim comparison (debug traces, gear_haste_rating=7000, 5 iterations each)
Old code (first_attack / !may_refund gates):
New code (1.0s per-target ICD from spell data):
Old code consistently allows sub-1.0s proc gaps in the first ESSB window of each fight (during Metamorphosis haste). New code respects the ICD across all windows.
cc @Aezeor