From 491cd828ddea20dfee8abe1dc8a82ca4643214fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 23:37:07 +0000 Subject: [PATCH 1/9] Initial plan From d7b882248c047f5e12c8c99da5238c75b5d7393b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 23:41:01 +0000 Subject: [PATCH 2/9] Stabilize ExtraIcons layout anchors Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/cc94c4eb-cf3c-4001-a8f3-16502ae1b350 Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Modules/ExtraIcons.lua | 24 ++++++++---- Tests/Modules/ExtraIcons_spec.lua | 64 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 6ba6e066..03b6bce6 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -42,11 +42,23 @@ local function cachePoint(vs, blizzFrame) vs.originalPoint = { point, relativeTo, relativePoint, x or 0, y or 0 } end +local function setSinglePoint(frame, point, relativeTo, relativePoint, x, y) + x, y = x or 0, y or 0 + if frame:GetNumPoints() == 1 then + local currentPoint, currentRelativeTo, currentRelativePoint, currentX, currentY = frame:GetPoint(1) + if currentPoint == point and currentRelativeTo == relativeTo and currentRelativePoint == relativePoint + and (currentX or 0) == x and (currentY or 0) == y then + return + end + end + frame:ClearAllPoints() + frame:SetPoint(point, relativeTo, relativePoint, x, y) +end + local function applyPoint(vs, blizzFrame, offsetX) local p = vs and vs.originalPoint if not p or not blizzFrame then return end - blizzFrame:ClearAllPoints() - blizzFrame:SetPoint(p[1], p[2], p[3], p[4] + (offsetX or 0), p[5]) + setSinglePoint(blizzFrame, p[1], p[2], p[3], p[4] + (offsetX or 0), p[5]) end local function horizontalBounds(point, width) @@ -532,7 +544,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared for _, itemFrame in ipairs(itemFrames) do if itemFrame.isActive then iconSize = itemFrame:GetWidth() or iconSize - lastActive = itemFrame + if itemFrame:IsShown() then lastActive = itemFrame end end end end) @@ -572,8 +584,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared icon.Icon:SetTexture(data.texture) icon.Icon:SetDesaturated(data.missing == true) - icon:ClearAllPoints() - icon:SetPoint("LEFT", container, "LEFT", xOffset, 0) + setSinglePoint(icon, "LEFT", container, "LEFT", xOffset, 0) icon:Show() updateIconCooldown(icon) @@ -589,8 +600,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared xOffset = xOffset + iconSize + spacing end - container:ClearAllPoints() - container:SetPoint("LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0) + setSinglePoint(container, "LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0) container:Show() if viewerConfig.ownsAnchor then updateMainViewerAnchor(vs, blizzFrame, container) end diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index 2ad34ebf..d9787a7f 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -1060,6 +1060,70 @@ describe("ExtraIcons real source", function() assert.are.equal(87, x) end) + it("anchors to the viewer when active item frames are hidden", function() + local activeFrame = TestHelpers.makeFrame({ shown = false, width = 22, height = 22 }) + activeFrame.isActive = true + UtilityCooldownViewer.childXPadding = 4 + UtilityCooldownViewer.iconScale = 1.0 + UtilityCooldownViewer:SetWidth(22) + UtilityCooldownViewer.GetItemFrames = function() + return { activeFrame } + end + UtilityCooldownViewer:SetPoint("CENTER", UIParent, "CENTER", 100, 0) + + itemCounts[HEALTHSTONE_ID] = 1 + itemIconsByID[HEALTHSTONE_ID] = "healthstone" + + ExtraIcons.InnerFrame = ExtraIcons:CreateFrame() + ExtraIcons.GetModuleConfig = function() + return makeViewersConfig({ { kind = "itemStack", itemStackId = "healthstones" } }) + end + + assert.is_true(ExtraIcons:UpdateLayout("hidden-active-frame")) + + local _, anchorFrame = ExtraIcons._viewers.utility.container:GetPoint(1) + assert.are.equal(UtilityCooldownViewer, anchorFrame) + end) + + it("does not reapply unchanged anchors on repeated layouts", function() + local activeFrame = TestHelpers.makeFrame({ shown = true, width = 22, height = 22 }) + activeFrame.isActive = true + UtilityCooldownViewer.childXPadding = 4 + UtilityCooldownViewer.iconScale = 1.0 + UtilityCooldownViewer:SetWidth(22) + UtilityCooldownViewer.GetItemFrames = function() + return { activeFrame } + end + UtilityCooldownViewer:SetPoint("CENTER", UIParent, "CENTER", 100, 0) + + itemCounts[HEALTHSTONE_ID] = 1 + itemIconsByID[HEALTHSTONE_ID] = "healthstone" + + ExtraIcons.InnerFrame = ExtraIcons:CreateFrame() + ExtraIcons.GetModuleConfig = function() + return makeViewersConfig({ { kind = "itemStack", itemStackId = "healthstones" } }) + end + + assert.is_true(ExtraIcons:UpdateLayout("initial")) + + local vs = ExtraIcons._viewers.utility + local viewerSetPoints = TestHelpers.getCalls(UtilityCooldownViewer, "SetPoint") + local viewerClears = TestHelpers.getCalls(UtilityCooldownViewer, "ClearAllPoints") + local containerSetPoints = TestHelpers.getCalls(vs.container, "SetPoint") + local containerClears = TestHelpers.getCalls(vs.container, "ClearAllPoints") + local iconSetPoints = TestHelpers.getCalls(vs.iconPool[1], "SetPoint") + local iconClears = TestHelpers.getCalls(vs.iconPool[1], "ClearAllPoints") + + assert.is_true(ExtraIcons:UpdateLayout("unchanged")) + + assert.are.equal(viewerSetPoints, TestHelpers.getCalls(UtilityCooldownViewer, "SetPoint")) + assert.are.equal(viewerClears, TestHelpers.getCalls(UtilityCooldownViewer, "ClearAllPoints")) + assert.are.equal(containerSetPoints, TestHelpers.getCalls(vs.container, "SetPoint")) + assert.are.equal(containerClears, TestHelpers.getCalls(vs.container, "ClearAllPoints")) + assert.are.equal(iconSetPoints, TestHelpers.getCalls(vs.iconPool[1], "SetPoint")) + assert.are.equal(iconClears, TestHelpers.getCalls(vs.iconPool[1], "ClearAllPoints")) + end) + it("does not iterate inaccessible GetItemFrames results", function() local inaccessibleFrames = {} UtilityCooldownViewer.childXPadding = 4 From bc87b07d3986980cd929e539d0475a2e48205b3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 00:12:53 +0000 Subject: [PATCH 3/9] Use FrameUtil lazy anchors in ExtraIcons Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/7ac040cd-97f3-4a61-954d-c015be57e7f1 Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Modules/ExtraIcons.lua | 19 +++---------------- Tests/Modules/ExtraIcons_spec.lua | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 03b6bce6..b0cbc944 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -42,23 +42,10 @@ local function cachePoint(vs, blizzFrame) vs.originalPoint = { point, relativeTo, relativePoint, x or 0, y or 0 } end -local function setSinglePoint(frame, point, relativeTo, relativePoint, x, y) - x, y = x or 0, y or 0 - if frame:GetNumPoints() == 1 then - local currentPoint, currentRelativeTo, currentRelativePoint, currentX, currentY = frame:GetPoint(1) - if currentPoint == point and currentRelativeTo == relativeTo and currentRelativePoint == relativePoint - and (currentX or 0) == x and (currentY or 0) == y then - return - end - end - frame:ClearAllPoints() - frame:SetPoint(point, relativeTo, relativePoint, x, y) -end - local function applyPoint(vs, blizzFrame, offsetX) local p = vs and vs.originalPoint if not p or not blizzFrame then return end - setSinglePoint(blizzFrame, p[1], p[2], p[3], p[4] + (offsetX or 0), p[5]) + FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + (offsetX or 0), p[5] } }) end local function horizontalBounds(point, width) @@ -584,7 +571,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared icon.Icon:SetTexture(data.texture) icon.Icon:SetDesaturated(data.missing == true) - setSinglePoint(icon, "LEFT", container, "LEFT", xOffset, 0) + FrameUtil.LazySetAnchors(icon, { { "LEFT", container, "LEFT", xOffset, 0 } }) icon:Show() updateIconCooldown(icon) @@ -600,7 +587,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared xOffset = xOffset + iconSize + spacing end - setSinglePoint(container, "LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0) + FrameUtil.LazySetAnchors(container, { { "LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0 } }) container:Show() if viewerConfig.ownsAnchor then updateMainViewerAnchor(vs, blizzFrame, container) end diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index d9787a7f..59a19463 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -235,6 +235,7 @@ describe("ExtraIcons real source", function() "C_Item", "C_PvP", "canaccesstable", + "issecretvalue", "ipairs", }) end) @@ -299,21 +300,21 @@ describe("ExtraIcons real source", function() UnregisterFrame = function() end, RequestLayout = function() end, }, - FrameUtil = { - ApplyFont = function(fontString, appliedGlobalConfig, moduleConfig) - applyFontCalls[#applyFontCalls + 1] = { - fontString = fontString, - globalConfig = appliedGlobalConfig, - moduleConfig = moduleConfig, - } - end, - }, GetGlobalConfig = function() return globalConfig end, } TestHelpers.LoadChunk("Constants.lua", "Unable to load Constants.lua")(nil, ns) TestHelpers.LoadChunk("Locales/en.lua", "Unable to load Locales/en.lua")(nil, ns) + _G.issecretvalue = function() return false end + TestHelpers.LoadChunk("FrameUtil.lua", "Unable to load FrameUtil.lua")(nil, ns) + ns.FrameUtil.ApplyFont = function(fontString, appliedGlobalConfig, moduleConfig) + applyFontCalls[#applyFontCalls + 1] = { + fontString = fontString, + globalConfig = appliedGlobalConfig, + moduleConfig = moduleConfig, + } + end _G.UIParent = TestHelpers.makeFrame({ name = "UIParent" }) EditModeManagerFrame = TestHelpers.makeHookableFrame(false) UtilityCooldownViewer = TestHelpers.makeHookableFrame(true) From 5b37a2d7668d25e2531aee486f4e837bb91b182d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 02:57:06 +0000 Subject: [PATCH 4/9] Add ELI5 ExtraIcons documentation Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/6b29a3f7-f340-4028-9490-51841f7126af Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Modules/ExtraIcons.lua | 112 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index b0cbc944..3e190d03 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -8,6 +8,10 @@ local FrameUtil = ns.FrameUtil local ExtraIcons = ns.Addon:NewModule("ExtraIcons") ns.Addon.ExtraIcons = ExtraIcons +-- ExtraIcons adds the user's chosen potions, trinkets, racials, and spells +-- directly after Blizzard's cooldown viewer rows. Think of each Blizzard row +-- as a train: this module adds a small extra car to the end without moving the +-- train station anchor that other ECM modules use. local BUILTIN_STACKS = ns.Constants.BUILTIN_STACKS local RACIAL_ABILITIES = ns.Constants.RACIAL_ABILITIES local DEFAULT_SIZE = ns.Constants.DEFAULT_EXTRA_ICON_SIZE @@ -15,6 +19,8 @@ local MAIN_BORDER_SCALE = ns.Constants.EXTRA_ICON_MAIN_BORDER_SCALE local UTILITY_BORDER_SCALE = ns.Constants.EXTRA_ICON_UTILITY_BORDER_SCALE local canAccessTable = _G.canaccesstable +-- Some racial abilities have different spell IDs for different races/factions. +-- This lookup lets a saved spell ID find its whole family of equivalent IDs. local RACIAL_SPELL_ALIASES = {} for _, racial in pairs(RACIAL_ABILITIES) do local spellIds = racial.spellIds or { racial.spellId } @@ -23,7 +29,9 @@ for _, racial in pairs(RACIAL_ABILITIES) do end end --- Ordered viewer keys mapped to their Blizzard frame globals. +-- Ordered viewer keys mapped to their Blizzard frame globals. The order matters +-- because the main and utility rows may need to move together as one visual +-- group when extra icons widen one row. local VIEWERS = { { key = "main", blizzKey = "EssentialCooldownViewer", borderScale = { MAIN_BORDER_SCALE, MAIN_BORDER_SCALE }, ownsAnchor = true }, -- Utility icon frames render square; keep the overlay square so extras do not look short. @@ -37,18 +45,27 @@ for _, v in ipairs(VIEWERS) do BLIZZ_KEY[v.key] = v.blizzKey end -------------------------------------------------------------------------------- local function cachePoint(vs, blizzFrame) + -- Save the Blizzard viewer's first anchor before ECM moves it. Future + -- layouts always start from this remembered "home" position so small + -- refreshes do not keep adding offsets on top of old offsets. if vs.originalPoint or not blizzFrame then return end local point, relativeTo, relativePoint, x, y = blizzFrame:GetPoint() vs.originalPoint = { point, relativeTo, relativePoint, x or 0, y or 0 } end local function applyPoint(vs, blizzFrame, offsetX) + -- Move the Blizzard viewer relative to its saved home position. The shared + -- FrameUtil helper does nothing when the anchor is already correct, which + -- prevents unnecessary clear/re-anchor work and visual flicker. local p = vs and vs.originalPoint if not p or not blizzFrame then return end FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + (offsetX or 0), p[5] } }) end local function horizontalBounds(point, width) + -- Convert an anchor name into "how far this frame reaches left and right + -- from its anchor." A left-anchored frame grows to the right, a right + -- anchored frame grows to the left, and a centered frame grows both ways. if point == "LEFT" or point == "TOPLEFT" or point == "BOTTOMLEFT" then return 0, width elseif point == "RIGHT" or point == "TOPRIGHT" or point == "BOTTOMRIGHT" then @@ -60,6 +77,8 @@ end --- Computes a per-viewer horizontal offset that re-centers both viewers as a --- single stacked group when they share the same original anchor. +--- ELI5: if two rows are stacked on the same nail, and one row becomes wider, +--- nudge each row so the whole stack still looks centered on that nail. local function getSharedOffsets(viewers) local offsets = { main = 0, utility = 0 } local mainState, utilState = viewers.main, viewers.utility @@ -102,6 +121,10 @@ local function getSharedOffsets(viewers) end local function updateMainViewerAnchor(vs, blizzFrame, rightFrame) + -- Chained ECM modules ask this module where the main row ends. This hidden + -- anchor stretches from Blizzard's main viewer to the extra-icon container, + -- so the next module attaches after the combined width instead of only + -- after Blizzard's original icons. local anchor = vs and vs.anchorFrame if not anchor then return end @@ -124,6 +147,8 @@ end -------------------------------------------------------------------------------- local function resolveEquipSlot(slotId) + -- Equipment entries track a gear slot, not a fixed item. Resolve the item + -- currently worn in that slot so swapping trinkets updates the extra icon. local itemId = GetInventoryItemID("player", slotId) if not itemId then return nil end local _, spellId = C_Item.GetItemSpell(itemId) @@ -134,6 +159,9 @@ local function resolveEquipSlot(slotId) end local function resolveItem(ids, showIfMissing) + -- Item entries can list several item IDs in priority order. Use the first + -- owned item with an icon; optionally show a grey missing icon for the first + -- configured item if none are owned. local missingData for _, entry in ipairs(ids) do @@ -151,6 +179,8 @@ local function resolveItem(ids, showIfMissing) end local function resolveKnownSpell(spellId) + -- A spell is usable only if the current character knows it and Blizzard can + -- provide an icon for it. if spellId and C_SpellBook.IsSpellKnown(spellId) then local texture = C_Spell.GetSpellTexture(spellId) if texture then return { spellId = spellId, texture = texture } end @@ -158,6 +188,8 @@ local function resolveKnownSpell(spellId) end local function resolveSpell(ids) + -- Spell entries also use priority order. Racial spells may resolve through + -- aliases because the configured spell may have a race-specific equivalent. for _, entry in ipairs(ids) do local spellId = type(entry) == "table" and entry.spellId or entry local data = resolveKnownSpell(spellId) @@ -176,11 +208,15 @@ local function resolveSpell(ids) end local function isNonPvpInstance() + -- "Non-PvP instance" means dungeons, raids, scenarios, and similar places + -- where potion rules may differ from battlegrounds and arenas. local inInstance, instanceType = IsInInstance() return inInstance and instanceType ~= "pvp" and instanceType ~= "arena" end local function shouldSuppressItemStack(itemStack) + -- Some built-in item groups intentionally disappear in rated PvP or + -- non-PvP instances so the row only shows context-relevant consumables. if not itemStack then return true end if itemStack.hideInRatedPvp and C_PvP.IsRatedMap() then return true end if itemStack.hideInInstances and isNonPvpInstance() then return true end @@ -188,6 +224,8 @@ local function shouldSuppressItemStack(itemStack) end local function resolveItemStack(entry, moduleConfig) + -- Custom item stacks are user-configured groups such as "my preferred + -- health potions." Resolve the stack's item list like a normal item entry. local itemStacks = moduleConfig and moduleConfig.itemStacks local itemStack = itemStacks and itemStacks.byId and itemStacks.byId[entry.itemStackId] return not shouldSuppressItemStack(itemStack) @@ -197,6 +235,9 @@ local function resolveItemStack(entry, moduleConfig) end local function resolveEntry(entry, moduleConfig) + -- Turn one saved config entry into one drawable icon, or nil if it should + -- not be shown right now. Built-in stack keys expand into their real entry + -- data before the normal kind-specific resolver runs. local kind, slotId, ids if entry.stackKey then local stack = BUILTIN_STACKS[entry.stackKey] @@ -213,6 +254,8 @@ end local _resolved = {} local function resolveEntries(entries, moduleConfig) + -- Reuse one temporary result table on every layout pass to avoid allocating + -- a new table during frequent UI refreshes. wipe(_resolved) for _, entry in ipairs(entries) do local data = not entry.disabled and resolveEntry(entry, moduleConfig) or nil @@ -226,6 +269,9 @@ end -------------------------------------------------------------------------------- local function createIcon(parent, size, borderScale) + -- Build one square extra icon using Blizzard-like parts: artwork, mask, + -- cooldown swipe, border, count text, and optional shadow. The caller + -- reuses these icons from a pool instead of creating new ones every layout. local icon = CreateFrame("Button", nil, parent) icon:SetSize(size, size) @@ -267,6 +313,9 @@ local function createIcon(parent, size, borderScale) end local function updateIconCooldown(icon) + -- Pick the right Blizzard cooldown API for the thing shown on this icon: + -- spells use spell cooldowns, equipment slots use inventory cooldowns, and + -- bag items use item cooldowns. if icon.spellId then local cdInfo = C_Spell.GetSpellCooldown(icon.spellId) if cdInfo and cdInfo.isOnGCD then @@ -304,6 +353,8 @@ local function updateIconCooldown(icon) end local function setIconCountText(icon, text) + -- Count text is optional. Nil means "hide the number"; any value means + -- "show this number on top of the icon." if text ~= nil then icon.Count:SetText(tostring(text)) icon.Count:Show() @@ -314,6 +365,8 @@ local function setIconCountText(icon, text) end local function updateIconCountText(icon, globalConfig, config) + -- Item icons can show stack counts and spell icons can show charges. Only + -- show useful numbers: one item or a non-charge spell should stay clean. if not icon.Count then return end FrameUtil.ApplyFont(icon.Count, globalConfig, config) @@ -337,6 +390,8 @@ local function updateIconCountText(icon, globalConfig, config) end --- Caches and returns the cooldown number font from a sibling Blizzard icon. +--- ELI5: copy Blizzard's number style so ECM's extra icons look like they +--- belong in the same row. local function getSiblingFont(viewer) local cached = viewer.__ecmCDFont if cached then return cached[1], cached[2], cached[3] end @@ -357,6 +412,8 @@ local function getSiblingFont(viewer) end local function getFrameValue(frame, methodName) + -- Diagnostics should never create a new error while trying to describe the + -- original problem, so every optional frame read is protected. if not frame or type(frame[methodName]) ~= "function" then return nil end @@ -370,6 +427,8 @@ local function getFrameValue(frame, methodName) end local function getItemFramesCount(itemFrames) + -- Count Blizzard's item frame array only when Lua is allowed to read it. + -- Some protected/tainted tables must not be iterated directly. if type(itemFrames) ~= "table" or not canAccessTable(itemFrames) then return nil end @@ -384,6 +443,8 @@ local function getItemFramesCount(itemFrames) end local function getCombatState() + -- Combat state is only extra debug context. If the API is unavailable or + -- errors in tests, leave it blank instead of failing layout. if type(InCombatLockdown) ~= "function" then return nil end @@ -393,6 +454,8 @@ local function getCombatState() end local function getViewerDiagnostics(blizzFrame, viewerKey, why, itemFrames) + -- Build a small snapshot for error logs so bug reports explain what the + -- Blizzard viewer looked like when ECM could not safely read its children. local itemFramesAccessible = nil if type(itemFrames) == "table" then itemFramesAccessible = canAccessTable(itemFrames) @@ -420,6 +483,9 @@ local function getViewerDiagnostics(blizzFrame, viewerKey, why, itemFrames) end local function getAccessibleItemFrames(blizzFrame, viewerKey, why) + -- Blizzard owns the real cooldown viewer children. Read them carefully: + -- if another addon or secure code makes the list unsafe, log once and keep + -- the layout conservative instead of breaking the UI. local ok, itemFrames = pcall(blizzFrame.GetItemFrames, blizzFrame) if not ok then local data = getViewerDiagnostics(blizzFrame, viewerKey, why, nil) @@ -449,6 +515,9 @@ end -------------------------------------------------------------------------------- function ExtraIcons:CreateFrame() + -- Create one invisible parent plus one container per Blizzard viewer. The + -- containers hold visible extra icons; the anchor frames are invisible + -- measuring sticks for modules that chain after ExtraIcons. local parent = CreateFrame("Frame", "ECMExtraIcons", UIParent) parent:SetFrameStrata("MEDIUM") parent:SetSize(1, 1) @@ -477,6 +546,8 @@ function ExtraIcons:CreateFrame() end function ExtraIcons:ShouldShow() + -- ExtraIcons only has something useful to do when the base module is + -- enabled and at least one Blizzard cooldown viewer row is visible. if not BarMixin.FrameProto.ShouldShow(self) then return false end for _, v in ipairs(VIEWERS) do local frame = _G[v.blizzKey] @@ -487,6 +558,8 @@ end --- Returns the logical anchor frame used by chained ECM modules so they --- inherit the combined width of Blizzard icons plus appended extra icons. +--- ELI5: tell the next module to stand after the whole row, including our +--- added icons, not just after Blizzard's original row. function ExtraIcons:GetMainViewerAnchor() local vs = self._viewers and self._viewers.main local anchor = vs and vs.anchorFrame @@ -495,6 +568,9 @@ function ExtraIcons:GetMainViewerAnchor() end function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, sharedOffsetX, moduleConfig, why) + -- Lay out one row. If there are no extra icons, restore the Blizzard row to + -- its saved home position. If there are extras, shift Blizzard left and put + -- the ECM icons immediately to its right so the combined row stays centered. local blizzFrame = _G[viewerConfig.blizzKey] local vs = self._viewers[viewerConfig.key] if not vs then return false end @@ -530,6 +606,10 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared local ok, err = pcall(function() for _, itemFrame in ipairs(itemFrames) do if itemFrame.isActive then + -- Match Blizzard's active icon size. Hidden active frames + -- still provide size, but they should not be used as the + -- right-side anchor because anchoring to hidden frames can + -- make extras jump or disappear. iconSize = itemFrame:GetWidth() or iconSize if itemFrame:IsShown() then lastActive = itemFrame end end @@ -571,6 +651,8 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared icon.Icon:SetTexture(data.texture) icon.Icon:SetDesaturated(data.missing == true) + -- Place each pooled extra icon at its x offset inside the container. + -- LazySetAnchors skips the work if the icon is already there. FrameUtil.LazySetAnchors(icon, { { "LEFT", container, "LEFT", xOffset, 0 } }) icon:Show() @@ -587,6 +669,8 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared xOffset = xOffset + iconSize + spacing end + -- Attach the container after the last shown Blizzard active icon, or after + -- the viewer itself if Blizzard has no visible active child to anchor to. FrameUtil.LazySetAnchors(container, { { "LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0 } }) container:Show() @@ -595,6 +679,9 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared end function ExtraIcons:UpdateLayout(why) + -- This is the main layout entry point. It resolves configured entries, + -- updates both viewer rows, keeps shared rows centered, and then refreshes + -- cooldown/count text for any icons that were placed. if not self.InnerFrame or not self._viewers then return false end local shouldShow = self:ShouldShow() @@ -635,6 +722,8 @@ function ExtraIcons:UpdateLayout(why) end function ExtraIcons:Refresh(why, force) + -- Refresh is lighter than layout: it keeps existing icons in place and only + -- updates timers/counts that can change while the row is already visible. if not BarMixin.FrameProto.Refresh(self, why, force) then return false end if not self._viewers then return false end @@ -663,28 +752,40 @@ end -------------------------------------------------------------------------------- function ExtraIcons:OnBagUpdateCooldown() + -- Bag item cooldowns can change without a full layout change, so only + -- refresh the visible cooldown swipes. self:ThrottledRefresh("OnBagUpdateCooldown") end function ExtraIcons:OnBagUpdateDelayed() + -- Bag contents changed. Re-resolve entries because an item may have been + -- gained, consumed, or removed. ns.Runtime.RequestLayout("ExtraIcons:OnBagUpdateDelayed") end function ExtraIcons:OnPlayerEquipmentChanged(slotId) + -- Only relayout for watched gear slots. This avoids work when unrelated + -- equipment changes cannot affect any configured extra icon. if self._trackedEquipSlots and self._trackedEquipSlots[slotId] then ns.Runtime.RequestLayout("ExtraIcons:OnPlayerEquipmentChanged") end end function ExtraIcons:OnPlayerEnteringWorld() + -- Instance/PvP state and Blizzard viewer state can change on zone load, so + -- rebuild the row after entering the world. ns.Runtime.RequestLayout("ExtraIcons:OnPlayerEnteringWorld") end function ExtraIcons:OnSpellsChanged() + -- Talents, racials, or spellbook updates can change which spell entries are + -- known by the character. ns.Runtime.RequestLayout("ExtraIcons:OnSpellsChanged") end --- Rebuilds the set of equipment slots referenced by current config. +--- ELI5: make a checklist of gear slots that matter, so equipment events can +--- ignore slots the user never configured. function ExtraIcons:_rebuildTrackedSlots() local tracked = {} local config = self:GetModuleConfig() @@ -704,6 +805,8 @@ function ExtraIcons:_rebuildTrackedSlots() end function ExtraIcons:HookEditMode() + -- Edit Mode lets Blizzard move its own frames. Hide ECM extras while the + -- user is editing, then recache positions after Edit Mode closes. local mgr = _G.EditModeManagerFrame if not mgr or self._editModeHooked then return end self._editModeHooked = true @@ -730,6 +833,8 @@ function ExtraIcons:HookEditMode() end function ExtraIcons:_hookViewer(viewerKey) + -- Watch Blizzard viewer show/hide/resize events. Whenever Blizzard changes + -- the base row, ask ECM to rebuild extras around the new row. local blizzKey = BLIZZ_KEY[viewerKey] local blizzFrame = _G[blizzKey] local vs = self._viewers and self._viewers[viewerKey] @@ -765,10 +870,13 @@ end -------------------------------------------------------------------------------- function ExtraIcons:OnInitialize() + -- Attach the shared bar-frame behavior used by ECM modules. BarMixin.AddFrameMixin(self, "ExtraIcons") end function ExtraIcons:OnEnable() + -- Start the module: create frames, register with the layout runtime, track + -- relevant equipment slots, and subscribe to game events that affect icons. self:EnsureFrame() ns.Runtime.RegisterFrame(self) self:_rebuildTrackedSlots() @@ -795,6 +903,8 @@ function ExtraIcons:OnEnable() end function ExtraIcons:OnDisable() + -- Stop the module and clear cached layout state so the next enable starts + -- from Blizzard's current frame positions. self:UnregisterAllEvents() self:UpdateLayout("OnDisable") ns.Runtime.UnregisterFrame(self) From b91584b98690bbe434b24a2737ec65d63e3da3df Mon Sep 17 00:00:00 2001 From: Argi <15852038+argium@users.noreply.github.com> Date: Mon, 18 May 2026 18:28:03 +1000 Subject: [PATCH 5/9] Add debug --- Modules/ExtraIcons.lua | 62 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 3e190d03..6dcd42ff 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -75,14 +75,53 @@ local function horizontalBounds(point, width) return -h, h end +local function getFrameDebugName(frame) + if not frame then return nil end + local name = frame.GetName and frame:GetName() + return name or tostring(frame) +end + +local function getPointDebugData(point) + if not point then return nil end + return { + point = point[1], + relativeTo = getFrameDebugName(point[2]), + relativePoint = point[3], + x = point[4], + y = point[5], + } +end + +local function logSharedOffsets(why, reason, mainFrame, utilFrame, mainPoint, utilPoint, offsets) + if not ns.IsDebugEnabled() then return end + ns.Log("ExtraIcons", "Shared offsets: " .. reason, { + reason = why, + main = { + shown = mainFrame and mainFrame:IsShown(), + width = mainFrame and mainFrame:GetWidth(), + point = getPointDebugData(mainPoint), + offset = offsets.main, + }, + utility = { + shown = utilFrame and utilFrame:IsShown(), + width = utilFrame and utilFrame:GetWidth(), + point = getPointDebugData(utilPoint), + offset = offsets.utility, + }, + }) +end + --- Computes a per-viewer horizontal offset that re-centers both viewers as a --- single stacked group when they share the same original anchor. --- ELI5: if two rows are stacked on the same nail, and one row becomes wider, --- nudge each row so the whole stack still looks centered on that nail. -local function getSharedOffsets(viewers) +local function getSharedOffsets(viewers, why) local offsets = { main = 0, utility = 0 } local mainState, utilState = viewers.main, viewers.utility - if not mainState or not utilState then return offsets end + if not mainState or not utilState then + logSharedOffsets(why, "missing viewer state", nil, nil, nil, nil, offsets) + return offsets + end local mainFrame = _G[BLIZZ_KEY.main] local utilFrame = _G[BLIZZ_KEY.utility] @@ -90,9 +129,16 @@ local function getSharedOffsets(viewers) cachePoint(utilState, utilFrame) local mp, up = mainState.originalPoint, utilState.originalPoint - if not mp or not up then return offsets end - if mainFrame and up[2] == mainFrame then return offsets end + if not mp or not up then + logSharedOffsets(why, "missing original point", mainFrame, utilFrame, mp, up, offsets) + return offsets + end + if mainFrame and up[2] == mainFrame then + logSharedOffsets(why, "utility anchored to main", mainFrame, utilFrame, mp, up, offsets) + return offsets + end if up[1] ~= mp[1] or up[2] ~= mp[2] or up[3] ~= mp[3] or up[4] ~= mp[4] then + logSharedOffsets(why, "independent anchors", mainFrame, utilFrame, mp, up, offsets) return offsets end @@ -106,7 +152,10 @@ local function getSharedOffsets(viewers) sharedRight = sharedRight and math.max(sharedRight, r) or r end end - if not sharedLeft then return offsets end + if not sharedLeft then + logSharedOffsets(why, "no shown viewers", mainFrame, utilFrame, mp, up, offsets) + return offsets + end local center = (sharedLeft + sharedRight) / 2 for _, v in ipairs(VIEWERS) do @@ -117,6 +166,7 @@ local function getSharedOffsets(viewers) offsets[v.key] = center - ((l + r) / 2) end end + logSharedOffsets(why, "computed", mainFrame, utilFrame, mp, up, offsets) return offsets end @@ -695,7 +745,7 @@ function ExtraIcons:UpdateLayout(why) -- When hidden, leave viewers nil so each call gets empty entries, which -- restores Blizzard viewer positions and hides extra-icon containers. local viewers = shouldShow and moduleConfig and moduleConfig.viewers - local offsets = getSharedOffsets(self._viewers) + local offsets = getSharedOffsets(self._viewers, why) local anyPlaced = false for _, v in ipairs(VIEWERS) do From 97fbdf9b74b75225f83210161d81cf76c28a737a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 08:46:42 +0000 Subject: [PATCH 6/9] Address ExtraIcons review feedback Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/4c2a168f-7f02-404f-a7ac-065bd406849a Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Constants.lua | 12 +- ECM.lua | 10 + Modules/ExtraIcons.lua | 323 +++++++----------------------- Tests/Modules/ExtraIcons_spec.lua | 18 +- 4 files changed, 108 insertions(+), 255 deletions(-) diff --git a/Constants.lua b/Constants.lua index 06bbf5b4..c673ba57 100644 --- a/Constants.lua +++ b/Constants.lua @@ -184,6 +184,8 @@ local BUILTIN_STACKS = { --- Default display order for builtin stack keys (matches default viewers.utility order). local BUILTIN_STACK_ORDER = { "trinket1", "trinket2" } +local DRACTHYR_WING_BUFFET_IDS = { 357214, 368970 } + --- Racial ability lookup keyed by UnitRace("player") raceFileName. --- One primary active racial per race. local RACIAL_ABILITIES = { @@ -210,10 +212,17 @@ local RACIAL_ABILITIES = { Vulpera = { spellId = 312411 }, -- Bag of Tricks MagharOrc = { spellId = 274738 }, -- Ancestral Call Mechagnome = { spellId = 312924 }, -- Hyper Organic Light Originator - Dracthyr = { spellIds = { 357214, 368970 } }, -- Tail Swipe + Dracthyr = { spellIds = DRACTHYR_WING_BUFFET_IDS }, -- Wing Buffet EarthenDwarf = { spellId = 436717 }, -- Azerite Surge } +--- Some racial abilities have different spell IDs. For example, Dracthyr evokers +--- have a more potent wing buffet compared to other classes. +local RACIAL_SPELL_ALIASES = { + [357214] = DRACTHYR_WING_BUFFET_IDS, + [368970] = DRACTHYR_WING_BUFFET_IDS, +} + local BLIZZARD_FRAMES = { "EssentialCooldownViewer", "UtilityCooldownViewer", @@ -287,6 +296,7 @@ constants.BLIZZARD_FRAMES = BLIZZARD_FRAMES constants.BUILTIN_STACKS = BUILTIN_STACKS constants.BUILTIN_STACK_ORDER = BUILTIN_STACK_ORDER constants.RACIAL_ABILITIES = RACIAL_ABILITIES +constants.RACIAL_SPELL_ALIASES = RACIAL_SPELL_ALIASES constants.RESOURCEBAR_CASTABLE_MAX_COLOR_SPELLS = resourceBarCastableMaxColorSpells constants.CLASS_COLORS = CLASS_COLORS constants.RESOURCEBAR_MAX_COLOR_TYPES = resourceBarMaxColorTypes diff --git a/ECM.lua b/ECM.lua index b41e7f2e..ba01f878 100644 --- a/ECM.lua +++ b/ECM.lua @@ -115,6 +115,16 @@ function ns.CloneValue(value) return copy end +function ns.GetFrameValue(frame, methodName) + if not frame or type(frame[methodName]) ~= "function" then + return nil + end + + local ok, value = pcall(frame[methodName], frame) + if ok then return value end + return nil +end + ns.Print = LibConsole:NewPrinter(function(message) print(ns.ColorUtil.Sparkle(L["ADDON_ABRV"] .. ":") .. " " .. message) end) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 6dcd42ff..1392da7b 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -8,64 +8,32 @@ local FrameUtil = ns.FrameUtil local ExtraIcons = ns.Addon:NewModule("ExtraIcons") ns.Addon.ExtraIcons = ExtraIcons --- ExtraIcons adds the user's chosen potions, trinkets, racials, and spells --- directly after Blizzard's cooldown viewer rows. Think of each Blizzard row --- as a train: this module adds a small extra car to the end without moving the --- train station anchor that other ECM modules use. local BUILTIN_STACKS = ns.Constants.BUILTIN_STACKS -local RACIAL_ABILITIES = ns.Constants.RACIAL_ABILITIES +local RACIAL_SPELL_ALIASES = ns.Constants.RACIAL_SPELL_ALIASES local DEFAULT_SIZE = ns.Constants.DEFAULT_EXTRA_ICON_SIZE local MAIN_BORDER_SCALE = ns.Constants.EXTRA_ICON_MAIN_BORDER_SCALE local UTILITY_BORDER_SCALE = ns.Constants.EXTRA_ICON_UTILITY_BORDER_SCALE local canAccessTable = _G.canaccesstable --- Some racial abilities have different spell IDs for different races/factions. --- This lookup lets a saved spell ID find its whole family of equivalent IDs. -local RACIAL_SPELL_ALIASES = {} -for _, racial in pairs(RACIAL_ABILITIES) do - local spellIds = racial.spellIds or { racial.spellId } - for _, spellId in ipairs(spellIds) do - RACIAL_SPELL_ALIASES[spellId] = spellIds - end -end - --- Ordered viewer keys mapped to their Blizzard frame globals. The order matters --- because the main and utility rows may need to move together as one visual --- group when extra icons widen one row. +local MAIN_VIEWER_KEY = "EssentialCooldownViewer" +local UTILITY_VIEWER_KEY = "UtilityCooldownViewer" local VIEWERS = { - { key = "main", blizzKey = "EssentialCooldownViewer", borderScale = { MAIN_BORDER_SCALE, MAIN_BORDER_SCALE }, ownsAnchor = true }, + { key = "main", blizzKey = MAIN_VIEWER_KEY, borderScale = { MAIN_BORDER_SCALE, MAIN_BORDER_SCALE }, ownsAnchor = true }, -- Utility icon frames render square; keep the overlay square so extras do not look short. - { key = "utility", blizzKey = "UtilityCooldownViewer", borderScale = { UTILITY_BORDER_SCALE, UTILITY_BORDER_SCALE } }, + { key = "utility", blizzKey = UTILITY_VIEWER_KEY, borderScale = { UTILITY_BORDER_SCALE, UTILITY_BORDER_SCALE } }, } -local BLIZZ_KEY = {} -for _, v in ipairs(VIEWERS) do BLIZZ_KEY[v.key] = v.blizzKey end -------------------------------------------------------------------------------- -- Shared horizontal centering -------------------------------------------------------------------------------- local function cachePoint(vs, blizzFrame) - -- Save the Blizzard viewer's first anchor before ECM moves it. Future - -- layouts always start from this remembered "home" position so small - -- refreshes do not keep adding offsets on top of old offsets. if vs.originalPoint or not blizzFrame then return end local point, relativeTo, relativePoint, x, y = blizzFrame:GetPoint() vs.originalPoint = { point, relativeTo, relativePoint, x or 0, y or 0 } end -local function applyPoint(vs, blizzFrame, offsetX) - -- Move the Blizzard viewer relative to its saved home position. The shared - -- FrameUtil helper does nothing when the anchor is already correct, which - -- prevents unnecessary clear/re-anchor work and visual flicker. - local p = vs and vs.originalPoint - if not p or not blizzFrame then return end - FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + (offsetX or 0), p[5] } }) -end - -local function horizontalBounds(point, width) - -- Convert an anchor name into "how far this frame reaches left and right - -- from its anchor." A left-anchored frame grows to the right, a right - -- anchored frame grows to the left, and a centered frame grows both ways. +local function getHorizontalBounds(point, width) if point == "LEFT" or point == "TOPLEFT" or point == "BOTTOMLEFT" then return 0, width elseif point == "RIGHT" or point == "TOPRIGHT" or point == "BOTTOMRIGHT" then @@ -111,11 +79,9 @@ local function logSharedOffsets(why, reason, mainFrame, utilFrame, mainPoint, ut }) end ---- Computes a per-viewer horizontal offset that re-centers both viewers as a ---- single stacked group when they share the same original anchor. ---- ELI5: if two rows are stacked on the same nail, and one row becomes wider, ---- nudge each row so the whole stack still looks centered on that nail. -local function getSharedOffsets(viewers, why) +--- Computes per-viewer offsets that re-center both viewers as a single stacked +--- group when they share the same original anchor. +local function getAdjustedOffsetsToRecenterViewer(viewers, why) local offsets = { main = 0, utility = 0 } local mainState, utilState = viewers.main, viewers.utility if not mainState or not utilState then @@ -123,8 +89,8 @@ local function getSharedOffsets(viewers, why) return offsets end - local mainFrame = _G[BLIZZ_KEY.main] - local utilFrame = _G[BLIZZ_KEY.utility] + local mainFrame = _G[MAIN_VIEWER_KEY] + local utilFrame = _G[UTILITY_VIEWER_KEY] cachePoint(mainState, mainFrame) cachePoint(utilState, utilFrame) @@ -147,7 +113,7 @@ local function getSharedOffsets(viewers, why) local frame = _G[v.blizzKey] local p = viewers[v.key].originalPoint if frame and frame:IsShown() and p then - local l, r = horizontalBounds(p[1], frame:GetWidth() or 0) + local l, r = getHorizontalBounds(p[1], frame:GetWidth() or 0) sharedLeft = sharedLeft and math.min(sharedLeft, l) or l sharedRight = sharedRight and math.max(sharedRight, r) or r end @@ -162,7 +128,7 @@ local function getSharedOffsets(viewers, why) local frame = _G[v.blizzKey] local p = viewers[v.key].originalPoint if frame and frame:IsShown() and p then - local l, r = horizontalBounds(p[1], frame:GetWidth() or 0) + local l, r = getHorizontalBounds(p[1], frame:GetWidth() or 0) offsets[v.key] = center - ((l + r) / 2) end end @@ -171,10 +137,6 @@ local function getSharedOffsets(viewers, why) end local function updateMainViewerAnchor(vs, blizzFrame, rightFrame) - -- Chained ECM modules ask this module where the main row ends. This hidden - -- anchor stretches from Blizzard's main viewer to the extra-icon container, - -- so the next module attaches after the combined width instead of only - -- after Blizzard's original icons. local anchor = vs and vs.anchorFrame if not anchor then return end @@ -196,9 +158,7 @@ end -- Entry resolution -------------------------------------------------------------------------------- -local function resolveEquipSlot(slotId) - -- Equipment entries track a gear slot, not a fixed item. Resolve the item - -- currently worn in that slot so swapping trinkets updates the extra icon. +local function getEquipSlotIconData(slotId) local itemId = GetInventoryItemID("player", slotId) if not itemId then return nil end local _, spellId = C_Item.GetItemSpell(itemId) @@ -208,10 +168,7 @@ local function resolveEquipSlot(slotId) return { itemId = itemId, texture = texture, slotId = slotId } end -local function resolveItem(ids, showIfMissing) - -- Item entries can list several item IDs in priority order. Use the first - -- owned item with an icon; optionally show a grey missing icon for the first - -- configured item if none are owned. +local function resolveFirstItem(ids, showIfMissing) local missingData for _, entry in ipairs(ids) do @@ -228,28 +185,24 @@ local function resolveItem(ids, showIfMissing) return missingData end -local function resolveKnownSpell(spellId) - -- A spell is usable only if the current character knows it and Blizzard can - -- provide an icon for it. +local function getKnownSpellData(spellId) if spellId and C_SpellBook.IsSpellKnown(spellId) then local texture = C_Spell.GetSpellTexture(spellId) if texture then return { spellId = spellId, texture = texture } end end end -local function resolveSpell(ids) - -- Spell entries also use priority order. Racial spells may resolve through - -- aliases because the configured spell may have a race-specific equivalent. +local function resolveFirstKnownSpell(ids) for _, entry in ipairs(ids) do local spellId = type(entry) == "table" and entry.spellId or entry - local data = resolveKnownSpell(spellId) + local data = getKnownSpellData(spellId) if data then return data end local aliases = RACIAL_SPELL_ALIASES[spellId] if aliases then for _, aliasSpellId in ipairs(aliases) do if aliasSpellId ~= spellId then - data = resolveKnownSpell(aliasSpellId) + data = getKnownSpellData(aliasSpellId) if data then return data end end end @@ -257,37 +210,25 @@ local function resolveSpell(ids) end end -local function isNonPvpInstance() - -- "Non-PvP instance" means dungeons, raids, scenarios, and similar places - -- where potion rules may differ from battlegrounds and arenas. - local inInstance, instanceType = IsInInstance() - return inInstance and instanceType ~= "pvp" and instanceType ~= "arena" -end +local function shouldShowItemStack(itemStack) + if not itemStack then return false end + if itemStack.hideInRatedPvp and C_PvP.IsRatedMap() then return false end -local function shouldSuppressItemStack(itemStack) - -- Some built-in item groups intentionally disappear in rated PvP or - -- non-PvP instances so the row only shows context-relevant consumables. - if not itemStack then return true end - if itemStack.hideInRatedPvp and C_PvP.IsRatedMap() then return true end - if itemStack.hideInInstances and isNonPvpInstance() then return true end - return false + local inInstance, instanceType = IsInInstance() + if itemStack.hideInInstances and inInstance and instanceType ~= "pvp" and instanceType ~= "arena" then return false end + return true end -local function resolveItemStack(entry, moduleConfig) - -- Custom item stacks are user-configured groups such as "my preferred - -- health potions." Resolve the stack's item list like a normal item entry. +local function resolveConfiguredItemStack(entry, moduleConfig) local itemStacks = moduleConfig and moduleConfig.itemStacks local itemStack = itemStacks and itemStacks.byId and itemStacks.byId[entry.itemStackId] - return not shouldSuppressItemStack(itemStack) + return shouldShowItemStack(itemStack) and itemStack.ids - and resolveItem(itemStack.ids, itemStack.showIfMissing == true) + and resolveFirstItem(itemStack.ids, itemStack.showIfMissing == true) or nil end local function resolveEntry(entry, moduleConfig) - -- Turn one saved config entry into one drawable icon, or nil if it should - -- not be shown right now. Built-in stack keys expand into their real entry - -- data before the normal kind-specific resolver runs. local kind, slotId, ids if entry.stackKey then local stack = BUILTIN_STACKS[entry.stackKey] @@ -296,16 +237,14 @@ local function resolveEntry(entry, moduleConfig) else kind, slotId, ids = entry.kind, entry.slotId, entry.ids end - if kind == "equipSlot" then return resolveEquipSlot(slotId) end - if kind == "item" then return ids and resolveItem(ids) end - if kind == "itemStack" then return resolveItemStack(entry, moduleConfig) end - if kind == "spell" then return ids and resolveSpell(ids) end + if kind == "equipSlot" then return getEquipSlotIconData(slotId) end + if kind == "item" then return ids and resolveFirstItem(ids) end + if kind == "itemStack" then return resolveConfiguredItemStack(entry, moduleConfig) end + if kind == "spell" then return ids and resolveFirstKnownSpell(ids) end end local _resolved = {} local function resolveEntries(entries, moduleConfig) - -- Reuse one temporary result table on every layout pass to avoid allocating - -- a new table during frequent UI refreshes. wipe(_resolved) for _, entry in ipairs(entries) do local data = not entry.disabled and resolveEntry(entry, moduleConfig) or nil @@ -319,9 +258,6 @@ end -------------------------------------------------------------------------------- local function createIcon(parent, size, borderScale) - -- Build one square extra icon using Blizzard-like parts: artwork, mask, - -- cooldown swipe, border, count text, and optional shadow. The caller - -- reuses these icons from a pool instead of creating new ones every layout. local icon = CreateFrame("Button", nil, parent) icon:SetSize(size, size) @@ -363,9 +299,6 @@ local function createIcon(parent, size, borderScale) end local function updateIconCooldown(icon) - -- Pick the right Blizzard cooldown API for the thing shown on this icon: - -- spells use spell cooldowns, equipment slots use inventory cooldowns, and - -- bag items use item cooldowns. if icon.spellId then local cdInfo = C_Spell.GetSpellCooldown(icon.spellId) if cdInfo and cdInfo.isOnGCD then @@ -402,9 +335,7 @@ local function updateIconCooldown(icon) end end -local function setIconCountText(icon, text) - -- Count text is optional. Nil means "hide the number"; any value means - -- "show this number on top of the icon." +local function applyIconCountText(icon, text) if text ~= nil then icon.Count:SetText(tostring(text)) icon.Count:Show() @@ -415,15 +346,13 @@ local function setIconCountText(icon, text) end local function updateIconCountText(icon, globalConfig, config) - -- Item icons can show stack counts and spell icons can show charges. Only - -- show useful numbers: one item or a non-charge spell should stay clean. if not icon.Count then return end FrameUtil.ApplyFont(icon.Count, globalConfig, config) if icon.itemId and (not config or config.showStackCount ~= false) then local count = C_Item.GetItemCount(icon.itemId) if count and count > 1 then - setIconCountText(icon, count) + applyIconCountText(icon, count) return end end @@ -431,54 +360,15 @@ local function updateIconCountText(icon, globalConfig, config) if icon.spellId and (not config or config.showCharges ~= false) then local charges = C_Spell.GetSpellCharges(icon.spellId) if charges and charges.maxCharges and charges.maxCharges > 1 and charges.currentCharges ~= nil then - setIconCountText(icon, charges.currentCharges) + applyIconCountText(icon, charges.currentCharges) return end end - setIconCountText(icon, nil) -end - ---- Caches and returns the cooldown number font from a sibling Blizzard icon. ---- ELI5: copy Blizzard's number style so ECM's extra icons look like they ---- belong in the same row. -local function getSiblingFont(viewer) - local cached = viewer.__ecmCDFont - if cached then return cached[1], cached[2], cached[3] end - - for _, child in ipairs({ viewer:GetChildren() }) do - local cooldown = child.Cooldown - if cooldown and cooldown.GetRegions then - local region = cooldown:GetRegions() - if region and region.IsObjectType and region:IsObjectType("FontString") and region.GetFont then - local fp, fs, ff = region:GetFont() - if fp and fs then - viewer.__ecmCDFont = { fp, fs, ff } - return fp, fs, ff - end - end - end - end -end - -local function getFrameValue(frame, methodName) - -- Diagnostics should never create a new error while trying to describe the - -- original problem, so every optional frame read is protected. - if not frame or type(frame[methodName]) ~= "function" then - return nil - end - - local ok, value = pcall(frame[methodName], frame) - if ok then - return value - end - - return nil + applyIconCountText(icon, nil) end local function getItemFramesCount(itemFrames) - -- Count Blizzard's item frame array only when Lua is allowed to read it. - -- Some protected/tainted tables must not be iterated directly. if type(itemFrames) ~= "table" or not canAccessTable(itemFrames) then return nil end @@ -492,56 +382,39 @@ local function getItemFramesCount(itemFrames) return ok and count or nil end -local function getCombatState() - -- Combat state is only extra debug context. If the API is unavailable or - -- errors in tests, leave it blank instead of failing layout. - if type(InCombatLockdown) ~= "function" then - return nil - end - - local ok, inCombat = pcall(InCombatLockdown) - return ok and inCombat == true or nil -end - -local function getViewerDiagnostics(blizzFrame, viewerKey, why, itemFrames) - -- Build a small snapshot for error logs so bug reports explain what the - -- Blizzard viewer looked like when ECM could not safely read its children. +local function getViewerDiagnostics(blizzFrame, viewerConfig, why, itemFrames) local itemFramesAccessible = nil if type(itemFrames) == "table" then itemFramesAccessible = canAccessTable(itemFrames) end return { - viewerKey = viewerKey, - blizzardFrameKey = BLIZZ_KEY[viewerKey], + viewerKey = viewerConfig.key, + blizzardFrameKey = viewerConfig.blizzKey, reason = why, viewerExists = blizzFrame ~= nil, - viewerName = getFrameValue(blizzFrame, "GetName"), - viewerShown = getFrameValue(blizzFrame, "IsShown"), - viewerWidth = getFrameValue(blizzFrame, "GetWidth"), - viewerHeight = getFrameValue(blizzFrame, "GetHeight"), - viewerAlpha = getFrameValue(blizzFrame, "GetAlpha"), - viewerNumPoints = getFrameValue(blizzFrame, "GetNumPoints"), + viewerName = ns.GetFrameValue(blizzFrame, "GetName"), + viewerShown = ns.GetFrameValue(blizzFrame, "IsShown"), + viewerWidth = ns.GetFrameValue(blizzFrame, "GetWidth"), + viewerHeight = ns.GetFrameValue(blizzFrame, "GetHeight"), + viewerAlpha = ns.GetFrameValue(blizzFrame, "GetAlpha"), + viewerNumPoints = ns.GetFrameValue(blizzFrame, "GetNumPoints"), viewerIconScale = blizzFrame and blizzFrame.iconScale or nil, viewerChildXPadding = blizzFrame and blizzFrame.childXPadding or nil, viewerHasGetItemFrames = blizzFrame ~= nil and type(blizzFrame.GetItemFrames) == "function", itemFramesType = type(itemFrames), itemFramesAccessible = itemFramesAccessible, itemFramesArrayCount = getItemFramesCount(itemFrames), - inCombatLockdown = getCombatState(), } end -local function getAccessibleItemFrames(blizzFrame, viewerKey, why) - -- Blizzard owns the real cooldown viewer children. Read them carefully: - -- if another addon or secure code makes the list unsafe, log once and keep - -- the layout conservative instead of breaking the UI. +local function getAccessibleItemFrames(blizzFrame, viewerConfig, why) local ok, itemFrames = pcall(blizzFrame.GetItemFrames, blizzFrame) if not ok then - local data = getViewerDiagnostics(blizzFrame, viewerKey, why, nil) + local data = getViewerDiagnostics(blizzFrame, viewerConfig, why, nil) data.error = tostring(itemFrames) - ns.ErrorLogOnce("ExtraIcons", "GetItemFrames:" .. viewerKey, - "Unable to read cooldown viewer item frames for " .. viewerKey .. " during " + ns.ErrorLogOnce("ExtraIcons", "GetItemFrames:" .. viewerConfig.key, + "Unable to read cooldown viewer item frames for " .. viewerConfig.key .. " during " .. tostring(why or "unknown") .. ": " .. tostring(itemFrames), data) return nil end @@ -551,9 +424,9 @@ local function getAccessibleItemFrames(blizzFrame, viewerKey, why) end if not canAccessTable(itemFrames) then - ns.ErrorLogOnce("ExtraIcons", "InaccessibleItemFrames:" .. viewerKey, - "Cooldown viewer item frames are inaccessible for " .. viewerKey .. " during " - .. tostring(why or "unknown"), getViewerDiagnostics(blizzFrame, viewerKey, why, itemFrames)) + ns.ErrorLogOnce("ExtraIcons", "InaccessibleItemFrames:" .. viewerConfig.key, + "Cooldown viewer item frames are inaccessible for " .. viewerConfig.key .. " during " + .. tostring(why or "unknown"), getViewerDiagnostics(blizzFrame, viewerConfig, why, itemFrames)) return nil end @@ -565,9 +438,6 @@ end -------------------------------------------------------------------------------- function ExtraIcons:CreateFrame() - -- Create one invisible parent plus one container per Blizzard viewer. The - -- containers hold visible extra icons; the anchor frames are invisible - -- measuring sticks for modules that chain after ExtraIcons. local parent = CreateFrame("Frame", "ECMExtraIcons", UIParent) parent:SetFrameStrata("MEDIUM") parent:SetSize(1, 1) @@ -578,10 +448,13 @@ function ExtraIcons:CreateFrame() container:SetFrameStrata("MEDIUM") container:SetSize(1, 1) - local anchor = CreateFrame("Frame", "ECMExtraIcons_" .. v.key .. "Anchor", parent) - anchor:SetFrameStrata("MEDIUM") - anchor:SetSize(1, 1) - anchor:Hide() + local anchor + if v.ownsAnchor then + anchor = CreateFrame("Frame", "ECMExtraIcons_" .. v.key .. "Anchor", parent) + anchor:SetFrameStrata("MEDIUM") + anchor:SetSize(1, 1) + anchor:Hide() + end self._viewers[v.key] = { anchorFrame = anchor, @@ -596,8 +469,6 @@ function ExtraIcons:CreateFrame() end function ExtraIcons:ShouldShow() - -- ExtraIcons only has something useful to do when the base module is - -- enabled and at least one Blizzard cooldown viewer row is visible. if not BarMixin.FrameProto.ShouldShow(self) then return false end for _, v in ipairs(VIEWERS) do local frame = _G[v.blizzKey] @@ -608,19 +479,14 @@ end --- Returns the logical anchor frame used by chained ECM modules so they --- inherit the combined width of Blizzard icons plus appended extra icons. ---- ELI5: tell the next module to stand after the whole row, including our ---- added icons, not just after Blizzard's original row. function ExtraIcons:GetMainViewerAnchor() local vs = self._viewers and self._viewers.main local anchor = vs and vs.anchorFrame if anchor and anchor:IsShown() then return anchor end - return _G[BLIZZ_KEY.main] + return _G[MAIN_VIEWER_KEY] end function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, sharedOffsetX, moduleConfig, why) - -- Lay out one row. If there are no extra icons, restore the Blizzard row to - -- its saved home position. If there are extras, shift Blizzard left and put - -- the ECM icons immediately to its right so the combined row stays centered. local blizzFrame = _G[viewerConfig.blizzKey] local vs = self._viewers[viewerConfig.key] if not vs then return false end @@ -632,7 +498,10 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared and {} or resolveEntries(entries, moduleConfig) if #items == 0 then - applyPoint(vs, blizzFrame, sharedOffsetX) + local p = vs.originalPoint + if p and blizzFrame then + FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + sharedOffsetX, p[5] } }) + end if isEditing then vs.originalPoint = nil end container:Hide() if viewerConfig.ownsAnchor then updateMainViewerAnchor(vs, blizzFrame, nil) end @@ -644,22 +513,17 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared vs.iconPool[i] = createIcon(container, DEFAULT_SIZE, viewerConfig.borderScale) end - local fontPath, fontSize, fontFlags = getSiblingFont(blizzFrame) local globalConfig = self:GetGlobalConfig() local iconSize = DEFAULT_SIZE local viewerScale = blizzFrame.iconScale or 1.0 local spacing = blizzFrame.childXPadding or 0 local lastActive = nil - local itemFrames = getAccessibleItemFrames(blizzFrame, viewerConfig.key, why) + local itemFrames = getAccessibleItemFrames(blizzFrame, viewerConfig, why) if itemFrames then local ok, err = pcall(function() for _, itemFrame in ipairs(itemFrames) do if itemFrame.isActive then - -- Match Blizzard's active icon size. Hidden active frames - -- still provide size, but they should not be used as the - -- right-side anchor because anchoring to hidden frames can - -- make extras jump or disappear. iconSize = itemFrame:GetWidth() or iconSize if itemFrame:IsShown() then lastActive = itemFrame end end @@ -668,7 +532,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared if not ok then iconSize = DEFAULT_SIZE lastActive = nil - local data = getViewerDiagnostics(blizzFrame, viewerConfig.key, why, itemFrames) + local data = getViewerDiagnostics(blizzFrame, viewerConfig, why, itemFrames) data.error = tostring(err) ns.ErrorLogOnce("ExtraIcons", "IterateItemFrames:" .. viewerConfig.key, "Unable to iterate cooldown viewer item frames for " .. viewerConfig.key .. " during " @@ -686,7 +550,10 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared -- on-screen centre already coincides with the original anchor; we only -- need to absorb the on-screen width of the gap + extra group. local extraOnScreen = (spacing + totalWidth) * viewerScale - applyPoint(vs, blizzFrame, sharedOffsetX - extraOnScreen / 2) + local p = vs.originalPoint + if p and blizzFrame then + FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + sharedOffsetX - extraOnScreen / 2, p[5] } }) + end local xOffset = 0 for i, data in ipairs(items) do @@ -701,26 +568,15 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared icon.Icon:SetTexture(data.texture) icon.Icon:SetDesaturated(data.missing == true) - -- Place each pooled extra icon at its x offset inside the container. - -- LazySetAnchors skips the work if the icon is already there. FrameUtil.LazySetAnchors(icon, { { "LEFT", container, "LEFT", xOffset, 0 } }) icon:Show() updateIconCooldown(icon) updateIconCountText(icon, globalConfig, moduleConfig) - if fontPath and fontSize then - local region = icon.Cooldown:GetRegions() - if region and region.IsObjectType and region:IsObjectType("FontString") and region.SetFont then - region:SetFont(fontPath, fontSize, fontFlags) - end - end - xOffset = xOffset + iconSize + spacing end - -- Attach the container after the last shown Blizzard active icon, or after - -- the viewer itself if Blizzard has no visible active child to anchor to. FrameUtil.LazySetAnchors(container, { { "LEFT", lastActive or blizzFrame, "RIGHT", spacing, 0 } }) container:Show() @@ -729,9 +585,6 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared end function ExtraIcons:UpdateLayout(why) - -- This is the main layout entry point. It resolves configured entries, - -- updates both viewer rows, keeps shared rows centered, and then refreshes - -- cooldown/count text for any icons that were placed. if not self.InnerFrame or not self._viewers then return false end local shouldShow = self:ShouldShow() @@ -745,7 +598,7 @@ function ExtraIcons:UpdateLayout(why) -- When hidden, leave viewers nil so each call gets empty entries, which -- restores Blizzard viewer positions and hides extra-icon containers. local viewers = shouldShow and moduleConfig and moduleConfig.viewers - local offsets = getSharedOffsets(self._viewers, why) + local offsets = getAdjustedOffsetsToRecenterViewer(self._viewers, why) local anyPlaced = false for _, v in ipairs(VIEWERS) do @@ -772,8 +625,6 @@ function ExtraIcons:UpdateLayout(why) end function ExtraIcons:Refresh(why, force) - -- Refresh is lighter than layout: it keeps existing icons in place and only - -- updates timers/counts that can change while the row is already visible. if not BarMixin.FrameProto.Refresh(self, why, force) then return false end if not self._viewers then return false end @@ -802,40 +653,28 @@ end -------------------------------------------------------------------------------- function ExtraIcons:OnBagUpdateCooldown() - -- Bag item cooldowns can change without a full layout change, so only - -- refresh the visible cooldown swipes. self:ThrottledRefresh("OnBagUpdateCooldown") end function ExtraIcons:OnBagUpdateDelayed() - -- Bag contents changed. Re-resolve entries because an item may have been - -- gained, consumed, or removed. ns.Runtime.RequestLayout("ExtraIcons:OnBagUpdateDelayed") end function ExtraIcons:OnPlayerEquipmentChanged(slotId) - -- Only relayout for watched gear slots. This avoids work when unrelated - -- equipment changes cannot affect any configured extra icon. if self._trackedEquipSlots and self._trackedEquipSlots[slotId] then ns.Runtime.RequestLayout("ExtraIcons:OnPlayerEquipmentChanged") end end function ExtraIcons:OnPlayerEnteringWorld() - -- Instance/PvP state and Blizzard viewer state can change on zone load, so - -- rebuild the row after entering the world. ns.Runtime.RequestLayout("ExtraIcons:OnPlayerEnteringWorld") end function ExtraIcons:OnSpellsChanged() - -- Talents, racials, or spellbook updates can change which spell entries are - -- known by the character. ns.Runtime.RequestLayout("ExtraIcons:OnSpellsChanged") end --- Rebuilds the set of equipment slots referenced by current config. ---- ELI5: make a checklist of gear slots that matter, so equipment events can ---- ignore slots the user never configured. function ExtraIcons:_rebuildTrackedSlots() local tracked = {} local config = self:GetModuleConfig() @@ -855,8 +694,6 @@ function ExtraIcons:_rebuildTrackedSlots() end function ExtraIcons:HookEditMode() - -- Edit Mode lets Blizzard move its own frames. Hide ECM extras while the - -- user is editing, then recache positions after Edit Mode closes. local mgr = _G.EditModeManagerFrame if not mgr or self._editModeHooked then return end self._editModeHooked = true @@ -882,12 +719,9 @@ function ExtraIcons:HookEditMode() end) end -function ExtraIcons:_hookViewer(viewerKey) - -- Watch Blizzard viewer show/hide/resize events. Whenever Blizzard changes - -- the base row, ask ECM to rebuild extras around the new row. - local blizzKey = BLIZZ_KEY[viewerKey] - local blizzFrame = _G[blizzKey] - local vs = self._viewers and self._viewers[viewerKey] +function ExtraIcons:_hookViewer(viewerConfig) + local blizzFrame = _G[viewerConfig.blizzKey] + local vs = self._viewers and self._viewers[viewerConfig.key] if not blizzFrame or not vs or vs.hooked then return end vs.hooked = true @@ -912,7 +746,7 @@ function ExtraIcons:_hookViewer(viewerKey) ns.Runtime.RequestLayout("ExtraIcons:OnSizeChanged") end) - ns.Log(self.Name, "Hooked " .. blizzKey) + ns.Log(self.Name, "Hooked " .. viewerConfig.blizzKey) end -------------------------------------------------------------------------------- @@ -920,13 +754,10 @@ end -------------------------------------------------------------------------------- function ExtraIcons:OnInitialize() - -- Attach the shared bar-frame behavior used by ECM modules. BarMixin.AddFrameMixin(self, "ExtraIcons") end function ExtraIcons:OnEnable() - -- Start the module: create frames, register with the layout runtime, track - -- relevant equipment slots, and subscribe to game events that affect icons. self:EnsureFrame() ns.Runtime.RegisterFrame(self) self:_rebuildTrackedSlots() @@ -947,14 +778,12 @@ function ExtraIcons:OnEnable() end self:HookEditMode() - for _, v in ipairs(VIEWERS) do self:_hookViewer(v.key) end + for _, v in ipairs(VIEWERS) do self:_hookViewer(v) end ns.Runtime.RequestLayout("ExtraIcons:OnEnable") end) end function ExtraIcons:OnDisable() - -- Stop the module and clear cached layout state so the next enable starts - -- from Blizzard's current frame positions. self:UnregisterAllEvents() self:UpdateLayout("OnDisable") ns.Runtime.UnregisterFrame(self) diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index 59a19463..adc2f877 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -303,6 +303,14 @@ describe("ExtraIcons real source", function() GetGlobalConfig = function() return globalConfig end, + GetFrameValue = function(frame, methodName) + if not frame or type(frame[methodName]) ~= "function" then + return nil + end + local ok, value = pcall(frame[methodName], frame) + if ok then return value end + return nil + end, } TestHelpers.LoadChunk("Constants.lua", "Unable to load Constants.lua")(nil, ns) TestHelpers.LoadChunk("Locales/en.lua", "Unable to load Locales/en.lua")(nil, ns) @@ -714,8 +722,8 @@ describe("ExtraIcons real source", function() it("hooks viewers only once", function() ExtraIcons.InnerFrame = ExtraIcons:CreateFrame() - ExtraIcons:_hookViewer("utility") - ExtraIcons:_hookViewer("utility") + ExtraIcons:_hookViewer({ key = "utility", blizzKey = "UtilityCooldownViewer" }) + ExtraIcons:_hookViewer({ key = "utility", blizzKey = "UtilityCooldownViewer" }) assert.is_true(ExtraIcons._viewers.utility.hooked) assert.are.equal(1, UtilityCooldownViewer:GetHookCount("OnShow")) @@ -785,7 +793,7 @@ describe("ExtraIcons real source", function() return enabled end - ExtraIcons:_hookViewer("utility") + ExtraIcons:_hookViewer({ key = "utility", blizzKey = "UtilityCooldownViewer" }) UtilityCooldownViewer._hooks.OnShow[1]() UtilityCooldownViewer._hooks.OnHide[1]() UtilityCooldownViewer._hooks.OnSizeChanged[1]() @@ -1018,10 +1026,6 @@ describe("ExtraIcons real source", function() assert.are.equal(14, vs.iconPool[2].slotId) assert.are.equal(COMBAT_POTION_ID, vs.iconPool[3].itemId) assert.are.equal(HEALTHSTONE_ID, vs.iconPool[4].itemId) - assert.same( - { "Fonts\\FRIZQT__.TTF", 17, "OUTLINE" }, - vs.iconPool[1].Cooldown.__fontRegion.__font - ) local point, relativeTo, relativePoint, x, y = UtilityCooldownViewer:GetPoint(1) assert.are.equal("CENTER", point) From 4974c13cfcb38566953748be5be9bccc6b361372 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 08:48:58 +0000 Subject: [PATCH 7/9] Document shared diagnostics helper Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/4c2a168f-7f02-404f-a7ac-065bd406849a Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Constants.lua | 2 +- ECM.lua | 1 + Tests/Modules/ExtraIcons_spec.lua | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Constants.lua b/Constants.lua index c673ba57..1f3bd565 100644 --- a/Constants.lua +++ b/Constants.lua @@ -184,7 +184,7 @@ local BUILTIN_STACKS = { --- Default display order for builtin stack keys (matches default viewers.utility order). local BUILTIN_STACK_ORDER = { "trinket1", "trinket2" } -local DRACTHYR_WING_BUFFET_IDS = { 357214, 368970 } +local DRACTHYR_WING_BUFFET_IDS = { 357214, 368970 } -- Base and enhanced evoker variants. --- Racial ability lookup keyed by UnitRace("player") raceFileName. --- One primary active racial per race. diff --git a/ECM.lua b/ECM.lua index ba01f878..af351342 100644 --- a/ECM.lua +++ b/ECM.lua @@ -115,6 +115,7 @@ function ns.CloneValue(value) return copy end +--- Safely calls a frame method for diagnostics and returns nil on error. function ns.GetFrameValue(frame, methodName) if not frame or type(frame[methodName]) ~= "function" then return nil diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index adc2f877..bccc734f 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -303,6 +303,7 @@ describe("ExtraIcons real source", function() GetGlobalConfig = function() return globalConfig end, + -- ECM.lua is not loaded in this isolated source spec. GetFrameValue = function(frame, methodName) if not frame or type(frame[methodName]) ~= "function" then return nil From 1b418e71907ea85611c5f3d7e17926f89352b6fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 08:50:58 +0000 Subject: [PATCH 8/9] Clarify ExtraIcons item stack helpers Agent-Logs-Url: https://github.com/argium/EnhancedCooldownManager/sessions/4c2a168f-7f02-404f-a7ac-065bd406849a Co-authored-by: argium <15852038+argium@users.noreply.github.com> --- Modules/ExtraIcons.lua | 11 +++++------ Tests/Modules/ExtraIcons_spec.lua | 5 +++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 1392da7b..444afe6b 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -212,14 +212,13 @@ end local function shouldShowItemStack(itemStack) if not itemStack then return false end - if itemStack.hideInRatedPvp and C_PvP.IsRatedMap() then return false end - + local hiddenInRatedPvp = itemStack.hideInRatedPvp and C_PvP.IsRatedMap() local inInstance, instanceType = IsInInstance() - if itemStack.hideInInstances and inInstance and instanceType ~= "pvp" and instanceType ~= "arena" then return false end - return true + local hiddenInInstance = itemStack.hideInInstances and inInstance and instanceType ~= "pvp" and instanceType ~= "arena" + return not hiddenInRatedPvp and not hiddenInInstance end -local function resolveConfiguredItemStack(entry, moduleConfig) +local function getConfiguredItemStackData(entry, moduleConfig) local itemStacks = moduleConfig and moduleConfig.itemStacks local itemStack = itemStacks and itemStacks.byId and itemStacks.byId[entry.itemStackId] return shouldShowItemStack(itemStack) @@ -239,7 +238,7 @@ local function resolveEntry(entry, moduleConfig) end if kind == "equipSlot" then return getEquipSlotIconData(slotId) end if kind == "item" then return ids and resolveFirstItem(ids) end - if kind == "itemStack" then return resolveConfiguredItemStack(entry, moduleConfig) end + if kind == "itemStack" then return getConfiguredItemStackData(entry, moduleConfig) end if kind == "spell" then return ids and resolveFirstKnownSpell(ids) end end diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index bccc734f..4841f9ae 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -723,8 +723,9 @@ describe("ExtraIcons real source", function() it("hooks viewers only once", function() ExtraIcons.InnerFrame = ExtraIcons:CreateFrame() - ExtraIcons:_hookViewer({ key = "utility", blizzKey = "UtilityCooldownViewer" }) - ExtraIcons:_hookViewer({ key = "utility", blizzKey = "UtilityCooldownViewer" }) + local utilityHookConfig = { key = "utility", blizzKey = "UtilityCooldownViewer" } + ExtraIcons:_hookViewer(utilityHookConfig) + ExtraIcons:_hookViewer(utilityHookConfig) assert.is_true(ExtraIcons._viewers.utility.hooked) assert.are.equal(1, UtilityCooldownViewer:GetHookCount("OnShow")) From 1452260d021ece1046e669d19ab98841ee96d89f Mon Sep 17 00:00:00 2001 From: Argi <15852038+argium@users.noreply.github.com> Date: Mon, 18 May 2026 20:58:34 +1000 Subject: [PATCH 9/9] Remove dead code. --- Modules/ExtraIcons.lua | 113 +------------ Tests/Modules/ExtraIcons_spec.lua | 267 ------------------------------ 2 files changed, 4 insertions(+), 376 deletions(-) diff --git a/Modules/ExtraIcons.lua b/Modules/ExtraIcons.lua index 444afe6b..fab24fc4 100644 --- a/Modules/ExtraIcons.lua +++ b/Modules/ExtraIcons.lua @@ -33,109 +33,6 @@ local function cachePoint(vs, blizzFrame) vs.originalPoint = { point, relativeTo, relativePoint, x or 0, y or 0 } end -local function getHorizontalBounds(point, width) - if point == "LEFT" or point == "TOPLEFT" or point == "BOTTOMLEFT" then - return 0, width - elseif point == "RIGHT" or point == "TOPRIGHT" or point == "BOTTOMRIGHT" then - return -width, 0 - end - local h = width / 2 - return -h, h -end - -local function getFrameDebugName(frame) - if not frame then return nil end - local name = frame.GetName and frame:GetName() - return name or tostring(frame) -end - -local function getPointDebugData(point) - if not point then return nil end - return { - point = point[1], - relativeTo = getFrameDebugName(point[2]), - relativePoint = point[3], - x = point[4], - y = point[5], - } -end - -local function logSharedOffsets(why, reason, mainFrame, utilFrame, mainPoint, utilPoint, offsets) - if not ns.IsDebugEnabled() then return end - ns.Log("ExtraIcons", "Shared offsets: " .. reason, { - reason = why, - main = { - shown = mainFrame and mainFrame:IsShown(), - width = mainFrame and mainFrame:GetWidth(), - point = getPointDebugData(mainPoint), - offset = offsets.main, - }, - utility = { - shown = utilFrame and utilFrame:IsShown(), - width = utilFrame and utilFrame:GetWidth(), - point = getPointDebugData(utilPoint), - offset = offsets.utility, - }, - }) -end - ---- Computes per-viewer offsets that re-center both viewers as a single stacked ---- group when they share the same original anchor. -local function getAdjustedOffsetsToRecenterViewer(viewers, why) - local offsets = { main = 0, utility = 0 } - local mainState, utilState = viewers.main, viewers.utility - if not mainState or not utilState then - logSharedOffsets(why, "missing viewer state", nil, nil, nil, nil, offsets) - return offsets - end - - local mainFrame = _G[MAIN_VIEWER_KEY] - local utilFrame = _G[UTILITY_VIEWER_KEY] - cachePoint(mainState, mainFrame) - cachePoint(utilState, utilFrame) - - local mp, up = mainState.originalPoint, utilState.originalPoint - if not mp or not up then - logSharedOffsets(why, "missing original point", mainFrame, utilFrame, mp, up, offsets) - return offsets - end - if mainFrame and up[2] == mainFrame then - logSharedOffsets(why, "utility anchored to main", mainFrame, utilFrame, mp, up, offsets) - return offsets - end - if up[1] ~= mp[1] or up[2] ~= mp[2] or up[3] ~= mp[3] or up[4] ~= mp[4] then - logSharedOffsets(why, "independent anchors", mainFrame, utilFrame, mp, up, offsets) - return offsets - end - - local sharedLeft, sharedRight - for _, v in ipairs(VIEWERS) do - local frame = _G[v.blizzKey] - local p = viewers[v.key].originalPoint - if frame and frame:IsShown() and p then - local l, r = getHorizontalBounds(p[1], frame:GetWidth() or 0) - sharedLeft = sharedLeft and math.min(sharedLeft, l) or l - sharedRight = sharedRight and math.max(sharedRight, r) or r - end - end - if not sharedLeft then - logSharedOffsets(why, "no shown viewers", mainFrame, utilFrame, mp, up, offsets) - return offsets - end - local center = (sharedLeft + sharedRight) / 2 - - for _, v in ipairs(VIEWERS) do - local frame = _G[v.blizzKey] - local p = viewers[v.key].originalPoint - if frame and frame:IsShown() and p then - local l, r = getHorizontalBounds(p[1], frame:GetWidth() or 0) - offsets[v.key] = center - ((l + r) / 2) - end - end - logSharedOffsets(why, "computed", mainFrame, utilFrame, mp, up, offsets) - return offsets -end - local function updateMainViewerAnchor(vs, blizzFrame, rightFrame) local anchor = vs and vs.anchorFrame if not anchor then return end @@ -485,12 +382,11 @@ function ExtraIcons:GetMainViewerAnchor() return _G[MAIN_VIEWER_KEY] end -function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, sharedOffsetX, moduleConfig, why) +function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, moduleConfig, why) local blizzFrame = _G[viewerConfig.blizzKey] local vs = self._viewers[viewerConfig.key] if not vs then return false end local container = vs.container - sharedOffsetX = sharedOffsetX or 0 cachePoint(vs, blizzFrame) local items = (not blizzFrame or not blizzFrame:IsShown() or isEditing or #entries == 0) @@ -499,7 +395,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared if #items == 0 then local p = vs.originalPoint if p and blizzFrame then - FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + sharedOffsetX, p[5] } }) + FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4], p[5] } }) end if isEditing then vs.originalPoint = nil end container:Hide() @@ -551,7 +447,7 @@ function ExtraIcons:_updateSingleViewer(viewerConfig, entries, isEditing, shared local extraOnScreen = (spacing + totalWidth) * viewerScale local p = vs.originalPoint if p and blizzFrame then - FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] + sharedOffsetX - extraOnScreen / 2, p[5] } }) + FrameUtil.LazySetAnchors(blizzFrame, { { p[1], p[2], p[3], p[4] - extraOnScreen / 2, p[5] } }) end local xOffset = 0 @@ -597,12 +493,11 @@ function ExtraIcons:UpdateLayout(why) -- When hidden, leave viewers nil so each call gets empty entries, which -- restores Blizzard viewer positions and hides extra-icon containers. local viewers = shouldShow and moduleConfig and moduleConfig.viewers - local offsets = getAdjustedOffsetsToRecenterViewer(self._viewers, why) local anyPlaced = false for _, v in ipairs(VIEWERS) do local entries = viewers and viewers[v.key] or {} - if self:_updateSingleViewer(v, entries, isEditing, offsets[v.key], moduleConfig, why) then + if self:_updateSingleViewer(v, entries, isEditing, moduleConfig, why) then anyPlaced = true end end diff --git a/Tests/Modules/ExtraIcons_spec.lua b/Tests/Modules/ExtraIcons_spec.lua index 4841f9ae..1583bdfe 100644 --- a/Tests/Modules/ExtraIcons_spec.lua +++ b/Tests/Modules/ExtraIcons_spec.lua @@ -539,124 +539,6 @@ describe("ExtraIcons real source", function() } end - local function makeActiveFrames(count, width) - local frames = {} - for i = 1, count do - local frame = TestHelpers.makeFrame({ shown = true, width = width, height = width }) - frame.isActive = true - frames[i] = frame - end - return frames - end - - local function getPointAnchorOffset(point, width) - if point == "LEFT" or point == "TOPLEFT" or point == "BOTTOMLEFT" then - return 0 - elseif point == "RIGHT" or point == "TOPRIGHT" or point == "BOTTOMRIGHT" then - return width - elseif point == "CENTER" or point == "TOP" or point == "BOTTOM" then - return width / 2 - end - - error("Unsupported point " .. tostring(point)) - end - - local function getFrameLeft(frame) - local point, relativeTo, relativePoint, x = frame:GetPoint(1) - local width = frame:GetWidth() or 0 - local anchorX = x or 0 - - if relativeTo and relativeTo ~= UIParent then - local relativeLeft = getFrameLeft(relativeTo) - local relativeWidth = relativeTo:GetWidth() or 0 - anchorX = relativeLeft + getPointAnchorOffset(relativePoint, relativeWidth) + (x or 0) - end - - return anchorX - getPointAnchorOffset(point, width) - end - - local function getViewerRowCenter(viewerFrame, container) - local rowWidth = viewerFrame:GetWidth() or 0 - if container and container:IsShown() then - rowWidth = rowWidth + (viewerFrame.childXPadding or 0) + ((container:GetWidth() or 0) * (container.__scale or 1)) - end - return getFrameLeft(viewerFrame) + (rowWidth / 2) - end - - local function resolveAnchorTarget(anchorTarget) - if anchorTarget == "UIParent" then - return UIParent - elseif anchorTarget == "main" then - return EssentialCooldownViewer - elseif anchorTarget == "utility" then - return UtilityCooldownViewer - end - return nil - end - - local function assertViewerRowsStayCenteredAfterMove(args) - UtilityCooldownViewer.childXPadding = 4 - UtilityCooldownViewer.iconScale = 1.0 - UtilityCooldownViewer:SetWidth(args.utilityViewerWidth) - UtilityCooldownViewer.GetItemFrames = function() - return makeActiveFrames(args.utilityActiveCount, 22) - end - UtilityCooldownViewer:SetPoint( - args.utilityAnchor.point, - resolveAnchorTarget(args.utilityAnchor.relativeTo), - args.utilityAnchor.relativePoint, - args.utilityAnchor.x, - args.utilityAnchor.y - ) - - EssentialCooldownViewer.childXPadding = 4 - EssentialCooldownViewer.iconScale = 1.0 - EssentialCooldownViewer:SetWidth(args.mainViewerWidth) - EssentialCooldownViewer.GetItemFrames = function() - return makeActiveFrames(args.mainActiveCount, 22) - end - EssentialCooldownViewer:SetPoint( - args.mainAnchor.point, - resolveAnchorTarget(args.mainAnchor.relativeTo), - args.mainAnchor.relativePoint, - args.mainAnchor.x, - args.mainAnchor.y - ) - - inventoryItemBySlot[13] = 101 - inventoryTextureBySlot[13] = "trinket-1" - inventorySpellByItem[101] = 9001 - inventoryItemBySlot[14] = 102 - inventoryTextureBySlot[14] = "trinket-2" - inventorySpellByItem[102] = 9002 - itemCounts[HEALTHSTONE_ID] = 1 - itemIconsByID[HEALTHSTONE_ID] = "healthstone" - - ExtraIcons.InnerFrame = ExtraIcons:CreateFrame() - - local config = makeViewersConfig(args.beforeUtility, args.beforeMain) - ExtraIcons.GetModuleConfig = function() - return config - end - - assert.is_true(ExtraIcons:UpdateLayout("before-move")) - - config.viewers.utility = args.afterUtility - config.viewers.main = args.afterMain - - assert.is_true(ExtraIcons:UpdateLayout("after-move")) - - local utilityCenter = getViewerRowCenter(UtilityCooldownViewer, ExtraIcons._viewers.utility.container) - local mainCenter = getViewerRowCenter(EssentialCooldownViewer, ExtraIcons._viewers.main.container) - - assert.are.equal(mainCenter, utilityCenter) - - if args.expectUtilityRelativeTo then - local _, relativeTo = UtilityCooldownViewer:GetPoint(1) - assert.are.equal(resolveAnchorTarget(args.expectUtilityRelativeTo), relativeTo) - end - end - it("requires at least one viewer to be visible in ShouldShow", function() assert.is_true(ExtraIcons:ShouldShow()) @@ -1406,155 +1288,6 @@ describe("ExtraIcons real source", function() assert.is_true(ExtraIcons._viewers.main.container:IsShown()) end) - for _, case in ipairs({ - { - name = "keeps same-parent viewer rows centered when utility becomes empty", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 0 }, - mainAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = { - { stackKey = "trinket1" }, - }, - afterUtility = {}, - afterMain = { - { stackKey = "trinket1" }, - { kind = "itemStack", itemStackId = "healthstones" }, - }, - }, - { - name = "keeps same-parent viewer rows centered when main becomes empty", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 0 }, - mainAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = { - { stackKey = "trinket1" }, - }, - afterUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - { stackKey = "trinket1" }, - }, - afterMain = {}, - }, - { - name = "keeps same-parent viewer rows centered when both viewers still have different ECM counts", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 0 }, - mainAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - { stackKey = "trinket1" }, - }, - beforeMain = { - { stackKey = "trinket2" }, - }, - afterUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - afterMain = { - { stackKey = "trinket2" }, - { stackKey = "trinket1" }, - }, - }, - { - name = "keeps same-parent viewer rows centered for center anchors when utility becomes empty", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "CENTER", relativeTo = "UIParent", relativePoint = "CENTER", x = 100, y = 0 }, - mainAnchor = { point = "CENTER", relativeTo = "UIParent", relativePoint = "CENTER", x = 100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = { - { stackKey = "trinket1" }, - }, - afterUtility = {}, - afterMain = { - { stackKey = "trinket1" }, - { kind = "itemStack", itemStackId = "healthstones" }, - }, - }, - { - name = "keeps same-parent viewer rows centered for top-left anchors when utility becomes empty", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "TOPLEFT", relativeTo = "UIParent", relativePoint = "TOPLEFT", x = 100, y = -100 }, - mainAnchor = { point = "TOPLEFT", relativeTo = "UIParent", relativePoint = "TOPLEFT", x = 100, y = -60 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = { - { stackKey = "trinket1" }, - }, - afterUtility = {}, - afterMain = { - { stackKey = "trinket1" }, - { kind = "itemStack", itemStackId = "healthstones" }, - }, - }, - { - name = "keeps same-parent viewer rows centered for right anchors when utility becomes empty", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "RIGHT", relativeTo = "UIParent", relativePoint = "RIGHT", x = -100, y = 0 }, - mainAnchor = { point = "RIGHT", relativeTo = "UIParent", relativePoint = "RIGHT", x = -100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = { - { stackKey = "trinket1" }, - }, - afterUtility = {}, - afterMain = { - { stackKey = "trinket1" }, - { kind = "itemStack", itemStackId = "healthstones" }, - }, - }, - { - name = "keeps a utility viewer anchored to main centered without same-parent coupling", - utilityViewerWidth = 22, - utilityActiveCount = 1, - mainViewerWidth = 48, - mainActiveCount = 2, - utilityAnchor = { point = "LEFT", relativeTo = "main", relativePoint = "LEFT", x = 26, y = -40 }, - mainAnchor = { point = "LEFT", relativeTo = "UIParent", relativePoint = "LEFT", x = 100, y = 40 }, - beforeUtility = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - beforeMain = {}, - afterUtility = {}, - afterMain = { - { kind = "itemStack", itemStackId = "healthstones" }, - }, - expectUtilityRelativeTo = "main", - }, - }) do - local currentCase = case - it(currentCase.name, function() - assertViewerRowsStayCenteredAfterMove(currentCase) - end) - end - it("prefers demonic healthstone over the legacy healthstone", function() local utilityIconChild = TestHelpers.makeFrame({ shown = true, width = 18, height = 18 }) utilityIconChild.GetSpellID = function() return 1 end