-
-
Notifications
You must be signed in to change notification settings - Fork 71
feat: Add maximum player limit feature for lite #515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
ours1505
commented
Apr 19, 2025
- Added maxPlayers and maxPlayersMessage options in the configuration file to allow setting a maximum number of connected players and customizing the full message.
- Updated documentation to explain how to configure the maximum player limit.
- Implemented maximum player count check in connection handling logic, rejecting connection requests that exceed the limit and returning a custom message.
- Added maxPlayers and maxPlayersMessage options in the configuration file to allow setting a maximum number of connected players and customizing the full message. - Updated documentation to explain how to configure the maximum player limit. - Implemented maximum player count check in connection handling logic, rejecting connection requests that exceed the limit and returning a custom message.
maxPlayers: 0 | ||
# Custom message to show when the maximum player limit is reached. | ||
# Default: §cThis Node is full.Please try another node. | ||
maxPlayersMessage: "§cThis Node is full.Please try another node." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use our text config component like other settings do to support the different legacy and modern text types.
Enabled: false, | ||
Routes: []Route{}, | ||
MaxPlayers: 0, // 0 means unlimited | ||
MaxPlayersMessage: "§cThis Node is full.Please try another node.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use text component directly. also don't call it Node, it's a proxy
MaxPlayers int `yaml:"maxPlayers,omitempty" json:"maxPlayers,omitempty"` // Maximum number of players to accept, 0 means unlimited | ||
MaxPlayersMessage string `yaml:"maxPlayersMessage,omitempty" json:"maxPlayersMessage,omitempty"` // Message to show when reaching max players limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can put the player specific settings into a sub-struct Players struct { MaxPlayers, MaxMessage }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this file. Just use atomic.Int64
.
) | ||
|
||
// ConnectionCounter tracks the number of active connections in LiteMode | ||
var ConnectionCounter = &counter{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not put global state like this. A process could run many Gate instances at once, they must have their own counter
// Use the configured message | ||
message := cfg.MaxPlayersMessage | ||
if message == "" { | ||
message = "§cThis Node is full.Please try another node." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stay DRY
cfg := getConfig() | ||
if cfg != nil && cfg.MaxPlayers > 0 { | ||
currentCount := ConnectionCounter.Count() | ||
if int(currentCount) >= cfg.MaxPlayers { | ||
log.Info("connection rejected due to maximum player limit", | ||
"current", currentCount, | ||
"max", cfg.MaxPlayers) | ||
|
||
// Use the configured message | ||
message := cfg.MaxPlayersMessage | ||
if message == "" { | ||
message = "§cThis Node is full.Please try another node." | ||
} | ||
|
||
// Disconnect and display custom message | ||
reason := &component.Text{ | ||
Content: message, | ||
S: component.Style{Color: color.Red}, | ||
} | ||
_ = netmc.CloseWith(client, packet.NewDisconnect(reason, client.Protocol(), client.State().State)) | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a lot of code, let's put it into a helper function
// reference to the current lite mode config | ||
var currentConfig *config.Config | ||
|
||
// SetConfig sets the current lite mode config | ||
func SetConfig(cfg *config.Config) { | ||
currentConfig = cfg | ||
} | ||
|
||
// getConfig returns the current lite mode config | ||
func getConfig() *config.Config { | ||
return currentConfig | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a cheeky hack to pass state. It should be handled differently.
I don't want this :)