Skip to content

Comments

Timer renyi 48#71

Open
reny1cao wants to merge 9 commits intodevfrom
Timer-Renyi-48
Open

Timer renyi 48#71
reny1cao wants to merge 9 commits intodevfrom
Timer-Renyi-48

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

</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