spell/Cast: clear player channel on early stop; unify player-object access

UnitChannelInfo showed a stale "Ritual of Summoning" countdown after
helping a warlock summon: g_channel only cleared at its computed endMs
or when a timed cast superseded it, so a ritual that ends early (when the
summon fills) lingered, and a following instant cast -- which supersedes
neither cast timing nor the channel -- couldn't dislodge it.

OnWorldTick now polls the player's UNIT_FIELD_CHANNEL_SPELL (+0x228) and
clears g_channel when it drops to 0, gated by a confirm latch so the
field's ~1-tick start lag can't wipe a freshly stamped channel. Both
channel-stamp sites route through a new StampChannel helper that re-arms
the latch. Also fixes early-ended channels generally (interrupt / cancel
/ LoS break), which previously self-expired only at endMs.

Cleanup: factor the player-object/descriptor read into shared
Unit::Identity::PlayerObject() / PlayerDescriptor(), now used by
State.cpp, Cast.cpp's poll, and ComboDuration. PlayerObject() uses the
object-manager GUID resolve (FUN_OBJECT_RESOLVE_BY_GUID) -- the fastest
player-object path, non-throwing (null pre-world) -- replacing the
duplicated resolves (State.cpp's token resolver, ComboDuration's
hand-rolled GUID plumbing).

Author: brues-code

src/aura/ComboDuration.cpp

