Skip to content

Commit e79babd

Browse files
MorquinDevlarMorquinDevlar
committed
Improved GMCP module architecture and exit information
Added: - PlayerDespawn cleanup handlers to all combat modules - User validation with validateUserForGMCP helper function - ExitLockChanged event for exit state notifications - GMCP handler to send Room.Info.Exits updates on lock changes - Exit details map with type, state, name, hasKey, and hasPicked fields - Package documentation explaining design patterns for each module Fixed: - Memory leaks from uncleaned tracking maps - Function naming inconsistencies in Status and Events modules - Redundant "exits" wrapper in Room.Info.Exits output Changed: - Cooldown timer interval from 250ms to 200ms - Mutex usage to RLock for read-only operations - Exit state values to "locked" and "open" Removed: - Unused imports from gmcp.go and combat modules - Unused gmcp_batcher.go file - Rate limiting code from damage module
1 parent 25cc387 commit e79babd

12 files changed

+564
-463
lines changed

‎internal/events/eventtypes.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,3 +595,15 @@ type CombatantFled struct {
595595
}
596596

597597
func (c CombatantFled) Type() string { return `CombatantFled` }
598+
599+
// Room Events
600+
601+
// ExitLockChanged fires when an exit's lock state changes
602+
type ExitLockChanged struct {
603+
Event
604+
RoomId int
605+
ExitName string
606+
Locked bool // true if now locked, false if now unlocked
607+
}
608+
609+
func (e ExitLockChanged) Type() string { return `ExitLockChanged` }

‎internal/hooks/PlayerDespawn_HandleLeave.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/GoMudEngine/GoMud/internal/rooms"
1414
"github.com/GoMudEngine/GoMud/internal/templates"
1515
"github.com/GoMudEngine/GoMud/internal/users"
16+
"github.com/GoMudEngine/GoMud/modules/gmcp"
1617
)
1718

1819
//
@@ -65,6 +66,10 @@ func HandleLeave(e events.Event) events.ListenerReturn {
6566
if err := users.LogOutUserByConnectionId(connId); err != nil {
6667
mudlog.Error("Log Out Error", "connectionId", connId, "error", err)
6768
}
69+
70+
// Clean up all GMCP state for this user
71+
gmcp.CleanupUser(evt.UserId)
72+
6873
connections.Remove(connId)
6974

7075
specialRooms := configs.GetSpecialRoomsConfig()

‎internal/rooms/rooms.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,12 @@ func (r *Room) SetExitLock(exitName string, locked bool) {
861861
}
862862
}
863863

864+
// Fire event for exit lock state change
865+
events.AddToQueue(events.ExitLockChanged{
866+
RoomId: r.RoomId,
867+
ExitName: exitName,
868+
Locked: locked,
869+
})
864870
}
865871

