From 7a97c4717732f6420ed5d8001c4613ef5a8e23a7 Mon Sep 17 00:00:00 2001 From: fuffc Date: Fri, 14 Aug 2026 22:39:49 +0200 Subject: [PATCH 1/5] fix(aura): identify a cached aura by its caster and descriptor slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two casters of one spell on one target are two auras in two descriptor slots, but `Aura::Source` held a single entry per `(target, spellId)`. The second cast overwrote the first caster's `casterGuid` and timing, and `OnAuraRemoved` for either copy evicted the entry backing the survivor. Observable from Lua, with a same-class groupmate debuffing your target: `sourceUnit` names the wrong caster, so a `PLAYER`-filtered query drops your own debuff; and the base-duration estimate stored for the other caster elapses while your talent-extended aura is still up, which `PushEnriched` reports as `expirationTime = 0` — an aura on the unit with no timer at all. Entries are now keyed `(target, spellId, caster)`, the identity the server uses. Recovering which instance a descriptor slot holds needs the two halves joined: SpellGo knows the caster but no slot, `OnAuraAdded` knows the slot but no caster, and they arrive in that order, so the application hook seats its slot on the newest cast capture still awaiting one. `Get` takes that slot and resolves by it first, falling back to the unit's sole entry for the spell — every single-caster case, and the paths with no descriptor to read a slot from. Several entries and no binding resolves to a miss: the caller's unknown-caster defaults beat a coin flip between two casters. `OnAuraRemoved` and `EvictAbsent` retire the instance in the vacated slot rather than every entry sharing its spell ID. Co-Authored-By: Claude Opus 5 --- src/aura/Data.cpp | 133 ++++++++------- src/aura/Source.cpp | 332 ++++++++++++++++++++++++++----------- src/aura/Source.h | 68 +++++--- src/lossofcontrol/Info.cpp | 3 +- 4 files changed, 354 insertions(+), 182 deletions(-) diff --git a/src/aura/Data.cpp b/src/aura/Data.cpp index 537886a..565378b 100644 --- a/src/aura/Data.cpp +++ b/src/aura/Data.cpp @@ -218,23 +218,33 @@ int PlayerLevel(const uint8_t *player) { desc + Offsets::OFF_UNIT_FIELD_LEVEL); } -// The caster GUID attributed to `(guid, spellID)`: the observed caster from -// the Aura::Source cache, else — when we never saw the cast — the unit itself -// if the spell is a self-only-target buff (self-buffs can't be cross-cast). -// 0 when genuinely unknown. Single source of truth for both display -// (sourceGUID/sourceUnit) and the PLAYER caster filter, so a self-buff shows a -// source AND matches HELPFUL|PLAYER instead of the two paths disagreeing. -uint64_t EffectiveCaster(uint64_t guid, uint32_t spellID) { +// Caster + timing attributed to ONE aura instance — the aura `spellID` sitting +// in absolute descriptor `slot` on `guid` (`Aura::Source::SLOT_UNBOUND` when +// the caller has no slot: the out-of-range group array, the cache fallback). +// The slot is what tells two casters' copies of one spell apart; without it +// they are indistinguishable from a spell ID alone. +// +// The caster is the observed one from the Aura::Source cache, else — when we +// never saw the cast — the unit itself if the spell is a self-only-target buff +// (self-buffs can't be cross-cast), else 0. Single source of truth for both +// display (sourceGUID/sourceUnit) and the PLAYER caster filter, so a self-buff +// shows a source AND matches HELPFUL|PLAYER instead of the two paths +// disagreeing. +struct Attribution { + uint64_t caster; + uint32_t expirationMs; + uint32_t durationMs; +}; + +Attribution Attribute(uint64_t guid, uint32_t spellID, int slot) { + Attribution a = {0, 0, 0}; if (guid == 0 || spellID == 0) - return 0; - uint64_t c = 0; - uint32_t expMs = 0; - uint32_t durMs = 0; - if (Aura::Source::Get(guid, spellID, &c, &expMs, &durMs) && c != 0) - return c; // observed caster - if (Spell::IsSelfBuff::IsSelfBuff(spellID)) - return guid; // self-only-target aura → cast by the unit itself - return 0; + return a; + Aura::Source::Get(guid, spellID, slot, &a.caster, &a.expirationMs, + &a.durationMs); + if (a.caster == 0 && Spell::IsSelfBuff::IsSelfBuff(spellID)) + a.caster = guid; // self-only-target aura → cast by the unit itself + return a; } } // namespace @@ -271,7 +281,7 @@ bool IsPlayerCast(const uint8_t *unit, int slot) { if (spellID == 0) return false; const uint64_t player = Unit::Identity::PlayerGuid(); - return player != 0 && EffectiveCaster(UnitGuid(unit), spellID) == player; + return player != 0 && Attribute(UnitGuid(unit), spellID, slot).caster == player; } // Applies the PLAYER / !PLAYER caster restriction. `isPlayerCast` is the @@ -321,12 +331,13 @@ bool MatchesAura(const Match &match, bool isPlayerCast, uint32_t spellID) { CcMatches(match.cc, spellID); } -// Group-array analog of IsPlayerCast: the group aura carries no caster, so we -// consult the Aura::Source cache by (guid, spellID). A miss counts as "not the -// player" (same as IsPlayerCast). +// Group-array analog of IsPlayerCast: the member has no descriptor, so there is +// no slot to attribute by and the cache is consulted by (guid, spellID) alone. +// A miss counts as "not the player" (same as IsPlayerCast). bool GroupIsPlayerCast(uint64_t guid, uint32_t spellID) { const uint64_t player = Unit::Identity::PlayerGuid(); - return player != 0 && EffectiveCaster(guid, spellID) == player; + return player != 0 && + Attribute(guid, spellID, Aura::Source::SLOT_UNBOUND).caster == player; } int FindNthSlot(const uint8_t *unit, int oneBasedIndex, Filter filter, @@ -521,10 +532,13 @@ static void BuildTable(void *L, uint32_t spellID, int applications, // buff table (gated on `isPlayer`); for everyone else it, the caster, and the // applied (caster-modified) duration come from the `Aura::Source` SMSG_SPELL_GO // cache when it observed the cast — a miss leaves the modern-truthful defaults -// (expiration 0, no sourceUnit/GUID). +// (expiration 0, no sourceUnit/GUID). `slot` is the absolute descriptor slot +// the aura occupies, which is what attributes it to the right caster when two +// of them hold the same spell on the unit; `Aura::Source::SLOT_UNBOUND` for the +// paths with no descriptor to read one from. static void PushEnriched(void *L, uint64_t guid, uint32_t spellID, bool isHelpful, int applications, int unitLevel, - bool isPlayer) { + bool isPlayer, int slot) { double duration = 0.0; double expirationTime = 0.0; uint64_t casterGuid = 0; @@ -536,25 +550,20 @@ static void PushEnriched(void *L, uint64_t guid, uint32_t spellID, expirationTime = PlayerBuffExpirationSeconds(entry); } if (spellID != 0 && guid != 0) { - uint64_t c = 0; - uint32_t expMs = 0; - uint32_t durMs = 0; - if (Aura::Source::Get(guid, spellID, &c, &expMs, &durMs)) { - // A cached expiration that already elapsed while the aura is still - // present (non-player casters get an underestimated base duration) - // is not meaningful — report unknown (0) rather than a negative - // remaining time. - if (expirationTime == 0.0 && expMs != 0 && !ExpirationElapsed(expMs)) - expirationTime = static_cast(expMs) * 0.001; - if (durMs != 0) - duration = static_cast(durMs) * 0.001; - } - // Caster (sourceGUID/sourceUnit): observed caster, else the self-buff - // inference — same resolution the PLAYER filter uses, so a self-buff - // both shows a source and matches HELPFUL|PLAYER. Timing stays unknown - // when we never saw the cast; only the caster is inferred, and it's - // exact. - casterGuid = EffectiveCaster(guid, spellID); + // Caster (sourceGUID/sourceUnit) and timing resolved together, so a + // self-buff both shows a source and matches HELPFUL|PLAYER instead of + // the two paths disagreeing. + const Attribution a = Attribute(guid, spellID, slot); + // A cached expiration that already elapsed while the aura is still + // present (non-player casters get an underestimated base duration) + // is not meaningful — report unknown (0) rather than a negative + // remaining time. + if (expirationTime == 0.0 && a.expirationMs != 0 && + !ExpirationElapsed(a.expirationMs)) + expirationTime = static_cast(a.expirationMs) * 0.001; + if (a.durationMs != 0) + duration = static_cast(a.durationMs) * 0.001; + casterGuid = a.caster; } BuildTable(L, spellID, applications, isHelpful, duration, expirationTime, casterGuid); @@ -565,7 +574,7 @@ void Push(void *L, const uint8_t *unit, int slot) { const bool isHelpful = slot < Offsets::UNIT_AURA_BUFF_COUNT; const int unitLevel = (unit != nullptr) ? PlayerLevel(unit) : 0; PushEnriched(L, UnitGuid(unit), spellID, isHelpful, ReadStacks(unit, slot), - unitLevel, unit != nullptr && unit == LocalPlayer()); + unitLevel, unit != nullptr && unit == LocalPlayer(), slot); } namespace { @@ -576,12 +585,13 @@ namespace { // applied (caster-modified) ms when known, else the Spell.dbc base. void PushFromCache(void *L, const uint8_t *unit, const Aura::Source::CachedAura &c, bool isHelpful) { - // `PushEnriched` re-reads the same cache entry `c` came from (by - // guid+spellID) to fill caster / expiration / applied duration, so the - // result is identical to reading `c`'s fields directly. Stacks aren't in - // SMSG_SPELL_GO, so `applications` is 1. + // `PushEnriched` re-reads the same cache entry `c` came from — passing + // `c.slot` back is what makes it land on that exact entry rather than on + // another caster's copy of the spell. Stacks aren't in SMSG_SPELL_GO, so + // `applications` is 1. const int unitLevel = (unit != nullptr) ? PlayerLevel(unit) : 0; - PushEnriched(L, UnitGuid(unit), c.spellId, isHelpful, 1, unitLevel, false); + PushEnriched(L, UnitGuid(unit), c.spellId, isHelpful, 1, unitLevel, false, + c.slot); } // Reconciles the `Aura::Source` cache against `unit`'s descriptor: when the @@ -598,15 +608,9 @@ void ReconcileCache(const uint8_t *unit) { if (unit == nullptr) return; uint32_t present[Offsets::UNIT_AURA_TOTAL]; - int n = 0; - for (int slot = 0; slot < Offsets::UNIT_AURA_TOTAL; ++slot) { - const uint32_t id = ReadSpellID(unit, slot); - if (id != 0) - present[n++] = id; - } - if (n == 0) - return; - Aura::Source::EvictAbsent(UnitGuid(unit), present, n); + for (int slot = 0; slot < Offsets::UNIT_AURA_TOTAL; ++slot) + present[slot] = ReadSpellID(unit, slot); + Aura::Source::EvictAbsent(UnitGuid(unit), present); } // True if `unit`'s descriptor currently exposes any visible aura in either @@ -813,7 +817,7 @@ bool PushNthGroupCacheFallback(void *L, uint64_t guid, const uint16_t *arr, continue; if (++seen == oneBasedIndex) { PushEnriched(L, guid, buf[i].spellId, filter == Filter::Helpful, 1, - GroupMemberLevel(guid), false); + GroupMemberLevel(guid), false, buf[i].slot); return true; } } @@ -833,7 +837,7 @@ void AppendGroupCacheFallbacks(void *L, uint64_t guid, const uint16_t *arr, continue; Game::Lua::PushNumber(L, static_cast(nextKey++)); PushEnriched(L, guid, buf[i].spellId, filter == Filter::Helpful, 1, level, - false); + false, buf[i].slot); Game::Lua::SetTable(L, outerIdx); } } @@ -856,7 +860,7 @@ bool PushGroupCacheFallbackMatch(void *L, uint64_t guid, const uint16_t *arr, if (!GroupFallbackEligible(arr, buf[i], match)) continue; PushEnriched(L, guid, buf[i].spellId, !harmful, 1, GroupMemberLevel(guid), - false); + false, buf[i].slot); return true; } return false; @@ -881,7 +885,7 @@ bool PushNthGroupAura(void *L, uint64_t guid, int oneBasedIndex, Filter filter, if (++matches == oneBasedIndex) { PushEnriched(L, guid, GroupSpellID(arr, slot), filter == Filter::Helpful, 1, GroupMemberLevel(guid), - false); + false, Aura::Source::SLOT_UNBOUND); return true; } } @@ -912,7 +916,7 @@ bool PushGroupAuraBySpellID(void *L, uint64_t guid, uint32_t spellID, if (!GroupSlotEligible(guid, arr, slot, match)) continue; PushEnriched(L, guid, spellID, slot < Offsets::UNIT_AURA_BUFF_COUNT, 1, - GroupMemberLevel(guid), false); + GroupMemberLevel(guid), false, Aura::Source::SLOT_UNBOUND); return true; } if (!GroupArrayHasVisibleAura(arr)) { @@ -953,7 +957,7 @@ bool PushGroupAuraBySpellName(void *L, uint64_t guid, const char *spellName, if (!GroupSlotEligible(guid, arr, slot, match)) continue; PushEnriched(L, guid, id, slot < Offsets::UNIT_AURA_BUFF_COUNT, 1, - GroupMemberLevel(guid), false); + GroupMemberLevel(guid), false, Aura::Source::SLOT_UNBOUND); return true; } if (!GroupArrayHasVisibleAura(arr)) { @@ -986,7 +990,8 @@ void AppendGroupAuras(void *L, uint64_t guid, Filter filter, Match match, continue; Game::Lua::PushNumber(L, static_cast(nextKey++)); PushEnriched(L, guid, GroupSpellID(arr, slot), - filter == Filter::Helpful, 1, level, false); + filter == Filter::Helpful, 1, level, false, + Aura::Source::SLOT_UNBOUND); Game::Lua::SetTable(L, outerIdx); } // Empty array (server delta-resend gap): supplement from the cache. diff --git a/src/aura/Source.cpp b/src/aura/Source.cpp index a405f08..7b5b0e0 100644 --- a/src/aura/Source.cpp +++ b/src/aura/Source.cpp @@ -122,6 +122,18 @@ bool WasRecentPlayerCast(uint32_t spellId) { // application hook fires for the same aura. enum Kind : int8_t { KIND_UNKNOWN = -1, KIND_HELPFUL = 0, KIND_HARMFUL = 1 }; +// An entry is one AURA INSTANCE, identified the way the server identifies one: +// `(target, spell, caster)`. Two same-class raiders' Corruption on one boss are +// two auras occupying two descriptor slots, so a `(target, spell)` identity +// collapses them into one entry — the later cast overwrites the earlier +// caster's timing, and `OnAuraRemoved` for either copy evicts both. +// +// The descriptor only stores spell IDs, so recovering WHICH instance a slot +// holds needs the slot bound to its entry. SpellGo knows the caster but no +// slot; `OnAuraAdded` knows the slot but no caster. They arrive in that order +// (SpellGo, then the SMSG_UPDATE_OBJECT that seats the aura), so the +// application hook binds the cast capture it belongs to — the newest entry for +// this `(target, spell)` still awaiting a slot. struct Entry { uint64_t targetGuid; uint64_t casterGuid; @@ -129,12 +141,13 @@ struct Entry { uint32_t expirationMs; // 0 = infinite / unknown duration uint32_t durationMs; // applied duration (incl. caster mods); 0 = none uint32_t stampMs; // last write time; EvictAbsent grace (see below) + int16_t slot; // absolute descriptor slot, SLOT_UNBOUND until bound int8_t kind; // Kind; descriptor-slot-derived, KIND_UNKNOWN if only seen via SpellGo bool used; }; // Sized for the realistic worst case: a fully raid-buffed 40-man plus its -// debuff load. We cache one entry per (target, spellId) for EVERY +// debuff load. We cache one entry per (target, spellId, caster) for EVERY // aura-applying SMSG_SPELL_GO we observe — not just auras on the player, but // every buff cast on every unit in view — so the live working set in a raid // is ~40 members × ~30 persistent buffs ≈ 1200, plus debuffs. At the old 256 @@ -142,7 +155,9 @@ struct Entry { // dropping their source (the "lost sourceGUID on a buff" report). 2048 gives // a fully-buffed 40-man comfortable headroom; the tick sweep still reclaims // expired/orphaned entries so it rarely approaches full outside a raid. -// (~40 bytes/entry → ~80 KB static.) +// (~40 bytes/entry → ~80 KB static.) Per-caster identity adds one entry per +// extra caster of the same spell on one target, which is bounded by the 16 +// debuff slots that can hold them. constexpr int kCacheSize = 2048; Entry g_cache[kCacheSize]; @@ -187,48 +202,68 @@ bool DescriptorListsAura(uint64_t guid, uint32_t spellId) { return false; } -// `fromCast` true: the SpellGo hook — authoritative caster + caster-modified -// (talented) timing. False: the OnAuraAdded application hook — timing only, -// no caster, and it must not clobber an entry SpellGo already owns (that -// would replace talented timing with the unmodified base), so it skips -// entries that already carry a caster. -void Store(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid, - uint32_t expirationMs, uint32_t durationMs, bool fromCast, - int8_t kind) { - if (targetGuid == 0 || spellId == 0) - return; +// ---- Entry lookup -------------------------------------------------------- - const uint32_t now = NowMs(); +// The exact aura instance `caster` has on `targetGuid` — the server's own +// identity for one aura, and what a repeat cast refreshes. +Entry *FindByCaster(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid) { + for (auto &e : g_cache) + if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && + e.casterGuid == casterGuid) + return &e; + return nullptr; +} + +// The instance seated in descriptor slot `slot`. +Entry *FindBySlot(uint64_t targetGuid, uint32_t spellId, int slot) { + if (slot < 0) + return nullptr; + for (auto &e : g_cache) + if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && + e.slot == slot) + return &e; + return nullptr; +} - // Update an existing entry for this exact aura instance. +// The newest cast capture for this aura still awaiting a descriptor slot — +// what an `OnAuraAdded` for `(target, spell)` is seating. Newest wins because +// SpellGo precedes the application by a packet, so the freshest unbound entry +// is the cast that just landed; an older one is a capture that never seated +// (fully resisted, immune) and is left to expire on its own. +Entry *FindFreshestUnbound(uint64_t targetGuid, uint32_t spellId) { + Entry *best = nullptr; + for (auto &e : g_cache) + if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && + e.slot == SLOT_UNBOUND && (best == nullptr || e.stampMs > best->stampMs)) + best = &e; + return best; +} + +// The one entry for `(target, spell)`, or null when there are none or several. +// Several means two casters and no way to tell their instances apart from a +// spell ID alone, which is exactly when guessing is what produces a wrong +// caster and a wrong timer. +Entry *FindSole(uint64_t targetGuid, uint32_t spellId) { + Entry *found = nullptr; for (auto &e : g_cache) { - if (e.used && e.targetGuid == targetGuid && e.spellId == spellId) { - e.stampMs = now; // refresh EvictAbsent grace on any touch - // Learn the classification whenever a slot-derived kind arrives — - // independent of caster/timing ownership, and never downgrade a - // known kind back to unknown. - if (kind != KIND_UNKNOWN) - e.kind = kind; - if (!fromCast && e.casterGuid != 0) - return; // SpellGo owns this entry; keep its caster + timing - if (casterGuid != 0) - e.casterGuid = casterGuid; - e.expirationMs = expirationMs; - e.durationMs = durationMs; - return; - } + if (!e.used || e.targetGuid != targetGuid || e.spellId != spellId) + continue; + if (found != nullptr) + return nullptr; + found = &e; } - // Take a free slot, else an expired one whose aura the descriptor no - // longer lists (a still-present aura keeps its slot so its caster isn't - // lost — see DescriptorListsAura), else evict round-robin. + return found; +} + +// Takes a free slot, else an expired one whose aura the descriptor no longer +// lists (a still-present aura keeps its slot so its caster isn't lost — see +// DescriptorListsAura), else evicts. +Entry *Claim(uint32_t now) { for (auto &e : g_cache) { if (!e.used || (e.expirationMs != 0 && now >= e.expirationMs && - !DescriptorListsAura(e.targetGuid, e.spellId))) { - e = {targetGuid, casterGuid, spellId, expirationMs, durationMs, - now, kind, true}; - return; - } + !DescriptorListsAura(e.targetGuid, e.spellId))) + return &e; } // Saturated. Honor the same invariant as the tick sweep: an entry whose // aura is still present on a resolvable unit is NEVER evicted — its caster @@ -253,9 +288,91 @@ void Store(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid, if (orphan == nullptr || e.stampMs < orphan->stampMs) orphan = &e; } - Entry *victim = (orphan != nullptr) ? orphan : lru; - *victim = {targetGuid, casterGuid, spellId, expirationMs, durationMs, now, - kind, true}; + return (orphan != nullptr) ? orphan : lru; +} + +// ---- Writes -------------------------------------------------------------- + +// The SpellGo hook: authoritative caster + caster-modified (talented) timing. +// Identity is `(target, spell, caster)`, so a second caster of the same spell +// opens its own entry instead of overwriting the first's, and a recast by the +// same caster refreshes theirs — keeping the descriptor slot already bound to +// it. +void StoreFromCast(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid, + uint32_t expirationMs, uint32_t durationMs) { + if (targetGuid == 0 || spellId == 0) + return; + const uint32_t now = NowMs(); + Entry *e = FindByCaster(targetGuid, spellId, casterGuid); + // A lone unattributed entry for this aura is the same instance seen without + // a caster (seated by an application hook while we were out of view, or a + // group-array guess), so claim it rather than opening a second entry for + // one aura — the same adoption `ApplyDurationModifiers` does. Only when + // it's the unit's only entry for the spell: with several, "the one this + // cast refreshes" is a guess, and guessing wrong writes this caster's timer + // onto another's aura. + if (e == nullptr) { + Entry *sole = FindSole(targetGuid, spellId); + if (sole != nullptr && sole->casterGuid == 0) + e = sole; + } + if (e == nullptr) { + e = Claim(now); + *e = {targetGuid, casterGuid, spellId, expirationMs, durationMs, + now, SLOT_UNBOUND, KIND_UNKNOWN, true}; + return; + } + e->stampMs = now; // refresh EvictAbsent grace on any touch + e->casterGuid = casterGuid; + e->expirationMs = expirationMs; + e->durationMs = durationMs; +} + +// The OnAuraAdded / OnAuraStacksChanged application hooks: a descriptor slot, +// a classification, and base timing — but no caster (except the local player's +// own recent cast, which `StampApplication` resolves). Seats the slot on the +// cast capture it belongs to, and must not clobber the caster + talented +// timing SpellGo already owns for that instance. +// +// `slot` is SLOT_UNBOUND for the out-of-range group-array path, which has no +// descriptor at all; there the only available identity is `(target, spell)`. +void StoreFromApplication(uint64_t targetGuid, uint32_t spellId, + uint64_t casterGuid, uint32_t expirationMs, + uint32_t durationMs, int slot, int8_t kind) { + if (targetGuid == 0 || spellId == 0) + return; + const uint32_t now = NowMs(); + // Anything outside the descriptor's slot range can't index it, so it binds + // nothing — `EvictAbsent` indexes the array by a bound entry's slot. + if (slot < 0 || slot >= Offsets::UNIT_AURA_TOTAL) + slot = SLOT_UNBOUND; + + Entry *e = FindBySlot(targetGuid, spellId, slot); + if (e == nullptr) + e = FindFreshestUnbound(targetGuid, spellId); + if (e == nullptr && slot < 0) + e = FindSole(targetGuid, spellId); + if (e == nullptr) { + e = Claim(now); + *e = {targetGuid, casterGuid, spellId, expirationMs, durationMs, now, + static_cast(slot), kind, true}; + return; + } + + e->stampMs = now; + if (slot >= 0) + e->slot = static_cast(slot); + // Learn the classification whenever a slot-derived kind arrives — + // independent of caster/timing ownership, and never downgrade a known kind + // back to unknown. + if (kind != KIND_UNKNOWN) + e->kind = kind; + if (e->casterGuid != 0) + return; // SpellGo owns this entry; keep its caster + talented timing + if (casterGuid != 0) + e->casterGuid = casterGuid; + e->expirationMs = expirationMs; + e->durationMs = durationMs; } // ---- Out-of-range group-member aura snapshots --------------------------- @@ -293,25 +410,29 @@ void StampGroupGuess(uint64_t guid, uint16_t spellId, int8_t kind, uint32_t now) const uint32_t base = SpellDurationMs(rec, /*casterIsPlayer*/ false); if (base == 0) return; - Store(guid, spellId, /*casterGuid*/ 0, now + base, base, /*fromCast*/ false, - kind); + StoreFromApplication(guid, spellId, /*casterGuid*/ 0, now + base, base, + SLOT_UNBOUND, kind); } -// Evict the entry for an aura the engine reports gone. Keyed by (target, -// spell) like the rest of the cache. Without this, the GetAuraDataByIndex -// fallback would keep surfacing a dropped aura until its computed expiry — -// e.g. a Rank 1 buff replaced by Rank 2 (engine drops Rank 1 from the -// descriptor) would show as a phantom second aura, or a dispelled buff would -// linger. -void Evict(uint64_t targetGuid, uint32_t spellId) { +// Evict the entry for an aura the engine reports gone, identified by the +// descriptor slot it vacated. Without this, the GetAuraDataByIndex fallback +// would keep surfacing a dropped aura until its computed expiry — e.g. a Rank +// 1 buff replaced by Rank 2 (engine drops Rank 1 from the descriptor) would +// show as a phantom second aura, or a dispelled buff would linger. +// +// The slot is what makes this safe when two casters hold the same spell on one +// target: only the instance that actually fell off goes. Falling back to a +// spell-ID match when nothing is bound to the slot keeps the pre-binding +// behaviour, but only while the match is unambiguous — dropping one of two +// casters' entries at random would take a live aura's caster with it. +void Evict(uint64_t targetGuid, uint32_t spellId, int slot) { if (targetGuid == 0 || spellId == 0) return; - for (auto &e : g_cache) { - if (e.used && e.targetGuid == targetGuid && e.spellId == spellId) { - e.used = false; - return; - } - } + Entry *e = FindBySlot(targetGuid, spellId, slot); + if (e == nullptr) + e = FindSole(targetGuid, spellId); + if (e != nullptr) + e->used = false; } // ---- Server-side duration modifiers (trigger-driven inference) ----------- @@ -702,13 +823,11 @@ void HandleSpellGo(uint64_t caster, uint32_t spellId, const uint64_t *targets, if (numTargets == 0) { // No explicit hit list (self-cast with caster-implicit target). - Store(caster, spellId, caster, expirationMs, durationMs, true, - KIND_UNKNOWN); + StoreFromCast(caster, spellId, caster, expirationMs, durationMs); return; } for (int i = 0; i < numTargets; ++i) - Store(targets[i], spellId, caster, expirationMs, durationMs, true, - KIND_UNKNOWN); + StoreFromCast(targets[i], spellId, caster, expirationMs, durationMs); } // SMSG_SPELL_GO parse (funnel subscriber). At the leaf handler the engine has @@ -741,12 +860,19 @@ const Net::PacketDispatch::AutoSubscribe _spellGoSub{&SpellGoSub}; // ---- Aura-application co-hooks (timing for proc / triggered auras) ------- -// Stamp expiration for an aura that just landed/refreshed on `unit`. Used by -// both the add and stack-change hooks. No caster is available from these -// paths, so it stamps timing only with `fromCast=false` — Store skips any -// entry SpellGo already owns, so a directly-cast aura keeps its talented -// timing. Base (unmodified) duration is the best estimate without a caster. -void StampApplication(void *unit, uint32_t spellId, int8_t kind) { +// Classify by the absolute aura slot: 0..BUFF_COUNT-1 = buff (helpful), +// BUFF_COUNT..TOTAL-1 = debuff (harmful). +int8_t KindForSlot(int slot) { + return slot >= Offsets::UNIT_AURA_BUFF_COUNT ? KIND_HARMFUL : KIND_HELPFUL; +} + +// Stamp expiration for an aura that just landed/refreshed in `slot` on `unit`. +// Used by both the add and stack-change hooks. No caster is available from +// these paths, so it stamps timing only — StoreFromApplication keeps whatever +// SpellGo already owns, so a directly-cast aura keeps its talented timing. +// Base (unmodified) duration is the best estimate without a caster. The slot +// is what binds this application to the cast capture behind it. +void StampApplication(void *unit, uint32_t spellId, int slot) { if (spellId == 0) return; const uint8_t *rec = Spell::Lookup::RecordForID(static_cast(spellId)); @@ -763,14 +889,8 @@ void StampApplication(void *unit, uint32_t spellId, int8_t kind) { const uint32_t durationMs = SpellDurationMs(rec, byPlayer); const uint64_t caster = byPlayer ? Unit::Identity::PlayerGuid() : 0; const uint32_t expirationMs = durationMs > 0 ? NowMs() + durationMs : 0; - Store(unitGuid, spellId, caster, expirationMs, durationMs, - /*fromCast*/ false, kind); -} - -// Classify by the absolute aura slot: 0..BUFF_COUNT-1 = buff (helpful), -// BUFF_COUNT..TOTAL-1 = debuff (harmful). -int8_t KindForSlot(int slot) { - return slot >= Offsets::UNIT_AURA_BUFF_COUNT ? KIND_HARMFUL : KIND_HELPFUL; + StoreFromApplication(unitGuid, spellId, caster, expirationMs, durationMs, + slot, KindForSlot(slot)); } // Bump the player-stat-inputs signal when an aura change hits the LOCAL @@ -790,7 +910,7 @@ OnAuraAdded_t g_origOnAuraAdded = nullptr; void __fastcall OnAuraAdded_h(void *unit, void *edx, uint32_t slot, uint32_t spellId) { g_origOnAuraAdded(unit, edx, slot, spellId); - StampApplication(unit, spellId, KindForSlot(static_cast(slot))); + StampApplication(unit, spellId, static_cast(slot)); NotifyIfPlayer(unit); } @@ -810,8 +930,7 @@ void __fastcall OnAuraStacksChanged_h(void *unit, void *edx, int slot, g_origOnAuraStacksChanged(unit, edx, slot, stackCount); StampApplication( unit, - Aura::Data::ReadSpellID(static_cast(unit), slot), - KindForSlot(slot)); + Aura::Data::ReadSpellID(static_cast(unit), slot), slot); NotifyIfPlayer(unit); } @@ -840,8 +959,7 @@ OnAuraRemoved_t g_origOnAuraRemoved = nullptr; void __fastcall OnAuraRemoved_h(void *unit, void *edx, uint32_t slot, uint32_t spellId) { g_origOnAuraRemoved(unit, edx, slot, spellId); - (void)slot; - Evict(Unit::Identity::GuidForObject(unit), spellId); + Evict(Unit::Identity::GuidForObject(unit), spellId, static_cast(slot)); NotifyIfPlayer(unit); } @@ -851,19 +969,25 @@ const Game::HookAutoRegister _hookAuraRemoved{ } // namespace -bool Get(uint64_t unitGuid, uint32_t spellId, uint64_t *outCaster, +bool Get(uint64_t unitGuid, uint32_t spellId, int slot, uint64_t *outCaster, uint32_t *outExpirationMs, uint32_t *outDurationMs) { if (unitGuid == 0 || spellId == 0) return false; - for (const auto &e : g_cache) { - if (e.used && e.targetGuid == unitGuid && e.spellId == spellId) { - *outCaster = e.casterGuid; - *outExpirationMs = e.expirationMs; - *outDurationMs = e.durationMs; - return true; - } - } - return false; + // Slot first: the only identity that separates two casters' copies of one + // spell. Then the sole entry, which is every single-caster case and the + // whole of the slotless (out-of-range group array, cache fallback) path. + // Several entries and no binding is a genuine ambiguity — reporting the + // miss leaves the caller its unknown-caster defaults instead of one + // caster's timer on the other's aura. + const Entry *e = FindBySlot(unitGuid, spellId, slot); + if (e == nullptr) + e = FindSole(unitGuid, spellId); + if (e == nullptr) + return false; + *outCaster = e->casterGuid; + *outExpirationMs = e->expirationMs; + *outDurationMs = e->durationMs; + return true; } bool AddDurationMod(uint32_t triggerSpellId, uint32_t affectedFamily, @@ -908,9 +1032,17 @@ uint32_t RefreshDurationByFamily(uint64_t unitGuid, uint32_t family, return 0; } -void EvictAbsent(uint64_t unitGuid, const uint32_t *presentSpellIds, int count) { - if (unitGuid == 0 || presentSpellIds == nullptr || count <= 0) +void EvictAbsent(uint64_t unitGuid, const uint32_t *slotSpellIds) { + if (unitGuid == 0 || slotSpellIds == nullptr) return; + bool anyPresent = false; + for (int s = 0; s < Offsets::UNIT_AURA_TOTAL; ++s) + if (slotSpellIds[s] != 0) { + anyPresent = true; + break; + } + if (!anyPresent) + return; // out of range vs genuinely buffless — see the header const uint32_t now = NowMs(); for (auto &e : g_cache) { if (!e.used || e.targetGuid != unitGuid) @@ -920,11 +1052,18 @@ void EvictAbsent(uint64_t unitGuid, const uint32_t *presentSpellIds, int count) if (now - e.stampMs < kEvictGraceMs) continue; bool present = false; - for (int i = 0; i < count; ++i) - if (presentSpellIds[i] == e.spellId) { - present = true; - break; - } + if (e.slot != SLOT_UNBOUND) { + // A bound entry is only backed by the slot it was seated in, so a + // slot now holding a different spell (or nothing) leaves it stale + // even when another caster's copy keeps the spell ID on the unit. + present = slotSpellIds[e.slot] == e.spellId; + } else { + for (int s = 0; s < Offsets::UNIT_AURA_TOTAL; ++s) + if (slotSpellIds[s] == e.spellId) { + present = true; + break; + } + } if (!present) e.used = false; } @@ -943,7 +1082,8 @@ int Enumerate(uint64_t unitGuid, bool harmful, CachedAura *out, int maxOut) { continue; if (e.expirationMs != 0 && now >= e.expirationMs) continue; // expired (infinite-duration entries pass) - out[n++] = {e.spellId, e.casterGuid, e.expirationMs, e.durationMs}; + out[n++] = {e.spellId, e.casterGuid, e.expirationMs, e.durationMs, + e.slot}; } return n; } diff --git a/src/aura/Source.h b/src/aura/Source.h index eb2d4ab..43c547d 100644 --- a/src/aura/Source.h +++ b/src/aura/Source.h @@ -19,7 +19,15 @@ // the client ever sees the caster + a server-authoritative duration is the // `SMSG_SPELL_GO` packet at cast time. We parse it (the same packet // nampower parses for its `AURA_CAST_ON_*` events) and remember, per -// `(targetGuid, spellId)`, who cast it and when it should expire. +// `(targetGuid, spellId, casterGuid)`, who cast it and when it should expire. +// +// The caster belongs in that key because it belongs in the server's: two +// same-class raiders' Corruption on one boss are two auras in two descriptor +// slots, and a `(target, spell)` cache holds one entry for both — the second +// cast overwrites the first caster's timing, and either copy falling off +// evicts the survivor's entry too. The descriptor stores only spell IDs, so +// each entry additionally records the slot its aura was seated in +// (`OnAuraAdded`), which is what lets a per-slot query name the right instance. // // `Aura::Data::Push` consults this to fill `sourceUnit` (caster, resolved // to a unit token) and `expirationTime` for non-player units. The cache is @@ -31,15 +39,25 @@ namespace Aura::Source { -// Looks up the cached caster + timing for the aura `spellId` currently on -// the unit identified by `unitGuid`. Returns true and fills the out params -// on a hit. `*outExpirationMs` is an absolute `GetTickCount`-epoch timestamp -// (0 = unknown / infinite-duration aura); `*outDurationMs` is the applied -// duration including the caster's modifiers (talents etc.; 0 = none) — use -// it for the `duration` field so it stays consistent with `expirationTime`; -// `*outCaster` is the caster's 64-bit GUID (never 0 on a hit). Returns false -// on a miss or for zero inputs. -bool Get(uint64_t unitGuid, uint32_t spellId, uint64_t *outCaster, +// `slot` value for a query with no descriptor slot to go on (out-of-range +// group array, cache fallback) and for an entry not yet seated in one. +constexpr int SLOT_UNBOUND = -1; + +// Looks up the cached caster + timing for the aura `spellId` occupying +// absolute descriptor `slot` on the unit identified by `unitGuid`. Returns +// true and fills the out params on a hit. `*outExpirationMs` is an absolute +// `GetTickCount`-epoch timestamp (0 = unknown / infinite-duration aura); +// `*outDurationMs` is the applied duration including the caster's modifiers +// (talents etc.; 0 = none) — use it for the `duration` field so it stays +// consistent with `expirationTime`; `*outCaster` is the caster's 64-bit GUID +// (0 when the aura was seen applied but its cast never observed). Returns +// false on a miss or for zero inputs. +// +// Pass `SLOT_UNBOUND` when the caller has no slot. Resolution is by slot +// first, then by spell ID when the unit carries only one entry for it; two +// casters' entries with no slot binding resolve to a miss rather than to a +// coin flip between them. +bool Get(uint64_t unitGuid, uint32_t spellId, int slot, uint64_t *outCaster, uint32_t *outExpirationMs, uint32_t *outDurationMs); // Refreshes `casterGuid`'s aura on `unitGuid` matching the same selector the @@ -76,19 +94,27 @@ struct CachedAura { uint64_t casterGuid; // 0 = caster unknown (application hook, no SpellGo) uint32_t expirationMs; // 0 = infinite / unknown uint32_t durationMs; // applied duration (incl. caster mods); 0 = none + int slot; // descriptor slot the aura was seated in; feed it + // back to `Get` to reach this exact entry again }; -// Evicts every cached entry for `unitGuid` whose spellId is NOT present in -// `presentSpellIds[0..count)`. Used to reconcile the cache against a unit's -// authoritative descriptor when it is back in view (a populated aura array -// means the unit is fully synced): an entry the descriptor no longer lists -// was removed while we couldn't observe it — e.g. a buff the owner cancelled -// while out of our range, whose `OnAuraRemoved` we never received — so drop -// it before the descriptor-drop fallback resurfaces it as a phantom. The -// caller must only invoke this when the descriptor is populated (count > 0); -// an empty array can't distinguish "out of range" from "genuinely buffless", -// so reconciling then would wrongly wipe still-valid out-of-range entries. -void EvictAbsent(uint64_t unitGuid, const uint32_t *presentSpellIds, int count); +// Evicts every cached entry for `unitGuid` the unit's descriptor contradicts. +// `slotSpellIds` is the raw `UNIT_FIELD_AURA` array — exactly +// `Offsets::UNIT_AURA_TOTAL` spell IDs indexed by absolute slot, 0 for empty. +// Used to reconcile the cache against a unit's authoritative descriptor when +// it is back in view (a populated aura array means the unit is fully synced): +// an entry the descriptor no longer backs was removed while we couldn't +// observe it — e.g. a buff the owner cancelled while out of our range, whose +// `OnAuraRemoved` we never received — so drop it before the descriptor-drop +// fallback resurfaces it as a phantom. +// +// An entry seated in a slot is checked against THAT slot, so one caster's +// copy going away retires its own entry even while another caster keeps the +// spell ID present on the unit. Entries never seated are checked against the +// whole array. An all-empty array is ignored: it can't distinguish "out of +// range" from "genuinely buffless", and reconciling then would wrongly wipe +// still-valid out-of-range entries. +void EvictAbsent(uint64_t unitGuid, const uint32_t *slotSpellIds); // Fills `out` with up to `maxOut` cached, non-expired auras on `unitGuid` // whose helpful/harmful classification matches `harmful`. Returns the count diff --git a/src/lossofcontrol/Info.cpp b/src/lossofcontrol/Info.cpp index 82d5f56..8c2e110 100644 --- a/src/lossofcontrol/Info.cpp +++ b/src/lossofcontrol/Info.cpp @@ -202,7 +202,8 @@ int BuildList(LocEntry *out, int maxOut) { e.endMs = 0; uint64_t caster = 0; uint32_t expMs = 0, durMs = 0; - if (Aura::Source::Get(playerGuid, spellID, &caster, &expMs, &durMs) && + if (Aura::Source::Get(playerGuid, spellID, slot, &caster, &expMs, + &durMs) && expMs != 0) { e.endMs = expMs; if (durMs != 0) From a2c8a9a0cab3e88bc9e776a9ddafde06c5f80757 Mon Sep 17 00:00:00 2001 From: Brues <5278969+brues-code@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:10:22 -0500 Subject: [PATCH 2/5] fix(aura): compare unbound-entry freshness wrap-safely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FindFreshestUnbound picked the newest unbound capture with a raw `stampMs >` on two absolute engine ticks. That compare inverts across the 2^32 ms tick wrap (~49.7 days uptime, or the rdtsc backend already past 2^31 after a warm reboot), so the OLDER capture would win and OnAuraAdded would seat the descriptor slot onto the wrong cast — mis-attributing that aura's caster and duration. Pick by smallest Time::Clock::Elapsed(stamp, now) instead, the wrap-safe signed difference the rest of the codebase uses for tick math. --- src/aura/Source.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/aura/Source.cpp b/src/aura/Source.cpp index 7b5b0e0..8d2e48d 100644 --- a/src/aura/Source.cpp +++ b/src/aura/Source.cpp @@ -229,13 +229,22 @@ Entry *FindBySlot(uint64_t targetGuid, uint32_t spellId, int slot) { // what an `OnAuraAdded` for `(target, spell)` is seating. Newest wins because // SpellGo precedes the application by a packet, so the freshest unbound entry // is the cast that just landed; an older one is a capture that never seated -// (fully resisted, immune) and is left to expire on its own. -Entry *FindFreshestUnbound(uint64_t targetGuid, uint32_t spellId) { +// (fully resisted, immune) and is left to expire on its own. "Newest" is the +// smallest `Time::Clock::Elapsed(stamp, now)` — a wrap-safe signed difference; +// a raw `stampMs >` compare inverts across the 2^32 ms tick wrap. +Entry *FindFreshestUnbound(uint64_t targetGuid, uint32_t spellId, uint32_t now) { Entry *best = nullptr; - for (auto &e : g_cache) - if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && - e.slot == SLOT_UNBOUND && (best == nullptr || e.stampMs > best->stampMs)) + uint32_t bestElapsed = 0; + for (auto &e : g_cache) { + if (!e.used || e.targetGuid != targetGuid || e.spellId != spellId || + e.slot != SLOT_UNBOUND) + continue; + const uint32_t elapsed = Time::Clock::Elapsed(e.stampMs, now); + if (best == nullptr || elapsed < bestElapsed) { best = &e; + bestElapsed = elapsed; + } + } return best; } @@ -349,7 +358,7 @@ void StoreFromApplication(uint64_t targetGuid, uint32_t spellId, Entry *e = FindBySlot(targetGuid, spellId, slot); if (e == nullptr) - e = FindFreshestUnbound(targetGuid, spellId); + e = FindFreshestUnbound(targetGuid, spellId, now); if (e == nullptr && slot < 0) e = FindSole(targetGuid, spellId); if (e == nullptr) { From 7a1a6c8d4b7b38cc8c84507a6cd9b145e869e28c Mon Sep 17 00:00:00 2001 From: Brues <5278969+brues-code@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:10:32 -0500 Subject: [PATCH 3/5] fix(aura): keep the observed caster on the aura fallback paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache-fallback push paths (the descriptor-drop fallback and the out-of-range group-member fallback) already hold each aura instance's caster and timing — Enumerate copies them straight out of the cache entry. But PushEnriched then re-resolved them through Get(guid, spell, slot), which returns a miss for an unbound entry that shares its spell with another caster's entry. So an aura Enumerate had fully attributed came back out with no sourceUnit/sourceGUID and no expirationTime. Hand the entry's own attribution to PushEnriched as `known` so the fallback reports what the cache already knew instead of re-deriving it. --- src/aura/Data.cpp | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/aura/Data.cpp b/src/aura/Data.cpp index 565378b..8dc4e70 100644 --- a/src/aura/Data.cpp +++ b/src/aura/Data.cpp @@ -536,9 +536,17 @@ static void BuildTable(void *L, uint32_t spellID, int applications, // the aura occupies, which is what attributes it to the right caster when two // of them hold the same spell on the unit; `Aura::Source::SLOT_UNBOUND` for the // paths with no descriptor to read one from. +// +// `known` short-circuits the by-(guid,spell,slot) resolve: the cache-fallback +// paths already hold the exact entry's attribution (Enumerate copied it out), +// so they pass it straight through. Re-resolving there would call `Get` a second +// time and, for an unbound entry sharing its spell with another caster's entry, +// return a miss — dropping a caster Enumerate already knew. Null for the +// descriptor / group-array paths, which have no per-entry attribution in hand. static void PushEnriched(void *L, uint64_t guid, uint32_t spellID, bool isHelpful, int applications, int unitLevel, - bool isPlayer, int slot) { + bool isPlayer, int slot, + const Attribution *known = nullptr) { double duration = 0.0; double expirationTime = 0.0; uint64_t casterGuid = 0; @@ -553,7 +561,12 @@ static void PushEnriched(void *L, uint64_t guid, uint32_t spellID, // Caster (sourceGUID/sourceUnit) and timing resolved together, so a // self-buff both shows a source and matches HELPFUL|PLAYER instead of // the two paths disagreeing. - const Attribution a = Attribute(guid, spellID, slot); + Attribution a = (known != nullptr) ? *known : Attribute(guid, spellID, slot); + // Self-buff inference for a pre-resolved entry whose caster the cache + // never learned (idempotent on the Attribute() path, which already + // applied it — the caster is nonzero there when it matched). + if (a.caster == 0 && Spell::IsSelfBuff::IsSelfBuff(spellID)) + a.caster = guid; // A cached expiration that already elapsed while the aura is still // present (non-player casters get an underestimated base duration) // is not meaningful — report unknown (0) rather than a negative @@ -569,6 +582,12 @@ static void PushEnriched(void *L, uint64_t guid, uint32_t spellID, casterGuid); } +// Attribution held directly by an `Aura::Source` cache entry (from `Enumerate`), +// for the cache-fallback push paths to hand to `PushEnriched` as `known`. +static Attribution CachedAttribution(const Aura::Source::CachedAura &c) { + return {c.casterGuid, c.expirationMs, c.durationMs}; +} + void Push(void *L, const uint8_t *unit, int slot) { const uint32_t spellID = ReadSpellID(unit, slot); const bool isHelpful = slot < Offsets::UNIT_AURA_BUFF_COUNT; @@ -585,13 +604,15 @@ namespace { // applied (caster-modified) ms when known, else the Spell.dbc base. void PushFromCache(void *L, const uint8_t *unit, const Aura::Source::CachedAura &c, bool isHelpful) { - // `PushEnriched` re-reads the same cache entry `c` came from — passing - // `c.slot` back is what makes it land on that exact entry rather than on - // another caster's copy of the spell. Stacks aren't in SMSG_SPELL_GO, so - // `applications` is 1. + // `c` already carries this exact instance's caster + timing (Enumerate + // copied it out), so hand it to `PushEnriched` as `known` rather than + // re-resolving by (guid, spell, slot) — an unbound entry sharing its spell + // with another caster's would otherwise resolve to a miss and drop the + // caster. Stacks aren't in SMSG_SPELL_GO, so `applications` is 1. const int unitLevel = (unit != nullptr) ? PlayerLevel(unit) : 0; + const Attribution a = CachedAttribution(c); PushEnriched(L, UnitGuid(unit), c.spellId, isHelpful, 1, unitLevel, false, - c.slot); + c.slot, &a); } // Reconciles the `Aura::Source` cache against `unit`'s descriptor: when the @@ -816,8 +837,9 @@ bool PushNthGroupCacheFallback(void *L, uint64_t guid, const uint16_t *arr, if (!GroupFallbackEligible(arr, buf[i], match)) continue; if (++seen == oneBasedIndex) { + const Attribution a = CachedAttribution(buf[i]); PushEnriched(L, guid, buf[i].spellId, filter == Filter::Helpful, 1, - GroupMemberLevel(guid), false, buf[i].slot); + GroupMemberLevel(guid), false, buf[i].slot, &a); return true; } } @@ -836,8 +858,9 @@ void AppendGroupCacheFallbacks(void *L, uint64_t guid, const uint16_t *arr, if (!GroupFallbackEligible(arr, buf[i], match)) continue; Game::Lua::PushNumber(L, static_cast(nextKey++)); + const Attribution a = CachedAttribution(buf[i]); PushEnriched(L, guid, buf[i].spellId, filter == Filter::Helpful, 1, level, - false, buf[i].slot); + false, buf[i].slot, &a); Game::Lua::SetTable(L, outerIdx); } } @@ -859,8 +882,9 @@ bool PushGroupCacheFallbackMatch(void *L, uint64_t guid, const uint16_t *arr, } if (!GroupFallbackEligible(arr, buf[i], match)) continue; + const Attribution a = CachedAttribution(buf[i]); PushEnriched(L, guid, buf[i].spellId, !harmful, 1, GroupMemberLevel(guid), - false, buf[i].slot); + false, buf[i].slot, &a); return true; } return false; From fe06eb18a8d5fde172532a6f118930f3a5cd2dfb Mon Sep 17 00:00:00 2001 From: Brues <5278969+brues-code@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:14:53 -0500 Subject: [PATCH 4/5] refactor(aura): route Get and Evict through one FindInstance resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Get and Evict each hand-rolled the same FindBySlot-then-FindSole ladder — the read-side rule for which instance a slot-bearing query names. Naming it once keeps the two from drifting into resolving the same query to different instances (reporting one caster while evicting another). No behavior change. --- src/aura/Source.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/aura/Source.cpp b/src/aura/Source.cpp index 8d2e48d..972a2e0 100644 --- a/src/aura/Source.cpp +++ b/src/aura/Source.cpp @@ -264,6 +264,17 @@ Entry *FindSole(uint64_t targetGuid, uint32_t spellId) { return found; } +// The instance a slot-bearing query means: the entry seated in `slot`, else the +// unit's sole entry for the spell — every single-caster case and the slotless +// (out-of-range group array, cache fallback) paths. Two casters' entries with +// no slot binding resolve to null (see FindSole), never to a coin flip. This is +// the read-side identity rule; both `Get` and `Evict` resolve through it so they +// can't drift into naming different instances for the same query. +Entry *FindInstance(uint64_t targetGuid, uint32_t spellId, int slot) { + Entry *e = FindBySlot(targetGuid, spellId, slot); + return e != nullptr ? e : FindSole(targetGuid, spellId); +} + // Takes a free slot, else an expired one whose aura the descriptor no longer // lists (a still-present aura keeps its slot so its caster isn't lost — see // DescriptorListsAura), else evicts. @@ -437,9 +448,7 @@ void StampGroupGuess(uint64_t guid, uint16_t spellId, int8_t kind, uint32_t now) void Evict(uint64_t targetGuid, uint32_t spellId, int slot) { if (targetGuid == 0 || spellId == 0) return; - Entry *e = FindBySlot(targetGuid, spellId, slot); - if (e == nullptr) - e = FindSole(targetGuid, spellId); + Entry *e = FindInstance(targetGuid, spellId, slot); if (e != nullptr) e->used = false; } @@ -982,15 +991,11 @@ bool Get(uint64_t unitGuid, uint32_t spellId, int slot, uint64_t *outCaster, uint32_t *outExpirationMs, uint32_t *outDurationMs) { if (unitGuid == 0 || spellId == 0) return false; - // Slot first: the only identity that separates two casters' copies of one - // spell. Then the sole entry, which is every single-caster case and the - // whole of the slotless (out-of-range group array, cache fallback) path. - // Several entries and no binding is a genuine ambiguity — reporting the - // miss leaves the caller its unknown-caster defaults instead of one - // caster's timer on the other's aura. - const Entry *e = FindBySlot(unitGuid, spellId, slot); - if (e == nullptr) - e = FindSole(unitGuid, spellId); + // Slot first, then the sole entry (see FindInstance): several entries and no + // slot binding is a genuine ambiguity, so reporting the miss leaves the + // caller its unknown-caster defaults instead of one caster's timer on the + // other's aura. + const Entry *e = FindInstance(unitGuid, spellId, slot); if (e == nullptr) return false; *outCaster = e->casterGuid; From e806c0923428a113eaf9772a2e7cdcd32707828a Mon Sep 17 00:00:00 2001 From: Brues <5278969+brues-code@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:27:42 -0500 Subject: [PATCH 5/5] perf(aura): bound cache scans to the active high-water prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Aura::Source lookup linear-scanned all 2048 cache slots. Get runs per-aura per-query — hundreds of times a frame when addons poll UnitAura in a raid — so the full scan dominated the read path. Track a high-water mark that Claim (the sole allocator) extends and FlushAll resets, and scan only [0, g_usedHigh). Every used entry lives below the mark, so no lookup can miss one. Outside a raid the working set is a handful of auras, cutting a ~2048-entry scan to ~30; in a raid it drops to the real working set. Claim still scans the full array — as the allocator it must hand out slots beyond the current prefix. --- src/aura/Source.cpp | 63 +++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/src/aura/Source.cpp b/src/aura/Source.cpp index 972a2e0..4423cb1 100644 --- a/src/aura/Source.cpp +++ b/src/aura/Source.cpp @@ -161,6 +161,15 @@ struct Entry { constexpr int kCacheSize = 2048; Entry g_cache[kCacheSize]; +// One past the highest slot ever claimed since the last flush — the active +// prefix `[0, g_usedHigh)`. Every used entry lives below it (Claim is the sole +// allocator and extends it; FlushAll resets it), so all lookups scan only this +// prefix instead of the full 2048. Outside a raid the working set is a handful +// of auras, so this is the difference between a ~30-entry scan and a 2048-entry +// one on the per-aura query path. Never lowered on eviction — an over-estimate +// only costs a few extra skipped (!used) slots, never a missed live entry. +int g_usedHigh = 0; + // SMSG_SPELL_GO arrives before the SMSG_UPDATE_OBJECT that adds the aura to // the target's descriptor, so for a brief window a just-captured entry names // an aura the descriptor doesn't list yet. `EvictAbsent` (run from a query's @@ -207,10 +216,12 @@ bool DescriptorListsAura(uint64_t guid, uint32_t spellId) { // The exact aura instance `caster` has on `targetGuid` — the server's own // identity for one aura, and what a repeat cast refreshes. Entry *FindByCaster(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid) { - for (auto &e : g_cache) + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && e.casterGuid == casterGuid) return &e; + } return nullptr; } @@ -218,10 +229,12 @@ Entry *FindByCaster(uint64_t targetGuid, uint32_t spellId, uint64_t casterGuid) Entry *FindBySlot(uint64_t targetGuid, uint32_t spellId, int slot) { if (slot < 0) return nullptr; - for (auto &e : g_cache) + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (e.used && e.targetGuid == targetGuid && e.spellId == spellId && e.slot == slot) return &e; + } return nullptr; } @@ -235,7 +248,8 @@ Entry *FindBySlot(uint64_t targetGuid, uint32_t spellId, int slot) { Entry *FindFreshestUnbound(uint64_t targetGuid, uint32_t spellId, uint32_t now) { Entry *best = nullptr; uint32_t bestElapsed = 0; - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.targetGuid != targetGuid || e.spellId != spellId || e.slot != SLOT_UNBOUND) continue; @@ -254,7 +268,8 @@ Entry *FindFreshestUnbound(uint64_t targetGuid, uint32_t spellId, uint32_t now) // caster and a wrong timer. Entry *FindSole(uint64_t targetGuid, uint32_t spellId) { Entry *found = nullptr; - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.targetGuid != targetGuid || e.spellId != spellId) continue; if (found != nullptr) @@ -275,15 +290,27 @@ Entry *FindInstance(uint64_t targetGuid, uint32_t spellId, int slot) { return e != nullptr ? e : FindSole(targetGuid, spellId); } +// Marks the slot `e` occupies as within the active prefix, so later lookups +// reach it. Every new entry passes through here (Claim is the only allocator), +// which is what keeps the `[0, g_usedHigh)` invariant true for all writers. +Entry *Register(Entry *e) { + const int idx = static_cast(e - g_cache); + if (idx >= g_usedHigh) + g_usedHigh = idx + 1; + return e; +} + // Takes a free slot, else an expired one whose aura the descriptor no longer // lists (a still-present aura keeps its slot so its caster isn't lost — see -// DescriptorListsAura), else evicts. +// DescriptorListsAura), else evicts. Scans the full array — as the allocator it +// must be able to hand out slots beyond the current active prefix (that is how +// the prefix grows), so it can't bound itself by g_usedHigh. Entry *Claim(uint32_t now) { for (auto &e : g_cache) { if (!e.used || (e.expirationMs != 0 && now >= e.expirationMs && !DescriptorListsAura(e.targetGuid, e.spellId))) - return &e; + return Register(&e); } // Saturated. Honor the same invariant as the tick sweep: an entry whose // aura is still present on a resolvable unit is NEVER evicted — its caster @@ -308,7 +335,7 @@ Entry *Claim(uint32_t now) { if (orphan == nullptr || e.stampMs < orphan->stampMs) orphan = &e; } - return (orphan != nullptr) ? orphan : lru; + return Register((orphan != nullptr) ? orphan : lru); } // ---- Writes -------------------------------------------------------------- @@ -608,7 +635,8 @@ void ApplyDurationModifiers(uint32_t triggerSpellId, uint64_t caster, if (!TriggerMatches(m, triggerSpellId, triggerRec)) continue; for (int t = 0; t < numTargets; ++t) { - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.targetGuid != targets[t]) continue; // The mechanic acts on the trigger-caster's own aura. Cast-applied @@ -716,8 +744,9 @@ const Game::ModuleAutoRegister _autoregDurationMod{&RegisterDurationModLua}; // Wipe the whole cache. Used on a map transition (see OnWorldTick). void FlushAll() { - for (auto &e : g_cache) - e.used = false; + for (int i = 0; i < g_usedHigh; ++i) + g_cache[i].used = false; + g_usedHigh = 0; // active prefix is empty again // Reset transition baselines too: post-transition group auras re-sync and // should be treated as first-sight (unknown age), not diffed as new. for (auto &s : g_groupSnaps) @@ -753,7 +782,8 @@ void OnWorldTick() { } const uint32_t now = NowMs(); - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.expirationMs == 0 || !Time::Clock::Reached(now, e.expirationMs)) continue; // A timer elapse doesn't prove the aura is gone — expirationMs is only @@ -1019,7 +1049,8 @@ uint32_t RefreshDurationByFamily(uint64_t unitGuid, uint32_t family, if (unitGuid == 0 || mask == 0 || casterGuid == 0) return 0; const uint32_t now = NowMs(); - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.targetGuid != unitGuid) continue; // Caller's own aura only, scoped as ApplyDurationModifiers scopes a @@ -1058,7 +1089,8 @@ void EvictAbsent(uint64_t unitGuid, const uint32_t *slotSpellIds) { if (!anyPresent) return; // out of range vs genuinely buffless — see the header const uint32_t now = NowMs(); - for (auto &e : g_cache) { + for (int i = 0; i < g_usedHigh; ++i) { + Entry &e = g_cache[i]; if (!e.used || e.targetGuid != unitGuid) continue; // Fresh SpellGo capture the descriptor hasn't synced yet — see @@ -1089,9 +1121,8 @@ int Enumerate(uint64_t unitGuid, bool harmful, CachedAura *out, int maxOut) { const int8_t want = harmful ? KIND_HARMFUL : KIND_HELPFUL; const uint32_t now = NowMs(); int n = 0; - for (const auto &e : g_cache) { - if (n >= maxOut) - break; + for (int i = 0; i < g_usedHigh && n < maxOut; ++i) { + const Entry &e = g_cache[i]; if (!e.used || e.targetGuid != unitGuid || e.kind != want) continue; if (e.expirationMs != 0 && now >= e.expirationMs)