Fix buff listed in two Cooldown Manager buff groups at once - #1265
Open
dfrisone wants to merge 1 commit into
Open
Fix buff listed in two Cooldown Manager buff groups at once#1265dfrisone wants to merge 1 commit into
dfrisone wants to merge 1 commit into
Conversation
CollectDefaultBuffTrackEntries excluded buffs claimed by other buff groups using an exact spellID match against the stored assignedSpells entry. A viewer slot's enumerated spellID can legitimately differ from what was stored (talent-override swap, or a fresh clean-cache forcing the frame's fallback onto cooldownInfo instead of GetSpellID), so a claimed buff could still pass the exclusion and appear in the default group's editor alongside its real home. Switch the exclusion to the same variant-aware StoreVariantValue / ResolveVariantValue matching RebuildSpellRouteMap and ns.GetCDMSpellsForBar already use elsewhere in this file.
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.
What does this PR do?
Fixes a buff appearing in two Cooldown Manager buff groups at once in the options editor, where it can only be removed from one of them.
Reported symptom: a tracked buff (e.g. Ice Cold) claimed by a custom buff group also shows up in the default buff group's icon strip. Right-clicking the phantom copy offers only "Delete Spell" (which opens Blizzard's CDM, since the default group can't un-track a Blizzard buff), and deleting it there removes the buff from every EUI group rather than the one intended.
Root cause
ns.CollectDefaultBuffTrackEntriesbuilds the default buff group's list from the live BuffIcon viewer pool minus the buffs claimed by other buff-family/hosted bars. That exclusion set was keyed by exact spellID, compared againstassignedSpellsas stored at add time.A viewer slot's enumerated spellID is not guaranteed to equal the stored one.
GetCanonicalSpellIDForFrameresolves in orderGetSpellID()->GetAuraSpellID()-> per-cooldownID clean cache ->cooldownInfo.spellID/overrideSpellID, so a talent-override swap, or a fresh clean cache while the aura is active (forcing thecooldownInfofallback), can yield a different-but-equivalent id. The exact-match exclusion then misses and the claimed buff is re-listed on the default group.Confirmed on a live Mage: buff viewer slot
161380reportsGetSpellID() = 44544whilecooldownInfo.spellID/overrideSpellIDboth report112965.C_Spell.GetSpellNameresolves both to "Fingers of Frost". The same slot, two different ids, no talent change required.The runtime router (
RebuildSpellRouteMap/ResolveCDIDToBar) already handles this correctly via variant-family matching, which is why the bars themselves render fine and the defect appears to be options-only.Fix
Build the exclusion set with
StoreVariantValueand test it withResolveVariantValue, the same base/override variant-family matchingRebuildSpellRouteMapandns.GetCDMSpellsForBaralready use elsewhere in this file. The options list now agrees with runtime routing instead of maintaining a second, weaker notion of buff identity.Not changed (both are working as designed, and were only reachable here because of the phantom listing):
How was it tested?
Live retail, Mage.
Diagnostic used to establish the id split, for anyone reproducing:
Screenshots
N/A -- no visual change. The fix removes an entry that should never have been listed.
Checklist
HookScript/hooksecurefunconly, neverSetScripton Blizzard frames -- N/A, reads only from our own config tables