Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions backend/src/main/java/server/SecretHitlerServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,14 @@ private static void onWebSocketMessage(WsMessageContext ctx) {
String iconId = message.getString(PARAM_ICON);
lobby.trySetUserIcon(iconId, ctx);
break;

case COMMAND_SET_LOBBY_SIZE:
// Ensure the game hasn't started yet
if (!lobby.isInGame()) {
int newSize = message.getInt("size");
lobby.setTargetLobbySize(newSize);
}
break;

default: // This is an invalid command.
throw new RuntimeException("unrecognized command " + message.get(PARAM_COMMAND));
Expand Down
14 changes: 11 additions & 3 deletions backend/src/main/java/server/util/Lobby.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ public class Lobby implements Serializable {

static String DEFAULT_ICON = "p_default";

private int targetLobbySize = SecretHitlerGame.MIN_PLAYERS;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming to minLobbySize?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed targetLobbySize to minLobbySize across both the Java backend and the React frontend state.

synchronized public void setTargetLobbySize(int size) {
if (size >= SecretHitlerGame.MIN_PLAYERS && size <= SecretHitlerGame.MAX_PLAYERS) {
this.targetLobbySize = size;
}
}

/**
* Constructs a new Lobby.
*/
Expand Down Expand Up @@ -392,6 +399,7 @@ synchronized public void updateUser(WsContext ctx, String userName) {
message = new JSONObject();
message.put(SecretHitlerServer.PARAM_PACKET_TYPE, SecretHitlerServer.PACKET_LOBBY);
message.put("usernames", activeUsernames.toArray());
message.put("targetLobbySize", targetLobbySize);
}
// Add user icons to the update message
JSONObject icons = new JSONObject(usernameToIcon);
Expand Down Expand Up @@ -494,11 +502,11 @@ synchronized public void startNewGame() {
usersInGame.clear();
usersInGame.addAll(userToUsername.values());

// Generate CpuPlayers if the lobby size has not been met
// Generate CpuPlayers if the target lobby size has not been met
List<String> cpuNames = new ArrayList<>();
cpuPlayers.clear();
if (usersInGame.size() < SecretHitlerGame.MIN_PLAYERS) {
int numCpuPlayersToGenerate = SecretHitlerGame.MIN_PLAYERS - usersInGame.size();
if (usersInGame.size() < targetLobbySize) {
int numCpuPlayersToGenerate = targetLobbySize - usersInGame.size();
int i = 1;
while (numCpuPlayersToGenerate > 0) {
String botName = "Bot " + i;
Expand Down
32 changes: 29 additions & 3 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ type AppState = {
lobbyFromURL: boolean;
usernames: string[];
icons: { [key: string]: string };
targetLobbySize: number;
gameState: GameState;
/* Stores the last gameState[PARAM_STATE] value to check for changes. */
lastState: any;
Expand Down Expand Up @@ -177,6 +178,7 @@ const defaultAppState: AppState = {
lobbyFromURL: false,
usernames: [],
icons: {},
targetLobbySize: 5,
gameState: DEFAULT_GAME_STATE,
lastState: {},
liberalPolicies: 0,
Expand Down Expand Up @@ -429,6 +431,7 @@ class App extends Component<{}, AppState> {
usernames: message[PARAM_USERNAMES],
icons: message[PARAM_ICON],
page: PAGE.LOBBY,
targetLobbySize: message["targetLobbySize"] || 5,
});
if (message[PARAM_ICON][this.state.name] === defaultPortrait) {
this.showChangeIconAlert();
Expand Down Expand Up @@ -917,9 +920,32 @@ class App extends Component<{}, AppState> {
<div id={"lobby-lower-container"}>
<div id={"lobby-player-area-container"}>
<div id={"lobby-player-text-choose-container"}>
<p id={"lobby-player-count-text"}>
Players ({this.state.usernames.length}/10)
</p>
<div style={{ display: "flex", flexDirection: "column", alignItems: "flex-start" }}>
<p id={"lobby-player-count-text"}>
Players ({this.state.usernames.length}/{this.state.targetLobbySize})
</p>

{isVIP ? (
<div style={{ marginTop: "5px", display: "flex", alignItems: "center", color: "white" }}>
<label style={{ marginRight: "10px" }}>Target Game Size:</label>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrapped the <select> in the <label> for accessibility.

<select
value={this.state.targetLobbySize}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left this as it is for now as suggested, but happy to revisit later if latency becomes an issue.

onChange={(e) => this.sendWSCommand({
command: WSCommandType.SET_LOBBY_SIZE,
size: parseInt(e.target.value)
})}
style={{ padding: "5px", borderRadius: "4px", backgroundColor: "#333", color: "white", border: "1px solid #555" }}
>
{[5, 6, 7, 8, 9, 10].map(n => <option key={n} value={n}>{n} Players</option>)}
</select>
</div>
) : (
<p style={{ marginTop: "5px", color: "gray", fontSize: "0.9em" }}>
Target Game Size: {this.state.targetLobbySize}
</p>
)}
</div>

<button
id={"lobby-change-icon-button"}
onClick={this.onClickChangeIcon}
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/types/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const enum WSCommandType {
PING = "ping",
START_GAME = "start-game",
GET_STATE = "get-state",
SET_LOBBY_SIZE = "set-lobby-size",
REGISTER_CHANCELLOR_VETO = "chancellor-veto",
REGISTER_PRESIDENT_VETO = "president-veto",
REGISTER_PEEK = "register-peek",
Expand All @@ -25,6 +26,7 @@ export const enum WSCommandType {
export type ServerRequestPayload =
| { command: WSCommandType.PING }
| { command: WSCommandType.START_GAME }
| { command: WSCommandType.SET_LOBBY_SIZE; size: number }
| { command: WSCommandType.GET_STATE }
| { command: WSCommandType.REGISTER_CHANCELLOR_VETO }
| { command: WSCommandType.REGISTER_PRESIDENT_VETO; veto: boolean }
Expand Down