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

[REC] Frost Mage Winter's Chill debuff tracking, IsInFlight, virtualQueue, realQueue #4267

Open
5 tasks done
mwojtkowski opened this issue Dec 25, 2024 · 9 comments
Open
5 tasks done
Assignees
Labels
📈 recommendations Ticket is a proposed change to action priorities. 🧑‍⚕️ triage Issue needs initial triage to determine if action needs to be taken.

Comments

@mwojtkowski
Copy link

mwojtkowski commented Dec 25, 2024

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

Mage - Frost

Describe the Issue

Addon sometimes behaves like the target still has Winter's Chill stacks, thus displaying wrong spells to cast.

Because Winter's Chill stacks aren't removed when a spell is cast but on spell impact, Frost Mage module uses several ways to predict remaining Winter's Chill stacks.

Most spells have an on impact function with removeDebuffStack( "target", "winters_chill" )

and also function to calculate remaining Winter's Chill stacks if spells are in flight

spec:RegisterStateExpr( "remaining_winters_chill", function ()
    local wc = debuff.winters_chill.stack

    if wc == 0 then return 0 end

    local projectiles = 0

    if prev_gcd[1].ice_lance and state:IsInFlight( "ice_lance" ) then projectiles = projectiles + 1 end
    if prev_gcd[1].frostbolt and state:IsInFlight( "frostbolt" ) then projectiles = projectiles + 1 end
    if prev_gcd[1].glacial_spike and state:IsInFlight( "glacial_spike" ) then projectiles = projectiles + 1 end

    return max( 0, wc - projectiles )
end )

What I observed is that the above function return value changes rapidly, jumping back and forth between values until the spell actually hits the target.

So I've made weakaura that prints out on every frame:

  1. Hekili.State:IsInFlight for Ice Lance, Frostbolt, Glacial Spike (both virtualQueue and realQueue)
  2. Hekili.State.debuff.winters_chill.stack and Hekili.State.remaining_winters_chill
  3. all entries in Hekili.State.queue and Hekili.State.realQueue

What I observed is that when spamming instant cast like Ice Lance

  1. If you cast Ice Lance and wait for it to hit the target (only one button press):
    • state.virtualQueue will be empty
    • state.realQueue will have an entry (PROJECTILE_IMPACT) for Ice Lance which will be removed when Ice Lance hits the target
  2. If you spam Ice Lance button:
    • state.virtualQueue will be empty
    • The first button press (spell cast) will add Ice Lance to state.realQueue but any subsequent button presses while CGD is up will remove Ice Lance from state.realQueue before Ice Lance hits the target
    • Spamming the button will also cause Hekili.State.debuff.winters_chill.stack to reset back to the previous value while Ice Lance is in flight.

Now because of problems in that 2nd point, if you're casting Frostbolt/Glacial Spike and start spamming Ice Lance to queue it right after Frostbolt/Glacial Spike, Hekili will report remaining_winters_chill = 2 or 1 (with both Frostbolt and Ice Lance in flight)

After testing with Bloodlust/Time Warp, it appears that if GCD is low enough, Ice Lance will appear in the virtualQueue, but spamming Ice Lance still removes it from both virtualQueue and realQueue before it lands on the target.

How to Reproduce

  1. Enter game as Frost Mage
  2. Disable cooldowns and minor cooldowns (for easier way to demonstrate the problem)
  3. Make sure you have no icicle stacks
  4. Hit target with Flurry
  5. Cast Comet Storm (if talented) and start spamming Ice Lance
  6. Addon will suggest Comet Storm (if talented) -> Ice Lance -> Ice Lance -> Ice Lance (untill second Ice Lance hits the target then it will swap to Frostbolt)
  7. Addon should suggest in order Comet Storm (if talented) -> Ice Lance -> Ice Lance -> Frostbolt

Snapshot (Link)

Snapshot was taken when 2nd Ice Lance was still in flight and didn't hit the target
https://pastebin.com/Pz5LUJhW

Raidbots Sim Report (Link)

No response

Additional Information

No response

Contact Information

No response

@mwojtkowski mwojtkowski added 📈 recommendations Ticket is a proposed change to action priorities. 🧑‍⚕️ triage Issue needs initial triage to determine if action needs to be taken. labels Dec 25, 2024
@mwojtkowski mwojtkowski changed the title [REC] Provide a short, descriptive title of you Recommendation issue [REC] Frost Mage Winbter's Chill debuff tracking, IsInFlight, virtualQueue, realQueue Dec 25, 2024
@mwojtkowski mwojtkowski changed the title [REC] Frost Mage Winbter's Chill debuff tracking, IsInFlight, virtualQueue, realQueue [REC] Frost Mage Winter's Chill debuff tracking, IsInFlight, virtualQueue, realQueue Dec 25, 2024
@syrifgit
Copy link
Collaborator

syrifgit commented Jan 4, 2025

Some changes have been made for projectiles. After the next addon update, if it's still not working right just comment and I can reopen the ticket.

@syrifgit syrifgit closed this as completed Jan 4, 2025
@mwojtkowski
Copy link
Author

mwojtkowski commented Jan 6, 2025

@syrifgit Thank you for responding.

I did some more testing (version from this commit 5e1c050), this time also using /etrace and WA (combat log event unfiltered), and I think I found why I saw such weird behavior from state.realQueue.

