Skip to content

Click-cast macros use base spell name, breaking spec-replacement overrides (Nature's Cure, Cleanse Spirit, etc.) #63

@doug-w

Description

@doug-w

Version

v4.3.6

Summary

Click-cast bindings to spec-replacement spells silently do nothing — or cast the wrong spell — because BuildMacroTextForBinding always writes the base spell's name into the /cast macro and relies on /cast being override-aware. That's true for proc-style overrides
(Flash of Light → Benediction) but not for spec-replacement overrides where the base and override have different names and different effects (Remove Corruption only dispels Curse + Poison; Nature's Cure dispels Magic + Curse + Poison).

The addon already understands this distinction in two of three places that touch the override chain — the macro builder is the one that doesn't.

Repro

  1. Resto Druid.
  2. In the click-cast editor, bind Alt + Right Click on party frames to Nature's Cure.
  3. The stored binding ends up as spellId = 440015, spellName = "Remove Corruption"GetAvailableSpells normalizes selections to the root via GetBaseSpell, and MigrateBindingsToRootSpells does the same on existing bindings.
  4. In a party with a Magic debuff on a member, hover the unit frame and Alt + Right Click. Nothing happens — Magic is not dispelled.
  5. Casting Nature's Cure on a targeted unit via the regular keybind / action bar works, confirming the spell itself is fine.

Where the bug lives

ClickCasting/Bindings.lua is internally inconsistent about the override chain:

Function Lines What it does Used for
IsSpellKnownByName 38–76 Calls GetOverrideSpell(spellId) and checks if the override is in the spellbook Bind-time "is this spell known?" check
GetSpellDisplayInfo 81–117 Calls GetOverrideSpell(baseSpellId) for display name/icon/id UI display
BuildMacroTextForBinding 1934–2015 No override resolution — uses base spellName directly The actual /cast macro

The comment at lines 43–45 explicitly cites this exact case ("Remove Corruption -> Nature's Cure"). But only the known-check honors it — the macro builder ignores it.

Why the existing comment in the macro builder isn't wrong, just incomplete

The justification at lines 1940–1944 cites Flash of Light → Benediction. That's a proc override: the base spell stays permanently in the spellbook, the override is a transient same-cost replacement, and /cast Flash of Light resolves correctly in either state because Flash
of Light is in the spellbook regardless of proc. Switching the macro to /cast Benediction would break when the proc expires — so the warning is accurate for that case.

A spec-replacement override is structurally different: the base spell isn't directly castable in the spec that has it replaced. For Resto Druid, spell 440015 ("Remove Corruption") isn't in the spellbook with includeOverrides=false — only Nature's Cure (88423) is. /cast Remove Corruption therefore doesn't resolve to anything castable, and the click does nothing. The comment's reasoning silently assumes case 1 applies to everything.

Suggested fix

In BuildMacroTextForBinding, before localizing the spell name, check whether the base spell is directly in the player's spellbook (IsSpellInSpellBook(includeOverrides=false)). If yes, it's a proc-style override → keep the base name (preserves the existing proc-safe
behavior). If no, it's a spec replacement → swap to GetOverrideSpell's result.

Spec-aware: ApplyBindings is already called on ACTIVE_PLAYER_SPECIALIZATION_CHANGED and TRAIT_CONFIG_UPDATED (Events.lua:21–53), so the macro is regenerated whenever the resolution would change. A binding created in Feral as Remove Corruption automatically becomes /cast Nature's Cure on switching to Resto, and back to /cast Remove Corruption on switching away — without re-binding.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions