PLAN-156-late-user-joins-the-game #236
Conversation
| if (gameRoomService.getGameState(ws.roomId) === GameState.GAME_NOT_STARTED) { | ||
| if (gameService.getVoters(ws.roomId).length > 1) { | ||
| gameRoomService.setGameState(ws.roomId, GameState.GAME_IN_PROGRESS); | ||
| publishGameState(ws); |
There was a problem hiding this comment.
Could we move this call into a single place and it would be executed after every action taken by players?
There was a problem hiding this comment.
Why should we send so many messages? For example if user makes two turns(for example changes his mind), we would publish game that has not changed state twice. I think we should leave game state publishing, to when its really needed or game state changes
|
|
||
| export const unsubscribe = (ws: GameWebSocket): void => { | ||
| const players = getPlayers(ws.roomId); | ||
| try { |
There was a problem hiding this comment.
This method got too large. Its hard to follow.
| const playerId = findPlayerByConnection(ws).id; | ||
|
|
||
| if ( | ||
| gameService.getVoters(ws.roomId).length === 2 && |
There was a problem hiding this comment.
why is this condition needed?
There was a problem hiding this comment.
Because, when there is only two people in the room and the game is not finished, when one of them logs out, in my opinion the game state should become game not started (we don't allow player to make votes, when he's the only one in the room).
|
|
||
| if ( | ||
| gameService.getVoters(ws.roomId).length === 2 && | ||
| gameRoomService.getGameState(ws.roomId) !== GameState.GAME_FINISHED |
There was a problem hiding this comment.
Can we create a general algorithm that would be applied no matter how many players we have in the game or what the game state is.
| payload: allPlayers, | ||
| }); | ||
| publishGameState(ws); | ||
| publishFinalBoard(ws); |
There was a problem hiding this comment.
This is repeated all over the place. Can we find a better way to determine when final board should be pushed apart from placing it directly in the code?
| newPlayer.status = PlayerStatus.FigurePlaced; | ||
| } else if (gameService.playerHasSkipped(params.roomId, params.playerId)) { | ||
| newPlayer.status = PlayerStatus.MoveSkipped; | ||
| } else if ( |
There was a problem hiding this comment.
theres no if condition
There was a problem hiding this comment.
There is, its in the next line and wrapped with ( ). It probably got formatted this way, because the line got too large
|
|
||
| export const resetGame = (ws: GameWebSocket): void => { | ||
| gameService.clearBoard(ws.roomId); | ||
| if (gameService.getVoters(ws.roomId).length < 2) { |
There was a problem hiding this comment.
why is this condition needed?
There was a problem hiding this comment.
Because, without this condition, if one player joins the room and presses restart game button, the game state would become GameInProgress and he would be able to place figures on the board (at least in FE)
| }; | ||
|
|
||
| export const figureMoved: Handler = (ws, payload: PlaceFigureMessage): void => { | ||
| if (gameRoomService.getGameState(ws.roomId) !== GameState.GAME_IN_PROGRESS) { |
There was a problem hiding this comment.
this seems like some sort of workaround. Why is this needed?
There was a problem hiding this comment.
Its for a safety reasons. Without this check, even if game state is GameNotStarted and user cannot place figure on the board, it would be possible to send a message for example with a bot
| const isOneVoteMissing = playerCount === votersWhoMadeMoveCount + 1; | ||
| const hasPlayerMoved = player[1].status !== PlayerStatus.ActionNotTaken; | ||
|
|
||
| if (isOneVoteMissing && voters.length > 1 && !hasPlayerMoved) { |
There was a problem hiding this comment.
why isOneVoteMissing not enough?
There was a problem hiding this comment.
I've got rid of voters.length > 1 condition, but hasPlayerMoved is needed, because, we need to know, if the particular player has not moved. It also reflects in method name, which is checkIfLastToVote
| roomId: string, | ||
| playerId: string, | ||
| ): boolean => { | ||
| const player = findPlayerById(roomId, playerId); |
There was a problem hiding this comment.
What is the logic behind puting methods into services?
Why is this method inside the players service, while getPlayers and getVoterWhoMadeActionCount are in gameService and gameRoomService?
No description provided.