Hekili / hekili

Hekili Priority Helper for DPS and Tanks (WoW Retail)
414 stars 219 forks source link

[REC] Multiple fire mage spells shows wrong value #3124

Closed shiftie closed 9 months ago

shiftie commented 9 months ago

Before You Begin

Describe the Issue

I've noticed that when I use Hekili with a fire mage, shifting power never gets cast. Its condition is:

buff.combustion.down & ( action.fire_blast.charges = 0 | variable.fire_blast_pooling ) & ( ! improved_scorch.active | debuff.improved_scorch.remains > cast_time + action.scorch.cast_time & ! buff.fury_of_the_sun_king.up ) & ! buff.hot_streak.react & variable.shifting_power_before_combustion

source: https://github.com/Hekili/hekili/blob/dragonflight/Dragonflight/APLs/MageFire.simc#L83

but it seems that action.fire_blast.charges is always equal to 3, so condition is never true.

I'm using a workaround with action.fire_blast.charges_fractional < 1 since action.fire_blast.charges_fractional seems to be accurate.

Same applies for action.phoenix_flames.charges.

Can you please fix action.fire_blast.charges & action.phoenix_flames.charges?

How to Reproduce

  1. Log a fire mage with shifting power talent
  2. Start a fight
  3. Observe shifting power never be recommended
  4. Open Hekili fire mage action list and select entrey 34. Shifting power
  5. Use a fire blast & mouse over entry condition to show debug popup, observe that action.fire_blast.charges is always equal to 3

image

edit: Other spells / buffs show wrong values & causes bad recommendations. c.f.: https://github.com/Hekili/hekili/issues/3124#issuecomment-1890909087

Snapshot (Link)

https://pastebin.com/NzhiJxuK

Raidbots Sim Report (Link)

No response

Additional Information

No response

Contact Information

No response

Hekili commented 9 months ago

I'll have to look closely at SimC because action.X.charges has tended to be the number of charges an ability has (i.e., max charges based on talents, etc.), not the current state of the charges. See cooldown.fire_blast.charges and/or cooldown.fire_blast.charges_fractional.

shiftie commented 9 months ago

I'll have to look closely at SimC because action.X.charges has tended to be the number of charges an ability has (i.e., max charges based on talents, etc.), not the current state of the charges. See cooldown.fire_blast.charges and/or cooldown.fire_blast.charges_fractional.

I think its supposed to be actual charges since .max_charges attribute exists like in https://github.com/Hekili/hekili/blob/dragonflight/Dragonflight/APLs/MageFire.simc#L149

also if .charges was "max" charges, conditions like action.fire_blast.charges = 0 (currently implemented in entry shown above) could never be true.

shiftie commented 9 months ago

@Hekili seems that cooldown.combustion.remains always returns 120 instead of actual remaining CD. For instance cooldown.shifting_power.remains shows actual remaining duration.

This breaks many other actions that rely on that value.

More generally combustion cooldown has issues, for example cooldown.combustion.up is false all the time. I've tested attributes described here https://github.com/simulationcraft/simc/wiki/Action-List-Conditional-Expressions#cooldowns, Seems there is a problem for that spell.

buff.flame_accelerant also shows wrong values, buff.flame_accelerant.up is false all the time as well.

Hekili commented 9 months ago

@Hekili seems that cooldown.combustion.remains always returns 120 instead of actual remaining CD. For instance cooldown.shifting_power.remains shows actual remaining duration.

When spells are toggled off (or broken out into a separate display), their cooldowns are faked so that the addon isn't trapped in an endless loop of preparing to use a spell that you don't intend to push. If it's that CDs are toggled off, toggling them on will resolve this. If it's that CDs are in a different display, you can change this behavior in /hekili > Actions by checking When Cooldowns Shown Separately, Use Actual Cooldown.

buff.flame_accelerant also shows wrong values, buff.flame_accelerant.up is false all the time as well.

I'd need to see a snapshot when flame_accelerant is applied but not detected.

Hekili commented 9 months ago

This should be resolved in the next release.

shiftie commented 9 months ago

ty