From 782f9186843c1738ba6756b1319740b4717fd6e2 Mon Sep 17 00:00:00 2001 From: sganggs Date: Sun, 4 Oct 2026 17:38:29 +0800 Subject: [PATCH] feedback3 W3C: room.kick names the confirmed player; a seat that changed hands while the host confirmed is refused (BAD_TARGET) --- docs/DESIGN.md | 2 +- public/js/screens/room.js | 9 ++++--- server/lobby.js | 15 ++++++----- shared/protocol.js | 5 ++-- test/lobby-kick.test.js | 52 ++++++++++++++++++++++++++++++--------- 5 files changed, 58 insertions(+), 25 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index e155d0f9..de5595ba 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -407,7 +407,7 @@ Each domain file `server/sim/content/{tokens,bonds,garrisons,items,bands,enemies Transport: one WebSocket per tab at `/ws`, JSON text frames `{ t: '', ...payload }`. Client requests may carry `rid` (request id); the server's direct reply echoes `rid` (`ok` or `error`). Server pushes have no `rid`. Max frame 64 KB inbound; rate limit 40 msgs/s per connection (excess dropped with `error RATE`). ### 8.1 Session & lobby -C→S: `hello {name, token?, version}` · `room.create {mode:'solo'|'coop', difficulty}` · `room.join {code}` · `room.leave` · `room.ready {ready}` · `room.setDifficulty {difficulty}` (host) · `room.addBot` / `room.removeBot {seat}` (host) · `room.kick {seat}` (host, before the match: another human leaves the room and gets `room.closed {reason:'kicked'}`; may join again) · `room.start` (host) · `room.loadout {entries}` (§16: any time in the lobby / room and during INFO_CHECK, then `WRONG_PHASE`; heavy rate-limit bucket; the match exposes it as `m.private.loadout`) · `ping {c}` +C→S: `hello {name, token?, version}` · `room.create {mode:'solo'|'coop', difficulty}` · `room.join {code}` · `room.leave` · `room.ready {ready}` · `room.setDifficulty {difficulty}` (host) · `room.addBot` / `room.removeBot {seat}` (host) · `room.kick {seat, playerId}` (host, before the match: another human leaves the room and gets `room.closed {reason:'kicked'}`; may join again; `playerId` = the player the host confirmed, a seat that changed hands meanwhile → `BAD_TARGET`) · `room.start` (host) · `room.loadout {entries}` (§16: any time in the lobby / room and during INFO_CHECK, then `WRONG_PHASE`; heavy rate-limit bucket; the match exposes it as `m.private.loadout`) · `ping {c}` S→C: `welcome {playerId, token, name, serverNow}` · `room.state {code, hostId, mode, difficulty, seats:[{seat, playerId, name, isBot, ready, connected}|null ×4]}` · `room.closed {reason}` · `pong {c, s}` · `error {rid?, code, msg}` · `ok {rid}` Platform additions (implemented): `welcome.{resumed, version}`, `room.state.inMatch`, `error.detail`; `room.closed.reason ∈ 'timeout'|'kicked'|'shutdown'`; WebSocket close codes 4001 (session taken over by another socket — client must not auto-reconnect), 4002 (no hello in time), 1008 (flooding), 1001 (shutdown). The host's `room.start` counts as the host being ready. After any (re)`hello` the server pushes `room.state` and, if a match runs, the match resends `m.public`/`m.private` (+`m.field`, or the field's `b.start` under client-side combat). **Reconnect window:** a dropped session stays resumable for 10 min (`net.js NET_DEFAULTS.reconnectWindowMs`; co-op seats keep playing their last lineup meanwhile, §6.6); a session that drops while its **solo** room's match runs stays resumable for the official `config.constants.singleReconnectTime` (86 400 s = 24 h; lobby option `soloReconnectWindowMs`; research 01 §1 "24小时内随时返回") — the untimed solo run simply waits. Only when the window expires does the drop become `match.onLeave` (co-op: 中途退出 = elimination; solo: the run ends `'abandoned'`). The client keeps its token in sessionStorage plus a recent-token list in localStorage, so reopening the tab resumes. Everything lives in server memory: a server restart ends every room and run. The lobby⇄match interface (constructor opts, `start/handle/onDisconnect/onReconnect/onLeave/dispose`) is documented verbatim at the top of `server/match/Match.js` and is normative. `server/data.js` provides `getData()` + index getters (`getChess`, `getBond`, `getGarrison`, `getItem`, `getBand`, `getEffect`, `getEnemy`, `getWave`, `getStage`, `getBoss`, `getToken`, `getConfig`, `getMode`). Note: the skill trigger rule lives at `chess.skill.trigger.rule` in data. diff --git a/public/js/screens/room.js b/public/js/screens/room.js index 84a8456d..675fc9ff 100644 --- a/public/js/screens/room.js +++ b/public/js/screens/room.js @@ -127,7 +127,7 @@ function SeatCard({ seat, index, room, facts, myId, busy, onAddBot, onRemoveBot, <${Button} variant="ghost" size="sm" square=${true} icon="close" loading=${busy === `rm${index}`} onClick=${() => onRemoveBot(index)} aria-label="移除 AI 队友" /> ` : null} ${!seat.isBot && !isMe && facts.isHost ? html`<${Tooltip} text="将该博士移出同盟"> - <${Button} variant="ghost" size="sm" square=${true} icon="close" loading=${busy === `kick${index}`} onClick=${() => onKick(index, seat.name)} aria-label="移出该博士" /> + <${Button} variant="ghost" size="sm" square=${true} icon="close" loading=${busy === `kick${index}`} onClick=${() => onKick(index, seat.name, seat.playerId)} aria-label="移出该博士" /> ` : null} `; @@ -197,11 +197,12 @@ export function RoomScreen() { const start = () => run('start', () => net.request('room.start', {})); const addBot = () => run('add', () => net.request('room.addBot', {})); const removeBot = (seat) => run(`rm${seat}`, () => net.request('room.removeBot', { seat })); - // the host removes a human before the match (community report #17): asked first; the player may join again - const kick = async (seat, name) => { + // the host removes a human before the match (community report #17): asked first; the player may join again. The + // confirmed player's id goes along: if they left and someone else took the seat meanwhile, the server refuses it. + const kick = async (seat, name, playerId) => { if (inFlight.current) return; const ok = await confirmDialog({ title: '移出同盟', text: `确定将「${name || '博士'}」移出同盟吗?对方可以凭同盟密钥重新加入。`, okText: '移出', danger: true }); - if (ok) run(`kick${seat}`, () => net.request('room.kick', { seat })); + if (ok) run(`kick${seat}`, () => net.request('room.kick', { seat, playerId })); }; const setDifficulty = (difficulty) => run('diff', () => net.request('room.setDifficulty', { difficulty })); const leave = async () => { diff --git a/server/lobby.js b/server/lobby.js index b48cfcd6..caa35a03 100644 --- a/server/lobby.js +++ b/server/lobby.js @@ -11,11 +11,13 @@ // * Host-only: room.setDifficulty, room.addBot, room.removeBot, room.kick, room.start. ▸ Changing the difficulty // un-readies the other humans. ▸ room.start requires every other human to be connected and ready; // the host's start counts as the host's ready (the host may still toggle room.ready for display). -// * room.kick {seat} (community report #17, owner approved): before the match only, the host removes another human -// like an AI seat (an AI seat stays room.removeBot's; never the host itself). The seat is freed at once and the -// player gets `room.closed {reason:'kicked'}` — now, or on the next resume when offline (with the result replay, -// as the grace timeout) —, so the reconnect token no longer leads back to the seat (it stays the player's -// identity: net.js sessions belong to players, not seats). ▸ No ban: the player may join again with the code. +// * room.kick {seat, playerId} (community report #17, owner approved): before the match only, the host removes another +// human like an AI seat (an AI seat stays room.removeBot's; never the host itself). `playerId` names the player the +// host confirmed: a seat that changed hands meanwhile (left, someone else joined) is refused with BAD_TARGET. The +// seat is freed at once and the player gets `room.closed {reason:'kicked'}` — now, or on the next resume when +// offline (with the result replay, as the grace timeout) —, so the reconnect token no longer leads back to the seat +// (it stays the player's identity: net.js sessions belong to players, not seats). ▸ No ban: the player may join +// again with the code. // * Host migration: when the host leaves (or is removed), the lowest-seat remaining human (connected // ones first) becomes host. A room without humans is disposed (bots never keep a room alive). // * Disconnect in LOBBY: the seat shows connected=false and is freed after `lobbyGraceMs` (60 s); a @@ -428,7 +430,7 @@ export class Lobby { } /** Host removes another human before the match (header: room.kick). */ - kick(session, { seat }) { + kick(session, { seat, playerId }) { const room = this.roomOf(session); if (!room) return fail(ERR.NOT_IN_ROOM); if (room.hostId !== session.playerId) return fail(ERR.NOT_HOST); @@ -436,6 +438,7 @@ export class Lobby { this.dropReplay(room, session.playerId); const target = room.seats[seat]; if (!target || target.left) return fail(ERR.BAD_TARGET, 'seat holds no player'); + if (target.playerId !== playerId) return fail(ERR.BAD_TARGET, 'seat changed hands'); // the confirmed player left meanwhile if (target.isBot) return fail(ERR.BAD_TARGET, 'seat holds an AI (room.removeBot)'); if (target.playerId === session.playerId) return fail(ERR.BAD_TARGET, 'cannot kick yourself'); const kicked = this.registry.byId(target.playerId); diff --git a/shared/protocol.js b/shared/protocol.js index 1d8fba4f..f4864b19 100644 --- a/shared/protocol.js +++ b/shared/protocol.js @@ -245,8 +245,9 @@ export const C2S = { 'room.setDifficulty': { difficulty: (v) => DIFFICULTIES.includes(v) }, 'room.addBot': {}, 'room.removeBot': { seat: (v) => isInt(v, 0, MAX_SEATS - 1) }, - // the host removes another human before the match (server/lobby.js kick; community report #17) - 'room.kick': { seat: (v) => isInt(v, 0, MAX_SEATS - 1) }, + // the host removes another human before the match (server/lobby.js kick; community report #17); playerId = the one the + // host confirmed — a seat that changed hands meanwhile is refused + 'room.kick': { seat: (v) => isInt(v, 0, MAX_SEATS - 1), playerId: isId }, 'room.start': {}, // operator loadout (DESIGN §16): stored per session/seat; accepted until the match leaves INFO_CHECK 'room.loadout': { entries: isLoadoutEntries }, diff --git a/test/lobby-kick.test.js b/test/lobby-kick.test.js index 7c9d961a..603c431e 100644 --- a/test/lobby-kick.test.js +++ b/test/lobby-kick.test.js @@ -1,6 +1,7 @@ // test/lobby-kick.test.js — room.kick (community report #17, owner approved): before the match the host removes another // human like an AI seat (server/lobby.js kick). The player gets room.closed {kicked} (now, or on the next resume when // offline), the reconnect token no longer leads back to the seat, and the player may join again (no ban) [ASSUMED]. +// The message names the confirmed player too: a seat that changed hands while the host's dialog was open is refused. import { describe, test, before, after, afterEach } from 'node:test'; import assert from 'node:assert/strict'; @@ -57,9 +58,11 @@ describe('room.kick (lobby)', () => { assert.deepEqual(cap.errors, [], 'no server errors logged'); }); - test('protocol: room.kick {seat} with a seat index', () => { - assert.equal(validateC2S({ t: 'room.kick', seat: 1 }), null); - assert.notEqual(validateC2S({ t: 'room.kick', seat: 9 }), null); + test('protocol: room.kick {seat, playerId} — a seat index and the confirmed player', () => { + assert.equal(validateC2S({ t: 'room.kick', seat: 1, playerId: 'p_0123456789' }), null); + assert.notEqual(validateC2S({ t: 'room.kick', seat: 1 }), null, 'the player is required'); + assert.notEqual(validateC2S({ t: 'room.kick', seat: 9, playerId: 'p_0123456789' }), null); + assert.notEqual(validateC2S({ t: 'room.kick', seat: 1, playerId: '' }), null); assert.notEqual(validateC2S({ t: 'room.kick' }), null); }); @@ -71,7 +74,7 @@ describe('room.kick (lobby)', () => { await ok(guest, { t: 'room.ready', ready: true }); await host.waitFor('room.state', (s) => seatOf(s, guest.id)?.ready); guest.clearInbox(); - await ok(host, { t: 'room.kick', seat: 1 }); + await ok(host, { t: 'room.kick', seat: 1, playerId: guest.id }); const closed = await guest.waitFor('room.closed'); assert.equal(closed.reason, 'kicked'); const after = await host.waitFor('room.state', (s) => s.seats[1] === null); @@ -91,7 +94,7 @@ describe('room.kick (lobby)', () => { await joinRoom(guest, st.code); await guest.terminate(); await host.waitFor('room.state', (s) => seatOf(s, guest.id)?.connected === false); - await ok(host, { t: 'room.kick', seat: 1 }); + await ok(host, { t: 'room.kick', seat: 1, playerId: guest.id }); await host.waitFor('room.state', (s) => s.seats[1] === null); // the host can start alone right away (the offline seat no longer blocks it with NOT_READY) await ok(host, { t: 'room.start' }); @@ -111,19 +114,44 @@ describe('room.kick (lobby)', () => { const st = await createRoom(host); const guest = await pool.player('Guest'); await joinRoom(guest, st.code); - await err(guest, { t: 'room.kick', seat: 0 }, ERR.NOT_HOST); - const self = await err(host, { t: 'room.kick', seat: 0 }, ERR.BAD_TARGET); + await err(guest, { t: 'room.kick', seat: 0, playerId: host.id }, ERR.NOT_HOST); + const self = await err(host, { t: 'room.kick', seat: 0, playerId: host.id }, ERR.BAD_TARGET); assert.match(self.detail || self.msg, /yourself/); await ok(host, { t: 'room.addBot' }); - await host.waitFor('room.state', (s) => s.seats[2]?.isBot); - await err(host, { t: 'room.kick', seat: 2 }, ERR.BAD_TARGET); - await err(host, { t: 'room.kick', seat: 3 }, ERR.BAD_TARGET); + const withBot = await host.waitFor('room.state', (s) => s.seats[2]?.isBot); + await err(host, { t: 'room.kick', seat: 2, playerId: withBot.seats[2].playerId }, ERR.BAD_TARGET); + await err(host, { t: 'room.kick', seat: 3, playerId: guest.id }, ERR.BAD_TARGET); + await err(host, { t: 'room.kick', seat: 1 }, ERR.BAD_MSG); const stranger = await pool.player('Stranger'); - await err(stranger, { t: 'room.kick', seat: 1 }, ERR.NOT_IN_ROOM); + await err(stranger, { t: 'room.kick', seat: 1, playerId: guest.id }, ERR.NOT_IN_ROOM); await ok(guest, { t: 'room.ready', ready: true }); await host.waitFor('room.state', (s) => seatOf(s, guest.id)?.ready); await ok(host, { t: 'room.start' }); await host.waitFor('room.state', (s) => s.inMatch === true); - await err(host, { t: 'room.kick', seat: 1 }, ERR.ROOM_STARTED); + await err(host, { t: 'room.kick', seat: 1, playerId: guest.id }, ERR.ROOM_STARTED); + }); + + test('a seat that changed hands while the host confirmed is refused: the newcomer stays (review of #17)', async () => { + const host = await pool.player('Host'); + const st = await createRoom(host); + const alice = await pool.player('Alice'); + await joinRoom(alice, st.code); + await host.waitFor('room.state', (s) => seatOf(s, alice.id)?.seat === 1); + // the host's dialog names Alice … she leaves and Bob takes seat 1 before the host confirms + await ok(alice, { t: 'room.leave' }); + await host.waitFor('room.state', (s) => s.seats[1] === null); + const bob = await pool.player('Bob'); + const joined = await joinRoom(bob, st.code); + assert.equal(seatOf(joined, bob.id).seat, 1); + bob.clearInbox(); + const stale = await err(host, { t: 'room.kick', seat: 1, playerId: alice.id }, ERR.BAD_TARGET); + assert.match(stale.detail || stale.msg, /changed hands/); + await bob.expectNone('room.closed'); + await ok(bob, { t: 'room.ready', ready: true }); + const still = await host.waitFor('room.state', (s) => seatOf(s, bob.id)?.ready); + assert.equal(seatOf(still, bob.id).seat, 1, 'Bob keeps the seat'); + // naming Bob works + await ok(host, { t: 'room.kick', seat: 1, playerId: bob.id }); + assert.equal((await bob.waitFor('room.closed')).reason, 'kicked'); }); });