Add(Aura Buff Reminders): flask reminder in combat - #1968
Open
dfrisone wants to merge 4 commits into
Open
Conversation
The flask check bailed out of every restricted context through ConsumablePresenceUnverifiable and reported "flask present", so the reminder could never appear in combat or during an active key. That gate predates the Midnight flask buffs being whitelisted: the player's own aura for those IDs reads back plainly under restriction, the same reads the raid buff reminders already rely on. Only the unwhitelisted legacy TWW IDs and the name scan stay unavailable, so both sit out rather than false-firing a missing flask. Food, Inky Blackness and the augment rune keep the blanket bail, none of them having a whitelisted ID to read. Adds "Show Below In Combat" (off by default) to apply the existing Show Below thresholds in combat and inside a key, so a flask running out mid-fight reminds at the configured five minutes instead of only once it has gone. Requested by Kira.
The reminder appeared mid-keystone with a flask active. Under restriction the scan skipped any flask buff ID missing from the static NON_SECRET_SPELL_IDS list, then treated "found nothing" as "no flask" and fired. That is proving a negative from an incomplete read, and the skipped IDs are exactly the ones a flask can be sitting on. Two changes. ShouldSpellAuraBeSecret is the authority on whether an aura is readable, not the hand-maintained list, so an ID missing from it is still read when the client allows it. And absence is now only reported when every candidate ID was actually readable: an ID that could not be read leaves the answer unknown, and unknown suppresses rather than claiming the flask is gone. A secret aura result is also treated as present now instead of having its fields indexed for the duration threshold.
The flask reminder appeared mid-keystone with a flask up. The aura reads fine there (confirmed in game: GetPlayerAuraBySpellID returns the flask), so the reminder could only have come from IsUnderDuration reporting the buff as low. A restricted read hands back a real duration with the expiration withheld as 0, and IsUnderDuration had no guard for it: expirationTime - GetTime() is then hugely negative and clears every threshold, so a fresh one-hour flask reads as seconds from expiring. GetEatingExpirationTime already rejects exactly this sentinel; the shared threshold helper now does too, which covers food, weapon enchants, poisons and rites along with flasks. Also backs out the previous commit's unreadable-ID suppression. That was built on the theory that the flask ID was being skipped, which the in-game check disproved, and a single unreadable ID anywhere in the set would have muted the reminder for everyone. The widened readability check and the secret-result guard from that commit are kept: both are correct independently.
Verified in game: inside a keystone GetPlayerAuraBySpellID returns nil for the player's own flask even though the buff is up with 59 minutes left and the ID is on the non-secret whitelist. The read is not secret, just empty, so every guard passed it through and the scan concluded there was no flask. That invalidates the assumption this feature was built on. The whitelist says an aura is not secret; it does not promise the API answers under a keystone's restrictions. An empty read there carries no information. The scan is still attempted under restriction, so a flask is reported missing wherever the client does answer, but an empty result now suppresses instead of claiming absence. In a keystone that returns the reminder to its previous silence, which is correct until there is an API that can see the buff.
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.
What does this PR do?
Requested by Kira. The flask check bailed out of every restricted context through
ConsumablePresenceUnverifiableand reported "flask present", so the reminder could never appear in combat.It now performs the read instead of bailing, so a missing flask reminds in combat wherever the client still answers. A new Show Below In Combat toggle (off by default) lets the existing Show Below thresholds apply in combat, so a flask running low reminds at the configured five minutes rather than only once it has gone.
Three correctness fixes came out of testing this.
ShouldSpellAuraBeSecretis now the authority on whether an aura is readable rather than the hand-maintained whitelist. A zeroexpirationTimeis treated as unknown rather than as about to expire: a restricted read hands back a real duration with the expiration withheld as 0, andexpirationTime - GetTime()then clears every threshold, so a fresh one-hour flask read as seconds from expiring. That guard lives in the sharedIsUnderDuration, so it covers food, weapon enchants, poisons and rites too. And an empty read no longer proves absence.Scope note: inside a Mythic+ key the reminder stays silent. Verified in game that
GetPlayerAuraBySpellIDreturns nil there for the player's own whitelisted flask while the buff is plainly up, the index scan errors, andUNIT_AURAisSecretWhenAurasRestricted. Nothing can distinguish "you have a flask" from "I cannot see", so silence is the only honest option rather than firing over an active flask.How was it tested?
Verified in game in combat outside a key, and in a key confirming the reminder does not fire over an active flask.
Checklist
HookScript/hooksecurefunconly, neverSetScripton Blizzard frames