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.

Sign up for GitHub

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

Jump to bottom

[Warrior] Implement charge as an actual movement out and back in #1237

Merged
merged 7 commits into from
Dec 16, 2024
Merged

[Warrior] Implement charge as an actual movement out and back in #1237

merged 7 commits into from
Dec 16, 2024

Conversation

hillerstorm
Copy link
Contributor

@hillerstorm hillerstorm commented Nov 30, 2024 •
edited
Loading

  • Movement speed bonuses like enchants, meta gems and talents are now ExclusiveEffects
  • Added MaxRange to all relevant Warrior abilities
  • Changed Charge to require 8 yards and move back in rapidly when used
  • Tweaked Arms default starting distance to 9 and changed pre-pull to use Charge a tiny bit earlier, to have it connect around 0
    • This change didn't affect the dps from before, which was the goal
  • Added hidden-by-default APL actions for using Charge in the APL, for Charge-weave testing
    • Instead of doing this as a strict sequence, I separated the two and put the Charge action at the top to always use it if it detects that it's out of melee range.

@1337LutZ
Copy link
Contributor

1337LutZ commented Dec 2, 2024

LGTM but would like @NerdEgghead to check the code ;D

NerdEgghead
NerdEgghead requested changes Dec 2, 2024
View reviewed changes
sim/common/cata/enchant_effects.go Outdated Show resolved Hide resolved
sim/druid/feral/TestFeral.results Outdated Show resolved Hide resolved
sim/paladin/talents_retribution.go Outdated Show resolved Hide resolved
sim/warrior/charge.go Outdated Show resolved Hide resolved
sim/warrior/charge.go Outdated Show resolved Hide resolved
sim/warrior/charge.go Outdated Show resolved Hide resolved
@hillerstorm hillerstorm marked this pull request as draft December 9, 2024 22:51
@github-actions github-actions bot added the Rogue label Dec 15, 2024
@hillerstorm hillerstorm marked this pull request as ready for review December 15, 2024 02:41
@hillerstorm
Copy link
Contributor Author

@zoristaken please check if you have some time :)

NerdEgghead
NerdEgghead approved these changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@NerdEgghead NerdEgghead left a comment

Choose a reason for hiding this comment

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

Nicely done!

@hillerstorm hillerstorm merged commit 9618783 into wowsims:master Dec 16, 2024
3 checks passed
@hillerstorm hillerstorm deleted the charge_experiment branch December 16, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NerdEgghead NerdEgghead NerdEgghead approved these changes

@1337LutZ 1337LutZ 1337LutZ approved these changes

Assignees
No one assigned
Labels
Druid Paladin Rogue Warrior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hillerstorm @1337LutZ @NerdEgghead