josh/ThordekkCore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(session): Add state validation for STATUS_TRANSFER deferred packets

Browse Source 
Prevents assertion failure in Map::RemovePlayerFromMap (Map.cpp:935)
when bot receives CMSG_WORLD_PORT_RESPONSE while in inconsistent state.

Root cause: Deferred packet processing for STATUS_TRANSFER packets was
calling handlers without validating player state. If a bot is IsInWorld()
but NOT IsInGrid(), the handler would trigger the assertion:
  ASSERT(remove) // fails when remove=false and not in grid

The fix adds state validation before processing STATUS_TRANSFER packets:
- Checks player exists
- Validates player is NOT in world (correct state for transfer)
- Logs critical warning if player is in world but not in grid
- Skips packet processing to prevent crash

Crash context: Map 727 (BG), InstanceId 1, Difficulty 0

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
This commit is contained in:
agatho
2026-02-04 20:34:42 -03:00
committed by luis
co-authored by Claude Opus 4.5
parent 9f10b1a1ef
commit c15cea0c81
1 changed files with 54 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/modules/Playerbot/Session/BotSession.cpp
+54 -5
View File
@@ -359,7 +359,7 @@ BotSession::BotSession(uint32 bnetAccountId)
bnetAccountId, // Use battlenet account as account ID for now
std::string(""), // Empty username (generated by Trinity)
bnetAccountId, // BattleNet account ID
std::string(""), // BattleNet account email (new in 11.2.7)
std::string(""), // BattleNet account email (new in 12.0.7)
std::shared_ptr<WorldSocket>(), // No socket (empty shared_ptr for bots)
SEC_PLAYER, // Security level
EXPANSION_LEVEL_CURRENT, // Current expansion
@@ -755,7 +755,7 @@ void BotSession::SendPacket(WorldPacket const* packet, bool forced)
_outgoingPackets.push(::std::move(packetCopy));
}
// TrinityCore 11.2: Override for new WorldSession::QueuePacket signature
// TrinityCore 12.0: Override for new WorldSession::QueuePacket signature
void BotSession::QueuePacket(WorldPacket&& packet)
{
// Store the packet in incoming queue
@@ -1585,7 +1585,7 @@ void BotSession::ProcessBotPackets()
try {
// Process packet through WorldSession's standard queue system
// This is safe to call without locks
// TrinityCore 11.2: QueuePacket now takes WorldPacket&& instead of WorldPacket*
// TrinityCore 12.0: QueuePacket now takes WorldPacket&& instead of WorldPacket*
WorldSession::QueuePacket(std::move(*packet));
}
catch (::std::exception const& e)
@@ -1692,13 +1692,62 @@ uint32 BotSession::ProcessDeferredPackets()
}
case STATUS_LOGGEDIN_OR_RECENTLY_LOGGOUT:
case STATUS_TRANSFER:
case STATUS_AUTHED:
{
opHandle->Call(this, *packet);
break;
}
case STATUS_TRANSFER:
{
// STATUS_TRANSFER packets (like CMSG_WORLD_PORT_RESPONSE) should only be processed
// when the player is in a valid transfer state: BeingTeleportedFar AND NOT in world.
// If player IS in world while BeingTeleportedFar, there's a state inconsistency
// that could cause ASSERT failure in Map::RemovePlayerFromMap (Map.cpp:935)
// when trying to remove from map with player not in grid.
Player* player = GetPlayer();
if (!player)
{
TC_LOG_WARN("playerbot.packets.deferred",
"Bot {} deferred STATUS_TRANSFER packet {} but player is nullptr",
GetPlayerName(), opHandle->Name);
break;
}
// Validate transfer state: Should be teleporting far and NOT in world
// (TeleportTo calls RemoveFromWorld before setting the far teleport semaphore)
if (player->IsInWorld())
{
// State inconsistency detected - player is in world but receiving transfer packet
// This could happen if player was re-added to world between TeleportTo and
// processing this deferred packet. Skip to prevent crash in RemovePlayerFromMap.
TC_LOG_ERROR("playerbot.packets.deferred",
"Bot {} deferred STATUS_TRANSFER packet {} SKIPPED - player in world but "
"receiving transfer packet (IsInWorld={}, IsBeingTeleportedFar={}, Map={})",
GetPlayerName(), opHandle->Name, player->IsInWorld(),
player->IsBeingTeleportedFar(), player->GetMapId());
// If player is in world and grid, they're already on a map - don't process transfer
// If player is in world but NOT in grid, that's a critical bug - log and skip
if (!player->IsInGrid())
{
TC_LOG_ERROR("playerbot.packets.deferred",
"CRITICAL: Bot {} is in world but NOT in grid (Map {}) - skipping "
"STATUS_TRANSFER to prevent assertion failure",
GetPlayerName(), player->GetMapId());
}
break;
}
// Player is NOT in world - safe to process transfer packet
opHandle->Call(this, *packet);
TC_LOG_DEBUG("playerbot.packets.deferred",
" Bot {} executed deferred STATUS_TRANSFER opcode {} on main thread",
GetPlayerName(), opHandle->Name);
break;
}
case STATUS_NEVER:
case STATUS_UNHANDLED:
case STATUS_IGNORED:
@@ -1960,7 +2009,7 @@ void BotSession::HandleBotPlayerLogin(BotLoginQueryHolder const& holder)
pCurrChar->LearnDefaultSkills();
pCurrChar->UpdateSkillsForLevel(); // Ensures skill values match current level
// Verify spell learning - in modern WoW (11.2), ALL combat spells are automatic
// Verify spell learning - in modern WoW (12.0), ALL combat spells are automatic
// TrinityCore's LearnDefaultSkills() + LearnSpecializationSpells() should teach everything
// This call is a safety net that re-calls the native methods in case of timing issues
sBotPostLoginConfigurator->ApplyClassSpells(pCurrChar);