feat: 8-player multiplayer support with command relay#1170
Conversation
- Update multiplayerStore.ts with multi-peer infrastructure: - Add peerChannels Map for tracking multiple connections - Add remotePeerIds array for all connected peers - Implement addPeer/removePeer for managing connections - Add broadcastMessage() for sending to all peers - Add sendToPeer() for targeted messages - Host automatically relays commands to other guests - Update Game.ts lockstep barrier for N players: - getExpectedPlayerIds() now returns all peer IDs - Supports 2-8 players in lockstep synchronization - Update useMultiplayer.ts for multi-peer sync: - Sync ALL guest data channels using addPeer() - Guest mode also uses addPeer for consistency - Delete dead code (~1400 lines): - src/hooks/useP2P.ts (superseded by useMultiplayer.ts) - src/engine/network/p2p/ConnectionCode.ts (not used) - src/rendering/GroundDetail.ts (CrystalField never imported) - tests/engine/network/connectionCode.test.ts https://claude.ai/code/session_01V7DKiAVN3pWV2NxZ7iYsjb
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39137cfea5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Get all remote peer IDs (supports 2-8 players) | ||
| const remotePeerIds = getAllPeerIds(); | ||
| for (const peerId of remotePeerIds) { | ||
| if (!players.includes(peerId)) { | ||
| players.push(peerId); |
There was a problem hiding this comment.
Align lockstep expected IDs with command player IDs
In multiplayer, getExpectedPlayerIds() now appends getAllPeerIds() (Game.ts 479-483), but those IDs are populated from network peer identifiers (addPeer(guest.pubkey, ...) and the synthetic host-... in useMultiplayer.ts 850-858, 875-881). Lockstep receipts are recorded under command.playerId (Game.ts 639-643), which is sourced from getLocalPlayerId()/slot IDs (useWorkerBridge.ts 136-149). In a normal lobby where player IDs are player1/player2, the peer IDs will never match the command receipts, so hasAllCommandsForTick() will wait indefinitely and the lockstep barrier can stall or desync.
Useful? React with 👍 / 👎.
| peerChannels: newPeerChannels, | ||
| remotePeerIds: newPeerIds, | ||
| // Set legacy fields to first peer for backwards compatibility | ||
| remotePeerId: state.remotePeerId || peerId, | ||
| dataChannel: state.dataChannel || dataChannel, |
There was a problem hiding this comment.
Accept commands from all connected peers
addPeer() still only sets the legacy remotePeerId to the first connected peer (multiplayerStore.ts 307-311), but Game.setupMultiplayerMessageHandler() rejects any command where command.playerId doesn’t match that single remotePeerId (Game.ts 170-176). In 3+ player lobbies, commands from any peer other than the first will be flagged as spoofed and dropped on every client, so those players’ actions never apply.
Useful? React with 👍 / 👎.
Update multiplayerStore.ts with multi-peer infrastructure:
Update Game.ts lockstep barrier for N players:
Update useMultiplayer.ts for multi-peer sync:
Delete dead code (~1400 lines):
https://claude.ai/code/session_01V7DKiAVN3pWV2NxZ7iYsjb