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
Changes from all commits
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
79 changes: 79 additions & 0 deletions app/components/player/kick-player.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { useFetcher } from "@remix-run/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);

return (
<fetcher.Form method="DELETE" action={`/room/${roomId}/player`}>
<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
@@ -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 =
@@ -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);

@@ -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?

@@ -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
@@ -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 &&
14 changes: 14 additions & 0 deletions app/engine/engine.test.ts
Original file line number Diff line number Diff line change
@@ -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 },
@@ -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,
29 changes: 29 additions & 0 deletions app/engine/engine.ts
Original file line number Diff line number Diff line change
@@ -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",
@@ -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(
1 change: 1 addition & 0 deletions app/engine/use-engine-context.ts
Original file line number Diff line number Diff line change
@@ -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,
3 changes: 2 additions & 1 deletion app/engine/use-game-engine.ts
Original file line number Diff line number Diff line change
@@ -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,
@@ -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,
12 changes: 10 additions & 2 deletions app/routes/room.$roomId.player.tsx
Original file line number Diff line number Diff line change
@@ -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();
@@ -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 } });
Loading