battle-royal-tetris/IMPROVEMENTS.md

# 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