Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep Energy and Focus tick randomized during reset actions. #1288

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

NerdEgghead
Copy link
Contributor

On branch feature/add-trinket-swapping
Changes to be committed:
modified: sim/core/energy.go
modified: sim/core/focus.go
modified: sim/death_knight/unholy/TestUnholy.results
modified: sim/hunter/beast_mastery/TestBM.results
modified: sim/rogue/combat/TestCombat.results
modified: sim/rogue/subtlety/TestSubtlety.results

 On branch feature/add-trinket-swapping
 Changes to be committed:
	modified:   sim/core/energy.go
	modified:   sim/core/focus.go
	modified:   sim/death_knight/unholy/TestUnholy.results
	modified:   sim/hunter/beast_mastery/TestBM.results
	modified:   sim/rogue/combat/TestCombat.results
	modified:   sim/rogue/subtlety/TestSubtlety.results
@NerdEgghead
Copy link
Contributor Author

While fixing a concurrency failure for Rogue + Hunter in the massive item swapping PR #1253 , I realized that the intended randomization of the Energy and Focus tick timers is often "un-done" during the reset process between iterations, because any Haste buffs that activate in between the Energy/Focus bar's reset action and the start of the pull end up re-syncing the timer to 0. This PR aims to fix that: @TheBackstabi @ToxicKevinFerm can you guys do a quick sanity check that the sims behave fine with this change in place given that there are test results changes?

Copy link
Contributor

@ToxicKevinFerm ToxicKevinFerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. The only thing that's affected seems to be BM with Focus Fire then, and that looks fixed.

@TheBackstabi
Copy link
Contributor

Also seems fine on my end. The final differences aren't large enough to be concerning and not seeing any failures.

@NerdEgghead NerdEgghead merged commit 60e3ee7 into master Jan 8, 2025
3 checks passed
@NerdEgghead NerdEgghead deleted the rotation_timing branch January 8, 2025 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants