18 KiB
script.js Code Review Cleanup - Implementation Plan
For Claude: REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
Goal: Fix memory leaks, reduce console noise, eliminate code duplication, and improve rendering performance in the main frontend script.
Architecture: Targeted fixes to a single 2,310-line vanilla JS file. No module splitting, no architectural changes. Each task modifies static/script.js and can be tested by refreshing the browser.
Tech Stack: Vanilla JavaScript, served directly by FastAPI StaticFiles. No build system.
CRITICAL: Do NOT push to git until all tasks pass manual testing.
Task 1: Add DEBUG flag and debugLog helper
Files:
- Modify:
static/script.js:1-3(top of file)
Step 1: Add DEBUG flag and helper after first line
At the top of the file, after console.log('...') on line 1, add:
const DEBUG = false;
function debugLog(...args) { if (DEBUG) console.log(...args); }
Step 2: Replace all console.log calls with debugLog
Replace every console.log( with debugLog( throughout the file. There are ~35 instances.
Keep console.error( unchanged - those should always be visible.
Lines to change (all console.log → debugLog):
- Lines 100, 104, 106, 109 (chat button clicks)
- Lines 117, 121, 123, 126 (stats button clicks)
- Lines 134, 138, 140, 143 (inventory button clicks)
- Line 563 (heatmap loaded)
- Line 648 (portals loaded)
- Line 710 (portals rendered)
- Lines 729, 732, 739, 742, 786, 789 (stats window)
- Lines 824, 827, 834, 837, 1029, 1032 (inventory window)
- Line 1433 (server status)
- Lines 1511, 1515, 1733 (render cycle)
- Lines 1801, 1804, 1811, 1814, 1851, 1854 (chat window)
- Line 2173 (milestone)
Step 3: Verify
Open browser, load map page. Console should be clean (no log spam). Temporarily set DEBUG = true, refresh - logs should reappear.
Step 4: Commit
git add static/script.js
git commit -m "Add DEBUG flag and gate console.log behind debugLog helper"
Task 2: Add named constants for magic numbers
Files:
- Modify:
static/script.js:298-300(constants block) - Modify:
static/script.js:556(heatmap fetch) - Modify:
static/script.js:1448-1452(polling intervals) - Modify:
static/script.js:2069(notification duration) - Modify:
static/script.js:2148(glow duration)
Step 1: Add constants after existing POLL_MS definition (line 300)
const POLL_RARES_MS = 300000; // 5 minutes
const POLL_KILLS_MS = 300000; // 5 minutes
const POLL_HEALTH_MS = 30000; // 30 seconds
const NOTIFICATION_DURATION_MS = 6000; // Rare notification display time
const GLOW_DURATION_MS = 5000; // Player glow after rare find
const MAX_HEATMAP_POINTS = 50000;
const HEATMAP_HOURS = 24;
Step 2: Replace hardcoded values
Line 556 - heatmap fetch URL:
// Before:
const response = await fetch(`${API_BASE}/spawns/heatmap?hours=24&limit=50000`);
// After:
const response = await fetch(`${API_BASE}/spawns/heatmap?hours=${HEATMAP_HOURS}&limit=${MAX_HEATMAP_POINTS}`);
Lines 1448-1452 - polling intervals:
// Before:
setInterval(pollTotalRares, 300000);
setInterval(pollTotalKills, 300000);
setInterval(pollServerHealth, 30000);
// After:
setInterval(pollTotalRares, POLL_RARES_MS);
setInterval(pollTotalKills, POLL_KILLS_MS);
setInterval(pollServerHealth, POLL_HEALTH_MS);
Line 2069 - notification timeout:
// Before:
}, 6000);
// After:
}, NOTIFICATION_DURATION_MS);
Line 2148 - glow timeout:
// Before:
}, 5000);
// After:
}, GLOW_DURATION_MS);
Step 3: Verify
Refresh page. Players load, positions update every 2s. Heatmap toggle works. (These confirm intervals and heatmap constants are correct.)
Step 4: Commit
git add static/script.js
git commit -m "Replace magic numbers with named constants"
Task 3: Fix polling interval memory leak
Files:
- Modify:
static/script.js:1440-1453(startPolling function)
Step 1: Replace startPolling function
Replace the current startPolling() function (lines 1440-1453) with:
const pollIntervals = [];
function startPolling() {
// Clear any existing intervals first (prevents leak on re-init)
pollIntervals.forEach(id => clearInterval(id));
pollIntervals.length = 0;
// Initial fetches
pollLive();
pollTotalRares();
pollTotalKills();
pollServerHealth();
// Set up recurring polls
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));
}
Also remove the old pollID variable and the if (pollID !== null) return; guard since the new function handles re-initialization properly.
Step 2: Verify
Refresh page. Players load, update every 2s. Open DevTools > Performance Monitor - verify no interval accumulation. Refresh several times rapidly - should not see multiplied polling.
Step 3: Commit
git add static/script.js
git commit -m "Fix polling interval memory leak - store all interval IDs"
Task 4: Fix highlightRareFinder DOM query
Files:
- Modify:
static/script.js:2138-2151(highlightRareFinder function)
Step 1: Replace highlightRareFinder function
Replace lines 2138-2151 with:
function highlightRareFinder(characterName) {
// Use element pool for O(1) lookup instead of querySelectorAll
for (const item of elementPools.activeListItems) {
if (item.playerData && item.playerData.character_name === characterName) {
item.classList.add('rare-finder-glow');
setTimeout(() => {
item.classList.remove('rare-finder-glow');
}, GLOW_DURATION_MS);
break;
}
}
}
Step 2: Verify
This fires on rare notifications. If not easily testable live, verify by temporarily adding to console:
highlightRareFinder('SomeOnlinePlayerName');
Confirm the player glows briefly in the list.
Step 3: Commit
git add static/script.js
git commit -m "Fix highlightRareFinder to use element pool instead of DOM query"
Task 5: Extract createWindow helper
Files:
- Modify:
static/script.js- add helper before showStatsWindow (~line 725) - Modify:
static/script.js:728-794(showStatsWindow) - Modify:
static/script.js:823-1036(showInventoryWindow) - Modify:
static/script.js:1800-1858(showChatWindow)
Step 1: Add createWindow helper function
Insert before showStatsWindow (around line 725):
/**
* Create or show a draggable window. Returns { win, content, isNew }.
* If window already exists, brings it to front and returns isNew: false.
*/
function createWindow(id, title, className, options = {}) {
const { width = '400px', height = '300px', onClose } = options;
// Check if window already exists - bring to front
const existing = document.getElementById(id);
if (existing) {
existing.style.display = 'flex';
if (!window.__chatZ) window.__chatZ = 10000;
window.__chatZ += 1;
existing.style.zIndex = window.__chatZ;
return { win: existing, content: existing.querySelector('.window-content'), isNew: false };
}
// Create new window
if (!window.__chatZ) window.__chatZ = 10000;
window.__chatZ += 1;
const win = document.createElement('div');
win.id = id;
win.className = className;
win.style.display = 'flex';
win.style.zIndex = window.__chatZ;
win.style.width = width;
win.style.height = height;
const header = document.createElement('div');
header.className = 'window-header';
const titleSpan = document.createElement('span');
titleSpan.textContent = title;
header.appendChild(titleSpan);
const closeBtn = document.createElement('button');
closeBtn.textContent = '\u00D7';
closeBtn.addEventListener('click', () => {
win.style.display = 'none';
if (onClose) onClose();
});
header.appendChild(closeBtn);
const content = document.createElement('div');
content.className = 'window-content';
win.appendChild(header);
win.appendChild(content);
document.body.appendChild(win);
makeDraggable(win, header);
return { win, content, isNew: true };
}
Step 2: Refactor showStatsWindow to use createWindow
Replace the window creation boilerplate in showStatsWindow with:
function showStatsWindow(name) {
debugLog('showStatsWindow called for:', name);
const windowId = `statsWindow-${name}`;
const { content, isNew } = createWindow(
windowId, `Stats: ${name}`, 'stats-window',
{ width: '800px', height: '600px' }
);
if (!isNew) {
debugLog('Existing stats window found, showing it');
return;
}
// Stats-specific content (time range buttons + iframes)
// ... keep existing content creation logic from original function ...
}
Keep ALL the stats-specific logic (time range buttons, iframe creation, updateStatsTimeRange) - only remove the duplicated window setup code.
Step 3: Refactor showChatWindow to use createWindow
Same pattern - replace window boilerplate, keep chat-specific content (message list, input field, send button).
Step 4: Refactor showInventoryWindow to use createWindow
Same pattern - replace window boilerplate, keep inventory-specific content (grid, icon rendering, tooltips).
Step 5: Verify
Test each window type:
- Click a player's chat button - window opens, is draggable, close works
- Click a player's stats button - window opens, iframes load, time range works
- Click a player's inventory button - window opens, items display, tooltips work
- Open multiple windows - z-index stacking works (latest on top)
- Close and reopen each - brings to front correctly
Step 6: Commit
git add static/script.js
git commit -m "Extract createWindow helper to eliminate window creation duplication"
Task 6: Add requestAnimationFrame batching for pan/zoom
Files:
- Modify:
static/script.js- add scheduleViewUpdate near updateView (~line 1295) - Modify:
static/script.js:1908(wheel handler) - Modify:
static/script.js:1919(mousemove drag handler) - Modify:
static/script.js:1935(touchmove handler)
Step 1: Add scheduleViewUpdate after updateView function
After the updateView() function (line 1295), add:
let pendingFrame = null;
function scheduleViewUpdate() {
if (!pendingFrame) {
pendingFrame = requestAnimationFrame(() => {
updateView();
pendingFrame = null;
});
}
}
Step 2: Replace updateView() calls in interaction handlers
Line 1908 (wheel zoom):
// Before: updateView();
// After: scheduleViewUpdate();
Line 1919 (mousemove drag):
// Before: updateView();
// After: scheduleViewUpdate();
Line 1935 (touchmove):
// Before: updateView();
// After: scheduleViewUpdate();
Keep the direct updateView() calls at:
- Line 1301 (fitToWindow) - one-time call, not a hot path
- Line 1767 (selectPlayer) - one-time call on player click
Step 3: Verify
Pan the map by dragging - should feel smooth, no jitter. Zoom with scroll wheel - smooth. Toggle heatmap on, zoom/pan - heatmap updates correctly. Overall responsiveness should be same or better.
Step 4: Commit
git add static/script.js
git commit -m "Add requestAnimationFrame batching for pan/zoom updates"
Task 7: Optimize renderTrails
Files:
- Modify:
static/script.js:1737-1757(renderTrails function)
Step 1: Replace renderTrails function
Replace lines 1737-1757 with:
function renderTrails(trailData) {
trailsContainer.innerHTML = '';
// Build point strings directly - avoid intermediate arrays
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] = { points: `${x},${y}`, count: 1 };
else { byChar[key].points += ` ${x},${y}`; byChar[key].count++; }
}
for (const name in byChar) {
if (byChar[name].count < 2) continue;
const poly = document.createElementNS('http://www.w3.org/2000/svg', 'polyline');
poly.setAttribute('points', byChar[name].points);
poly.setAttribute('stroke', getColorFor(name));
poly.setAttribute('fill', 'none');
poly.setAttribute('class', 'trail-path');
trailsContainer.appendChild(poly);
}
}
Step 2: Verify
Refresh page. Player trails should render as colored lines following player positions. Verify trails match player colors. Multiple players should each have distinct trail lines.
Step 3: Commit
git add static/script.js
git commit -m "Optimize renderTrails to build SVG point strings directly"
Task 8: Remove redundant .slice() in renderList
Files:
- Modify:
static/script.js:1487(renderList function)
Step 1: Remove .slice()
Line 1487:
// Before:
const sorted = filtered.slice().sort(currentSort.comparator);
// After:
filtered.sort(currentSort.comparator);
const sorted = filtered;
Step 2: Verify
Refresh page. Player list should sort correctly. Click each sort button (name, KPH, kills, rares, total kills, KPR) - all should work correctly.
Step 3: Commit
git add static/script.js
git commit -m "Remove redundant .slice() before .sort() in renderList"
Task 9: Add centralized error handling
Files:
- Modify:
static/script.js- add handleError near top (~after debugLog) - Modify:
static/style.css- add .error-toast CSS - Modify:
static/script.js- update catch blocks throughout
Step 1: Add handleError function after debugLog
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(), GLOW_DURATION_MS);
}
}
Step 2: Add CSS for error toast
In static/style.css, add at the end:
/* Error Toast */
.error-toast {
position: fixed;
bottom: 20px;
right: 20px;
background: rgba(220, 38, 38, 0.9);
color: white;
padding: 12px 20px;
border-radius: 8px;
font-size: 13px;
z-index: 99999;
animation: toastFadeIn 0.3s ease;
max-width: 400px;
}
@keyframes toastFadeIn {
from { opacity: 0; transform: translateY(10px); }
to { opacity: 1; transform: translateY(0); }
}
Step 3: Update catch blocks
Background polling (showUI = false):
// pollLive catch (~line 1329):
// Before: console.error('Live or trails fetch failed:', e);
// After: handleError('Player update', e);
// pollTotalRares catch (~line 1339):
// Before: console.error('Total rares fetch failed:', e);
// After: handleError('Rare counter', e);
// pollTotalKills catch (~line 1358):
// Before: console.error('Total kills fetch failed:', e);
// After: handleError('Kill counter', e);
// pollServerHealth catch (~line 1375):
// Before: console.error('Server health fetch failed:', e);
// After: handleError('Server health', e);
User-triggered actions (showUI = true):
// showInventoryWindow catch (~line 1024):
// Before: content.innerHTML = `<div class="error">Failed to load...</div>`;
// After: handleError('Inventory', err, true);
// content.innerHTML = `<div class="error">Failed to load inventory</div>`;
// fetchHeatmapData catch (~line 565):
// Before: console.error('Heatmap fetch error:', err);
// After: handleError('Heatmap', err);
// fetchPortalData catch (~line 650):
// Before: (similar)
// After: handleError('Portals', err);
Step 4: Verify
- Normal operation - no toasts appear
- Temporarily break a URL (e.g., change API_BASE) - verify console.error appears with
[context]prefix - For user-triggered errors (inventory of offline player) - verify red toast appears bottom-right and auto-dismisses
Step 5: Commit
git add static/script.js static/style.css
git commit -m "Add centralized error handling with UI toast for user-facing errors"
Task 10: Full Manual Testing Pass
Files: None modified - testing only
CRITICAL: Do not push to git until ALL tests pass.
Test Checklist:
| # | Test | How to Verify | Status |
|---|---|---|---|
| 1 | Page load | Map renders, players appear, no console errors | |
| 2 | Live tracking | Players update positions every ~2 seconds | |
| 3 | Player filter | Type name in filter, list updates correctly | |
| 4 | Sort buttons | Each of 6 sort options works (name, KPH, kills, rares, total_kills, KPR) | |
| 5 | Player selection | Click player in list, map pans and zooms to them | |
| 6 | Pan/zoom | Mouse drag smooth, scroll wheel zoom smooth, no jitter or lag | |
| 7 | Heatmap | Toggle on - renders points. Pan/zoom - updates. Toggle off - clears | |
| 8 | Portals | Toggle on - portal icons appear. Toggle off - clears | |
| 9 | Chat window | Opens, draggable, close button works, receives WebSocket messages | |
| 10 | Stats window | Opens, iframes load, time range buttons work, draggable | |
| 11 | Inventory window | Opens, items display with icons, hover tooltip works, draggable | |
| 12 | Multiple windows | Open several windows - latest always on top (z-index) | |
| 13 | Console (DEBUG=false) | Console is clean, only errors/warnings visible | |
| 14 | Console (DEBUG=true) | Set DEBUG=true in code, refresh - verbose logs appear | |
| 15 | Page reload x5 | Reload 5 times rapidly - no interval accumulation, consistent behavior | |
| 16 | Trail rendering | Player trails visible as colored lines matching player dot colors | |
| 17 | Error handling | Briefly disconnect network - no crashes, console shows [context] errors |
If all pass: Ready to push. Inform user and await approval.
If any fail: Fix the specific issue, re-test that item, then re-run full checklist.