Add script.js code review and cleanup design document

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs/plans/2026-02-26-script-js-review-design.md

@@ -0,0 +1,206 @@
+# script.js Code Review & Performance Cleanup
+
+## Overview
+
+Comprehensive cleanup of `static/script.js` (2,310 lines, 54 functions) - the main frontend controller for the live player tracking map. Approach B: surgical fixes + performance pass without full refactor.
+
+**File:** `static/script.js` (+ minor CSS for error toast)
+**Risk:** Low - targeted changes only, no architectural restructuring
+**IMPORTANT:** Do NOT push to git until extensive manual testing is complete.
+
+---
+
+## Changes
+
+### 1. Bug Fixes
+
+#### 1a. Store All Polling Interval IDs (Memory Leak Fix)
+
+Currently only `pollID` is stored. The other 3 intervals (`pollTotalRares`, `pollTotalKills`, `pollServerHealth`) leak on page reload.
+
+**Fix:** Use a `pollIntervals` array, clear all before restarting.
+
+```javascript
+const pollIntervals = [];
+function startPolling() {
+    pollIntervals.forEach(id => clearInterval(id));
+    pollIntervals.length = 0;
+    pollIntervals.push(setInterval(pollLive, POLL_MS));
+    pollIntervals.push(setInterval(pollTotalRares, POLL_RARES_MS));
+    pollIntervals.push(setInterval(pollTotalKills, POLL_KILLS_MS));
+    pollIntervals.push(setInterval(pollServerHealth, POLL_HEALTH_MS));
+}
+```
+
+#### 1b. Fix `highlightRareFinder()` DOM Query
+
+Replace expensive `querySelectorAll` + string search with existing element pool lookup.
+
+```javascript
+// O(1) lookup from existing pool instead of O(n) DOM query
+for (const item of elementPools.activeListItems) {
+    if (item.playerData?.character_name === characterName) {
+        item.classList.add('rare-finder-glow');
+        setTimeout(() => item.classList.remove('rare-finder-glow'), GLOW_DURATION_MS);
+        break;
+    }
+}
+```
+
+---
+
+### 2. Console Logging & Constants Cleanup
+
+#### 2a. DEBUG Flag
+
+Gate all `console.log` calls behind a flag. Errors and warnings stay always-visible.
+
+```javascript
+const DEBUG = false;
+function debugLog(...args) { if (DEBUG) console.log(...args); }
+```
+
+Replace all `console.log(...)` with `debugLog(...)` throughout. Keep `console.error(...)` as-is.
+
+#### 2b. Named Constants
+
+Add to existing constants block:
+
+```javascript
+const POLL_RARES_MS = 300000;
+const POLL_KILLS_MS = 300000;
+const POLL_HEALTH_MS = 30000;
+const NOTIFICATION_DURATION_MS = 6000;
+const GLOW_DURATION_MS = 5000;
+const MAX_HEATMAP_POINTS = 50000;
+const HEATMAP_HOURS = 24;
+```
+
+Replace hardcoded values where they appear.
+
+---
+
+### 3. Extract Window Creation Helper
+
+Three functions (`showChatWindow`, `showStatsWindow`, `showInventoryWindow`) duplicate ~50 lines of window setup.
+
+**Extract `createWindow(id, title, className, options)`** that handles:
+- Existing window detection and bring-to-front
+- Z-index management
+- DOM structure (header, close button, content area)
+- `makeDraggable()` setup
+
+Each `show*Window()` function then contains only its unique content logic.
+
+---
+
+### 4. Performance Pass
+
+#### 4a. requestAnimationFrame for Pan/Zoom
+
+Batch `updateView()` calls during mousemove/wheel into animation frames.
+
+```javascript
+let pendingFrame = null;
+function scheduleViewUpdate() {
+    if (!pendingFrame) {
+        pendingFrame = requestAnimationFrame(() => {
+            updateView();
+            pendingFrame = null;
+        });
+    }
+}
+```
+
+Replace direct `updateView()` calls in mousemove and wheel handlers with `scheduleViewUpdate()`.
+
+#### 4b. Optimize renderTrails()
+
+Build SVG point strings directly instead of creating intermediate arrays:
+
+```javascript
+const byChar = {};
+for (const pt of trailData) {
+    const { x, y } = worldToPx(pt.ew, pt.ns);
+    const key = pt.character_name;
+    if (!byChar[key]) byChar[key] = `${x},${y}`;
+    else byChar[key] += ` ${x},${y}`;
+}
+for (const name in byChar) {
+    const poly = document.createElementNS(...);
+    poly.setAttribute('points', byChar[name]);
+    ...
+}
+```
+
+Eliminates: `Object.entries()` allocation, per-character `.map()`, per-point `.join()`.
+
+#### 4c. Remove Redundant .slice()
+
+```javascript
+// filtered is already a local copy from .filter(), no need to .slice()
+filtered.sort(currentSort.comparator);
+const sorted = filtered;
+```
+
+---
+
+### 5. Centralized Error Handling
+
+Add `handleError(context, error, showUI)` for consistent error logging. Background polling uses `showUI = false`, user-triggered actions use `showUI = true` with a brief toast notification.
+
+```javascript
+function handleError(context, error, showUI = false) {
+    console.error(`[${context}]`, error);
+    if (showUI) {
+        const msg = document.createElement('div');
+        msg.className = 'error-toast';
+        msg.textContent = `${context}: ${error.message || 'Unknown error'}`;
+        document.body.appendChild(msg);
+        setTimeout(() => msg.remove(), 5000);
+    }
+}
+```
+
+Requires minor CSS addition for `.error-toast` styling.
+
+---
+
+## Testing Plan
+
+**CRITICAL: All changes must be manually tested before any git push.**
+
+### Test Checklist
+1. **Page load** - Map renders, players appear, no console errors
+2. **Live tracking** - Players update positions every 2 seconds
+3. **Player list** - Filter by name works, all 6 sort options work
+4. **Player selection** - Click player in list, map centers on them
+5. **Pan/zoom** - Mouse drag panning smooth, scroll wheel zoom smooth, no jitter
+6. **Heatmap** - Toggle on/off, renders correctly, updates during pan/zoom
+7. **Portals** - Toggle on/off, portals appear at correct positions
+8. **Chat window** - Opens, receives messages via WebSocket, draggable
+9. **Stats window** - Opens, iframes load, time range buttons work, draggable
+10. **Inventory window** - Opens, items display, tooltip on hover, draggable
+11. **Rare notifications** - If testable, fireworks + player glow appear
+12. **Page reload** - No interval accumulation (check DevTools memory)
+13. **Console output** - Clean when DEBUG=false, verbose when DEBUG=true
+14. **Error scenarios** - Disconnect network briefly, verify graceful handling
+
+---
+
+## Files Modified
+
+| File | Changes |
+|------|---------|
+| `static/script.js` | All changes above |
+| `static/style.css` | Add `.error-toast` CSS |
+
+---
+
+## What's NOT Changing
+
+- File stays as single script.js (no module split)
+- WebSocket reconnection logic stays as-is (fixed 2s retry)
+- Element pooling system untouched (already optimized)
+- Rendering pipeline structure stays the same
+- No architectural changes