simplify/enhance collections

internal/collections/collections.go

@@ -0,0 +1,524 @@
+package collections
+
+import (
+	"fmt"
+	"sync"
+	"time"
+
+	"eq2emu/internal/database"
+	"eq2emu/internal/packets"
+)
+
+// Manager provides centralized management for the collections system
+// following the SIMPLIFICATION.md methodology while maintaining C++ API compatibility
+type Manager struct {
+	// Core components
+	masterList *MasterList
+	db         *database.Database
+
+	// Thread safety
+	mutex sync.RWMutex
+
+	// Performance monitoring
+	stats struct {
+		// Collection operations
+		CollectionsLoaded      int64
+		CollectionsSaved       int64
+		CollectionItemsUpdated int64
+
+		// Player operations  
+		PlayersLoaded   int64
+		PlayersUpdated  int64
+		ItemChecks      int64
+
+		// Packet operations
+		PacketsSent   int64
+		PacketErrors  int64
+		PacketUpdates int64
+
+		// Cache performance
+		CacheHits   int64
+		CacheMisses int64
+	}
+
+	// Configuration
+	config struct {
+		EnableCaching        bool
+		EnableStatistics     bool
+		EnableValidation     bool
+		MaxPlayersToTrack    int
+		AutoSaveInterval     time.Duration
+		PacketBuilderTimeout time.Duration
+	}
+}
+
+// NewManager creates a new collections manager with default configuration
+func NewManager(db *database.Database) *Manager {
+	m := &Manager{
+		masterList: NewMasterList(),
+		db:         db,
+	}
+
+	// Set default configuration
+	m.config.EnableCaching = true
+	m.config.EnableStatistics = true
+	m.config.EnableValidation = true
+	m.config.MaxPlayersToTrack = 1000
+	m.config.AutoSaveInterval = 5 * time.Minute
+	m.config.PacketBuilderTimeout = 30 * time.Second
+
+	return m
+}
+
+// Initialize loads all collections from database and prepares the manager
+func (m *Manager) Initialize() error {
+	if m.db == nil {
+		return fmt.Errorf("database connection is required")
+	}
+
+	err := m.masterList.LoadAllCollections(m.db)
+	if err != nil {
+		return fmt.Errorf("failed to load collections: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.CollectionsLoaded = int64(m.masterList.Size())
+	}
+
+	return nil
+}
+
+// === C++ API Compatibility Methods ===
+
+// AddCollection adds a collection to the master list (C++ API compatibility)
+func (m *Manager) AddCollection(collection *Collection) bool {
+	if collection == nil {
+		return false
+	}
+
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if m.config.EnableStatistics {
+		m.stats.CollectionsLoaded++
+	}
+
+	return m.masterList.AddCollection(collection)
+}
+
+// GetCollection retrieves a collection by ID (C++ API compatibility)
+func (m *Manager) GetCollection(id int32) *Collection {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	if m.config.EnableStatistics {
+		if collection := m.masterList.GetCollection(id); collection != nil {
+			m.stats.CacheHits++
+			return collection
+		}
+		m.stats.CacheMisses++
+		return nil
+	}
+
+	return m.masterList.GetCollection(id)
+}
+
+// ClearCollections removes all collections (C++ API compatibility)
+func (m *Manager) ClearCollections() {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	m.masterList.ClearCollections()
+}
+
+// Size returns the number of collections (C++ API compatibility)
+func (m *Manager) Size() int32 {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	return int32(m.masterList.Size())
+}
+
+// NeedsItem checks if any collection needs the specified item (C++ API compatibility)
+func (m *Manager) NeedsItem(itemID int32) bool {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	if m.config.EnableStatistics {
+		m.stats.ItemChecks++
+	}
+
+	return m.masterList.NeedsItem(itemID)
+}
+
+// GetMasterList returns the master collection list (C++ API compatibility)
+func (m *Manager) GetMasterList() *MasterList {
+	return m.masterList
+}
+
+// === Enhanced Go Methods ===
+
+// GetCollectionsSafe retrieves all collections with thread safety
+func (m *Manager) GetCollectionsSafe() map[int32]*Collection {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	return m.masterList.GetAllCollections()
+}
+
+// UpdateCollection updates an existing collection with validation
+func (m *Manager) UpdateCollection(collection *Collection) error {
+	if collection == nil {
+		return fmt.Errorf("collection cannot be nil")
+	}
+
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if m.config.EnableValidation {
+		if collection.GetID() <= 0 {
+			return fmt.Errorf("invalid collection ID: %d", collection.GetID())
+		}
+		if len(collection.GetName()) == 0 {
+			return fmt.Errorf("collection name cannot be empty")
+		}
+	}
+
+	err := m.masterList.UpdateCollection(collection)
+	if err != nil {
+		return err
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.CollectionsLoaded++
+	}
+
+	return nil
+}
+
+// === Packet Building Methods ===
+
+// SendCollectionUpdate builds and returns collection update packet data (C++ API compatibility)
+func (m *Manager) SendCollectionUpdate(characterID int32, clientVersion uint32, playerCollections map[int32]*Collection) ([]byte, error) {
+	packet, exists := packets.GetPacket("WS_CollectionUpdate")
+	if !exists {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to get WS_CollectionUpdate packet structure: packet not found")
+	}
+
+	// Build collection array
+	collectionArray := make([]map[string]interface{}, 0, len(playerCollections))
+	for _, collection := range playerCollections {
+		// Build collection items array
+		itemsArray := make([]map[string]interface{}, 0, len(collection.CollectionItems))
+		for _, item := range collection.CollectionItems {
+			itemsArray = append(itemsArray, map[string]interface{}{
+				"item_flag": item.Found,
+			})
+		}
+
+		collectionData := map[string]interface{}{
+			"collection_name":    collection.GetName(),
+			"collection_category": collection.GetCategory(),
+			"completed":          collection.Completed,
+			"collection_id":      collection.GetID(),
+			"level":              collection.GetLevel(),
+			"ready_to_turn_in":   collection.GetIsReadyToTurnIn(),
+			"num_items":          len(collection.CollectionItems),
+			"items_array":        itemsArray,
+		}
+		collectionArray = append(collectionArray, collectionData)
+	}
+
+	data := map[string]interface{}{
+		"num_collections":      len(playerCollections),
+		"collections_array":    collectionArray,
+		"new_collection_flag":  1,
+	}
+
+	builder := packets.NewPacketBuilder(packet, clientVersion, 0)
+	packetData, err := builder.Build(data)
+	if err != nil {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to build collection update packet: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.PacketsSent++
+		m.stats.PacketUpdates++
+	}
+
+	return packetData, nil
+}
+
+// SendCollectionFilter builds and returns collection filter packet data (C++ API compatibility)
+func (m *Manager) SendCollectionFilter(characterID int32, clientVersion uint32, itemID int32, collectionsForItem []*Collection) ([]byte, error) {
+	packet, exists := packets.GetPacket("WS_CollectionFilter")
+	if !exists {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to get WS_CollectionFilter packet structure: packet not found")
+	}
+
+	// Build filter array
+	filterArray := make([]map[string]interface{}, 0, len(collectionsForItem))
+	for _, collection := range collectionsForItem {
+		collectionItem := collection.GetCollectionItemByItemID(itemID)
+		if collectionItem != nil {
+			filterData := map[string]interface{}{
+				"collection_id":        collection.GetID(),
+				"collection_item_num":  collectionItem.Index,
+			}
+			filterArray = append(filterArray, filterData)
+		}
+	}
+
+	data := map[string]interface{}{
+		"num_filters":    len(filterArray),
+		"filters_array":  filterArray,
+		"unknown3":       m.Size(), // Player collection count
+	}
+
+	builder := packets.NewPacketBuilder(packet, clientVersion, 0)
+	packetData, err := builder.Build(data)
+	if err != nil {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to build collection filter packet: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.PacketsSent++
+	}
+
+	return packetData, nil
+}
+
+// SendCollectionItem builds and returns collection item packet data (C++ API compatibility)
+func (m *Manager) SendCollectionItem(characterID int32, clientVersion uint32, collectionID int32, itemID int32) ([]byte, error) {
+	packet, exists := packets.GetPacket("WS_CollectionItem")
+	if !exists {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to get WS_CollectionItem packet structure: packet not found")
+	}
+
+	collection := m.GetCollection(collectionID)
+	if collection == nil {
+		return nil, fmt.Errorf("collection %d not found", collectionID)
+	}
+
+	collectionItem := collection.GetCollectionItemByItemID(itemID)
+	if collectionItem == nil {
+		return nil, fmt.Errorf("collection item %d not found in collection %d", itemID, collectionID)
+	}
+
+	data := map[string]interface{}{
+		"collection_id":         collectionID,
+		"collection_item_num":   collectionItem.Index,
+		"name":                  collection.GetName(),
+		"description":           collection.GetCategory(),
+		"level":                 collection.GetLevel(),
+		"max_coin":              collection.RewardCoin,
+		"min_coin":              collection.RewardCoin,
+	}
+
+	builder := packets.NewPacketBuilder(packet, clientVersion, 0)
+	packetData, err := builder.Build(data)
+	if err != nil {
+		if m.config.EnableStatistics {
+			m.stats.PacketErrors++
+		}
+		return nil, fmt.Errorf("failed to build collection item packet: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.PacketsSent++
+	}
+
+	return packetData, nil
+}
+
+// === Player Collection Management ===
+
+// CreatePlayerList creates a new player collection list
+func (m *Manager) CreatePlayerList(characterID int32) *PlayerList {
+	return NewPlayerList(characterID, m.db)
+}
+
+// LoadPlayerCollections loads all collections for a player
+func (m *Manager) LoadPlayerCollections(characterID int32) (*PlayerList, error) {
+	playerList := NewPlayerList(characterID, m.db)
+	err := playerList.LoadPlayerCollections(m.masterList)
+	if err != nil {
+		return nil, fmt.Errorf("failed to load player collections: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.PlayersLoaded++
+	}
+
+	return playerList, nil
+}
+
+// SavePlayerCollections saves all player collection progress
+func (m *Manager) SavePlayerCollections(playerList *PlayerList) error {
+	if playerList == nil {
+		return fmt.Errorf("player list cannot be nil")
+	}
+
+	err := playerList.SaveAllCollections()
+	if err != nil {
+		return fmt.Errorf("failed to save player collections: %v", err)
+	}
+
+	if m.config.EnableStatistics {
+		m.stats.CollectionsSaved++
+	}
+
+	return nil
+}
+
+// === Statistics and Monitoring ===
+
+// GetStatistics returns comprehensive manager statistics
+func (m *Manager) GetStatistics() map[string]interface{} {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	stats := make(map[string]interface{})
+
+	// Manager stats
+	stats["collections_loaded"] = m.stats.CollectionsLoaded
+	stats["collections_saved"] = m.stats.CollectionsSaved
+	stats["collection_items_updated"] = m.stats.CollectionItemsUpdated
+
+	// Player stats  
+	stats["players_loaded"] = m.stats.PlayersLoaded
+	stats["players_updated"] = m.stats.PlayersUpdated
+	stats["item_checks"] = m.stats.ItemChecks
+
+	// Packet stats
+	stats["packets_sent"] = m.stats.PacketsSent
+	stats["packet_errors"] = m.stats.PacketErrors
+	stats["packet_updates"] = m.stats.PacketUpdates
+
+	// Cache stats
+	stats["cache_hits"] = m.stats.CacheHits
+	stats["cache_misses"] = m.stats.CacheMisses
+
+	if m.stats.CacheHits+m.stats.CacheMisses > 0 {
+		hitRate := float64(m.stats.CacheHits) / float64(m.stats.CacheHits+m.stats.CacheMisses) * 100
+		stats["cache_hit_rate"] = fmt.Sprintf("%.2f%%", hitRate)
+	}
+
+	// Master list stats
+	masterStats := m.masterList.GetStatistics()
+	for key, value := range masterStats {
+		stats["master_"+key] = value
+	}
+
+	// Configuration
+	stats["config_caching_enabled"] = m.config.EnableCaching
+	stats["config_statistics_enabled"] = m.config.EnableStatistics
+	stats["config_validation_enabled"] = m.config.EnableValidation
+
+	return stats
+}
+
+// ResetStatistics clears all performance counters
+func (m *Manager) ResetStatistics() {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	m.stats = struct {
+		CollectionsLoaded      int64
+		CollectionsSaved       int64
+		CollectionItemsUpdated int64
+		PlayersLoaded          int64
+		PlayersUpdated         int64
+		ItemChecks             int64
+		PacketsSent            int64
+		PacketErrors           int64
+		PacketUpdates          int64
+		CacheHits              int64
+		CacheMisses            int64
+	}{}
+}
+
+// === Configuration Management ===
+
+// SetConfiguration updates manager configuration
+func (m *Manager) SetConfiguration(config map[string]interface{}) {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if val, ok := config["enable_caching"].(bool); ok {
+		m.config.EnableCaching = val
+	}
+	if val, ok := config["enable_statistics"].(bool); ok {
+		m.config.EnableStatistics = val
+	}
+	if val, ok := config["enable_validation"].(bool); ok {
+		m.config.EnableValidation = val
+	}
+	if val, ok := config["max_players_to_track"].(int); ok {
+		m.config.MaxPlayersToTrack = val
+	}
+	if val, ok := config["auto_save_interval"].(time.Duration); ok {
+		m.config.AutoSaveInterval = val
+	}
+}
+
+// GetConfiguration returns current manager configuration
+func (m *Manager) GetConfiguration() map[string]interface{} {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	return map[string]interface{}{
+		"enable_caching":         m.config.EnableCaching,
+		"enable_statistics":      m.config.EnableStatistics,
+		"enable_validation":      m.config.EnableValidation,
+		"max_players_to_track":   m.config.MaxPlayersToTrack,
+		"auto_save_interval":     m.config.AutoSaveInterval,
+		"packet_builder_timeout": m.config.PacketBuilderTimeout,
+	}
+}
+
+// === Validation and Health Checks ===
+
+// ValidateCollections performs comprehensive validation
+func (m *Manager) ValidateCollections() []string {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	return m.masterList.ValidateCollections()
+}
+
+// IsHealthy returns true if the manager is in a healthy state
+func (m *Manager) IsHealthy() bool {
+	m.mutex.RLock()
+	defer m.mutex.RUnlock()
+
+	if m.db == nil {
+		return false
+	}
+
+	if m.masterList == nil {
+		return false
+	}
+
+	if m.config.EnableValidation {
+		issues := m.masterList.ValidateCollections()
+		return len(issues) == 0
+	}
+
+	return true
+}

internal/collections/collections_test.go

@@ -0,0 +1,653 @@
+package collections
+
+import (
+	"testing"
+)
+
+// MockDatabase implements a simple in-memory database for testing
+type MockDatabase struct {
+	collections map[int32]map[string]interface{}
+	collectionItems map[int32][]map[string]interface{}
+	collectionRewards map[int32][]map[string]interface{}
+	characterCollections map[int32][]map[string]interface{}
+	characterCollectionItems map[string][]int32
+}
+
+func NewMockDatabase() *MockDatabase {
+	return &MockDatabase{
+		collections: make(map[int32]map[string]interface{}),
+		collectionItems: make(map[int32][]map[string]interface{}),
+		collectionRewards: make(map[int32][]map[string]interface{}),
+		characterCollections: make(map[int32][]map[string]interface{}),
+		characterCollectionItems: make(map[string][]int32),
+	}
+}
+
+// Mock database methods (simplified implementations for testing)
+func (db *MockDatabase) Query(query string, args ...interface{}) (interface{}, error) {
+	// This is a simplified mock - in real tests you'd want more sophisticated query parsing
+	return nil, nil
+}
+
+func (db *MockDatabase) QueryRow(query string, args ...interface{}) interface{} {
+	return nil
+}
+
+func (db *MockDatabase) Exec(query string, args ...interface{}) (interface{}, error) {
+	return nil, nil
+}
+
+func setupTestData(db *MockDatabase) {
+	// Collection 1: Shiny Objects
+	db.collections[1] = map[string]interface{}{
+		"id": 1,
+		"collection_name": "Shiny Objects",
+		"collection_category": "Common Collections",
+		"level": 10,
+	}
+	db.collectionItems[1] = []map[string]interface{}{
+		{"item_id": 1001, "item_index": 0, "found": 0},
+		{"item_id": 1002, "item_index": 1, "found": 0},
+		{"item_id": 1003, "item_index": 2, "found": 0},
+	}
+	db.collectionRewards[1] = []map[string]interface{}{
+		{"reward_type": "coin", "reward_value": "1000", "reward_quantity": 1},
+		{"reward_type": "xp", "reward_value": "500", "reward_quantity": 1},
+	}
+
+	// Collection 2: Rare Gems
+	db.collections[2] = map[string]interface{}{
+		"id": 2,
+		"collection_name": "Rare Gems",
+		"collection_category": "Valuable Collections",
+		"level": 25,
+	}
+	db.collectionItems[2] = []map[string]interface{}{
+		{"item_id": 2001, "item_index": 0, "found": 0},
+		{"item_id": 2002, "item_index": 1, "found": 0},
+	}
+	db.collectionRewards[2] = []map[string]interface{}{
+		{"reward_type": "item", "reward_value": "5001", "reward_quantity": 1},
+	}
+}
+
+func createTestManager() *Manager {
+	db := NewMockDatabase()
+	setupTestData(db)
+	
+	// For now, we'll create collections manually since we can't easily mock the database queries
+	manager := NewManager(nil) // Pass nil for now since we're testing without real DB
+	
+	// Manually create test collections
+	collection1 := NewWithData(1, "Shiny Objects", "Common Collections", 10, nil)
+	collection1.CollectionItems = []CollectionItem{
+		{ItemID: 1001, Index: 0, Found: 0},
+		{ItemID: 1002, Index: 1, Found: 0},
+		{ItemID: 1003, Index: 2, Found: 0},
+	}
+	collection1.RewardCoin = 1000
+	collection1.RewardXP = 500
+	
+	collection2 := NewWithData(2, "Rare Gems", "Valuable Collections", 25, nil)
+	collection2.CollectionItems = []CollectionItem{
+		{ItemID: 2001, Index: 0, Found: 0},
+		{ItemID: 2002, Index: 1, Found: 0},
+	}
+	collection2.RewardItems = []CollectionRewardItem{
+		{ItemID: 5001, Quantity: 1},
+	}
+	
+	manager.AddCollection(collection1)
+	manager.AddCollection(collection2)
+	
+	return manager
+}
+
+// Test Manager Creation and Initialization
+func TestManagerCreation(t *testing.T) {
+	manager := NewManager(nil)
+	if manager == nil {
+		t.Fatal("NewManager returned nil")
+	}
+
+	if manager.masterList == nil {
+		t.Error("Manager master list is nil")
+	}
+
+	if manager.Size() != 0 {
+		t.Errorf("Expected empty manager, got size %d", manager.Size())
+	}
+
+	// Check default configuration
+	config := manager.GetConfiguration()
+	if config["enable_caching"].(bool) != true {
+		t.Error("Expected caching to be enabled by default")
+	}
+	if config["enable_statistics"].(bool) != true {
+		t.Error("Expected statistics to be enabled by default")  
+	}
+	if config["enable_validation"].(bool) != true {
+		t.Error("Expected validation to be enabled by default")
+	}
+}
+
+// Test Collection Operations
+func TestCollectionOperations(t *testing.T) {
+	manager := createTestManager()
+
+	// Test Size
+	if manager.Size() != 2 {
+		t.Errorf("Expected 2 collections, got %d", manager.Size())
+	}
+
+	// Test GetCollection
+	collection := manager.GetCollection(1)
+	if collection == nil {
+		t.Fatal("Failed to retrieve collection 1")
+	}
+	if collection.GetName() != "Shiny Objects" {
+		t.Errorf("Expected 'Shiny Objects', got '%s'", collection.GetName())
+	}
+	if collection.GetLevel() != 10 {
+		t.Errorf("Expected level 10, got %d", collection.GetLevel())
+	}
+
+	// Test non-existent collection
+	nonExistent := manager.GetCollection(999)
+	if nonExistent != nil {
+		t.Error("Expected nil for non-existent collection")
+	}
+
+	// Test NeedsItem
+	if !manager.NeedsItem(1001) {
+		t.Error("Expected collection to need item 1001")
+	}
+	if manager.NeedsItem(9999) {
+		t.Error("Expected no collection to need item 9999")
+	}
+}
+
+// Test Collection Management
+func TestCollectionManagement(t *testing.T) {
+	manager := createTestManager()
+
+	// Test adding new collection
+	newCollection := NewWithData(3, "Test Collection", "Test Category", 1, nil)
+	newCollection.CollectionItems = []CollectionItem{
+		{ItemID: 3001, Index: 0, Found: 0},
+	}
+
+	if !manager.AddCollection(newCollection) {
+		t.Error("Failed to add new collection")
+	}
+
+	if manager.Size() != 3 {
+		t.Errorf("Expected 3 collections after adding, got %d", manager.Size())
+	}
+
+	// Test adding duplicate collection
+	duplicate := NewWithData(3, "Duplicate", "Test", 1, nil)
+	if manager.AddCollection(duplicate) {
+		t.Error("Should not be able to add collection with duplicate ID")
+	}
+
+	// Test UpdateCollection
+	newCollection.Name = "Updated Test Collection"
+	err := manager.UpdateCollection(newCollection)
+	if err != nil {
+		t.Errorf("Failed to update collection: %v", err)
+	}
+
+	updated := manager.GetCollection(3)
+	if updated.GetName() != "Updated Test Collection" {
+		t.Error("Collection name was not updated")
+	}
+}
+
+// Test Item Needs Detection
+func TestItemNeeds(t *testing.T) {
+	manager := createTestManager()
+
+	testCases := []struct {
+		itemID   int32
+		expected bool
+		name     string
+	}{
+		{1001, true, "Collection 1 item 1"},
+		{1002, true, "Collection 1 item 2"}, 
+		{1003, true, "Collection 1 item 3"},
+		{2001, true, "Collection 2 item 1"},
+		{2002, true, "Collection 2 item 2"},
+		{9999, false, "Non-existent item"},
+		{0, false, "Invalid item ID"},
+	}
+
+	for _, tc := range testCases {
+		result := manager.NeedsItem(tc.itemID)
+		if result != tc.expected {
+			t.Errorf("%s: expected %v, got %v for item %d", tc.name, tc.expected, result, tc.itemID)
+		}
+	}
+}
+
+// Test Master List Operations
+func TestMasterListOperations(t *testing.T) {
+	manager := createTestManager()
+	masterList := manager.GetMasterList()
+
+	if masterList == nil {
+		t.Fatal("Master list is nil")
+	}
+
+	// Test getting all collections
+	allCollections := masterList.GetAllCollections()
+	if len(allCollections) != 2 {
+		t.Errorf("Expected 2 collections in master list, got %d", len(allCollections))
+	}
+
+	// Test collections by category
+	commonCollections := masterList.FindCollectionsByCategory("Common Collections")
+	if len(commonCollections) != 1 {
+		t.Errorf("Expected 1 common collection, got %d", len(commonCollections))
+	}
+
+	// Test collections by level
+	level10Collections := masterList.GetCollectionsByExactLevel(10)
+	if len(level10Collections) != 1 {
+		t.Errorf("Expected 1 level 10 collection, got %d", len(level10Collections))
+	}
+
+	levelRangeCollections := masterList.FindCollectionsByLevel(10, 30)
+	if len(levelRangeCollections) != 2 {
+		t.Errorf("Expected 2 collections in level range 10-30, got %d", len(levelRangeCollections))
+	}
+
+	// Test collections needing specific item
+	collectionsNeedingItem := masterList.GetCollectionsNeedingItem(1001)
+	if len(collectionsNeedingItem) != 1 {
+		t.Errorf("Expected 1 collection needing item 1001, got %d", len(collectionsNeedingItem))
+	}
+}
+
+// Test Player Collection Management  
+func TestPlayerCollectionManagement(t *testing.T) {
+	manager := createTestManager()
+
+	// Create player list
+	playerList := manager.CreatePlayerList(12345)
+	if playerList == nil {
+		t.Fatal("Failed to create player list")
+	}
+
+	if playerList.CharacterID != 12345 {
+		t.Errorf("Expected character ID 12345, got %d", playerList.CharacterID)
+	}
+
+	if playerList.Size() != 0 {
+		t.Errorf("Expected empty player list, got size %d", playerList.Size())
+	}
+
+	// Add collection to player
+	collection := manager.GetCollection(1).Clone()
+	if !playerList.AddCollection(collection) {
+		t.Error("Failed to add collection to player")
+	}
+
+	if playerList.Size() != 1 {
+		t.Errorf("Expected 1 collection in player list, got %d", playerList.Size())
+	}
+
+	// Test player collection operations
+	if playerList.HasCollectionsToHandIn() {
+		t.Error("Player should not have collections ready to turn in initially")
+	}
+
+	// Mark all items as found
+	for i := range collection.CollectionItems {
+		collection.CollectionItems[i].Found = 1
+	}
+
+	if !playerList.HasCollectionsToHandIn() {
+		t.Error("Player should have collections ready to turn in after finding all items")
+	}
+
+	readyCollections := playerList.GetCollectionsToHandIn()
+	if len(readyCollections) != 1 {
+		t.Errorf("Expected 1 collection ready to turn in, got %d", len(readyCollections))
+	}
+}
+
+// Test Packet Building (these would normally require packet definitions)
+func TestPacketBuilding(t *testing.T) {
+	manager := createTestManager()
+
+	// Since we don't have actual packet definitions loaded for testing,
+	// these tests will expect errors, but we can test the code paths
+	
+	characterID := int32(12345)
+	clientVersion := uint32(57048)
+
+	// Test collection update packet - should fail without packet definitions
+	playerCollections := make(map[int32]*Collection)
+	playerCollections[1] = manager.GetCollection(1)
+	
+	_, err := manager.SendCollectionUpdate(characterID, clientVersion, playerCollections)
+	if err == nil {
+		t.Error("Expected error for missing packet definition, but got none")
+	}
+
+	// Test collection filter packet - should fail without packet definitions
+	collectionsForItem := []*Collection{manager.GetCollection(1)}
+	_, err = manager.SendCollectionFilter(characterID, clientVersion, 1001, collectionsForItem)
+	if err == nil {
+		t.Error("Expected error for missing packet definition, but got none")
+	}
+
+	// Test collection item packet - should fail without packet definitions
+	_, err = manager.SendCollectionItem(characterID, clientVersion, 1, 1001)
+	if err == nil {
+		t.Error("Expected error for missing packet definition, but got none")
+	}
+}
+
+// Test Statistics and Monitoring
+func TestStatisticsAndMonitoring(t *testing.T) {
+	manager := createTestManager()
+
+	// Perform some operations to generate statistics
+	_ = manager.GetCollection(1)  // Cache hit
+	_ = manager.GetCollection(999) // Cache miss
+	manager.NeedsItem(1001)
+	manager.NeedsItem(9999)
+
+	stats := manager.GetStatistics()
+	if stats == nil {
+		t.Fatal("Statistics returned nil")
+	}
+
+	// Check some expected statistics exist
+	if _, exists := stats["collections_loaded"]; !exists {
+		t.Error("Expected collections_loaded statistic")
+	}
+	if _, exists := stats["cache_hits"]; !exists {
+		t.Error("Expected cache_hits statistic")
+	}
+	if _, exists := stats["cache_misses"]; !exists {
+		t.Error("Expected cache_misses statistic")
+	}
+	if _, exists := stats["item_checks"]; !exists {
+		t.Error("Expected item_checks statistic")
+	}
+
+	// Test reset statistics
+	manager.ResetStatistics()
+	newStats := manager.GetStatistics()
+	if newStats["cache_hits"].(int64) != 0 {
+		t.Error("Expected cache hits to be reset to 0")
+	}
+}
+
+// Test Configuration Management
+func TestConfigurationManagement(t *testing.T) {
+	manager := createTestManager()
+
+	// Test default configuration
+	config := manager.GetConfiguration()
+	if !config["enable_caching"].(bool) {
+		t.Error("Expected caching to be enabled by default")
+	}
+
+	// Test configuration update
+	newConfig := map[string]interface{}{
+		"enable_caching":    false,
+		"enable_validation": false,
+		"max_players_to_track": 500,
+	}
+	manager.SetConfiguration(newConfig)
+
+	updatedConfig := manager.GetConfiguration()
+	if updatedConfig["enable_caching"].(bool) {
+		t.Error("Expected caching to be disabled after update")
+	}
+	if updatedConfig["enable_validation"].(bool) {
+		t.Error("Expected validation to be disabled after update")
+	}
+	if updatedConfig["max_players_to_track"].(int) != 500 {
+		t.Error("Expected max_players_to_track to be updated to 500")
+	}
+}
+
+// Test Validation and Health Checks
+func TestValidationAndHealthChecks(t *testing.T) {
+	manager := createTestManager()
+
+	// Test health check - manager with nil db is not healthy by design
+	if manager.IsHealthy() {
+		t.Error("Manager with nil database should not be healthy")
+	}
+
+	// Test validation
+	issues := manager.ValidateCollections()
+	if len(issues) > 0 {
+		t.Errorf("Expected no validation issues, got %d: %v", len(issues), issues)
+	}
+
+	// Add invalid collection to test validation
+	invalidCollection := NewWithData(-1, "", "", -1, nil)
+	manager.AddCollection(invalidCollection)
+
+	validationIssues := manager.ValidateCollections()
+	if len(validationIssues) == 0 {
+		t.Error("Expected validation issues for invalid collection")
+	}
+}
+
+// Test Thread Safety (basic test)
+func TestThreadSafety(t *testing.T) {
+	manager := createTestManager()
+
+	// Perform concurrent operations
+	done := make(chan bool, 10)
+	
+	// Start multiple goroutines performing different operations
+	for i := 0; i < 10; i++ {
+		go func(id int) {
+			defer func() { done <- true }()
+			
+			// Mix of read and write operations
+			manager.GetCollection(1)
+			manager.NeedsItem(1001)
+			manager.GetStatistics()
+			
+			// Try to add a collection (most will fail due to duplicate IDs, which is expected)
+			newCollection := NewWithData(int32(1000+id), "Test", "Category", 1, nil)
+			manager.AddCollection(newCollection)
+		}(i)
+	}
+
+	// Wait for all goroutines to complete
+	for i := 0; i < 10; i++ {
+		<-done
+	}
+
+	// Manager should still be functional
+	if manager.Size() < 2 { // At least original 2 collections
+		t.Error("Manager appears corrupted after concurrent access")
+	}
+}
+
+// Test Collection Item Management
+func TestCollectionItemManagement(t *testing.T) {
+	manager := createTestManager()
+	collection := manager.GetCollection(1)
+
+	if collection == nil {
+		t.Fatal("Failed to get test collection")
+	}
+
+	// Test getting collection item by ID
+	item := collection.GetCollectionItemByItemID(1001)
+	if item == nil {
+		t.Fatal("Failed to get collection item 1001")
+	}
+	if item.ItemID != 1001 {
+		t.Errorf("Expected item ID 1001, got %d", item.ItemID)
+	}
+
+	// Test needs item
+	if !collection.NeedsItem(1001) {
+		t.Error("Collection should need item 1001")
+	}
+
+	// Test marking item found
+	if !collection.MarkItemFound(1001) {
+		t.Error("Failed to mark item 1001 as found")
+	}
+
+	// Test item is no longer needed
+	if collection.NeedsItem(1001) {
+		t.Error("Collection should not need item 1001 after marking as found")
+	}
+
+	// Test collection is not ready to turn in (still missing items)
+	if collection.GetIsReadyToTurnIn() {
+		t.Error("Collection should not be ready to turn in with only 1 of 3 items found")
+	}
+
+	// Mark all items as found
+	collection.MarkItemFound(1002)
+	collection.MarkItemFound(1003)
+
+	// Test collection is now ready to turn in
+	if !collection.GetIsReadyToTurnIn() {
+		t.Error("Collection should be ready to turn in after finding all items")
+	}
+
+	// Test progress calculation
+	progress := collection.GetProgress()
+	if progress != 100.0 {
+		t.Errorf("Expected 100%% progress, got %.2f%%", progress)
+	}
+}
+
+// Test Collection Cloning
+func TestCollectionCloning(t *testing.T) {
+	manager := createTestManager()
+	original := manager.GetCollection(1)
+
+	if original == nil {
+		t.Fatal("Failed to get original collection")
+	}
+
+	// Clone the collection
+	clone := original.Clone()
+	if clone == nil {
+		t.Fatal("Failed to clone collection")
+	}
+
+	// Verify clone has same data
+	if clone.GetID() != original.GetID() {
+		t.Error("Clone has different ID")
+	}
+	if clone.GetName() != original.GetName() {
+		t.Error("Clone has different name")
+	}
+	if clone.GetCategory() != original.GetCategory() {
+		t.Error("Clone has different category")
+	}
+	if clone.GetLevel() != original.GetLevel() {
+		t.Error("Clone has different level")
+	}
+
+	// Verify clone is independent (modify clone)
+	clone.Name = "Modified Clone"
+	if original.GetName() == "Modified Clone" {
+		t.Error("Modifying clone affected original")
+	}
+
+	// Verify collection items were cloned
+	if len(clone.CollectionItems) != len(original.CollectionItems) {
+		t.Error("Clone has different number of collection items")
+	}
+
+	// Modify clone items and verify original is unaffected
+	if len(clone.CollectionItems) > 0 {
+		clone.CollectionItems[0].Found = 1
+		if original.CollectionItems[0].Found == 1 {
+			t.Error("Modifying clone item affected original")
+		}
+	}
+}
+
+// Test Edge Cases and Error Conditions
+func TestEdgeCases(t *testing.T) {
+	manager := createTestManager()
+
+	// Test nil operations
+	if manager.AddCollection(nil) {
+		t.Error("Should not be able to add nil collection")
+	}
+
+	err := manager.UpdateCollection(nil)
+	if err == nil {
+		t.Error("Should return error for nil collection update")
+	}
+
+	// Test invalid collection data
+	invalidCollection := NewWithData(0, "", "", -5, nil)
+	err = manager.UpdateCollection(invalidCollection)
+	if err == nil {
+		t.Error("Should return error for invalid collection data")
+	}
+
+	// Test empty manager operations
+	emptyManager := NewManager(nil)
+	if emptyManager.Size() != 0 {
+		t.Error("Empty manager should have size 0")
+	}
+	if emptyManager.NeedsItem(1001) {
+		t.Error("Empty manager should not need any items")
+	}
+
+	// Test clear collections
+	manager.ClearCollections()
+	if manager.Size() != 0 {
+		t.Error("Manager should be empty after clearing collections")
+	}
+
+	// Test operations on cleared manager
+	if manager.GetCollection(1) != nil {
+		t.Error("Should return nil for collection after clearing")
+	}
+	if manager.NeedsItem(1001) {
+		t.Error("Cleared manager should not need any items")
+	}
+}
+
+// Benchmark tests
+func BenchmarkGetCollection(b *testing.B) {
+	manager := createTestManager()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		manager.GetCollection(1)
+	}
+}
+
+func BenchmarkNeedsItem(b *testing.B) {
+	manager := createTestManager()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		manager.NeedsItem(1001)
+	}
+}
+
+func BenchmarkGetStatistics(b *testing.B) {
+	manager := createTestManager()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		manager.GetStatistics()
+	}
+}

internal/packets/opcodes.go

@@ -123,6 +123,11 @@ const (
 	OP_ChatFiltersMsg
 	OP_EqChatChannelUpdateCmd
 
+	// Collection system opcodes
+	OP_EqCollectionUpdateCmd
+	OP_EqCollectionFilterCmd
+	OP_EqCollectionItemCmd
+
 	// Add more opcodes as needed...
 	_maxInternalOpcode // Sentinel value
 )
@@ -207,6 +212,11 @@ var OpcodeNames = map[InternalOpcode]string{
 	OP_ChatSendIgnoresMsg:             "OP_ChatSendIgnoresMsg",
 	OP_ChatFiltersMsg:                 "OP_ChatFiltersMsg",
 	OP_EqChatChannelUpdateCmd:         "OP_EqChatChannelUpdateCmd",
+
+	// Collection system opcodes
+	OP_EqCollectionUpdateCmd: "OP_EqCollectionUpdateCmd",
+	OP_EqCollectionFilterCmd: "OP_EqCollectionFilterCmd",
+	OP_EqCollectionItemCmd:   "OP_EqCollectionItemCmd",
 }
 
 // OpcodeManager handles the mapping between client-specific opcodes and internal opcodes