From a9a9c6885ce45dd914c93f35968824982aa3a435 Mon Sep 17 00:00:00 2001 From: Andrea Bergonzo Date: Mon, 7 Sep 2026 03:39:54 +0100 Subject: [PATCH 1/4] fix(cdm): preserve independent cooldown viewer slots Use cooldownID claims for collided spell families while retaining ordinary talent override behavior. Preserve claims across missing siblings and protect inactive buff placements during repopulation. Validate with mocked production picker, routing and repopulation regressions and Lua syntax checks. Claude Fable 5.1 review started but hit the session limit before a verdict; in-game testing remains pending. --- .../EllesmereUICdmHooks.lua | 43 ++-- .../EllesmereUICdmSpellPicker.lua | 180 +++++++++++++-- .../EllesmereUICooldownManager.lua | 17 +- .../EUI_CooldownManager_Options.lua | 21 +- tests/README.md | 12 + tests/cdm_collision_claims.lua | 207 ++++++++++++++++++ tests/run_cdm_collision_tests.py | 47 ++++ 7 files changed, 487 insertions(+), 40 deletions(-) create mode 100644 tests/README.md create mode 100644 tests/cdm_collision_claims.lua create mode 100644 tests/run_cdm_collision_tests.py diff --git a/EllesmereUICooldownManager/EllesmereUICdmHooks.lua b/EllesmereUICooldownManager/EllesmereUICdmHooks.lua index ae20d58c9..3738fccba 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmHooks.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmHooks.lua @@ -961,10 +961,10 @@ local _cdidRouteMap = {} local _divertedSpellsBuff = {} local _divertedSpellsCD = {} --- cooldownID-level buff diversions: a collided buff (two viewer slots sharing one --- canonical spellID) is tracked on a custom bar by cooldownID (cd-claim marker in --- assignedSpells, ns.CdClaimMarker); checked BEFORE the sid map, so it outranks a pair claim. -local _divertedBuffCdIDs = {} +-- CooldownID-level diversions: collided Buff or CD/Utility viewer slots are +-- tracked by cooldownID (cd-claim marker in assignedSpells, ns.CdClaimMarker). +-- Checked before the sid maps so one family claim never captures its sibling. +local _divertedCdIDs = {} --- Equipment-slot diversions, inventory slot -> barKey. Blizzard's own equipment --- cooldown entry carries an equipSlot and NO spell of its own, so the slot is its --- only routing key; a bar listing that slot (-13/-14 et al) claims the frame the @@ -1148,7 +1148,7 @@ function ns.RebuildSpellRouteMap() wipe(_divertedDirectCD) wipe(_divertedVarBaseBuff) wipe(_divertedVarBaseCD) - wipe(_divertedBuffCdIDs) + wipe(_divertedCdIDs) wipe(ns._divertedSlotCD) _routeMapBuilt = false @@ -1230,7 +1230,7 @@ function ns.RebuildSpellRouteMap() local claims = sd and ns.CollectCdClaimSet(sd) if claims then for cdID in pairs(claims) do - _divertedBuffCdIDs[cdID] = bd.key + _divertedCdIDs[cdID] = bd.key end end end @@ -1277,12 +1277,12 @@ function ns.RebuildSpellRouteMap() end -- Cd-claimed hosted buffs (collided slots hosted by cd-claim marker instead -- of the sid-keyed hostedBuffSpellIDs flag): claim the cooldownID in - -- _divertedBuffCdIDs, same map/priority as Pass 1. ResolveCDIDToBar checks + -- _divertedCdIDs, same map/priority as Pass 1. ResolveCDIDToBar checks -- it before any sid map, so it works for any target bar type. local claims = sd and ns.CollectCdClaimSet(sd) if claims then for cdID in pairs(claims) do - _divertedBuffCdIDs[cdID] = bd.key + _divertedCdIDs[cdID] = bd.key end end end @@ -1323,6 +1323,11 @@ function ns.RebuildSpellRouteMap() for _, bd in ipairs(p.cdmBars.bars) do if bd.enabled and bd.isGhostBar then CollectDiversionsFor(bd, ghostAliasSkip) + local claims = ns.GetBarSpellData(bd.key) + claims = claims and ns.CollectCdClaimSet(claims) + if claims then + for cdID in pairs(claims) do _divertedCdIDs[cdID] = bd.key end + end end end @@ -1349,14 +1354,12 @@ local function ResolveCDIDToBar(cdID, viewerDefaultBar) local cached = _cdidRouteMap[cdID] if cached then return cached end - -- cooldownID-level claim first (collided buffs tracked by slot). Needs no + -- cooldownID-level claim first (collided Buff or CD/Utility slot). Needs no -- cooldownInfo read, so it also works while every sid field is secret. - if viewerDefaultBar == "buffs" then - local cdRoute = _divertedBuffCdIDs[cdID] - if cdRoute then - _cdidRouteMap[cdID] = cdRoute - return cdRoute - end + local cdRoute = _divertedCdIDs[cdID] + if cdRoute then + _cdidRouteMap[cdID] = cdRoute + return cdRoute end local RVV = ns.ResolveVariantValue @@ -8499,6 +8502,16 @@ local function CollectAndReanchor() if ns.QueueReanchor then ns.QueueReanchor() end end end + -- Live-only compatibility pass: when two CD/Utility cooldownIDs share + -- one spell family, replace a legacy positive family claim with one + -- exact slot marker. The sibling then keeps its Blizzard viewer home. + if prof and prof._barFilterModelV6 and ns.MigrateCollidedCDAssignments then + local migrated = ns.MigrateCollidedCDAssignments() + if migrated and migrated > 0 then + if ns.RebuildSpellRouteMap then ns.RebuildSpellRouteMap() end + if ns.QueueReanchor then ns.QueueReanchor() end + end + end -- Automatic base-bar materialization (once per spec+layout per session): -- untouched default-bar spells render through the frames-as-truth fallback -- without ever being recorded in assignedSpells, so export strings shipped diff --git a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua index 93f7b629a..2e6c128a4 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua @@ -373,7 +373,6 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) end local result = {} - local seen = {} local viewerOrder = 0 local entries = {} @@ -383,17 +382,7 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) for frame in viewer.itemFramePool:EnumerateActive() do if frame:IsShown() or frame.cooldownInfo then local sid = GetCanonicalSpellIDForFrame(frame) - -- Dedup identity: BUFF viewer can have two cooldownIDs share - -- one spellID (e.g. Diabolist: Demonic Art vs Diabolic - -- Ritual), so key on cooldownID there (sid-dedup would - -- wrongly merge them); CD/util viewers keep sid-dedup to - -- collapse a spell shown in two viewers. Non-colliding specs - -- are unaffected: a unique sid there implies a unique cooldownID. - local cd = frame.cooldownID - local dkey = (includeBuffViewer and type(cd) == "number") - and ("c" .. cd) or sid - if _IsUsableSID(sid) and dkey ~= nil and not seen[dkey] then - seen[dkey] = true + if _IsUsableSID(sid) then entries[#entries + 1] = { sid = sid, cdID = frame.cooldownID, @@ -414,13 +403,144 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) return a.sid < b.sid end) - for i, e in ipairs(entries) do - result[i] = e -- preserve metadata for picker + -- Blizzard can expose two distinct CD/Utility slots whose spell IDs belong + -- to one base/override family. Those slots must retain cooldownID identity; + -- ordinary single-slot families stay spell-keyed so talent swaps remain + -- stable. Buff entries have always been cooldownID-keyed for the same reason. + local collided = {} + if not includeBuffViewer then + -- A saved slot claim keeps its identity while its sibling is untalented + -- or absent from the live pool. Re-adding must move the existing marker. + local p = ECME and ECME.db and ECME.db.profile + for _, bd in ipairs(p and p.cdmBars and p.cdmBars.bars or {}) do + local sd = ns.GetBarSpellData and ns.GetBarSpellData(bd.key) + local claims = sd and ns.CollectCdClaimSet and ns.CollectCdClaimSet(sd) + for cdID in pairs(claims or {}) do collided[cdID] = true end + end + for i = 1, #entries do + local a = entries[i] + if type(a.cdID) == "number" then + for j = i + 1, #entries do + local b = entries[j] + if type(b.cdID) == "number" and a.cdID ~= b.cdID + and IsVariantOf(a.sid, b.sid) then + collided[a.cdID] = true + collided[b.cdID] = true + end + end + end + end + end + + local seen = {} + for _, e in ipairs(entries) do + local cdKeyed = includeBuffViewer or (e.cdID and collided[e.cdID]) + local dkey = cdKeyed and type(e.cdID) == "number" and ("c" .. e.cdID) or e.sid + if dkey ~= nil and not seen[dkey] then + seen[dkey] = true + e.isCdCollision = not includeBuffViewer and collided[e.cdID] == true + result[#result + 1] = e + end end return result end ns.EnumerateCDMViewerSpells = EnumerateCDMViewerSpells +function ns.IsBuffViewerCdID(cdID) + if type(cdID) ~= "number" then return false end + for _, e in ipairs(EnumerateCDMViewerSpells(true)) do + if e.cdID == cdID then return true end + end + for _, e in ipairs(EnumerateCDMViewerSpells(false)) do + if e.cdID == cdID then return false end + end + -- Untalented slots can lack a live frame. Static family membership is + -- sufficient here; the arranged category is only needed for bar placement. + local gci = C_CooldownViewer and C_CooldownViewer.GetCooldownViewerCooldownInfo + local info = gci and gci(cdID) + local cat = info and info.category + if type(cat) == "number" and not (issecretvalue and issecretvalue(cat)) then + if ns.CDM_BUFF_CATS[cat] then return true end + if ns.CDM_ICON_CD_CATS[cat] then return false end + end + return nil -- Unavailable metadata does not prove a legacy buff is a cooldown. +end + +--- Convert legacy positive spell-family assignments into cooldownID claims when +--- the live CD/Utility viewers prove that multiple slots currently collide. +--- Idempotent and deliberately unflagged: a collision may appear only after a +--- talent swap. Exact stored spell matches win, then the slot already shown in +--- the bar's Blizzard viewer, preserving the old layout with minimum movement. +function ns.MigrateCollidedCDAssignments() + local p = ECME and ECME.db and ECME.db.profile + local bars = p and p.cdmBars and p.cdmBars.bars + if type(bars) ~= "table" then return 0 end + + local entries = EnumerateCDMViewerSpells(false) + local collided = {} + for _, e in ipairs(entries) do + if e.isCdCollision and type(e.cdID) == "number" then + collided[#collided + 1] = e + end + end + if #collided == 0 then return 0 end + + local usedCd = {} + for _, bd in ipairs(bars) do + local sd = bd.key and ns.GetBarSpellData(bd.key) + local claims = sd and ns.CollectCdClaimSet and ns.CollectCdClaimSet(sd) + if claims then + for cdID in pairs(claims) do usedCd[cdID] = true end + end + end + + local changed = 0 + for _, bd in ipairs(bars) do + local isCdFamily = bd.key == "__ghost_cd" + or (not (ns.IsBarBuffFamily and ns.IsBarBuffFamily(bd)) + and bd.barType ~= "custom_buff") + local sd = isCdFamily and bd.key and ns.GetBarSpellData(bd.key) + local list = sd and sd.assignedSpells + if list then + for i = 1, #list do + local sid = list[i] + if type(sid) == "number" and sid > 0 + and not (sd.customSpellIDs and sd.customSpellIDs[sid]) + and not (sd.spellDurations and sd.spellDurations[sid]) + and not (sd.customSpellDurations and sd.customSpellDurations[sid]) + and not (sd.hostedBuffSpellIDs and sd.hostedBuffSpellIDs[sid]) then + local best, bestRank + for _, e in ipairs(collided) do + if not usedCd[e.cdID] and IsVariantOf(sid, e.sid) then + local exact = sid == e.sid + local barType = ns.GetBarType and ns.GetBarType(bd) + local viewerMatch = (barType == "cooldowns" + and e.viewerName == "EssentialCooldownViewer") + or (barType == "utility" + and e.viewerName == "UtilityCooldownViewer") + local rank = (exact and viewerMatch) and 1 + or (exact and 2) + or (viewerMatch and 3) + or 4 + if not best or rank < bestRank + or (rank == bestRank and (e.layoutIndex or 0) < (best.layoutIndex or 0)) then + best, bestRank = e, rank + end + end + end + if best then + list[i] = ns.CdClaimMarker(best.cdID) + usedCd[best.cdID] = true + changed = changed + 1 + end + end + end + end + end + if changed > 0 then ns._spellOrderDirty = true end + return changed +end + -- Unified spell list helpers: ONE add path and ONE remove path for every CDM -- bar's assignedSpells list (default/custom/ghost bars). Variant-aware via -- IsVariantOf, so adding the same spell under a different variant ID is a @@ -487,6 +607,8 @@ function ns.RemoveSpellFromBar(barKey, spellID) if not idx then return nil end local removed = table.remove(sd.assignedSpells, idx) ns._spellOrderDirty = true + local removedCd = ns.CdClaimMarkerToCdID and ns.CdClaimMarkerToCdID(removed) + if removedCd and sd.hostedBuffCdIDs then sd.hostedBuffCdIDs[removedCd] = nil end -- Clean up auxiliary per-spell metadata for the removed entry if sd.customSpellDurations then sd.customSpellDurations[removed] = nil end if sd.spellDurations then sd.spellDurations[removed] = nil end @@ -610,6 +732,7 @@ function ns.GetCDMSpellsForBar(barKey, includeUntalented) -- Variant-keyed lookup of spells already on THIS bar (for onEUIBar flag). local ourPool = {} local sd = ns.GetBarSpellData(barKey) + local ourCdClaims = sd and ns.CollectCdClaimSet and ns.CollectCdClaimSet(sd) if sd and sd.assignedSpells then for _, sid in ipairs(sd.assignedSpells) do if sid and sid ~= 0 then @@ -627,7 +750,9 @@ function ns.GetCDMSpellsForBar(barKey, includeUntalented) local name = C_Spell.GetSpellName(sid) local tex = C_Spell.GetSpellTexture(sid) if name then - local isOnThisBar = (ResolveVariantValue(ourPool, sid) == true) + local isOnThisBar = e.isCdCollision and e.cdID and ourCdClaims + and ourCdClaims[e.cdID] == true + or (not e.isCdCollision and ResolveVariantValue(ourPool, sid) == true) spells[#spells + 1] = { cdID = e.cdID, spellID = sid, @@ -637,6 +762,7 @@ function ns.GetCDMSpellsForBar(barKey, includeUntalented) cdmCatGroup = isBuffType and "buff" or "cooldown", onEUIBar = isOnThisBar, isKnown = true, -- live viewer pool members are always learned + isCdCollision = e.isCdCollision, } end end @@ -1772,6 +1898,14 @@ function ns.AddTrackedBuffByCdID(barKey, cdID) return ns.AddTrackedSpell(barKey, ns.CdClaimMarker(cdID)) end +--- Track one collided CD/Utility viewer slot by cooldownID. Callers use this +--- only for entries marked isCdCollision by live viewer enumeration; ordinary +--- spells continue through AddTrackedSpell and retain talent-stable identity. +function ns.AddTrackedCooldownByCdID(barKey, cdID) + if type(cdID) ~= "number" or cdID <= 0 or IsBarBuffFamily(barKey) then return false end + return ns.AddTrackedSpell(barKey, ns.CdClaimMarker(cdID)) +end + function ns.RemoveTrackedBuffCdID(barKey, cdID) if type(cdID) ~= "number" then return false end -- RemoveSpellFromBar doesn't itself trigger route/reanchor; caller must. @@ -1833,6 +1967,8 @@ function ns.AddHostedBuffByCdID(barKey, cdID) -- skip pricier frame-flag checks. A cd-claimed hosted buff resolves its -- own "c"..cooldownID key independently -- only the table's existence matters. sd.hostedBuffSpellIDs = sd.hostedBuffSpellIDs or {} + sd.hostedBuffCdIDs = sd.hostedBuffCdIDs or {} + sd.hostedBuffCdIDs[cdID] = true return ns.AddTrackedSpell(barKey, ns.CdClaimMarker(cdID)) end @@ -1841,6 +1977,8 @@ function ns.RemoveHostedBuffByCdID(barKey, cdID) -- RemoveSpellFromBar doesn't itself trigger route/reanchor; caller must. local removed = ns.RemoveSpellFromBar(barKey, ns.CdClaimMarker(cdID)) if not removed then return false end + local sd = ns.GetBarSpellData(barKey) + if sd and sd.hostedBuffCdIDs then sd.hostedBuffCdIDs[cdID] = nil end if ns.RebuildSpellRouteMap then ns.RebuildSpellRouteMap() end if ns.QueueReanchor then ns.QueueReanchor() end return true @@ -1857,6 +1995,11 @@ function ns.RemoveTrackedSpell(barKey, idx) local removedID = list[idx] table.remove(list, idx) ns._spellOrderDirty = true + local removedCdClaim = removedID and ns.CdClaimMarkerToCdID + and ns.CdClaimMarkerToCdID(removedID) + local removedHostedCd = removedCdClaim and ( + (sd.hostedBuffCdIDs and sd.hostedBuffCdIDs[removedCdClaim]) + or (ns.IsBuffViewerCdID and ns.IsBuffViewerCdID(removedCdClaim))) -- Hosted-buff removal? Entry is a MARKER, or a legacy plain entry -- representing the buff (flag set, no marker in list). A plain entry @@ -1876,6 +2019,13 @@ function ns.RemoveTrackedSpell(barKey, idx) -- Host flip changes resolution routing: retire memoized results. ns._cdmResGen = (ns._cdmResGen or 0) + 1 end + elseif removedHostedCd then + if sd.hostedBuffCdIDs then sd.hostedBuffCdIDs[removedCdClaim] = nil end + elseif removedCdClaim and not IsBarBuffFamily(barKey) + and barKey ~= (ns.GHOST_CD_BAR_KEY or "__ghost_cd") then + -- A removed collided CD/Utility slot must ghost by cooldownID. Ghosting + -- its shared positive spell family would hide its sibling too. + ns.AddTrackedSpell(ns.GHOST_CD_BAR_KEY or "__ghost_cd", removedID) else -- Auxiliary metadata cleanup, mirroring RemoveSpellFromBar's side -- effects for symmetry with index-based removal. diff --git a/EllesmereUICooldownManager/EllesmereUICooldownManager.lua b/EllesmereUICooldownManager/EllesmereUICooldownManager.lua index 5d38e2b0d..2480eaea3 100644 --- a/EllesmereUICooldownManager/EllesmereUICooldownManager.lua +++ b/EllesmereUICooldownManager/EllesmereUICooldownManager.lua @@ -916,10 +916,10 @@ function ns.ListHasHostedMarker(list, spellID) end ------------------------------------------------------------------------------- --- Cd-claim markers: a collided buff (two Blizzard buff-viewer slots sharing one canonical --- spellID, e.g. Diabolist Demonic Art vs Diabolic Ritual) can't be told apart by spellID, so --- a claimed slot is tracked by its cooldownID instead, using the same marker-in-assignedSpells --- pattern as hosted-buff markers (add/remove/drag/reorder reuse the existing machinery). +-- Cd-claim markers: collided Blizzard viewer slots sharing one canonical/base/ +-- override family can't be told apart by spellID, so a claimed Buff or CD/Utility +-- slot is tracked by cooldownID. The marker-in-assignedSpells pattern lets +-- add/remove/drag/reorder reuse the existing machinery. -- -- Encoding: -(BASE + cooldownID). BASE sits beyond HOSTED_BUFF_MARKER_BASE (+ max plausible -- spellID), so every hosted-buff-marker check (bounded at HOSTED_BUFF_MARKER_BASE) already excludes cd-claim markers. @@ -9096,6 +9096,15 @@ function ns.RepopulateFromBlizzard() -- A spell ID is "user-added" (preserved across repopulate) if it's a negative preset marker, a custom spell ID added via the picker, or a racial belonging to this character. local function IsUserAdded(sd, id) if type(id) ~= "number" or id == 0 then return false end + -- CD/Utility collision claims represent ordinary Blizzard viewer slots + -- and repopulate releases them. Hosted collided Buff claims remain a + -- deliberate user-added diversion and must survive. + local claimCd = ns.CdClaimMarkerToCdID and ns.CdClaimMarkerToCdID(id) + if claimCd then + return (sd.hostedBuffCdIDs and sd.hostedBuffCdIDs[claimCd]) + or not ns.IsBuffViewerCdID + or ns.IsBuffViewerCdID(claimCd) ~= false + end if id < 0 then return true end if sd.customSpellIDs and sd.customSpellIDs[id] then return true end if _myRacialsSet and _myRacialsSet[id] then return true end diff --git a/EllesmereUIOptions/EUI_CooldownManager_Options.lua b/EllesmereUIOptions/EUI_CooldownManager_Options.lua index e1a7fbc8d..e2a395304 100644 --- a/EllesmereUIOptions/EUI_CooldownManager_Options.lua +++ b/EllesmereUIOptions/EUI_CooldownManager_Options.lua @@ -13424,8 +13424,10 @@ initFrame:SetScript("OnEvent", function(self) -- Check if this spell is already on THIS bar (only gray-out we -- still do for CD/util/buff custom bars). Spells on OTHER bars -- are always claimable -- AddTrackedSpell auto-moves them. - local onThisBar = not isDisabled and excludeSet - and (excludeSet[sp.cdID] or excludeSet[sp.spellID]) + local onThisBar = not isDisabled and ( + (sp.isCdCollision and sp.onEUIBar) + or (not sp.isCdCollision and excludeSet + and (excludeSet[sp.cdID] or excludeSet[sp.spellID]))) -- Apply the grayed "already on this bar" appearance and swap the row -- to its non-interactive state. Used both for spells already present @@ -13482,8 +13484,11 @@ initFrame:SetScript("OnEvent", function(self) ShowWrongBarTypePopup(sp.name, sp.cdmCatGroup == "buff") return end - -- Always pass spellID (assignedSpells stores spellIDs) - if onSelect then onSelect(sp.spellID, sp.isExtra) end + -- Collided CD/Utility slots carry cooldownID identity; + -- ordinary entries continue to store their spellID. + if onSelect then + onSelect(sp.spellID, sp.isExtra, sp.cdID, sp.isCdCollision) + end -- Keep the picker open so multiple spells can be added in a -- row; gray this row in place to reflect that it was added. if notLearned then EllesmereUI.HideWidgetTooltip() end @@ -14916,8 +14921,12 @@ initFrame:SetScript("OnEvent", function(self) end end end - ShowSpellPicker(self, bd.key, nil, excl, function(newSpellID, isExtra) - ns.AddTrackedSpell(bd.key, newSpellID, isExtra) + ShowSpellPicker(self, bd.key, nil, excl, function(newSpellID, isExtra, newCdID, isCdCollision) + if isCdCollision and newCdID and ns.AddTrackedCooldownByCdID then + ns.AddTrackedCooldownByCdID(bd.key, newCdID) + else + ns.AddTrackedSpell(bd.key, newSpellID, isExtra) + end FinalizeAdd() end) end diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 000000000..6435d14c0 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,12 @@ +# CDM collision regressions + +Run `python tests/run_cdm_collision_tests.py` with Python 3 and Node/npm available. +The runner uses Fengari through `npx`, loads the production spell picker, and +extracts the production routing and repopulation functions into a temporary Lua +module. No WoW installation or SavedVariables are read or changed. + +Coverage includes independent cooldown slot placement, migration, ordinary talent +overrides, absent siblings, ghosting and restoration, hosted buffs, and repopulation. +The mocks represent distinct Blizzard cooldownIDs in one base/override family. +They do not establish the live Beacon cooldownIDs, secret-value behavior, visual +ordering, or persistence across actual WoW reloads and talent swaps. diff --git a/tests/cdm_collision_claims.lua b/tests/cdm_collision_claims.lua new file mode 100644 index 000000000..6d35455aa --- /dev/null +++ b/tests/cdm_collision_claims.lua @@ -0,0 +1,207 @@ +local ns = {} + +EUI_CLIENT_BLOCKED = false +Enum = { CooldownViewerCategory = {} } +issecretvalue = function() return false end + +local bases = { [200025] = 53563, [9002] = 9001 } +C_Spell = { + GetBaseSpell = function(id) return bases[id] or id end, + GetOverrideSpell = function(id) + if id == 53563 then return 200025 end + if id == 9001 then return 9002 end + return id + end, + GetSpellName = function(id) return "Spell " .. id end, + GetSpellTexture = function(id) return id end, +} +C_SpellBook = { + FindSpellOverrideByID = function(id) + if id == 53563 then return 200025 end + if id == 9001 then return 9002 end + return id + end, +} + +local function Frame(cdID, sid, layoutIndex) + return { + cooldownID = cdID, + cooldownInfo = { spellID = sid }, + layoutIndex = layoutIndex, + IsShown = function() return true end, + GetSpellID = function() return sid end, + } +end + +local function Pool(frames) + return { + EnumerateActive = function() + local i = 0 + return function() + i = i + 1 + return frames[i] + end + end, + } +end + +EssentialCooldownViewer = { itemFramePool = Pool({ + Frame(65, 53563, 1), + Frame(90, 9001, 2), +}) } +UtilityCooldownViewer = { itemFramePool = Pool({ + Frame(66, 200025, 1), +}) } +BuffIconCooldownViewer = { itemFramePool = Pool({ + Frame(201, 7001, 1), + Frame(202, 7001, 2), +}) } + +local bars = { + { key = "cooldowns", barType = "cooldowns", enabled = true }, + { key = "utility", barType = "utility", enabled = true }, + { key = "buffs", barType = "buffs", enabled = true }, + { key = "buff-extra", barType = "buffs", enabled = true }, + { key = "__ghost_cd", isGhostBar = true, enabled = true }, +} +local stores = { + cooldowns = { assignedSpells = { 53563 } }, + utility = { assignedSpells = { 156910 } }, + buffs = { assignedSpells = {} }, + ["buff-extra"] = { assignedSpells = {} }, + ["__ghost_cd"] = { assignedSpells = {} }, +} + +ns.ECME = { db = { profile = { cdmBars = { bars = bars } } } } +ns.barDataByKey = {} +for _, bar in ipairs(bars) do ns.barDataByKey[bar.key] = bar end +ns.cdmBarFrames = {} +ns.cdmBarIcons = {} +ns.ComputeTopRowStride = function(_, count) return math.max(count, 1), 1, count end +ns.GetBarSpellData = function(key) return stores[key] end +ns.CD_CLAIM_MARKER_BASE = 3000000000 +ns.CdClaimMarker = function(cdID) return -(ns.CD_CLAIM_MARKER_BASE + cdID) end +ns.CdClaimMarkerToCdID = function(id) + if type(id) == "number" and id <= -ns.CD_CLAIM_MARKER_BASE then + return -id - ns.CD_CLAIM_MARKER_BASE + end +end +ns.CollectCdClaimSet = function(sd) + local out + for _, id in ipairs(sd and sd.assignedSpells or {}) do + local cdID = ns.CdClaimMarkerToCdID(id) + if cdID then out = out or {}; out[cdID] = true end + end + return out +end +ns.HostedBuffMarkerToSpell = function() return nil end +ns.ListHasHostedMarker = function() return false end +ns.SlotIDFromKey = function() return nil end + +local chunk = assert(loadfile("EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua")) +chunk("EllesmereUICooldownManager", ns) + +local function equal(actual, expected, label) + if actual ~= expected then + error(('%s: expected %s, got %s'):format(label, tostring(expected), tostring(actual)), 2) + end +end + +local entries = ns.EnumerateCDMViewerSpells(false) +equal(#entries, 3, "collided slots remain enumerable") +equal(entries[1].cdID, 65, "first collided slot") +equal(entries[1].isCdCollision, true, "first collision flag") +equal(entries[3].cdID, 66, "second collided slot") +equal(entries[3].isCdCollision, true, "second collision flag") +equal(entries[2].isCdCollision, false, "ordinary slot remains spell-keyed") + +equal(ns.MigrateCollidedCDAssignments(), 1, "legacy migration count") +equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(65), "exact legacy slot claim") +equal(stores.utility.assignedSpells[1], 156910, "unrelated utility assignment") + +equal(ns.AddTrackedCooldownByCdID("utility", 66), true, "add collided sibling") +equal(stores.utility.assignedSpells[2], ns.CdClaimMarker(66), "sibling stored independently") +equal(ns.AddTrackedCooldownByCdID("cooldowns", 66), true, "move collided sibling") +equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(65), "first slot survives sibling move") +equal(stores.cooldowns.assignedSpells[2], ns.CdClaimMarker(66), "moved sibling arrives") +equal(#stores.utility.assignedSpells, 1, "sibling removed from old bar only") + +equal(ns.RemoveTrackedSpell("cooldowns", 1), true, "remove first collided slot") +equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(66), "second slot survives removal") +equal(stores.__ghost_cd.assignedSpells[1], ns.CdClaimMarker(65), "removed slot ghosts independently") + +stores.cooldowns.assignedSpells = { 9001 } +stores.utility.assignedSpells = {} +equal(ns.AddTrackedSpell("utility", 9002), true, "ordinary override move") +equal(#stores.cooldowns.assignedSpells, 0, "ordinary base leaves old bar") +equal(stores.utility.assignedSpells[1], 9002, "ordinary override has one home") + +equal(ns.AddTrackedBuffByCdID("buff-extra", 201), true, "first buff collision claim") +equal(ns.AddTrackedBuffByCdID("buffs", 202), false, "default buff bar rejects slot claims") +equal(stores["buff-extra"].assignedSpells[1], ns.CdClaimMarker(201), "buff collision behavior retained") + +equal(ns.AddHostedBuffByCdID("cooldowns", 202), true, "host collided buff") +equal(ns.AddHostedBuffByCdID("utility", 202), true, "move hosted collided buff") +equal(stores.cooldowns.hostedBuffCdIDs[202], nil, "move clears old hosted metadata") +equal(stores.utility.hostedBuffCdIDs[202], true, "move retains new hosted metadata") + +stores.cooldowns.assignedSpells = { 53563 } +stores.cooldowns.spellDurations = { [53563] = 15 } +equal(ns.MigrateCollidedCDAssignments(), 0, "duration-backed custom is not migrated") +equal(stores.cooldowns.assignedSpells[1], 53563, "custom entry remains spell-keyed") +stores.cooldowns.spellDurations = nil + +-- Saved claims remain slot-keyed while a sibling is absent after a talent swap. +stores.cooldowns.assignedSpells = { ns.CdClaimMarker(65) } +stores.utility.assignedSpells = { ns.CdClaimMarker(66) } +stores.__ghost_cd.assignedSpells = {} +UtilityCooldownViewer.itemFramePool = Pool({}) +entries = ns.GetCDMSpellsForBar("cooldowns") +equal(entries[1].isCdCollision, true, "claim survives absent sibling") +equal(entries[1].onEUIBar, true, "claimed row stays selected") +equal(ns.AddTrackedCooldownByCdID("utility", 65), true, "move surviving slot") +equal(#stores.cooldowns.assignedSpells, 0, "move removes old marker") +UtilityCooldownViewer.itemFramePool = Pool({ Frame(66, 200025, 1) }) +equal(ns.MigrateCollidedCDAssignments(), 0, "migration is idempotent") + +-- Execute the production routing section extracted by the Python runner. +if arg and arg[1] then + wipe = function(t) for k in pairs(t) do t[k] = nil end end + local routeChunk = assert(loadfile(arg[1])) + routeChunk("EllesmereUICooldownManager", ns) + stores.cooldowns.assignedSpells = { ns.CdClaimMarker(65) } + stores.utility.assignedSpells = { ns.CdClaimMarker(66), 53563 } + ns.RebuildSpellRouteMap() + -- Claims must resolve before any spell-info query, including secret states. + C_CooldownViewer = { GetCooldownViewerCooldownInfo = function() + error("claimed slot must not read spell info") + end } + equal(ns.ResolveCDIDToBar(65, "utility"), "cooldowns", "exact slot outranks family") + equal(ns.ResolveCDIDToBar(66, "cooldowns"), "utility", "sibling route is independent") + equal(ns.RemoveTrackedSpell("cooldowns", 1), true, "remove routed slot") + equal(ns.ResolveCDIDToBar(65, "cooldowns"), "__ghost_cd", "ghost routing invalidates cache") + equal(ns.ResolveCDIDToBar(66, "cooldowns"), "utility", "ghost leaves sibling visible") + equal(ns.AddTrackedCooldownByCdID("cooldowns", 65), true, "restore ghosted slot") + equal(ns.ResolveCDIDToBar(65, "utility"), "cooldowns", "restore invalidates ghost cache") + equal(#stores.__ghost_cd.assignedSpells, 0, "restored slot leaves ghost") + + -- Repopulate must not drop legacy buff claims while they are untalented. + BuffIconCooldownViewer.itemFramePool = Pool({}) + C_CooldownViewer.GetCooldownViewerCooldownInfo = function(cdID) + if cdID == 201 then return { category = 2, spellID = 7001 } end + end + stores.cooldowns.assignedSpells = { ns.CdClaimMarker(65), ns.CdClaimMarker(201), -13 } + stores.utility.assignedSpells = { ns.CdClaimMarker(66) } + stores["buff-extra"].assignedSpells = { ns.CdClaimMarker(203) } + ns.GetActiveSpecKey = function() return "65" end + ns.FullCDMRebuild = function() ns.RebuildSpellRouteMap() end + ns.ReseedAssignedSpellsFromLiveIcons = function() end + C_Timer = { After = function(_, fn) fn() end } + ns.RepopulateFromBlizzard() + equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(201), "inactive legacy buff survives repopulate") + equal(stores.cooldowns.assignedSpells[2], -13, "equipment preset survives repopulate") + equal(#stores.utility.assignedSpells, 0, "CD claim released on repopulate") + equal(stores["buff-extra"].assignedSpells[1], ns.CdClaimMarker(203), "unavailable claim preserved conservatively") +end + +print("cdm collision claim harness: PASS") diff --git a/tests/run_cdm_collision_tests.py b/tests/run_cdm_collision_tests.py new file mode 100644 index 000000000..8ff775df5 --- /dev/null +++ b/tests/run_cdm_collision_tests.py @@ -0,0 +1,47 @@ +"""Run mocked CDM regressions against the production picker and routing code. + +Requires npx (Fengari); run from any directory with Python 3. +The routing section is extracted verbatim to avoid booting the WoW frame UI. +""" + +from pathlib import Path +import shutil +import subprocess +import tempfile + + +def main(): + root = Path(__file__).resolve().parents[1] + hooks = (root / "EllesmereUICooldownManager/EllesmereUICdmHooks.lua").read_text(encoding="utf-8") + start = hooks.index("local _cdidRouteMap = {}") + end_marker = "ns._cdidRouteMap = _cdidRouteMap" + end = hooks.index(end_marker, start) + len(end_marker) + main_source = (root / "EllesmereUICooldownManager/EllesmereUICooldownManager.lua").read_text(encoding="utf-8") + repop_start = main_source.index("function ns.RepopulateFromBlizzard()") + repop_end = main_source.index("\nend\n", repop_start) + len("\nend\n") + npx = shutil.which("npx.cmd") or shutil.which("npx") + if not npx: + raise SystemExit("npx is required for the Fengari Lua harness") + with tempfile.TemporaryDirectory(prefix="eui-cdm-test-") as folder: + route = Path(folder) / "routing.lua" + route.write_text( + "local _, ns = ...\nlocal ECME = ns.ECME\n" + "local GHOST_CD_BAR_KEY = '__ghost_cd'\n" + "local MAIN_BAR_KEYS = { cooldowns = true, utility = true, buffs = true }\n" + "local SaveCurrentSpecProfile = function() end\n" + + hooks[start:end] + "\n" + main_source[repop_start:repop_end], encoding="utf-8") + result = subprocess.run( + [npx, "--yes", "--package=fengari-node-cli", "fengari", + "tests/cdm_collision_claims.lua", str(route)], + cwd=root, check=True, capture_output=True, text=True, + ) + print(result.stdout, end="") + if result.stderr: + print(result.stderr, end="") + # Some Fengari errors do not set a nonzero process exit code. + if "cdm collision claim harness: PASS" not in result.stdout: + raise SystemExit("CDM harness did not reach its final assertion") + + +if __name__ == "__main__": + main() From d953545f1d0cf3da01a979437ebbb3b3e6764054 Mon Sep 17 00:00:00 2001 From: Andrea Bergonzo Date: Tue, 8 Sep 2026 03:31:42 +0100 Subject: [PATCH 2/4] fix(cdm): constrain collision migration to direct overrides Address Fable review findings: require distinct source spell identities and a direct override link, migrate before picker adds, and never ghost unclassified legacy buff slots. Cover duplicate views, false base ties, two displayed Virtue icons and index-based ordering. --- .../EllesmereUICdmSpellPicker.lua | 19 +++++++++- tests/cdm_collision_claims.lua | 37 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua index 2e6c128a4..963139681 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua @@ -383,8 +383,11 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) if frame:IsShown() or frame.cooldownInfo then local sid = GetCanonicalSpellIDForFrame(frame) if _IsUsableSID(sid) then + local info = frame.cooldownInfo entries[#entries + 1] = { sid = sid, + identitySID = info and _IsUsableSID(info.spellID) and info.spellID or sid, + overrideSID = info and _IsUsableSID(info.overrideSpellID) and info.overrideSpellID or nil, cdID = frame.cooldownID, viewerName = vName, viewerOrder = viewerOrder, @@ -422,8 +425,15 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) if type(a.cdID) == "number" then for j = i + 1, #entries do local b = entries[j] + -- A base-only tie can join unrelated abilities. Require + -- distinct source spells with a direct override link. + -- Source IDs also distinguish slots displaying the same + -- override, while duplicate views of one spell stay merged. if type(b.cdID) == "number" and a.cdID ~= b.cdID - and IsVariantOf(a.sid, b.sid) then + and a.identitySID ~= b.identitySID + and (a.overrideSID == b.identitySID or b.overrideSID == a.identitySID + or _GetOverride(a.identitySID) == b.identitySID + or _GetOverride(b.identitySID) == a.identitySID) then collided[a.cdID] = true collided[b.cdID] = true end @@ -1903,6 +1913,12 @@ end --- spells continue through AddTrackedSpell and retain talent-stable identity. function ns.AddTrackedCooldownByCdID(barKey, cdID) if type(cdID) ~= "number" or cdID <= 0 or IsBarBuffFamily(barKey) then return false end + -- Settle legacy assignments before a picker click can create a second + -- representation of the same slot ahead of the first reanchor. + if ns.MigrateCollidedCDAssignments() > 0 then + if ns.RebuildSpellRouteMap then ns.RebuildSpellRouteMap() end + if ns.QueueReanchor then ns.QueueReanchor() end + end return ns.AddTrackedSpell(barKey, ns.CdClaimMarker(cdID)) end @@ -2022,6 +2038,7 @@ function ns.RemoveTrackedSpell(barKey, idx) elseif removedHostedCd then if sd.hostedBuffCdIDs then sd.hostedBuffCdIDs[removedCdClaim] = nil end elseif removedCdClaim and not IsBarBuffFamily(barKey) + and ns.IsBuffViewerCdID(removedCdClaim) == false and barKey ~= (ns.GHOST_CD_BAR_KEY or "__ghost_cd") then -- A removed collided CD/Utility slot must ghost by cooldownID. Ghosting -- its shared positive spell family would hide its sibling too. diff --git a/tests/cdm_collision_claims.lua b/tests/cdm_collision_claims.lua index 6d35455aa..34125534d 100644 --- a/tests/cdm_collision_claims.lua +++ b/tests/cdm_collision_claims.lua @@ -204,4 +204,41 @@ if arg and arg[1] then equal(stores["buff-extra"].assignedSpells[1], ns.CdClaimMarker(203), "unavailable claim preserved conservatively") end +-- Duplicate views and base-only API ties must not migrate unrelated assignments. +for _, sd in pairs(stores) do sd.assignedSpells = {} end +EssentialCooldownViewer.itemFramePool = Pool({ Frame(301, 8001, 1) }) +UtilityCooldownViewer.itemFramePool = Pool({ Frame(302, 8001, 1) }) +stores.cooldowns.assignedSpells = { 8001 } +equal(#ns.EnumerateCDMViewerSpells(false), 1, "same spell in two viewers remains deduplicated") +equal(ns.MigrateCollidedCDAssignments(), 0, "duplicate views do not migrate") +bases[8002] = 8001 +UtilityCooldownViewer.itemFramePool = Pool({ Frame(302, 8002, 1) }) +equal(ns.EnumerateCDMViewerSpells(false)[1].isCdCollision, false, "base-only tie does not collide") +equal(ns.MigrateCollidedCDAssignments(), 0, "base-only tie does not migrate") + +-- Distinct source spells remain separable even when both show Virtue. +local light = Frame(65, 53563, 1) +light.cooldownInfo.overrideSpellID = 200025 +light.GetSpellID = function() return 200025 end +EssentialCooldownViewer.itemFramePool = Pool({ light }) +UtilityCooldownViewer.itemFramePool = Pool({ Frame(66, 200025, 1) }) +equal(#ns.EnumerateCDMViewerSpells(false), 2, "same displayed override retains distinct source slots") +stores.cooldowns.assignedSpells = { 53563 } +ns.AddTrackedCooldownByCdID("cooldowns", 65) +equal(#stores.cooldowns.assignedSpells, 1, "early picker click leaves one assignment") +equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(65), "early picker settles legacy identity") +equal(ns.MigrateCollidedCDAssignments(), 0, "early click cannot capture sibling later") +stores.cooldowns.assignedSpells = { ns.CdClaimMarker(65), ns.CdClaimMarker(66), 156910 } +ns.MoveTrackedSpell("cooldowns", 1, 3) +equal(stores.cooldowns.assignedSpells[3], ns.CdClaimMarker(65), "index move retains slot marker") +ns.SwapTrackedSpells("cooldowns", 1, 3) +equal(stores.cooldowns.assignedSpells[1], ns.CdClaimMarker(65), "index swap retains first slot") +equal(stores.cooldowns.assignedSpells[3], ns.CdClaimMarker(66), "index swap retains sibling") + +stores.cooldowns.assignedSpells = { ns.CdClaimMarker(999) } +stores.__ghost_cd.assignedSpells = {} +C_CooldownViewer = { GetCooldownViewerCooldownInfo = function() return nil end } +equal(ns.RemoveTrackedSpell("cooldowns", 1), true, "remove unavailable legacy buff") +equal(#stores.__ghost_cd.assignedSpells, 0, "unavailable legacy buff is never ghosted") + print("cdm collision claim harness: PASS") From 06c6479fb99dacfd17b8b98f9d57b412001cdb31 Mon Sep 17 00:00:00 2001 From: Andrea Bergonzo Date: Tue, 8 Sep 2026 04:01:01 +0100 Subject: [PATCH 3/4] fix(cdm): suppress redundant overridden cooldown claims Keep each saved slot assignment while temporarily hiding the overridden base when its separately claimed native replacement is present. Refresh suppression with live pool membership and preserve clean metadata through combat. Validate using supplied Beacon metadata and mocked production routing. Fable 5.1 reviewed the implementation and follow-up; final low-priority cache retry and coverage fixes are locally verified. --- .../EllesmereUICdmHooks.lua | 25 +++++ .../EllesmereUICdmSpellPicker.lua | 64 +++++++++++ .../EllesmereUICooldownManager.lua | 4 + tests/README.md | 7 ++ tests/cdm_collision_claims.lua | 100 +++++++++++++++++- 5 files changed, 199 insertions(+), 1 deletion(-) diff --git a/EllesmereUICooldownManager/EllesmereUICdmHooks.lua b/EllesmereUICooldownManager/EllesmereUICdmHooks.lua index 3738fccba..339b48c9c 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmHooks.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmHooks.lua @@ -1140,8 +1140,22 @@ local _routeMapBuilt = false --- overwrite via preserveExisting=false. Family split: each bar writes --- _divertedSpellsBuff or _divertedSpellsCD so buff/CD bars claiming the same --- spellID (e.g. Divine Shield 642) never clobber each other. +function ns.RefreshRedundantOverrideClaims() + local updated = ns.GetRedundantOverrideClaims + and ns.GetRedundantOverrideClaims(_divertedCdIDs) or {} + local previous = ns._redundantOverrideClaims or {} + local changed = false + for cdID in pairs(updated) do if not previous[cdID] then changed = true; break end end + if not changed then + for cdID in pairs(previous) do if not updated[cdID] then changed = true; break end end + end + ns._redundantOverrideClaims = updated + if changed then wipe(_cdidRouteMap) end +end + function ns.RebuildSpellRouteMap() wipe(_cdidRouteMap) + ns._redundantOverrideClaims = nil wipe(_divertedSpellsBuff) wipe(_divertedSpellsCD) wipe(_divertedDirectBuff) @@ -1331,6 +1345,7 @@ function ns.RebuildSpellRouteMap() end end + ns.RefreshRedundantOverrideClaims() _routeMapBuilt = true end @@ -1354,6 +1369,14 @@ local function ResolveCDIDToBar(cdID, viewerDefaultBar) local cached = _cdidRouteMap[cdID] if cached then return cached end + -- Transiently suppress only the overridden base. Its saved bar and order + -- remain intact, and rebuilding after a talent swap restores its route. + if ns._redundantOverrideClaims and ns._redundantOverrideClaims[cdID] then + local hiddenBar = ns.GHOST_CD_BAR_KEY or "__ghost_cd" + _cdidRouteMap[cdID] = hiddenBar + return hiddenBar + end + -- cooldownID-level claim first (collided Buff or CD/Utility slot). Needs no -- cooldownInfo read, so it also works while every sid field is secret. local cdRoute = _divertedCdIDs[cdID] @@ -6534,6 +6557,8 @@ local function CollectAndReanchor() -- diversions) and NOT _cdidRouteMap (lazy cache, empty post-build). if not _routeMapBuilt and ns.RebuildSpellRouteMap then ns.RebuildSpellRouteMap() + else + ns.RefreshRedundantOverrideClaims() end wipe(_scratch_usedFrames) diff --git a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua index 963139681..aff00e81b 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua @@ -456,6 +456,70 @@ local function EnumerateCDMViewerSpells(includeBuffViewer) end ns.EnumerateCDMViewerSpells = EnumerateCDMViewerSpells +-- A separately claimed replacement makes its overridden base slot redundant. +-- Keep the saved claims intact so the base returns to its own bar on talent swap. +-- Only clean CD/Utility metadata participates; buff slots remain independent. +function ns.GetRedundantOverrideClaims(claims) + local inCombat = InCombatLockdown and InCombatLockdown() + local infoByID = ns._overrideClaimInfo or {} + if inCombat and not ns._overrideClaimInfo then ns._overrideClaimRefreshPending = true end + if not inCombat then + ns._overrideClaimRefreshPending = nil + infoByID = {} + local gci = C_CooldownViewer and C_CooldownViewer.GetCooldownViewerCooldownInfo + for cdID in pairs(claims) do + local info = gci and gci(cdID) + if not info then ns._overrideClaimRefreshPending = true end + if info and _IsUsableSID(info.spellID) and _IsUsableSID(info.overrideSpellID) + and type(info.category) == "number" + and not (issecretvalue and issecretvalue(info.category)) + and ns.CDM_ICON_CD_CATS[info.category] + and not (issecretvalue and issecretvalue(info.isInvisible)) + and info.isInvisible ~= true + and not (issecretvalue and issecretvalue(info.isKnown)) + and info.isKnown == true then + infoByID[cdID] = { spellID = info.spellID, overrideSpellID = info.overrideSpellID } + end + end + ns._overrideClaimInfo = infoByID + end + + -- Match the intake pool membership on every reanchor. A catalog entry + -- alone does not prove that a replacement frame exists to show instead. + local present = {} + for _, viewerName in ipairs({ "EssentialCooldownViewer", "UtilityCooldownViewer" }) do + local viewer = _G[viewerName] + local pool = viewer and viewer.itemFramePool + if pool and pool.EnumerateActive then + for frame in pool:EnumerateActive() do + if (frame:IsShown() or frame.cooldownInfo) and _IsUsableSID(frame.cooldownID) then + present[frame.cooldownID] = true + end + end + end + end + local replacements = {} + for cdID, barKey in pairs(claims) do + local bd = barDataByKey[barKey] + local info = infoByID[cdID] + if bd and bd.enabled and not bd.isGhostBar and info and present[cdID] + and info.spellID == info.overrideSpellID then + replacements[info.spellID] = cdID + end + end + local hidden = {} + for cdID, barKey in pairs(claims) do + local bd = barDataByKey[barKey] + local info = infoByID[cdID] + if bd and bd.enabled and not bd.isGhostBar and info + and info.spellID ~= info.overrideSpellID + and replacements[info.overrideSpellID] then + hidden[cdID] = true + end + end + return hidden +end + function ns.IsBuffViewerCdID(cdID) if type(cdID) ~= "number" then return false end for _, e in ipairs(EnumerateCDMViewerSpells(true)) do diff --git a/EllesmereUICooldownManager/EllesmereUICooldownManager.lua b/EllesmereUICooldownManager/EllesmereUICooldownManager.lua index 2480eaea3..a301b8f37 100644 --- a/EllesmereUICooldownManager/EllesmereUICooldownManager.lua +++ b/EllesmereUICooldownManager/EllesmereUICooldownManager.lua @@ -10574,6 +10574,10 @@ eventFrame:SetScript("OnEvent", function(_, event, unit, updateInfo, arg3) -- Buffer combat exit: brief out-of-combat blips (mob dies, re-aggro) shouldn't flash visibility changes. C_Timer.After(0.1, function() if not InCombatLockdown() then + if ns._overrideClaimRefreshPending then + ns.RebuildSpellRouteMap() + if ns.QueueReanchor then ns.QueueReanchor() end + end _inCombat = false _CDMApplyVisibility() ns.RefreshItemCountOOCBars() diff --git a/tests/README.md b/tests/README.md index 6435d14c0..50cb51547 100644 --- a/tests/README.md +++ b/tests/README.md @@ -10,3 +10,10 @@ overrides, absent siblings, ghosting and restoration, hosted buffs, and repopula The mocks represent distinct Blizzard cooldownIDs in one base/override family. They do not establish the live Beacon cooldownIDs, secret-value behavior, visual ordering, or persistence across actual WoW reloads and talent swaps. + +Redundant-override cases use supplied live metadata for native Virtue (29265) +and Light overridden by Virtue (90506). Suppression applies only to explicit +cooldownID claims when a known native replacement also exists in a live viewer +pool. Saved assignments stay intact. Plain spell-ID assignments are outside this +rule. Tests cover missing/invisible replacements, combat metadata caching, talent +changes, and route cache invalidation; actual in-game rendering remains manual. diff --git a/tests/cdm_collision_claims.lua b/tests/cdm_collision_claims.lua index 34125534d..531d8fac6 100644 --- a/tests/cdm_collision_claims.lua +++ b/tests/cdm_collision_claims.lua @@ -173,11 +173,14 @@ if arg and arg[1] then stores.utility.assignedSpells = { ns.CdClaimMarker(66), 53563 } ns.RebuildSpellRouteMap() -- Claims must resolve before any spell-info query, including secret states. + local routeInfoReads = 0 C_CooldownViewer = { GetCooldownViewerCooldownInfo = function() - error("claimed slot must not read spell info") + routeInfoReads = routeInfoReads + 1 + return nil end } equal(ns.ResolveCDIDToBar(65, "utility"), "cooldowns", "exact slot outranks family") equal(ns.ResolveCDIDToBar(66, "cooldowns"), "utility", "sibling route is independent") + equal(routeInfoReads, 0, "claimed routes do not read spell info") equal(ns.RemoveTrackedSpell("cooldowns", 1), true, "remove routed slot") equal(ns.ResolveCDIDToBar(65, "cooldowns"), "__ghost_cd", "ghost routing invalidates cache") equal(ns.ResolveCDIDToBar(66, "cooldowns"), "utility", "ghost leaves sibling visible") @@ -241,4 +244,99 @@ C_CooldownViewer = { GetCooldownViewerCooldownInfo = function() return nil end } equal(ns.RemoveTrackedSpell("cooldowns", 1), true, "remove unavailable legacy buff") equal(#stores.__ghost_cd.assignedSpells, 0, "unavailable legacy buff is never ghosted") +if arg and arg[1] then + -- User-provided live Beacon metadata, with the exact saved placement. + for _, sd in pairs(stores) do sd.assignedSpells = {} end + stores.cooldowns.assignedSpells = { ns.CdClaimMarker(29265) } + stores.utility.assignedSpells = { ns.CdClaimMarker(90506), 156910 } + stores.__ghost_cd.assignedSpells = { 53563 } + EssentialCooldownViewer.itemFramePool = Pool({ Frame(29265, 200025, 1) }) + UtilityCooldownViewer.itemFramePool = Pool({ Frame(90506, 53563, 1) }) + local info = { + [29265] = { spellID = 200025, overrideSpellID = 200025, isKnown = true, category = 0, flags = 0 }, + [90506] = { spellID = 53563, overrideSpellID = 200025, isKnown = true, category = 0, flags = 2 }, + } + local combat, reads = false, 0 + InCombatLockdown = function() return combat end + C_CooldownViewer.GetCooldownViewerCooldownInfo = function(cdID) + reads = reads + 1 + assert(not combat, "combat rebuild must use clean cached metadata") + return info[cdID] + end + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(29265, "cooldowns"), "cooldowns", "native Virtue remains above") + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "__ghost_cd", "overridden Light duplicate is hidden") + equal(stores.utility.assignedSpells[1], ns.CdClaimMarker(90506), "hidden Light keeps Utility slot") + equal(stores.utility.assignedSpells[2], 156910, "Faith assignment unchanged") + info[777] = { spellID = 888, overrideSpellID = 888, isKnown = true, category = 0 } + equal(ns.ResolveCDIDToBar(777, "utility"), "utility", "unrelated route is cached") + ns.RefreshRedundantOverrideClaims() + equal(ns._cdidRouteMap[777], "utility", "unchanged suppression preserves unrelated route cache") + local cachedReads = reads + ns.ResolveCDIDToBar(777, "utility") + equal(reads, cachedReads, "unchanged refresh avoids another route metadata query") + EssentialCooldownViewer.itemFramePool = Pool({}) + ns.RefreshRedundantOverrideClaims() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "absent native frame restores Light on reanchor") + EssentialCooldownViewer.itemFramePool = Pool({ Frame(29265, 200025, 1) }) + ns.RefreshRedundantOverrideClaims() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "__ghost_cd", "returning native frame clears cached fallback") + info[29265].isInvisible = true + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "invisible native cannot suppress Light") + info[29265].isInvisible = false + bars[1].enabled = false + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "disabled native bar cannot suppress Light") + bars[1].enabled = true + local secretCategory = 123456789 + issecretvalue = function(value) return value == secretCategory end + info[29265].category = secretCategory + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "secret-tagged metadata is rejected") + issecretvalue = function() return false end + info[29265].category = 0 + ns.RebuildSpellRouteMap() + combat = true + local beforeReads = reads + ns.RebuildSpellRouteMap() + equal(reads, beforeReads, "combat rebuild makes no metadata calls") + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "__ghost_cd", "duplicate stays hidden in combat") + ns._overrideClaimInfo = nil + ns.RebuildSpellRouteMap() + equal(ns._overrideClaimRefreshPending, true, "combat login requests refresh") + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "combat login fails open without metadata") + combat = false + ns.RebuildSpellRouteMap() + equal(ns._overrideClaimRefreshPending, nil, "post-combat rebuild clears request") + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "__ghost_cd", "post-combat rebuild suppresses duplicate") + info[29265].isKnown = false + info[90506].overrideSpellID = 53563 + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "Light returns after removing Virtue talent") + info[29265].isKnown = true + info[90506].overrideSpellID = 200025 + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "__ghost_cd", "retalenting Virtue hides duplicate again") + stores.cooldowns.assignedSpells = {} + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "unclaimed native Virtue leaves sole claimed icon visible") + stores.__ghost_cd.assignedSpells = { ns.CdClaimMarker(29265) } + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "hidden native Virtue does not suppress Light slot") + stores.__ghost_cd.assignedSpells = {} + stores.cooldowns.assignedSpells = { ns.CdClaimMarker(29265) } + info[29265].category = 2 + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "buff entry cannot suppress cooldown") + info[29265].category = 0 + info[29265].isKnown = false + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "unknown replacement cannot suppress cooldown") + info[29265] = nil + ns.RebuildSpellRouteMap() + equal(ns.ResolveCDIDToBar(90506, "cooldowns"), "utility", "unavailable metadata keeps icon visible") + equal(ns._overrideClaimRefreshPending, true, "transient missing metadata requests post-combat retry") +end + print("cdm collision claim harness: PASS") From 9439d2af0619edf2bd5f65cd65d41b6a7a43f119 Mon Sep 17 00:00:00 2001 From: Andrea Bergonzo Date: Tue, 8 Sep 2026 04:14:25 +0100 Subject: [PATCH 4/4] fix(cdm): classify claimed cooldown previews correctly --- .../EllesmereUICdmSpellPicker.lua | 2 -- .../EUI_CooldownManager_Options.lua | 15 ++++++--------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua index aff00e81b..7fdcc828c 100644 --- a/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua +++ b/EllesmereUICooldownManager/EllesmereUICdmSpellPicker.lua @@ -2057,8 +2057,6 @@ function ns.RemoveHostedBuffByCdID(barKey, cdID) -- RemoveSpellFromBar doesn't itself trigger route/reanchor; caller must. local removed = ns.RemoveSpellFromBar(barKey, ns.CdClaimMarker(cdID)) if not removed then return false end - local sd = ns.GetBarSpellData(barKey) - if sd and sd.hostedBuffCdIDs then sd.hostedBuffCdIDs[cdID] = nil end if ns.RebuildSpellRouteMap then ns.RebuildSpellRouteMap() end if ns.QueueReanchor then ns.QueueReanchor() end return true diff --git a/EllesmereUIOptions/EUI_CooldownManager_Options.lua b/EllesmereUIOptions/EUI_CooldownManager_Options.lua index e2a395304..14509f7db 100644 --- a/EllesmereUIOptions/EUI_CooldownManager_Options.lua +++ b/EllesmereUIOptions/EUI_CooldownManager_Options.lua @@ -15277,14 +15277,12 @@ initFrame:SetScript("OnEvent", function(self) local hostedSid = (not cdClaim) and ns.HostedBuffMarkerToSpell and ns.HostedBuffMarkerToSpell(id) if cdClaim then - -- Cd-claimed collided-buff slot on a CD/util bar - -- (Diabolist Demonic Art vs Diabolic Ritual): here - -- `tracked` ALIASES sd.assignedSpells (the buff-bar - -- branch above builds a fresh dedup list), so resolve - -- the marker to a display sid IN THIS RENDER STEP - -- ONLY -- never write back into `id`/`tracked[i]` - -- (that corrupts the saved marker). Same clean-cache - -- + cooldownInfo fallback as the buff-bar preview. + -- Resolve slot claims for display without changing the + -- saved marker. Only buff-family claims use buff settings. + local claimData = ns.GetBarSpellData(bd.key) + slot._previewHostedBuff = (claimData and claimData.hostedBuffCdIDs + and claimData.hostedBuffCdIDs[cdClaim]) + or (ns.IsBuffViewerCdID and ns.IsBuffViewerCdID(cdClaim)) local csid = ns._cdmCleanSidByCDID and ns._cdmCleanSidByCDID[cdClaim] if not (type(csid) == "number" and csid > 0) then local gci = C_CooldownViewer and C_CooldownViewer.GetCooldownViewerCooldownInfo @@ -15309,7 +15307,6 @@ initFrame:SetScript("OnEvent", function(self) end slot._previewSpellID = csid slot._previewCdID = cdClaim - slot._previewHostedBuff = true end elseif hostedSid then -- Hosted-buff marker: previews as its spell, flagged so