Fix(Action Bars): Lua error during delve vehicle combat from cooldown-event taint - #1975
Open
dfrisone wants to merge 1 commit into
Open
Fix(Action Bars): Lua error during delve vehicle combat from cooldown-event taint#1975dfrisone wants to merge 1 commit into
dfrisone wants to merge 1 commit into
Conversation
…ooldowns Registering Blizzard's own ActionBarButtonEventsFrame for ACTIONBAR_UPDATE_COOLDOWN and SPELL_UPDATE_COOLDOWN put our taint on their dispatch, which runs ActionButton_UpdateCooldown into SetCooldown -- the same secret-cooldown-under-taint chain already fixed once for our own buttons -- and left the rest of that tick's native call stack tainted, the objective tracker included. It reproduced during vehicle combat in a delve. Both events come off Blizzard's shared broadcaster and go onto our own bar frame instead, repainting the seven Blizzard-owned buttons through ForceCooldownPaint, which is already the established secret-safe path. That keeps the coverage full mode had -- including the spell-typed delve buttons that carry no action slot and so fire SPELL_UPDATE_COOLDOWN rather than the action-bar event -- without our taint ever reaching their dispatch. The repaint returns immediately unless a vehicle or extra-action button is actually up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug: reported via Svart; fix authored by Svart (svart2521) and re-submitted here on his behalf while his GitHub account is suspended.
Issue: a Lua error during vehicle combat in a delve, with the damage spreading past the action bars -- the objective tracker was caught in the same tick. It only reproduced in a vehicle, which is what made it look like a delve-specific problem rather than a cooldown one.
Root cause: EllesmereUIActionBars.lua registered Blizzard's own ActionBarButtonEventsFrame for ACTIONBAR_UPDATE_COOLDOWN and SPELL_UPDATE_COOLDOWN. Registering their shared frame puts our taint on their dispatch, and that dispatch runs ActionButton_UpdateCooldown into SetCooldown -- the same secret-cooldown-under-taint chain already fixed once for our own buttons in QuietlyHideBlizzButton's OnShow hook. Once it fires, the rest of that tick's native call stack is tainted, which is why an unrelated frame like the objective tracker ends up in the error.
Fix: take both events off Blizzard's broadcaster and register them on our own bar frame, repainting the seven Blizzard-owned buttons (OverrideActionBarButton1-6 and ExtraActionButton1) through ForceCooldownPaint, which is already the established secret-safe path. Coverage is unchanged, including the spell-typed delve buttons that carry no action slot and therefore fire SPELL_UPDATE_COOLDOWN rather than the action-bar event, and the repaint returns immediately unless a vehicle or extra-action button is actually up.