Custom spell and item icons and external defensive bars

[argium]

Add Icons for Any Spell or Item

Custom spell and item icons can now be added to the cooldown manager. Simply find the item or spell ID and add it in settings.

External Defensives

External defensives cast on you can now be displayed as a bar. #84

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[argium]

@codex review

[github-actions]

Test Results

897 tests  +117   897 ✅ +117   1s ⏱️ -1s
  5 suites ±  0     0 💤 ±  0 
  5 files   ±  0     0 ❌ ±  0 

Results for commit 966150f. ± Comparison against base commit 8a3a307.

This pull request removes 540 and adds 657 tests. Note that renamed tests count towards both.

Libs/LibEvent/Tests.LibEvent_spec_lua:354 ‑ LibEvent initializes _stats as an empty table
Libs/LibEvent/Tests.LibEvent_spec_lua:360 ‑ LibEvent increments _stats on each event fire
Libs/LibEvent/Tests.LibEvent_spec_lua:372 ‑ LibEvent tracks stats independently per event
Libs/LibEvent/Tests.LibEvent_spec_lua:389 ‑ LibEvent tracks stats independently per target
Libs/LibEvent/Tests.LibEvent_spec_lua:405 ‑ LibEvent GetEventStats returns the target's _stats table
Libs/LibEvent/Tests.LibEvent_spec_lua:417 ‑ LibEvent ResetEventStats clears all counters for the target
Libs/LibEvent/Tests.LibEvent_spec_lua:434 ‑ LibEvent ResetEventStats does not affect other targets
Libs/LibEvent/Tests.LibEvent_spec_lua:451 ‑ LibEvent does not increment stats when no callbacks are registered for the event
Libs/LibSettingsBuilder/Tests.LibSettingsBuilder_spec_lua:1007 ‑ LibSettingsBuilder Custom respects varType override
Libs/LibSettingsBuilder/Tests.LibSettingsBuilder_spec_lua:1028 ‑ LibSettingsBuilder propagateModifiers propagates layout=false to composite children
…

Libs/LibEvent/Tests.LibEvent_spec_lua:354 ‑ LibEvent does not initialize _stats when metrics debug is disabled
Libs/LibEvent/Tests.LibEvent_spec_lua:360 ‑ LibEvent does not increment _stats when metrics debug is disabled
Libs/LibEvent/Tests.LibEvent_spec_lua:373 ‑ LibEvent GetEventStats returns an empty table when metrics debug is disabled
Libs/LibEvent/Tests.LibEvent_spec_lua:385 ‑ LibEvent ResetEventStats is a no-op when metrics debug is disabled
Libs/LibEvent/Tests.LibEvent_spec_lua:402 ‑ LibEvent does not increment stats when no callbacks are registered for the event
Libs/LibSettingsBuilder/Tests.Builder_spec_lua:119 ‑ LibSettingsBuilder Builder uses the section category for an unnamed page in a multi-page section
Libs/LibSettingsBuilder/Tests.Builder_spec_lua:152 ‑ LibSettingsBuilder Builder registers root-bound composite rows from an empty path
Libs/LibSettingsBuilder/Tests.Builder_spec_lua:190 ‑ LibSettingsBuilder Builder rejects deprecated desc fields at registration time
Libs/LibSettingsBuilder/Tests.Builder_spec_lua:214 ‑ LibSettingsBuilder Builder rejects removed condition fields at registration time
Libs/LibSettingsBuilder/Tests.Builder_spec_lua:245 ‑ LibSettingsBuilder Builder keeps page handles limited to the v2 public surface
…

♻️ This comment has been updated with latest results.

[argium]

@copilot review this pr

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 457afe9daa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

@copilot review this pr

Reviewed the two new modules (Modules/ExtraIcons.lua, Modules/ExternalBars.lua) and their UI counterparts. Sticking to concrete defects only.

1. Deferred OnEnable callback in ExtraIcons is missing an IsEnabled() guard — Modules/ExtraIcons.lua:703-707

C_Timer.After(0.1, function()
    self:HookEditMode()
    for _, v in ipairs(VIEWERS) do self:_hookViewer(v.key) end
    ns.Runtime.RequestLayout("ExtraIcons:OnEnable")
end)

If the module is disabled within 100 ms of being enabled, this still installs hooks on Blizzard frames and requests a layout against a disabled module. ExternalBars:OnEnable (line 977) does the right thing with an if not self:IsEnabled() then return end — ExtraIcons should mirror that.

2. vs.hooked is sticky across disable/enable — Modules/ExtraIcons.lua:652-678
_hookViewer early-returns when vs.hooked is true, and OnDisable (715-719) doesn't reset that flag or _editModeHooked. Because HookScript cannot be undone, the original closures remain attached after disable; on re-enable they keep firing RequestLayout for a module whose IsEnabled() is true again, but the _isEditModeActive flag is also reset to nil in OnDisable so the very first OnShow/OnHide after re-enable runs against stale state. Either keep the hook flags across the addon lifetime (and never null _isEditModeActive in OnDisable), or guard the closures themselves with if not self:IsEnabled() then return end.

3. Dead _warned field in ExternalBars — Modules/ExternalBars.lua:912, 968

$ rg "_warned" Modules/ExternalBars.lua
912:        self._warned = false
968:    self._warned = false

Set in two places, never read. Per AGENTS.md "Remove dead code, stale fields…" — drop both assignments (and the surrounding if not self._editLocked then on 911 collapses).

4. setDurationText does formatted output on a secret value — Modules/ExternalBars.lua:200-209

local function setDurationText(durationText, remaining)
    if issecretvalue(remaining) then
        durationText:SetFormattedText("%.0f", remaining)
        return
    end
    durationText:SetText(formatDurationText(remaining))
end

Per AGENTS.md secret-value rules, remaining may not be used in arithmetic/comparison/formatting outside of built-ins/APIs that explicitly accept secrets. FontString:SetFormattedText is not in the explicit allow-list and %.0f triggers a numeric format conversion. Safer to either SetText("") / hide the duration FontString in the secret branch, or pass the raw secret through SetText(tostring(...)) only if you have verified SetFormattedText accepts secrets — the conservative fix is to suppress the text when the value is secret.

5. _GetStyleConfig allocates and re-copies on every layout — Modules/ExternalBars.lua:251-259
The wipe + for k,v in pairs(moduleConfig) runs on every UpdateLayout. Style config only changes on settings updates, so this is unnecessary churn on a relatively hot path. Cache once and invalidate on the settings-change event.

6. Nit / single-caller wrapper — UI/ExtraIconsOptions.lua:488

function ExtraIconsOptions._otherViewer(viewerKey)
    return viewerKey == "utility" and "main" or "utility"
end

Called from exactly one site (line 736). AGENTS.md explicitly disallows single-caller wrappers — inline it.

Verified false positives I want to call out so you don't chase them:

• C_SpellBook.IsSpellKnown (Modules/ExtraIcons.lua:140) is not deprecated. The deprecation list (docs/BlizzardDeprecatedApis.md:41) names the bare global IsSpellKnown (from Blizzard_DeprecatedSpellBook), not the C_SpellBook.* namespaced replacement.
• layoutMovedButton mutation in UI/ExternalBarsOptions.lua:15-16 is fine — OptionUtil.CreateLayoutBreadcrumbArgs returns a fresh table per call (UI/OptionUtil.lua:137-148), so the file owns its copy.
• _editLocked = false before the pcall (Modules/ExternalBars.lua:879) is intentional — it's the default and the pcall flips it to true on demand. Not a bug.

Tests, lint, and overall structure look healthy; the items above are the only ones I'd block on.