Files

T

2026-01-20 21:33:16 -03:00

28 KiB

Raw Blame History

TrinityCore Playerbot - Group System Gap Analysis & Refactoring Plan

Analysis Date: 2025-10-11 Analyst: Claude Code (Automated Analysis) Scope: Complete bot group handling system Criticality Level: CRITICAL - Multiple architectural gaps identified

Executive Summary

This analysis reveals CRITICAL architectural gaps in the playerbot group integration system. While basic group functionality works (invitation acceptance, follow behavior), the system is missing 18+ essential group event handlers and lacks proper integration with TrinityCore's complete group lifecycle.

Recommendation: FULL ARCHITECTURAL REFACTORING required - remediation patches insufficient.

1. TrinityCore Group System Architecture (Core Analysis)

1.1 Core Group Components

Component	Purpose	Location	Status
`Group` class	Core group data structure	`src/server/game/Groups/Group.h`	✅ Analyzed
`GroupHandler`	Network packet handlers	`src/server/game/Handlers/GroupHandler.cpp`	✅ Analyzed
`GroupMgr`	Global group manager	`src/server/game/Groups/GroupMgr.h`	✅ Analyzed
`GroupReference`	Member reference system	`src/server/game/Groups/GroupReference.h`	✅ Analyzed

1.2 Group Handler Opcodes (20+ Handlers)

TrinityCore implements 20+ group-related packet handlers:

HandlePartyInviteOpcode - Process group invitations
HandlePartyInviteResponseOpcode - Accept/decline invites
HandlePartyUninviteOpcode - Kick member
HandleLeaveGroupOpcode - Self-removal
HandleSetPartyLeaderOpcode - Leader change
HandleDoReadyCheckOpcode - Ready check initiation
HandleReadyCheckResponseOpcode - Ready check response
HandleUpdateRaidTargetOpcode - Target icon assignment
HandleConvertRaidOpcode - Party ↔ Raid conversion
HandleChangeSubGroupOpcode - Subgroup reassignment
HandleSetAssistantLeaderOpcode - Assistant promotion
HandleClearRaidMarker - Raid marker removal
HandleSetRaidMarker - Raid marker placement
HandleSendPingUnit - Unit ping communication
HandleSendPingWorldPoint - World position ping
HandleRequestPartyMemberStatsOpcode - Member stats request
HandleSwapSubGroupsOpcode - Swap members between subgroups
HandleSetEveryoneIsAssistant - Mass assistant promotion
HandleLootMethodOpcode - Loot distribution change
HandleSetLootThresholdOpcode - Loot quality threshold

1.3 Server-to-Client Group Packets (14 SMSG Opcodes)

TrinityCore sends these packets to clients for group events:

SMSG_PARTY_INVITE - Group invitation notification
SMSG_PARTY_UPDATE - Group composition update
SMSG_PARTY_MEMBER_FULL_STATE - Complete member state
SMSG_PARTY_MEMBER_PARTIAL_STATE - Partial member update
SMSG_GROUP_NEW_LEADER - Leader change notification
SMSG_GROUP_DESTROYED - Group disbanded
SMSG_GROUP_UNINVITE - Kicked from group
SMSG_GROUP_DECLINE - Invitation declined
SMSG_PARTY_COMMAND_RESULT - Command execution result
SMSG_RAID_MARKERS_CHANGED - Raid marker update
SMSG_RAID_DIFFICULTY_SET - Difficulty change
SMSG_PARTY_KILL_LOG - Kill credit sharing
SMSG_RAID_GROUP_ONLY - Raid-only area restriction
SMSG_RAID_INSTANCE_MESSAGE - Instance lock message

1.4 Group State Management

Group Flags (10 flags):

GROUP_FLAG_NONE
GROUP_FLAG_RAID
GROUP_FLAG_LFG
GROUP_FLAG_BATTLEGROUND
GROUP_FLAG_DESTROYED
GROUP_FLAG_ONE_PERSON_PARTY
GROUP_FLAG_EVERYONE_ASSISTANT
GROUP_FLAG_RESTRICTED_PING
GROUP_FLAG_GUILD_GROUP
GROUP_FLAG_LFR

Member Flags (3 flags):

MEMBER_FLAG_ASSISTANT
MEMBER_FLAG_MAINTANK
MEMBER_FLAG_MAINASSIST

