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]>
