Skip to content

fix: close the remaining dual-faction paths from #166 #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
FrancescoBorzi merged 3 commits into from
Jul 2, 2026
Merged

fix: close the remaining dual-faction paths from #166 #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f23f340
fix: drop the fake-store record on WG wartime logout instead of leaki…
FrancescoBorzi Jul 2, 2026
003447b
fix: remove charms before unfaking so charm end cannot restore the fa…
FrancescoBorzi Jul 2, 2026
7fe3759
fix: verify the faction template for native players in EnforceBGTeamC…
FrancescoBorzi Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/CFBG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,18 @@ void CFBG::EnforceBGTeamConsistency(Player* player)
{
if (IsPlayerFake(player))
ClearFakePlayer(player);

// A foreign template can outlive its writer on a non-faked native
// (e.g. a charm undone after an unfake). Repair it unless a legitimate
// writer owns it: GM-on holds FACTION_FRIENDLY; an active MOD_FACTION
// aura restores natives correctly on removal.
if (!player->IsGameMaster() && !player->HasAuraType(SPELL_AURA_MOD_FACTION))
{
ChrRacesEntry const* raceEntry = sChrRacesStore.LookupEntry(player->getRace(true));
if (raceEntry && player->GetFaction() != raceEntry->FactionID)
SetFactionForRace(player, player->getRace(true), player->GetTeamId(true));
Comment on lines +335 to +339

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Treat active charm as a legitimate faction writer here.

ClearFakePlayer now documents charm as owning/restoring faction state, but this repair path only skips GM and SPELL_AURA_MOD_FACTION. If a native player is actively charmed, this can overwrite the charm-owned faction before the charm ends; skip this repair while player->IsCharmed().

Proposed fix 
-        if (!player->IsGameMaster() && !player->HasAuraType(SPELL_AURA_MOD_FACTION))
+        if (!player->IsGameMaster() && !player->IsCharmed() && !player->HasAuraType(SPELL_AURA_MOD_FACTION))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!player->IsGameMaster() && !player->HasAuraType(SPELL_AURA_MOD_FACTION))
{
ChrRacesEntry const* raceEntry = sChrRacesStore.LookupEntry(player->getRace(true));
if (raceEntry && player->GetFaction() != raceEntry->FactionID)
SetFactionForRace(player, player->getRace(true), player->GetTeamId(true));
if (!player->IsGameMaster() && !player->IsCharmed() && !player->HasAuraType(SPELL_AURA_MOD_FACTION))
{
ChrRacesEntry const* raceEntry = sChrRacesStore.LookupEntry(player->getRace(true));
if (raceEntry && player->GetFaction() != raceEntry->FactionID)
SetFactionForRace(player, player->getRace(true), player->GetTeamId(true));
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/CFBG.cpp` around lines 335 - 339, The faction repair in
CFBG::SetFactionForRace should also respect active charm state, not just Game
Master or SPELL_AURA_MOD_FACTION. Update the guard so that when
player->IsCharmed() is true, this recovery path is skipped and does not
overwrite the faction currently controlled by the charm. Keep the existing
race/faction comparison logic intact, but add the charm check alongside the
existing IsGameMaster and aura condition near the SetFactionForRace call site.

}

return;
}

Expand Down Expand Up @@ -536,6 +548,13 @@ void CFBG::ClearFakePlayer(Player* player)
if (!IsPlayerFake(player))
return;

// Unwind any charm ON the player while the fake record still exists:
// Unit::RemoveCharmedBy blindly restores the faction snapshotted at charm
// time (the fake one), which the restore below then overwrites. A charm
// ending after the unfake would re-plant the fake template (#166).
if (player->IsCharmed())
player->RemoveCharmAuras();

player->setRace(_fakePlayerStore[player].RealRace);
player->SetDisplayId(_fakePlayerStore[player].RealMorph);
player->SetNativeDisplayId(_fakePlayerStore[player].RealNativeMorph);
Expand All @@ -548,6 +567,13 @@ void CFBG::ClearFakePlayer(Player* player)
_fakePlayerStore.erase(player);
}

// Erase-only: no race/morph/faction/m_team restore. Used at wartime WG logout,
// where Player::RemoveFromWorld still erases PlayersInWar keyed on the fake team.
void CFBG::DropFakePlayerRecord(Player* player)
{
_fakePlayerStore.erase(player);
}

void CFBG::ReapplyFakePlayer(Player* player)
{
FakePlayer const* info = GetFakePlayer(player);
Expand Down
1 change: 1 addition & 0 deletions src/CFBG.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class CFBG
void SetFakeRaceAndMorphForBF(Player* player, TeamId assignedTeam);
void SetFactionForRace(Player* player, uint8 Race, TeamId teamId);
void ClearFakePlayer(Player* player);
void DropFakePlayerRecord(Player* player);
void ReapplyFakePlayer(Player* player);
void DoForgetPlayersInList(Player* player);
void FitPlayerInTeam(Player* player, bool action, Battleground* bg);
Expand Down
22 changes: 15 additions & 7 deletions src/CFBG_SC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,22 @@ class CFBG_Player : public PlayerScript
if (!sCFBG->IsEnableSystem() || !sCFBG->IsPlayerFake(player))
return;

// Only clear the WG fake state when the battlefield is not actively at
// war. During a running war the player may safely relog and rejoin
// their assigned faction, so we leave the fake state intact for that
// case. BG fakes are always cleaned up by OnBattlegroundRemovePlayerAtLeave
// and do not need to be handled here.
// Only WG fakes are handled here; BG fakes are owned by
// OnBattlegroundRemovePlayerAtLeave. Outside a war, fully restore.
// During a running war, only drop the Player*-keyed record: relog
// constructs a new Player*, so the entry can never serve the rejoin
// (Battlefield::TryRejoinAfterLogout re-fakes via the GUID-keyed
// _wgWarAssignmentStore) and would dangle. Restoring race/faction here
// would flip m_team before Player::RemoveFromWorld and mis-key core's
// PlayersInWar erase.
Battlefield* bf = sBattlefieldMgr->GetBattlefieldToZoneId(player->GetZoneId());
if (bf && bf->GetTypeId() == BATTLEFIELD_WG && !bf->IsWarTime())
sCFBG->ClearFakePlayer(player);
if (bf && bf->GetTypeId() == BATTLEFIELD_WG)
{
if (!bf->IsWarTime())
sCFBG->ClearFakePlayer(player);
else
sCFBG->DropFakePlayerRecord(player);
}
}

// Fires after Player::UpdateZone has finished all Battlefield/OutdoorPvP/WorldState
Expand Down
Loading