Skip to content

Fix buff listed in two Cooldown Manager buff groups at once - #1265

Open
dfrisone wants to merge 1 commit into
EllesmereGaming:mainfrom
dfrisone:Ice-Cold-Buff-Bug
Open

Fix buff listed in two Cooldown Manager buff groups at once#1265
dfrisone wants to merge 1 commit into
EllesmereGaming:mainfrom
dfrisone:Ice-Cold-Buff-Bug

Conversation

@dfrisone

@dfrisone dfrisone commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

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.CollectDefaultBuffTrackEntries builds 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 against assignedSpells as stored at add time.

A viewer slot's enumerated spellID is not guaranteed to equal the stored one. GetCanonicalSpellIDForFrame resolves in order GetSpellID() -> GetAuraSpellID() -> per-cooldownID clean cache -> cooldownInfo.spellID/overrideSpellID, so a talent-override swap, or a fresh clean cache while the aura is active (forcing the cooldownInfo fallback), 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 161380 reports GetSpellID() = 44544 while cooldownInfo.spellID/overrideSpellID both report 112965. C_Spell.GetSpellName resolves 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 StoreVariantValue and test it with ResolveVariantValue, the same base/override variant-family matching RebuildSpellRouteMap and ns.GetCDMSpellsForBar already 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):

  • The default buff group offers "Delete Spell" -> Blizzard CDM rather than a direct remove. Blizzard owns tracked-buff visibility; the default group is a mirror of whatever the viewer pool exposes minus other groups' claims.
  • Untracking a buff in Blizzard's CDM removes it from all EUI groups. It destroys the viewer frame every group routes, so there is nothing left to render anywhere.

How was it tested?

Live retail, Mage.

  • Reproduced the double-listing before the fix using the id-split slot above, verified it no longer occurs after.
  • Verified ordinary buffs with no id drift still appear in exactly one group (no regression in the normal exclusion path).
  • Verified right-click on a claiming group still offers "Remove Spell" and removes from that group only.

Diagnostic used to establish the id split, for anyone reproducing:

/run for f in BuffIconCooldownViewer.itemFramePool:EnumerateActive()do local i=f.cooldownInfo print(f.cooldownID,f.GetSpellID and f:GetSpellID(),i and i.spellID,i and i.overrideSpellID)end

Screenshots

N/A -- no visual change. The fix removes an entry that should never have been listed.

Checklist

  • New settings default OFF (no behavior change without opt-in) -- N/A, no new settings; this is a correctness fix to an existing list
  • Zero cost while disabled: no events registered, no polling, no hooks doing work, no frames built -- N/A, no new feature; no new events/hooks/frames
  • Cheap while enabled: event-driven (no polling, no timer-based logic, no per-frame allocations) -- cold path (options page enumeration only), no runtime/per-frame impact
  • No writes onto Blizzard-owned frames (weak-table pattern used); HookScript/hooksecurefunc only, never SetScript on Blizzard frames -- N/A, reads only from our own config tables
  • Tested in-game, works on live retail. Not exercised on the 12.1 PTR client; the change is a pure Lua table-lookup swap using helpers already present in this file, with no API surface that differs between clients.
Shocked Baby GIF

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant