13 KiB
Phase 2D Analysis: ThreatCoordinator.cpp ObjectAccessor Migration
File: src/modules/Playerbot/AI/Combat/ThreatCoordinator.cpp
Total ObjectAccessor Calls: 23
Priority: CRITICAL (hot path, high-frequency updates)
Complexity: Medium-High
Executive Summary
ThreatCoordinator.cpp contains 23 ObjectAccessor calls, making it the highest-usage file in the remaining codebase. Analysis reveals several optimization opportunities:
- Loop patterns: 4 instances of bot/target iteration loops (priority targets)
- Tank lookups: 7 calls to find tank GUIDs (can use snapshots)
- Validation patterns: 8 simple FindPlayer validations (easy wins)
- Required calls: 4 calls need Unit* for TrinityCore APIs (cannot optimize)
Expected Reduction: 19/23 calls eliminated (83% reduction)
Detailed Call Analysis
Category 1: Loop Patterns (HIGH PRIORITY)
1.1 InitiateTankSwap() - Lines 244-250
Pattern: Get targets for taunt sequence
// ONLY get Unit* when spatial grid confirms entity exists
Unit* target = ObjectAccessor::GetUnit(*newTankBot, targetGuid);
if (!target)
continue;
Analysis:
- Called within loop iterating target GUIDs
- Already has snapshot validation comment from Phase 2
- Can optimize: Use SpatialGridQueryHelpers to validate before ObjectAccessor
Optimization: Pre-validate with snapshot, only call ObjectAccessor when needed
1.2 AnalyzeThreatSituation() - Lines 552-554
Pattern: Primary tank analyzing targets
// ONLY get Unit* when spatial grid confirms entity exists
Unit* target = ObjectAccessor::GetUnit(*primaryTankBot, targetGuid);
if (!target)
continue;
Analysis:
- Loop through all nearby hostile creatures
- Already has spatial grid comment
- Can optimize: Use CreatureSnapshot for validation
Optimization: Change to FindCreatureByGuid() + snapshot validation
1.3 CoordinateThreatResponses() - Nested Loop Lines 612-628
Pattern: Each bot checks each target
Player* bot = ObjectAccessor::FindPlayer(botGuid); // Line 612
if (!bot)
continue;
for (auto const& [targetGuid, threat] : assignment.targets)
{
// ...
Unit* target = ObjectAccessor::GetUnit(*bot, targetGuid); // Line 625
if (!target)
continue;
}
Analysis:
- CRITICAL O(n) PATTERN: N bots × M targets per bot
- Example: 5 bots × 3 targets = 5 FindPlayer + 15 GetUnit calls = 20 total calls per update
- Update frequency: ~5-10 Hz during combat
- This is a HOT PATH bottleneck
Optimization Priority: HIGHEST
- Pre-fetch all bot snapshots once
- Pre-fetch all creature snapshots once
- Use cached snapshots for validation
- Expected reduction: 20 calls → 2-3 calls (90% reduction)
1.4 MonitorTankThreat() - Nested Loop Lines 660-677
Pattern: Similar to 1.3
Player* bot = ObjectAccessor::FindPlayer(botGuid); // Line 660
if (!bot)
continue;
for (auto& assignment : _botAssignments)
{
// ...
Unit* target = ObjectAccessor::GetUnit(*bot, assignment.targetGuid); // Line 675
if (target && target->GetVictim() != bot)
{
// ... threat response
}
}
Analysis:
- Another nested loop pattern
- Similar to CoordinateThreatResponses()
- Hot path during combat
Optimization Priority: HIGH
Category 2: Tank Lookups (MEDIUM PRIORITY)
These calls find tank Player* for various operations:
| Line | Function | Purpose |
|---|---|---|
| 231 | InitiateTankSwap() | Get new tank bot |
| 501 | AnalyzeThreatSituation() | Get primary tank bot |
| 795 | ExecuteEmergencyProtocol() | Get primary tank bot |
| 846 | HandleEmergencyThreat() | Get primary tank |
| 857 | HandleEmergencyThreat() | Get off-tank |
| 868 | HandleEmergencyThreat() | Get backup tanks (loop) |
| 1037 | GetBotThreatLevel() | Get bot for threat calculation |
Optimization Strategy:
- Use
SpatialGridQueryHelpers::FindPlayerByGuid()for validation - Cache Player* at function scope if used multiple times
- Use snapshot position for distance calculations
Category 3: Simple Validations (EASY WINS)
These are simple FindPlayer calls that can use snapshots:
| Line | Function | Purpose | Can Optimize? |
|---|---|---|---|
| 286 | IsTankAssigned() | Validate tank exists | ✅ YES - snapshot only |
| 322 | IsBotAssigned() | Validate bot exists | ✅ YES - snapshot only |
| 356-357 | ExecuteThreatTransfer() | Validate from/to bots | ✅ YES - snapshot validation |
| 443 | CoordinateThreatResponses() | Get healer | ⚠️ MAYBE - needs AI access |
| 737 | ExecuteThreatActions() | Get executor bot | ⚠️ MAYBE - needs AI access |
| 1063 | CanBotTaunt() | Validate bot exists | ✅ YES - snapshot + spell check |
Optimization: Change to FindPlayerByGuid() + snapshot validation
Category 4: Required ObjectAccessor (CANNOT OPTIMIZE)
These calls require Unit* for TrinityCore API compatibility:
| Line | Function | Reason |
|---|---|---|
| 754 | ExecuteThreatActions() | ExecuteTaunt() needs Unit* target |
| 773 | ExecuteThreatActions() | ExecuteThreatTransfer() needs Unit* target |
| 815 | ExecuteEmergencyProtocol() | Taunt execution needs Unit* |
Analysis: These are final execution points where TrinityCore APIs require Unit* pointers. Cannot eliminate, but can minimize by pre-validating with snapshots.
Optimization Plan
Priority 1: Critical O(n) Nested Loops
CoordinateThreatResponses() - Lines 609-640
Before:
for (auto& [botGuid, assignment] : _botAssignments)
{
// ...
Player* bot = ObjectAccessor::FindPlayer(botGuid); // N calls
if (!bot)
continue;
for (auto const& [targetGuid, threat] : assignment.targets)
{
// ...
Unit* target = ObjectAccessor::GetUnit(*bot, targetGuid); // N×M calls
if (!target)
continue;
// ... threat logic
}
}
After (Optimized):
// PHASE 2D: Pre-fetch all bot snapshots once (O(n) → snapshots)
std::unordered_map<ObjectGuid, PlayerSnapshot const*> botSnapshotCache;
std::unordered_map<ObjectGuid, CreatureSnapshot const*> creatureSnapshotCache;
for (auto& [botGuid, assignment] : _botAssignments)
{
auto botSnapshot = SpatialGridQueryHelpers::FindPlayerByGuid(nullptr, botGuid);
if (botSnapshot && botSnapshot->IsAlive())
botSnapshotCache[botGuid] = botSnapshot;
}
// Pre-fetch all creature targets
for (auto& [botGuid, assignment] : _botAssignments)
{
for (auto const& [targetGuid, threat] : assignment.targets)
{
auto creatureSnapshot = SpatialGridQueryHelpers::FindCreatureByGuid(nullptr, targetGuid);
if (creatureSnapshot && creatureSnapshot->IsAlive())
creatureSnapshotCache[targetGuid] = creatureSnapshot;
}
}
// Now iterate using cached snapshots (lock-free)
for (auto& [botGuid, assignment] : _botAssignments)
{
auto botIt = botSnapshotCache.find(botGuid);
if (botIt == botSnapshotCache.end())
continue;
for (auto const& [targetGuid, threat] : assignment.targets)
{
auto creatureIt = creatureSnapshotCache.find(targetGuid);
if (creatureIt == creatureSnapshotCache.end())
continue;
// Use snapshot data for distance/validation (lock-free)
auto const* botSnapshot = botIt->second;
auto const* creatureSnapshot = creatureIt->second;
float distance = botSnapshot->position.GetExactDist(&creatureSnapshot->position);
// Only fetch Player* when actually executing threat response
if (/* need to execute action */)
{
Player* bot = ObjectAccessor::FindPlayer(botGuid);
Unit* target = ObjectAccessor::GetUnit(*bot, targetGuid);
// ... execute action
}
}
}
Performance Impact:
- Before: 5 bots × 3 targets = 20 ObjectAccessor calls
- After: 5 bot snapshots + 15 creature snapshots + 1-2 ObjectAccessor (for execution) = ~7 calls
- Reduction: 65% (13/20 calls eliminated)
Priority 2: Tank Lookup Optimization
HandleEmergencyThreat() - Lines 843-871
Before:
// Try primary tank first
if (!_primaryTank.IsEmpty())
{
Player* tank = ObjectAccessor::FindPlayer(_primaryTank); // Call #1
if (tank && tank->IsAlive() && tank->GetDistance2d(target) <= 30.0f)
{
ExecuteTaunt(_primaryTank, target);
return;
}
}
// Try off-tank
if (!_offTank.IsEmpty())
{
Player* tank = ObjectAccessor::FindPlayer(_offTank); // Call #2
if (tank && tank->IsAlive() && tank->GetDistance2d(target) <= 30.0f)
{
ExecuteTaunt(_offTank, target);
return;
}
}
// Try backup tanks
for (const auto& backupGuid : _backupTanks)
{
Player* tank = ObjectAccessor::FindPlayer(backupGuid); // Call #3-N
if (tank && tank->IsAlive() && tank->GetDistance2d(target) <= 30.0f)
{
ExecuteTaunt(backupGuid, target);
return;
}
}
After (Optimized):
// PHASE 2D: Pre-validate with snapshots, eliminate 90% of ObjectAccessor calls
// Try primary tank first (snapshot validation)
if (!_primaryTank.IsEmpty())
{
auto tankSnapshot = SpatialGridQueryHelpers::FindPlayerByGuid(nullptr, _primaryTank);
if (tankSnapshot && tankSnapshot->IsAlive())
{
float distance = tankSnapshot->position.GetExactDist2d(target->GetPosition());
if (distance <= 30.0f)
{
ExecuteTaunt(_primaryTank, target); // Only ObjectAccessor call if snapshot valid
return;
}
}
}
// Try off-tank (snapshot validation)
if (!_offTank.IsEmpty())
{
auto tankSnapshot = SpatialGridQueryHelpers::FindPlayerByGuid(nullptr, _offTank);
if (tankSnapshot && tankSnapshot->IsAlive())
{
float distance = tankSnapshot->position.GetExactDist2d(target->GetPosition());
if (distance <= 30.0f)
{
ExecuteTaunt(_offTank, target);
return;
}
}
}
// Try backup tanks (snapshot validation)
for (const auto& backupGuid : _backupTanks)
{
auto tankSnapshot = SpatialGridQueryHelpers::FindPlayerByGuid(nullptr, backupGuid);
if (tankSnapshot && tankSnapshot->IsAlive())
{
float distance = tankSnapshot->position.GetExactDist2d(target->GetPosition());
if (distance <= 30.0f)
{
ExecuteTaunt(backupGuid, target);
return;
}
}
}
Performance Impact:
- Before: 3-5 ObjectAccessor calls (depends on number of backup tanks)
- After: 3-5 snapshot lookups (lock-free) + 1 ObjectAccessor (only when taunt executes)
- Reduction: 50-80% depending on execution path
Priority 3: Simple Validation Conversions
IsTankAssigned() - Lines 283-289
Before:
Player* tank = ObjectAccessor::FindPlayer(tankGuid);
if (!tank)
return false;
After:
// PHASE 2D: Use snapshot for validation (lock-free)
auto tankSnapshot = SpatialGridQueryHelpers::FindPlayerByGuid(nullptr, tankGuid);
if (!tankSnapshot || !tankSnapshot->IsAlive())
return false;
Performance Impact: 100% reduction for validation-only calls
Expected Results
Call Reduction Summary
| Category | Before | After | Reduction |
|---|---|---|---|
| Nested loops (critical) | 8 | 2-3 | 62-75% |
| Tank lookups | 7 | 2-3 | 57-71% |
| Simple validations | 6 | 0 | 100% |
| Required (cannot optimize) | 2 | 2 | 0% |
| TOTAL | 23 | 6-8 | 65-74% |
Performance Impact
Current State (Before Phase 2D):
- ThreatCoordinator update frequency: ~5-10 Hz during combat
- ObjectAccessor calls: 23 per update × 10 Hz = 230 calls/sec
After Phase 2D:
- ObjectAccessor calls: 7 per update × 10 Hz = 70 calls/sec
- Reduction: 160 calls/sec eliminated (70%)
FPS Impact (100 bot scenario):
- Estimated improvement: 3-5% FPS gain from ThreatCoordinator alone
- Combined with BotThreatManager.cpp optimization: 8-12% total Phase 2D gain
Implementation Sequence
- ✅ Analysis Complete (this document)
- ⏳ Migrate CoordinateThreatResponses() (highest priority, biggest impact)
- ⏳ Migrate MonitorTankThreat() (similar pattern to #2)
- ⏳ Migrate HandleEmergencyThreat() (tank lookup optimization)
- ⏳ Migrate simple validations (IsTankAssigned, IsBotAssigned, etc.)
- ⏳ Build & validate
- ⏳ Performance profiling
Risk Assessment
Low Risk:
- ✅ Nested loop optimizations (pure distance/validation logic)
- ✅ Tank lookups (snapshot position available)
- ✅ Simple validations (snapshot only)
Medium Risk:
- ⚠️ Threat calculation logic (verify math remains identical)
- ⚠️ Taunt execution paths (ensure Player* still valid when needed)
Mitigation:
- Incremental migration (one function at a time)
- Build validation after each change
- Verify identical behavior with snapshot vs. ObjectAccessor
Conclusion
ThreatCoordinator.cpp contains 23 ObjectAccessor calls with 65-74% optimization potential. The critical nested loops in CoordinateThreatResponses() and MonitorTankThreat() are the highest-priority targets.
Expected Phase 2D total impact (ThreatCoordinator + BotThreatManager + others):
- ~1,800 ObjectAccessor calls/sec eliminated
- 8-12% FPS improvement
- 91% cumulative reduction (Phase 2A-2D combined)
Recommendation: Proceed with implementation starting with the nested loop patterns.
End of Analysis