Conversation
WalkthroughThis change updates the logic for determining the maximum number of players in a game lobby. The method now accepts an optional parameter for the number of player teams, adjusts the player count to be divisible by the team count, and modifies how the cap is calculated based on the map and mode. Changes
Sequence Diagram(s)sequenceDiagram
participant MapPlaylist
participant Config
participant DefaultServerConfig
MapPlaylist->>Config: lobbyMaxPlayers(map, mode, numPlayerTeams?)
Config->>DefaultServerConfig: delegate to implementation
DefaultServerConfig->>DefaultServerConfig: Calculate base player count
DefaultServerConfig->>DefaultServerConfig: If Team mode, scale and adjust for team divisibility
DefaultServerConfig-->>Config: Return max player count
Config-->>MapPlaylist: Return max player count
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant MapPlaylist
participant Config
participant DefaultServerConfig
MapPlaylist->>Config: lobbyMaxPlayers(map, mode, numPlayerTeams)
Config->>DefaultServerConfig: lobbyMaxPlayers(map, mode, numPlayerTeams)
DefaultServerConfig-->>Config: returns adjusted max player count
Config-->>MapPlaylist: returns max player count
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/server/MapPlaylist.ts (1)
48-49: Consider team size constraints when generating team count.The random team generation (2-6 teams) works well, but you might want to consider the relationship between total expected players and team count to avoid very small teams.
For example, if the expected player count is low and you generate 6 teams, you could end up with teams of only 1-2 players each. Consider adding logic like:
const numPlayerTeams = - mode === GameMode.Team ? 2 + Math.floor(Math.random() * 5) : undefined; + mode === GameMode.Team + ? Math.min(6, Math.max(2, 2 + Math.floor(Math.random() * 5))) + : undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/configuration/Config.ts(1 hunks)src/core/configuration/DefaultConfig.ts(1 hunks)src/server/MapPlaylist.ts(1 hunks)
🔇 Additional comments (3)
src/server/MapPlaylist.ts (1)
54-54: LGTM! Clean parameter passing.The updated method call correctly passes the new
numPlayerTeamsparameter and maintains backward compatibility.src/core/configuration/Config.ts (1)
32-36: LGTM! Clean interface extension.The method signature is properly updated with the optional
numPlayerTeamsparameter. The typenumber | undefinedclearly expresses the intent and maintains backward compatibility.src/core/configuration/DefaultConfig.ts (1)
146-150: LGTM! Clean method signature implementation.The method signature correctly implements the interface with proper parameter typing and naming.
## Description: Ensure that team sizes are equal. Closes #841 ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced - [x] I understand that submitting code with bugs that could have been caught through manual testing blocks releases and new features for all contributors --------- Co-authored-by: Scott Anderson <662325+scottanderson@users.noreply.github.com>
Description:
Ensure that team sizes are equal.
Closes #841
Please complete the following: