search

vendethiel / GladiusEx

13 stars 19 forks source link

Fix detached cooldown groups only showing spells for arena1 #33

Closed versayaa closed 3 years ago

versayaa commented 3 years ago

Currently, detached cooldown groups seem to only show the cooldowns for arena1. This can be fixed by disabling- and re-enabling the Detached Group, but only until you reload.

A related issue: https://www.curseforge.com/wow/addons/gladiusex/issues/92

It seems the spells for arena2 and arena3 were not being updated, because gs.frame was never set for them (unless you've had the Detached Group option disabled earlier that session).

This change allows UpdateGroupIcons to fully run for all units, not just arena1.

vendethiel commented 3 years ago

Thanks! I'll take a look soon, I remember that issue.

versayaa commented 3 years ago

Apologies, it seems I've been a bit to quick opening this PR. While this does cause the icons to show up, they do not properly go on cooldown. I'll close this for now and investigate further!

vendethiel commented 3 years ago

The way it's supposed to work is that you only ever update detached cooldown groups from "header units".

versayaa commented 3 years ago

The way it's supposed to work is that you only ever update detached cooldown groups from "header units".

Ah, I see. I think the issue might be that UpdateGroupIcons is only ever calling GetCooldownList for unit, which will thus always be arena1/player. I added a loop there to iterate over the other units, and to add their cooldowns to detached_spells.

It also seems dunit was always nil, so detached_spells did not contain the unit, only the spellId, and therefore UpdateGroupIconFrames fell back to arena1/player for icon_unit, meaning it would look up the cooldown for the wrong unit. https://github.com/vendethiel/GladiusEx/blob/bc0f90725fb0ad57d605f9c1446dfa4047eb6cf9/modules/cooldowns.lua#L804 https://github.com/vendethiel/GladiusEx/blob/bc0f90725fb0ad57d605f9c1446dfa4047eb6cf9/modules/cooldowns.lua#L718

After fixing these issues, I noticed that there was often a delay before a spell for non-arena1/player units went on cooldown. I believe this is because the LibCooldownTracker callbacks for non-arena1/player units were effectively ignored, and so the cooldown icons would only update once an event for arena1/ player itself came in (or some other event that triggers UpdateIcons for it). I therefor changed the unit to GetHeaderUnit(unit) at the start of UpdateGroupIcons for detached groups.

I've done a handful of skirimishes, and it seems to work now. I'll do some more testing tomorrow!