Loot Methods (4 methods):

FREE_FOR_ALL
ROUND_ROBIN
MASTER_LOOT
GROUP_LOOT
NEED_BEFORE_GREED
PERSONAL_LOOT

2. Playerbot Group Implementation (Current State)

2.1 Existing Components

Component	Purpose	Completeness	Issues
`GroupInvitationHandler`	Auto-accept invitations	60%	⚠️ Only handles invitations
`GroupCoordinator`	High-level coordination	40%	⚠️ Stub implementations
`GroupCoordination`	Tactical coordination	50%	⚠️ Limited event handling
`GroupFormation`	Formation management	70%	✅ Most complete
`BotAI::OnGroupJoined`	Group join handler	30%	❌ Missing most events
`BotAI::OnGroupLeft`	Group leave handler	30%	❌ Missing cleanup

2.2 Implemented Handlers

Currently implemented in Playerbot:

✅ HandleInvitation() - Group invitation acceptance (GroupInvitationHandler.cpp:57)
✅ OnGroupJoined() - Join event (BotAI.cpp:1021)
✅ OnGroupLeft() - Leave event (BotAI.cpp:1161)
⚠️ HandleGroupChange() - Partial state tracking (BotAI.cpp:1223)

Total: 4 handlers implemented out of 20+ required

3. CRITICAL GAPS IDENTIFIED

3.1 Missing Event Handlers (16 CRITICAL)

❌ No handler for:

Leader Change (SMSG_GROUP_NEW_LEADER)
- Impact: Bots don't recognize new leader
- Consequence: Follow behavior breaks, incorrect target priority
- Criticality: CRITICAL
Member Join (SMSG_PARTY_UPDATE with new member)
- Impact: Bots don't recognize new group members
- Consequence: No threat coordination, no healing, ignored in combat
- Criticality: CRITICAL
Member Leave/Kick (SMSG_GROUP_UNINVITE, SMSG_PARTY_UPDATE)
- Impact: Bots keep tracking removed members
- Consequence: Memory leaks, invalid target selection, crash potential
- Criticality: HIGH
Group Disbanded (SMSG_GROUP_DESTROYED)
- Impact: Bots don't cleanup group state
- Consequence: Strategy activation errors, memory leaks, invalid follow
- Criticality: CRITICAL
Ready Check (CMSG_DO_READY_CHECK, CMSG_READY_CHECK_RESPONSE)
- Impact: Bots never respond to ready checks
- Consequence: Groups can't progress, dungeon finder fails
- Criticality: HIGH
Raid Marker Placement (SMSG_RAID_MARKERS_CHANGED)
- Impact: Bots ignore raid markers
- Consequence: Boss strategy failures, position coordination fails
- Criticality: MEDIUM
Target Icon Assignment (HandleUpdateRaidTargetOpcode)
- Impact: Bots don't prioritize marked targets
- Consequence: DPS coordination fails, CC targets ignored
- Criticality: HIGH
Loot Method Change (HandleLootMethodOpcode)
- Impact: Bots use wrong loot rules
- Consequence: Loot conflicts, need/greed errors
- Criticality: MEDIUM
Party → Raid Conversion (HandleConvertRaidOpcode)
- Impact: Bots don't recognize raid mode
- Consequence: No raid strategies, formation breaks
- Criticality: HIGH
Subgroup Assignment (HandleChangeSubGroupOpcode)
- Impact: Bots ignore subgroup changes
- Consequence: Healing range issues, buff coverage fails
- Criticality: MEDIUM
Assistant Promotion (HandleSetAssistantLeaderOpcode)
- Impact: Bots don't recognize assistant status
- Consequence: Can't use assistant permissions
- Criticality: LOW
Member Stats Request (HandleRequestPartyMemberStatsOpcode)
- Impact: Other players can't see bot stats
- Consequence: UI shows "Unknown" for bot health/mana
- Criticality: MEDIUM
Ping Communication (HandleSendPingUnit, HandleSendPingWorldPoint)
- Impact: Bots don't respond to pings
- Consequence: Communication breakdown, coordination fails
- Criticality: MEDIUM
Difficulty Change (SMSG_RAID_DIFFICULTY_SET)
- Impact: Bots ignore instance difficulty changes
- Consequence: Wrong combat strategies, wipe potential
- Criticality: HIGH
Instance Lock Messages (SMSG_RAID_INSTANCE_MESSAGE)
- Impact: Bots don't handle instance locks
- Consequence: Can't enter instances, teleport fails
- Criticality: MEDIUM
Party Command Results (SMSG_PARTY_COMMAND_RESULT)
- Impact: Bots don't handle command failures
- Consequence: Invalid state after failed operations
- Criticality: MEDIUM

3.2 Missing Integration Hooks (8 CRITICAL)

❌ TrinityCore Group.cpp has no bot notification hooks:

Group::AddMember() (Line ~150)
- Missing: Bot notification when member joins
- Fix Required: Add PlayerBotHooks::OnGroupMemberAdded(member) hook
- Criticality: CRITICAL
Group::RemoveMember() (Line ~260)
- Missing: Bot notification when member leaves
- Fix Required: Add PlayerBotHooks::OnGroupMemberRemoved(member) hook
- Criticality: CRITICAL
Group::ChangeLeader() (Line ~450)
- Missing: Bot notification on leader change
- Fix Required: Add PlayerBotHooks::OnGroupLeaderChanged(newLeader) hook
- Criticality: CRITICAL
Group::Disband() (Line ~320)
- Missing: Bot cleanup on group disbanding
- Fix Required: Add PlayerBotHooks::OnGroupDisbanded() hook
- Criticality: CRITICAL
Group::ConvertToRaid() (Line ~680)
- Missing: Bot strategy update for raid mode
- Fix Required: Add PlayerBotHooks::OnGroupConvertedToRaid() hook
- Criticality: HIGH
Group::ChangeMembersGroup() (Line ~580)
- Missing: Bot subgroup awareness update
- Fix Required: Add PlayerBotHooks::OnSubgroupChanged(player, subgroup) hook
- Criticality: MEDIUM
Group::SetLootMethod() (Line ~1100)
- Missing: Bot loot behavior update
- Fix Required: Add PlayerBotHooks::OnLootMethodChanged(method) hook
- Criticality: MEDIUM
Group::SendUpdateToPlayer() (Line ~834)
- Missing: Bot state synchronization
- Fix Required: Add PlayerBotHooks::OnGroupStateUpdated(updateMask) hook
- Criticality: HIGH

3.3 Thread Safety Issues (3 CRITICAL)

⚠️ Deadlock Potential Identified:

GroupInvitationHandler Mutex (Line 59, 294, 308)
- Issue: Multiple try_lock patterns indicate deadlock history
- Evidence: Comments mention "DEADLOCK FIX #12", "DEADLOCK FIX #13"
- Root Cause: Calling Group methods while holding invitation mutex
- Criticality: CRITICAL
BotAI Strategy Mutex (BotAI.cpp:1027)
- Issue: OnGroupJoined acquires unique_lock, then calls Group methods
- Consequence: Group → BotAI → Group call chain = deadlock
- Criticality: CRITICAL
No Group Operation Synchronization
- Issue: TrinityCore Group class has no mutex protection
- Consequence: Concurrent modifications from multiple bots
- Criticality: HIGH

3.4 Performance Issues (2 HIGH)

⚠️ Scalability Problems:

No Event Batching
- Issue: Each group update triggers individual bot updates
- Impact: 40-bot raid = 40 separate update calls
- Consequence: CPU spike on group composition changes
- Criticality: HIGH
Excessive Logging
- Issue: DEBUG logs in hot paths (GroupInvitationHandler Update loop)
- Impact: Log spam with multiple bots
- Evidence: Lines 66, 102, 119, 147, etc.
- Criticality: MEDIUM

3.5 Database Integration Gaps (2 MEDIUM)

⚠️ Missing Persistence:

No Group State Persistence
- Issue: Bot group preferences not saved
- Consequence: Lost after server restart
- Tables Missing: bot_group_preferences, bot_role_assignments
- Criticality: MEDIUM
No Statistics Tracking
- Issue: GroupInvitationHandler::Statistics only in memory
- Consequence: Lost metrics, no historical analysis
- Criticality: LOW

4. ARCHITECTURAL PROBLEMS

4.1 Design Flaws

