ThordekkCore/PHASE_1_CLEANUP_PLAN.md

PHASE 1.11: CLEANUP PLAN - Legacy Code Removal

Executive Summary

After completing Phase 1 implementation, we need to remove legacy code, unused files, diagnostic logging, and temporary documentation that's no longer needed. This ensures a clean, maintainable codebase.

Duration: 4 hours Risk: LOW (only removes unused code) Benefit: Reduced maintenance burden, cleaner codebase

---

1. DIAGNOSTIC LOGGING TO REMOVE

Files with Temporary Diagnostic Code

1.1 src/modules/Playerbot/AI/BotAI.cpp

Location: Lines ~1216-1264 (added during Phase 2 diagnosis)

// REMOVE: Diagnostic logging for manager execution
static uint32 updateManagersCallCount = 0;
if (++updateManagersCallCount % 10 == 0)
{
    TC_LOG_ERROR("module.playerbot", "🔧 UpdateManagers ENTRY: Bot {}, IsInWorld()={}...");
}

// REMOVE: Per-manager call tracing
TC_LOG_ERROR("module.playerbot", "🎯 Calling QuestManager->Update() for bot {}", _bot->GetName());
TC_LOG_ERROR("module.playerbot", "✅ Returned from QuestManager->Update() for bot {}", _bot->GetName());

Action: Remove all TC_LOG_ERROR diagnostic statements (keep production TC_LOG_DEBUG if appropriate) Lines to Remove: ~60 lines of diagnostic code

1.2 src/modules/Playerbot/AI/BehaviorManager.cpp

Location: Lines ~60-258 (diagnostic logging added during diagnosis)

// REMOVE: Update() entry logging
static uint32 updateCallCounter = 0;
bool shouldLog = (++updateCallCounter % 10 == 0);

if (shouldLog)
{
    TC_LOG_ERROR("module.playerbot", "🔍 [{}] Update() ENTRY: enabled={}, busy={}...");
}

// REMOVE: ValidatePointers() detailed failure logging
if (!m_bot->IsInWorld())
{
    if (shouldLog)
    {
        TC_LOG_ERROR("module.playerbot", "❌ [{}] ValidatePointers FAILED: Bot {} IsInWorld()=false...");
    }
}

Action: Remove diagnostic TC_LOG_ERROR statements, keep essential TC_LOG_DEBUG Lines to Remove: ~40 lines of diagnostic code

---

2. TEMPORARY DOCUMENTATION FILES TO REMOVE

Files Created During Diagnosis (No Longer Needed)

REMOVE:
- AI_UPDATE_DEBUG_PATCH.txt (diagnostic patch notes)
- AI_UPDATE_ROOT_CAUSE_FOUND.md (temporary analysis)
- CONFIG_FIX_VERIFICATION.md (verification notes)
- HANDOVER.md (session handover notes)
- PHASE_2_MANAGER_DIAGNOSIS.md (diagnosis notes)
- nul (empty file artifact)

Action: Delete these files from repository root Rationale: Information consolidated into CRITICAL_BOT_ISSUES_DIAGNOSIS.md

---

3. LEGACY CODE TO REFACTOR/REMOVE

3.1 BotSession.cpp - Old Group Check Logic

File: src/modules/Playerbot/Session/BotSession.cpp Location: Lines 946-960 (before line 932)

Current Code (LEGACY):

// Lines 946-960: Group check BEFORE IsInWorld() - CAUSES ISSUE #1
if (Player* player = GetPlayer())
{
    Group* group = player->GetGroup();
    if (group)
    {
        if (BotAI* ai = GetAI())
        {
            ai->OnGroupJoined(group);
        }
    }
}

Action: Remove this block (will be replaced by state machine in Phase 1.7) Lines to Remove: ~15 lines

3.2 BotAI.cpp - First Update Static Set

File: src/modules/Playerbot/AI/BotAI.cpp Location: Lines 116-136

Current Code (LEGACY):

// Static set to track first update per bot GUID - CAUSES RACE CONDITIONS
static std::unordered_set<ObjectGuid> _initializedBots;

if (_initializedBots.find(GetBot()->GetGUID()) == _initializedBots.end())
{
    // First update logic
    _initializedBots.insert(GetBot()->GetGUID());
    // Strategy activation
}

Action: Remove static set pattern (replaced by BotInitStateMachine) Lines to Remove: ~25 lines Note: Will be replaced by proper state machine initialization in Phase 1.7

3.3 ClassAI.cpp - Ad-hoc Target Finding

File: src/modules/Playerbot/AI/ClassAI/ClassAI.cpp Location: Lines 75-79

