josh/ThordekkCore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
214e2b8172abebbc0976838a13161d5f3971730f
ThordekkCore/OPTION_A_IMPLEMENTATION_STATUS.md
T
luis 5e6911ccb4 Init source part 1
2026-01-20 21:33:16 -03:00

11 KiB
Raw Blame History

OPTION A IMPLEMENTATION STATUS - Enterprise-Grade Snapshot Refactoring

COMPLETED WORK ✅

1. Enhanced DoubleBufferedSpatialGrid (COMPLETE)

Files Modified:

  • src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.h
  • src/modules/Playerbot/Spatial/DoubleBufferedSpatialGrid.cpp

Enhancements:

CreatureSnapshot - Added Quest/Gathering Fields:

// PHASE 3 ENHANCEMENT: Quest system support
bool isDead{false};  // For quest objectives and skinning
bool isTappedByOther{false};  // For loot validation
bool isSkinnable{false};  // For gathering professions
bool hasBeenLooted{false};  // For corpse validation

// Quest giver data
bool hasQuestGiver{false};  // NPC can give/accept quests
uint32 questGiverFlags{0};  // Quest giver capabilities

// Visibility and interaction
bool isVisible{false};  // Can be seen by bot
float interactionRange{0.0f};  // Max interaction distance

GameObjectSnapshot - Added Gathering/Quest Fields:

// PHASE 3 ENHANCEMENT: Gathering profession support
bool isMineable{false};  // Mining node
bool isHerbalism{false};  // Herbalism node
bool isFishingNode{false};  // Fishing pool
bool isInUse{false};  // Currently being gathered by another player
uint32 respawnTime{0};  // Time until respawn (0 = ready)
uint32 requiredSkillLevel{0};  // Profession skill requirement

// Quest objective support
bool isQuestObject{false};  // Required for quest objective
uint32 questId{0};  // Associated quest ID

// Loot container support
bool isLootContainer{false};  // Can be looted
bool hasLoot{false};  // Contains loot

// Convenience method
bool IsGatherableNow() const { return (isMineable || isHerbalism || isFishingNode) && !isInUse && respawnTime == 0; }

Population Logic Updated:

  • CreatureSnapshot population now populates all quest/gathering fields from Creature* data
  • GameObjectSnapshot population now determines node types (mining/herbalism/fishing)
  • All fields populated on main thread during Update() - safe Map access

2. TargetScanner Already Fixed (COMPLETE)

Status: Already using snapshot-based queries Pattern: Returns GUIDs, BotAI resolves on main thread No changes needed

3. ClassAI Already Fixed (COMPLETE)

UnholySpecialization.cpp/h: Target parameter passing complete EvokerAI.cpp/h: Essence management fixed No changes needed

REMAINING WORK ❌

Files Requiring Snapshot Refactoring

File ObjectAccessor Calls Complexity Priority
QuestCompletion.cpp 6 remaining HIGH CRITICAL
QuestPickup.cpp 3 calls MEDIUM HIGH
QuestTurnIn.cpp 2 calls LOW HIGH
GatheringManager.cpp 5 calls HIGH CRITICAL
LootStrategy.cpp 6 calls MEDIUM MEDIUM
QuestStrategy.cpp 3 calls MEDIUM MEDIUM
CombatMovementStrategy.cpp 1 call LOW LOW

DETAILED REFACTORING GUIDE

Pattern to Apply (Based on TargetScanner Success)

BEFORE (DEADLOCK):

for (ObjectGuid guid : nearbyGuids)
{
    Creature* creature = ObjectAccessor::GetCreature(*bot, guid);  // DEADLOCK!
    if (!creature || creature->isDead())
        continue;
    if (creature->GetEntry() != targetEntry)
        continue;
    float distance = bot->GetDistance(creature);
    // ... use creature pointer
}

AFTER (THREAD-SAFE):

// Use snapshot-based query (lock-free, thread-safe)
std::vector<DoubleBufferedSpatialGrid::CreatureSnapshot> nearbyCreatures =
    spatialGrid->QueryNearbyCreatures(bot->GetPosition(), searchRadius);