@@ -41,24 +41,12 @@ uint32_t NowMs() {
 // ---- Combo-point snapshot (NetClient send co-hook) ------------------------
 
 // The player's live combo points, read the same way Script_GetComboPoints
-// (0x0051A190) does: the CP byte on the CGPlayer +0xE68 sub-struct. The
-// player is resolved via the non-throwing object resolver (guid comes from
-// the login global; 0 pre-world -> resolver returns null -> 0 CP), so this
-// is safe from any context the send hook can fire in.
+// (0x0051A190) does: the CP byte on the CGPlayer +0xE68 sub-struct. The player
+// comes from the shared Unit::Identity::PlayerObject() (the object-manager GUID
+// resolve, our fastest player-object path — returns null pre-world rather than
+// throwing), so this stays safe from any context the send hook can fire in.
 uint8_t CurrentComboPoints() {
-    const uint64_t guid = Unit::Identity::PlayerGuid();
-    if (guid == 0)
-        return 0;
-    using ResolveByGUID_t = void *(__fastcall *)(uint32_t typeMask,
-                                                 const char *debugName,
-                                                 uint32_t guidLo, uint32_t guidHi,
-                                                 int line);
-    auto resolve =
-        reinterpret_cast<ResolveByGUID_t>(Offsets::FUN_OBJECT_RESOLVE_BY_GUID);
-    auto *player = static_cast<const uint8_t *>(
-        resolve(Offsets::OBJ_TYPE_PLAYER, "ClassicAPI",
-                static_cast<uint32_t>(guid), static_cast<uint32_t>(guid >> 32),
-                0x1029));
+    const uint8_t *player = Unit::Identity::PlayerObject();
     if (player == nullptr)
         return 0;
     const uint8_t *info = *reinterpret_cast<const uint8_t *const *>(

src/spell/Cast.cpp

@@ -115,6 +115,22 @@ TrackedSpell g_channel{0, 0, 0, 0};
 // the WorldTick VAR==0 clear must not touch them.
 bool g_castFromServer = false;
 
+// Armed once the broadcast UNIT_FIELD_CHANNEL_SPELL (+0x228) has been seen to
+// reflect g_channel's spell — the enable for OnWorldTick's channel-stop poll.
+// StampChannel re-arms it (false) on every channel stamp so that field's
+// ~1-tick start lag can't false-clear a channel we just stamped.
+bool g_channelConfirmed = false;
+
+// Stamp the player's channel and re-arm the stop latch. A channel supersedes
+// any in-flight cast. Both channel-stamp sites (SMSG_SPELL_START's instant-
+// channel path and the MSG_CHANNEL_START handler) route through here so
+// neither can forget to reset the latch.
+void StampChannel(int spellID, int startMs, int endMs) {
+    g_channel = TrackedSpell{spellID, startMs, endMs, 0};
+    g_channelConfirmed = false;
+    g_cast.spellID = 0;
+}
+
 int NowMs() { return static_cast<int>(reinterpret_cast<TickMs_t>(Offsets::FUN_OS_TICKCOUNT_MS)()); }
 
 void *Resolve(const char *token) {
@@ -355,10 +371,8 @@ void HandleSpellStart(uint64_t caster, int spellID, uint32_t castTime) {
             // Info) at endMs. (MSG_CHANNEL_START re-stamps moments later with
             // the server's duration — see ChannelStart_h.)
             const int dur = ChannelDurationMs(spellID);
-            if (dur > 0) {
-                g_channel = TrackedSpell{spellID, now, now + dur, 0};
-                g_cast.spellID = 0; // a channel supersedes any cast
-            }
+            if (dur > 0)
+                StampChannel(spellID, now, now + dur);
             return;
         }
         // channel && castTime > 0 → a CAST-THEN-CHANNEL spell (Mind Control:
@@ -474,9 +488,7 @@ int __stdcall ChannelStart_h(uint32_t *opCode, Net::CDataStore *packet) {
         packet->m_read = saved;
         if (spellID != 0 && duration > 0) {
             const int now = NowMs();
-            g_channel =
-                TrackedSpell{spellID, now, now + static_cast<int>(duration), 0};
-            g_cast.spellID = 0; // the channel supersedes the completed cast
+            StampChannel(spellID, now, now + static_cast<int>(duration));
         }
     }
     return g_origChannelStart(opCode, packet);
@@ -615,10 +627,32 @@ void OnWorldTick() {
         *reinterpret_cast<const int *>(Offsets::VAR_CURRENT_CAST_SPELL) == 0)
         g_cast.spellID = 0;
 
-    // Channels are stamped from SMSG_SPELL_START (HandleSpellStart) and expire
-    // on their computed endMs (self-expiry in PushChannelInfo). We don't poll
-    // the broadcast +0x228 field for the player — it lags the channel start by
-    // ~1 tick, so the bar would show ~1s late.
+    // Detect a player CHANNEL that ended before its computed endMs — the
+    // summon-ritual case (UNIT_FIELD_CHANNEL_SPELL = 698, cleared when the
+    // summon fills), and equally an interrupted / cancelled / LoS-broken
+    // channel. Without this, g_channel only clears at endMs (self-expiry in
+    // PushChannelInfo) or when a later TIMED cast supersedes it — so an early
+    // end followed by an INSTANT cast (which supersedes nothing) leaves a
+    // ghost bar counting down to a duration that no longer reflects reality.
+    //
+    // We deliberately don't use +0x228 to START the bar — it's a broadcast
+    // UpdateField that lags the channel start by ~1 tick (HandleSpellStart
+    // stamps from the packet instead). For the STOP that lag is harmless, but a
+    // naive "clear when field == 0" would wipe a channel we just stamped during
+    // the lag window. Guard with a latch: only honor the 0 once the field has
+    // been seen to reflect our channel spell at least once. A channel too short
+    // to ever appear in the field just self-expires at endMs.
+    if (g_channel.spellID != 0) {
+        const uint8_t *desc = Unit::Identity::PlayerDescriptor();
+        if (desc != nullptr) {
+            const int chan = *reinterpret_cast<const int *>(
+                desc + Offsets::OFF_UNIT_FIELD_CHANNEL_SPELL);
+            if (chan == g_channel.spellID)
+                g_channelConfirmed = true;
+            else if (chan == 0 && g_channelConfirmed)
+                g_channel.spellID = 0;
+        }
+    }
 }
 
 static const Tick::WorldTick::AutoSubscribe _tickSub{&OnWorldTick};

src/unit/Identity.cpp

@@ -96,6 +96,37 @@ uint64_t PlayerGuid() {
     return g_charSelectGuid; // early-login fallback (0 until char-select)
 }
 
+const uint8_t *PlayerObject() {
+    // Resolve the player's CGObject straight from the object manager by GUID —
+    // one hash lookup, no token-string parsing, and non-throwing: the resolver
+    // returns null when the GUID isn't loaded (pre-world) instead of raising,
+    // so this is safe from any context (world tick, packet send hook, Lua
+    // callback). The GUID comes from PlayerGuid() — 0 before char-select, which
+    // short-circuits to null here. NOT VAR_LOCAL_PLAYER_PTR: its +0xC0 is only
+    // the GUID, not the canonical CGPlayer whose descriptor is readable.
+    const uint64_t guid = PlayerGuid();
+    if (guid == 0)
+        return nullptr;
+    using ResolveByGuid_t = void *(__fastcall *)(uint32_t typeMask,
+                                                 const char *debugName,
+                                                 uint32_t guidLo, uint32_t guidHi,
+                                                 int line);
+    auto resolve =
+        reinterpret_cast<ResolveByGuid_t>(Offsets::FUN_OBJECT_RESOLVE_BY_GUID);
+    return static_cast<const uint8_t *>(
+        resolve(Offsets::OBJ_TYPE_PLAYER, "ClassicAPI",
+                static_cast<uint32_t>(guid), static_cast<uint32_t>(guid >> 32),
+                /*line*/ 0));
+}
+
+const uint8_t *PlayerDescriptor() {
+    const uint8_t *player = PlayerObject();
+    if (player == nullptr)
+        return nullptr;
+    return *reinterpret_cast<const uint8_t *const *>(
+        player + Offsets::OFF_UNIT_DESCRIPTOR);
+}
+
 uint64_t GuidForObject(const void *unitObject) {
     if (unitObject == nullptr)
         return 0;

src/unit/Identity.h

@@ -28,6 +28,22 @@ namespace Unit::Identity {
 // re-dereferencing the globals inline.
 uint64_t PlayerGuid();
 
+// The local player's in-world object pointer, resolved straight from the
+// object manager by GUID (`FUN_OBJECT_RESOLVE_BY_GUID`) — the fastest path we
+// have (one hash lookup, no token-string work) and naturally non-throwing:
+// null before the player object exists (pre-world) or before char-select,
+// never a raise. Safe from any context — Lua callback, world tick, packet
+// hook. Deliberately NOT `VAR_LOCAL_PLAYER_PTR`: that global carries the
+// player GUID at +0xC0 but is NOT the CGPlayer whose descriptor and
+// CGPlayer-local fields are readable (see Offsets.h).
+const uint8_t *PlayerObject();
+
+// The local player's `m_objectFields` descriptor (`PlayerObject() +
+// OFF_UNIT_DESCRIPTOR`), or null. This is the broadcast UpdateField block
+// (health / power / flags / UNIT_FIELD_CHANNEL_SPELL / …). Prefer this over
+// re-deriving the player object + descriptor offset in each module.
+const uint8_t *PlayerDescriptor();
+
 // The 64-bit GUID of a resolved CGUnit/CGObject pointer, read through the
 // pointer at `+0x08` to an instance block whose first 8 bytes are the GUID
 // (verified in `Script_GetInventoryItemLink`; see `OFF_UNIT_GUID_PTR`).

src/unit/State.cpp

@@ -21,31 +21,18 @@
 #include "Offsets.h"
 #include "aura/Data.h"
 #include "spell/Lookup.h"
+#include "unit/Identity.h"
 
 #include <cstdint>
 
 namespace Unit::State {
 
 namespace {
 
-using ResolveUnitToken_t = void *(__fastcall *)(const char *token);
 using CancelAuraSend_t = void(__fastcall *)(int spellID);
 
-const uint8_t *Player() {
-    auto fn = reinterpret_cast<ResolveUnitToken_t>(Offsets::FUN_RESOLVE_UNIT_TOKEN);
-    return static_cast<const uint8_t *>(fn("player"));
-}
-
-const uint8_t *PlayerDescriptor() {
-    auto *player = Player();
-    if (player == nullptr)
-        return nullptr;
-    return *reinterpret_cast<const uint8_t *const *>(
-        player + Offsets::OFF_UNIT_DESCRIPTOR);
-}
-
 uint32_t PlayerMovementFlags() {
-    auto *player = Player();
+    auto *player = Unit::Identity::PlayerObject();
     if (player == nullptr)
         return 0;
     return *reinterpret_cast<const uint32_t *>(
@@ -63,7 +50,7 @@ uint32_t MountDisplayID(const uint8_t *desc) {
 // `UNIT_FIELD_MOUNTDISPLAYID` from the descriptor; non-zero means
 // the engine is rendering a mount under the player.
 int __fastcall Script_IsMounted(void *L) {
-    auto *desc = PlayerDescriptor();
+    auto *desc = Unit::Identity::PlayerDescriptor();
     Game::Lua::PushBool(L, MountDisplayID(desc) != 0);
     return 1;
 }
@@ -80,11 +67,11 @@ int __fastcall Script_IsMounted(void *L) {
 // No-op if the player isn't mounted (early-outs on the
 // `UNIT_FIELD_MOUNTDISPLAYID` check before walking auras).
 int __fastcall Script_Dismount(void *L) {
-    auto *desc = PlayerDescriptor();
+    auto *desc = Unit::Identity::PlayerDescriptor();
     if (MountDisplayID(desc) == 0)
         return 0;
 
-    auto *player = Player();
+    auto *player = Unit::Identity::PlayerObject();
     if (player == nullptr)
         return 0;
 
@@ -117,7 +104,7 @@ int __fastcall Script_Dismount(void *L) {
 // `OFF_PLAYER_FIELD_VIS_BYTES` in Offsets.h for the full bit map
 // and known limitations (untested for shapeshift forms).
 int __fastcall Script_IsStealthed(void *L) {
-    auto *desc = PlayerDescriptor();
+    auto *desc = Unit::Identity::PlayerDescriptor();
     bool stealthed = false;
     if (desc != nullptr) {
         const uint32_t visBytes = *reinterpret_cast<const uint32_t *>(
@@ -177,12 +164,12 @@ int __fastcall Script_IsSwimming(void *L) {
 // vanilla's summoning ritual has no UI surface, which is what
 // motivates this function.
 int __fastcall Script_IsAssistingRitual(void *L) {
-    auto *player = Player();
+    auto *player = Unit::Identity::PlayerObject();
     if (player == nullptr) {
         Game::Lua::PushBool(L, 0);
         return 1;
     }
-    auto *desc = PlayerDescriptor();
+    auto *desc = Unit::Identity::PlayerDescriptor();
     if (desc == nullptr) {
         Game::Lua::PushBoolean(L, 0);
         return 1;