Fix(Aura Buff Reminders): raid buff reminder stuck on in Mythic+ - #1965
Open
dfrisone wants to merge 3 commits into
Open
Fix(Aura Buff Reminders): raid buff reminder stuck on in Mythic+#1965dfrisone wants to merge 3 commits into
dfrisone wants to merge 3 commits into
Conversation
GetUnitAuraBySpellID is documented SecretWhenUnitAuraRestricted, so inside a keystone or a raid every other group member's aura comes back secret. The player path had no secrecy guard and so always read as buffed, while the non-player path treated a secret as "no buff". The coverage count therefore scored every other beneficiary as missing for the whole run, and the raid buff reminder sat on screen permanently with a badge that blamed people who were buffed (and, for the stat-filtered buffs, people who were never in the denominator to begin with). _unitHasBuff now reports readability alongside the answer, and CountGroupBuffCoverage drops unreadable members from both sides of the ratio. Under restriction the count falls back to the player alone, matching how the rest of the module suppresses rather than false-fires. Reported by Kira.
The class table lists Paladin, Monk, Druid, Shaman and Demon Hunter under both Intellect and Attack Power, so without spec comms it counted them for both and the denominator over-reported. Role settled some of that, but three damager pairs share the DAMAGER role and could not be settled at all: Balance vs Feral, Elemental vs Enhancement, Havoc vs Devourer. Restructure the ladder as spec, then class, then role for hybrids only, and have UnitBenefits report whether it actually decided. An unreadable class, an unreadable role, or a hybrid whose role spans both stats now leaves the denominator instead of being guessed as a beneficiary, matching how unreadable aura state is handled. Guessing "yes" is what leaves a reminder on screen for someone who never wanted the buff. Also settles Vengeance, which role could always have answered: a Demon Hunter tank is no longer counted as an Intellect beneficiary. Pure classes are unaffected and were always exact: a Mage is never in the Battle Shout count, a Warrior never in the Arcane Intellect one.
Verified in game inside a keystone: GetPlayerAuraBySpellID(1459) returns nil for the player's own Arcane Intellect while the buff is active. Not secret, just empty, so every existing guard passed it through and both the direct check and the coverage count concluded the player was unbuffed. This is what actually put the reminder on screen in Kira's report. The previous commit removed the unreadable group members from the count, but the player remained in it reading as missing, so have < total stayed true and the reminder persisted regardless. PlayerHasAuraByID and the player path of _unitHasBuff now treat an empty read under AurasRestricted as unknown rather than absent, and the player's own slot in CountGroupBuffCoverage honours readability the same way the group members' slots already do. With nothing readable the count collapses to zero and the reminder stays silent, which is correct until the buff can actually be seen.
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.
Bug: reported by Kira; the raid buff reminder sat on screen for a whole key, and appeared to fire whenever a player died and was battle-rezzed.
Issue: in a keystone the Arcane Intellect reminder stayed up for a Mage the entire run, with a coverage badge that seemed to blame people who were buffed. It looked tied to a rez because a death and rez fire the events that re-run the refresh, so the icon re-glowed each time.
Root cause: two separate aura reads report "absent" when they actually mean "cannot see". C_UnitAuras.GetUnitAuraBySpellID is documented SecretWhenUnitAuraRestricted, so inside a key every other group member's aura comes back secret and _unitHasBuff scored them all as missing. GetPlayerAuraBySpellID then returns nil for the player's own whitelisted buff there (verified in game: not secret, simply empty), so the player was scored missing too and have < total stayed true no matter who was in the group.
Fix: _unitHasBuff now reports readability alongside its answer, and CountGroupBuffCoverage drops any member it cannot read from both sides of the ratio, the player included. Undecidable hybrids leave the count as well: Paladin, Monk, Druid, Shaman and Demon Hunter sit in both beneficiary sets, and Balance vs Feral, Elemental vs Enhancement and Havoc vs Devourer all share the DAMAGER role, so without spec comms they were guessed as beneficiaries. Verified in a key: the reminder no longer appears while the group is buffed.