fix(cdm): show keybinds for spells on Action Bars 9 and 10 - #1230
Open
dfrisone wants to merge 1 commit into
Open
fix(cdm): show keybinds for spells on Action Bars 9 and 10#1230dfrisone wants to merge 1 commit into
dfrisone wants to merge 1 commit into
Conversation
The CDM keybind cache scanned only the eight bars that have native binding commands (ACTIONBUTTONn and MULTIACTIONBAR1-7BUTTONn), so a spell sitting on EUI's extra Bar 9 or Bar 10 never got a cache entry and its icon showed no key. Those two bars have no Blizzard binding command: their keys come from the EUI_BAR9_BUTTONn / EUI_BAR10_BUTTONn commands in the Action Bars module's Bindings.xml, routed through the button with SetOverrideBindingClick. Add both prefixes to the scan, at higher priority than bar 1 like every other non-main bar. Because that click route always fires against the button's live "action" attribute, resolve the slot from EABButton<slot> when it exists so a custom-paged Bar 9/10 reads the slot the key actually casts, falling back to the base page slot (13-24 / 109-120) when the Action Bars module is not loaded.
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
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.
Reported by @expor on 8.7.5: with "show keybinds" enabled in the Cooldown Manager, keys displayed for every ability except the ones placed on Action Bar 9. Toggling the CDM option, and toggling keybinds on Bar 9 itself, changed nothing.
Cause
RebuildKeybindCacheenumerates the action bars by binding-command prefix, and listed only the eight bars WoW gives a native command to:Bars 9 and 10 have no native binding command. Their keys come from the
EUI_BAR9_BUTTONn/EUI_BAR10_BUTTONncommands declared in the Action Bars module'sBindings.xml, routed through the button withSetOverrideBindingClick. Those slots were therefore never scanned, no cache entry was ever produced for them, and the icon had nothing to display. That also explains why neither toggle had any effect: the lookup was not reaching those slots at all.The bar count in
BAR_CONFIG(10) and the prefix count in this consumer (8) had simply drifted apart with nothing tying them together.Fix
Add both prefixes to the scan, positioned ahead of
ACTIONBUTTONso the more specific bar wins over bar 1, matching how bars 2 through 8 are already ordered.EUI_BAR9_BUTTONnEUI_BAR10_BUTTONnOne wrinkle beyond the slot ranges: because these two bars have no native command, their keys are always click routed, so a keypress fires against the button's live
actionattribute rather than a fixed slot. The new entries resolve their slot fromEABButton<slot>when that button exists, so a custom paged Bar 9 or Bar 10 reads the slot the key actually casts. They fall back to the base page slot when the Action Bars module is not loaded, which is the same fallback shape the main bar branch already uses for its page lookup.Notes
Read only, plus text writes on our own frames, so it stays safe on the existing debounced in-combat path. No new events, no new strings, no options changes.
Confirmed in game on the reporter's case: keys now show for abilities on Bar 9.
The press mirror hook in
EllesmereUICdmHooks.luahas the same eight bar blind spot, but it rides Blizzard'sMultiActionButtonDown, which Bar 9 and Bar 10 buttons never call. Different feature and a different fix, so it is untouched here.