-
Notifications
You must be signed in to change notification settings - Fork 5
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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`} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
); | ||
} |
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 ? ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alphabetize these and the fields in |
||
/> | ||
) : ( | ||
<PlayerScore | ||
key={i} | ||
player={p} | ||
hasBoardControl={p.userId === boardControl} | ||
winning={p.score === maxScore} | ||
/> | ||
), | ||
)} | ||
</div> | ||
); | ||
} |
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙌 |
||
draft.players.set(PLAYER2.userId, PLAYER2); | ||
}), | ||
}, | ||
{ | ||
name: "Round start", | ||
state: initialState, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ enableMapSet(); | |
|
||
export enum ActionType { | ||
Join = "join", | ||
Kick = "kick", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you call this |
||
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( | ||
|
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The game engine did not give us |
||
board, | ||
buzzes: state.buzzes, | ||
category, | ||
|
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you change this to a 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 } }); | ||
|
There was a problem hiding this comment.
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}