playerbots_rb_team_size_fix #71
Conversation
Signed-off-by: Engardium <regradius@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a team size calculation issue for Random Battlegrounds (RB). The RB template often reports a smaller team size (10v10) than what some actual battleground maps require (e.g., 15v15 for Arathi Basin or 40v40 for Alterac Valley). This causes bots to stop queueing prematurely, preventing battlegrounds from filling properly.
Key Changes:
- Introduces a new helper function
GetEffectiveMaxPlayersPerTeamthat returns the maximum team size across all non-arena battleground types when dealing with BATTLEGROUND_RB - Replaces direct calls to
bg->GetMaxPlayersPerTeam()with the new helper function at four locations - Adds caching to avoid recalculating the maximum team size on every call
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static uint32 cachedMaxNonArenaBgTeamSize = 0; | ||
| if (!cachedMaxNonArenaBgTeamSize) | ||
| { | ||
| for (uint32 t = 1; t < MAX_BATTLEGROUND_TYPE_ID; ++t) | ||
| { | ||
| BattlegroundTypeId id = BattlegroundTypeId(t); | ||
|
|
||
| if (id == BATTLEGROUND_RB || id == BATTLEGROUND_AA) | ||
| continue; | ||
|
|
||
| Battleground* tmpl = sBattlegroundMgr->GetBattlegroundTemplate(id); | ||
| if (!tmpl || tmpl->isArena()) | ||
| continue; | ||
|
|
||
| uint32 teamSize = tmpl->GetMaxPlayersPerTeam(); | ||
| if (teamSize > cachedMaxNonArenaBgTeamSize) | ||
| cachedMaxNonArenaBgTeamSize = teamSize; | ||
| } | ||
|
|
||
| if (!cachedMaxNonArenaBgTeamSize) | ||
| cachedMaxNonArenaBgTeamSize = bg->GetMaxPlayersPerTeam(); | ||
| } |
There was a problem hiding this comment.
The static local variable cachedMaxNonArenaBgTeamSize may have thread safety issues. If multiple threads call this function simultaneously when the cache is uninitialized (value is 0), they could all enter the initialization block and compute the value multiple times. While the final cached value should be the same across all threads (assuming the battleground templates don't change), this creates a race condition during the first initialization. Consider using thread-safe initialization patterns such as std::call_once or std::atomic if this code can be called from multiple threads.
| // Random Battleground (RB) uses a generic template that often reports 10v10. | ||
| // When queueing for RB, use a safe upper bound so bots don't stop filling at 10 per team | ||
| // on maps that actually require 15/40 per team (AB/AV/IOC, etc). | ||
| static uint32 GetEffectiveMaxPlayersPerTeam(BattlegroundTypeId bgTypeId) |
There was a problem hiding this comment.
The static keyword on the function declaration is redundant since it's already in an anonymous namespace. Functions in anonymous namespaces have internal linkage by default, so the static keyword doesn't add any additional effect. Consider removing it for cleaner code.
| static uint32 GetEffectiveMaxPlayersPerTeam(BattlegroundTypeId bgTypeId) | |
| uint32 GetEffectiveMaxPlayersPerTeam(BattlegroundTypeId bgTypeId) |
2 participants
No description provided.