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).

Describe the Issue

Scorch has a tiny grace period between when it hits and when it actually changes the state of the 'Hot Streak' buff. Because of this, simc considers scorch to be a valid ability to count for hot_streak_spells_in_flight. Relevant code in the simc codebase here.

At present, Hekili incorrectly doesn't consider Scorch when counting up hot_streak_spells_in_flight.

I've attached a simple sim report using a modified Fire Mage profile. Its set to only Pyroblast if Hot Streak is up and hot_streak_spells_in_flight is truthy. You'll notice that it Pyroblasts many times, indicating that simc considers a Scorch to be countable for hot_streak_spells_in_flight.

How to Reproduce

Use the following profile:

actions=/pyroblast,if=buff.hot_streak.react&hot_streak_spells_in_flight
actions+=/scorch

1. Cast Fire Blast twice to build up Hot Streak
2. Use Combustion
3. Start casting Scorch

Expected Behavior:
The Scorch is counted for hot_streak_spells_in_flight, so a Pyroblast gets recommended

Actual Behavior
Scorch is recommended

Snapshot (Link)

https://pastebin.com/gEiyJude

Raidbots Sim Report (Link)

https://www.raidbots.com/simbot/report/pppvgMN4LbttJWyfBAC8xW/simc

Additional Information

No response

Contact Information

Discord: stntz

@phatzz I'll make a fork tonight with this change. Don't use the code in my comment, it has an issue or two.

I ended up with this:

You'd probably be better off tweaking the flightTime variable on Scorch and just uncommenting "scorch," in hot_streak_spells. What happens if you increase flightTime to something like 0.03 or 0.05?

If you're comfortable, you can try adding flightTime = 0.015, to the scorch model in MageFire.lua and changing the handler to impact and see if that makes the difference you're expecting.

I'm still not convinced this is the right angle of attack. We're potentially making the addon more like SimC, but less like the actual game.

Understood. I wanted to investigate this to eliminate as many issues from mage specs as possible.

I ended up with this:

spec:RegisterStateExpr( "hot_streak_spells_in_flight", function ()
    local count = 0

    for i, spell in ipairs( hot_streak_spells ) do
        if state:IsInFlight( spell ) then count = count + 1 end
    end

    local ability = class.abilities[ "scorch" ]
    local time_since_last_cast = max(0, state.query_time - ability.lastCast )   
    if ability.lastCast > 0 and time_since_last_cast > 0 and time_since_last_cast < 0.015 then
        count = count + 1
    end

    return count
end )

This preserves all the existing behavior of Scorch, but adds in specific support for the hot_streak_spells_in_flight.

I'll take a look at the rounding issue, because I'm still getting some recommendation swapping even with the expression change.

I made those two changes. I think you also have to add "scorch" to the hot_streak_spells table, also in MageFire.lua. I then tested with this profile:

actions=/shifting_power,if=hot_streak_spells_in_flight
actions+=/scorch

Intent here being that Shifting Power should be displayed only if Scorch is in flight.

Result: the recommendation display rapidly flickered between recommending Shifting Power and Scorch. Seems like it sometimes thinks a Scorch is in flight, other times it doesn't. Screen recording below.

I managed to capture snapshots of when Shifting Power was recommended and when Scorch was recommended.

Shifting Power recommendation: https://pastebin.com/ufPUMXhK
Scorch recommendation: https://pastebin.com/asYUx5n6

Note that this was recorded with the 0.2s event batching disabled.

Processing precombat action list [ testfire - precombat ].

Current recommendation was NO ACTION at +10.00s.
The current minimum delay (0.00) is greater than the current maximum delay (-0.00). Exiting list (precombat).
Exiting precombat with recommendation of NO ACTION at +10.00s.

Likely some messiness related to rounding. You could try messing with the rounding in State.lua when the clock is advanced:

That 2 on the last line might need to be a 3 or 4 for time increments of 0.015 to be relevant. Or the rounding itself might need to be dropped. Not sure.

I'm still not convinced this is the right angle of attack. We're potentially making the addon more like SimC, but less like the actual game.

I'm still not convinced this is the right angle of attack. We're potentially making the addon more like SimC, but less like the actual game.

Understood. I wanted to investigate this to eliminate as many issues from mage specs as possible.

I ended up with this:

spec:RegisterStateExpr( "hot_streak_spells_in_flight", function ()
    local count = 0

    for i, spell in ipairs( hot_streak_spells ) do
        if state:IsInFlight( spell ) then count = count + 1 end
    end

    local ability = class.abilities[ "scorch" ]
    local time_since_last_cast = max(0, state.query_time - ability.lastCast )   
    if ability.lastCast > 0 and time_since_last_cast > 0 and time_since_last_cast < 0.015 then
        count = count + 1
    end

    return count
end )

This preserves all the existing behavior of Scorch, but adds in specific support for the hot_streak_spells_in_flight.

I'll take a look at the rounding issue, because I'm still getting some recommendation swapping even with the expression change.

Any chance there is a pull request available, i wouldn't mind testing mage with this change implemented

Oh, interesting catch. The question is, how will it actually play since the addon will never really make a recommendation inside the grace period that you could react to before it updates again for the impact. But I can set a fixed travel time on Scorch. What does simc use?

Simc uses a travel time of 15ms. Relevant code.

Regarding whether this is actually useful in the addon: yes, it is. While I'm casting scorch, I should be queuing up my next spell to be casted. If I have a spellqueuewindow of 200ms set, then in the last 200ms of my scorch cast, I can queue up an ability that will be executed the instant scorch finishes casting.

The key here is that when scorch finishes casting and relevant buffs are up (Combust or Searing Touch talent is active), that queued spell will be casted in a window where hot_streak_spells_in_flight == 1. With that in mind, while I'm casting scorch, and Hekili is giving me a recommendation for an ability to press right now, which will be casted at the start of the next available gcd, Hekili should consider hot_streak_spells_in_flight == 1. This change wouldn't introduce a window where a recommendation is only valid for 15ms. The window where the recommendation is valid is the entire duration of the Scorch cast.