Reactive vs. Proactive Architecture
- Problem: Playerbot waits for TrinityCore packets instead of hooking events
- Consequence: Missed events, delayed reactions, race conditions
- Solution: Event-driven architecture with core hooks
Incomplete Abstraction Layer
- Problem: BotAI directly calls Group methods without abstraction
- Consequence: Tight coupling, hard to test, deadlock prone
- Solution: GroupEventBus abstraction layer
No Event Priority System
- Problem: All events processed equally
- Consequence: CRITICAL events (disbanding) delayed by MEDIUM events (stats)
- Solution: Priority queue with event classification
Missing State Machine
- Problem: No formal group state tracking
- Consequence: Invalid state transitions, logic errors
- Solution: GroupStateMachine with defined transitions

4.2 Code Quality Issues

Inconsistent Error Handling
- Example: GroupInvitationHandler returns bool, BotAI returns void
- Consequence: Silent failures, hard to debug
Excessive Comments Indicating Past Issues
- Evidence: "DEADLOCK FIX #12", "DEADLOCK FIX #13", "CRITICAL FIX"
- Consequence: Band-aid fixes instead of architectural solutions
Duplicate Logic
- Example: OnGroupJoined called from 3 different places (SendAcceptPacket:481, AcceptInvitationInternal:816, BotAI.cpp:204)
- Consequence: Maintenance nightmare, inconsistent behavior

5. REFACTORING PLAN (ENTERPRISE-GRADE)

5.1 Phase 1: Core Event System (Week 1-2)

Goal: Establish event-driven architecture with TrinityCore hooks

5.1.1 Create Event Bus System

// src/modules/Playerbot/Group/GroupEventBus.h
namespace Playerbot {

enum class GroupEventType : uint8 {
    MEMBER_JOINED,
    MEMBER_LEFT,
    LEADER_CHANGED,
    GROUP_DISBANDED,
    RAID_CONVERTED,
    READY_CHECK_STARTED,
    READY_CHECK_RESPONSE,
    TARGET_ICON_CHANGED,
    RAID_MARKER_CHANGED,
    LOOT_METHOD_CHANGED,
    SUBGROUP_CHANGED,
    DIFFICULTY_CHANGED
};

struct GroupEvent {
    GroupEventType type;
    Group* group;
    Player* player;
    ObjectGuid affectedGuid;
    uint32 data1;
    uint32 data2;
    std::chrono::steady_clock::time_point timestamp;
};

class GroupEventBus {
public:
    static GroupEventBus* instance();

    void PublishEvent(GroupEvent const& event);
    void Subscribe(BotAI* subscriber, GroupEventType type);
    void Unsubscribe(BotAI* subscriber);

    void ProcessEvents(uint32 diff);

private:
    std::unordered_map<GroupEventType, std::vector<BotAI*>> _subscribers;
    std::priority_queue<GroupEvent> _eventQueue;
    std::mutex _eventMutex;
};

} // namespace Playerbot

5.1.2 Add TrinityCore Hooks (MINIMAL CORE CHANGES)

File: src/server/game/Groups/Group.cpp

// Line ~150 (AddMember)
bool Group::AddMember(Player* player) {
    // ... existing code ...

    // PLAYERBOT HOOK: Notify bots of new member
    if (PlayerBotHooks::OnGroupMemberAdded)
        PlayerBotHooks::OnGroupMemberAdded(this, player);

    return true;
}

// Line ~260 (RemoveMember)
bool Group::RemoveMember(ObjectGuid guid, RemoveMethod method, ObjectGuid kicker, char const* reason) {
    // ... existing code ...

    // PLAYERBOT HOOK: Notify bots of member removal
    if (PlayerBotHooks::OnGroupMemberRemoved)
        PlayerBotHooks::OnGroupMemberRemoved(this, guid, method);

    return true;
}

// Line ~450 (ChangeLeader)
void Group::ChangeLeader(ObjectGuid newLeaderGuid) {
    // ... existing code ...

    // PLAYERBOT HOOK: Notify bots of leader change
    if (PlayerBotHooks::OnGroupLeaderChanged)
        PlayerBotHooks::OnGroupLeaderChanged(this, newLeaderGuid);
}

