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

Timer renyi 48 #71

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Timer renyi 48 #71

wants to merge 9 commits into from

Conversation

reny1cao
Copy link
Collaborator

Two separated timer, one in the back-end GameIO, one in the front-end GamePrompt. When started a game, will emit a "start timer" event to let the front-end know it's time to start the timer. At the same time will start an interval in the back-end. When the interval expired, it will call nextGameTurn.

@reny1cao reny1cao requested a review from tonyc856 June 19, 2020 02:47
Comment on lines 14 to 17
gameIO.state.io.on("start timer", () => {
clearTimeout(countDown);
setTimer(45)
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be in its own useEffect():
useEffect(() => {
gameIO.state.io.on("start timer", () => {
clearTimeout(countDown);
setTimer(45)
});
}, []); // [] means set the listener only once on mount


gameIO.state.io.on("start timer", () => {
clearTimeout(countDown);
setTimer(45)
Copy link
Collaborator

@tonyc856 tonyc856 Jun 19, 2020

Choose a reason for hiding this comment

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

We should add and use the server's countdown time to gameState.gameTurn (maybe as gameState.gameTurn.timeLeft).

The reason for this is, if the user refreshes the page during the game, I believe the countdown time would be incorrect. By using the current countdown time from the gameState.gameTurn when the game page reloads, we can guarantee it to be accurate.


const [timer, setTimer] = useState(45);

let countDown = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

const [countdown, setCountDown] = useState(null);

variables that are not stateful can be inaccurate upon the page being rerendered

@@ -14,7 +29,7 @@ function GamePrompt({ gameState, player }) {
<TimerIcon fontSize="large" />
</Grid>
<Grid item>
<Typography variant="h6">45s left</Typography>
<Typography variant="h6">{gameState.winner ? 0 : countDown}s left</Typography>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious - why not just have it set as just countdown? I don't see the need to just show a 0 on the timer when the game is over

Copy link
Collaborator Author

@reny1cao reny1cao Jul 1, 2020

Choose a reason for hiding this comment

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

Oh, it's because It feels odd to me that the timer is running after the game ended

this.gameIO
.to(matchId)
.emit("game state change", match.getGameState());
this.gameIO.to(matchId).emit("start timer");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this emit anymore now that we listen to gameState.timeEnd's change on the frontend

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I forget to delete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants