-
Notifications
You must be signed in to change notification settings - Fork 4
Bot pause resume #264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bot pause resume #264
Conversation
added pause dialogs and messages to handle the client-side pause notification
Added a notification dialog to give the user information eg. game has been unpaused, puzzle info, line changes, etc.
added various gates to robot movement based on if the game is paused. Allowed pausing/unpausing through an api call
added a stop for promises in progress also added some hotfixes for pause
…it, going to do the safe for unpause game, then have to actually clal the methods
…f it does somehow work, need to do two things. First, make it so pausing and resuming requests sent by client/server are separate, so client can't unpause if server does. Second, make code more elegeant, ts solution is ugly as hell 🥀
saved game state as well as robot positions in save files
…d now? Gonna add comments and see if there's anything I can make better
…s kinda crapy though, ima see how I can make my code cleaner and better
…in pauseGame and unpauseGame
allowed pause to rollback the robot positions. Also allowed saveGame to load from a saved positions, so hopefully less errors
Forced clients to be paused even on join or reload.
* advanced stop procedure added a stop for promises in progress also added some hotfixes for pause * added robot saves saved game state as well as robot positions in save files * Added pause rollback allowed pause to rollback the robot positions. Also allowed saveGame to load from a saved positions, so hopefully less errors * pause on reload Forced clients to be paused even on join or reload. --------- Co-authored-by: ymmot239 <[email protected]> ^ I'm pulling this play feature code into bot-pause-resume to integrate with my side of the feature
…o integrate my unpauseGame function with what's currently being done, I have to just check that out and besides that this task is done
…r than it is right now as it's calling a function in a file it shouldn't really be calling
Added back some functions that were removed during a merge from main. Also removed some unused functions in executor.
Fixed the bug with executors not running all the commands Also cleaned up the paused api being sent to the clients.
… when the method is called in managers.ts
…vil method. Dunno if I can get it to look any better though
…probably the best it's gonna be to avoid looping dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a bot pause/resume feature that automatically pauses the game when bots disconnect and resumes when all bots reconnect. The implementation includes a pause handler to manage dependency cycles, game state persistence through save files, and UI dialogs for pause notifications.
- Adds automatic game pause on bot disconnect and resume when all bots reconnect
- Creates a separate pauseHandler module to resolve dependency cycles
- Extends save functionality to include board positions and robot locations
- Updates UI with pause/unpause dialogs and prevents moves during pause
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
File | Description |
---|---|
src/server/robot/robot.ts | Adds sendStopPacket method and improves packet formatting |
src/server/robot/robot-manager.ts | Adds removeRobot and stopAllRobots methods for disconnect handling |
src/server/command/executor.ts | Modifies command execution to respect pause state |
src/server/command/command.ts | Updates command groups to check pause state before execution |
src/server/api/tcp-interface.ts | Implements bot disconnect detection and pause/unpause triggers |
src/server/api/save-manager.ts | Extends save format to include FEN and robot positions |
src/server/api/pauseHandler.ts | New module handling pause logic and robot positioning |
src/server/api/managers.ts | Adds disconnectedBots set for tracking |
src/server/api/game-manager.ts | Updates game managers to respect pause state |
src/server/api/api.ts | Adds pause/unpause endpoints and refactors positioning logic |
src/common/game-end-reasons.ts | Adds GAME_PAUSED and GAME_UNPAUSED enum values |
src/client/game/game.tsx | Implements pause UI state and prevents moves when paused |
src/client/game/admin-dialogs.tsx | New pause and notification dialog components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/server/robot/robot.ts
Outdated
} | ||
|
||
public async sendStopPacket(): Promise<void> { | ||
console.log("Stopoping the robot: " + this.id); |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in console log: 'Stopoping' should be 'Stopping'.
console.log("Stopoping the robot: " + this.id); | |
console.log("Stopping the robot: " + this.id); |
Copilot uses AI. Check for mistakes.
src/server/api/save-manager.ts
Outdated
*/ | ||
|
||
import { Side } from "../../common/game-types"; | ||
2; |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line appears to be a stray '2;' that doesn't serve any purpose and should be removed.
2; |
Copilot uses AI. Check for mistakes.
src/server/api/pauseHandler.ts
Outdated
export let gamePaused = false; | ||
export let pauser: string = "none"; | ||
|
||
export function setPaused(theFlag) { |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter 'theFlag' should have explicit type annotation. Should be 'theFlag: boolean'.
export function setPaused(theFlag) { | |
export function setPaused(theFlag: boolean) { |
Copilot uses AI. Check for mistakes.
src/server/api/pauseHandler.ts
Outdated
} | ||
|
||
// created a pause and unpause game function separately from the endpoint call so that another backend function can call it as well. | ||
export function pauseGame(clientSide) { |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter 'clientSide' should have explicit type annotation. Should be 'clientSide: boolean'.
Copilot uses AI. Check for mistakes.
src/server/api/pauseHandler.ts
Outdated
return { message: "success" }; | ||
} | ||
|
||
export function unpauseGame(clientSide) { |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter 'clientSide' should have explicit type annotation. Should be 'clientSide: boolean'.
Copilot uses AI. Check for mistakes.
src/server/command/command.ts
Outdated
const promises = this.commands.map((move) => move.execute()); | ||
const promises = this.commands.map((move) => { | ||
if (!gamePaused) return move.execute(); | ||
}); |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map function may return undefined for paused commands, creating an array with undefined values. This should either filter out undefined values or handle the paused case differently.
}); | |
}).filter(Boolean); |
Copilot uses AI. Check for mistakes.
src/server/command/command.ts
Outdated
for (const command of this.commands) { | ||
promise = promise.then(() => command.execute()); | ||
promise = promise.then(() => { | ||
if (!gamePaused) return command.execute(); |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to ParallelCommandGroup, this may return undefined when the game is paused, potentially breaking the promise chain.
if (!gamePaused) return command.execute(); | |
if (!gamePaused) { | |
return command.execute(); | |
} else { | |
return Promise.resolve(); | |
} |
Copilot uses AI. Check for mistakes.
src/client/game/admin-dialogs.tsx
Outdated
} | ||
|
||
/** | ||
* SHows a closable notification dialog |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: 'SHows' should be 'Shows'.
* SHows a closable notification dialog | |
* Shows a closable notification dialog |
Copilot uses AI. Check for mistakes.
move; | ||
}; //send a do-nothing function if game is paused |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The paused move handler just references 'move' without doing anything. This should either be an empty function or have a comment explaining why the move parameter is referenced.
move; | |
}; //send a do-nothing function if game is paused | |
// Do nothing when the game is paused | |
}; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good To Me
This code basically pauses the game when a bot is disconnected, and doesn't unpause until all bots are reconnected.
For this file, to fix dependency cycles, I had to make a separate "pauseHandler" file that both api.ts and tcp-interface.ts would call to pause/unpause the game. I couldn't put it in api.ts because otherwise dependency cycles exist (api.ts imports from tcp-interface.ts, and vice versa), and I couldn't put it in managers.ts because there are some dependency cycles related to Simulator, Robot, and Managers I believe. As far as I know, this is the only way that works.
I also had to move a function "setAllRobotsToDefaultPositions" into pauseHandler even though it doesn't exactly belong because again, dependency issues, as api.ts would call unpause game but then unpause game would call this function.
I hate dependency cycles.