The StartupSpawnOrchestrator was blocking ALL bot spawning once
startup phases completed (Phase 5 = COMPLETED). This prevented:
- Warm pool bots from logging in for BG queues
- JIT bots from spawning for on-demand content
- Any runtime spawn requests
The orchestrator now checks if the priority queue has items in
COMPLETED phase and allows spawning to proceed. This enables:
- Warm pool bot login: bots already queued for spawn via BotSpawner
- Runtime spawning: new bot requests after startup
- BG queue population: bots can now actually join queues
Evidence from logs showed:
- "BG shortage fully satisfied from warm pool" (bots selected)
- "Queued pool bot Player-1-XXXX for login via BotSpawner"
- But "Phase 2 throttling active - spawn deferred" (blocked!)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
BotAI::~BotAI() was accessing _bot->GetGUID() for blackboard cleanup,
but during destruction _bot may be a dangling pointer to already-freed
memory. This caused ACCESS_VIOLATION crashes (C0000005) during bot
session destruction.
The fix uses _cachedBotGuid (captured in constructor) instead, following
the same safe pattern already used in UnsubscribeFromEventBuses().
Root cause: Race condition where Player object is destroyed before BotAI
destructor completes blackboard cleanup.
Evidence from crash logs:
- "Removed bot blackboard for Player-1-214CB312020" (hex pointer value)
- "AFKSimulator::OnShutdown - Bot AFK simulator shutdown" (empty name)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
BREAKING CHANGE: Renamed WoW112* files to WoW120* variants
Migration changes:
- Rename WoW112CharacterCreation.h to WoW120CharacterCreation.h
- Rename ResourceTypes_WoW112.h to ResourceTypes_WoW120.h
- Rename SpellValidation_WoW112.h to SpellValidation_WoW120.h
- Rename SpellValidation_WoW112_Part2.h to SpellValidation_WoW120_Part2.h
- Rename CombatSpecializationTemplate_WoW112.h to WoW120 variant
- Rename ResourceSystemWoW112.md to WoW120 variant
- Update WoW112Spells namespace to WoW120Spells in all files
- Update all include paths from WoW112 to WoW120
- Update comments referencing "11.2" to "12.0"
- Add Spirit stat to BaseStats (MAX_STATS = 5 in WoW 12.0)
- Update Difficulty enum types from uint8 to int16
Part of WoW 12.0 API migration (TrinityCore commit b0a596908d5c1b5b09f90e97b17a7fc785e5366f)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
- Rename WoW112CharacterCreation.h to WoW120CharacterCreation.h
- Add Spirit stat to BaseStats struct (MAX_STATS = 5 in WoW 12.0)
- Set Spirit values: 18 for melee, 28 for casters, 25 for hybrids
- Update CMakeLists.txt to reference new header file
- Update BotCharacterCreator.cpp include path
Part of WoW 12.0 API migration (commit b0a596908d5c1b5b09f90e97b17a7fc785e5366f)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
During bot destruction, the Player object may be destroyed before
the BotAI destructor runs, causing _bot to become a dangling pointer.
When UnsubscribeFromEventBuses() tries to access _bot->GetGUID(),
it crashes with ACCESS_VIOLATION at GenericEventBus.h line 277.
Fix:
- Add _cachedBotGuid member to BotAI, initialized at construction
- Add UnsubscribeByGuid() method to GenericEventBus template
- Update UnsubscribeFromEventBuses() to use cached GUID directly
via EventBus<T>::instance()->UnsubscribeByGuid(_cachedBotGuid)
This ensures safe cleanup even when Player is already destroyed.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
JIT bots were failing to create (1/29 success rate) because
Player::Create() calls ValidateAppearance() which requires valid
ChrCustomizationChoice entries for WoW 11.x.
The fix generates default customizations using DB2Manager APIs:
- GetCustomiztionOptions() to get available options for race/gender
- GetCustomiztionChoices() to get valid choices for each option
- Uses first available choice for each customization option
Applied to both CreateBotCharacter() overloads to fix JIT/Instance bot
creation for BG queue population.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Instance bots (JIT bots for BG/dungeon) should NEVER run quest behavior.
This was causing crashes when:
1. Bot worker thread calls QuestAcceptanceManager::AcceptQuest
2. SmartAI::OnQuestAccept is triggered
3. SmartScript (not thread-safe) crashes with memory corruption
Fix: Check IsInstanceBot() in both IsActive() and GetRelevance() to
completely disable quest strategy for instance bots.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Added null pointer validation to:
- WorldObject::AddToObjectUpdate() - validates GetMap() before calling
- WorldObject::RemoveFromObjectUpdate() - same validation
- Map::AddUpdateObject() - validates object pointer
- Map::RemoveUpdateObject() - same validation
These checks prevent potential crashes from memory corruption scenarios
where the Map pointer or object pointer could be invalid.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Previous implementation validated handlers with lock held, then released
the lock before dispatching. This created a race window where another
thread could delete the BotAI between validation and dispatch, causing
ACCESS_VIOLATION crash.
Windows SEH exceptions (like access violations) are NOT caught by C++
catch(...), so the server crashes immediately.
Fix: Hold the recursive mutex during dispatch. This is safe because:
1. _subscriptionMutex is recursive - handlers can call Unsubscribe()
2. Single lock acquisition for entire dispatch phase
3. No race window between validation and dispatch
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Problem: Instance bots sitting in BG queue were considered "active" and never
timed out. If the BG never popped (not enough players), bots accumulated
indefinitely, causing "HARD CAP REACHED" errors (563 bots when only 19 requested).
Root cause:
- IsInActiveInstanceState() returns true for bots in BG/LFG queue
- Active bots never trigger idle timeout
- If BG never starts, bots wait in queue forever
Solution: Add a 5-minute queue timeout separate from idle timeout:
- Track _queueAccumulatorMs - time spent waiting in queue
- Track _hasEnteredInstance - whether bot actually entered content
- If bot is in queue for >5 minutes without entering instance → logout
New member variables:
- _instanceBotStartTime: when bot was marked as instance bot
- _queueAccumulatorMs: accumulated queue wait time
- _hasEnteredInstance: true once bot enters dungeon/BG/arena
Constants:
- INSTANCE_BOT_IDLE_TIMEOUT_MS = 60s (existing)
- INSTANCE_BOT_QUEUE_TIMEOUT_MS = 5 minutes (new)
This prevents bot explosion when BGs don't pop due to population imbalance.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Root cause: When warm pool bots were already logged in, BotSpawner::SpawnBot()
would return failure (bot already in _botSessions). WarmUpBot treated this as
a failure and moved bots to Maintenance, so they never queued for BG.
The fix: At the start of WarmUpBot, check if bot is already online via
ObjectAccessor::FindPlayer(). If so:
- Mark as instance bot
- Queue directly for BG using BGBotManager::QueueBotForBG()
- Set slot state to Assigned
- Call OnBotWarmupComplete(true)
- Return early (skip spawning)
This ensures warm pool bots that are already online get properly queued
for battlegrounds instead of being moved to Maintenance.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
The issue: AddPlayerBot() only queues spawn to _pendingSpawns and returns
immediately. MarkAsInstanceBot() was called right after, but the session
doesn't exist yet, causing "session not found" errors and bots not being
properly tracked as instance bots.
The fix:
- Add markAsInstanceBot field to BotPendingConfiguration
- Set markAsInstanceBot=true in BotCloneEngine and InstanceBotPool
- Apply marking in BotPostLoginConfigurator::ApplyPendingConfiguration()
AFTER the session is guaranteed to exist
- Remove premature MarkAsInstanceBot() call from JITBotFactory
This ensures JIT/warm pool bots get:
- Proper idle timeout (60 seconds)
- Restricted behavior (no questing, BattlePetManager, etc.)
- Correct instance bot tracking
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
ROOT CAUSE: QueueStatePoller was using BattlegroundQueue::GetPlayersInQueue()
which returns from m_SelectionPools (populated during matchmaking), not from
m_QueuedGroups (the actual queue). This caused the system to always see 0
players in queue even after bots were successfully queued.
SYMPTOMS:
- Bots logged "Successfully queued for BG" but next poll showed 0/10 in queue
- BG queue population kept trying to fill already-filled queues
- BGs took forever to start due to count mismatch
FIX:
- Added GetQueuedPlayersCount(teamId, bracketId) to BattlegroundQueue
- This function iterates m_QueuedGroups for the specific bracket and team
- Only counts players not already invited to a BG instance
- Updated QueueStatePoller to use the new function instead of GetPlayersInQueue()
TESTING: Verified bots are queued and count now reflects actual queued players
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Warm pool bots were being "assigned" to BG but never actually queued
because they weren't in the world when QueueStatePoller tried to call
ObjectAccessor::FindPlayer(). The bots were in "login in progress" state.
Fix: WarmUpBot now reads the contentId and instanceType from the slot
(set by AssignBot) and includes them in BotPendingConfiguration. After
the bot logs in, BotPostLoginConfigurator queues them for the BG/dungeon.
This applies the same pattern used for JIT bots - deferred queueing
after login instead of immediate queueing.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
JIT-created bots were never being queued for BG because the onComplete
callback tried to use ObjectAccessor::FindPlayer() before bots entered
the world. Replaced direct callback queueing with deferred post-login
queueing via BotPostLoginConfigurator (same pattern as LFG dungeons).
Changes:
- BotPostLoginConfigurator: Implement BG queueing after bot login
- QueueStatePoller: Use battlegroundIdToQueue field instead of callback
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Three-layer defense against dangling pointers in _updateObjects:
1. BotWorldSessionMgr: Call SetDestroyedObject(true) early in
RemovePlayerBot() before clearing update mask
2. BaseEntity: Block re-adding to _updateObjects once marked destroyed
by checking !m_isDestroyedObject in AddToObjectUpdateIfNeeded()
3. Map: Add IsDestroyedObject() check before BuildUpdate() as final
safety net for partially corrupted objects
Also fix BG bot count calculation in QueueStatePoller to use
maxPlayersPerTeam instead of minPlayersPerTeam (15v15 for SotA
instead of 5v5)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
ROOT CAUSE: Bots in groups were just following the master instead of
executing battleground tactics because MoveToPosition() refused to
interrupt FOLLOW_MOTION_TYPE movement.
When a grouped bot enters a battleground:
1. Bot has FOLLOW_MOTION_TYPE active (following group leader)
2. BG AI calls MoveToPosition() to send bot to flag/objective
3. MoveToPosition() sees FOLLOW_MOTION_TYPE and returns false
4. Bot keeps following master, ignoring all BG tactics
SOLUTION: Add battleground exception to FOLLOW_MOTION_TYPE check.
When bot is in an active battleground (STATUS_IN_PROGRESS), the
BG movement is allowed to clear follow motion and take over.
This allows BG tactics (flag capture, defense, etc.) to work
while preserving follow behavior for normal group play outside BGs.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
ROOT CAUSE: The crash occurred when dispatching events to handlers that
had been deleted between validation and dispatch. This was triggered when
Arathi Basin battleground ended - players are removed and their BotAI
objects deleted while events are still being dispatched.
The previous validation at line 730 only checked if the GUID existed in
_subscriberPointers, but did NOT validate that the pointer was still the
same. This allowed three failure modes:
1. Bot unsubscribed (GUID removed) - correctly skipped
2. Bot deleted and REPLACED with new bot at same GUID - dispatched to WRONG object!
3. Bot deleted but GUID not yet removed from map - dispatched to FREED memory! (CRASH)
SOLUTION: Store the original BotAI* pointer alongside the handler and validate
BOTH GUID existence AND pointer match during the second validation pass.
This catches cases 2 and 3 where the underlying object has changed.
Changes:
- handlersToDispatch now stores struct with {guid, botAI*, handler*}
- Validation now checks: it->second == info.botAI (pointer match)
- Added catch(...) block to catch any remaining edge cases
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
ROOT CAUSE: The counter mismatch ("1 in-flight, 0 active workers") was
caused by incorrect memory ordering. totalSubmitted and totalCompleted
were using memory_order_relaxed for writes, but WaitForCompletion used
memory_order_acquire for reads. With relaxed writes and acquire reads,
there's no happens-before relationship - the acquire load doesn't
synchronize with relaxed stores.
SOLUTION: Changed all writes to totalSubmitted and totalCompleted to use
memory_order_release. Combined with the acquire reads in WaitForCompletion,
this creates proper synchronization via the release-acquire pattern.
Changes:
- ThreadPool.h Submit(): totalSubmitted now uses release ordering
- ThreadPool.h Submit() failure path: totalCompleted now uses release
- ThreadPool.cpp RecordTaskCompletion(): totalCompleted uses release
- ThreadPool.cpp outer catch: totalCompleted uses release
- ThreadPool.cpp WaitForCompletion auto-correction: uses release
This should eliminate the "ghost counter" issue that required the
auto-correction workaround added in the previous commit.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Root cause identified: The "1 in-flight, 0 active workers" timeout
was caused by a counter mismatch where totalSubmitted > totalCompleted
even though all tasks had actually finished. This can happen if an
exception occurs during task submission that doesn't properly update
the completed counter.
Solution: In WaitForCompletion(), detect the mismatch condition:
- All queues are empty
- All workers are sleeping (0 active)
- But counters show in-flight tasks
When detected, log a warning and correct the counters to prevent
the permanent 10-second timeout on every update cycle.
This is a workaround - the real fix would be to ensure all code
paths properly maintain counter balance. But this prevents the
timeout from blocking bot updates indefinitely.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Added periodic logging (every 100 registrations) to verify that
RegisterTaskStart is actually being called. This helps diagnose why
"0 tasks tracked" is showing despite tasks being in-flight.
Also improved LogStuckTasks to always output a summary showing
total tracked and stuck count for better diagnostics.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
1. ThreadPool counter fix:
- The outer catch block in WorkerThread::Run() was only updating
the per-worker counter, not the pool's totalCompleted
- This caused GetInFlightTasks() to return permanently inflated
values when exceptions occurred (explaining "1 in-flight, 0 active
workers" with all tasks done)
- Now properly updates pool counter in outer catch
2. Improved stuck task detection:
- LogStuckTasks now always outputs a summary showing how many tasks
are tracked and how many are stuck
- This helps diagnose whether tasks are being registered properly
Expected fix: The "1 in-flight, 0 active workers" issue should no longer
occur if the root cause was exception handling leaving the counter in
an inconsistent state.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
When ThreadPool wait times out (>2s), we now log which specific bot(s)
are stuck. This helps identify:
- Deadlocks within bot update tasks
- Infinite loops in AI subsystems
- Slow operations (pathfinding, database, etc.)
Implementation:
- Added thread-safe registry tracking executing bot tasks
- RAII guard ensures tasks are always unregistered on exit
- LogStuckTasks() called at 2s warning and 10s timeout
- Logs bot name and GUID with execution time
Example output:
STUCK TASK DETECTED: Bot Anderenz (GUID: Player-XXX) has been
executing for 5432ms!
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Similar to the VMAP fix, this caches failed MMAP load attempts using a
bitset per TerrainInfo instance. This prevents the server from repeatedly
attempting to load MMAP tiles that don't exist (e.g., unused dungeon maps,
boost experience maps, etc.), which was causing significant slowdowns.
Changes:
- Add _mmapLoadFailed bitset to TerrainInfo class
- Skip LoadMMapImpl early if grid already marked as failed
- Cache FileNotFound, VersionMismatch, ReadFromFileFailed, and
LibraryError results
- Change expected "FileNotFound" log level from WARN to DEBUG
- Update analysis documentation with fix status
Impact: Near-instant returns for grids with missing MMAP data instead of
repeated file I/O attempts.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Problem: VMAP loading for maps without data (Boost Experience, phased zones,
newer dungeons) was retried on EVERY access, causing:
- Thousands of file access attempts per session
- Log spam with "Could not load VMAP" errors
- Server slowdown from repeated disk I/O
Solution: Added _vmapLoadFailed bitset to TerrainInfo that caches failed
VMAP load attempts, similar to existing _gridFileExists pattern for maps.
Changes:
- TerrainMgr.h: Added _vmapLoadFailed bitset
- TerrainMgr.cpp: Check bitset before load, cache failures, reduce log level
Maps affected: 1949, 1950 (8.0 Boost), 1554, 1557 (7.0 Boost), 1465 (Tanaan),
2648, 2649, 2662, 2669 (TWW dungeons), and others without VMAP data.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
PROBLEM: Extreme lag with 100+ bots caused by SpatialGridManager issues:
1. DATA RACE BUG in GetGrid():
- Writing lastAccessTime under shared_lock using const_cast
- Multiple threads writing same memory location concurrently = undefined behavior
- Could cause crashes, memory corruption, or silent data corruption
2. PERFORMANCE ISSUE in GetGrid():
- Called thousands of times per second (100+ bots × multiple calls per update)
- Each call: acquire shared_lock + chrono::steady_clock::now()
- Unnecessary overhead for a keep-alive timestamp that's rarely checked
3. Same issues in UpdateGrid() and TouchGrid()
SOLUTION:
1. GetGrid(): Remove lastAccessTime update entirely
- This method is for READ-ONLY grid access
- lastAccessTime is only used for cleanup of inactive grids
- No need to update on every access
2. UpdateGrid(): Split into 3 phases
- Phase 1: Get grid pointer under shared_lock (fast)
- Phase 2: Update grid WITHOUT holding manager lock
- Phase 3: Update lastAccessTime under exclusive lock
3. TouchGrid(): Use unique_lock (exclusive) for write operation
- This method explicitly updates lastAccessTime
- Must use exclusive lock for write operations
IMPACT:
- Eliminates ~1000+ chrono::now() calls per second
- Fixes potential memory corruption from data race
- Reduces lock contention on SpatialGridManager
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
PROBLEM 1: "Pool bot failed to login via BotSpawner"
- Bots in maintenance state after warmup failure
PROBLEM 2: "No account ID for bot (not in slot or CharacterCache)"
- Bots with account_id = 0 in playerbot_instance_pool table
- CharacterCache doesn't have the account association
ROOT CAUSE:
The ON DUPLICATE KEY UPDATE clause in SyncToDatabase() did NOT include
`account_id`, so:
1. Bot initially saved with account_id = 0 (bug in some code path)
2. WarmUpBot corrects it from CharacterCache and updates the slot
3. SyncToDatabase INSERTs the correct account_id, BUT
4. ON DUPLICATE KEY UPDATE doesn't update account_id column
5. On restart, LoadFromDatabase loads account_id = 0 again
SOLUTION:
1. Add `account_id = VALUES(account_id)` to ON DUPLICATE KEY UPDATE
- Now account_id corrections are persisted properly
2. Add account_id repair in LoadFromDatabase()
- If account_id = 0, try to get it from CharacterCache
- Log warning if CharacterCache also has no account
This ensures bots with previously corrupted account_id values
will be repaired on next load and the fix will be persisted.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
PROBLEM: QuestCompletion used std::mutex for read-heavy data structures
which blocked ALL readers when ANY read was happening. With 100+ bots
calling quest-related functions from ThreadPool workers, this caused
severe lock contention and task delays.
AFFECTED MUTEXES (all read-heavy patterns):
- _pausedBotsMutex: Checked on EVERY quest event (HandleQuestEvent)
- _questOrderMutex: Read frequently during quest prioritization
- _objectiveOrderMutex: Read during objective sequencing
SOLUTION:
1. Changed _pausedBotsMutex from std::mutex to std::shared_mutex
- Read operations (find) use std::shared_lock (concurrent readers OK)
- Write operations (insert/erase) use std::unique_lock (exclusive)
2. Changed _questOrderMutex from std::mutex to std::shared_mutex
- All operations use std::unique_lock (mostly writes in current code)
- Future optimization: use shared_lock for read-only accesses
3. Changed _objectiveOrderMutex from std::mutex to std::shared_mutex
- All operations use std::unique_lock (writes only currently)
This eliminates a major ThreadPool bottleneck where every bot's
quest event processing was blocking on mutex acquisition.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
PROBLEM: HumanizationConfig used std::mutex which blocked ALL readers
when ANY read was happening. With 100+ concurrent bots calling
GetActivityConfig() and GetHourlyActivityMultiplier() from ThreadPool
workers, this caused severe lock contention and task delays.
SOLUTION:
1. Changed std::mutex to std::shared_mutex for read-heavy access
2. Use std::unique_lock only during Load()/Reload() (rare operation)
3. Use std::shared_lock for GetActivityConfig() (multiple readers OK)
4. Made GetHourlyActivityMultiplier() lock-free since _hourlyMultipliers
is set once during Load() and never modified at runtime
5. Changed C-array to std::array<float, 24> for better type safety
This eliminates a major ThreadPool bottleneck where every bot's
HumanizationManager::Update() was blocking on mutex acquisition.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
Root cause: ThreadPool::WaitForCompletion could block world thread for
35+ seconds (5s + 30s), leaving insufficient buffer before FreezeDetector
triggers at 60 seconds. Combined with other World::Update operations,
total time exceeded 60s causing forced crash.
Changes:
- BotWorldSessionMgr: Reduce wait timeouts from 5s+30s to 2s+8s (10s max)
- ThreadPool::WaitForCompletion: Add hard cap of 15 seconds regardless
of caller-specified timeout
- ThreadPool.h: Change default timeout from milliseconds::max() to 10s
This ensures world thread never blocks more than 15 seconds in
WaitForCompletion, leaving 45+ seconds buffer for FreezeDetector.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: luis <[email protected]>
