Talent-aware dispels: "Only Dispellable by You" follows what you can actually cleanse - #254
Open
Krathe82 wants to merge 8 commits into
Open
Talent-aware dispels: "Only Dispellable by You" follows what you can actually cleanse#254Krathe82 wants to merge 8 commits into
Krathe82 wants to merge 8 commits into
Conversation
Field report (tRp, resto shaman DWARF, v5.3.1): "bleeds are now shown with dispel
overlay on other players (which are not dwarf) when being a dwarf".
The bleed repair is scoped to the player's own frame, and that scoping is correct
-- but it could never take effect on a frame that changed hands. For a dwarf
shaman the gap slot exists in BOTH plans, own frame and everyone else's, with the
same key and the same filter string ("HARMFUL|!RAID_PLAYER_DISPELLABLE"). The only
difference is includeDispelTypes: {Poison,Bleed} on self, {Poison} elsewhere. That
map was in NEITHER signature, so the struct sig matched (no rebuild) and the
tuning sig matched (no re-tune), and the container kept whatever map it was first
built with. A frame built while driving the player carried Bleed from then on --
and header children get reassigned to other units by roster churn, which is how a
self-only repair ended up lighting bleeds across the raid.
The dfDispelSelf latch shipped with that repair was necessary but not sufficient:
it forces a re-plan on the self/non-self edge, and the re-plan then produced
identical signatures, so nothing was applied. Recomputing is not applying.
Serialise the slot's includeDispelTypes into the TUNING half, sorted so the same
set always serialises identically -- an unordered pairs() walk would move the
signature at random and re-tune on every drive. Tuning rather than structural
because SetAuraSlotCandidateFilters is a live mutator, so this re-pushes in place
with no teardown and no stranded buttons.
The row had no line at all for which signature moved, which is the first thing anyone asks when a setting appears not to apply. It is also the proof for the dispel-type map fix: on a frame whose self-ness flips, the includeDispelTypes map differs while the key set and the filter strings do not, so before that fix neither signature moved and this branch was never reached.
The probe asked the wrong question. Blizzard documents the two calls as answering
different things:
IsSpellInSpellBook -- "Returns true if a spell should be found in the spellbook.
This function can also return true for spells that aren't known, such as
override spells granted by an aura linked to class talents."
IsSpellKnown -- "Returns true if a player knows a spell."
Poison Cleansing Totem is exactly a talent-linked entry, so the spellbook probe
answered true for every shaman and the poison gap slot was added whether or not
the totem was talented. Field-reported: "it's still showing when they don't have
the totem talent. It's a half fix."
C_SpellBook.IsSpellKnown is now the primary probe. ⚠ It is NOT nonexistent -- it
is in SpellBookDocumentation.lua beside IsSpellInSpellBook, and an earlier review
removed it as a bogus fallback on the strength of "as far as I know". Restored as
the call that should have been there first, with IsSpellInSpellBook kept below it
as a last resort that over-reports rather than leaving the feature dead.
Also: the talent handler now bumps the aura layout version itself. The drives are
version-gated, so a talent change could only re-plan because ForceRefreshAllFrames
ends in InvalidateAuraLayout -- the dispel overlay depending on the Aura Designer's
refresh to notice a talent, which breaks silently the moment that call is guarded
on AD being enabled.
Plus a debug line on the overlay's TUNE decision: the row had none, and "which
signature moved" is the first question when a setting appears not to apply.
Re-talenting Poison Cleansing Totem never brought the poison gap back, while untalenting removed it — one-way, the same shape as every other missing trigger this week. PLAYER_TALENT_UPDATE is not the signal. A talent edit does not necessarily raise it, and even when something fires, the spell is not KNOWN yet at probe time: the spellbook grant arrives afterwards with SPELLS_CHANGED. Removal appeared to work only because the spell goes away promptly. Watch TRAIT_CONFIG_UPDATED and SPELLS_CHANGED, for the two different lags, and bump the layout version only when the capability actually flips — SPELLS_CHANGED fires constantly. The flip cache exists solely for that gate; the probe itself stays uncached so every real read is live. Shamans only. Also logs the overlay's structural BUILD/REBUILD decision. Adding or removing the gap slot changes the slot key set, so a talent change is structural and never reached the tune branch's line — which is why the log was silent through exactly the transition being tested.
It is not 'is this the player's frame' -- it is 'does the racial gap apply here', which is that AND being a dwarf. Printed as 'self' it produced the line 'unit=player self=false', which reads as the unit test being broken rather than a non-dwarf short-circuiting before it runs. A diagnostic that reads as a fault is worse than no diagnostic.
The engine's dispel flag is class/spec shaped and talents are not part of it, in
BOTH directions: it misses what a talent GRANTS (a shaman's Poison Cleansing
Totem) and asserts what a talent GATES (a priest's disease cure, which needs
Improved Purify). Both field-reported.
DISPEL_SPELLS is a per-class curated list of dispel spells and the types each
grants, cross-checked against the addons that already do this on retail rather
than authored from memory -- that check found four errors in the from-memory pass
(Monk's talent types exactly backwards, Mage and Paladin each missing a spell,
Hunter absent entirely, which would have treated every hunter as dispelling
nothing) and the field validation then caught two classic-era IDs that do not
resolve on retail and were dropped. All eight dispel-capable classes are
validated against live characters, including Warlock's pet path -- the one entry
probed through the PET spellbook bank, re-checked on UNIT_PET because summoning
or swapping a pet changes what the player can cleanse with no spellbook event.
Two probes, split by what each answers:
talent entries -> C_SpellBook.IsSpellKnown. IsSpellInSpellBook returns true for
UNKNOWN talent entries by Blizzard's own documentation -- the original
over-report. ⚠ The flag means "the spellbook probe LIES about this spell"
(override-linked grants), NOT "this spell is talent-gated": Remove Curse and
Detox are talent-gated and track correctly as base entries, verified both
ways in game.
base entries -> IsSpellInSpellBook with includeOverrides, because a base spell
replaced by an upgrade is still usable and only that call follows the
override chain.
The correction SUBTRACTS from the engine's answer rather than replacing it --
fail-open by design: a wrong ID or an uncurated class can only over-show (what
the overlay does today), never hide a dispel the player has. If nothing in a
class list probes known, nothing is subtracted at all: negatives from a
non-functioning list are worthless. Applied to the dispel overlay and the debuff
row's by-me mode together, so the two displays cannot disagree about the same
debuff. ALL mode is untouched: it rides DISPELLABLE, which is
capability-independent by definition.
/df debug dispelcap prints each probe, its conclusion, the resolved spell name
(the check that caught both bad IDs), and -- the half that can regress someone --
exactly what would be subtracted for this character.
Field-caught (priest, full raid, 2026-08-27): with "Dispellable By Me" on, the POISON overlay lit raid-wide for a priest who cannot dispel poison. The DISPEL debug log ruled out everything downstream -- racialSelf=false on every build (no dwarf gap slot), e=none (empty exclude, correct for a priest knowing both Purify halves). The filter token itself was wrong. Blizzard's token table (Blizzard_FrameXMLUtil/AuraUtil.lua) is explicit and the names point the wrong way: RAID = harmful auras THE PLAYER can dispel RAID_PLAYER_DISPELLABLE = auras SOMEONE IN THE PLAYER'S RAID can dispel By-me shipped on the raid-scoped token, justified by an "observed player-scoped in practice" note -- an observation made solo and in tiny test groups, the one setting where the two sets are identical. In a real raid with poison-dispellers present, the token behaved exactly as documented. Race, range, and the shaman gap work were all coincidental. Changed in lockstep, per the sibling rule this system already carries: - overlay by-me main slot: HARMFUL|RAID - overlay gap slot: HARMFUL|!RAID -- the old raid-scoped negation had its own hole: any raid-mate able to dispel a gap type (an Evoker covers bleeds) made the aura pass the raid-scoped token, fail the negation, and silently darken the dwarf's self-bleed marker whenever the roster composition changed. - debuff row by-me record + its gap-repair sibling: same token swap. The by-me record and the "raid" category record are now the same set by construction, which is not new overlap -- player-dispellable was already a subset of raid-dispellable, so the raid record was already emptied by its dispel negation whenever both were enabled. neg() just no longer emits the identical component twice. Filter strings live in the TUNING half of the split signatures on both surfaces, so existing sessions re-tune in place on reload -- no stranded slots. ⚠ Unverified in the field yet, and the test HAS to be a mixed raid: solo, the wrong token is indistinguishable from the right one -- which is how it shipped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Against stable (the live line, per v5.3.1 shipping from it).
The problem, field-reported twice in one day
The engine's dispel flag (
RAID_PLAYER_DISPELLABLE) is class/spec shaped — talents are not part of it, in both directions:IsSpellInSpellBook) answers "should this appear in the spellbook", which per Blizzard's own documentation returns true for talent entries the player has not taken — so every shaman saw the poison overlay, talented or not.What this does
DISPEL_SPELLS— a per-class curated table of dispel spells and the types each grants, all eight dispel-capable classes. Cross-checked against the addons that already solve this on retail rather than authored from memory (the check found four errors in the from-memory pass, including Hunter missing entirely), then validated in game class by class — which caught two classic-era IDs that do not resolve on retail; both dropped.Two probes, split by what each answers. Talent entries use
C_SpellBook.IsSpellKnown("Returns true if a player knows a spell"). Base entries useIsSpellInSpellBookwithincludeOverrides, because a base spell replaced by an upgrade is still usable and only that call follows the override chain. ⚠ Thetalentflag means "the spellbook probe lies about this spell" (override-linked grants), not "this spell is talent-gated" — Remove Curse and Detox are talent-gated and track correctly as base entries, verified both ways in game.Subtract from the engine, never replace it — fail-open by design. A wrong ID or an uncurated class can only over-show (what the overlay already does today), never hide a dispel the player has. If nothing in a class list probes known, nothing is subtracted at all. Applied to the overlay and the debuff row's by-me mode together so the two displays cannot disagree. ALL mode untouched —
DISPELLABLEis capability-independent by definition.Capability changes apply live. A talent edit fires no spec event and the spellbook grant lands late (
SPELLS_CHANGED), so a watcher onTRAIT_CONFIG_UPDATED+SPELLS_CHANGED(+UNIT_PETfor the Warlock, whose only dispel belongs to the Imp) re-plans when the capability set actually flips. The dispel-type maps join the overlay's tuning signature — they were in neither signature, so a plan that differed only by map was computed and silently never applied; that staleness is also what let a Dwarf's bleed repair leak onto other players' frames as containers changed hands (field-reported on 5.3.1, first commit here)./df debug dispelcapprints each probe, its conclusion, the resolved spell name (the check that caught both bad IDs), and — the half that can regress someone — exactly what would be subtracted for this character.Added: the by-me base token itself was wrong (
29b1c8a8)Field-caught after this PR went up: a Priest with "Only Dispellable by You" saw the poison overlay lit raid-wide. Root cause is one level below everything above — the mode's base filter rode
RAID_PLAYER_DISPELLABLE. The token names point the wrong way and Blizzard's own token table (Blizzard_FrameXMLUtil/AuraUtil.lua) is explicit:RAID= "harmful auras the player can dispel" (player-scoped — what by-me means)RAID_PLAYER_DISPELLABLE= "auras someone in the player's raid can dispel" (raid-scoped)Solo and in small same-class groups the two sets are identical, which is how the wrong one survived testing. The fix moves by-me (overlay + debuff row) to
HARMFUL|RAIDat all four sites; the gap slot's negation follows to!RAID, which also closes a second hole — the old raid-scoped negation went dark for a Dwarf's self-bleed the moment any raid-mate could dispel that type. ⚠ Not yet confirmed in game: validation requires a mixed raid (solo, the two tokens are indistinguishable by construction).Confirmed in game (Krathe)
Shaman poison following the totem both directions without a reload; Priest disease following Improved Purify both directions; the Dwarf bleed correctly self-only; all eight classes' dump readings, including the pet-bank path with and without an Imp.
Verification
luac -pclean on every changed file, CRLF intact,_ENVglobals diffed.Features/Dispel.luaandFeatures/Auras.luadiffer from the tested tree only by locally-held debug tooling, deliberately excluded. Changelog entries included for the two confirmed user-visible fixes.