MosswartOverlord/docs/plans/2026-02-26-script-js-cleanup-plan.md
erik 3d0a0b33a3 Add script.js cleanup implementation plan
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-26 08:58:30 +00:00

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.logdebugLog):

  • 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:

  1. Click a player's chat button - window opens, is draggable, close works
  2. Click a player's stats button - window opens, iframes load, time range works
  3. Click a player's inventory button - window opens, items display, tooltips work
  4. Open multiple windows - z-index stacking works (latest on top)
  5. 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

  1. Normal operation - no toasts appear
  2. Temporarily break a URL (e.g., change API_BASE) - verify console.error appears with [context] prefix
  3. 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.