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

[9.0.23] Offgcd abilities proposed ignoring power cost #737

Closed
sirTribble opened this issue Dec 14, 2020 · 8 comments
Closed

[9.0.23] Offgcd abilities proposed ignoring power cost #737

sirTribble opened this issue Dec 14, 2020 · 8 comments
Projects

Comments

@sirTribble
Copy link

Describe the bug
Shield of righteous propose even when i don't have 3 holy power

To Reproduce
example script

Define(shield_of_the_righteous 53600)
SpellInfo(shield_of_the_righteous holypower=3 cd=1 gcd=0 offgcd=1)
Define(avenger_shield 31935)
SpellInfo(avenger_shield cd=15)

AddIcon
{
spell(shield_of_the_righteous)
spell(avenger_shield)
}

cast avenger shield. while avenger shield on CD Shield of righteous will be proposed even if under 3 HP

Expected behavior
Shield of righteous should be proposed only on 3 HP or more

Class, specialization and level

  • Class: Paladin
  • Specialization: any, example on prot
  • Level: 60
@Sidoine Sidoine added this to To do in Bugs Dec 15, 2020
@Sidoine Sidoine moved this from To do to In progress in Bugs Dec 15, 2020
@Sidoine
Copy link
Owner

Sidoine commented Dec 15, 2020

It's the expected outcome. If there is nothing better to show, it will display the first available ability even if there isn't any power, but with the translucent icon.

@Sidoine Sidoine moved this from In progress to Standby in Bugs Dec 15, 2020
@sirTribble
Copy link
Author

sirTribble commented Dec 15, 2020

ok. in that case, there is some best action priority inconvenience, regarding offgcd spells with cost :)
There is updated example:
Define(shield_of_the_righteous 53600)
SpellInfo(shield_of_the_righteous holypower=3 cd=1 gcd=0 offgcd=1)
Define(avenger_shield 31935)
SpellInfo(avenger_shield cd=15)
Define(judgment 20271)
Define(consecration 26573)

AddIcon
{
#spell(shield_of_the_righteous)
spell(avenger_shield)
spell(judgment)
spell(consecration)
}

When all abilities on CD, Next Best action will show ability with the shortest remaining CD, so player can spam it to being queued.

if you uncomment spell(shield_of_the_righteous), then it will be shown as a next best action when everything on CD until CD is actually finished (or i get 3 HP). this crate small lag between usage of abilities but it is there unfortunately. hope you understand what i mean.

and i understand that i can always add Holypower() condition to negate this behavior but may be there is some tweaking for best action will be possible also.

@johnnylam88
Copy link
Contributor

I think a different change might be better. The problem isn't that Shield of the Righteous has a spell cost. The problem is that the spell cost is a resource that has regeneration rate that is not positive (it's zero, or less than zero if out of combat). In that sense, Shield of the Righteous will never be ready no longer how much you wait, unless you start using Holy Power generators on your own.

@sirTribble
Copy link
Author

I think a different change might be better. The problem isn't that Shield of the Righteous has a spell cost. The problem is that the spell cost is a resource that has regeneration rate that is not positive (it's zero, or less than zero if out of combat). In that sense, Shield of the Righteous will never be ready no longer how much you wait, unless you start using Holy Power generators on your own.

You are absolutely right! did not thought about it like this

@johnnylam88
Copy link
Contributor

johnnylam88 commented Dec 16, 2020

In-game, GetPowerRegen() seems to return very miniscule numbers instead of zero for these resources that don't have any natural regeneration. I think if we just round these off to zero in Ovale, then the timespan module will just return empty sets when evaluated for when abilities that use these resources but the resources aren't yet available.

UPDATE: doing more testing in-game, GetPowerRegen() does sometimes return 0, correctly in the case of holy power. I'll pursue the change regarding spell costs that require builders to generate the resource.

@johnnylam88
Copy link
Contributor

I have a working patch for this change, but it needs further testing on classes that use pool_resource before it's good to go.

johnnylam88 added a commit to johnnylam88/Ovale that referenced this issue Dec 17, 2020
Modify getTimeToPowerStateAt() to check every power type if one is
not specified, instead of just the primary resource type.

Fixes issue Sidoine#737.
Sidoine pushed a commit that referenced this issue Dec 17, 2020
…nts (#746)

* Consider very small power regeneration rates to be zero

The Blizzard API function GetPowerRegen() can return very small
numbers when the actual regeneration rate is zero. Treat very small
numbers to be equal to zero in getPowerRateAt().

Also, add some type signatures to getPowerRateAt() while we are
here.

* Check "time to power" for every power needed for a spell

Modify getTimeToPowerStateAt() to check every power type if one is
not specified, instead of just the primary resource type.

Fixes issue #737.
@johnnylam88
Copy link
Contributor

@sirTribble Is this fixed? Can it be closed?

@sirTribble
Copy link
Author

yep thx

Bugs automation moved this from Standby to Done Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Bugs
Done
Development

No branches or pull requests

3 participants