feat: customizable lobby size for bot games (Fixes #180)#197
Conversation
ShrimpCryptid
left a comment
There was a problem hiding this comment.
Hi, thank you so much for the contribution! I left some initial thoughts and suggestions here, feel free to re-request me once you've had a chance to take a look. I generally like this PR and I'm very pleased that it's such a small change overall :)
I will warn that it might take a while before these changes are populated on production, because the development branch and the production branch are out of sync due to some bugs that got introduced in the Typescript refactor. I haven't had time to dedicate to chasing those down yet.
|
|
||
| static String DEFAULT_ICON = "p_default"; | ||
|
|
||
| private int targetLobbySize = SecretHitlerGame.MIN_PLAYERS; |
There was a problem hiding this comment.
Consider renaming to minLobbySize?
There was a problem hiding this comment.
Renamed targetLobbySize to minLobbySize across both the Java backend and the React frontend state.
| <div style={{ marginTop: "5px", display: "flex", alignItems: "center", color: "white" }}> | ||
| <label style={{ marginRight: "10px" }}>Target Game Size:</label> | ||
| <select | ||
| value={this.state.targetLobbySize} |
There was a problem hiding this comment.
We don't do this anywhere else on the app but I wonder if this value needs to be set optimistically in case of high latency. Not something that needs to be changed now!
There was a problem hiding this comment.
Left this as it is for now as suggested, but happy to revisit later if latency becomes an issue.
|
|
||
| {isVIP ? ( | ||
| <div style={{ marginTop: "5px", display: "flex", alignItems: "center", color: "white" }}> | ||
| <label style={{ marginRight: "10px" }}>Target Game Size:</label> |
There was a problem hiding this comment.
I'm trying to decide on the best language here, since I worry that "Target game size" isn't immediately obvious, and you can actually have more players than the target size. My next thought is "Minimum game size", but that might suggest to players that they have to hit a minimum # of players before they can start a game.
What do you think about changing this to "Game Size" and changing the options in the dropdown to {n}+ Players?
There was a problem hiding this comment.
"Game Size" sounds good.
Changed the label to "Game Size" with "{n}+ Players" options, and wrapped the <select> in the <label> for screen readers.
I also added a quick ternary check so it correctly says "10 Players" instead of "10+ Players" when maxed out, along with a small subtitle reminding players about the bots.
|
|
||
| {isVIP ? ( | ||
| <div style={{ marginTop: "5px", display: "flex", alignItems: "center", color: "white" }}> | ||
| <label style={{ marginRight: "10px" }}>Target Game Size:</label> |
There was a problem hiding this comment.
Could you please change the label element here to wrap the select element for accessibility? Or add an id to the select and a for field to the label. https://dequeuniversity.com/rules/axe/4.5/select-name
There was a problem hiding this comment.
Wrapped the <select> in the <label> for accessibility.
| <button | ||
| id={"lobby-change-icon-button"} | ||
| onClick={this.onClickChangeIcon} | ||
| > | ||
| CHANGE ICON | ||
| </button> |
There was a problem hiding this comment.
There was a problem hiding this comment.
Moved the new controls completely out of the top flex container, so the 'Change Icon' button is back to its normal size.
|
@ShrimpCryptid Thanks so much for the review! No worries at all about the deployment delay. I've pushed a new commit that addresses all your feedback. Let me know if everything looks good or if anything needs to be changed. |
ShrimpCryptid
left a comment
There was a problem hiding this comment.
Sorry it took me a bit to get back to this! I like the changes, had one more suggestion.
| usernames: message[PARAM_USERNAMES], | ||
| icons: message[PARAM_ICON], | ||
| page: PAGE.LOBBY, | ||
| minLobbySize: message["minLobbySize"] || 5, |
There was a problem hiding this comment.
Could you please make minLobbySize into a constant?
There was a problem hiding this comment.
I've extracted the string key into a PARAM_MIN_LOBBY_SIZE constant on both the constants/index.ts and the SecretHitlerServer.java.

Problem
Currently, the game only generates bots to fulfill the minimum required player count (5 players). Users cannot choose to play larger games (e.g., 7 or 10 players) filled primarily with bots. This PR addresses [Issue #180] to allow the lobby host to set a custom target game size that the server will auto-fill with bot players.
Solution
I introduced a
targetLobbySizevariable to the lobby state and updated the bot generation logic inLobby.javato fill the lobby up to this target size rather than the hardcodedMIN_PLAYERS. I also wired up a pre-existing but unusedCOMMAND_SET_LOBBY_SIZEWebSocket command in the backend and added a dropdown UI to the frontend for the lobby host to control this setting.Type of change
Change summary:
targetLobbySizestate tracking to the Java backend and modified bot generation math.set-lobby-sizeWebSocket command inSecretHitlerServer.javato update the lobby size securely.commands.ts).App.tsxfor the host (VIP) to adjust the target game size.PACKET_LOBBYpayload so non-hosts can view the updated settings.Steps to Verify:
$env:DEBUG_MODE="true"; ./gradlew run) and the React frontend.Target Game Size: 8and that it updates dynamically if the host changes it again.Screenshots:
(screenshots showing the new dropdown in the lobby UI and the generated bots in-game)
Dropdown in the lobby UI for Host:
Dropdown in the lobby UI for non-host player:
Generated bots in-game for Host:
Generated bots in-game for non-host:
Keyfiles:
backend/src/ main/java/server/SecretHitlerServer.javabackend/src/main/java/server/util/Lobby.javafrontend/src/App.tsxfrontend/src/types/commands.ts