From 87bf7b7189b0ef08c024afd6f6871b1aa59f3e48 Mon Sep 17 00:00:00 2001 From: erik Date: Thu, 26 Feb 2026 08:55:06 +0000 Subject: [PATCH] Add script.js code review and cleanup design document Co-Authored-By: Claude Opus 4.6 --- .../2026-02-26-script-js-review-design.md | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 docs/plans/2026-02-26-script-js-review-design.md diff --git a/docs/plans/2026-02-26-script-js-review-design.md b/docs/plans/2026-02-26-script-js-review-design.md new file mode 100644 index 00000000..e16b3a51 --- /dev/null +++ b/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