fix(cdm): let a tracked bar find its debuff on the target - #1997
Open
dfrisone wants to merge 1 commit into
Open
Conversation
Reported for a rogue Blind macro that sets focus to the target, clears the
target, targets and blinds someone else, then restores the original target
from focus, all inside one macro. Afterwards the player's bleeds on that
target stop showing on the tracked bar until they switch target and back.
The stall is Blizzard's. CooldownViewerMixin:OnPlayerTargetChanged only
refreshes when UnitGUID("target") differs from the one it stored, and this
macro ends on the GUID it started with, so the refresh never runs and the
item stays inactive. Its frame-scoped aura cache is keyed on unit token and
stamped with GetTime(), and nothing on the target-change path marks it dirty,
so a second target change inside one frame reads the stale list.
Our display is a faithful mirror of that, so it goes quiet with it. But the
bind-miss fallback that already exists for "the viewer has not bound this aura
yet" only ever asked GetPlayerAuraBySpellID, which cannot see a debuff on
somebody else. It now asks the target too, so the bar rides out the stall.
Only a readable sourceUnit mismatch rejects the aura, so another player's copy
of the same debuff cannot drive the bar while an unreadable one still shows.
Field reads need no new guarding: the consumer already classifies duration,
expirationTime and applications before comparing them.
PLAYER_TARGET_CHANGED joins both tick wake sets as well. Target-applied auras
bind and release on that edge and on no player-scoped one, so a parked ticker
had no way to learn a new target already carried a tracked debuff.
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
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.
Reported by @guld (Cooldown Manager, 9.1.6, so this is current). A common
M+ rogue macro:
After pressing it, the player's bleeds on the target stop showing on the
tracked bar until they manually switch target and back.
The stall is Blizzard's
CooldownViewerMixin:OnPlayerTargetChangedonly refreshes when the target GUIDdiffers from the one it stored:
The macro nets
GUID(A) -> nil -> GUID(B) -> GUID(A). Any target-changed eventdispatched after the macro body reads A, matches the stored A, and returns
without calling
RefreshActiveFramesForTargetChange, which is the only thingthat re-drives the items. Switching away and back manually produces two
GUID-different edges, which is exactly why that workaround works.
Its
GetUnitAurasCachedis also keyed on unit token and stamped withGetTime(), and the only thing that marks it dirty isOnUnitAura, never atarget change -- so two target changes inside one frame leave the later read
returning the stale list.
Why ours goes quiet with it, and what changes
EUI's tracked bars are a stateless per-tick mirror of
blzChild:IsActive(),with no unit, GUID or latch of our own, so when Blizzard's item goes inactive
we follow it down. (The viewer is parked offscreen rather than hidden, which is
correct and stays that way -- hiding it would unregister its own
UNIT_AURAand
PLAYER_TARGET_CHANGED.)There is already a bind-miss fallback for "the viewer has not bound this aura
yet", but it only asked
GetPlayerAuraBySpellID, which by definition cannotsee a debuff on somebody else. It now asks the target as well, so the bar rides
out the stall instead of waiting for a real target switch.
Two details worth review:
sourceUnitmismatch rejects the aura. That field readssecret on an enemy in restricted content, so an unreadable one is left to
show rather than blanking the bar; a readable one belonging to another player
is dropped so their copy of the same debuff cannot drive it.
duration,expirationTimeandapplicationswithissecretvaluebeforecomparing them.
PLAYER_TARGET_CHANGEDalso joins both tick wake sets. Target-applied aurasbind and release on that edge and on no player-scoped one, so a parked ticker
previously had no way to learn that a newly selected target already carried a
tracked debuff.
Testing
luac -pclean on both files; house style check clean. Not click-tested. Thereproduction is the macro above with a bleed running; the check is that the bar
survives it without a manual target switch. Worth confirming separately that
another player's copy of the same debuff does not light the bar.