From aeacf9e68f4ce45fc992d7029cde0d9c4b97733d Mon Sep 17 00:00:00 2001 From: Josue Zamudio Date: Tue, 24 Mar 2026 20:46:34 +0000 Subject: [PATCH] Add performance improvements documentation and game-logic enhancements - Add IMPROVEMENTS.md with detailed analysis of performance issues and bugs - Update CLAUDE.md with negative Y overflow explanation - Update README.md with socket events documentation - Enhance game-logic.js with improved comments and validation - Improve server/index.js with better documentation and edge case handling --- CLAUDE.md | 11 +++ IMPROVEMENTS.md | 212 +++++++++++++++++++++++++++++++++++++++++++ README.md | 8 ++ server/game-logic.js | 39 +++++++- server/index.js | 70 +++++++++++++- 5 files changed, 335 insertions(+), 5 deletions(-) create mode 100644 IMPROVEMENTS.md diff --git a/CLAUDE.md b/CLAUDE.md index bb999d6..259ae89 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,17 @@ cd server && npm run test:coverage - `LOBBY_ROOM = 'global-lobby'` - Socket.io room for all players - Garbage rules: 2 lines cleared -> 1 garbage row, 3 -> 2, 4 (Tetris) -> 4 rows +### Piece Spawn & Negative Y Overflow + +Pieces spawn at `y = 0` (top of visible board). The coordinate system allows **negative Y values** as intentional overflow space: + +- **Negative Y allowed**: `isValidPosition()` only checks `newY >= BOARD_HEIGHT`, not `newY < 0` +- **Purpose**: When garbage is received, pieces are pushed up (y decreases) into negative territory +- **Player survival**: Player is NOT eliminated when piece goes negative — only when piece LOCKS with blocks in rows 0-1 +- **Recovery**: Piece drops naturally back into visible area; player can continue playing + +This design gives players a buffer zone to recover from garbage attacks before elimination. + ### Socket Events | Event | Direction | Payload | diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md new file mode 100644 index 0000000..5e795b7 --- /dev/null +++ b/IMPROVEMENTS.md @@ -0,0 +1,212 @@ +# Tetris Battle Royale - Performance Improvements and Bug Fixes + +This document outlines identified performance issues and bugs in the Tetris Battle Royale codebase, along with recommended fixes. + +--- + +## Performance Issues + +### 1. **Excessive JSON Serialization** (Critical) +**Location:** `server/index.js:577-592`, `public/js/game.js:354-370`, `renderer.js:83-149` + +**Problem:** The `getStates()` function uses `JSON.parse(JSON.stringify())` on every 50ms tick, serializing the entire game state for all players. With 8 players, this means serializing ~8 boards × 20 rows × 10 cols = 1600 cells per tick. + +**Impact:** High CPU usage, network bandwidth waste, potential lag with many players. + +**Fix:** Only send delta changes or use a more efficient serialization format. Consider: +- Sending only changed fields per player +- Batching multiple ticks into one message +- Using a binary format like Protocol Buffers or MessagePack + +--- + +### 2. **Client-Side Auto-Drop Conflicts** (Bug) +**Location:** `public/js/game.js:348-351` + +**Problem:** The client has its own `update(deltaTime)` function that auto-drops pieces independently: + +```javascript +if (!this.inputLocked && Date.now() - this.lastDrop >= this.dropInterval) { + this.drop(); + this.lastDrop = Date.now(); +} +``` + +**Bug:** This conflicts with server authority. The server drops every 50ms, but the client also tries to auto-drop based on `dropInterval`. This causes visual desync. + +**Fix:** Remove client-side auto-drop. Client should only render what server sends. + +--- + +### 3. **Ghost Piece Calculation Wasted** (Performance) +**Location:** `server/index.js:582` + +**Problem:** Server sends `ghostY` to all clients every tick, but clients recalculate it in `game.js:297-305` anyway. + +**Fix:** Remove redundant client-side ghost Y calculation. + +--- + +### 4. **Renderer Re-renders Every Frame** (Performance) +**Location:** `renderer.js:83-149` + +**Problem:** `renderPlayer()` is called every `requestAnimationFrame` (~60fps) even when state hasn't changed. It clears and redraws the entire canvas each time. + +**Fix:** Implement dirty checking - only re-render when state actually changes. Cache previous state and compare. + +--- + +### 5. **Duplicate Tetromino Definitions** (Maintenance Bug) +**Location:** `server/game-logic.js:8-16`, `public/js/game.js:8-16` + +**Problem:** Tetrominoes are defined in both server and client code. They can drift out of sync. + +**Fix:** Share definitions via a common module or sync from server on connection. + +--- + +## Bugs + +### 1. **Incorrect Elimination Logic on Client** (Critical Bug) +**Location:** `public/js/game.js:228-246` + +**Problem:** Client eliminates player when piece is partially above board: + +```javascript +let pieceAboveBoard = false; +for (let row = 0; row < this.currentPiece.shape.length; row++) { + for (let col = 0; col < this.currentPiece.shape[row].length; col++) { + if (this.currentPiece.shape[row][col]) { + const boardY = this.currentPiece.y + row; + if (boardY < 0) { + pieceAboveBoard = true; + break; + } + } + } +} +if (pieceAboveBoard) { + this.gameOver = true; + this.eliminated = true; +} +``` + +**Bug:** Server only eliminates when piece **LOCKS** with blocks in rows 0-1. Client eliminates as soon as any part goes negative, which is wrong. + +**Fix:** Remove client-side elimination logic. Trust server authority. + +--- + +### 2. **Garbage Row Insertion Order Wrong** (Bug) +**Location:** `public/js/game.js:311-320` + +**Problem:** +```javascript +this.board.pop(); // Remove bottom +this.board.unshift(garbageRow); // Add to top +``` + +**Bug:** This adds garbage to the TOP of the board instead of the bottom. Server correctly uses `addGarbageRow()` which shifts up and adds to bottom. + +**Fix:** Match server logic - shift up (pop top), add garbage to bottom. + +--- + +### 3. **Typo in Color Constant** (Bug) +**Location:** `public/js/game.js:19` + +```javascript +const GARbage_COLOR = '#666666'; // Should be GARBAGE_COLOR +``` + +**Fix:** Rename to `GARBAGE_COLOR`. + +--- + +### 4. **Hold Piece Sequence Index Desync** (Bug) +**Location:** `server/index.js:287-299` + +**Problem:** When client holds, server advances sequence index but client doesn't. Client uses random piece generator instead of shared queue. + +**Fix:** Client should track sequence index from server state and not generate pieces locally. + +--- + +### 5. **Input Lock Not Synced** (Bug) +**Location:** `public/js/game.js:39-41` + +**Problem:** Client has `inputLocked` and `applyDelayHit()` but server never sends this state. Client auto-drop can interfere with server ticks. + +**Fix:** Remove client-side input lock or implement server-side delay hit mechanism. + +--- + +### 6. **Spectator Cannot Rejoin as Player** (UX Bug) +**Location:** `server/index.js:613-638` + +**Problem:** After game over, spectators are converted to players with reset state, but they don't get sent back to lobby screen to rejoin. + +**Fix:** After game over, emit event to switch all clients back to lobby screen. + +--- + +## Implementation Priority + +### Priority 1 (Critical - Fix Immediately) +1. **Fix garbage row insertion** in client (`game.js:311-320`) + - Change `unshift` to `push`, remove `pop` +2. **Fix elimination logic** to match server authority + - Remove client-side `pieceAboveBoard` check in `lockPiece()` +3. **Remove client-side auto-drop** to prevent desync + - Remove `update()` method's auto-drop logic in `game.js` + +### Priority 2 (Performance - High Impact) +4. **Reduce JSON serialization frequency** + - Batch state updates or send only changed fields +5. **Implement dirty checking in renderer** + - Only re-render when state actually changes +6. **Share tetromino definitions** + - Move to common module or sync from server + +### Priority 3 (Bugs - Medium Impact) +7. **Fix typo** `GARbage_COLOR` → `GARBAGE_COLOR` +8. **Sync hold piece sequence index** between client/server +9. **Fix spectator rejoin flow** after game over + - Emit lobby reset event to all clients + +--- + +## Testing Strategy + +After making changes: + +1. **Unit tests:** Update existing tests to verify fixes +2. **Integration tests:** Test multi-player scenarios +3. **Manual testing:** + - Join with 2-8 browser tabs + - Test garbage reception and elimination + - Test hold mechanic + - Test spectator mode and rejoin + +--- + +## Files to Modify + +| File | Changes Needed | Priority | +|------|---------------|----------| +| `public/js/game.js` | Fix garbage insertion, remove client elimination, remove auto-drop | P1 | +| `public/js/game.js` | Fix typo `GARbage_COLOR` | P3 | +| `public/js/renderer.js` | Add dirty checking | P2 | +| `server/index.js` | Fix spectator rejoin flow | P3 | +| `public/js/` | Create shared tetromino module | P2 | +| `server/index.js` | Optimize state serialization | P2 | + +--- + +## Notes + +- Server is authoritative - client should never make game logic decisions +- All game state changes should originate from server +- Client is purely for rendering and input collection +- 50ms tick rate is good for real-time gameplay +- Current test suite (105 tests) provides good coverage for regression testing diff --git a/README.md b/README.md index 6abc9ad..036dc94 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,14 @@ When you clear rows, garbage rows are sent to random opponents: - 3 lines cleared → 2 garbage rows sent - 4 lines (Tetris) → 4 garbage rows sent +### Garbage & Elimination + +When you receive garbage, your current piece is pushed upward. Pieces can temporarily exist **above the visible board** (negative Y) — you're not eliminated yet! The piece will drop back down naturally. + +**You're only eliminated when:** +- A piece locks (is placed) with blocks in the top 2 rows (rows 0-1) +- This happens if too much garbage accumulates and you can't clear rows fast enough + The last player standing wins! ## Setup diff --git a/server/game-logic.js b/server/game-logic.js index 6a4d27f..c446304 100644 --- a/server/game-logic.js +++ b/server/game-logic.js @@ -49,6 +49,9 @@ function getPieceFromType(pieceType) { * @param {number[][]} board - The game board * @param {number[][]} shape - Optional custom shape (for rotation) * @returns {boolean} True if position is valid + * + * Note: Negative Y is allowed (overflow/spawn area above visible board). + * This allows pieces to be pushed up by garbage without immediate elimination. */ function isValidPosition(piece, x, y, board, shape = null) { const pieceShape = shape || piece.shape; @@ -59,11 +62,14 @@ function isValidPosition(piece, x, y, board, shape = null) { const newY = y + row; // Check bounds + // Note: Only checks if piece goes BELOW board (newY >= 20) + // Negative Y is allowed - pieces can exist above visible board if (newX < 0 || newX >= BOARD_WIDTH || newY >= BOARD_HEIGHT) { return false; } // Check collision with locked pieces (only if on board) + // Note: Only checks collision if piece is in visible area (newY >= 0) if (newY >= 0 && board[newY][newX]) { return false; } @@ -93,6 +99,14 @@ function getGhostY(piece, board) { * Check if board has overflowed (blocks in top 2 rows) * @param {number[][]} board - The game board * @returns {boolean} True if board has overflowed + * + * Checks rows 0 AND 1 because: + * - I piece at y=0 only places blocks in row 1 (unique shape) + * - T piece at y=0 also only fills row 1 + * - If we only checked row 0, these pieces would NOT trigger elimination + * + * This is called when a piece LOCKS, not when garbage is received. + * Player is eliminated only when they lock a piece that overflows. */ function checkBoardOverflow(board) { for (let row = 0; row < 2; row++) { @@ -110,18 +124,30 @@ function checkBoardOverflow(board) { * @param {number[][]} board - The game board * @param {object} currentPiece - Current falling piece (may be null) * @returns {object} Object with new board, updated piece, and gap position + * + * When garbage is added: + * - All board rows shift up by 1 (top row removed) + * - New garbage row added at bottom with one random gap + * - Current piece y position decreases by 1 (pushed up) + * + * Note: Piece y can become negative (above visible board). This is intentional + * - Player is NOT eliminated when piece goes negative + * - Piece will drop naturally back into visible area + * - Elimination only occurs when piece LOCKS with blocks in rows 0-1 */ function addGarbageRow(board, currentPiece) { - // Remove top row + // Remove top row (all rows shift up) const newBoard = board.slice(1); - // Add garbage row with random gap + // Add garbage row with random gap at bottom const garbageRow = Array(BOARD_WIDTH).fill(GARBAGE_COLOR); const gap = Math.floor(Math.random() * BOARD_WIDTH); garbageRow[gap] = 0; newBoard.push(garbageRow); // Push current piece up by 1 row + // Note: y can become negative (above visible board) - this is intentional + // Player survives and piece will drop naturally back into view let updatedPiece = currentPiece; if (currentPiece) { updatedPiece = { @@ -138,6 +164,13 @@ function addGarbageRow(board, currentPiece) { * @param {object} piece - The piece to lock * @param {number[][]} board - The game board * @returns {number[][]} New board with piece locked + * + * Only locks blocks that are within visible board bounds [0, 20). + * Blocks at negative Y (above board) are clipped and not locked. + * + * Example: I piece at y=-2 has blocks at rows -2,-1,0,1 + * - Only blocks at rows 0 and 1 get locked to board + * - Blocks at rows -2 and -1 are clipped (discarded) */ function lockPieceToBoard(piece, board) { const newBoard = board.map(row => [...row]); @@ -148,6 +181,8 @@ function lockPieceToBoard(piece, board) { const boardY = piece.y + row; const boardX = piece.x + col; + // Only lock blocks within visible board bounds + // Blocks above board (negative Y) are clipped if (boardY >= 0 && boardY < BOARD_HEIGHT && boardX >= 0 && boardX < BOARD_WIDTH) { newBoard[boardY][boardX] = piece.color; } diff --git a/server/index.js b/server/index.js index 3243621..bde5d44 100644 --- a/server/index.js +++ b/server/index.js @@ -1,3 +1,12 @@ +/** + * Tetris Battle Royale Server + * + * Express + Socket.io server that handles all game logic authoritatively. + * Manages a single global lobby with 2-8 players competing in real-time. + * + * @module server + */ + const express = require('express'); const http = require('http'); const { Server } = require('socket.io'); @@ -29,7 +38,42 @@ const io = new Server(server); // Serve static files app.use(express.static(path.join(__dirname, '../public'))); -// Single global lobby +/** + * @typedef {Object} Player + * @property {string} id - Socket.io connection ID + * @property {string} name - Player's display name + * @property {number} score - Current score + * @property {number} lines - Total lines cleared + * @property {number} level - Current level (affects drop speed) + * @property {number[][]} board - 20x10 game board + * @property {object|null} currentPiece - Currently falling piece + * @property {object|null} nextPiece - Next piece to spawn + * @property {object|null} holdPiece - Held piece (for hold mechanic) + * @property {boolean} canHold - Whether hold is available this turn + * @property {boolean} eliminated - Whether player has been eliminated + * @property {boolean} ready - Whether player is ready to start + * @property {number} dropCounter - Frame counter for auto-drop + * @property {number} dropInterval - Milliseconds between auto-drops + * @property {object[]} garbageReceived - History of garbage received + */ + +/** + * @typedef {Object} Spectator + * @property {string} id - Socket.io connection ID + * @property {string} name - Spectator's display name + */ + +/** + * Global lobby state - holds all game state server-side + * + * @type {Object} + * @property {Map} players - Active players (socketId -> Player) + * @property {Map} spectators - Spectators (socketId -> Spectator) + * @property {boolean} gameStarted - Whether game is currently running + * @property {NodeJS.Timeout|null} gameInterval - setInterval reference for game tick + * @property {string[]} pieceQueue - Shared piece queue (7-bag system) + * @property {Map} playerSequenceIndex - Each player's position in piece queue + */ const lobby = { players: new Map(), spectators: new Map(), @@ -45,7 +89,12 @@ const LOBBY_ROOM = 'global-lobby'; io.on('connection', (socket) => { console.log('Player connected:', socket.id); - // Join global lobby + /** + * Handle player joining the lobby + * @event join-lobby + * @param {object} data - Event data + * @param {string} data.playerName - Name of the player joining + */ socket.on('join-lobby', ({ playerName }) => { // Add socket to lobby room so they receive lobby events socket.join(LOBBY_ROOM); @@ -103,6 +152,10 @@ io.on('connection', (socket) => { console.log(`${playerName} joined global lobby (${lobby.players.size} players)`); }); + /** + * Handle player marking themselves as ready + * @event ready + */ socket.on('ready', () => { const player = lobby.players.get(socket.id); if (!player) return; @@ -124,6 +177,10 @@ io.on('connection', (socket) => { } }); + /** + * Handle player marking themselves as not ready + * @event unready + */ socket.on('unready', () => { const player = lobby.players.get(socket.id); if (!player) return; @@ -137,6 +194,13 @@ io.on('connection', (socket) => { }); }); + /** + * Handle player moving piece left or right + * @event player-move + * @param {object} data - Event data + * @param {string} data.playerId - ID of the player + * @param {'left'|'right'} data.direction - Direction to move + */ socket.on('player-move', ({ playerId, direction }) => { if (!lobby.gameStarted) return; @@ -590,4 +654,4 @@ function checkGameOver() { server.listen(PORT, () => { console.log(`Tetris Battle Royale server running on port ${PORT}`); console.log(`Open http://localhost:${PORT} in 2-8 browser tabs to play!`); -}); +}); \ No newline at end of file