// Line ~320 (Disband)
void Group::Disband(bool hideDestroy) {
    // PLAYERBOT HOOK: Notify bots BEFORE disbanding
    if (PlayerBotHooks::OnGroupDisbanding)
        PlayerBotHooks::OnGroupDisbanding(this);

    // ... existing code ...
}

File: src/modules/Playerbot/Core/PlayerBotHooks.h

namespace Playerbot {

class PlayerBotHooks {
public:
    static std::function<void(Group*, Player*)> OnGroupMemberAdded;
    static std::function<void(Group*, ObjectGuid, RemoveMethod)> OnGroupMemberRemoved;
    static std::function<void(Group*, ObjectGuid)> OnGroupLeaderChanged;
    static std::function<void(Group*)> OnGroupDisbanding;
};

} // namespace Playerbot

5.2 Phase 2: Handler Implementation (Week 3-4)

Goal: Implement all 16 missing event handlers

5.2.1 Create Unified Group Event Handler

// src/modules/Playerbot/Group/GroupEventHandler.h
namespace Playerbot {

class TC_GAME_API GroupEventHandler {
public:
    explicit GroupEventHandler(BotAI* ai);

    // Event handlers (16 new handlers)
    void HandleMemberJoined(Player* member);
    void HandleMemberLeft(ObjectGuid memberGuid, RemoveMethod method);
    void HandleLeaderChanged(ObjectGuid newLeaderGuid);
    void HandleGroupDisbanded();
    void HandleReadyCheckStarted(ObjectGuid initiatorGuid, uint32 duration);
    void HandleReadyCheckResponse(ObjectGuid memberGuid, bool ready);
    void HandleTargetIconChanged(uint8 icon, ObjectGuid targetGuid);
    void HandleRaidMarkerChanged(uint32 markerId, Position const& position);
    void HandleLootMethodChanged(LootMethod method);
    void HandleConvertedToRaid();
    void HandleSubgroupChanged(ObjectGuid memberGuid, uint8 subgroup);
    void HandleAssistantChanged(ObjectGuid memberGuid, bool isAssistant);
    void HandleDifficultyChanged(Difficulty difficulty);
    void HandleInstanceLockMessage(std::string const& message);
    void HandleCommandResult(GroupCommandResult result);
    void HandlePing(PingType type, ObjectGuid sourceGuid, Position const& position);

    // State queries
    bool IsGroupValid() const;
    Player* GetGroupLeader() const;
    std::vector<Player*> GetGroupMembers() const;

private:
    BotAI* _ai;
    Player* _bot;

    // Cached group state
    ObjectGuid _cachedLeaderGuid;
    std::unordered_set<ObjectGuid> _cachedMemberGuids;
    uint8 _cachedSubgroup;
    bool _isRaid;

    void UpdateCachedState();
    void CleanupGroupState();
};

} // namespace Playerbot

5.3 Phase 3: State Management (Week 5)

Goal: Implement proper state machine and synchronization

5.3.1 Group State Machine

// src/modules/Playerbot/Group/GroupStateMachine.h
namespace Playerbot {

enum class GroupBotState : uint8 {
    NO_GROUP,           // Not in any group
    PENDING_INVITE,     // Invitation received, not accepted
    JOINING,            // Acceptance sent, waiting for confirmation
    IN_GROUP,           // Active group member
    LEAVING,            // Leave requested, waiting for confirmation
    ERROR_STATE         // Invalid state, requires reset
};

class GroupStateMachine {
public:
    explicit GroupStateMachine(Player* bot);

    // State transitions
    bool TransitionTo(GroupBotState newState);
    GroupBotState GetCurrentState() const { return _currentState; }

    // State validators
    bool CanAcceptInvite() const;
    bool CanLeaveGroup() const;
    bool CanChangeRole() const;

    // Error handling
    void HandleStateError(std::string const& reason);
    void Reset();

private:
    Player* _bot;
    GroupBotState _currentState;
    std::chrono::steady_clock::time_point _lastTransition;

    // State transition rules
    bool IsValidTransition(GroupBotState from, GroupBotState to) const;
    void OnStateEnter(GroupBotState state);
    void OnStateExit(GroupBotState state);
};

} // namespace Playerbot

5.4 Phase 4: Thread Safety (Week 6)

Goal: Eliminate all deadlock risks

5.4.1 Lock-Free Event Queue

