refactor: key fake-player tracking by ObjectGuid instead of raw Player*#156
Closed
Nyeriah wants to merge 1 commit into
Closed
refactor: key fake-player tracking by ObjectGuid instead of raw Player*#156Nyeriah wants to merge 1 commit into
Nyeriah wants to merge 1 commit into
Conversation
Replaces `unordered_map<Player*, FakePlayer> _fakePlayerStore` with
`unordered_set<ObjectGuid> _fakePlayerGuids`, drops the `FakePlayer`
struct and `GetFakePlayer()` method, and adds a GUID-form
`ClearFakePlayer(ObjectGuid)` overload that resolves the player via
`ObjectAccessor::FindPlayer` and handles both online and offline cleanup.
Why GUID
--------
- Raw Player* keys can survive a logged-out player (e.g. WG logout
during an active war intentionally skips ClearFakePlayer); the next
Player allocation may reuse that address and inherit the stale entry,
contaminating a different character. AzerothCore's project convention
is to store ObjectGuid for any reference that may outlive a single
call/tick; this module's Player*-keyed map was the lone exception.
Why no struct
-------------
- RealRace / RealTeamID derive from `getRace(true)` / `GetTeamId(true)`
(m_realRace survives `setRace`, which only touches m_race).
- RealNativeMorph derives from `ChrRacesEntry::model_m`/`model_f` for
the player's gender -- canonically the native model.
- Fake{Race,Morph,TeamID} were write-once values already on the live
player and never read back outside the debug command.
- RealMorph (DisplayId at fake time) was the only non-derivable field;
it captured transient transformation visuals (bear form, ghost wolf,
transform aura). Restoring that snapshot was fragile -- the aura it
intended to preserve may have expired during the fake -- so the new
code derives the canonical race+gender model on clear. An active
transformation aura will reapply on its next tick.
Side effects
------------
- `OnBattlefieldWarEnd` collapses to one call per GUID and now
correctly drops entries for players who logged out during the war
(previously leaked because the FindPlayer + IsPlayerFake dance
silently skipped offline GUIDs).
- `.cfbg debug` no longer prints a "Fake record" block; the live
"Native" / "Current" lines above it already covered every field the
block printed.
- SQL command help text updated to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Switches the fake-player tracking from
unordered_map<Player*, FakePlayer> _fakePlayerStoretounordered_set<ObjectGuid> _fakePlayerGuids, drops theFakePlayerstruct +GetFakePlayer()method, and adds a GUID-formClearFakePlayer(ObjectGuid)that resolves the player viaObjectAccessor::FindPlayerand handles online + offline cleanup uniformly.Why GUID
Raw
Player*keys can outlive the player they describe. TheOnPlayerLogouthook intentionally skipsClearFakePlayerwhile a WG war is active (so a relog can rejoin without re-faking), but the Player object is still deleted at the end of the logout flow. The entry in_fakePlayerStoresurvives with a dangling pointer key, and the nextnew Player(...)allocation can reuse that address —_fakePlayerStore.contains(newPlayer)then returnstruefor an unrelated character, andClearFakePlayerwould stamp the new player with the old player's stored race/morph/team/faction.ObjectGuid is the project-wide convention for any reference that may outlive a single call/tick (see the project CLAUDE.md "Long-lived references" note); this module's
Player*-keyed map was the lone exception.Why no struct
Six of the seven
FakePlayerfields are derivable on demand:RealRace←getRace(true)(m_realRace survivessetRace, which only modifies m_race perUnit::setRace).RealTeamID←GetTeamId(true)(derived from m_realRace).RealNativeMorph←ChrRacesEntry::model_m/model_ffor the player's gender — by definition the native model for race+gender.Fake{Race,Morph,TeamID}were write-once values already on the live player after fake application; only the dead-codeApplyFakeVisualsForBF(removed in refactor(WG): remove unused PrepareFakeTeamForBF/ApplyFakeVisualsForBF #155) and the debug dump ever read them back.Only
RealMorph(theDisplayIdsnapshot at fake time) is genuinely non-derivable; it captured the player's active transformation visual (bear form, ghost wolf, transform aura). That snapshot was fragile though — the aura it intended to preserve may have expired during the fake — so the new code derives the canonical race+gender model on clear; an active transformation aura will reapply on its next tick.Side effects
OnBattlefieldWarEndcorrectly drops offline entries now. Previously the loop didFindPlayer(guid)and silently skipped offline GUIDs, leaking their entries into_fakePlayerStoreforever. The new GUID-formClearFakePlayererases the set entry whether the player is online or not.IsPlayerFakechecks needed; the GUID form is a no-op for unfaked entries..cfbg debugno longer prints a "Fake record:" block. The block was strictly redundant with the live "Native" / "Current" lines above it (which print real race/team fromgetRace(true)/GetTeamId(true)and current race/team/display from the live player fields). SQL help text updated accordingly.Test plan
-DMODULES=static) — confirm no unresolved symbols.SaveToDBwritesgetRace(true)=m_realRace, not the fakem_race)..cfbg debugstill prints sensible state for both faked and unfaked targets.