Hekili Priority Helper

Hekili Priority Helper

50M Downloads

[REC] Roll the Bones is suggested incorrectly for a 2nd time, after KIR has been used and RTB+Loaded Dice reroll has already been done previously inside that KIR buff window

outlawrogueforever opened this issue ยท 8 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

Rogue - Outlaw

Describe the Issue

Hekili recommends to use RTB for a second time inside a KIR longer buffs window, shortly after already rerolling RTB with Loaded Dice after using KIR to lock in longer RTB buffs (rules are that you can actually RTB with Loaded Dice again after using KIR which gives you a chance to get new buffs without rolling away the longer KIR buffs, but only once, as a 2nd RTB after KIR would overwrite and remove existing RTB buffs).

The logic/line I think that is responsible for this rule:
Variable - set rtb_reroll:
If all active Roll the Bones buffs are ahead of its container buff and have under 40s remaining, then reroll again with Loaded Dice active in an attempt to get even more buffs
Logic:
variable.rtb_reroll & rtb_buffs_longer = 0 | rtb_buffs_normal = 0 & rtb_buffs_longer >= 1 & rtb_buffs < 6 & rtb_buffs_max_remains <= 39 & ! stealthed.all & buff.loaded_dice.up

How to replicate:

  1. We get 4+ buffs via an initial RTB roll and some lucky procs getting CTO a few times, and we lock these in with KIR.

  2. We use RTB + Loaded Dice after using KIR to potentially get the remaining missing buffs or extend existing ones (correctly suggested based on the rules above).

  3. We continue with the rotation, ADR resets and we get a new Loaded Dice buff, and another recommendation for RTB comes up before the existing KIR buff window has expired (we still have like 10-20+ seconds left on the KIR longer buffs), we then use the recommended RTB and we delete the KIR buffs because the logic isn't able to identify that we have already used up our one opportunity to use RTB + Loaded dice after each KIR.

We still have buffs rolling as you can see in the snapshot:

Roll the Bones Buffs (vs. 37.69):

  • broadside 0.18 : 8.00 lose | count_the_odds
  • buried_treasure 19.43 : 37.72 keep | keep_it_rolling
  • grand_melee 16.63 : 38.00 lose | keep_it_rolling
  • ruthless_precision 19.37 : 60.00 lose | keep_it_rolling
  • skull_and_crossbones 13.70 : 38.00 lose | keep_it_rolling
  • true_bearing 19.43 : 37.72 keep | keep_it_rolling

This usually happens during lucky moments of CDR, where you reset ADR very quickly right after using KIR and still have Loaded Dice up already from the previous ADR, or if your normal RTB roll gave you 5+ buffs whilst playing KIR.