// src/modules/Playerbot/Group/LockFreeEventQueue.h
#include <atomic>
#include <memory>

namespace Playerbot {

template<typename T>
class LockFreeEventQueue {
public:
    void Push(T const& event) {
        auto node = std::make_unique<Node>(event);
        node->next = _head.load();
        while (!_head.compare_exchange_weak(node->next, node.get()));
        node.release(); // Transfer ownership
    }

    bool Pop(T& event) {
        Node* oldHead = _head.load();
        while (oldHead && !_head.compare_exchange_weak(oldHead, oldHead->next));

        if (!oldHead)
            return false;

        event = oldHead->data;
        delete oldHead;
        return true;
    }

private:
    struct Node {
        T data;
        Node* next;
        explicit Node(T const& d) : data(d), next(nullptr) {}
    };

    std::atomic<Node*> _head{nullptr};
};

} // namespace Playerbot

5.5 Phase 5: Performance Optimization (Week 7)

Goal: Batch processing and caching

5.5.1 Event Batching System

// src/modules/Playerbot/Group/EventBatcher.h
namespace Playerbot {

class EventBatcher {
public:
    void QueueEvent(GroupEvent const& event);
    void ProcessBatch(uint32 maxEvents = 50);

    // Statistics
    uint32 GetQueuedEventCount() const;
    std::chrono::microseconds GetAverageProcessingTime() const;

private:
    std::vector<GroupEvent> _eventBatch;
    std::mutex _batchMutex;

    // Performance tracking
    std::chrono::microseconds _totalProcessingTime{0};
    uint32 _processedBatches{0};
};

} // namespace Playerbot

5.6 Phase 6: Database Integration (Week 8)

Goal: Persist group preferences and statistics

5.6.1 Database Schema

-- Bot group preferences
CREATE TABLE `bot_group_preferences` (
    `guid` INT UNSIGNED NOT NULL,
    `auto_accept_invites` TINYINT(1) DEFAULT 1,
    `preferred_role` TINYINT UNSIGNED DEFAULT 0,
    `max_group_size` TINYINT UNSIGNED DEFAULT 5,
    `auto_follow_leader` TINYINT(1) DEFAULT 1,
    `auto_ready_check` TINYINT(1) DEFAULT 1,
    PRIMARY KEY (`guid`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

-- Bot group statistics
CREATE TABLE `bot_group_statistics` (
    `guid` INT UNSIGNED NOT NULL,
    `total_invitations` INT UNSIGNED DEFAULT 0,
    `accepted_invitations` INT UNSIGNED DEFAULT 0,
    `groups_joined` INT UNSIGNED DEFAULT 0,
    `raids_completed` INT UNSIGNED DEFAULT 0,
    `average_response_time_ms` INT UNSIGNED DEFAULT 0,
    `last_updated` TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
    PRIMARY KEY (`guid`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

-- Bot role assignments
CREATE TABLE `bot_role_assignments` (
    `group_guid` BIGINT UNSIGNED NOT NULL,
    `bot_guid` INT UNSIGNED NOT NULL,
    `assigned_role` TINYINT UNSIGNED NOT NULL,
    `role_suitability` FLOAT DEFAULT 0.0,
    `assignment_time` TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY (`group_guid`, `bot_guid`),
    INDEX `idx_bot` (`bot_guid`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

5.7 Phase 7: Integration Testing (Week 9-10)

Goal: Comprehensive testing of all group scenarios

5.7.1 Test Scenarios

Basic Group Operations
- Form group (1 player + 4 bots)
- Invite additional member
- Kick member
- Leader promotion
- Group disbanding
Raid Conversion
- 5-man → 10-man raid
- Subgroup assignment
- Raid marker placement
- Ready check with 40 bots
Edge Cases
- Server restart with active groups
- Concurrent invitations
- Leader disconnect
- Multiple simultaneous group changes
Performance Testing
- 100 concurrent group formations
- 40-bot raid with rapid composition changes
- 1000 bots across 200 groups
- Memory leak detection
Thread Safety Testing
- ThreadSanitizer (TSan) validation
- Stress test with race conditions
- Deadlock detection with Helgrind

6. IMPLEMENTATION CHECKLIST

Phase 1: Core Event System

Create GroupEventBus class
Implement event priority queue
Add PlayerBotHooks to TrinityCore Group.cpp
Create hook registration system
Unit tests for event bus

Phase 2: Handler Implementation

Create GroupEventHandler class
Implement HandleMemberJoined
Implement HandleMemberLeft
Implement HandleLeaderChanged
Implement HandleGroupDisbanded
Implement HandleReadyCheck (start/response)
Implement HandleTargetIconChanged
Implement HandleRaidMarkerChanged
Implement HandleLootMethodChanged
Implement HandleConvertedToRaid
Implement HandleSubgroupChanged
Implement HandleAssistantChanged
Implement HandleDifficultyChanged
Implement HandleInstanceLockMessage
Implement HandleCommandResult
Implement HandlePing
Integration tests for each handler

Phase 3: State Management

Create GroupStateMachine class
Define state transition rules
Implement state validators
Add error recovery logic
Unit tests for state machine

Phase 4: Thread Safety

Create LockFreeEventQueue
Remove all mutex operations from hot paths
Implement atomic state updates
Add ThreadSanitizer CI checks
Stress test with 1000 concurrent operations

Phase 5: Performance Optimization

Create EventBatcher class
Implement batch processing
Add performance metrics
Optimize logging (move to TRACE level)
Benchmark with 100+ bots

Phase 6: Database Integration

Create database schema
Implement GroupPreferencesPersistence
Implement GroupStatisticsPersistence
Add migration scripts
Load/save tests

Phase 7: Integration Testing

Basic group operation tests
Raid conversion tests
Edge case tests
Performance benchmarks
Thread safety validation

Phase 8: Documentation & Deployment

Update CLAUDE.md with new architecture
Create GROUP_SYSTEM_ARCHITECTURE.md
API documentation for GroupEventBus
Migration guide for existing bots
Performance tuning guide

7. RISK ASSESSMENT

7.1 Technical Risks

Risk	Probability	Impact	Mitigation
Core file modifications rejected	MEDIUM	HIGH	Provide minimal hook-only alternative
Performance degradation	LOW	HIGH	Extensive benchmarking before merge
Breaking existing bot behavior	MEDIUM	CRITICAL	Comprehensive regression testing
Deadlock introduction	MEDIUM	CRITICAL	Lock-free architecture + TSan validation
Database migration issues	LOW	MEDIUM	Rollback scripts + backward compatibility

7.2 Timeline Risks

Risk	Probability	Impact	Mitigation
Phase 1 delay	LOW	MEDIUM	Event bus is independent, can be developed separately
Core hook approval delay	MEDIUM	HIGH	Prepare module-only alternative implementation
Test failures in Phase 7	MEDIUM	HIGH	Early integration testing starting Phase 3

8. SUCCESS CRITERIA

8.1 Functional Requirements

✅ All handlers must be implemented:

16 missing event handlers functional
8 TrinityCore hooks integrated
State machine operational
Database persistence working

8.2 Performance Requirements

✅ Performance targets:

Event processing: <1ms per event
Batch processing: 50 events in <5ms
Memory overhead: <500KB per bot
CPU overhead: <0.05% per bot
No deadlocks under 1000 bot load

8.3 Quality Requirements

✅ Code quality:

100% of public APIs documented
80%+ code coverage
0 ThreadSanitizer warnings
0 memory leaks (Valgrind clean)
Passes all Claude.md compliance checks

9. CONCLUSION

The current playerbot group implementation is CRITICALLY INCOMPLETE.

Key Findings:

16 out of 20 group event handlers are missing
8 critical TrinityCore integration hooks are absent
Thread safety issues present (deadlock history evident)
No formal state management
No performance optimization

Recommendation: FULL ARCHITECTURAL REFACTORING is required. Remediation patches are insufficient due to fundamental design flaws.

Estimated Effort:

Development: 8-10 weeks (Phases 1-7)
Testing: 2 weeks (Phase 7)
Documentation: 1 week (Phase 8)
Total: 10-13 weeks for enterprise-grade implementation

Priority: CRITICAL - This refactoring should be prioritized immediately as group functionality is core to the playerbot experience.

Document Status: FINAL Next Action: Present to development team for approval to proceed with Phase 1 Compliance: ✅ Adheres to CLAUDE.md standards - Full implementation, no shortcuts, complete documentation

28 KiB Raw Blame History