Skip to content
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

Allow kicking of players #68

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
87 changes: 87 additions & 0 deletions app/components/player/kick-player.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { useFetcher } from "@remix-run/react";
import * as React from "react";

import type { Action, Player } from "~/engine";
import { useEngineContext } from "~/engine";
import useSoloAction from "~/utils/use-solo-action";
import { PlayerScoreBox } from "./player";

function KickPlayer({
hasBoardControl,
player,
winning = false,
}: {
hasBoardControl: boolean;
player: Player;
winning?: boolean;
}) {
return (
<PlayerScoreBox player={player} hasBoardControl={hasBoardControl}>
<div className="flex w-full items-center gap-2 text-2xl">
<p className="font-handwriting font-bold text-slate-300">
{player.name}
</p>
<div className="ml-auto flex items-center gap-2">
<button
Copy link
Owner

Choose a reason for hiding this comment

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

oh, can you also add alt text to this that shows up on hover like Kick player ${player.name}

type="submit"
className="flex grow items-center rounded-md
p-1
text-slate-300
transition-colors hover:bg-slate-800
hover:text-white focus:bg-slate-800 focus:text-white"
>
{/* Heroicon name: solid/x-mark */}
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
strokeWidth={1.5}
stroke="currentColor"
className="h-5 w-5"
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="M6 18 18 6M6 6l12 12"
/>
</svg>
</button>
{winning && "👑"}
</div>
</div>
</PlayerScoreBox>
);
}

export function KickPlayerForm({
roomId,
player,
winning = false,
}: {
roomId: number;
player: Player;
winning?: boolean;
}) {
const { soloDispatch, boardControl } = useEngineContext();

const fetcher = useFetcher<Action>();
useSoloAction(fetcher, soloDispatch);

const formRef = React.useRef<HTMLFormElement | null>(null);

return (
<fetcher.Form
method="DELETE"
action={`/room/${roomId}/player`}
Copy link
Owner

Choose a reason for hiding this comment

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

🙏 nice

ref={formRef}
>
<input type="hidden" name="userId" value={player.userId} />
<input type="hidden" name="name" value={player.name} />
<KickPlayer
player={player}
hasBoardControl={player.userId === boardControl}
winning={winning}
/>
</fetcher.Form>
);
}
34 changes: 25 additions & 9 deletions app/components/player/player.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { GameState, useEngineContext } from "~/engine";
import { formatDollars, stringToHslColor } from "~/utils";
import { RoomProps } from "../game";
import { EditPlayerForm } from "./edit-player";
import { KickPlayerForm } from "./kick-player";