Are there any existing issues regarding: spec:RegisterStateExpr( "rtb_buffs_normal", function () in the RogueOutlaw.lua?

It doesn't seem to ever return a value (always set to 0 when pressing normal Roll the Bones (while playing KIR talent)).

rtb_buffs_longer works fine and returns the correct value when pressing KIR to lock in buffs.

I think its possible that because rtb_buffs_normal doesn't work and always returns 0, even when pressing RTB with Loaded Dice again after KIR, the incorrect recommendation for RTB is sent via the logic above, when rtb_buffs_normal should be at 2 instead of 0.

Suggested Solution:
A solution I could imagine to work would be to fix rtb_buffs_normal to work again, or to make another function that would be called 'rtb_buffs_longer_extended' that would be responsible for tracking when we use a followup RTB + Loaded Dice after KIR, to help us avoid using Two RTBs during a single KIR Longer buffs window:

This would return a count for each buff that was made longer by KIR, and then had a following RTB+Loaded dice roll during that KIR buff window to extend/add new buffs to the RTB buff list. Then you can add rtb_buffs_longer_extended = 0 to the logic:

variable.rtb_reroll & rtb_buffs_longer = 0 | rtb_buffs_normal = 0 & rtb_buffs_longer >= 1 & rtb_buffs < 6 & rtb_buffs_max_remains <= 39 & ! stealthed.all & buff.loaded_dice.up & rtb_buffs_longer_extended=0

This would prevent recommendations for another rtb reroll during a KIR buff window that has existing rtb_buffs.longer > 0 that have already been affected by another RTB+Loaded Dice after KIR.

Snapshot (Link)

https://pastebin.com/RxyJxUFH

Raidbots Sim Report (Link)

No response

Additional Information

No response

Contact Information

ame9781

commented

@Hekili I assume it is due to rtbAuraAppliedBy was deprecated and it was used to detect rtb_buffs_normal. (For example, that was the last time it was used https://github.com/Hekili/hekili/blob/57edcfdd5d6fcbf9815f3cd68fc555d82c30e1c7/Dragonflight/RogueOutlaw.lua).
If you need some more snapshots i will try to provide

commented

Also, during Crackshot, Between the Eyes can trigger Count the Odds and apply Roll the Bones buff, because upon casting Between the Eyes a Dispatch is also being automatically cast and it can trigger Count the Odds

commented

@IIeTpoc
Thanks for contributing and helping out!

Yeah I'm pretty certain its because rtb_buffs_normal isn't working that RTB is being suggested incorrectly now after KIR is used and RTB is used once after.

Often suggests to do another RTB again a 2nd time and wipe away the KIR buffs before they are close to done.

If rtb_buffs_normal is working properly, it would return a value > 1, because the 1st RTB after KIR is used would give you some rtb_buffs_normal again.

Fixing rtb_buffs_normal to correctly return count when RTB is used will resolve this issue immediately as the logic above shouldn't return true anymore since rtb_buffs_normal will have a value again in these circumstances:

variable.rtb_reroll & rtb_buffs_longer = 0 | rtb_buffs_normal = 0 & rtb_buffs_longer >= 1 & rtb_buffs < 6 & rtb_buffs_max_remains <= 39 & ! stealthed.all & buff.loaded_dice.up

commented

@IIeTpoc Thanks for contributing and helping out!

Yeah I'm pretty certain its because rtb_buffs_normal isn't working that RTB is being suggested incorrectly now after KIR is used and RTB is used once after.

Often suggests to do another RTB again a 2nd time and wipe away the KIR buffs before they are close to done.

If rtb_buffs_normal is working properly, it would return a value > 1, because the 1st RTB after KIR is used would give you some rtb_buffs_normal again.

Fixing rtb_buffs_normal to correctly return count when RTB is used will resolve this issue immediately as the logic above shouldn't return true anymore since rtb_buffs_normal will have a value again in these circumstances:

variable.rtb_reroll & rtb_buffs_longer = 0 | rtb_buffs_normal = 0 & rtb_buffs_longer >= 1 & rtb_buffs < 6 & rtb_buffs_max_remains <= 39 & ! stealthed.all & buff.loaded_dice.up

rtb_buffs_normal are fixed. you can wait till pull request will be merged or manually change the file locally

commented

@IIeTpoc Hi I've just tested this, so it does work slightly for when you do a fresh RTB with 0 buffs up, but if you RTB with loaded dice up, it doesn't always consistently count towards rtb_buff_normal and it changes back to 0 and breaks again.

For example:

I start the fight and press RTB, I get 1 buff and now rtb_buffs_normal returns 1.
Then, 15 seconds later use RTB again with Loaded Dice, and rtb_buffs_normal returns 0 again (no KIR has been pressed yet just to clarify).

Issue is around here I think (I've marked the line):

if bone.up and bone.remains == primary then n = n + 1 end

spec:RegisterStateExpr( "rtb_buffs_normal", function ()
    local n = 0
    local primary = rtb_primary_remains

    for _, rtb in ipairs( rtb_buff_list ) do
        local bone = buff[ rtb ]
        if bone.up and bone.remains == primary then n = n + 1 end (Issue is happening here on this line of code)
    end

    return n
end )

Is it because when we use RTB with Loaded Dice while we still have one buff up, the next buffs go up to 39 seconds (if there are 9 or more seconds left on the first buff), but primary is always returning 30 on RTB presses?

so bone.remains == primary can never be true, unless you use RTB with 0 buffs, probably because rtb_primary_remains doesn't return up to extra 9 seconds on top of 30seconds from buff refresh?

It seems like its supposed to based on lastRoll:

spec:RegisterStateExpr( "rtb_primary_remains", function ()
    return max( lastRoll, action.roll_the_bones.lastCast ) + rollDuration - query_time
end )

But maybe there's another bug that's affecting this, will try to find out via further testing.

commented

@IIeTpoc

Changing rtb_primary_remains to this:

spec:RegisterStateExpr( "rtb_primary_remains", function ()
return max( lastRoll ) + rollDuration - query_time
end )

The change to remove 'action.roll_the_bones.lastCast' seems to slightly improve it, but we still don't consistently get a value correctly returned to rtb_buffs_normal when we should when using RTB (with or without Loaded Dice), as rtb_primary_remains timer is occasionally slightly off the bone.remains by half a second when pressing RTB.

It also doesn't always correctly return a value even when bone.remains = rtb_primary_remains is actually true (both exact same value), so there is something else that is happening that I can't see which is preventing rtb_buffs_normal to return a value of 1 or more even when the condition 'if bone.up and bone.remains == primary' is true.

commented

Then we need Hekili to add information about rtb_primary_remains, lastRoll, rollDuration to the snapshot to debug it more thoroughly,

commented

Should be resolved(ish) in 21; otherwise, wait until the first 11.0.5 release comes to open a new ticket.