866872
func (r *Room) GetExitInfo(exitName string) (exitInfo exit.RoomExit, ok bool) {

‎modules/gmcp/gmcp.Char.Combat.Cooldown.go‎

Lines changed: 73 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// Package gmcp handles Combat Cooldown timer updates for GMCP.
2+
//
3+
// Sends high-frequency timer updates (5Hz) during combat for smooth countdown animations.
4+
// Uses a dedicated timer that only runs when players are in combat to minimize CPU usage.
15
package gmcp
26

37
import (
@@ -49,12 +53,16 @@ func InitCombatCooldownTimer() {
4953

5054
// Register the GMCP event handler
5155
events.RegisterListener(GMCPCombatCooldownUpdate{}, handleCombatCooldownUpdate)
56+
57+
// Clean up when player disconnects
58+
events.RegisterListener(events.PlayerDespawn{}, handleCooldownPlayerDespawn)
5259
}
5360

5461
// handleNewRound updates round tracking
5562
func (ct *CombatCooldownTimer) handleNewRound(e events.Event) events.ListenerReturn {
5663
evt, ok := e.(events.NewRound)
5764
if !ok {
65+
mudlog.Error("GMCPCombatCooldown", "action", "handleNewRound", "error", "type assertion failed", "expectedType", "events.NewRound", "actualType", fmt.Sprintf("%T", e))
5866
return events.Continue
5967
}
6068

@@ -104,9 +112,8 @@ func (ct *CombatCooldownTimer) start() {
104112
}
105113

106114
ct.running = true
107-
// Reduced update frequency from 100ms to 250ms to reduce network traffic
108-
// This still provides smooth updates (4 per second) while reducing load
109-
ct.ticker = time.NewTicker(250 * time.Millisecond)
115+
// 200ms interval provides 5 updates per second for smooth countdown animation
116+
ct.ticker = time.NewTicker(200 * time.Millisecond)
110117

111118
go func() {
112119
for {
@@ -182,30 +189,43 @@ func (ct *CombatCooldownTimer) sendUpdates() {
182189

183190
// Send updates
184191
for _, userId := range playerIds {
185-
if user := users.GetByUserId(userId); user != nil {
186-
// Check if still in combat
187-
if user.Character.Aggro == nil {
188-
// Skip players no longer in combat
189-
// They will be removed by the CombatStatus module
190-
continue
191-
}
192+
user := users.GetByUserId(userId)
193+
if user == nil {
194+
// User no longer exists, clean up stale entry
195+
ct.playerMutex.Lock()
196+
delete(ct.players, userId)
197+
ct.playerMutex.Unlock()
198+
mudlog.Warn("CombatCooldownTimer", "action", "sendUpdates", "issue", "user not found, cleaning up stale entry", "userId", userId)
199+
continue
200+
}
192201

193-
// Queue cooldown update event
194-
events.AddToQueue(GMCPCombatCooldownUpdate{
195-
UserId: userId,
196-
CooldownSeconds: remainingSeconds,
197-
MaxSeconds: maxSeconds,
198-
NameActive: "Combat Round",
199-
NameIdle: "Ready",
200-
})
202+
// Check if still in combat
203+
if user.Character.Aggro == nil {
204+
// Skip players no longer in combat
205+
// They will be removed by the CombatStatus module
206+
continue
201207
}
202-
// Note: We don't remove players here to avoid deadlock
203-
// The CombatStatus module will handle removing players
208+
209+
// Queue cooldown update event
210+
events.AddToQueue(GMCPCombatCooldownUpdate{
211+
UserId: userId,
212+
CooldownSeconds: remainingSeconds,
213+
MaxSeconds: maxSeconds,
214+
NameActive: "Combat Round",
215+
NameIdle: "Ready",
216+
})
204217
}
205218
}
206219

207220
// TrackCombatPlayer starts tracking cooldown for a player entering combat
208221
func TrackCombatPlayer(userId int) {
222+
// Validate user exists before tracking
223+
user := users.GetByUserId(userId)
224+
if user == nil {
225+
mudlog.Warn("CombatCooldownTimer", "action", "TrackCombatPlayer", "issue", "attempted to track non-existent user", "userId", userId)
226+
return
227+
}
228+
209229
if cooldownTimer != nil {
210230
cooldownTimer.AddPlayer(userId)
211231
}
@@ -214,18 +234,22 @@ func TrackCombatPlayer(userId int) {
214234
// UntrackCombatPlayer stops tracking cooldown for a player leaving combat
215235
func UntrackCombatPlayer(userId int) {
216236
if cooldownTimer != nil {
217-
// Send final 0.0 update before removing
218-
timingConfig := configs.GetTimingConfig()
219-
maxSeconds := float64(timingConfig.RoundSeconds)
220-
221-
// Queue final update event
222-
events.AddToQueue(GMCPCombatCooldownUpdate{
223-
UserId: userId,
224-
CooldownSeconds: 0.0,
225-
MaxSeconds: maxSeconds,
226-
NameActive: "Combat Round",
227-
NameIdle: "Ready",
228-
})
237+
// Check if user still exists before sending final update
238+
user := users.GetByUserId(userId)
239+
if user != nil {
240+
// Send final 0.0 update before removing
241+
timingConfig := configs.GetTimingConfig()
242+
maxSeconds := float64(timingConfig.RoundSeconds)
243+
244+
// Queue final update event
245+
events.AddToQueue(GMCPCombatCooldownUpdate{
246+
UserId: userId,
247+
CooldownSeconds: 0.0,
248+
MaxSeconds: maxSeconds,
249+
NameActive: "Combat Round",
250+
NameIdle: "Ready",
251+
})
252+
}
229253

230254
cooldownTimer.RemovePlayer(userId)
231255
}
@@ -235,22 +259,14 @@ func UntrackCombatPlayer(userId int) {
235259
func handleCombatCooldownUpdate(e events.Event) events.ListenerReturn {
236260
evt, typeOk := e.(GMCPCombatCooldownUpdate)
237261
if !typeOk {
238-
return events.Continue
239-
}
240-
241-
if evt.UserId < 1 {
262+
mudlog.Error("GMCPCombatCooldown", "action", "handleCombatCooldownUpdate", "error", "type assertion failed", "expectedType", "GMCPCombatCooldownUpdate", "actualType", fmt.Sprintf("%T", e))
242263
return events.Continue
243264
}
244265

245266
mudlog.Debug("CombatCooldownTimer", "action", "handleCombatCooldownUpdate", "userId", evt.UserId, "cooldown", evt.CooldownSeconds)
246267

247-
// Make sure they have GMCP enabled
248-
user := users.GetByUserId(evt.UserId)
249-
if user == nil {
250-
return events.Continue
251-
}
252-
253-
if !isGMCPEnabled(user.ConnectionId()) {
268+
_, valid := validateUserForGMCP(evt.UserId, "GMCPCombatCooldown")
269+
if !valid {
254270
return events.Continue
255271
}
256272

@@ -262,7 +278,6 @@ func handleCombatCooldownUpdate(e events.Event) events.ListenerReturn {
262278
"name_idle": evt.NameIdle,
263279
}
264280

265-
// Send the GMCP update
266281
events.AddToQueue(GMCPOut{
267282
UserId: evt.UserId,
268283
Module: "Char.Combat.Cooldown",
@@ -271,3 +286,17 @@ func handleCombatCooldownUpdate(e events.Event) events.ListenerReturn {
271286

272287
return events.Continue
273288
}
289+
290+
// handleCooldownPlayerDespawn cleans up when player leaves
291+
func handleCooldownPlayerDespawn(e events.Event) events.ListenerReturn {
292+
evt, typeOk := e.(events.PlayerDespawn)
293+
if !typeOk {
294+
mudlog.Error("GMCPCombatCooldown", "action", "handleCooldownPlayerDespawn", "error", "type assertion failed", "expectedType", "events.PlayerDespawn", "actualType", fmt.Sprintf("%T", e))
295+
return events.Continue
296+
}
297+
298+
// Stop tracking cooldown for this player
299+
UntrackCombatPlayer(evt.UserId)
300+
301+
return events.Continue
302+
}

‎modules/gmcp/gmcp.Char.Combat.Damage.go‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1+
// Package gmcp handles Combat Damage notification updates for GMCP.
2+
//
3+
// Stateless module that immediately forwards damage/healing events to players.
4+
// No deduplication needed as each damage event is meaningful.
15
package gmcp
26

37
import (
8+
"fmt"
9+
410
"github.com/GoMudEngine/GoMud/internal/events"
5-
"github.com/GoMudEngine/GoMud/internal/users"
11+
"github.com/GoMudEngine/GoMud/internal/mudlog"
612
)
713

814
// GMCPCombatDamageUpdate is sent when damage or healing occurs
@@ -23,25 +29,18 @@ func init() {
2329

2430
// Keep the internal event for backward compatibility
2531
events.RegisterListener(GMCPCombatDamageUpdate{}, handleCombatDamageUpdate)
32+
2633
}
2734

2835
func handleCombatDamageUpdate(e events.Event) events.ListenerReturn {
2936
evt, typeOk := e.(GMCPCombatDamageUpdate)
3037
if !typeOk {
38+
mudlog.Error("GMCPCombatDamage", "action", "handleCombatDamageUpdate", "error", "type assertion failed", "expectedType", "GMCPCombatDamageUpdate", "actualType", fmt.Sprintf("%T", e))
3139
return events.Continue
3240
}
3341

34-
if evt.UserId < 1 {
35-
return events.Continue
36-
}
37-
38-
// Make sure they have GMCP enabled
39-
user := users.GetByUserId(evt.UserId)
40-
if user == nil {
41-
return events.Continue
42-
}
43-
44-
if !isGMCPEnabled(user.ConnectionId()) {
42+
_, valid := validateUserForGMCP(evt.UserId, "GMCPCombatDamage")
43+
if !valid {
4544
return events.Continue
4645
}
4746

@@ -53,7 +52,6 @@ func handleCombatDamageUpdate(e events.Event) events.ListenerReturn {
5352
"target": evt.Target,
5453
}
5554

56-
// Send the GMCP update immediately
5755
events.AddToQueue(GMCPOut{
5856
UserId: evt.UserId,
5957
Module: "Char.Combat.Damage",
@@ -67,6 +65,7 @@ func handleCombatDamageUpdate(e events.Event) events.ListenerReturn {
6765
func handleDamageDealtForGMCP(e events.Event) events.ListenerReturn {
6866
evt, typeOk := e.(events.DamageDealt)
6967
if !typeOk {
68+
mudlog.Error("GMCPCombatDamage", "action", "handleDamageDealtForGMCP", "error", "type assertion failed", "expectedType", "events.DamageDealt", "actualType", fmt.Sprintf("%T", e))
7069
return events.Continue
7170
}
7271

@@ -99,6 +98,7 @@ func handleDamageDealtForGMCP(e events.Event) events.ListenerReturn {
9998
func handleHealingReceivedForGMCP(e events.Event) events.ListenerReturn {
10099
evt, typeOk := e.(events.HealingReceived)
101100
if !typeOk {
101+
mudlog.Error("GMCPCombatDamage", "action", "handleHealingReceivedForGMCP", "error", "type assertion failed", "expectedType", "events.HealingReceived", "actualType", fmt.Sprintf("%T", e))
102102
return events.Continue
103103
}
104104

@@ -119,12 +119,19 @@ func handleHealingReceivedForGMCP(e events.Event) events.ListenerReturn {
119119
// SendCombatDamage sends a damage/healing update
120120
// This is exported so it can be called from combat code
121121
func SendCombatDamage(userId int, amount int, damageType string, source string, target string) {
122-
// Send the update directly for immediate processing
123-
handleCombatDamageUpdate(GMCPCombatDamageUpdate{
122+
// Validate user exists before sending damage update
123+
_, valid := validateUserForGMCP(userId, "GMCPCombatDamage")
124+
if !valid {
125+
return
126+
}
127+
128+
// Queue the update event
129+
events.AddToQueue(GMCPCombatDamageUpdate{
124130
UserId: userId,
125131
Amount: amount,
126132
DamageType: damageType,
127133
Source: source,
128134
Target: target,
129135
})
130136
}
137+

0 commit comments

Comments
 (0)