Cleaned up mudlog initialization checks and added command cleanup

- Removed redundant mudlog.IsInitialized() checks throughout codebase
- Added unregisterCommands() methods to combat modules for proper
cleanup
- Added RemoveUserCommand/RemoveMobCommand methods to plugin API
- Added architectural documentation for combat registry initialization
- Fixed BaseTimer initialization to create stopChan in Start()
- Documented combat config integration with main config system

Author: MorquinDevlar

internal/combat/common_timer.go

@@ -23,7 +23,7 @@ func NewBaseTimer(name string, updateFunc func()) *BaseTimer {
 	return &BaseTimer{
 		name:       name,
 		updateFunc: updateFunc,
-		stopChan:   make(chan bool),
+		// stopChan created in Start() as buffered channel
 	}
 }
 
@@ -45,9 +45,7 @@ func (bt *BaseTimer) Start() error {
 	bt.wg.Add(1)
 	go bt.runUpdateLoop()
 
-	if mudlog.IsInitialized() {
-		mudlog.Info("BaseTimer started", "name", bt.name)
-	}
+	mudlog.Info("BaseTimer started", "name", bt.name)
 	return nil
 }
 
@@ -85,15 +83,11 @@ func (bt *BaseTimer) Stop() error {
 	select {
 	case <-done:
 		// Timer stopped cleanly
-		if mudlog.IsInitialized() {
-			mudlog.Info("BaseTimer stopped", "name", bt.name)
-		}
+		mudlog.Info("BaseTimer stopped", "name", bt.name)
 	case <-time.After(2 * time.Second):
 		// Timeout - log warning and continue
-		if mudlog.IsInitialized() {
-			mudlog.Error("BaseTimer stop timeout", "name", bt.name,
-				"message", "Timer goroutine did not stop within 2 seconds")
-		}
+		mudlog.Error("BaseTimer stop timeout", "name", bt.name,
+			"message", "Timer goroutine did not stop within 2 seconds")
 	}
 
 	return nil

internal/combat/delegator.go

@@ -20,9 +20,7 @@ import (
 func AttackPlayerVsMob(user *users.UserRecord, mob *mobs.Mob) AttackResult {
 	// Validate inputs
 	if user == nil || mob == nil {
-		if mudlog.IsInitialized() {
-			mudlog.Error("AttackPlayerVsMob", "error", "nil combatant", "user", user != nil, "mob", mob != nil)
-		}
+		mudlog.Error("AttackPlayerVsMob", "error", "nil combatant", "user", user != nil, "mob", mob != nil)
 		return AttackResult{
 			Hit:              false,
 			MessagesToSource: []string{"Combat error: invalid target"},

internal/combat/registry.go

@@ -28,10 +28,8 @@ func RegisterCombatSystem(name string, system ICombatSystem) error {
 	}
 
 	combatRegistry[name] = system
-	// Only log if mudlog is initialized (it might not be during tests)
-	if mudlog.IsInitialized() {
-		mudlog.Info("Combat Registry", "action", "registered", "system", name)
-	}
+	// NOTE: Cannot use mudlog here as this may be called during init()
+	// before mudlog is initialized. Logging happens later when systems are activated.
 	return nil
 }
 
@@ -79,10 +77,7 @@ func SetActiveCombatSystem(name string) error {
 	// Shutdown current system if active (outside of lock)
 	if oldSystem != nil {
 		if err := oldSystem.Shutdown(); err != nil {
-			// Only log if mudlog is initialized (it might not be during tests)
-			if mudlog.IsInitialized() {
-				mudlog.Error("Combat Registry", "action", "shutdown failed", "system", oldName, "error", err)
-			}
+			mudlog.Error("Combat Registry", "action", "shutdown failed", "system", oldName, "error", err)
 			// Continue anyway - we don't want to block switching due to shutdown errors
 		}
 	}
@@ -189,11 +184,13 @@ func ClearRegistry() {
 }
 
 // InitializeRegistry sets up the combat registry event handlers
+// ARCHITECTURAL NOTE: This must be called explicitly from main() rather than
+// using init() to avoid event handler registration during init phase, which
+// could cause initialization order issues. The combat system needs the event
+// system to be fully initialized before registering handlers.
 func InitializeRegistry() {
 	events.RegisterListener(SwitchCombatSystemEvent{}, handleCombatSystemSwitch)
-	if mudlog.IsInitialized() {
-		mudlog.Info("Combat Registry", "action", "event handler registered")
-	}
+	mudlog.Info("Combat Registry", "action", "event handler registered")
 }
 
 // handleCombatSystemSwitch processes combat system switch events
@@ -236,7 +233,11 @@ func handleCombatSystemSwitch(e events.Event) events.ListenerReturn {
 		return events.Continue
 	}
 
-	// Save the combat system to the main config
+	// ARCHITECTURAL NOTE: Combat configuration is integrated into the main config
+	// rather than using plugin-specific config. This is intentional as combat
+	// is a core system that needs to be initialized before plugins are loaded.
+	// The active combat system must be persisted globally to ensure correct
+	// initialization on server restart.
 	if err := configs.SetVal("GamePlay.Combat.Style", evt.NewSystem); err != nil {
 		user.SendText(fmt.Sprintf(`<ansi fg="red">Failed to save combat system setting: %s</ansi>`, err.Error()))
 	}

internal/mapper/mapper.go

@@ -998,15 +998,15 @@ func PreCacheMaps() {
 func validateRoomBiomes() {
 	missingBiomeCount := 0
 	invalidBiomeCount := 0
-	
+
 	for _, roomId := range rooms.GetAllRoomIds() {
 		room := rooms.LoadRoom(roomId)
 		if room == nil {
 			continue
 		}
-		
+
 		originalBiome := room.Biome
-		
+
 		// Check if room has no biome
 		if originalBiome == "" {
 			zoneBiome := rooms.GetZoneBiome(room.Zone)
@@ -1022,7 +1022,7 @@ func validateRoomBiomes() {
 			}
 		}
 	}
-	
+
 	if missingBiomeCount > 0 || invalidBiomeCount > 0 {
 		mudlog.Info("Biome validation complete", "missing", missingBiomeCount, "invalid", invalidBiomeCount)
 	}

internal/plugins/plugins.go

@@ -283,6 +283,20 @@ func (p *Plugin) AddMobCommand(command string, handlerFunc mobcommands.MobComman
 
 }
 
+// RemoveUserCommand removes a previously registered user command
+func (p *Plugin) RemoveUserCommand(command string) {
+	if p.Callbacks.userCommands != nil {
+		delete(p.Callbacks.userCommands, command)
+	}
+}
+
+// RemoveMobCommand removes a previously registered mob command
+func (p *Plugin) RemoveMobCommand(command string) {
+	if p.Callbacks.mobCommands != nil {
+		delete(p.Callbacks.mobCommands, command)
+	}
+}
+
 // Adds an embedded file system to the plugin
 func (p *Plugin) AttachFileSystem(f embed.FS) error {
 

internal/rooms/rooms.go

@@ -2198,7 +2198,6 @@ func (r *Room) Validate() error {
 		}
 	}
 
-
 	// Make sure all items are validated (and have uids)
 	for i := range r.Items {
 		r.Items[i].Validate()

modules/combat-rounds/combat.go

@@ -28,9 +28,7 @@ func NewRoundBasedCombat(plug *plugins.Plugin) *RoundBasedCombat {
 
 // Initialize sets up the combat system
 func (rbc *RoundBasedCombat) Initialize() error {
-	if mudlog.IsInitialized() {
-		mudlog.Info("Combat System", "module", "combat-rounds", "action", "initializing commands")
-	}
+	mudlog.Info("Combat System", "module", "combat-rounds", "action", "initializing commands")
 
 	// Register combat commands
 	rbc.registerCommands()
@@ -62,6 +60,9 @@ func (rbc *RoundBasedCombat) Shutdown() error {
 	}
 	mudlog.Info("Combat System", "module", "combat-rounds", "action", "timer stopped")
 
+	// Unregister commands
+	rbc.unregisterCommands()
+
 	rbc.active = false
 	if mudlog.IsInitialized() {
 		mudlog.Info("Combat System", "module", "combat-rounds", "status", "shutdown")

modules/combat-rounds/commands.go

@@ -32,9 +32,24 @@ func (rbc *RoundBasedCombat) registerCommands() {
 	rbc.plug.AddMobCommand("attack", rbc.mobAttackCommand, false)
 	rbc.plug.AddMobCommand("flee", rbc.mobFleeCommand, false)
 
-	if mudlog.IsInitialized() {
-		mudlog.Info("Combat Commands", "action", "registered", "module", "combat-rounds")
-	}
+	mudlog.Info("Combat Commands", "action", "registered", "module", "combat-rounds")
+}
+
+// unregisterCommands removes all combat-related commands
+func (rbc *RoundBasedCombat) unregisterCommands() {
+	// Remove user commands
+	rbc.plug.RemoveUserCommand("attack")
+	rbc.plug.RemoveUserCommand("kill")
+	rbc.plug.RemoveUserCommand("flee")
+	rbc.plug.RemoveUserCommand("consider")
+	rbc.plug.RemoveUserCommand("combatinfo")
+	rbc.plug.RemoveUserCommand("config")
+
+	// Remove mob commands
+	rbc.plug.RemoveMobCommand("attack")
+	rbc.plug.RemoveMobCommand("flee")
+
+	mudlog.Info("Combat Commands", "action", "unregistered", "module", "combat-rounds")
 }
 
 // fleeCommand handles player flee commands

modules/combat-rounds/timer.go

@@ -55,7 +55,11 @@ func NewRoundBasedTimer(c *RoundBasedCombat) *RoundBasedTimer {
 
 // Start implements ICombatTimer
 func (rt *RoundBasedTimer) Start() error {
-	// Defer registration to avoid deadlock when called from event handler
+	// ARCHITECTURAL NOTE: Event registration is deferred to prevent deadlocks
+	// The combat system switching uses events, and registering listeners during
+	// event processing (when combat system initializes) would cause a deadlock
+	// in the event system. This pattern is unique to combat modules due to their
+	// dynamic initialization/shutdown during runtime.
 	go func() {
 		// Small delay to ensure we're out of the event handler
 		time.Sleep(1 * time.Millisecond)

modules/combat-twitch/combat.go

@@ -71,6 +71,9 @@ func (tc *TwitchCombat) Shutdown() error {
 		}
 	}
 
+	// Unregister commands
+	tc.unregisterCommands()
+
 	mudlog.Info("Combat System", "module", "combat-twitch", "status", "shutdown")
 	return nil
 }