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

Totem of Invigorating Flame + Vanilla Blood Fury #129

Merged
merged 13 commits into from
Feb 10, 2024

Conversation

tyler-cb
Copy link
Contributor

@tyler-cb tyler-cb commented Feb 7, 2024

No description provided.

kayla-glick and others added 3 commits February 7, 2024 17:08

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@tyler-cb tyler-cb changed the title Totem of Invigorating Flame Totem of Invigorating Flame + Vanilla Blood Fury Feb 7, 2024
Comment on lines 75 to 83
OnGain: func(aura *Aura, sim *Simulation) {
apBonus := (character.GetBaseStats()[stats.AttackPower] + (character.GetStat(stats.Strength) * 2)) * 0.25
character.AddStatDynamic(sim, stats.AttackPower, apBonus)
},

OnExpire: func(aura *Aura, sim *Simulation) {
apBonus := (character.GetBaseStats()[stats.AttackPower] + (character.GetStat(stats.Strength) * 2)) * 0.25
character.AddStatDynamic(sim, stats.AttackPower, -apBonus)
},
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to cache the added value when you add it, then remove the cached value. How you've done it right now if your strength stat changes during this auras duration the expire will remove the new value instead of what it originally added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, I see, what's the best way to store the apBonus value between ongain and expire?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tyler-cb you can just declare it as a variable outside of the aura and then assign it in the onGain

tyler-cb and others added 4 commits February 8, 2024 01:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Duration: time.Second * 15,
// Tooltip is misleading; ap bonus is base AP plus AP from current strength, does not include +attackpower on items/buffs
OnGain: func(aura *Aura, sim *Simulation) {
bloodFuryAP := (character.GetBaseStats()[stats.AttackPower] + (character.GetStat(stats.Strength) * 2)) * 0.25
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to change this to = as := creates a new value instead of assigning i think?

tyler-cb and others added 2 commits February 10, 2024 12:55

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Collaborator

@kayla-glick kayla-glick left a comment

Choose a reason for hiding this comment

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

LGTM now. What do you think @Horatio27 ?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@kayla-glick kayla-glick merged commit e3ca4b2 into wowsims:master Feb 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants