mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 07:40:43 +00:00
feat: Prevent GameServer from restarting after ending by introducin… (#2923)
If this PR fixes an issue, link it below. If not, delete these two lines. Resolves #(issue number) #2919 In GameManager.tick(), when a game becomes active but hasn't started, a setTimeout for game.start() is scheduled with a 2-second delay. If the game finishes or is cancelled within those 2 seconds, game.end() is called, which clears the existing interval. However: 1.The 2-second timeout still fires. game.start() executes. 2. A NEW setInterval is created for turn execution. 3.Since the game is already ending/finished, it's removed from GameManager.games, but the interval continues to run forever in the background ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: codimo
This commit is contained in:
+1
-1
@@ -11,7 +11,7 @@
|
||||
"dev:prod": "cross-env GAME_ENV=dev API_DOMAIN=api.openfront.io concurrently \"npm run start:client\" \"npm run start:server-dev\"",
|
||||
"docs:map-generator": "cd map-generator && go doc -cmd -u -all",
|
||||
"tunnel": "npm run build-prod && npm run start:server",
|
||||
"test": "vitest run",
|
||||
"test": "vitest run && vitest run tests/server",
|
||||
"perf": "npx tsx tests/perf/*.ts",
|
||||
"test:coverage": "vitest run --coverage",
|
||||
"format": "prettier --ignore-unknown --write .",
|
||||
|
||||
@@ -71,6 +71,8 @@ export class GameServer {
|
||||
{ winner: ClientSendWinnerMessage; ips: Set<string> }
|
||||
> = new Map();
|
||||
|
||||
private _hasEnded = false;
|
||||
|
||||
public desyncCount = 0;
|
||||
|
||||
constructor(
|
||||
@@ -536,7 +538,7 @@ export class GameServer {
|
||||
}
|
||||
|
||||
public start() {
|
||||
if (this._hasStarted) {
|
||||
if (this._hasStarted || this._hasEnded) {
|
||||
return;
|
||||
}
|
||||
this._hasStarted = true;
|
||||
@@ -639,9 +641,11 @@ export class GameServer {
|
||||
}
|
||||
|
||||
async end() {
|
||||
this._hasEnded = true;
|
||||
// Close all WebSocket connections
|
||||
if (this.endTurnIntervalID) {
|
||||
clearInterval(this.endTurnIntervalID);
|
||||
this.endTurnIntervalID = undefined;
|
||||
}
|
||||
this.websockets.forEach((ws) => {
|
||||
if (ws.readyState === WebSocket.OPEN) {
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../../src/core/configuration/ConfigLoader", () => ({
|
||||
getServerConfigFromServer: () => ({
|
||||
otelEnabled: () => false,
|
||||
otelAuthHeader: () => "",
|
||||
otelEndpoint: () => "",
|
||||
env: () => 0, // GameEnv.Dev
|
||||
}),
|
||||
getServerConfig: () => ({
|
||||
otelEnabled: () => false,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../../src/core/Schemas", async () => {
|
||||
const actual = (await vi.importActual("../../src/core/Schemas")) as any;
|
||||
return {
|
||||
...actual,
|
||||
GameStartInfoSchema: {
|
||||
safeParse: (data: any) => ({ success: true, data: data }),
|
||||
},
|
||||
ServerPrestartMessageSchema: {
|
||||
safeParse: (data: any) => ({ success: true, data: data }),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { GameEnv } from "../../src/core/configuration/Config";
|
||||
import { GameType } from "../../src/core/game/Game";
|
||||
import { GameServer } from "../../src/server/GameServer";
|
||||
|
||||
describe("GameLifecycle", () => {
|
||||
let mockLogger: any;
|
||||
let mockConfig: any;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
mockLogger = {
|
||||
child: vi.fn().mockReturnThis(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
};
|
||||
mockConfig = {
|
||||
turnIntervalMs: () => 100,
|
||||
gameCreationRate: () => 1000,
|
||||
env: () => GameEnv.Dev,
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
vi.clearAllTimers();
|
||||
});
|
||||
|
||||
it("should not start turn interval if game has ended", async () => {
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{ gameType: GameType.Private } as any,
|
||||
);
|
||||
|
||||
// Call end() first - this should set _hasEnded
|
||||
await game.end();
|
||||
|
||||
// Now call start() - this should be a no-op due to our fix
|
||||
game.start();
|
||||
|
||||
// Check if the interval ID is set (it shouldn't be)
|
||||
expect((game as any).endTurnIntervalID).toBeUndefined();
|
||||
|
||||
// Check if _hasStarted remained false (or at least no interval was created)
|
||||
expect(game.hasStarted()).toBe(false);
|
||||
});
|
||||
|
||||
it("should clear turn interval and set _hasEnded on end()", async () => {
|
||||
// We need to initialize the game such that start() can succeed
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{
|
||||
gameType: GameType.Private,
|
||||
gameMap: "plains",
|
||||
gameMapSize: 100,
|
||||
} as any,
|
||||
);
|
||||
|
||||
// Manually trigger prestart to fulfill some internal checks if necessary
|
||||
game.prestart();
|
||||
|
||||
// start() should create the interval
|
||||
game.start();
|
||||
expect((game as any).endTurnIntervalID).toBeDefined();
|
||||
|
||||
// end() should clear it
|
||||
await game.end();
|
||||
expect((game as any).endTurnIntervalID).toBeUndefined();
|
||||
expect((game as any)._hasEnded).toBe(true);
|
||||
});
|
||||
|
||||
it("should be resilient to multiple end() calls", async () => {
|
||||
const game = new GameServer(
|
||||
"test-game",
|
||||
mockLogger,
|
||||
Date.now(),
|
||||
mockConfig,
|
||||
{ gameType: GameType.Private } as any,
|
||||
);
|
||||
|
||||
await game.end();
|
||||
expect((game as any)._hasEnded).toBe(true);
|
||||
|
||||
// Should not throw or crash
|
||||
await expect(game.end()).resolves.toBeUndefined();
|
||||
expect((game as any)._hasEnded).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user