Hekili Priority Helper

Hekili Priority Helper

53M Downloads

[BUG] Minimum operator (>?) not properly evaluating

Closed this issue ยท 3 comments

commented

Before You Begin

  • I confirm that I have downloaded the latest version of the addon.
  • I am not playing on a private server.
  • I checked for an existing, open ticket for this issue and was not able to find one.
  • I edited the title of this issue (above) so that it describes the issue I am reporting.
  • I am reporting an issue with the default priority included with the specialization (imported or edited priorities are not supported).

Spec

Evoker - Augmentation

Describe the Issue

Started playing Aug today with Hekili and found it was never recommending the ability "Time Skip".
Took a look at the Action that has to do with Time Skip and it was as follows:
( ( ( cooldown.fire_breath.remains >? 20 ) + ( cooldown.upheaval.remains >? 20 ) ) ) >= 30 & ( cooldown.breath_of_eons.remains >= 20 & talent.breath_of_eons.enabled | ! talent.breath_of_eons.enabled & cooldown.deep_breath.remains >= 20 )

Just taking a small snippet and explaining:
cooldown.fire_breath.remains >? 20 evaluates to, either 20 or the remaining cooldown of fire breath, whichever is lower.
I tried to replicate the conditions the line wanted by just pressing the spells and couldn't get it to evaluate to true, I'm not sure why, but it seems that the Min operator isn't being evaluated correctly.

I replaced the action with this:
((cooldown.fire_breath.remains + (20 - cooldown.fire_breath.remains) * (cooldown.fire_breath.remains > 20)) + (cooldown.upheaval.remains + (20 - cooldown.upheaval.remains) * (cooldown.upheaval.remains > 20))) > 30 & cooldown.breath_of_eons.remains >= 20 & talent.breath_of_eons.enabled
To achieve functionally the same thing.

I'm sure there are more cases of the min (or max, perhaps?) operator existing in APLs and causing issues, but I haven't looked into it yet. I've just downloaded all the APLs to review.

How to Reproduce

  1. Enter the game as Augmentation Evoker
  2. Take the Breath of Eons Talent
  3. Cast Fire Breath
  4. Cast Upheaval
  5. Cast Breath of Eons
  6. Observe no Time Skip recommendation.

Snapshot (Link)

https://pastebin.com/g4PRCeqH

Raidbots Sim Report (Link)

https://www.raidbots.com/simbot/report/tJnJFuuEJTBLs12gXRzhBz

Additional Information

Raidbots properly evaluates the min operator, so you will see Time Skip being used in the sim.

Contact Information

trevorc (discord)

commented

I put in a workaround for 1.0.5c so that Time Skip is recommended.

For 1.0.6-alpha1, I've put in a behind-the-scenes fix that should make the workaround unnecessary, but a lot of testing is needed to make sure it doesn't break other priorities.

commented

Upon a quick scan, I found that these specs also utilize the min operator. I'm not sure if they have any issues with it, but if the min operator is the problem, they likely do.

DH Havoc and Veng
Feral Druid
Evoker Aug and Dev
Arcane Mage
Shadow Priest
Assassination Rogue
Shaman Ele and Enh
Demo Warlock

commented

if the min operator is the problem

It isn't.