Spamming instant spell casts (Ice Lance, Flurry, Comet Storm) while GCD is up triggers SPELL_CAST_FAILED subEvent in CLEU.
Spell is then removed from the queue because of this part in CLEU_HANDLER (Events.lua)

elseif subtype == "SPELL_CAST_FAILED" then
    state:RemoveSpellEvent( ability.key, true, "CAST_FINISH" ) -- remove next cast finish.
    if ability.isProjectile then state:RemoveSpellEvent( ability.key, true, "PROJECTILE_IMPACT", true ) end -- remove last impact.
    -- Hekili:ForceUpdate( "SPELL_CAST_FAILED" )

this is /etrace when casting Ice Lance, last 3 lines are when I spammed Ice Lance when GCD was still up
Screenshot 2025-01-06 000542

Three casts of Ice Lance while spamming Ice Lance button
WA with trigger CLEU:SPELL_CAST_FAILED, CLEU:SPELL_CAST_SUCCESS
It prints GetTime() Event:SubEvent SpellID
Screenshot 2025-01-06 001745

In a situation where we have 2 stacks of Winter's Chill

  1. state.debuff.winters_chill.stack has a value of 2
  2. We cast Ice Lance, which triggers SPELL_CAST_SUCCESS
  3. SPELL_CAST_SUCCESS adds Ice Lance to state.realQueue and state:IsInFlight( "ice_lance", true ) will return true
  4. state.debuff.winters_chill.stack now has a value of 1
  5. GCD is up, and we are still spamming Ice Lance button, this triggers SPELL_CAST_FAILED
  6. SPELL_CAST_FAILED removes Ice Lance from the queue before it hits the target; state:IsInFlight( "ice_lance", true ) will return false
  7. state.debuff.winters_chill.stack is now back to a value of 2
  8. Ice Lance hits the target
  9. state.debuff.winters_chill.stack now has a value of 1

So in normal fight encounters where people tend to spam abilities to queue them right after spell casts (Frostbolt, Glacial Spike, etc.) will make Hekili suggest spells that would rely on the target having the Winter's Chill debuff.

I've added if not real and #queue > 0 then print(#queue) end inside IsInFlight function to check if Ice Lance will be added to virtualQueue, and virtualQueue is empty inside that function when casting Ice Lance (it works fine for Frostbolt, Glacial Spike) which results in state:IsInFlight( "ice_lance" ) returning wrong value

Another thing with Winter's Chill debuff (state.debuff.winters_chill.stack) is that it is reduced on Ice Lance cast (SPELL_CAST_SUCCESS) but it should be on projectile impact like with Frostbolt

@syrifgit
Copy link
Collaborator

syrifgit commented Jan 6, 2025

@Hekili

@syrifgit syrifgit reopened this Jan 6, 2025
@Hekili
Copy link
Owner

Hekili commented Jan 7, 2025

Thanks for the extended analysis here.I think the SPELL_CAST_FAILED bit is the most significant piece; Ice Lance is instant so SPELL_CAST_FAILED will never fire for things like movement, just failure to start casting. I'm going to test functionality with that section removed.

@yokaii4
Copy link

yokaii4 commented Jan 7, 2025

Thanks for the extended analysis here.I think the SPELL_CAST_FAILED bit is the most significant piece; Ice Lance is instant so SPELL_CAST_FAILED will never fire for things like movement, just failure to start casting. I'm going to test functionality with that section removed.

can this be something that can be applied to all of those specs with instant casts like IL that have a similar sequence as frost? Would be a huge improvement overall if it is

@mwojtkowski
Copy link
Author

Looking at the CLEU_HANDLER function, you have access to the abilities table. Maybe adding a check to see if a spell is an instant cast would solve this problem?

something like

elseif subtype == "SPELL_CAST_FAILED" and ability.cast > 0 then

@yokaii4
Copy link

yokaii4 commented Jan 7, 2025

Looking at the CLEU_HANDLER function, you have access to the abilities table. Maybe adding a check to see if a spell is an instant cast would solve this problem?

something like

elseif subtype == "SPELL_CAST_FAILED" and ability.cast > 0 then

I unfortunately cannot help from a tech standpoint because I have 0 programming experience, I was just the one sending over the frost feedback. I would say that this could be huge especially for specs like fire mage, for example, where the addon might work way better if we can find an order for instant casts like IL, or even queuecasting situations like what happens with flurry

@nikolavuljan
Copy link

Looking at the CLEU_HANDLER function, you have access to the abilities table. Maybe adding a check to see if a spell is an instant cast would solve this problem?

something like

elseif subtype == "SPELL_CAST_FAILED" and ability.cast > 0 then

One sidenote is that I dont know how this would work on proc-instant spells like pyroblast.

I'm not an expert but this could also be affecting the phoenix flames/pyroblast and maybe even others that seem inconsistantly included/excluded from the "hot_streak_spells_in_flight".

@mwojtkowski
Copy link
Author

mwojtkowski commented Jan 9, 2025

I've noticed that sometimes when casting Frostbolt, Winter's Chill debuff counter also changes in value.

and after a bit of testing, I saw that spells with cast time can also trigger SPELL_CAST_FAILED when spamming the button
Screenshot 2025-01-09 201906

@mwojtkowski mwojtkowski mentioned this issue Jan 21, 2025
30 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 recommendations Ticket is a proposed change to action priorities. 🧑‍⚕️ triage Issue needs initial triage to determine if action needs to be taken.
Projects
None yet
Development

No branches or pull requests

5 participants