diff --git a/Conditionals.lua b/Conditionals.lua index 625d156..ed4814e 100644 --- a/Conditionals.lua +++ b/Conditionals.lua @@ -3470,6 +3470,51 @@ function CleveRoids.ClearSpellNameCaches() _baseNameCacheSize = 0 end +-- Resolve one aura on `unit` straight from ClassicAPI. This is the source of truth +-- for aura state: C_UnitAuras reads the unit's own descriptor, and its Aura::Source +-- cache reconstructs duration/expirationTime for ANY unit from the observed +-- SMSG_SPELL_GO -- caster-modified, so talent extensions and combo-point finisher +-- scaling are already applied, and Carnage's roll-gated Rip/Rake refresh is handled +-- in the DLL rather than mirrored here. +-- +-- Matching stays ours: by spellID when the conditional gave a number, else by +-- lowercased name (C_UnitAuras' own by-name lookup is case-sensitive and exact, +-- which would miss [debuff:thunder_clap]). +-- +-- Returns found, stacks, remaining, spellId. `remaining` is -1 for an aura with no +-- duration (permanent), nil when ClassicAPI has no timing for it -- an aura cast +-- before we logged in, or one refreshed at max stacks -- and seconds otherwise. +-- Presence and stacks are always reliable; only timing is best-effort. +local function ResolveUnitAuraViaClassicAPI(unit, searchID, searchName, isbuff) + local filter = isbuff and "HELPFUL" or "HARMFUL" + local i = 1 + while i <= 48 do + local name, _, count, _, duration, expirationTime, _, _, _, spellId = + C_UnitAuras.UnitAura(unit, i, filter) + if not name then break end + + local hit + if searchID then + hit = (spellId == searchID) + elseif searchName then + hit = (_string_lower(name) == searchName) + end + + if hit then + local remaining + if expirationTime and expirationTime > 0 then + remaining = expirationTime - GetTime() + if remaining < 0 then remaining = 0 end + elseif duration == 0 then + remaining = -1 -- no duration: permanent aura + end + return true, count or 0, remaining, spellId + end + i = i + 1 + end + return false +end + function CleveRoids.ValidateAura(unit, args, isbuff) if not args or not UnitExists(unit) then return false end @@ -3803,73 +3848,31 @@ function CleveRoids.ValidateAura(unit, args, isbuff) local ops = CleveRoids.operators local cmp = CleveRoids.comparators - -- For non-player units with time comparisons, try to get time from tracking systems + -- Non-player aura timing, straight from ClassicAPI. This replaces the old + -- lib.allBuffAuras lookup and the AllCasterAuraTracking / FindAllCasterAuraByName + -- fallback beneath it: both existed only because vanilla cannot report a timer for + -- an aura on another unit, which the Aura::Source cache now does. It also drops + -- the libdebuff UnitBuff timeleft bug those comments worked around. local nonPlayerAuraTimeRemaining = nil - if not isPlayer and args.name then - -- NOTE: libdebuff's UnitBuff has a bug where timeleft returns incorrect values - -- (showing ~1000s instead of actual remaining time). Skip it for buff time checks - -- and rely on all-caster tracking from AURA_CAST events instead. - - -- Fast path: Check lib.allBuffAuras (spellName-indexed, O(1) lookup) - -- More efficient than FindAllCasterAuraByName which does ID→name translation - if nonPlayerAuraTimeRemaining == nil and isbuff then - local lib = type(CleveRoids.libdebuff) == "table" and CleveRoids.libdebuff or nil - if lib and lib.allBuffAuras then - local targetGuid = CleveRoids.GetGUID(unit) - if targetGuid then - local buffEntries = lib.allBuffAuras[targetGuid] - if buffEntries then - -- Try exact name match first - local casters = buffEntries[args.name] - -- Try lowercase match if exact didn't work - if not casters and searchName then - for bName, c in pairs(buffEntries) do - if _string_lower(bName) == searchName then - casters = c - break - end - end - end - if casters then - for _, cData in pairs(casters) do - local elapsed = GetTime() - (cData.startTime or 0) - local rem = cData.duration > 0 and (cData.duration - elapsed) or -1 - if rem == nil or rem > 0 or cData.duration <= 0 then - nonPlayerAuraTimeRemaining = rem - if not found then - found = true - stacks = 0 - end - break - end - end - end - end - end + if not isPlayer and (searchID or searchName) then + local capFound, capStacks, capRemaining = + ResolveUnitAuraViaClassicAPI(unit, searchID, searchName, isbuff) + if capFound then + if not found then + found = true + stacks = capStacks or 0 end + -- Left nil when ClassicAPI has no timing (aura predates login, or a + -- max-stack refresh); the caller then falls back to its found-and-0 default. + nonPlayerAuraTimeRemaining = capRemaining end - -- Second try: All-caster tracking from AURA_CAST events (works for any caster) - -- Only use if libdebuff didn't find it (libdebuff has more accurate timing for player casts) - if nonPlayerAuraTimeRemaining == nil then - local targetGuid = CleveRoids.GetGUID(unit) - if targetGuid then - local remaining, casterGuid = CleveRoids.FindAllCasterAuraByName(targetGuid, args.name) - - -- Debug output when enabled - if CleveRoids.debug then - local hasData = CleveRoids.AllCasterAuraTracking[targetGuid] ~= nil - DEFAULT_CHAT_FRAME:AddMessage(string.format( - "|cffff9900[AuraLookup]|r %s on GUID %s: hasData=%s, remaining=%s", - tostring(args.name), string.sub(tostring(targetGuid), 1, 16), - tostring(hasData), tostring(remaining) - )) - end - - if remaining then - nonPlayerAuraTimeRemaining = remaining - end - end + if CleveRoids.debug then + DEFAULT_CHAT_FRAME:AddMessage(string.format( + "|cffff9900[AuraLookup]|r %s on %s: found=%s, stacks=%s, remaining=%s", + tostring(args.name), tostring(unit), tostring(capFound), + tostring(capStacks), tostring(capRemaining) + )) end end diff --git a/docs/CLASSICAPI-TODO.md b/docs/CLASSICAPI-TODO.md index b4a71c2..288cb75 100644 --- a/docs/CLASSICAPI-TODO.md +++ b/docs/CLASSICAPI-TODO.md @@ -25,10 +25,23 @@ API references are line numbers into `C:\Git\ClassicAPI\docs\API.md`. `applications` (stacks), `duration`. - **Unlocks:** `[dispellable]` / `[curse]` / `[magic]` target conditionals and spellID-based (rank/locale-proof) aura matching. -- **Caveat:** `expirationTime` is only populated for `unit=="player"`; it's `0` - for target/focus (vanilla server limitation). Target debuff *timers* still - need the existing libdebuff tracking — only presence/stacks/school/spellId - are reliable cross-unit. +- **~~Caveat: `expirationTime` is player-only~~ — NO LONGER TRUE.** This note said + target/focus `expirationTime` was always `0`, so debuff *timers* had to stay on + libdebuff. ClassicAPI has since added the `Aura::Source` cache: for a non-player + unit `expirationTime` is reconstructed from the observed `SMSG_SPELL_GO`, and + `duration` is the **caster-modified** value (talent extensions like Improved + Shadow Word: Pain included). It also handles combo-point finisher scaling + automatically (Rupture at 4 CP reads 14s, no registration) and ships Carnage's + roll-gated Rip/Rake refresh in the DLL (`src/turtle/Carnage.cpp`, exposed as + `RegisterAuraDurationModifierByTrigger`). +- **Real remaining caveats** (all best-effort, from `docs/API.md`): + - Only auras observed *after login* carry caster/timing; older ones report + `expirationTime` 0 and `sourceUnit` nil. + - Max-stack refresh is a blind spot: re-applying at max stacks (Shadow Weaving + 5→5) emits no client-visible change, so the entry elapses and evicts. + - Out-of-range group members are spell-ID only, with `applications` always 1. +- **Consequence:** most of libdebuff is now redundant — see + "libdebuff retirement" below. - **DONE (slice 1 — dispel-type conditionals):** added `ClassicAPI.lua` detection module (`CleveRoids.ClassicAPI`, mirrors NampowerAPI's @@ -150,11 +163,13 @@ Findings from the full Conditionals.lua audit. These look like ClassicAPI candidates but are **worse** than the current implementation — recorded so we don't re-investigate. -- **Auras / `ValidateAura`** — `C_UnitAuras.expirationTime` is **player-only** - (`0` for every other unit). It cannot replace the nampower `GetUnitField` - batch read or the remote-duration tracking (libdebuff / overflow slots). The - dispel-*type* path already uses `C_UnitAuras` (that's the one thing it's good - for); aura *timing/stacks* on non-player units must stay on nampower/libdebuff. +- ~~**Auras / `ValidateAura`** — `C_UnitAuras.expirationTime` is player-only~~ — + **STALE, do not trust this entry.** It was written before the `Aura::Source` + cache landed. Non-player `expirationTime`/`duration` now work (caster-modified, + talent extensions included), so this is no longer a reason to keep libdebuff's + remote-duration tracking. See Tier 1 §1 and "libdebuff retirement" for the + current picture and the real caveats. The nampower `GetUnitField` batch read is + a separate question and has not been re-examined. - **`GetCurrentShapeshiftIndex` form loop / `[stance]`·`[form]`** — `GetShapeshiftFormID()` returns the **DBC form id** (Cat=1, Bear=5, Shadowform=28…), NOT the 1-based **bar index** these conditionals compare @@ -183,6 +198,39 @@ don't re-investigate. added `[focus]`/`[nofocus]`. `/focus` is provided by ClassicAPI's companion addon. +## libdebuff retirement + +libdebuff exists because vanilla cannot report debuff durations on units other +than the player. ClassicAPI's `Aura::Source` cache now does exactly that, so most +of the library is redundant. This is a staged replacement, not a delete: measure +first, then remove per group. + +Current footprint: 28 public `lib:` methods, ~726 internal references in +`Utility.lua`, consumers in 8 files (`Conditionals.lua` 45, `Compatibility/pfUI.lua` +39, `Core.lua` 9). Note pfUI did **not** delete its own libdebuff — v9.0.25 still +ships ~1695 lines of it, re-based on `C_UnitAuras.GetAuraDataByIndex` / +`GetAuraDataBySpellName`. "Re-base on C_UnitAuras", not "remove", is the precedent. + +**Group A — replaceable by `C_UnitAuras` (do these first):** +`GetDuration`, `GetDebuffCaster`, `IsOurDebuff`, `UnitBuff`/`UnitDebuff`, +`FindPlayerDebuff`/`FindPlayerBuff`, `GetAllDebuffsOnTarget`, `GetCachedIcon`, +`ApplyCarnageRefresh`, and the Dark Harvest trio (`ApplyDarkHarvestStart`/`End`, +`GetDarkHarvestReduction`, `GetTimeRemainingWithDarkHarvest`) — ClassicAPI ships +Carnage refresh and Dark Harvest tick compression in the DLL. + +**Group B — no `C_UnitAuras` equivalent, keep:** +`ShouldApplyDebuffRank`, `DidSpellFail`, `WasSpellReflected`, `DidTargetEvade`, +`ProcessMissReason`, `IsPersonalDebuff`, `GetSpellRank`/`GetSpellBaseName`, +`HasPendingCast`. These are miss/rank/learning logic, not aura state. + +**Gate before removing Group A:** confirm parity in-game on one spell where +ClassicAPI does the hard part — Rip under Carnage. Compare +`CleveRoids.libdebuff:GetDuration(spellID)` against +`C_UnitAuras.GetUnitAuraBySpellID(unit, spellID).duration` and the derived +remaining (`expirationTime - GetTime()`), on a target you have debuffed. If those +agree across a Carnage proc, Group A can go. Watch the documented best-effort +gaps: an aura cast before you logged in, and refresh-at-max-stacks. + ## Marginal / optional follow-ups - **`[swimming]`** → `IsSwimming()` would drop the nampower-2.36 version gate +