-
Notifications
You must be signed in to change notification settings - Fork 51
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
Prot Paladin: Righteous fury + Hand of Reckoning #1064
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e66efed
add RF toggle to prot paladin spec options
wsphillips 09a3e29
implement Righteous Fury + Hand of Reckoning
wsphillips eeca2b9
Merge branch 'master' of github.com:wowsims/sod into righteous_fury
wsphillips e6ee286
add RF to tests/update tests, refactor RF implementation, add change …
wsphillips c3a2937
Merge branch 'master' of github.com:wowsims/sod into righteous_fury
wsphillips 0f44583
Merge branch 'master' into righteous_fury
wsphillips File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package paladin | ||
|
|
||
| import ( | ||
| "github.com/wowsims/sod/sim/core" | ||
| "github.com/wowsims/sod/sim/core/proto" | ||
| ) | ||
|
|
||
| func (paladin *Paladin) registerRighteousFury() { | ||
| if !paladin.Options.RighteousFury { | ||
| return | ||
| } | ||
| horRune := proto.PaladinRune_RuneHandsHandOfReckoning | ||
| hasHoR := paladin.hasRune(horRune) | ||
|
|
||
| actionID := core.ActionID{SpellID: core.TernaryInt32(hasHoR, int32(horRune), 25780)} | ||
|
|
||
| rfThreatMultiplier := 0.6 + core.TernaryFloat64(hasHoR, 0.2, 0.0) | ||
| // Improved Righteous Fury is multiplicative. | ||
| rfThreatMultiplier *= 1.0 + []float64{0.0, 0.16, 0.33, 0.5}[paladin.Talents.ImprovedRighteousFury] | ||
|
|
||
| paladin.OnSpellRegistered(func(spell *core.Spell) { | ||
| if spell.SpellSchool.Matches(core.SpellSchoolHoly) { | ||
| spell.ThreatMultiplier *= 1.0 + rfThreatMultiplier | ||
| } | ||
| }) | ||
|
|
||
| rfAura := core.MakePermanent(&core.Aura{Label: "Righteous Fury", ActionID: actionID}) | ||
|
|
||
| // Passive effects granted by Hand of Reckoning rune; only active if Righteous Fury is on. | ||
| if hasHoR { | ||
|
|
||
| // Damage which takes you below 35% health is reduced by 20% (DR component of WotLK's Ardent Defender) | ||
| rfDamageReduction := 0.2 | ||
|
|
||
| handler := func(sim *core.Simulation, spell *core.Spell, result *core.SpellResult) { | ||
| incomingDamage := result.Damage | ||
| if (paladin.CurrentHealth()-incomingDamage)/paladin.MaxHealth() <= 0.35 { | ||
| result.Damage -= (paladin.MaxHealth()*0.35 - (paladin.CurrentHealth() - incomingDamage)) * rfDamageReduction | ||
| if sim.Log != nil { | ||
| paladin.Log(sim, "Righteous Fury absorbs %d damage", int32(incomingDamage-result.Damage)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| paladin.AddDynamicDamageTakenModifier(handler) | ||
|
|
||
| // Gives you mana when healed by other friendly targets' spells equal to 25% of the amount healed. | ||
| horManaMetrics := paladin.NewManaMetrics(actionID) | ||
|
|
||
| rfAura.OnHealTaken = func(aura *core.Aura, sim *core.Simulation, spell *core.Spell, result *core.SpellResult) { | ||
| if spell.IsOtherAction(proto.OtherAction_OtherActionHealingModel) { | ||
| manaGained := result.Damage * 0.25 | ||
| paladin.AddMana(sim, manaGained, horManaMetrics) | ||
| } | ||
| } | ||
| } | ||
| paladin.RegisterAura(*rfAura) | ||
| } | ||
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the bonus threat only apply if RF is active?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or are we just assuming it's always active for prot I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function starts with a check to see if RF was enabled via the button in the UI spec options. Since Rets won't have the option on their UI, the
OnSpellRegisteredwon't get ever get evaluated for them (it will default to false in the options struct). For prot, it also works correctly if you turn it off in the UI.Also note: In game, the passives for Hand of Reckoning are only active when you have Righteous fury on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative here is I could have the
OnSpellRegisteredjust append each spell to an array (e.g.paladin.holySpells) and have the aura'sOnGainandOnExpireapply/remove the threat multiplier on the list of spells. The disadvantage to that is you're doing a multiply/divide for each spell at the beginning and end of each iteration versus doing it once during the build phase. Putting the threat mod onto the aura itself would only be advantageous if some other future item or aura for paladins modified threat for specific Holy spells, in which case I think we can cross that bridge if we ever get there.