From 5ef0ed714d11df3af012a8530608ed43d4b0dd4b Mon Sep 17 00:00:00 2001 From: Senryoku Date: Fri, 24 May 2024 11:20:19 +0200 Subject: [PATCH] Winston: Add pile number parameter to Skip/Take pile messages. By rejecting messages with incorrect current pile number, we can prevent unintentional repeated messages. It was easy to send a repeated skip message by double clicking on the Skip button in medium latency situation, unintentionally immediately skipping an additional pile. --- client/src/App.ts | 5 +++-- src/Session.ts | 15 +++++++++----- src/SocketType.ts | 4 ++-- src/server.ts | 12 +++++------ test/winston.ts | 53 +++++++++++++++++++++-------------------------- 5 files changed, 44 insertions(+), 45 deletions(-) diff --git a/client/src/App.ts b/client/src/App.ts index 0ee84e131..7075eabb2 100644 --- a/client/src/App.ts +++ b/client/src/App.ts @@ -1972,13 +1972,14 @@ export default defineComponent({ winstonDraftTakePile() { if (!this.winstonDraftState) return; const cards = this.winstonDraftState.piles[this.winstonDraftState.currentPile] as UniqueCard[]; - this.socket.emit("winstonDraftTakePile", (answer) => { + this.socket.emit("winstonDraftTakePile", this.winstonDraftState.currentPile, (answer) => { if (answer.code === 0) for (const c of cards) this.addToDeck(c); else console.error(answer); }); }, winstonDraftSkipPile() { - this.socket.emit("winstonDraftSkipPile", (answer) => { + if (!this.winstonDraftState) return; + this.socket.emit("winstonDraftSkipPile", this.winstonDraftState.currentPile, (answer) => { if (answer.code !== 0) { console.error(answer); } diff --git a/src/Session.ts b/src/Session.ts index 2268c91c3..9106c0e36 100644 --- a/src/Session.ts +++ b/src/Session.ts @@ -955,9 +955,11 @@ export class Session implements IIndexable { } } - winstonSkipPile(): SocketAck { + winstonSkipPile(num: number): SocketAck { const s = this.draftState; if (!this.drafting || !isWinstonDraftState(s)) return new SocketError("This session is not drafting."); + if (s.currentPile !== num) return new SocketError("Invalid pile number."); + // If the card pool is empty, make sure there is another pile to pick if ( !s.cardPool.length && @@ -983,16 +985,18 @@ export class Session implements IIndexable { this.winstonNextRound(); } else { ++s.currentPile; - if (s.piles[s.currentPile].length === 0) this.winstonSkipPile(); + if (s.piles[s.currentPile].length === 0) this.winstonSkipPile(s.currentPile); else for (const uid of this.users) Connections[uid].socket.emit("winstonDraftSync", s.syncData(uid)); } return new SocketAck(); } - winstonTakePile() { + winstonTakePile(num: number): SocketAck { const s = this.draftState; - if (!this.drafting || !isWinstonDraftState(s)) return false; + if (!this.drafting || !isWinstonDraftState(s)) return new SocketError("This session is not drafting."); + if (s.currentPile !== num) return new SocketError("Invalid pile number."); + this.draftLog?.users[s.currentPlayer()].picks.push({ pickedPile: s.currentPile, piles: [...s.piles.map((p, idx) => p.slice(0, idx < s.currentPile ? -1 : undefined).map((c) => c.id))], @@ -1001,7 +1005,8 @@ export class Session implements IIndexable { if (s.cardPool.length > 0) s.piles[s.currentPile] = [s.cardPool.pop()!]; else s.piles[s.currentPile] = []; this.winstonNextRound(); - return true; + + return new SocketAck(); } ///////////////////// Winston Draft End ////////////////////// diff --git a/src/SocketType.ts b/src/SocketType.ts index 78b2908c1..227566731 100644 --- a/src/SocketType.ts +++ b/src/SocketType.ts @@ -254,8 +254,8 @@ export interface ClientToServerEvents { gridDraftPick: (choice: number, ack: (result: SocketAck) => void) => void; rochesterDraftPick: (choices: Array, ack: (result: SocketAck) => void) => void; rotisserieDraftPick: (uniqueCardID: UniqueCardID, ack: (result: SocketAck) => void) => void; - winstonDraftTakePile: (ack: (result: SocketAck) => void) => void; - winstonDraftSkipPile: (ack: (result: SocketAck) => void) => void; + winstonDraftTakePile: (num: number, ack: (result: SocketAck) => void) => void; + winstonDraftSkipPile: (num: number, ack: (result: SocketAck) => void) => void; winchesterDraftPick: (pickedColumn: number, ack: (result: SocketAck) => void) => void; housmanDraftPick: (handIndex: number, revealedCardsIndex: number, ack: (result: SocketAck) => void) => void; minesweeperDraftPick: (row: number, col: number, ack: (result: SocketAck) => void) => void; diff --git a/src/server.ts b/src/server.ts index 2f41c22d7..af3f2128b 100644 --- a/src/server.ts +++ b/src/server.ts @@ -391,19 +391,17 @@ function rotisserieDraftPick( } // Winston Draft -function winstonDraftTakePile(userID: UserID, sessionID: SessionID, ack: (result: SocketAck) => void) { +function winstonDraftTakePile(userID: UserID, sessionID: SessionID, num: number, ack: (result: SocketAck) => void) { if (!checkDraftAction(userID, Sessions[sessionID], "winston", ack)) return; - const r = Sessions[sessionID].winstonTakePile(); - - if (!r) ack?.(new SocketError("Internal error.")); - else ack?.(new SocketAck()); + const r = Sessions[sessionID].winstonTakePile(num); + ack?.(r); } -function winstonDraftSkipPile(userID: UserID, sessionID: SessionID, ack: (result: SocketAck) => void) { +function winstonDraftSkipPile(userID: UserID, sessionID: SessionID, num: number, ack: (result: SocketAck) => void) { if (!checkDraftAction(userID, Sessions[sessionID], "winston", ack)) return; - const r = Sessions[sessionID].winstonSkipPile(); + const r = Sessions[sessionID].winstonSkipPile(num); ack?.(r); } diff --git a/test/winston.ts b/test/winston.ts index bc2c6fcba..7504627a3 100644 --- a/test/winston.ts +++ b/test/winston.ts @@ -12,8 +12,7 @@ import { ackNoError, getUID, } from "./src/common.js"; -import { WinstonDraftState } from "../src/WinstonDraft.js"; -import { SocketAck } from "../src/Message.js"; +import { WinstonDraftState, WinstonDraftSyncData } from "../src/WinstonDraft.js"; describe(`Winston Draft`, function () { for (let playerCount = 2; playerCount <= 4; ++playerCount) @@ -63,7 +62,7 @@ describe(`Winston Draft`, function () { done(); }); - let states: any[] = []; + const states: WinstonDraftSyncData[] = []; it("When session owner launch Winston draft, everyone should receive a startWinstonDraft event", function (done) { ownerIdx = clients.findIndex((c) => getUID(c) === Sessions[sessionID].owner); nonOwnerIdx = 1 - ownerIdx; @@ -78,7 +77,7 @@ describe(`Winston Draft`, function () { (() => { const _idx = index; - c.once("winstonDraftNextRound", function (state) { + c.once("winstonDraftSync", function (state) { states[_idx] = state; receivedState += 1; if (connectedClients == clients.length && receivedState == clients.length) done(); @@ -97,35 +96,31 @@ describe(`Winston Draft`, function () { }); it("Non-owner reconnects, draft restarts.", function (done) { - clients[nonOwnerIdx].once("rejoinWinstonDraft", function () { - done(); - }); + clients[nonOwnerIdx].once("rejoinWinstonDraft", () => done()); clients[nonOwnerIdx].connect(); }); it("Every player takes the first pile possible and the draft should end.", function (done) { let draftEnded = 0; for (let c = 0; c < clients.length; ++c) { + clients[c].on("winstonDraftSync", (data) => (states[c] = data)); clients[c].on("winstonDraftNextRound", function (userID) { if (userID === getUID(clients[c])) - clients[c].emit("winstonDraftTakePile", (r: SocketAck) => { - expect(r.code).to.equal(0); - }); + clients[c].emit("winstonDraftTakePile", states[c].currentPile, ackNoError); }); clients[c].once("winstonDraftEnd", function () { draftEnded += 1; + clients[c].removeListener("winstonDraftSync"); clients[c].removeListener("winstonDraftNextRound"); if (draftEnded == clients.length) done(); }); } - getCurrentPlayer().emit("winstonDraftTakePile", ackNoError); + getCurrentPlayer().emit("winstonDraftTakePile", 0, ackNoError); }); it("When session owner launch Winston draft, everyone should receive a startWinstonDraft event", function (done) { - states = []; let connectedClients = 0; let receivedState = 0; - let index = 0; for (const c of clients) { c.once("startWinstonDraft", function () { connectedClients += 1; @@ -133,14 +128,11 @@ describe(`Winston Draft`, function () { }); (() => { - const _idx = index; - c.once("winstonDraftNextRound", function (state) { - states[_idx] = state; + c.once("winstonDraftNextRound", function () { receivedState += 1; if (connectedClients == clients.length && receivedState == clients.length) done(); }); })(); - ++index; } clients[ownerIdx].emit("startWinstonDraft", 6, true, ackNoError); }); @@ -153,7 +145,7 @@ describe(`Winston Draft`, function () { if (nextRound == clients.length) done(); }); } - getCurrentPlayer().emit("winstonDraftTakePile", ackNoError); + getCurrentPlayer().emit("winstonDraftTakePile", 0, ackNoError); }); it("Skiping, then taking pile.", function (done) { @@ -164,8 +156,8 @@ describe(`Winston Draft`, function () { if (nextRound == clients.length) done(); }); } - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); - getCurrentPlayer().emit("winstonDraftTakePile", ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 0, ackNoError); + getCurrentPlayer().emit("winstonDraftTakePile", 1, ackNoError); }); it("Skiping, skiping, then taking pile.", function (done) { @@ -176,9 +168,9 @@ describe(`Winston Draft`, function () { if (nextRound == clients.length) done(); }); } - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); - getCurrentPlayer().emit("winstonDraftTakePile", ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 0, ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 1, ackNoError); + getCurrentPlayer().emit("winstonDraftTakePile", 2, ackNoError); }); it("Skiping, skiping and skiping.", function (done) { @@ -190,28 +182,31 @@ describe(`Winston Draft`, function () { if (receivedRandomCard && nextRound == clients.length) done(); }); } - getCurrentPlayer().on("winstonDraftRandomCard", function (card) { + getCurrentPlayer().once("winstonDraftRandomCard", function (card) { if (card) receivedRandomCard = true; }); - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); - getCurrentPlayer().emit("winstonDraftSkipPile", ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 0, ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 1, ackNoError); + getCurrentPlayer().emit("winstonDraftSkipPile", 2, ackNoError); }); it("Every player takes the first pile possible and the draft should end.", function (done) { this.timeout(2000); let draftEnded = 0; for (let c = 0; c < clients.length; ++c) { + clients[c].on("winstonDraftSync", (data) => (states[c] = data)); clients[c].on("winstonDraftNextRound", function (userID) { - if (userID === getUID(clients[c])) clients[c].emit("winstonDraftTakePile", ackNoError); + if (userID === getUID(clients[c])) + clients[c].emit("winstonDraftTakePile", states[c].currentPile, ackNoError); }); clients[c].once("winstonDraftEnd", function () { draftEnded += 1; + clients[c].removeListener("winstonDraftSync"); clients[c].removeListener("winstonDraftNextRound"); if (draftEnded == clients.length) done(); }); } - getCurrentPlayer().emit("winstonDraftTakePile", ackNoError); + getCurrentPlayer().emit("winstonDraftTakePile", 0, ackNoError); }); }); });