diff --git a/server/internal/wsapi/chat_test.go b/server/internal/wsapi/chat_test.go index 27973b6..d0f6b8a 100644 --- a/server/internal/wsapi/chat_test.go +++ b/server/internal/wsapi/chat_test.go @@ -60,9 +60,7 @@ func TestChatWorksInTheLobbyAndInAGame(t *testing.T) { // Started inline rather than through readyAndStart: pvpLobby has already // drained the room states that helper waits for. - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) host.await("game_started") guest.await("game_started") @@ -80,7 +78,7 @@ func TestAJoinerSeesNothingSaidBeforeTheySatDown(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() host.say("bí mật") @@ -88,9 +86,7 @@ func TestAJoinerSeesNothingSaidBeforeTheySatDown(t *testing.T) { guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) if got := guest.await("chat_history").GetChatHistory().GetMessages(); len(got) != 0 { t.Errorf("a joiner was handed %d messages from before they arrived: %+v", len(got), got) @@ -110,7 +106,7 @@ func TestCreatingARoomReplaysAnEmptyHistory(t *testing.T) { c := dial(t, url) c.hello("Chủ phòng") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() if got := c.await("chat_history").GetChatHistory().GetMessages(); len(got) != 0 { t.Errorf("a new room came with %d messages", len(got)) @@ -156,23 +152,19 @@ func TestChatHistorySurvivesAGame(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") host.say("trước ván") host.await("chat_message") - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) start := host.await("game_started").GetGameStarted() guest.await("game_started") resignAndSettle(t, host, guest, start) @@ -327,7 +319,7 @@ func TestChatDoesNotKeepARoomAlive(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() host.await("room_state") // Chatting throughout the window; the clock must keep running anyway. @@ -350,14 +342,12 @@ func TestVacatedSeatKeepsItsWordsButLosesItsAuthor(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") diff --git a/server/internal/wsapi/codec.go b/server/internal/wsapi/codec.go index 1314fb8..5cb94e3 100644 --- a/server/internal/wsapi/codec.go +++ b/server/internal/wsapi/codec.go @@ -78,9 +78,9 @@ func wordReportedMsg(word string) *noituv1.ServerMessage { // errorMsg carries a UI key, never prose and never an internal error string. // The Vietnamese copy lives in the frontend so all wording stays in one place, // and a raw error would leak server internals to anyone with a socket. -func errorMsg(code string) *noituv1.ServerMessage { +func errorMsg(code errCode) *noituv1.ServerMessage { return &noituv1.ServerMessage{Payload: &noituv1.ServerMessage_Error{ - Error: &noituv1.ServerError{Code: code, Message: code}, + Error: &noituv1.ServerError{Code: string(code), Message: string(code)}, }} } diff --git a/server/internal/wsapi/dispatch.go b/server/internal/wsapi/dispatch.go index 4a78400..2aa4a87 100644 --- a/server/internal/wsapi/dispatch.go +++ b/server/internal/wsapi/dispatch.go @@ -6,6 +6,7 @@ import ( "time" noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1" + "github.com/tiennm99dev/noitu/server/internal/game" "github.com/tiennm99dev/noitu/server/internal/vietnamese" ) @@ -19,8 +20,8 @@ import ( // registered resume token, and accepting them before the handshake would mean // carrying "maybe not greeted yet" through every branch below. func (s *session) dispatch(msg *noituv1.ClientMessage) error { - if _, isHello := msg.GetPayload().(*noituv1.ClientMessage_Hello); !isHello && s.nickname() == "" { - s.send(errorMsg("handshake_required")) + if _, isHello := msg.GetPayload().(*noituv1.ClientMessage_Hello); !isHello && !s.greeted { + s.send(errorMsg(codeHandshakeRequired)) return errHandshake } @@ -29,16 +30,18 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { return s.handleHello(p.Hello) case *noituv1.ClientMessage_StartBotGame: + if s.refuseMidGame() { + return nil + } // The limiter is charged before the payload is inspected, so a bad // difficulty costs the same as a good one and cannot be used to probe // for free. - if !s.roomLimiter.allow(time.Now()) { - s.send(errorMsg("too_many_rooms")) + if !s.allowRoom() { return nil } difficulty, ok := Difficulty(p.StartBotGame.GetDifficulty()) if !ok { - s.send(errorMsg("unknown_difficulty")) + s.send(errorMsg(codeUnknownDifficulty)) return nil } if err := s.hub.startBotRoom(s, difficulty); err != nil { @@ -46,10 +49,12 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { } case *noituv1.ClientMessage_CreateRoom: + if s.refuseMidGame() { + return nil + } // Creating a room allocates a goroutine and an engine, so one // connection must not be able to mint them without limit. - if !s.roomLimiter.allow(time.Now()) { - s.send(errorMsg("too_many_rooms")) + if !s.allowRoom() { return nil } if err := s.hub.createRoom(s); err != nil { @@ -57,30 +62,35 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { } case *noituv1.ClientMessage_JoinRoom: + if s.refuseMidGame() { + return nil + } if !s.hub.joinLimiter.allow(s.remoteIP, time.Now()) { metrics.joinsRefused.Add("too_many_attempts", 1) - s.send(errorMsg("too_many_attempts")) + s.send(errorMsg(codeTooManyAttempts)) return nil } if err := s.hub.joinRoom(p.JoinRoom.GetRoomCode(), s); err != nil { metrics.joinsRefused.Add("room_not_found", 1) - s.send(errorMsg("room_not_found")) + s.send(errorMsg(codeRoomNotFound)) } case *noituv1.ClientMessage_QuickMatch: + if s.refuseMidGame() { + return nil + } if r, _ := s.currentRoom(); r != nil { - s.send(errorMsg("already_in_a_room")) + s.send(errorMsg(codeAlreadyInARoom)) return nil } // A match mints a room exactly as CreateRoom does, so it is charged // the same way and for the same reason. - if !s.roomLimiter.allow(time.Now()) { - s.send(errorMsg("too_many_rooms")) + if !s.allowRoom() { return nil } if err := s.hub.quickMatch(s); err != nil { if errors.Is(err, errAlreadyQueued) { - s.send(errorMsg("already_queued")) + s.send(errorMsg(codeAlreadyQueued)) } else { s.send(roomCreateError(s.id, err)) } @@ -93,67 +103,54 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { s.send(quickMatchStatusMsg(false)) case *noituv1.ClientMessage_SubmitWord: - s.handleSubmit(p.SubmitWord) + // A dropped submission would otherwise leave the player waiting out the + // turn clock with no idea their word never arrived. + word, seq := p.SubmitWord.GetWord(), p.SubmitWord.GetTurnSeq() + s.toRoom(s.submitLimiter, codeNotInAGame, codeBusy, func(id game.PlayerID) any { + return submitInput{sess: s, player: id, word: word, turnSeq: seq} + }) case *noituv1.ClientMessage_Resign: - // A silently dropped resignation leaves the player staring at a board - // they thought they had left. - if r, id := s.currentRoom(); r != nil { - if !r.send(resignInput{sess: s, player: id}) { - s.send(errorMsg("game_already_over")) - } - } else { - s.send(errorMsg("not_in_a_game")) - } + // No budget of its own beyond the frame limiter: only one can ever be + // accepted per game, and a refusal goes to the sender alone. A silently + // dropped resignation leaves the player staring at a board they thought + // they had left. + s.toRoom(nil, codeNotInAGame, codeGameAlreadyOver, func(id game.PlayerID) any { + return resignInput{sess: s, player: id} + }) case *noituv1.ClientMessage_ClaimDeadEnd: // Rate-limited on the same budget as a submission: a claim is the // alternative to playing a word, not a second action alongside it. - if !s.submitLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) - return nil - } - if r, id := s.currentRoom(); r != nil { - if !r.send(claimDeadEndInput{sess: s, player: id}) { - s.send(errorMsg("busy")) - } - } else { - s.send(errorMsg("not_in_a_game")) - } + s.toRoom(s.submitLimiter, codeNotInAGame, codeBusy, func(id game.PlayerID) any { + return claimDeadEndInput{sess: s, player: id} + }) case *noituv1.ClientMessage_ReportWord: s.handleReportWord(p.ReportWord) case *noituv1.ClientMessage_SetReady: - s.toRoom(lobbyInput{sess: s, action: lobbyReady, ready: p.SetReady.GetReady()}) + s.toLobby(lobbyInput{action: lobbyReady, ready: p.SetReady.GetReady()}) case *noituv1.ClientMessage_StartGame: - s.toRoom(lobbyInput{sess: s, action: lobbyStart}) + s.toLobby(lobbyInput{action: lobbyStart}) case *noituv1.ClientMessage_KickPlayer: - s.toRoom(lobbyInput{sess: s, action: lobbyKick, target: playerIDFor(p.KickPlayer.GetPlayerId())}) + s.toLobby(lobbyInput{action: lobbyKick, target: game.PlayerID(p.KickPlayer.GetPlayerId())}) case *noituv1.ClientMessage_LeaveRoom: - s.toRoom(lobbyInput{sess: s, action: lobbyLeave}) + s.toLobby(lobbyInput{action: lobbyLeave}) case *noituv1.ClientMessage_SendChat: // Its own budget, so a talkative player never runs out of moves. The // seat itself is checked by the room, which is the only place that - // knows whether this connection still holds one. - if !s.chatLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) - return nil - } - r, id := s.currentRoom() - if r == nil { - s.send(errorMsg("not_in_a_room")) - return nil - } - // A dropped line would leave the player watching their own message - // fail to appear with no reason given. - if !r.send(chatInput{sess: s, player: id, text: p.SendChat.GetText()}) { - s.send(errorMsg("busy")) - } + // knows whether this connection still holds one. A dropped line would + // leave the player watching their own message fail to appear with no + // reason given. + text := p.SendChat.GetText() + s.toRoom(s.chatLimiter, codeNotInARoom, codeBusy, func(id game.PlayerID) any { + return chatInput{sess: s, player: id, text: text} + }) case *noituv1.ClientMessage_Ping: s.send(pongMsg(p.Ping.GetClientTimeMs(), time.Now().UnixMilli())) @@ -161,65 +158,107 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { return nil } +// allowRoom charges the room budget, answering too_many_rooms when it is +// spent. Every path that mints a room — a bot game, a code, a quick match — +// holds a goroutine and an engine, so all three share it. +func (s *session) allowRoom() bool { + if s.roomLimiter.allow(time.Now()) { + return true + } + s.send(errorMsg(codeTooManyRooms)) + return false +} + +// refuseMidGame answers already_in_a_game, and reports true, when this +// connection is seated in a room whose game is still being played. +// +// Opening or joining another room is what walks a player out of the one they +// are in, and mid-game that would be an abandonment the other players only +// learn of when the reconnect window runs out. Leaving is a deliberate act +// with its own message; a room change is not allowed to be a quiet way of +// doing it. A lobby or a finished game is left freely, as before. +// +// Checked before any limiter is charged, so a refusal costs nothing. The +// flag is read outside the room goroutine, which makes it a snapshot: a game +// starting at the same instant is a race the grace window already settles. +func (s *session) refuseMidGame() bool { + r, _ := s.currentRoom() + if r == nil || !r.liveCounted.Load() { + return false + } + s.send(errorMsg(codeAlreadyInAGame)) + return true +} + // roomCreateError names the refusal a room could not be opened for. A full // server is the player's business — they should wait, not retry at once — and // anything else is the server's, logged here because the client is only told // that it failed. func roomCreateError(sessionID string, err error) *noituv1.ServerMessage { if errors.Is(err, errServerFull) { - return errorMsg("server_full") + return errorMsg(codeServerFull) } if errors.Is(err, errDraining) { // The same key Shutdown sends to everyone already seated: a room // refused for this reason will not open a moment later the way a full // one might, so the client is told the same thing either way. - return errorMsg("server_restarting") + return errorMsg(codeServerRestarting) } slog.Error("open room", "session", sessionID, "err", err) - return errorMsg("room_start_failed") + return errorMsg(codeRoomStartFailed) } -// toRoom forwards one lobby action to the room this connection is seated in. +// toRoom forwards one input to the room this connection is seated in. // -// Rate-limited like a submission: every accepted action is broadcast to every -// seat, so an unbounded one lets a player flood the other's outbox until -// their session is closed for falling behind. A dropped action would leave a -// button that did nothing and no reason why, so every failure answers. -func (s *session) toRoom(in lobbyInput) { - if !s.submitLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) +// Every failure answers, because every input here is something the player +// is waiting to see the effect of: limiter, when set, is charged first and +// refuses with too_fast; notIn answers a connection seated nowhere; and +// dropped answers a room that would not take the input — finished, or with +// its inbox full. build receives the seat the connection holds, read at the +// same moment as the room so the two cannot disagree. +func (s *session) toRoom(limiter *bucket, notIn, dropped errCode, build func(game.PlayerID) any) { + if limiter != nil && !limiter.allow(time.Now()) { + s.send(errorMsg(codeTooFast)) return } r, id := s.currentRoom() if r == nil { - s.send(errorMsg("not_in_a_room")) + s.send(errorMsg(notIn)) return } - in.player = id - if !r.send(in) { - s.send(errorMsg("not_in_a_room")) + if !r.send(build(id)) { + s.send(errorMsg(dropped)) } } +// toLobby forwards one lobby action. +// +// Rate-limited like a submission: every accepted action is broadcast to every +// seat, so an unbounded one lets a player flood the other's outbox until +// their session is closed for falling behind. +func (s *session) toLobby(in lobbyInput) { + s.toRoom(s.submitLimiter, codeNotInARoom, codeNotInARoom, func(id game.PlayerID) any { + in.sess, in.player = s, id + return in + }) +} + // handleHello completes the handshake, resuming a prior game when the client // presents a token that is still live. func (s *session) handleHello(h *noituv1.Hello) error { if v := h.GetProtocolVersion(); v != ProtocolVersion { - s.send(errorMsg("protocol_version_mismatch")) + s.send(errorMsg(codeProtocolVersionMismatch)) return errors.New("wsapi: protocol version mismatch") } // The handshake is a one-shot transition. A second Hello would re-register // the session and rewrite the nickname of a player already seated in a // game, which nothing downstream expects. - s.mu.Lock() - repeat := s.greeted - s.greeted = true - s.mu.Unlock() - if repeat { - s.send(errorMsg("already_greeted")) + if s.greeted { + s.send(errorMsg(codeAlreadyGreeted)) return errors.New("wsapi: repeated hello") } + s.greeted = true s.setNickname(sanitizeNickname(h.GetNickname())) s.hub.register(s) @@ -235,7 +274,7 @@ func (s *session) handleHello(h *noituv1.Hello) error { // latch waiting forever for a reply that was never coming — this // connection is answered and carries on as a fresh session instead of // being closed, since a fresh session is exactly what it is. - s.send(errorMsg("session_not_resumable")) + s.send(errorMsg(codeSessionNotResumable)) } return nil } @@ -251,11 +290,11 @@ func (s *session) resumeFrom(prior *session) { metrics.resumesAttempted.Add(1) r, id := prior.currentRoom() if r == nil { - s.send(errorMsg("game_already_over")) + s.send(errorMsg(codeGameAlreadyOver)) return } if !r.send(resumeInput{player: id, sess: s, prior: prior}) { - s.send(errorMsg("game_already_over")) + s.send(errorMsg(codeGameAlreadyOver)) return } // Deliberately no attach and no close here. The room has not decided yet, @@ -263,23 +302,6 @@ func (s *session) resumeFrom(prior *session) { // the game it was trying to rejoin. } -func (s *session) handleSubmit(w *noituv1.SubmitWord) { - if !s.submitLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) - return - } - r, id := s.currentRoom() - if r == nil { - s.send(errorMsg("not_in_a_game")) - return - } - // A dropped submission would otherwise leave the player waiting out the - // turn clock with no idea their word never arrived. - if !r.send(submitInput{sess: s, player: id, word: w.GetWord(), turnSeq: w.GetTurnSeq()}) { - s.send(errorMsg("busy")) - } -} - // handleReportWord validates a word report and, once it is worth logging, // hands it to the current room for the context only the room goroutine may // read — the syllable in play, and the room's own mode and code. @@ -291,19 +313,19 @@ func (s *session) handleSubmit(w *noituv1.SubmitWord) { // still file a report, acknowledged with mode "none" and no link. func (s *session) handleReportWord(m *noituv1.ReportWord) { if !s.chatLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) + s.send(errorMsg(codeTooFast)) return } word, syllables, err := vietnamese.Normalize(sanitizeText(m.GetWord(), maxWordRunes, maxNicknameMarks)) if err != nil || !vietnamese.HasEnoughSyllables(syllables) { - s.send(errorMsg("word_report_refused")) + s.send(errorMsg(codeWordReportRefused)) return } if _, already := s.reportedWords[word]; !already { if len(s.reportedWords) >= maxWordReportsPerSession { - s.send(errorMsg("word_report_limit")) + s.send(errorMsg(codeWordReportLimit)) return } s.reportedWords[word] = struct{}{} @@ -313,7 +335,7 @@ func (s *session) handleReportWord(m *noituv1.ReportWord) { // word and the room's context, never about who filed it. if r, _ := s.currentRoom(); r != nil { if !r.send(reportWordInput{sess: s, word: word}) { - s.send(errorMsg("busy")) + s.send(errorMsg(codeBusy)) } return } diff --git a/server/internal/wsapi/drain_test.go b/server/internal/wsapi/drain_test.go index d8aee07..550ca4e 100644 --- a/server/internal/wsapi/drain_test.go +++ b/server/internal/wsapi/drain_test.go @@ -5,8 +5,6 @@ import ( "net/http" "testing" "time" - - noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1" ) // TestReadyzFlipsOnDrain: /healthz is liveness and never moves; /readyz is @@ -55,7 +53,7 @@ func TestDrainRefusesNewRooms(t *testing.T) { c := dial(t, url) c.hello("Người thử") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() if got := c.await("error").GetError().GetCode(); got != "server_restarting" { t.Errorf("error code = %q, want server_restarting", got) @@ -73,9 +71,7 @@ func TestLiveGameCountTracksGamesNotLobbies(t *testing.T) { t.Fatalf("a lobby with nobody playing counted as %d live games, want 0", got) } - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) _, waits, _ := awaitLead(t, host, guest) if got := api.LiveGameCount(); got != 1 { diff --git a/server/internal/wsapi/errcodes.go b/server/internal/wsapi/errcodes.go new file mode 100644 index 0000000..82b45e0 --- /dev/null +++ b/server/internal/wsapi/errcodes.go @@ -0,0 +1,51 @@ +package wsapi + +// errCode is a ServerError.code: a key the client turns into a message, not +// text shown as is. Every code the transport can send is declared here and +// nowhere else, so the vocabulary the web client has to translate is one list +// — web/tests/error-codes.test.js reads it from this file. +type errCode string + +const ( + codeAlreadyGreeted errCode = "already_greeted" + codeAlreadyInAGame errCode = "already_in_a_game" + codeAlreadyInARoom errCode = "already_in_a_room" + codeAlreadyQueued errCode = "already_queued" + codeBadFrame errCode = "bad_frame" + codeBusy errCode = "busy" + codeCannotJoinOwnRoom errCode = "cannot_join_own_room" + codeCannotKickSelf errCode = "cannot_kick_self" + codeGameAlreadyOver errCode = "game_already_over" + codeGameInProgress errCode = "game_in_progress" + codeGameNotStarted errCode = "game_not_started" + codeGameStartFailed errCode = "game_start_failed" + codeHandshakeRequired errCode = "handshake_required" + codeKicked errCode = "kicked" + codeMustUnreadyFirst errCode = "must_unready_first" + codeNeedMorePlayers errCode = "need_more_players" + codeNoOneToKick errCode = "no_one_to_kick" + codeNotADeadEnd errCode = "not_a_dead_end" + codeNotEveryoneReady errCode = "not_everyone_ready" + codeNotInAGame errCode = "not_in_a_game" + codeNotInARoom errCode = "not_in_a_room" + codeNotTheOwner errCode = "not_the_owner" + codeNotYourSeat errCode = "not_your_seat" + codeNotYourTurn errCode = "not_your_turn" + codeOwnerNeedsNoReady errCode = "owner_needs_no_ready" + codePlayerIsReady errCode = "player_is_ready" + codePlayerOffline errCode = "player_offline" + codeProtocolVersionMismatch errCode = "protocol_version_mismatch" + codeRoomFull errCode = "room_full" + codeRoomIdleClosed errCode = "room_idle_closed" + codeRoomNotFound errCode = "room_not_found" + codeRoomStartFailed errCode = "room_start_failed" + codeServerFull errCode = "server_full" + codeServerRestarting errCode = "server_restarting" + codeSessionNotResumable errCode = "session_not_resumable" + codeTooFast errCode = "too_fast" + codeTooManyAttempts errCode = "too_many_attempts" + codeTooManyRooms errCode = "too_many_rooms" + codeUnknownDifficulty errCode = "unknown_difficulty" + codeWordReportLimit errCode = "word_report_limit" + codeWordReportRefused errCode = "word_report_refused" +) diff --git a/server/internal/wsapi/game_test.go b/server/internal/wsapi/game_test.go index 9e40288..a6a79d1 100644 --- a/server/internal/wsapi/game_test.go +++ b/server/internal/wsapi/game_test.go @@ -119,14 +119,12 @@ func TestPvPGameAlternatesTurns(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) lead, waits, start := awaitLead(t, host, guest) @@ -162,14 +160,12 @@ func TestTurnTimeoutEndsGameServerSide(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) _, waits, _ := awaitLead(t, host, guest) @@ -231,14 +227,12 @@ func TestReplayingAWordIsRejected(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) lead, waits, start := awaitLead(t, host, guest) diff --git a/server/internal/wsapi/hub.go b/server/internal/wsapi/hub.go index 1dccbf7..e0adcbe 100644 --- a/server/internal/wsapi/hub.go +++ b/server/internal/wsapi/hub.go @@ -9,7 +9,6 @@ import ( "time" "github.com/tiennm99dev/noitu/server/internal/bot" - "github.com/tiennm99dev/noitu/server/internal/game" ) // roomCodeAlphabet omits 0/O and 1/I/L. Players read these codes aloud and @@ -257,28 +256,27 @@ func (h *hub) joinRoom(code string, s *session) error { // handleCreate/handleStartBot is deliberately generous: a room that fails to // seat its creator still held a goroutine and a registry entry for a moment, // and the gauge should say so. +// +// The ceiling, the code draw and the registration share one critical +// section, so two creators racing for the last slot cannot both get it and +// two that happen to draw the same code cannot both be handed it. func (h *hub) newRegisteredRoom(mode string) (*room, error) { if h.draining.Load() { return nil, errDraining } - code, err := h.reserveCode() + h.mu.Lock() + defer h.mu.Unlock() + + if len(h.rooms) >= h.maxRooms { + return nil, errServerFull + } + code, err := h.unusedCodeLocked() if err != nil { return nil, err } - r := newRoom(h, code, h.turnLimit, h.graceFor, h.idleFor, mode) - - // The ceiling is checked under the same lock that registers the room, so - // two creators racing for the last slot cannot both get it. - h.mu.Lock() - if len(h.rooms) >= h.maxRooms { - h.mu.Unlock() - r.cancel() - return nil, errServerFull - } h.rooms[code] = r - h.mu.Unlock() metrics.roomsTotal.Add(mode, 1) metrics.roomsLive.Add(mode, 1) @@ -287,8 +285,8 @@ func (h *hub) newRegisteredRoom(mode string) (*room, error) { return r, nil } -// roomCount is how many rooms are live right now. Tests use it to assert that -// a room was evicted. +// roomCount is how many rooms are live right now: lobbies and running games +// alike. func (h *hub) roomCount() int { h.mu.Lock() defer h.mu.Unlock() @@ -324,20 +322,14 @@ func (h *hub) evict(code string) { delete(h.rooms, code) } -// reserveCode draws an unused room code. +// unusedCodeLocked draws a room code nobody holds. The caller holds h.mu. // // crypto/rand, not math/rand: a predictable code lets someone walk into a // stranger's private game, which is a guessing attack on a 6-character secret // rather than a fairness question. -func (h *hub) reserveCode() (string, error) { +func (h *hub) unusedCodeLocked() (string, error) { for range codeAttempts { - code := randomCode() - - h.mu.Lock() - _, taken := h.rooms[code] - h.mu.Unlock() - - if !taken { + if code := randomCode(); h.rooms[code] == nil { return code, nil } } @@ -391,13 +383,9 @@ func (h *hub) shutdown() { h.mu.Unlock() for _, s := range sessions { - s.send(errorMsg("server_restarting")) + s.send(errorMsg(codeServerRestarting)) } for _, r := range rooms { r.cancel() } } - -// playerIDFor is the seat a session holds. Declared here because the mapping -// between a connection and a seat is registry knowledge, not game knowledge. -func playerIDFor(seatName string) game.PlayerID { return game.PlayerID(seatName) } diff --git a/server/internal/wsapi/limits_test.go b/server/internal/wsapi/limits_test.go index 43cc498..48c219a 100644 --- a/server/internal/wsapi/limits_test.go +++ b/server/internal/wsapi/limits_test.go @@ -57,13 +57,13 @@ func TestRoomCapRefusesTheNextRoom(t *testing.T) { for range 2 { c := dial(t, url) c.hello("Chủ phòng") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() c.await("room_state") } third := dial(t, url) third.hello("Người thứ ba") - third.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + third.createRoom() if got := third.await("error").GetError().GetCode(); got != "server_full" { t.Errorf("error code = %q, want server_full", got) } @@ -204,7 +204,7 @@ func TestIdleRoomReleasesItsSeats(t *testing.T) { api, url := newTestServer(t, chainDict(), Config{IdleFor: 200 * time.Millisecond}) host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() host.await("room_state") if got := host.await("error").GetError().GetCode(); got != "room_idle_closed" { @@ -216,19 +216,8 @@ func TestIdleRoomReleasesItsSeats(t *testing.T) { } // The connection is free again: a second room opens and seats it. - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() if host.await("room_state").GetRoomState().GetRoomCode() == "" { t.Error("could not be seated in a new room after the idle close") } } - -// pvpGame seats two players, starts the game and sorts them into the one who -// drew the first turn and the one who waits. -func pvpGame(t *testing.T, url string) (lead, waits *testClient, start *noituv1.GameStarted) { - t.Helper() - host, guest, _ := pvpLobby(t, url) - guest.setReady(true) - host.await("room_state") - host.startGame() - return awaitLead(t, host, guest) -} diff --git a/server/internal/wsapi/lobby_test.go b/server/internal/wsapi/lobby_test.go index 4775f1b..2080028 100644 --- a/server/internal/wsapi/lobby_test.go +++ b/server/internal/wsapi/lobby_test.go @@ -17,9 +17,7 @@ func TestUnknownRoomCodeIsRefused(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{}) c := dial(t, url) c.hello("Khách") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: "ZZZZZZ"}, - }}) + c.joinRoom("ZZZZZZ") if code := c.await("error").GetError().GetCode(); code != "room_not_found" { t.Errorf("error code = %q, want room_not_found", code) @@ -85,14 +83,12 @@ func TestLobbyOpensWithNobodyReady(t *testing.T) { // frames the handshake produces, and the helper consumes them. host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) hostState := host.await("room_state").GetRoomState() guestState := guest.await("room_state").GetRoomState() @@ -156,7 +152,7 @@ func TestStartIsRefusedUntilTheGuestIsReady(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() // Alone in the room. @@ -167,9 +163,7 @@ func TestStartIsRefusedUntilTheGuestIsReady(t *testing.T) { guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") @@ -227,9 +221,7 @@ func TestNextGameNeedsAFreshReady(t *testing.T) { t.Errorf("starting a second game without a fresh ready returned %q", got) } - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) second := host.await("game_started").GetGameStarted() guest.await("game_started") @@ -265,9 +257,7 @@ func TestRoomKeepsARunningWinTally(t *testing.T) { } // A second game the other way round leaves the series level. - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) host.await("game_started") second := guest.await("game_started").GetGameStarted() @@ -357,9 +347,7 @@ func TestKickFreesAnUnreadySeatOnly(t *testing.T) { } // The seat is free, and a kick is not a ban. - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) if got := guest.await("room_state").GetRoomState(); otherSlot(got) == nil { t.Errorf("a kicked player could not come back: %+v", got) } @@ -385,14 +373,10 @@ func TestOwnerLeavingPromotesTheOtherPlayer(t *testing.T) { // person to walk in. third := dial(t, url) third.hello("Người mới") - third.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + third.joinRoom(code) guest.await("room_state") third.await("room_state") - third.setReady(true) - guest.await("room_state") - guest.startGame() + agreeAndStart(guest, third) guest.await("game_started") third.await("game_started") @@ -437,7 +421,7 @@ func TestIdleLobbyCloses(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() host.await("room_state") if got := host.await("error").GetError().GetCode(); got != "room_idle_closed" { @@ -482,7 +466,7 @@ func TestOneConnectionCannotStrandRooms(t *testing.T) { const rooms = 4 for range rooms { - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() c.await("room_state") } @@ -503,3 +487,55 @@ func TestOneConnectionCannotStrandRooms(t *testing.T) { time.Sleep(20 * time.Millisecond) } } + +// TestNoOtherRoomMidGame guards a player walking out of a running game by +// opening or joining another room: every room-opening message is refused with +// already_in_a_game, the player keeps their seat, and the same requests are +// allowed again once that game is over. +func TestNoOtherRoomMidGame(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}) + host, guest, start := pvpRoom(t, url) + + other := dial(t, url) + other.hello("Phòng bên") + other.createRoom() + otherCode := other.await("room_state").GetRoomState().GetRoomCode() + + attempts := map[string]func(){ + "create": host.createRoom, + "join": func() { host.joinRoom(otherCode) }, + "quickMatch": host.quickMatch, + "botGame": func() { + host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_StartBotGame{ + StartBotGame: &noituv1.StartBotGame{Difficulty: noituv1.Difficulty_DIFFICULTY_EASY}, + }}) + }, + } + for name, attempt := range attempts { + attempt() + if got := host.await("error").GetError().GetCode(); got != "already_in_a_game" { + t.Errorf("%s mid-game returned %q, want already_in_a_game", name, got) + } + } + + // Still seated: the game plays out to its end with the host in it. + resignAndSettle(t, host, guest, start) + + host.createRoom() + if code := host.await("room_state").GetRoomState().GetRoomCode(); code == "" || code == otherCode { + t.Errorf("create after the game returned room %q, want a fresh room", code) + } +} + +// TestResignInTheLobbyIsRefused guards a resignation sent before any game +// has started: it is answered with game_not_started, the way a dead-end claim +// there is, rather than dropped without a word. +func TestResignInTheLobbyIsRefused(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}) + host, _, _ := pvpLobby(t, url) + + host.resign() + if got := host.await("error").GetError().GetCode(); got != "game_not_started" { + t.Errorf("resigning in the lobby returned %q, want game_not_started", got) + } +} diff --git a/server/internal/wsapi/meanings_test.go b/server/internal/wsapi/meanings_test.go index f26d0bf..93140c3 100644 --- a/server/internal/wsapi/meanings_test.go +++ b/server/internal/wsapi/meanings_test.go @@ -71,14 +71,12 @@ func TestResumeReplaysMeanings(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) hostStart := host.await("game_started").GetGameStarted() guest.await("game_started") diff --git a/server/internal/wsapi/metrics.go b/server/internal/wsapi/metrics.go index 5bbe7f9..f50bdc6 100644 --- a/server/internal/wsapi/metrics.go +++ b/server/internal/wsapi/metrics.go @@ -3,8 +3,7 @@ package wsapi import "expvar" // metricSet is the process's answer to "how is the game actually being -// played" — the question plans/reports/brainstormer-260921-0016 names as -// unanswerable with nothing but a dozen scattered slog calls. It is exposed +// played", which a dozen scattered slog calls cannot answer. It is exposed // at GET /debug/vars, but only when the operator opts in by setting // NOITU_DEBUG_ADDR to a separate listen address: see cmd/noitu-server/main.go. // Nothing here is read anywhere in the game logic — a counter that fed a diff --git a/server/internal/wsapi/multiplayer_test.go b/server/internal/wsapi/multiplayer_test.go index 094a9df..3d4d4a0 100644 --- a/server/internal/wsapi/multiplayer_test.go +++ b/server/internal/wsapi/multiplayer_test.go @@ -22,16 +22,14 @@ func roomOf(t *testing.T, url string, n int) (clients []*testClient, state *noit names := []string{"Chủ phòng", "Khách", "Người thứ ba", "Người thứ tư", "Người thứ năm"} host := dial(t, url) host.hello(names[0]) - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() state = host.await("room_state").GetRoomState() clients = append(clients, host) for i := 1; i < n; i++ { c := dial(t, url) c.hello(names[i]) - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: state.GetRoomCode()}, - }}) + c.joinRoom(state.GetRoomCode()) clients = append(clients, c) // Everybody seated sees the arrival, which is also what keeps each // client's inbox drained before the next assertion reads from it. @@ -72,9 +70,7 @@ func TestARoomSeatsFourAndRefusesTheFifth(t *testing.T) { fifth := dial(t, url) fifth.hello("Người thứ năm") - fifth.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: state.GetRoomCode()}, - }}) + fifth.joinRoom(state.GetRoomCode()) if got := fifth.await("error").GetError().GetCode(); got != "room_full" { t.Errorf("the fifth joiner got %q, want room_full", got) } @@ -175,19 +171,8 @@ func TestKickNamesASeat(t *testing.T) { func TestAGameOutlivesItsFirstElimination(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{TurnLimit: 10 * time.Second}) clients, _ := roomOf(t, url, 3) - host, second, third := clients[0], clients[1], clients[2] - second.setReady(true) - third.setReady(true) - for _, c := range clients { - c.await("room_state") - } - - host.startGame() - starts := map[*testClient]*noituv1.GameStarted{} - for _, c := range clients { - starts[c] = c.await("game_started").GetGameStarted() - } + starts := startWith(t, clients) // The player on turn gives up. Two are left, so the game does not end. // Which player that is, is drawn at the start, so the test follows the @@ -258,19 +243,8 @@ func TestAGameOutlivesItsFirstElimination(t *testing.T) { func TestResigningOutOfTurnIsRefused(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{TurnLimit: 10 * time.Second}) clients, _ := roomOf(t, url, 3) - host, second, third := clients[0], clients[1], clients[2] - second.setReady(true) - third.setReady(true) - for _, c := range clients { - c.await("room_state") - } - - host.startGame() - starts := map[*testClient]*noituv1.GameStarted{} - for _, c := range clients { - starts[c] = c.await("game_started").GetGameStarted() - } + starts := startWith(t, clients) // The seat two along from the leader: not on turn, and not the one that // would inherit the turn either. @@ -299,19 +273,8 @@ func TestResigningOutOfTurnIsRefused(t *testing.T) { func TestLeavingMidGameFreesTheSeatAndLeavesTheRestPlaying(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{TurnLimit: 10 * time.Second}) clients, _ := roomOf(t, url, 3) - host, second, third := clients[0], clients[1], clients[2] - second.setReady(true) - third.setReady(true) - for _, c := range clients { - c.await("room_state") - } - - host.startGame() - starts := map[*testClient]*noituv1.GameStarted{} - for _, c := range clients { - starts[c] = c.await("game_started").GetGameStarted() - } + starts := startWith(t, clients) // The seat two along from the leader, as above: it is neither on turn nor // the one that inherits the turn, so nothing about the position depends on @@ -361,6 +324,31 @@ func TestLeavingMidGameFreesTheSeatAndLeavesTheRestPlaying(t *testing.T) { } } +// startWith readies every guest in a lobby roomOf seated, has the owner start, +// and returns each client's GameStarted. +// +// The guests declare one at a time, each declaration drained off every client +// before the next is sent. Readies sent together from separate connections +// reach the room in whatever order the scheduler picks, so an owner who +// started after seeing only the first room_state could overtake the last one +// and be refused with not_everyone_ready. +func startWith(t *testing.T, clients []*testClient) map[*testClient]*noituv1.GameStarted { + t.Helper() + for _, guest := range clients[1:] { + guest.setReady(true) + for _, c := range clients { + c.await("room_state") + } + } + + clients[0].startGame() + starts := make(map[*testClient]*noituv1.GameStarted, len(clients)) + for _, c := range clients { + starts[c] = c.await("game_started").GetGameStarted() + } + return starts +} + // onTurnClient is the client that drew the first turn. func onTurnClient(t *testing.T, clients []*testClient, starts map[*testClient]*noituv1.GameStarted) *testClient { t.Helper() @@ -393,18 +381,8 @@ func clientWithID(t *testing.T, clients []*testClient, starts map[*testClient]*n func TestStandingsReachEverySeat(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{TurnLimit: 10 * time.Second}) clients, _ := roomOf(t, url, 3) - host, second, third := clients[0], clients[1], clients[2] - second.setReady(true) - third.setReady(true) - for _, c := range clients { - c.await("room_state") - } - host.startGame() - starts := map[*testClient]*noituv1.GameStarted{} - for _, c := range clients { - starts[c] = c.await("game_started").GetGameStarted() - } + starts := startWith(t, clients) // Each player gives up on their own turn, which is the only way to. The // first to act goes, the seat behind them inherits the position and goes diff --git a/server/internal/wsapi/presence_test.go b/server/internal/wsapi/presence_test.go index 770b002..01fd483 100644 --- a/server/internal/wsapi/presence_test.go +++ b/server/internal/wsapi/presence_test.go @@ -135,14 +135,12 @@ func TestGraceExpiryAwardsTheGame(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) host.await("game_started") guest.await("game_started") diff --git a/server/internal/wsapi/protocol_test.go b/server/internal/wsapi/protocol_test.go index 13b29a3..59fa818 100644 --- a/server/internal/wsapi/protocol_test.go +++ b/server/internal/wsapi/protocol_test.go @@ -34,7 +34,7 @@ func TestProtocolVersionMismatchIsRefused(t *testing.T) { func TestHandshakeIsRequiredFirst(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{}) c := dial(t, url) - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() if code := c.await("error").GetError().GetCode(); code != "handshake_required" { t.Errorf("error code = %q, want handshake_required", code) diff --git a/server/internal/wsapi/quick_match_test.go b/server/internal/wsapi/quick_match_test.go index 4964e3d..0e576b4 100644 --- a/server/internal/wsapi/quick_match_test.go +++ b/server/internal/wsapi/quick_match_test.go @@ -108,7 +108,7 @@ func TestQuickMatchRefusedWhenAlreadySeated(t *testing.T) { c := dial(t, url) c.hello("Chủ phòng") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() c.await("room_state") c.quickMatch() @@ -141,7 +141,7 @@ func TestQuickMatchServerFullTellsBothSides(t *testing.T) { filler := dial(t, url) filler.hello("Chiếm chỗ") - filler.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + filler.createRoom() filler.await("room_state") waiter := dial(t, url) diff --git a/server/internal/wsapi/regression_test.go b/server/internal/wsapi/regression_test.go index 067c814..06fd6c7 100644 --- a/server/internal/wsapi/regression_test.go +++ b/server/internal/wsapi/regression_test.go @@ -53,27 +53,8 @@ func TestNicknamesCannotStackCombiningMarks(t *testing.T) { // them back in turn order: lead moves first, waits answers. func startPvP(t *testing.T, url string) (lead, waits *testClient, code string) { t.Helper() - host := dial(t, url) - host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) - code = host.await("room_state").GetRoomState().GetRoomCode() - - guest := dial(t, url) - guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) - host.await("room_state") - guest.await("room_state") - - // The lobby is where a game is agreed now: the guest readies and the owner - // starts it. - guest.setReady(true) - host.await("room_state") - host.startGame() - // Returned in turn order rather than as owner and joiner: the first turn - // is drawn, so a test that plays a move has to be handed the player who - // may play it. + host, guest, code := pvpLobby(t, url) + agreeAndStart(host, guest) lead, waits, _ = awaitLead(t, host, guest) return lead, waits, code } @@ -109,9 +90,7 @@ func TestStrangerCannotResignForASeatedPlayer(t *testing.T) { stranger := dial(t, url) stranger.hello("Kẻ lạ") - stranger.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + stranger.joinRoom(code) // Not room_full: a four-seat room with two people in it has seats going // spare. Arriving in the middle of a game is what is refused, and a // stranger holding the code is refused it like anybody else. @@ -132,9 +111,7 @@ func TestStrangerCannotSubmitForASeatedPlayer(t *testing.T) { stranger := dial(t, url) stranger.hello("Kẻ lạ") - stranger.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + stranger.joinRoom(code) stranger.await("error") stranger.submit("b c", 1) @@ -148,12 +125,10 @@ func TestCannotJoinYourOwnRoom(t *testing.T) { _, url := newTestServer(t, chainDict(), Config{}) host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + host.joinRoom(code) if got := host.await("error").GetError().GetCode(); got != "cannot_join_own_room" { t.Errorf("self-join returned %q", got) } @@ -198,7 +173,7 @@ func TestAbandonedRoomIsEvicted(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() host.await("room_state") _ = host.conn.Close(websocket.StatusGoingAway, "") @@ -221,7 +196,7 @@ func TestRoomCreationIsRateLimited(t *testing.T) { c.hello("Người thử") for range roomBurst + 3 { - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() } for range 40 { @@ -334,16 +309,14 @@ func TestOpponentNeverSeesAnUnsanitizedNickname(t *testing.T) { host := dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() hostile := " Kẻ" + string(nul) + " xấu" + string(zeroWidthSpace) + string(bidiOverride) + " " + strings.Repeat("z", 40) guest := dial(t, url) guest.hello(hostile) - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) shown := otherSlot(host.await("room_state").GetRoomState()).GetName() for _, r := range []rune{nul, zeroWidthSpace, bidiOverride} { @@ -404,14 +377,12 @@ func TestResumeAfterGameEndedLandsInTheLobby(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) host.await("game_started") guest.await("game_started") diff --git a/server/internal/wsapi/resume_test.go b/server/internal/wsapi/resume_test.go index 1f3abf2..462a965 100644 --- a/server/internal/wsapi/resume_test.go +++ b/server/internal/wsapi/resume_test.go @@ -18,14 +18,12 @@ func TestResumeWithinGraceRestoresGame(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) hostStart := host.await("game_started").GetGameStarted() guest.await("game_started") @@ -76,7 +74,7 @@ func TestUnknownResumeTokenIsAnsweredNotSilent(t *testing.T) { t.Errorf("error code = %q, want session_not_resumable", code) } - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() if code := c.await("room_state").GetRoomState().GetRoomCode(); code == "" { t.Error("a connection answered session_not_resumable must still be usable as a fresh session") } @@ -89,7 +87,7 @@ func TestFreshHelloIsNotToldItCannotResume(t *testing.T) { c := dial(t, url) c.hello("Người chơi") - c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + c.createRoom() if m := c.recv(); payloadCase(m) == "error" { t.Fatalf("a fresh Hello with no resume token got %q, want none", m.GetError().GetCode()) } @@ -103,14 +101,12 @@ func TestChatHistoryIsReplayedOnResumeInTheLobby(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") @@ -142,14 +138,12 @@ func TestChatHistoryIsReplayedOnResumeMidGame(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) readyAndStart(t, host, guest) host.await("game_started") guest.await("game_started") @@ -172,14 +166,12 @@ func TestResumeReclaimsALobbySeat(t *testing.T) { host := dial(t, url) welcome := host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code := host.await("room_state").GetRoomState().GetRoomCode() guest := dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") @@ -204,9 +196,7 @@ func TestResumeReclaimsALobbySeat(t *testing.T) { } // And the room still works from both sides. - guest.setReady(true) - second.await("room_state") - second.startGame() + agreeAndStart(second, guest) second.await("game_started") guest.await("game_started") } diff --git a/server/internal/wsapi/room.go b/server/internal/wsapi/room.go index 71fa0bc..325d66d 100644 --- a/server/internal/wsapi/room.go +++ b/server/internal/wsapi/room.go @@ -2,6 +2,7 @@ package wsapi import ( "context" + "iter" "log/slog" "sync/atomic" "time" @@ -13,11 +14,10 @@ import ( ) // This file holds the room's core type, its constructor, its input loop, -// and the small seat-authority helpers every other file in this package -// reads. Everything that only ever runs on the room goroutine still lives -// wherever the review's file split put it (room_lobby.go, room_game.go, -// room_presence.go, room_chat.go, bot_board.go) — this is a file boundary, -// not a change to who may touch a *room. +// and the small seat helpers every other file in this package reads. The +// handlers are split by topic (room_lobby.go, room_game.go, +// room_presence.go, room_chat.go) — a file boundary, not a change to who may +// touch a *room: all of it runs on the room goroutine. // roomModeBot and roomModePvP are the two values a room's mode ever takes. // They double as the label under which every mode-keyed metric and the @@ -42,6 +42,11 @@ const ( minPlayers = 2 ) +// seatIDs are the engine seat names, indexed by position. An id says which +// seat a player is in and nothing about their role: an owner who leaves hands +// that on, and the seat they vacate is refilled by an ordinary guest. +var seatIDs = [maxPlayers]game.PlayerID{"p1", "p2", "p3", "p4"} + // minOpeningOutDegree keeps the first word from being a dead end. Opening on a // syllable with two continuations makes for a game that ends before it starts. const minOpeningOutDegree = 20 @@ -136,8 +141,8 @@ type room struct { // liveCounted mirrors whether this room's game is the one hub.liveGames is // currently counting. Atomic rather than plain, because drain reads - // hub.liveGameCount() from outside the room goroutine while this flips on - // the goroutine itself; the CompareAndSwap in run's teardown is what + // hub.liveGameCount(), and a seated session asks whether it is mid-game, + // from outside the room goroutine while this flips on the goroutine itself; the CompareAndSwap in run's teardown is what // guarantees exactly one hub.gameFinished() per hub.gameStarted() even when // the room is cancelled mid-game instead of finishing normally. liveCounted atomic.Bool @@ -259,12 +264,8 @@ func (r *room) run() { defer metrics.roomsLive.Add(r.mode, -1) // Catches a room cancelled with a game still running — drain forcing the // last stragglers closed, or a shutdown mid-game — which never reaches - // broadcastGameOver's own decrement. - defer func() { - if r.liveCounted.CompareAndSwap(true, false) { - r.hub.gameFinished() - } - }() + // broadcastGameOver's own call. + defer r.stopCountingLive() // Whatever ended the room — everybody leaving, the idle window, a server // shutdown — the connections still seated in it must stop pointing here. // A session that keeps a dead room would answer every later action with @@ -405,7 +406,7 @@ func (r *room) run() { // A lobby nobody started a game in. Whoever is still sitting in it // is told why it closed rather than watching their buttons stop // working. - r.broadcastError("room_idle_closed") + r.broadcastError(codeRoomIdleClosed) return } @@ -451,14 +452,7 @@ func (r *room) inLobby() bool { return r.engine == nil || r.engine.Over() } // occupied reports whether anybody still holds a seat, including a player // inside their reconnect window. An empty room has nothing left to wait for. -func (r *room) occupied() bool { - for _, s := range r.seats { - if s != nil { - return true - } - } - return false -} +func (r *room) occupied() bool { return r.seatedCount() > 0 } // freeSeat returns the index a joiner would take, or -1 when the room is full. func (r *room) freeSeat() int { @@ -493,17 +487,22 @@ func (r *room) allConnected() bool { return true } -func (r *room) broadcastError(code string) { - for _, s := range r.seats { - if s != nil && s.sess != nil { - s.sess.send(errorMsg(code)) +// connected yields every seat with a socket behind it, in seat order: the +// recipients of anything the room says. A seat inside its reconnect window +// is skipped, and is caught up by the resume that ends it. +func (r *room) connected() iter.Seq[*seat] { + return func(yield func(*seat) bool) { + for _, s := range r.seats { + if s != nil && s.sess != nil && !yield(s) { + return + } } } } -func (r *room) sendTo(p game.PlayerID, msg *noituv1.ServerMessage) { - if s := r.seatOf(p); s != nil && s.sess != nil { - s.sess.send(msg) +func (r *room) broadcastError(code errCode) { + for s := range r.connected() { + s.sess.send(errorMsg(code)) } } @@ -515,3 +514,37 @@ func (r *room) seatOf(p game.PlayerID) *seat { } return nil } + +// takeSeat puts sess in seat index i and binds the connection to it. +// +// Every way into a room goes through here — creating one, joining one, +// starting a bot game — so each is also where the connection stops waiting +// for a quick match: a plain CreateRoom might be seating somebody who was +// queued from another attempt, and one dequeue serves every entry path. +// +// The caller checks the connection is still alive afterwards. It can die +// between the hub handing this room the seating message and the room +// goroutine draining it, and nothing else would ever tell the room so. +func (r *room) takeSeat(i int, nickname string, sess *session) *seat { + s := &seat{ + id: seatIDs[i], + nickname: nickname, + sess: sess, + // Seated now, so the conversation up to this point is not theirs to + // read. A room code is pasted into group chats by design. + chatFrom: r.chatSeq, + } + r.seats[i] = s + sess.attach(r, s.id) + r.hub.cancelQuickMatch(sess) + return s +} + +// stopCountingLive ends this room's share of hub.liveGames, once. Both a game +// finishing and the room exiting mid-game reach here, and the CompareAndSwap +// is what keeps the pair from counting one game's end twice. +func (r *room) stopCountingLive() { + if r.liveCounted.CompareAndSwap(true, false) { + r.hub.gameFinished() + } +} diff --git a/server/internal/wsapi/room_chat.go b/server/internal/wsapi/room_chat.go index c698c06..711e9c7 100644 --- a/server/internal/wsapi/room_chat.go +++ b/server/internal/wsapi/room_chat.go @@ -31,13 +31,13 @@ func (r *room) handleChat(m chatInput) { // - kicked, or replaced by a reconnect - can still have a frame in flight, // and by the time the room drains it that seat may belong to somebody else. if !r.occupies(m.sess, m.player) { - m.sess.send(errorMsg("not_your_seat")) + m.sess.send(errorMsg(codeNotYourSeat)) return } // A bot room has no conversation. Checked here rather than in the session, // because r.strategy is room-goroutine state. if r.strategy != nil { - m.sess.send(errorMsg("not_in_a_room")) + m.sess.send(errorMsg(codeNotInARoom)) return } @@ -64,10 +64,7 @@ func (r *room) handleChat(m chatInput) { } metrics.chatLines.Add(1) - for _, s := range r.seats { - if s == nil || s.sess == nil { - continue - } + for s := range r.connected() { // Best effort: a chat frame is dropped rather than allowed to close a // session whose outbox is full. Losing a line is recoverable - the // next replay carries it - and closing a session costs its owner the diff --git a/server/internal/wsapi/room_game.go b/server/internal/wsapi/room_game.go index 8fa2732..49dfff3 100644 --- a/server/internal/wsapi/room_game.go +++ b/server/internal/wsapi/room_game.go @@ -27,14 +27,21 @@ import ( // leaves the room instead, which handleLobby answers. func (r *room) handleResign(m resignInput) { if !r.occupies(m.sess, m.player) { - m.sess.send(errorMsg("not_your_seat")) + m.sess.send(errorMsg(codeNotYourSeat)) return } - if r.engine == nil || r.engine.Over() { + // A lobby answers, as a dead-end claim does there, because the player is + // waiting to see a resignation land. A game that has just ended does not: + // its game_over is already on the way and says everything a reply would. + if r.engine == nil { + m.sess.send(errorMsg(codeGameNotStarted)) + return + } + if r.engine.Over() { return } if r.engine.Turn() != m.player { - m.sess.send(errorMsg("not_your_turn")) + m.sess.send(errorMsg(codeNotYourTurn)) return } before := r.mark() @@ -53,23 +60,23 @@ func (r *room) handleResign(m resignInput) { // enough to be the whole cost of asking wrongly. func (r *room) handleClaimDeadEnd(m claimDeadEndInput) { if !r.occupies(m.sess, m.player) { - m.sess.send(errorMsg("not_your_seat")) + m.sess.send(errorMsg(codeNotYourSeat)) return } if r.engine == nil { - m.sess.send(errorMsg("game_not_started")) + m.sess.send(errorMsg(codeGameNotStarted)) return } if r.engine.Over() { return } if r.engine.Turn() != m.player { - m.sess.send(errorMsg("not_your_turn")) + m.sess.send(errorMsg(codeNotYourTurn)) return } if r.engine.HasLegalMove() { metrics.deadEndClaims.Add("false", 1) - m.sess.send(errorMsg("not_a_dead_end")) + m.sess.send(errorMsg(codeNotADeadEnd)) return } @@ -84,18 +91,15 @@ func (r *room) handleClaimDeadEnd(m claimDeadEndInput) { func (r *room) handleStartBot(m startBotInput) { strategy, err := bot.New(m.difficulty, rand.New(rand.NewPCG(rand.Uint64(), rand.Uint64()))) if err != nil { - m.sess.send(errorMsg("room_start_failed")) + m.sess.send(errorMsg(codeRoomStartFailed)) r.cancel() return } r.strategy = strategy - s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} - r.seats[0] = s + s := r.takeSeat(0, m.sess.nickname(), m.sess) r.seats[1] = &seat{id: botPlayerID, nickname: "Máy"} - r.owner = "p1" - m.sess.attach(r, "p1") - r.hub.cancelQuickMatch(m.sess) + r.owner = s.id if s.sess.ctx.Err() != nil { // A bot room has no lobby to fall back to and no idle timer covering it @@ -103,14 +107,14 @@ func (r *room) handleStartBot(m startBotInput) { // strategy) — a grace window here would leave the bot's own seat // holding the room open forever with nothing left to vacate it. The // room ends now instead, the same way a failed bot.New or beginGame - // above already does. + // already does. r.cancel() return } if err := r.beginGame(); err != nil { slog.Error("could not start bot game", "room", r.code, "err", err) - m.sess.send(errorMsg("game_start_failed")) + m.sess.send(errorMsg(codeGameStartFailed)) r.cancel() } } @@ -170,19 +174,17 @@ func (r *room) beginGame() error { r.liveCounted.Store(true) state := r.engine.Snapshot() - for _, s := range r.seats { + for s := range r.connected() { r.sendGameStarted(s, state) } r.maybeScheduleBot() return nil } -// sendGameStarted renders the opening position for one seat. my_turn and is_me -// are per-recipient, which is why this is built per seat rather than broadcast. +// sendGameStarted renders the opening position for one connected seat. +// my_turn and is_me are per-recipient, which is why this is built per seat +// rather than broadcast. func (r *room) sendGameStarted(s *seat, state game.State) { - if s == nil || s.sess == nil { - return - } s.sess.send(&noituv1.ServerMessage{Payload: &noituv1.ServerMessage_GameStarted{ GameStarted: &noituv1.GameStarted{ OpeningWord: r.opening, @@ -201,13 +203,14 @@ func (r *room) sendGameStarted(s *seat, state game.State) { // handleSubmit runs one human move through the engine. func (r *room) handleSubmit(m submitInput) { if !r.occupies(m.sess, m.player) { - m.sess.send(errorMsg("not_your_seat")) + m.sess.send(errorMsg(codeNotYourSeat)) return } if r.engine == nil { - r.sendTo(m.player, errorMsg("game_not_started")) + m.sess.send(errorMsg(codeGameNotStarted)) return } + metrics.wordsSubmitted.Add(1) // A submission stamped with an old turn is answering a position that no // longer exists — a double-submit, or a word typed as the clock ran out. @@ -216,11 +219,9 @@ func (r *room) handleSubmit(m submitInput) { // The rejection carries the server's sequence, not the client's stale one, // so the client can resynchronise from the refusal instead of having to // wait for the next turn update to discover where the game actually is. - metrics.wordsSubmitted.Add(1) - if m.turnSeq != r.turnSeq { - r.sendTo(m.player, moveRejectedMsg(noituv1.RejectReason_REJECT_REASON_NOT_YOUR_TURN, m.word, r.turnSeq, "")) r.recordRejection(game.ReasonNotYourTurn, m.word) + m.sess.send(moveRejectedMsg(noituv1.RejectReason_REJECT_REASON_NOT_YOUR_TURN, m.word, r.turnSeq, "")) return } @@ -233,8 +234,10 @@ func (r *room) handleSubmit(m submitInput) { before := r.mark() move, reason := r.engine.Submit(m.player, word, time.Now()) if reason != game.ReasonNone { - r.sendTo(m.player, moveRejectedMsg(RejectReason(reason), word, m.turnSeq, r.nearMissFor(reason, word))) + // Counted before the player is told, as every metric here is: a + // reader who sees the refusal must also see it counted. r.recordRejection(reason, word) + m.sess.send(moveRejectedMsg(RejectReason(reason), word, m.turnSeq, r.nearMissFor(reason, word))) // A rejection for an expired turn also took this player out of the // game, and everybody has to be told which. r.applyEliminations(before) @@ -276,8 +279,8 @@ func (r *room) nearMissFor(reason game.RejectReason, raw string) string { // recordRejection counts one rejected submission and logs it at Info. // -// This is the corpus feedback loop the improvement report calls the input to -// every decision about the dictionary: which words players actually type that +// This is the corpus feedback loop that feeds every decision about the +// dictionary: which words players actually type that // the game does not accept, and why. The word logged is never the raw typed // text — it is normalized the same way the engine would have matched it // (NFC, lowercase, single-spaced) and capped, so the line is useful for corpus @@ -405,7 +408,7 @@ func (r *room) maybeScheduleBot() { func (r *room) broadcastTurn(move *game.Move) { state := r.engine.Snapshot() meanings := r.moveMeanings(move) - for _, s := range r.seats { + for s := range r.connected() { r.sendTurnUpdate(s, state, move, meanings) } } @@ -419,13 +422,10 @@ func (r *room) moveMeanings(move *game.Move) []dictionary.Sense { return r.dict.Meanings(move.Word) } -// sendTurnUpdate renders one position for one seat. by_me, my_turn and is_me -// are all per-recipient, which is why there is no single shared frame; the -// move's meanings are not, and arrive looked up. +// sendTurnUpdate renders one position for one connected seat. by_me, my_turn +// and is_me are all per-recipient, which is why there is no single shared +// frame; the move's meanings are not, and arrive looked up. func (r *room) sendTurnUpdate(s *seat, state game.State, move *game.Move, meanings []dictionary.Sense) { - if s == nil || s.sess == nil { - return - } update := &noituv1.TurnUpdate{ CurrentSyllable: state.Current, MyTurn: state.Turn == s.id, @@ -514,10 +514,7 @@ func (r *room) broadcastElimination(id game.PlayerID, suggestions []string) { reason := r.wireEndReason(id) metrics.eliminations.Add(reason.String(), 1) - for _, s := range r.seats { - if s == nil || s.sess == nil { - continue - } + for s := range r.connected() { msg := &noituv1.PlayerEliminated{ PlayerId: string(id), Name: name, @@ -549,9 +546,7 @@ func (r *room) wireEndReason(p game.PlayerID) noituv1.GameEndReason { // broadcastGameOver reports the result from each seat's point of view. func (r *room) broadcastGameOver(state game.State) { metrics.gamesFinished.Add(r.mode, 1) - if r.liveCounted.CompareAndSwap(true, false) { - r.hub.gameFinished() - } + r.stopCountingLive() // The reason the game ended is the reason the last player went out, which // with two seats is the only elimination there was. @@ -573,10 +568,7 @@ func (r *room) broadcastGameOver(state game.State) { order = append(order, standing.Player) } - for _, s := range r.seats { - if s == nil || s.sess == nil { - continue - } + for s := range r.connected() { s.sess.send(&noituv1.ServerMessage{Payload: &noituv1.ServerMessage_GameOver{ GameOver: &noituv1.GameOver{ IWon: state.Winner == s.id, diff --git a/server/internal/wsapi/room_lobby.go b/server/internal/wsapi/room_lobby.go index ae6b988..bf3c6b9 100644 --- a/server/internal/wsapi/room_lobby.go +++ b/server/internal/wsapi/room_lobby.go @@ -16,20 +16,13 @@ import ( // The code goes out in the RoomState the run loop broadcasts, so a client can // never be handed a code before the seat behind it exists. func (r *room) handleCreate(m createInput) { - s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} - r.seats[0] = s - r.owner = "p1" + s := r.takeSeat(0, m.sess.nickname(), m.sess) + r.owner = s.id r.autoStart = m.autoStart - m.sess.attach(r, "p1") r.lobbyChanged = true - // A quick match already popped this session off the pairing queue before - // sending it here, but a plain CreateRoom might still be seating somebody - // who was also waiting in it from another attempt — one dequeue serves - // both room-entry paths. - r.hub.cancelQuickMatch(m.sess) if s.sess.ctx.Err() != nil { - r.disconnectGhostSeat(s) + r.holdSeat(s) return } // Deliberately sent to a brand-new room's creator, where it is always @@ -49,7 +42,7 @@ func (r *room) handleJoin(m joinInput) { free := r.freeSeat() if free < 0 || !r.occupied() { metrics.joinsRefused.Add("room_full", 1) - m.sess.send(errorMsg("room_full")) + m.sess.send(errorMsg(codeRoomFull)) return } // A room can have a free seat and still be mid-game — four people can @@ -57,32 +50,21 @@ func (r *room) handleJoin(m joinInput) { // something to seat somebody for: they would have no words, no score, and // no way to be told what they had missed. if !r.inLobby() { - m.sess.send(errorMsg("game_in_progress")) + m.sess.send(errorMsg(codeGameInProgress)) return } - for _, s := range r.seats { - if s != nil && s.sess == m.sess { - m.sess.send(errorMsg("cannot_join_own_room")) + for s := range r.connected() { + if s.sess == m.sess { + m.sess.send(errorMsg(codeCannotJoinOwnRoom)) return } } - id := seatIDs[free] - s := &seat{ - id: id, - nickname: distinguish(m.sess.nickname(), r.takenNicknames(id)), - sess: m.sess, - // Seated now, so the conversation up to this point is not theirs to - // read. A room code is pasted into group chats by design. - chatFrom: r.chatSeq, - } - r.seats[free] = s - m.sess.attach(r, string(id)) + s := r.takeSeat(free, distinguish(m.sess.nickname(), r.takenNicknames()), m.sess) r.lobbyChanged = true - r.hub.cancelQuickMatch(m.sess) if s.sess.ctx.Err() != nil { - r.disconnectGhostSeat(s) + r.holdSeat(s) return } r.sendChatHistory(s) @@ -99,7 +81,7 @@ func (r *room) handleJoin(m joinInput) { // owner here to answer with server_restarting the way lobbyStart // does, so both seats are told directly; the lobby view they are // left in still shows each other, via lobbyChanged below. - r.broadcastError("server_restarting") + r.broadcastError(codeServerRestarting) return } r.autoStart = false @@ -107,7 +89,7 @@ func (r *room) handleJoin(m joinInput) { r.broadcastRoomState() if err := r.beginGame(); err != nil { slog.Error("could not start quick-matched game", "room", r.code, "err", err) - r.broadcastError("game_start_failed") + r.broadcastError(codeGameStartFailed) } } } @@ -119,19 +101,19 @@ func (r *room) handleJoin(m joinInput) { // see the state that refused them. func (r *room) handleLobby(m lobbyInput) { if !r.occupies(m.sess, m.player) { - m.sess.send(errorMsg("not_your_seat")) + m.sess.send(errorMsg(codeNotYourSeat)) return } if r.strategy != nil { // A bot room has no lobby: one player, no readiness, nobody to kick. - m.sess.send(errorMsg("not_in_a_room")) + m.sess.send(errorMsg(codeNotInARoom)) return } // Leaving is the exception: a player may want out of a game it is not // their turn in, and resigning is not open to them then. Readying, // starting and kicking all belong to a room between games. if !r.inLobby() && m.action != lobbyLeave { - m.sess.send(errorMsg("game_in_progress")) + m.sess.send(errorMsg(codeGameInProgress)) return } @@ -143,7 +125,7 @@ func (r *room) handleLobby(m lobbyInput) { if isOwner { // The owner's readiness is StartGame. A flag of their own would // only be something they had to set before every single start. - m.sess.send(errorMsg("owner_needs_no_ready")) + m.sess.send(errorMsg(codeOwnerNeedsNoReady)) return } mine.ready = m.ready @@ -151,7 +133,7 @@ func (r *room) handleLobby(m lobbyInput) { case lobbyStart: if !isOwner { - m.sess.send(errorMsg("not_the_owner")) + m.sess.send(errorMsg(codeNotTheOwner)) return } switch { @@ -160,50 +142,55 @@ func (r *room) handleLobby(m lobbyInput) { // starts; this lobby existed before that point, and starting its // game now would raise hub.liveGames after the drain decided how // long to wait for exactly that number to reach zero. - m.sess.send(errorMsg("server_restarting")) + m.sess.send(errorMsg(codeServerRestarting)) return case r.seatedCount() < minPlayers: - m.sess.send(errorMsg("need_more_players")) + m.sess.send(errorMsg(codeNeedMorePlayers)) return case !r.allConnected(): - m.sess.send(errorMsg("player_offline")) + m.sess.send(errorMsg(codePlayerOffline)) return case !r.guestsReady(): - m.sess.send(errorMsg("not_everyone_ready")) + m.sess.send(errorMsg(codeNotEveryoneReady)) return } if err := r.beginGame(); err != nil { slog.Error("could not start pvp game", "room", r.code, "err", err) - r.broadcastError("game_start_failed") + r.broadcastError(codeGameStartFailed) } case lobbyKick: if !isOwner { - m.sess.send(errorMsg("not_the_owner")) + m.sess.send(errorMsg(codeNotTheOwner)) return } target := r.seatOf(m.target) switch { case target == nil: - m.sess.send(errorMsg("no_one_to_kick")) + m.sess.send(errorMsg(codeNoOneToKick)) return case target == mine: // Leaving is what an owner who wants out does, and it hands the // room on. Kicking yourself would drop the seat and the role // together while the others were still sitting here. - m.sess.send(errorMsg("cannot_kick_self")) + m.sess.send(errorMsg(codeCannotKickSelf)) return case target.ready: // Readiness is a commitment, and the owner does not get to // overrule one: a player who is ready is waiting on the owner, // not in the way. - m.sess.send(errorMsg("player_is_ready")) + m.sess.send(errorMsg(codePlayerIsReady)) return } - if target.sess != nil { - target.sess.send(errorMsg("kicked")) - } + // Released before being told, not after. The notice is what prompts + // the client to act on being out, and an action that overtook the + // release would still find the connection bound here and be refused + // as somebody else's seat rather than as no room at all. + kicked := target.sess r.vacate(target) + if kicked != nil { + kicked.send(errorMsg(codeKicked)) + } r.lobbyChanged = true case lobbyLeave: @@ -211,7 +198,7 @@ func (r *room) handleLobby(m lobbyInput) { // Unreadying first is deliberate friction: a player the other one // is waiting on should have to take that back before walking away. if mine.ready { - m.sess.send(errorMsg("must_unready_first")) + m.sess.send(errorMsg(codeMustUnreadyFirst)) return } } else { @@ -239,12 +226,12 @@ func (r *room) guestsReady() bool { return true } -// takenNicknames is every name already in this room except one seat's own, so -// a joiner can be told apart from all of them. -func (r *room) takenNicknames(except game.PlayerID) []string { +// takenNicknames is every name already in this room, so a joiner can be told +// apart from all of them. +func (r *room) takenNicknames() []string { names := make([]string, 0, maxPlayers) for _, s := range r.seats { - if s != nil && s.id != except { + if s != nil { names = append(names, s.nickname) } } @@ -330,10 +317,7 @@ func (r *room) promote() { func (r *room) broadcastRoomState() { canStart := r.canStart() - for _, s := range r.seats { - if s == nil || s.sess == nil { - continue - } + for s := range r.connected() { s.sess.send(&noituv1.ServerMessage{Payload: &noituv1.ServerMessage_RoomState{ RoomState: &noituv1.RoomState{ RoomCode: r.code, @@ -368,8 +352,3 @@ func (r *room) playerSlots(me game.PlayerID) []*noituv1.PlayerSlot { } return slots } - -// seatIDs are the engine seat names, indexed by position. An id says which -// seat a player is in and nothing about their role: an owner who leaves hands -// that on, and the seat they vacate is refilled by an ordinary guest. -var seatIDs = [maxPlayers]game.PlayerID{"p1", "p2", "p3", "p4"} diff --git a/server/internal/wsapi/room_presence.go b/server/internal/wsapi/room_presence.go index 4a44176..60f7294 100644 --- a/server/internal/wsapi/room_presence.go +++ b/server/internal/wsapi/room_presence.go @@ -9,28 +9,29 @@ import ( // Presence: a seat's reconnect window, opening it, closing it, and what a // resume does once a connection comes back inside one. -// disconnectGhostSeat opens the seat's reconnect window the moment it is -// filled, for a connection that turns out to have already torn down. +// holdSeat opens a seat's reconnect window: the connection behind it is gone, +// and the seat waits graceFor for it to come back. // -// The session can die between the hub handing this room the seating message -// and the room goroutine draining it off the queue — nothing else ever learns -// that, because leaveRoom only notifies a room the session was already -// attached to, and attaching is exactly what has not happened yet. Left -// seated as if connected, allConnected() would report true and quick match's -// own auto-start (see handleJoin) could begin a game against a socket nobody -// is behind. Applying the same grace window handleDisconnect would reuses the -// one mechanism that already bounds this instead of adding a second one. -func (r *room) disconnectGhostSeat(s *seat) { +// A dropped connection is not a player leaving. The seat is kept whether a +// game is running or the room is sitting in its lobby, so refreshing the page +// does not cost somebody the room they are in. +// +// It is also what a seat gets the moment it is filled by a connection that +// turns out to have already torn down. The session can die between the hub +// handing this room the seating message and the room goroutine draining it, +// and nothing else ever learns that: leaveRoom only notifies a room the +// session was already attached to. Left seated as if connected, +// allConnected() would report true and quick match's own auto-start could +// begin a game against a socket nobody is behind. +func (r *room) holdSeat(s *seat) { s.sess = nil s.graceUntil = time.Now().Add(r.graceFor) + // Presence is part of the room's state, and the run loop is what sends it. + r.lobbyChanged = true } // handleDisconnect holds the seat open for the player who dropped out of it. // -// A dropped connection is not a player leaving. The seat is kept for the -// reconnect window whether a game is running or the room is sitting in its -// lobby, so refreshing the page does not cost somebody the room they are in. -// // The turn clock is deliberately not paused. A player who drops on their own // turn loses it the way anybody else would; the window decides only whether // they are still in the game afterwards. @@ -41,11 +42,7 @@ func (r *room) handleDisconnect(m disconnectInput) { if s == nil || s.sess == nil || s.sess != m.sess { return } - s.sess = nil - s.graceUntil = time.Now().Add(r.graceFor) - // Presence is part of the room's state, and the run loop is what sends it. - // There is nothing extra to say to the players who are still here. - r.lobbyChanged = true + r.holdSeat(s) } // nextGraceExpiry is the earliest reconnect window still open. @@ -115,8 +112,12 @@ func (r *room) eliminateAbsent(s *seat, now time.Time) { // client would then be shown a board the server does not believe in. func (r *room) handleResume(m resumeInput) { s := r.seatOf(m.player) - if s == nil { - m.sess.send(errorMsg("session_not_resumable")) + // The token is single-use, but two connections presenting it at once can + // both find it registered before either resume lands. Only the first may + // take the seat: the second would displace it and leave that connection + // attached to a seat that is no longer its own. + if s == nil || (s.sess != nil && s.sess != m.prior) { + m.sess.send(errorMsg(codeSessionNotResumable)) return } @@ -124,9 +125,9 @@ func (r *room) handleResume(m resumeInput) { // its socket is either gone or about to be, and leaving it registered would // let a third connection claim the same seat. metrics.resumesSucceeded.Add(1) - m.sess.attach(r, string(m.player)) + m.sess.attach(r, m.player) if m.prior != nil { - m.sess.hub.unregister(m.prior.resumeToken) + r.hub.unregister(m.prior.resumeToken) m.prior.close() } s.sess = m.sess @@ -146,7 +147,7 @@ func (r *room) handleResume(m resumeInput) { // handleCreate and handleJoin guard against. Reopening the window it // just closed leaves the seat exactly as reachable as it was before // this resume was ever attempted. - r.disconnectGhostSeat(s) + r.holdSeat(s) return } @@ -169,9 +170,7 @@ func (r *room) handleResume(m resumeInput) { // detachAll releases every connection still bound to this room as it exits. func (r *room) detachAll() { - for _, s := range r.seats { - if s != nil && s.sess != nil { - s.sess.release(r) - } + for s := range r.connected() { + s.sess.release(r) } } diff --git a/server/internal/wsapi/session.go b/server/internal/wsapi/session.go index 5415a0c..8d14ad1 100644 --- a/server/internal/wsapi/session.go +++ b/server/internal/wsapi/session.go @@ -148,9 +148,8 @@ type session struct { // greeted marks the handshake done. It is a one-shot transition: a second // Hello would re-register the session and rewrite its nickname mid-game. + // Touched only from dispatch, like reportedWords, so it needs no lock. greeted bool - - closeOnce sync.Once } func newSession(ctx context.Context, conn *websocket.Conn, h *hub, remoteIP string) *session { @@ -189,11 +188,11 @@ func (s *session) setNickname(n string) { } // attach binds this connection to a room seat. -func (s *session) attach(r *room, seatName string) { +func (s *session) attach(r *room, id game.PlayerID) { s.mu.Lock() previous, previousID := s.room, s.playerID s.room = r - s.playerID = playerIDFor(seatName) + s.playerID = id s.mu.Unlock() // Releasing the old room is not tidiness. Nothing else tells it this @@ -232,30 +231,12 @@ func (s *session) currentRoom() (*room, game.PlayerID) { // must not be able to stall the game its opponent is still playing. A full // outbox closes the session instead. func (s *session) send(m *noituv1.ServerMessage) { - raw, err := Encode(m) - if err != nil { - slog.Error("encode failed", "session", s.id, "err", err) - return - } - - select { - case s.out <- raw: - case <-s.ctx.Done(): - default: + if s.enqueue(m) == errOutboxFull { slog.Warn("outbox full, closing session", "session", s.id) s.close() } } -// close signals teardown. It does not cancel the read context: that is done by -// run once the writer has flushed, so a client is told why it is being -// disconnected before the socket goes. -func (s *session) close() { - s.closeOnce.Do(func() { - s.cancel() - }) -} - // trySend queues a message and reports whether it fit. // // The difference from send is what a full outbox means: send closes the @@ -267,23 +248,44 @@ func (s *session) close() { // not — it is the frame that corrects a whole panel, and there is nothing // behind it — so that one still goes through send. This is for ChatMessage. func (s *session) trySend(m *noituv1.ServerMessage) bool { + err := s.enqueue(m) + if err == errOutboxFull { + slog.Warn("outbox full, dropping chat", "session", s.id) + } + return err == nil +} + +// errOutboxFull and errSessionClosed are why enqueue did not queue a frame. +// Only the first is the caller's to act on: a closed session is already on +// its way out, and a frame for it is simply not needed any more. +var ( + errOutboxFull = errors.New("wsapi: outbox full") + errSessionClosed = errors.New("wsapi: session closed") +) + +// enqueue encodes m and offers it to the outbox without blocking. +func (s *session) enqueue(m *noituv1.ServerMessage) error { raw, err := Encode(m) if err != nil { slog.Error("encode failed", "session", s.id, "err", err) - return false + return err } select { case s.out <- raw: - return true + return nil case <-s.ctx.Done(): - return false + return errSessionClosed default: - slog.Warn("outbox full, dropping chat", "session", s.id) - return false + return errOutboxFull } } +// close signals teardown. It does not cancel the read context: that is done by +// run once the writer has flushed, so a client is told why it is being +// disconnected before the socket goes. +func (s *session) close() { s.cancel() } + // run drives the connection until it closes. func (s *session) run() { defer s.close() @@ -336,13 +338,13 @@ func (s *session) readLoop() error { return err } if !s.frameLimiter.allow(time.Now()) { - s.send(errorMsg("too_fast")) + s.send(errorMsg(codeTooFast)) return errFlood } msg, err := Decode(typ, raw) if err != nil { - s.send(errorMsg("bad_frame")) + s.send(errorMsg(codeBadFrame)) return err } if err := s.dispatch(msg); err != nil { diff --git a/server/internal/wsapi/wsapi_test.go b/server/internal/wsapi/wsapi_test.go index 18e9049..58fe6e6 100644 --- a/server/internal/wsapi/wsapi_test.go +++ b/server/internal/wsapi/wsapi_test.go @@ -203,37 +203,62 @@ func (c *testClient) send(m *noituv1.ClientMessage) { func (c *testClient) recv() *noituv1.ServerMessage { c.t.Helper() + m, err := c.read() + if err != nil { + c.t.Fatalf("read: %v", err) + } + return m +} + +// read is recv without the Fatal, so await can report what it did see before +// giving up. +func (c *testClient) read() (*noituv1.ServerMessage, error) { ctx, cancel := context.WithTimeout(c.ctx, 5*time.Second) defer cancel() typ, raw, err := c.conn.Read(ctx) if err != nil { - c.t.Fatalf("read: %v", err) + return nil, err } if typ != websocket.MessageBinary { - c.t.Fatalf("expected a binary frame, got %v", typ) + return nil, fmt.Errorf("expected a binary frame, got %v", typ) } var m noituv1.ServerMessage if err := proto.Unmarshal(raw, &m); err != nil { - c.t.Fatalf("unmarshal: %v", err) + return nil, fmt.Errorf("unmarshal: %w", err) } - return &m + return &m, nil } // await reads until a message of the wanted case arrives, so a test states the -// event it cares about rather than every frame that precedes it. +// event it cares about rather than every frame that precedes it. A failure +// names every frame it skipped, since the one that arrived instead is almost +// always the explanation. func (c *testClient) await(want string) *noituv1.ServerMessage { c.t.Helper() + var seen []string for range 20 { - m := c.recv() + m, err := c.read() + if err != nil { + c.t.Fatalf("awaiting %q: read: %v (skipped %v)", want, err, seen) + } if payloadCase(m) == want { return m } + seen = append(seen, describe(m)) } - c.t.Fatalf("never received a %q message", want) + c.t.Fatalf("never received a %q message (skipped %v)", want, seen) return nil } +// describe names a skipped frame, with the error code when it is one. +func describe(m *noituv1.ServerMessage) string { + if e := m.GetError(); e != nil { + return "error:" + e.GetCode() + } + return payloadCase(m) +} + // mySlot is the recipient's own row in a RoomState, which is the only place // their role and their readiness live now. func mySlot(state *noituv1.RoomState) *noituv1.PlayerSlot { @@ -373,15 +398,31 @@ func pvpRoom(t *testing.T, url string) (host, guest *testClient, start *noituv1. t.Helper() host, guest, _ = pvpLobby(t, url) - guest.setReady(true) - host.await("room_state") - host.startGame() - + agreeAndStart(host, guest) start = host.await("game_started").GetGameStarted() guest.await("game_started") return host, guest, start } +// pvpGame seats two players, starts the game and sorts them into the one who +// drew the first turn and the one who waits. +func pvpGame(t *testing.T, url string) (lead, waits *testClient, start *noituv1.GameStarted) { + t.Helper() + host, guest, _ := pvpLobby(t, url) + agreeAndStart(host, guest) + return awaitLead(t, host, guest) +} + +// agreeAndStart has the guest of a freshly seated pair declare ready and the +// owner start once the room has said so. The owner waits on that room_state +// because a StartGame that overtook the readiness would be refused. +func agreeAndStart(host, guest *testClient) { + host.t.Helper() + guest.setReady(true) + host.await("room_state") + host.startGame() +} + // pvpLobby seats two players and stops there: the room exists, nobody is ready // and no game has been started. func pvpLobby(t *testing.T, url string) (host, guest *testClient, code string) { @@ -389,14 +430,12 @@ func pvpLobby(t *testing.T, url string) (host, guest *testClient, code string) { host = dial(t, url) host.hello("Chủ phòng") - host.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) + host.createRoom() code = host.await("room_state").GetRoomState().GetRoomCode() guest = dial(t, url) guest.hello("Khách") - guest.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ - JoinRoom: &noituv1.JoinRoom{RoomCode: code}, - }}) + guest.joinRoom(code) host.await("room_state") guest.await("room_state") return host, guest, code @@ -408,9 +447,7 @@ func readyAndStart(t *testing.T, host, guest *testClient) { t.Helper() host.await("room_state") guest.await("room_state") - guest.setReady(true) - host.await("room_state") - host.startGame() + agreeAndStart(host, guest) } // awaitLead reads both game_started messages and sorts the pair into the one @@ -432,6 +469,18 @@ func awaitLead(t *testing.T, host, guest *testClient) (lead, waits *testClient, return guest, host, guestStart } +func (c *testClient) createRoom() { + c.t.Helper() + c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}}) +} + +func (c *testClient) joinRoom(code string) { + c.t.Helper() + c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_JoinRoom{ + JoinRoom: &noituv1.JoinRoom{RoomCode: code}, + }}) +} + func (c *testClient) setReady(ready bool) { c.t.Helper() c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_SetReady{ @@ -594,11 +643,7 @@ func awaitNoRooms(t *testing.T, api *Server, what string) { deadline := time.Now().Add(5 * time.Second) for { - api.hub.mu.Lock() - left := len(api.hub.rooms) - api.hub.mu.Unlock() - - if left == 0 { + if api.hub.roomCount() == 0 { return } if time.Now().After(deadline) { diff --git a/web/src/lib/i18n/vi.js b/web/src/lib/i18n/vi.js index 921953f..77c2645 100644 --- a/web/src/lib/i18n/vi.js +++ b/web/src/lib/i18n/vi.js @@ -335,6 +335,7 @@ export const endReasonMessages = { */ export const errorMessages = { already_greeted: 'Phiên chơi đã được mở rồi.', + already_in_a_game: 'Bạn đang trong một ván đấu. Hãy rời ván trước đã.', already_in_a_room: 'Bạn đang ở trong một phòng khác.', already_queued: 'Bạn đang chờ ghép trận rồi.', bad_frame: 'Máy chủ không đọc được dữ liệu gửi lên.', diff --git a/web/tests/error-codes.test.js b/web/tests/error-codes.test.js index f280c92..7fd23a2 100644 --- a/web/tests/error-codes.test.js +++ b/web/tests/error-codes.test.js @@ -1,7 +1,7 @@ // ServerError.code is a UI key, so the server decides the vocabulary and this // file has to speak all of it. The proto schema cannot help here — the codes -// are string literals in the Go transport, not an enum — so the guard reads -// them out of the source that emits them. +// are strings in the Go transport, not an enum — so the guard reads them out +// of the one file that declares them, server/internal/wsapi/errcodes.go. // // Without this, adding a code on the server degrades silently to the generic // fallback: the player is told "something went wrong" for a situation the @@ -14,35 +14,46 @@ import { describe, expect, it } from 'vitest'; import { errorMessages } from '../src/lib/i18n/vi.js'; const wsapiDir = fileURLToPath(new URL('../../server/internal/wsapi', import.meta.url)); +const registry = 'errcodes.go'; + +/** Every code declared in the registry, as its wire string. */ +function serverErrorCodes() { + const source = readFileSync(join(wsapiDir, registry), 'utf8'); + return new Set([...source.matchAll(/errCode = "([a-z_]+)"/g)].map(([, code]) => code)); +} /** - * Every error-code literal in the transport, excluding its own tests. Both the - * one-recipient and the broadcast call sites count: a code that only ever goes - * to both players is no less a code the client has to know. + * Call sites that hand an emitter a string literal instead of a registry + * constant. Go converts an untyped literal to errCode silently, so this is + * what keeps a new code from bypassing the list the rest of this file checks. */ -function serverErrorCodes() { - const codes = new Set(); - const files = readdirSync(wsapiDir).filter((f) => f.endsWith('.go') && !f.endsWith('_test.go')); - - for (const file of files) { - const source = readFileSync(join(wsapiDir, file), 'utf8'); - for (const [, code] of source.matchAll(/(?:errorMsg|broadcastError)\("([a-z_]+)"\)/g)) { - codes.add(code); - } - } - return codes; +function literalCallSites() { + const files = readdirSync(wsapiDir).filter( + (f) => f.endsWith('.go') && !f.endsWith('_test.go') && f !== registry + ); + return files.flatMap((file) => + [ + ...readFileSync(join(wsapiDir, file), 'utf8').matchAll( + /\b(?:errorMsg|broadcastError|toRoom)\([^)]*"/g + ) + ].map(([call]) => `${file}: ${call}`) + ); } describe('server error codes', () => { const codes = serverErrorCodes(); - it('finds the call sites at all, so an empty match cannot pass as agreement', () => { - // A rename of errorMsg would otherwise turn this whole file green by - // finding nothing to check. + it('finds the codes at all, so an empty match cannot pass as agreement', () => { + // A renamed type or moved registry would otherwise turn this whole file + // green by finding nothing to check. expect(codes.size).toBeGreaterThan(10); expect(codes.has('room_not_found')).toBe(true); }); + it('declares every code in the registry, never inline at a call site', () => { + expect(literalCallSites()).toEqual([]); + }); + it('has a Vietnamese message for every code the server can send', () => { const missing = [...codes].filter((code) => !(code in errorMessages)).sort(); expect(missing).toEqual([]);