Current Code (LEGACY):

void ClassAI::OnCombatUpdate(uint32 diff)
{
    if (!_currentCombatTarget) // Often NULL - ISSUE #2
    {
        UpdateBuffs();
        return; // No combat logic executed
    }
}

Action: Will be enhanced with robust target acquisition (Phase 2 of refactoring) Note: Mark for Phase 2 cleanup, not Phase 1

3.4 LeaderFollowBehavior.cpp - Combat Relevance

File: src/modules/Playerbot/Movement/LeaderFollowBehavior.cpp Location: Line 168

Current Code (LEGACY):

float LeaderFollowBehavior::GetRelevance(BotAI* ai) const
{
    if (bot->IsInCombat())
        return 10.0f; // PROBLEM: Still active during combat - ISSUE #2 & #3
}

Action: Will be fixed in Phase 2 of refactoring (Behavior Priority System) Note: Mark for Phase 2 cleanup, not Phase 1

---

4. UNUSED STRATEGY FILES (IF ANY)

Strategy Pattern Legacy

Location: src/modules/Playerbot/AI/Strategy/

Files to Audit:

• Check for unused strategy classes
• Check for deprecated strategy implementations
• Check for duplicate strategy logic

Action: Identify and remove if:

• ✅ Not referenced by any active code
• ✅ Superseded by Phase 2 implementations
• ✅ No unit tests depend on them

Process:

1. Run grep to find references: grep -r "ClassName" src/
2. Check CMakeLists.txt for compilation
3. If zero references and not compiled, mark for deletion
4. Document removal rationale

---

5. GIT STATUS CLEANUP

Current Untracked Files (from git status)

?? AI_UPDATE_DEBUG_PATCH.txt
?? AI_UPDATE_ROOT_CAUSE_FOUND.md
?? CONFIG_FIX_VERIFICATION.md
?? HANDOVER.md
?? PHASE_2_1_BEHAVIOR_MANAGER.md
?? PHASE_2_2_COMBAT_MOVEMENT.md
?? PHASE_2_3_COMBAT_ACTIVATION.md
?? PHASE_2_4_MANAGER_REFACTOR.md
?? PHASE_2_5_IDLE_STRATEGY.md
?? PHASE_2_6_INTEGRATION_TESTING.md
?? PHASE_2_7_CLEANUP.md
?? PHASE_2_8_DOCUMENTATION.md
?? PLAYERBOT_REFACTORING_MASTER_PLAN.md
?? nul
?? src/modules/Playerbot/AI/Strategy/IdleStrategy.cpp
?? src/modules/Playerbot/AI/Strategy/IdleStrategy.h

Actions:

1. Delete temporary files (AI_UPDATE_, CONFIG_FIX_, HANDOVER.md, nul)
2. Keep Phase 2 milestone docs (PHASE_2_X.md) - move to docs/ folder
3. Keep master plan (PLAYERBOT_REFACTORING_MASTER_PLAN.md) - move to docs/
4. Add to git IdleStrategy files (Phase 2 implementation)

---

6. CLEANUP EXECUTION PLAN

Step-by-Step Process (4 hours)

Step 1: Audit Phase (1 hour)

# 1. List all diagnostic logging
grep -r "TC_LOG_ERROR.*🔧\|🎯\|✅\|❌\|🔍" src/modules/Playerbot/

# 2. Find static initialization patterns
grep -r "static.*_initialized" src/modules/Playerbot/

# 3. List untracked files
git status --short

# 4. Find unused includes
# (Manual review of #include statements)

Step 2: Remove Diagnostic Logging (1 hour)

# Edit BotAI.cpp - remove lines 1216-1264
# Edit BehaviorManager.cpp - remove lines 60-258 (diagnostic only)
# Keep essential TC_LOG_DEBUG statements for production debugging

Step 3: Delete Temporary Files (0.5 hours)

rm AI_UPDATE_DEBUG_PATCH.txt
rm AI_UPDATE_ROOT_CAUSE_FOUND.md
rm CONFIG_FIX_VERIFICATION.md
rm HANDOVER.md
rm PHASE_2_MANAGER_DIAGNOSIS.md
rm nul

# Move milestone docs to organized location
mkdir -p docs/phase2_milestones
mv PHASE_2_*.md docs/phase2_milestones/
mv PLAYERBOT_REFACTORING_MASTER_PLAN.md docs/

Step 4: Mark Legacy Code for Refactoring (0.5 hours)

// Add comments to legacy code that will be replaced
// BotSession.cpp line 946
// TODO(Phase1.7): Remove this legacy group check - replaced by BotInitStateMachine
// This code causes Issue #1 (race condition on login)

// BotAI.cpp line 116
// TODO(Phase1.7): Remove static initialization set - replaced by state machine
// Current pattern has race conditions

// ClassAI.cpp line 75
// TODO(Phase2): Enhance target acquisition - current implementation causes Issue #2

// LeaderFollowBehavior.cpp line 168
// TODO(Phase2): Replace with behavior priority system - causes Issue #2 & #3

Step 5: Verify Cleanup (1 hour)

# 1. Rebuild to ensure no compilation errors
cd build
msbuild TrinityCore.sln /p:Configuration=Release /p:Platform=x64

# 2. Run unit tests
# (If Phase 1 tests exist)

# 3. Check git status is clean
git status

# 4. Verify no diagnostic logging in production
grep -r "TC_LOG_ERROR.*🔧\|🎯\|✅\|❌\|🔍" src/modules/Playerbot/ | wc -l
# Expected: 0 lines

---

7. CLEANUP ACCEPTANCE CRITERIA

✅ Success Metrics

1. Diagnostic Logging Removed

   • Zero emoji-based TC_LOG_ERROR statements in production code
   • Only essential TC_LOG_DEBUG statements remain
   • ~100 lines of diagnostic code removed

2. Temporary Files Deleted

   • All diagnosis documents removed from root
   • Milestone docs organized in docs/ folder
   • git status shows clean working tree

3. Legacy Code Marked

   • TODO comments added to code that will be replaced
   • Clear indication of which phase will replace it
   • No immediate functionality broken

4. Compilation Success

   • Zero compilation errors after cleanup
   • Zero new warnings introduced
   • All existing tests still pass

5. Git History Clean

   • Clear commit message documenting cleanup
   • No accidental deletions of needed files
   • Easy rollback if issues discovered

---

8. COMMIT MESSAGE TEMPLATE

[PlayerBot] Phase 1 Cleanup: Remove diagnostic logging and temporary files

Summary:
- Removed ~100 lines of diagnostic logging from BotAI.cpp and BehaviorManager.cpp
- Deleted 6 temporary diagnosis documents
- Organized Phase 2 milestone docs into docs/phase2_milestones/
- Added TODO markers to legacy code for upcoming refactoring
- No functional changes, code cleanup only

Files Modified:
- src/modules/Playerbot/AI/BotAI.cpp (~60 lines removed)
- src/modules/Playerbot/AI/BehaviorManager.cpp (~40 lines removed)

Files Deleted:
- AI_UPDATE_DEBUG_PATCH.txt
- AI_UPDATE_ROOT_CAUSE_FOUND.md
- CONFIG_FIX_VERIFICATION.md
- HANDOVER.md
- PHASE_2_MANAGER_DIAGNOSIS.md
- nul

Files Moved:
- PHASE_2_*.md → docs/phase2_milestones/
- PLAYERBOT_REFACTORING_MASTER_PLAN.md → docs/

Verification:
- ✅ Clean build (0 errors, 0 new warnings)
- ✅ All Phase 2 tests pass
- ✅ No diagnostic emoji logging in production
- ✅ Git status clean

🤖 Generated with Claude Code
Co-Authored-By: Claude <[email protected]>

---

9. RISK MITIGATION

What Could Go Wrong

Risk	Probability	Impact	Mitigation
Accidentally delete needed code	🟢 LOW	🔴 HIGH	Review diffs carefully, test after cleanup
Break compilation	🟢 LOW	🟡 MEDIUM	Rebuild immediately after changes
Remove needed diagnostic logs	🟡 MEDIUM	🟢 LOW	Keep TC_LOG_DEBUG for production
Git merge conflicts	🟢 LOW	🟡 MEDIUM	Cleanup in dedicated branch

Rollback Plan

If issues discovered after cleanup:

1. Git revert the cleanup commit
2. Selectively re-apply safe changes
3. Document what couldn't be cleaned up and why

---

10. PHASE 1 COMPLETION DEFINITION

Phase 1 is considered COMPLETE when:

1. ✅ All 10 tasks (1.1-1.10) implemented
2. ✅ All unit tests passing (Task 1.8)
3. ✅ Integration tests passing (Task 1.10)
4. ✅ Performance validation passed (Task 1.9)
5. ✅ Cleanup completed (Task 1.11) ← This document
6. ✅ Documentation updated
7. ✅ Code review passed
8. ✅ Git history clean
9. ✅ Ready for Phase 2 development

---

Cleanup Plan Created: 2025-10-06 Phase: 1.11 Duration: 4 hours Status: READY FOR EXECUTION