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.

Sign up for GitHub

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

Added voting feature and strikethrough on invalid answers, next question button #7

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Added voting feature and strikethrough on invalid answers, next question button #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4237eab
Added ugly flags to toggle votes
SpencerHC Aug 4, 2024
164171b
Fixed flag icons, added strikethrough on 1 user repeat answers
SpencerHC Aug 30, 2024
0a5be59
fixed bug with first answering being invalid
SpencerHC Sep 12, 2024
1c72e61
Removed half baked comment
SpencerHC Sep 12, 2024
97c8889
Added next question button
SpencerHC Sep 12, 2024
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
66 changes: 61 additions & 5 deletions game/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,17 @@ const NUM_CATEGORIES = CATEGORIES.length;

export type Phase = "write" | "review";

export type answer = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type answer = {
export type Answer = {

answer: string;
valid: boolean;
};

export type B = {
categories: string[];
userIds: UserId[];
// Info of the current round.
answers: Record<UserId, string[]>;
answers: Record<UserId, answer[]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a step back and revisit our data structures for the match state(s). I thought about it for a while and here is what I came up with:

type B = {
  // The N categories, common for each round.
  categories: string[];

  // The players.
  userIds: UserId[];

  // Answers entered by all the users for all the categories for a round.
  answers: Record<UserId, string>[];

  // User votes for the previous answers. This is how many points the voter says it is worth.
  // Note that the first user in the data structure is the writer and the second is the voter.
  votes: Record<UserId, Record<UserId, number>>[]

  // The current round index.
  round: number;
  // The current phase.
  phase: Phase
  // If in the "review" phase, what category are we reviewing.
  categoryIndex: number;
}

type PB = {
  // Answers as the user is entering them in the Read phase.
  answers: string[];
}

This is assuming we ditch everything between rounds (otherwise we would need an extra nested array for the Round).

I don't know if we need a isValid boolean for answers. Maybe we can simply compute that every time without saving it to the state. If we decide to save it to the state, we need to be clear about what the info means, because there are many ways an answer can be invalid (duplicate in a given category, down voted, duplicate for a given user).

playerVotes: Record<string, Record<UserId, number>>;
round: number;
roundStep: number;
simlmx marked this conversation as resolved.
Show resolved Hide resolved
phase: Phase;
Expand All @@ -47,6 +53,11 @@ type WritePayload = {
answer: string;
};

type VotePayload = {
userId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to pass the userId as a parameter: we have access to the userId that is making the move, always.

Suggested change
userId: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

Scratch that we need a userId, the userId of the player who wrote the answer we are voting against.

answer: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
answer: string;
categoryIndex: number;
points: number;

altough it's fine to keep the points for a later PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is categoryIndex the index of the answer in the category?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops disregard, other comment referring to this was hidden for some reason

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right to be confused I think we also do need the userId in the payload, but it's the user id of the "voteD" player, not the voter! This is confusing 😅

};

const write: PlayerMove<GS, WritePayload> = {
executeNow({ board, playerboard, payload }) {
const { answer, index } = payload as WritePayload;
Expand All @@ -69,14 +80,58 @@ const write: PlayerMove<GS, WritePayload> = {
}

board.answers = {};

board.playerVotes = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's swap the if at line 76:

// We are done with this phase when any player has entered 10 answers.
const weAreDone = answerRound.every((a) => a.length > 0)

if (!weAreDone) {
  return;
}

// When we are done we go to the Write phase.
board.phase = 'review' as const;

board.answer = {}
board.playerVotes = {}
...

const allAnswers: string[] = [];
for (const [userId, playerboard] of Object.entries(playerboards)) {
let checkedAnswers: string[] = [];
const playerAnswers = playerboard.answers[board.round];
board.answers[userId] = playerAnswers;
board.answers[userId] = playerAnswers.map((a) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would skip the validation here and simply copy the answers from the playerboard to the board. This will simplify the code a big deal.

If we really want to save the "validity" to the state, we should probably do it in a separate function.

If we don't write it to the state we can simply write functions like

isAnswerUniqueInCategory(...: {answer: string; allAnswers: Record<UserId, string>})
isAnswerUniqueForUser(...)

that we would use in the backend and frontend.

In any case I would not worry about it too much for now!

if (
playerAnswers.filter(
(answer) => answer.toLowerCase() === a.toLowerCase(),
).length > 1
) {
const valid = checkedAnswers.includes(a) ? false : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

const valid = !checkAnswers.includes(a)

checkedAnswers.push(a);
return {
answer: a,
valid: valid,
};
} else {
return {
answer: a,
valid: true,
};
}
});

allAnswers.push(...playerAnswers);
}
allAnswers.forEach((a) => {
board.playerVotes[a] = {};
for (const [userId] of Object.entries(playerboards)) {
board.playerVotes[a][userId] = 1;
}
});
Comment on lines +110 to +115
Copy link
Contributor

Choose a reason for hiding this comment

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

See the new proposed votes datastructure!

Suggested change
allAnswers.forEach((a) => {
board.playerVotes[a] = {};
for (const [userId] of Object.entries(playerboards)) {
board.playerVotes[a][userId] = 1;
}
});
// Initialize the votes to 1 points each.
// TODO: deal with 2-pointers
const votesForOneAnswer = Object.fromEntries(userIds.map((userId) => [userId, 1]))
board.votes = board.categories.map(() => Object.fromEntries(
userIds.map((userId) => [userId, {...votesForOneAnswer}])
));

Not sure if there is a simpler way 🤔

},
};

const vote: PlayerMove<GS, VotePayload> = {
executeNow({ board, playerboard, payload }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
executeNow({ board, playerboard, payload }) {
executeNow({ board, playerboard, userId, payload }) {

if (board.playerVotes[payload.answer][payload.userId] === 1) {
board.playerVotes[payload.answer][payload.userId] = 0;
} else {
board.playerVotes[payload.answer][payload.userId] = 1;
Comment on lines +121 to +124
Copy link
Contributor

@simlmx simlmx Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm jumping ahead a little here with the logic getting closer to supporting 2-pointers.

Suggested change
if (board.playerVotes[payload.answer][payload.userId] === 1) {
board.playerVotes[payload.answer][payload.userId] = 0;
} else {
board.playerVotes[payload.answer][payload.userId] = 1;
const { categoryIndex, userId: answerUserId } = payload;
const currentVote = board.votes[categoryIndex][answerUserId][userId];
// TODO `maxPoints` should be calculated from the answer.
const maxPoints = 1;
const newVote = (currentVote + 1) % (maxPoints + 1);
board.votes[categoryIndex][answerUserId][userId] = newvote;

EDITED: added the other userId as we need two!

}
},
};

const nextStep: PlayerMove<GS> = {
executeNow({ board }) {
board.roundStep++;
},
};

const game = {
initialBoards: ({ players }) => {
const board = {
Expand All @@ -85,6 +140,7 @@ const game = {
roundStep: 0,
userIds: players,
answers: {},
playerVotes: {},
// TODO No letter to start with, then a count them and then only we pick the letter.
letter: "A",
phase: "write" as const,
Expand All @@ -100,11 +156,11 @@ const game = {

return { board, playerboards };
},
playerMoves: { write },
playerMoves: { write, vote, nextStep },
minPlayers: 1,
maxPlayers: 10,
} satisfies Game<GS>;

export type G = typeof game;

export { game, write };
export { game, write, vote, nextStep };
24 changes: 17 additions & 7 deletions ui/src/Board.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
makeUseMakeMove,
useUsername,
useIsPlayer,
useMyUserId,
} from "@lefun/ui";

import classNames from "classnames";
Expand All @@ -20,6 +21,7 @@ import { useLingui } from "@lingui/react";

import { useFonts } from "./useFonts";
import { UserId } from "@lefun/core";
import { Flag } from "./Flag";

const useSelector = makeUseSelector<GS>();
const useMakeMove = makeUseMakeMove<G>();
Expand Down Expand Up @@ -83,11 +85,12 @@ function AnswerInput({ index }: { index: number }) {
<TextBox
caption={category}
text={newAnswer}
readonly={!isPlayer}
readOnly={!isPlayer}
onChange={(text: string) => {
setNewAnswer(text);
makeMove("write", { index, answer: text });
}}
strikeThrough={false}
/>
);
}
Expand All @@ -96,6 +99,7 @@ function ReviewContent() {
const userIds = useSelector((state) => state.board.userIds);
const roundstep = useSelector((state) => state.board.roundStep);
const category = useSelector((state) => state.board.categories[roundstep]);
const makeMove = useMakeMove();
return (
<div>
<div className="flex justify-center">{category}</div>
Expand All @@ -104,6 +108,7 @@ function ReviewContent() {
<ReviewPlayer key={userId} userId={userId} />
))}
</div>
<button onClick={() => makeMove("nextStep")}>Next Question</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<button onClick={() => makeMove("nextStep")}>Next Question</button>
<button onClick={() => makeMove("nextStep")}>Next Category</button>

</div>
);
}
Expand All @@ -123,17 +128,22 @@ function WriteContent() {

function ReviewPlayer({ userId }: { userId: UserId }) {
const username = useUsername(userId);
const playerUserId = useMyUserId();
const roundStep = useSelector((state) => state.board.roundStep);
const answers = useSelector((state) => state.board.answers[userId]);
const answer = answers[roundStep];

return (
<TextBox
caption={username || ""}
text={answer}
readonly={true}
captionAlwaysOnTop={true}
/>
<div className="flex justify-center">
<TextBox
caption={username || ""}
text={answer.answer}
strikeThrough={!answer.valid}
readOnly={true}
captionAlwaysOnTop={true}
/>
<Flag userId={playerUserId} answer={answer.answer}></Flag>
</div>
);
}

Expand Down
29 changes: 29 additions & 0 deletions ui/src/Flag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { UserId } from "@lefun/core";
import { makeUseSelector, makeUseMakeMove } from "@lefun/ui";
import { G, GS } from "categorio-game";
export function Flag({ userId, answer }: { userId: UserId; answer: string }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function Flag({ userId, answer }: { userId: UserId; answer: string }) {
export function Flag({ categoryIndex }: { categoryIndex: number }) {

const useMakeMove = makeUseMakeMove<G>();
const useSelector = makeUseSelector<GS>();
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably have a separate file, maybe hooks.ts where we put those two lines. All makeUse* functions are doing is adding the typing information to the use* functions (so that type checking is done when we call them).

So this would become a simple import:

import { useMakeMove, useSelector } from './hooks'

let flagged: number = useSelector(
(state) => state.board.playerVotes[answer][userId],
);
const makeMove = useMakeMove();
console.log(flagged);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(flagged);

return (
<button
onClick={() => makeMove("vote", { userId: userId, answer: answer })}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
onClick={() => makeMove("vote", { userId: userId, answer: answer })}
onClick={() => makeMove("vote", { answer })}

>
<svg
xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
viewBox="0 0 24 24"
fill={!!flagged ? "#000000" : "#D40000"}
className="icon icon-tabler icons-tabler-filled icon-tabler-flag"
>
<path stroke="none" d="M0 0h24v24H0z" fill="none" />
<path d="M4 5a1 1 0 0 1 .3 -.714a6 6 0 0 1 8.213 -.176l.351 .328a4 4 0 0 0 5.272 0l.249 -.227c.61 -.483 1.527 -.097 1.61 .676l.005 .113v9a1 1 0 0 1 -.3 .714a6 6 0 0 1 -8.213 .176l-.351 -.328a4 4 0 0 0 -5.136 -.114v6.552a1 1 0 0 1 -1.993 .117l-.007 -.117v-16z" />
</svg>
</button>
);
}
15 changes: 9 additions & 6 deletions ui/src/TextBox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import classNames from "classnames";
export default function TextBox({
caption,
text,
readonly,
readOnly,
strikeThrough,
onChange,
captionAlwaysOnTop = false,
}: {
caption: string;
text: string;
readonly: boolean;
readOnly: boolean;
strikeThrough: boolean;
onChange?: (text: string) => void;
captionAlwaysOnTop?: boolean;
}) {
Expand All @@ -33,17 +35,18 @@ export default function TextBox({
<input
type="text"
placeholder={captionOnTop ? "" : caption}
readOnly={readonly}
readOnly={readOnly}
className={classNames(
"px-3.5 w-full",
"pb-1.5 pt-2",
"border border-neutral-400 rounded-sm",
"text-neutral-800 placeholder:text-neutral-700",
"font-medium placeholder:font-light",
"text-lg placeholder:text-base",
readonly
readOnly
? "focus:caret-none focus: outline-none"
: "focus:caret-primary focus:outline-primary",
strikeThrough ? "line-through" : "",
)}
value={text}
onChange={(e) => {
Expand All @@ -52,8 +55,8 @@ export default function TextBox({
onChange(value);
}
}}
onFocus={() => !readonly && setFocused(true)}
onBlur={() => !readonly && setFocused(false)}
onFocus={() => !readOnly && setFocused(true)}
onBlur={() => !readOnly && setFocused(false)}
></input>
</div>
);
Expand Down