From 93af738e3635a319f89c38e031d86cd7ff304fbf Mon Sep 17 00:00:00 2001 From: sasa Date: Sun, 30 Aug 2026 23:42:26 +0300 Subject: [PATCH] fix: two-player lobby deadlock --- data/app.js | 5 ++-- docs/API_CONTRACT.md | 5 ++++ docs/openapi.yaml | 3 +- include/game_types.h | 2 +- src/game_lifecycle.c | 9 ++++-- src/http_api.c | 3 +- src/session_manager.c | 4 ++- test/host/test_human_game_integration.c | 38 +++++++++++++++++++++++++ test/web/test_lobby_controls.js | 18 ++++++++++++ 9 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 test/web/test_lobby_controls.js diff --git a/data/app.js b/data/app.js index 8d6c96b..1560022 100644 --- a/data/app.js +++ b/data/app.js @@ -549,7 +549,7 @@ ui.modeControls.hidden = !playerOne; if (playerOne) startLobbyInfoPolling(); else stopLobbyInfoPolling(); - const playerTwoReady = info?.player2Available === false; + const playerTwoReady = state.players[1].trim() !== '' || info?.player2Available === false; const canStart = playerOne && (state.mode === 'bot' || playerTwoReady); setSpriteIcon(ui.lobbyIcon, canStart ? 'icon-check' : 'icon-hourglass'); document.querySelectorAll('.mode-button').forEach(button => { @@ -673,8 +673,7 @@ el('join-form').addEventListener('submit', event => { event.preventDefault(); - const requestedRole = info?.player1Available ? 'player1' : info?.player2Available ? 'player2' : 'spectator'; - join(requestedRole); + join('player'); }); document.querySelectorAll('.avatar-button').forEach(button => button.addEventListener('click', () => { ui.name.value = button.dataset.name; diff --git a/docs/API_CONTRACT.md b/docs/API_CONTRACT.md index 47bab9e..5e37c8e 100644 --- a/docs/API_CONTRACT.md +++ b/docs/API_CONTRACT.md @@ -66,6 +66,11 @@ The session token is in each POST body. For `GET /api/state`, it is supplied in `X-Session-Token`; absence creates a spectator-safe view. It is never a URL parameter. +For the primary “Play” action, `requestedRole: "player"` atomically assigns +the first available player slot (`player1`, then `player2`). This avoids a +client-side availability race; the response and every authorized state snapshot +contain the assigned concrete role. + `leave` and `profile-reset` release only the requesting session; profile data is cleared by the browser in Milestone 022. A player leaving an active match aborts that match and returns remaining valid players to the lobby. `game/reset` is diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 0773aa9..a45a91c 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -240,7 +240,8 @@ components: description: Opaque, reboot-scoped session token. Role: type: string - enum: [player1, player2, spectator] + enum: [player1, player2, spectator, player] + description: '`player` atomically assigns the first available player slot; responses always contain player1 or player2.' Mode: type: string enum: [human, bot] diff --git a/include/game_types.h b/include/game_types.h index ee5e1e1..6cc4b36 100644 --- a/include/game_types.h +++ b/include/game_types.h @@ -10,7 +10,7 @@ enum { CELL_UNKNOWN, CELL_WATER, CELL_SHIP, CELL_MISS, CELL_HIT }; typedef uint8_t phase_t; enum { PHASE_LOBBY, PHASE_PREPARING, PHASE_IN_PROGRESS, PHASE_FINISHED, PHASE_REMATCH_WAIT }; typedef uint8_t role_t; -enum { ROLE_PLAYER_1, ROLE_PLAYER_2, ROLE_SPECTATOR }; +enum { ROLE_PLAYER_1, ROLE_PLAYER_2, ROLE_SPECTATOR, ROLE_AUTO_PLAYER }; typedef uint8_t game_mode_t; enum { MODE_HUMAN, MODE_BOT }; diff --git a/src/game_lifecycle.c b/src/game_lifecycle.c index 8d7fa76..2963031 100644 --- a/src/game_lifecycle.c +++ b/src/game_lifecycle.c @@ -99,16 +99,18 @@ void game_lifecycle_set_bot_scheduler(game_lifecycle_t *lifecycle, scheduler_t s lifecycle_result_t game_lifecycle_join(game_lifecycle_t *lifecycle, role_t requested_role, const char *name, uint8_t *session_index) { if (lifecycle == NULL) return LIFECYCLE_RESULT_UNAUTHORIZED; - if ((requested_role == ROLE_PLAYER_1 || requested_role == ROLE_PLAYER_2) && lifecycle->game.state.phase != PHASE_LOBBY) { + if ((requested_role == ROLE_PLAYER_1 || requested_role == ROLE_PLAYER_2 || requested_role == ROLE_AUTO_PLAYER) && lifecycle->game.state.phase != PHASE_LOBBY) { return LIFECYCLE_RESULT_NO_PLAYER_SLOT; } if (requested_role == ROLE_PLAYER_2 && !session_manager_player_present(&lifecycle->sessions, 0U)) { return LIFECYCLE_RESULT_NO_PLAYER_SLOT; } - if (requested_role == ROLE_PLAYER_2 && (lifecycle->game.state.mode == MODE_BOT || lifecycle->bot_reserved)) { + if ((requested_role == ROLE_PLAYER_2 || requested_role == ROLE_AUTO_PLAYER) && (lifecycle->game.state.mode == MODE_BOT || lifecycle->bot_reserved)) { return LIFECYCLE_RESULT_NO_PLAYER_SLOT; } - return session_result(session_manager_join(&lifecycle->sessions, requested_role, name, lifecycle->random, session_index)); + const lifecycle_result_t result = session_result(session_manager_join(&lifecycle->sessions, requested_role, name, lifecycle->random, session_index)); + if (result == LIFECYCLE_RESULT_OK) ++lifecycle->game.state.version; + return result; } lifecycle_result_t game_lifecycle_resume(game_lifecycle_t *lifecycle, @@ -126,6 +128,7 @@ lifecycle_result_t game_lifecycle_leave(game_lifecycle_t *lifecycle, uint8_t ses if (lifecycle == NULL || session_index >= kSessionCapacity || !lifecycle->sessions.entries[session_index].occupied) return LIFECYCLE_RESULT_UNAUTHORIZED; const bool player = session_index < kPlayerCapacity && lifecycle->sessions.entries[session_index].role == (role_t)session_index; if (player && lifecycle->game.state.phase != PHASE_LOBBY) return_to_lobby(lifecycle); + else ++lifecycle->game.state.version; memset(&lifecycle->sessions.entries[session_index], 0, sizeof(session_t)); ++lifecycle->recovery_generation; lifecycle->recovery_reason = RECOVERY_REASON_SESSION_LEFT; diff --git a/src/http_api.c b/src/http_api.c index eb6a781..3cefc6f 100644 --- a/src/http_api.c +++ b/src/http_api.c @@ -206,6 +206,7 @@ static bool parse_join(const http_api_request_t *request, char name[kDisplayName if (strcmp(role_value, "player1") == 0) *role = ROLE_PLAYER_1; else if (strcmp(role_value, "player2") == 0) *role = ROLE_PLAYER_2; else if (strcmp(role_value, "spectator") == 0) *role = ROLE_SPECTATOR; + else if (strcmp(role_value, "player") == 0) *role = ROLE_AUTO_PLAYER; else *role = (role_t)UINT8_MAX; return true; } @@ -353,7 +354,7 @@ bool http_api_handle(http_api_t *api, const http_api_request_t *request, http_ap response_error(response, 400U, "MALFORMED_JSON", "Некорректный JSON", version); return true; } - if (role > ROLE_SPECTATOR) { + if (role > ROLE_AUTO_PLAYER) { response_error(response, 400U, "INVALID_ROLE", "Некорректная роль", version); return true; } diff --git a/src/session_manager.c b/src/session_manager.c index 226321a..715166d 100644 --- a/src/session_manager.c +++ b/src/session_manager.c @@ -48,11 +48,13 @@ session_result_t session_manager_join(session_manager_t *manager, role_t request char sanitized[kDisplayNameBytes + 1] = {0}; if (!sanitize_name(name, sanitized)) return SESSION_RESULT_INVALID_NAME; int8_t selected = -1; - if (requested_role == ROLE_PLAYER_1) selected = first_free(manager, 0, 1); + if (requested_role == ROLE_AUTO_PLAYER) selected = first_free(manager, 0, kPlayerCapacity); + else if (requested_role == ROLE_PLAYER_1) selected = first_free(manager, 0, 1); else if (requested_role == ROLE_PLAYER_2) selected = first_free(manager, 1, 2); else if (requested_role == ROLE_SPECTATOR) selected = first_free(manager, kPlayerCapacity, kSessionCapacity); else return SESSION_RESULT_INVALID_ROLE; if (selected < 0) return requested_role == ROLE_SPECTATOR ? SESSION_RESULT_NO_SPECTATOR_SLOT : SESSION_RESULT_NO_PLAYER_SLOT; + if (requested_role == ROLE_AUTO_PLAYER) requested_role = (role_t)selected; session_t candidate = {.role = requested_role, .occupied = true, .connected = true}; memcpy(candidate.name, sanitized, sizeof(candidate.name)); for (uint8_t offset = 0; offset < kSessionTokenBytes; offset += 4) { diff --git a/test/host/test_human_game_integration.c b/test/host/test_human_game_integration.c index 52f832c..60281ff 100644 --- a/test/host/test_human_game_integration.c +++ b/test/host/test_human_game_integration.c @@ -151,8 +151,46 @@ static void test_human_game_journey(void) { assert(application.lifecycle.game.state.phase == PHASE_LOBBY); } +static void test_atomic_two_player_lobby(void) { + test_random_t random = {.value = 91U}; + application_t application; + application_init(&application, (random_source_t){.next_u32 = next_random, .context = &random}); + http_api_t api; + http_api_init(&api, &application); + http_api_response_t response = call(&api, HTTP_API_ROUTE_JOIN, HTTP_API_POST, + "{\"name\":\"Алиса\",\"requestedRole\":\"player\"}", NULL); + assert(response.status == 200U && strstr(response.body, "\"role\":\"player1\"") != NULL); + char player_1[33]; token_from_response(&response, player_1); + response = call(&api, HTTP_API_ROUTE_JOIN, HTTP_API_POST, + "{\"name\":\"Борис\",\"requestedRole\":\"player\"}", NULL); + assert(response.status == 200U && strstr(response.body, "\"role\":\"player2\"") != NULL); + char player_2[33]; token_from_response(&response, player_2); + const uint32_t game_id = application.lifecycle.game.state.game_id; + response = call(&api, HTTP_API_ROUTE_STATE, HTTP_API_GET, NULL, player_1); + assert(response.status == 200U && strstr(response.body, "\"viewer\":\"player1\"") != NULL && strstr(response.body, "Алиса") != NULL && strstr(response.body, "Борис") != NULL); + response = call(&api, HTTP_API_ROUTE_STATE, HTTP_API_GET, NULL, player_2); + assert(response.status == 200U && strstr(response.body, "\"viewer\":\"player2\"") != NULL); + char config[96]; + snprintf(config, sizeof(config), "{\"token\":\"%s\",\"gameId\":%u,\"mode\":\"human\"}", player_2, (unsigned int)game_id); + response = call(&api, HTTP_API_ROUTE_CONFIG, HTTP_API_POST, config, NULL); + assert(response.status == 403U); + char start[80]; snprintf(start, sizeof(start), "{\"token\":\"%s\",\"gameId\":%u}", player_2, (unsigned int)game_id); + response = call(&api, HTTP_API_ROUTE_START, HTTP_API_POST, start, NULL); assert(response.status == 403U); + snprintf(config, sizeof(config), "{\"token\":\"%s\",\"gameId\":%u,\"mode\":\"human\"}", player_1, (unsigned int)game_id); + response = call(&api, HTTP_API_ROUTE_CONFIG, HTTP_API_POST, config, NULL); assert(response.status == 200U); + response = call(&api, HTTP_API_ROUTE_START, HTTP_API_POST, start, NULL); assert(response.status == 403U); + snprintf(start, sizeof(start), "{\"token\":\"%s\",\"gameId\":%u}", player_1, (unsigned int)game_id); + response = call(&api, HTTP_API_ROUTE_START, HTTP_API_POST, start, NULL); assert(response.status == 200U && application.lifecycle.game.state.phase == PHASE_IN_PROGRESS); + response = call(&api, HTTP_API_ROUTE_LEAVE, HTTP_API_POST, (snprintf(start, sizeof(start), "{\"token\":\"%s\",\"gameId\":%u}", player_2, (unsigned int)game_id), start), NULL); + assert(response.status == 200U && !application.lifecycle.sessions.entries[1].occupied); + response = call(&api, HTTP_API_ROUTE_JOIN, HTTP_API_POST, "{\"name\":\"Вера\",\"requestedRole\":\"player\"}", NULL); + assert(response.status == 200U && strstr(response.body, "\"role\":\"player2\"") != NULL); + resume(&api, player_1, "player1"); +} + int main(void) { test_human_game_journey(); + test_atomic_two_player_lobby(); puts("human game integration tests passed"); return 0; } diff --git a/test/web/test_lobby_controls.js b/test/web/test_lobby_controls.js new file mode 100644 index 0000000..466f4f3 --- /dev/null +++ b/test/web/test_lobby_controls.js @@ -0,0 +1,18 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); + +const app = fs.readFileSync(path.join(__dirname, '../../data/app.js'), 'utf8'); + +test('Play requests the server-authoritative next player slot', () => { + assert.match(app, /join\('player'\)/); + assert.doesNotMatch(app, /info\?\.player1Available \? 'player1'/); +}); + +test('lobby controls are visible only to player1 and enable when player2 is present', () => { + assert.match(app, /const playerOne = role === 'player1'/); + assert.match(app, /ui\.modeControls\.hidden = !playerOne/); + assert.match(app, /state\.players\[1\]\.trim\(\) !== '' \|\| info\?\.player2Available === false/); + assert.match(app, /const canStart = playerOne && \(state\.mode === 'bot' \|\| playerTwoReady\)/); +});