feat: migrate multiplayer to PeerJS P2P and add 3-4 player support#10
feat: migrate multiplayer to PeerJS P2P and add 3-4 player support#10
Conversation
Summary of ChangesHello @schaoss, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural shift for the game's multiplayer functionality, transitioning from a centralized server approach to a peer-to-peer model. This change not only enhances the robustness and directness of online interactions but also expands the game's capacity to include 3 and 4 players. The underlying state management and user interface components have been thoroughly revised to support these new capabilities, removing obsolete server-side elements in the process. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a major architectural change, migrating the multiplayer functionality from a server-based Socket.io implementation to a client-side P2P model using PeerJS. It also successfully introduces support for 3 and 4 player games. The changes are extensive, touching everything from the core game state management in Zustand to the UI components and game logic utilities. The addition of a dedicated multiplayerStore is a clean way to encapsulate the P2P logic. I've identified one high-severity bug in the move calculation that should be addressed.
| const newSteps = stepsTaken + dist | ||
| const newSteps = stepsTaken + dist // Approximation. `pathResult.path.length - 1` is better. | ||
| // Use path length if available | ||
| const actualSteps = pathResult ? pathResult.path.length : newSteps |
There was a problem hiding this comment.
There's a small but important bug here in calculating the number of steps taken. pathResult.path.length includes the starting node, so the number of steps is actually path.length - 1. Using path.length will incorrectly calculate one extra step, which will affect the calculation for any subsequent moves within the same turn (e.g., it will prevent a second step when it should be allowed).
For example, a path from A -> B has a length of 2, but represents 1 step.
| const actualSteps = pathResult ? pathResult.path.length : newSteps | |
| const actualSteps = pathResult ? pathResult.path.length - 1 : newSteps |
- Rewrite MinimaxAI evaluation to support 3/4 player modes: - evaluateZOCDistance() now calculates for all players dynamically - evaluateTerritoryPotential() compares current player vs all opponents - actionHeuristic() considers all opponents - Zobrist hashing supports Y and G player stones - Move ordering added at all minimax levels - Improve PeerJS connection reliability: - Add 8 STUN servers for better NAT traversal - Add retry mechanism with exponential backoff (3 retries) - Increase join timeout to 30 seconds - Add connection status tracking - Improve multiplayer error handling: - Add ERROR_MESSAGES with i18n keys and suggestions - Add new error types: PEER_UNAVAILABLE, NETWORK_ERROR, TIMEOUT - Update all 6 locale files with descriptive error messages - Add comprehensive test coverage: - minimax-ai.multiplayer.test.ts (8 tests) - multiplayerStore.error.test.ts (8 tests) - multiplayerStore.ice.test.ts (5 tests) - ai.action.test.ts (8 tests) - ai.multiplayer.test.ts (8 tests) All 65 tests passing, build successful.
…action application
fix(store): applyActionSequence actor detection & test cleanup
…ean handlers on cancel
…sure no late AI replies applied
…sure no late AI replies applied
Add unit tests verifying TurnManager correctly handles late AI replies: - turnmanager.race.test.ts: Tests delayed agent responses are ignored after timeout - turnmanager.worker.test.ts: Tests worker simulation with FakeWorker for timing scenarios Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Verifies MinimaxAI.getBestMove correctly selects from getLegalActions and handles claimed territory filtering. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…ate AI replies Prevents race condition where delayed AI worker responses could be applied after timeout: - TurnManager: Pass requestId to agent.getAction(); verify returned requestId matches - AIWorker: Include requestId in postMessage response - PlayerAgent: Extend interface to accept optional requestId parameter - RandomAgent/MinimaxAgent: Add cancel flag, ignore late messages, implement cancel() - HumanAgent: Conform to updated PlayerAgent interface This ensures late AI replies (arriving after timeout/cancellation) are discarded. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Changes in this PR (automated update)\n\n- Fix: module alias resolution in tests (tsconfig/vite/vitest aligned)\n- Fix: store.applyActionSequence actor detection and atomic application\n- Fix: TurnManager now ignores late AI replies using requestId and cancels agents on timeout\n- Fix: Agents (RandomAgent, MinimaxAgent) clean worker handlers on resolution/rejection to avoid late handlers\n- Style: ran eslint --fix against UI/test files to satisfy pre-commit hooks\n\nCI summary:\n- bun run build: passed\n- bun test: 66 tests passed, 0 failed\n\nNotes:\n- No new dependencies added.\n- Agent.cancel() must be respected by any new agent implementations to avoid late replies.