// https://stackoverflow.com/questions/70524820/is-there-still-no-easy-way-to-split-strings-with-compound-emojis-into-an-array
const COMPOUND_EMOJI_REGEX =
Expand Down Expand Up @@ -111,7 +112,8 @@ function getMaxScore(others: Player[], you?: Player) {
* - Shows each player's name and score
*/
export function PlayerScores({ roomId, userId }: RoomProps) {
const { players, boardControl, type, round } = useEngineContext();
const { players, boardControl, type, round, numAnswered } =
useEngineContext();

const yourPlayer = players.get(userId);

Expand All @@ -126,6 +128,11 @@ export function PlayerScores({ roomId, userId }: RoomProps) {
type !== GameState.GameOver &&
(type !== GameState.PreviewRound || round !== 0);

const canKick =
(type === GameState.ShowBoard || type === GameState.PreviewRound) &&
numAnswered === 0 &&
round === 0;

return (
<div className="flex flex-col gap-2 sm:grid sm:grid-cols-3">
{yourPlayer ? (
Copy link
Owner

Choose a reason for hiding this comment

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

can you not kick yourself?

Expand All @@ -143,14 +150,23 @@ export function PlayerScores({ roomId, userId }: RoomProps) {
/>
)
) : null}
{sortedOtherPlayers.map((p, i) => (
<PlayerScore
key={i}
player={p}
hasBoardControl={p.userId === boardControl}
winning={p.score === maxScore}
/>
))}
{sortedOtherPlayers.map((p, i) =>
canKick ? (
<KickPlayerForm
key={i}
roomId={roomId}
player={p}
winning={p.score === maxScore}
Comment on lines +156 to +159
Copy link
Owner

Choose a reason for hiding this comment

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

alphabetize these and the fields in PlayerScore? that would make it easier to compare their props

/>
) : (
<PlayerScore
key={i}
player={p}
hasBoardControl={p.userId === boardControl}
winning={p.score === maxScore}
/>
),
)}
</div>
);
}
3 changes: 2 additions & 1 deletion app/engine/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export function isPlayerAction(action: Action): action is {
} {
return (
(action.type === ActionType.Join ||
action.type === ActionType.ChangeName) &&
action.type === ActionType.ChangeName ||
action.type === ActionType.Kick) &&
action.payload !== null &&
typeof action.payload === "object" &&
"userId" in action.payload &&
Expand Down
14 changes: 14 additions & 0 deletions app/engine/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ const PLAYER1_JOIN_ACTION: Action = {
payload: { name: PLAYER1.name, userId: PLAYER1.userId },
};

const PLAYER1_KICK_ACTION: Action = {
type: ActionType.Kick,
payload: { name: PLAYER1.name, userId: PLAYER1.userId },
};

const PLAYER2_JOIN_ACTION: Action = {
type: ActionType.Join,
payload: { name: PLAYER2.name, userId: PLAYER2.userId },
Expand Down Expand Up @@ -146,6 +151,15 @@ describe("gameEngine", () => {
draft.players.set(PLAYER2.userId, PLAYER2);
}),
},
{
name: "Two players join, first is kicked, second gets board control",
state: initialState,
actions: [PLAYER1_JOIN_ACTION, PLAYER2_JOIN_ACTION, PLAYER1_KICK_ACTION],
expectedState: produce(initialState, (draft) => {
draft.boardControl = PLAYER2.userId;
Copy link
Owner

Choose a reason for hiding this comment

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

🙌

draft.players.set(PLAYER2.userId, PLAYER2);
}),
},
{
name: "Round start",
state: initialState,
Expand Down
29 changes: 29 additions & 0 deletions app/engine/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enableMapSet();

export enum ActionType {
Join = "join",
Kick = "kick",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered calling this "Leave", but since a kick can be initiated by another player, I went with kick. I'm open to "Leave" if folks think that is better.

Copy link
Owner

Choose a reason for hiding this comment

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

I think "kick" is better, since anyone can kick anyone else. You can also kick yourself

ChangeName = "change_name",
StartRound = "start_round",
ChooseClue = "choose_clue",
Expand Down Expand Up @@ -112,6 +113,34 @@ export function gameEngine(state: State, action: Action): State {
draft.boardControl = action.payload.userId;
}
});
case ActionType.Kick:
if (!isPlayerAction(action)) {
throw new Error("PlayerKick action must have an associated player");
}
return produce(state, (draft) => {
// Don't kick players after the game has started.
if (
(draft.type !== GameState.ShowBoard &&
draft.type !== GameState.PreviewRound) ||
draft.numAnswered > 0 ||
draft.round > 0
) {
return;
}
// Don't allow the only player to be kicked.
if (draft.players.size === 1) {
Copy link
Owner

Choose a reason for hiding this comment

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

nice catch, can you prevent this on the client side as well (once players can kick themselves)?

return;
}
// If this player has board control, give it to the next player.
if (draft.boardControl === action.payload.userId) {
const players = Array.from(draft.players.keys());
Copy link
Owner

Choose a reason for hiding this comment

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

can you call this playerIds to disambiguate it from Map<string, Player>?

players.sort();
const index = players.indexOf(action.payload.userId);
const nextPlayer = players[(index + 1) % players.length];
draft.boardControl = nextPlayer;
}
draft.players.delete(action.payload.userId);
});
case ActionType.ChangeName:
if (!isPlayerAction(action)) {
throw new Error(
Expand Down
1 change: 1 addition & 0 deletions app/engine/use-engine-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const GameEngineContext = React.createContext<
type: GameState.PreviewRound,
activeClue: null,
answers: new Map(),
numAnswered: 0,
answeredBy: () => false,
board: { categories: [], categoryNames: [] },
boardControl: null,
Expand Down
3 changes: 2 additions & 1 deletion app/engine/use-game-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { getSupabase } from "~/supabase";
import type { Action } from "./engine";
import { gameEngine, getWinningBuzzer } from "./engine";
import { applyRoomEventsToState, isTypedRoomEvent } from "./room-event";
import { getClueValue, State, stateFromGame } from "./state";
import { State, getClueValue, stateFromGame } from "./state";

export enum ConnectionState {
ERROR,
Expand Down Expand Up @@ -74,6 +74,7 @@ function stateToGameEngine(
*/
answeredBy,
answers: state.answers.get(clueKey) ?? new Map<string, string>(),
numAnswered: state.numAnswered,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The game engine did not give us numAnswered (the number of answered clues in the round) from the State, so I added code to thread it through.

board,
buzzes: state.buzzes,
category,
Expand Down
12 changes: 10 additions & 2 deletions app/routes/room.$roomId.player.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import { getRoom } from "~/models/room.server";
import { getSolve, markAttempted } from "~/models/solves.server";

export async function action({ request, params }: ActionFunctionArgs) {
if (request.method !== "POST" && request.method !== "PATCH") {
if (
request.method !== "POST" &&
request.method !== "PATCH" &&
request.method !== "DELETE"
) {
throw new Response("method not allowed", { status: 405 });
}
const formData = await request.formData();
Expand All @@ -28,7 +32,11 @@ export async function action({ request, params }: ActionFunctionArgs) {
}

const type =
request.method === "POST" ? ActionType.Join : ActionType.ChangeName;
Copy link
Owner

Choose a reason for hiding this comment

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

can you change this to a switch (request.method) now instead of multiple ternary operators? the cute/annoying way to do this without reassignment is something like:

const type = () => {
    switch (request.method) {
        case "POST":
            return ActionType.Join;
        // ...
}();

request.method === "POST"
? ActionType.Join
: request.method === "PATCH"
? ActionType.ChangeName
: ActionType.Kick;

if (roomId === -1) {
return json({ type, payload: { userId, name } });
Expand Down
Loading