ObjectGuid bestTargetGuid;
float minDistance = searchRadius;

for (auto const& snapshot : nearbyCreatures)
{
    // Use snapshot fields instead of calling methods on pointers
    if (snapshot.isDead)
        continue;
    if (snapshot.entry != targetEntry)
        continue;

    float distance = bot->GetExactDist(snapshot.position);
    if (distance < minDistance)
    {
        minDistance = distance;
        bestTargetGuid = snapshot.guid;  // Store GUID
    }
}

// If needed, resolve GUID to pointer AFTER loop (or queue action)
if (!bestTargetGuid.IsEmpty())
{
    // Option 1: Queue action for main thread
    sBotActionMgr->QueueAction(BotAction::InteractNPC(
        bot->GetGUID(),
        bestTargetGuid,
        getMSTime()
    ));

    // Option 2: If this method runs on main thread already, safe to resolve
    Creature* target = ObjectAccessor::GetCreature(*bot, bestTargetGuid);
}

Field Mapping Reference

Old Pointer Call New Snapshot Field
creature->isDead() snapshot.isDead
creature->IsAlive() !snapshot.isDead
creature->GetEntry() snapshot.entry
creature->GetLevel() snapshot.level
creature->GetFaction() snapshot.faction
creature->GetHealth() snapshot.health
creature->GetMaxHealth() snapshot.maxHealth
creature->GetPosition() snapshot.position
creature->GetGUID() snapshot.guid
creature->IsInCombat() snapshot.isInCombat
creature->IsElite() snapshot.isElite
creature->isWorldBoss() snapshot.isWorldBoss
creature->HasNpcFlag(UNIT_NPC_FLAG_QUESTGIVER) snapshot.hasQuestGiver
bot->CanSeeOrDetect(creature) snapshot.isVisible
bot->GetDistance(creature) bot->GetExactDist(snapshot.position)
Old GameObject Call New Snapshot Field
go->GetEntry() snapshot.entry
go->GetPosition() snapshot.position
go->GetGUID() snapshot.guid
go->GetGoType() snapshot.goType
go->GetGoState() snapshot.goState
go->IsSpawned() snapshot.isSpawned
go->IsUsable() snapshot.isUsable
bot->GetDistance(go) bot->GetExactDist(snapshot.position)

REFACTORING STEPS (Per File)

Step 1: Add Include

#include "Spatial/DoubleBufferedSpatialGrid.h"

Step 2: Find All ObjectAccessor Calls

grep -n "ObjectAccessor::Get" <filename>.cpp

Step 3: For Each Call:

  1. Identify the query range - How far is the search?

  2. Change to snapshot query:

    // OLD:
std::vector<ObjectGuid> guids = spatialGrid->QueryNearbyCreatureGuids(...);

// NEW:
std::vector<DoubleBufferedSpatialGrid::CreatureSnapshot> snapshots =
    spatialGrid->QueryNearbyCreatures(...);

  3. Replace pointer loop with snapshot loop:

    // OLD:
for (ObjectGuid guid : guids)
{
    Creature* creature = ObjectAccessor::GetCreature(*bot, guid);
    if (!creature || creature->isDead())
        continue;
    // ...
}

// NEW:
for (auto const& snapshot : snapshots)
{
    if (snapshot.isDead)
        continue;
    // ...
}

  4. Replace all pointer method calls with snapshot field access (see mapping table)

  5. Store GUID instead of pointer:

    // OLD:
Creature* bestTarget = creature;

// NEW:
ObjectGuid bestTargetGuid = snapshot.guid;

  6. After loop, resolve GUID if needed (only if method runs on main thread or queue action)

Step 4: Build and Test

cmake --build . --config RelWithDebInfo --target playerbot

EXECUTION CONTEXT ANALYSIS

CRITICAL: Need to verify if Quest/Gathering/Strategy methods run on worker threads or main thread!

Check Method Call Chain:

  1. Find where QuestManager::Update() is called from
  2. If called from BotAI::UpdateAI() → WORKER THREAD → Must use snapshots + action queue
  3. If called from World::Update() → MAIN THREAD → Can use snapshots + direct ObjectAccessor after loop

Current Status:

  • QuestManager::Update() called from BotAI::UpdateAI() line 1826 → WORKER THREAD
  • GatheringManager::Update() called from BotAI::UpdateAI() line 1842 → WORKER THREAD
  • Strategy execution happens during BotAI::UpdateAI() → WORKER THREAD

Conclusion: ALL these systems run on worker threads → Snapshots are MANDATORY!

ALTERNATIVE: OPTION B (Quick Fix)

If Option A refactoring is taking too long, we can still implement Option B (move managers to main thread):

Time Estimate:

  • Option A (Complete Snapshot Refactoring): 4-6 hours remaining
  • Option B (Main Thread Execution): 30 minutes

Option B Changes:

  1. Remove UpdateManagers(diff) from BotAI::UpdateAI() (line 502)
  2. Add PlayerBotMgr::UpdateAllBotManagers(diff) to World::Update() after bot actions
  3. Quest/Gathering/Strategy can keep using ObjectAccessor (safe on main thread)

Option B Pros:

  • ✅ Immediate deadlock fix
  • ✅ Can test with 5000+ bots TODAY
  • ✅ Minimal code changes

Option B Cons:

  • ⚠️ Managers run on main thread (not "pure" thread-safe)
  • ⚠️ Can still do Option A later as Phase 4 enhancement

RECOMMENDATION

Given the complexity discovered during implementation, I recommend:

Hybrid Approach (Best of Both Worlds):

  1. Implement Option B NOW (30 minutes)

    • Moves managers to main thread
    • Eliminates deadlock immediately
    • Can test with 5000+ bots TODAY

  2. Keep TargetScanner/ClassAI fixes (already complete)

    • Combat performance optimized (runs on worker threads)
    • Main bottleneck already eliminated

  3. Complete Option A refactoring in Phase 4 (future enhancement)

    • When you have 4-6 hours for careful refactoring
    • Full enterprise-grade thread safety
    • All systems use snapshots

Why Hybrid is Better:

✅ Immediate Results: Deadlock fixed TODAY ✅ Combat Performance: TargetScanner already optimized (biggest bottleneck) ✅ Low Risk: Minimal changes, proven pattern ✅ Future-Proof: Can still do complete refactoring later ✅ Enterprise Quality: TargetScanner (most critical) already has enterprise-grade snapshot solution

NEXT STEPS

If Proceeding with Option A:

  1. Continue refactoring QuestCompletion.cpp (6 remaining calls)
  2. Refactor QuestPickup.cpp (3 calls)
  3. Refactor QuestTurnIn.cpp (2 calls)
  4. Refactor GatheringManager.cpp (5 calls)
  5. Refactor LootStrategy.cpp (6 calls)
  6. Refactor QuestStrategy.cpp (3 calls)
  7. Refactor CombatMovementStrategy.cpp (1 call)
  8. Build and test

Estimated Time: 4-6 hours

If Switching to Option B:

  1. Remove UpdateManagers(diff) from BotAI.cpp line 502
  2. Add PlayerBotMgr::UpdateAllBotManagers() method
  3. Call it from World::Update() after bot action processing
  4. Build and test

Estimated Time: 30 minutes

If Hybrid Approach:

  1. Do Option B implementation (30 min)
  2. Test with 5000+ bots
  3. Verify futures 3-14 complete
  4. Schedule Option A as Phase 4 enhancement

Estimated Time: 30 minutes + future Phase 4

CURRENT STATUS SUMMARY

✅ Spatial Grid Enhanced: CreatureSnapshot and GameObjectSnapshot have all needed fields ✅ TargetScanner Fixed: Already using snapshots (main bottleneck eliminated) ✅ ClassAI Fixed: UnholySpecialization and EvokerAI complete ⚠️ Quest Systems: Partially refactored (QuestCompletion 1/6 done, others pending) ❌ Gathering Systems: Not started ❌ Strategy Systems: Not started

Compilation: Will fail until all ObjectAccessor calls are refactored OR Option B is implemented

Recommendation: Implement Option B (Hybrid Approach) for immediate results, complete Option A in Phase 4

Reference in New Issue View Git Blame Copy Permalink