Audit src/index/ and src/index.ts comments (#225 Phase 2)

src/index.ts

@@ -49,8 +49,7 @@ import { initMapStorage } from "./lib/map-storage.ts";
 import { PrefabInspectorHandler } from "./index/prefab-inspector-handler.ts";
 import { installPrefabLinkTooltip } from "./lib/prefab-link-tooltip.ts";
 
-// Fetched once and shared via Promise so multiple handlers awaiting the
-// same JSON do not issue duplicate requests.
+// WHY: fetched once and shared via Promise so multiple handlers awaiting the same JSON do not issue duplicate requests.
 const prefabMeshSizes: Promise<PrefabMeshSizes> = fetchJson(
   "prefab-mesh-sizes.json",
 );
@@ -83,8 +82,7 @@ function main() {
     .catch(printError);
 
   initMapStorage();
-  // Restore stored input values first so downstream init steps and handlers
-  // observe the restored state instead of HTML defaults.
+  // WHY: restore stored input values first so downstream init steps and handlers observe the restored state instead of HTML defaults.
   rememberValue.init();
   presetButton.init();
   copyButton.init();
@@ -199,8 +197,7 @@ function main() {
     },
     fileHandler,
   );
-  // Never appended to the DOM: the worker draws the sign/marker-free composite
-  // into it and the terrain viewer samples the placeholder as its texture.
+  // WHY: never appended to the DOM. The worker draws the sign/marker-free composite into it and the terrain viewer samples the placeholder as its texture.
   const mapCompositeCanvas = document.createElement("canvas");
   new MapCanvasHandler(
     {
@@ -224,8 +221,7 @@ function main() {
     markerStore,
     fileHandler,
   );
-  // One controller owns the shared #prefab-tooltip DOM for both the 2D map
-  // hover and the terrain viewer hover.
+  // WHY: one controller owns the shared #prefab-tooltip DOM for both the 2D map hover and the terrain viewer hover.
   const prefabTooltipController = new PrefabTooltipController(
     component("prefab-tooltip", HTMLElement),
     labelHandler,

src/index/controller-highlight.ts

@@ -1,8 +1,11 @@
 export const HIGHLIGHT_CLASS = "reset-target-highlight";
 
-// Mark each target's closest <tr> or <li> ancestor, plus any collapsed
-// <details> ancestor so the cue stays visible when the section is folded.
-// Targets without any of these ancestors (e.g. inside a dialog) are skipped.
+/**
+ * Marks each target's closest `<tr>` or `<li>` ancestor plus any
+ * collapsed `<details>` ancestor so the cue stays visible when the
+ * section is folded. Targets without any of these ancestors (e.g.
+ * inside a dialog) are skipped.
+ */
 export function setHighlightFor(
   targets: Iterable<Element>,
   on: boolean,

src/index/cursor-coords-display-handler.ts

@@ -11,9 +11,11 @@ interface Doms {
 
 const EMPTY = "E/W: -, N/S: -, Elev: -";
 
-// Sink for cursor events that renders the coords/elevation line. Elevation is
-// resolved here (not in CursorHandler) because it is asynchronous and not
-// needed by every subscriber.
+/**
+ * Sink for cursor events that renders the coords / elevation line.
+ * Elevation is resolved here rather than in `CursorHandler` because it
+ * is asynchronous and not needed by every subscriber.
+ */
 export class CursorCoordsDisplayHandler {
   #doms: Doms;
   #dtmHandler: DtmHandler;

src/index/cursor-handler.ts

@@ -11,17 +11,22 @@ interface Doms {
 }
 
 export interface CursorEventMessage {
-  // null when the cursor left the canvas.
+  /** `null` when the cursor left the canvas. */
   event: MouseEvent | null;
-  // null when the cursor is outside the map area or when the map size is not
-  // yet known (no map_info.xml loaded).
+  /**
+   * `null` when the cursor is outside the map area, or when the map
+   * size is not yet known (no `map_info.xml` loaded).
+   */
   coords: GameCoords | null;
 }
 
-// Streams "what is under the cursor" events to any number of subscribers
-// (coords/elevation display, prefab tooltip, …). Holding the single source
-// here avoids per-feature `mousemove` listeners and lets the canvas-to-game
-// coord conversion happen once per cursor sample.
+/**
+ * Streams "what is under the cursor" events to any number of
+ * subscribers (coords/elevation display, prefab tooltip, and so on).
+ * Holding the single source here avoids per-feature `mousemove`
+ * listeners and lets the canvas-to-game coord conversion happen once
+ * per cursor sample.
+ */
 export class CursorHandler {
   #doms: Doms;
   #dtmHandler: DtmHandler;
@@ -46,10 +51,13 @@ export class CursorHandler {
     });
   }
 
-  // Source-side debounce keeps the dispatch rate near rAF so that even on a
-  // fast-moving cursor we don't fire dozens of subscriber chains per frame.
-  // Subscribers are free to layer their own throttle on top when their work
-  // (DOM writes, async fetches) is more expensive than the dispatch itself.
+  /**
+   * Source-side debounce keeps the dispatch rate near rAF so even a
+   * fast-moving cursor does not fire dozens of subscriber chains per
+   * frame. Subscribers can layer their own throttle on top when their
+   * work (DOM writes, async fetches) is more expensive than the
+   * dispatch itself.
+   */
   #dispatch = throttledInvoker(() => this.#dispatchImmediately(), 16);
 
   async #dispatchImmediately() {

src/index/dialog-handler.ts

@@ -57,8 +57,11 @@ export class DialogHandler {
 const TERMINATED_STATES = ["completed", "skipped"] as const;
 
 export class FileProgressionIndicator {
-  // Direct name -> <li> lookup. Avoids scanning by textContent, which is
-  // fragile under i18n, whitespace differences, or duplicate task names.
+  /**
+   * Direct name-to-`<li>` lookup. Avoids scanning by `textContent`,
+   * which is fragile under i18n, whitespace differences, or duplicate
+   * task names.
+   */
   #liByName = new Map<string, HTMLLIElement>();
   #liList: HTMLLIElement[] = [];
 
@@ -68,8 +71,7 @@ export class FileProgressionIndicator {
       li.textContent = taskName;
       li.classList.add("processing");
       this.#liList.push(li);
-      // First occurrence wins so setState is deterministic when duplicate
-      // task names appear.
+      // INVARIANT: the first occurrence wins so setState is deterministic when duplicate task names appear.
       if (!this.#liByName.has(taskName)) this.#liByName.set(taskName, li);
     }
   }

src/index/dnd-handler.ts

@@ -9,13 +9,14 @@ interface Doms {
   dragovered: HTMLElement;
 }
 
-// Tracks nested dragenter/dragleave pairs so we only react to the outermost
-// boundary crossings. dragenter/dragleave fire for every child element the
-// cursor crosses, so a naive close() on dragleave would fire while the cursor
-// is still over the page. By incrementing on enter and decrementing on leave,
-// the counter returns to 0 only when the cursor truly leaves the target. The
-// previous implementation relied on the clientX/Y === 0 heuristic, which is
-// unreliable across browsers and only matched the window's top-left corner.
+/**
+ * Tracks nested dragenter/dragleave pairs so we only react to the
+ * outermost boundary crossings. `dragenter` and `dragleave` fire for
+ * every child element the cursor crosses, so a naive `close()` on
+ * `dragleave` would fire while the cursor is still over the page.
+ * Incrementing on enter and decrementing on leave returns the counter
+ * to 0 only when the cursor truly leaves the target.
+ */
 export interface DragCounter {
   enter(): boolean;
   leave(): boolean;
@@ -71,8 +72,7 @@ export class DndHandler {
     dom.dragovered.addEventListener("drop", (event) => {
       if (!event.dataTransfer?.types.includes("Files")) return;
       event.preventDefault();
-      // drop consumes the dragenter without firing a matching dragleave,
-      // so reset the counter to keep it consistent for the next drag.
+      // WHY: drop consumes the dragenter without firing a matching dragleave, so reset the counter to keep it consistent for the next drag.
       counter.reset();
       this.#listeners.dispatchNoAwait({
         files: Array.from(event.dataTransfer.items).flatMap((item) =>

src/index/dtm-handler.ts

@@ -15,9 +15,7 @@ export class DtmHandler {
   #dtmRaw: CacheHolder<Uint8Array | null>;
   #mapSize = new CacheHolder<GameMapSize | null>(
     () => getHightMapSize(),
-    () => {
-      // Do nothing
-    },
+    () => {},
   );
   #listeners = new events.ListenerManager<EventMessage>();
 
@@ -34,9 +32,7 @@ export class DtmHandler {
           return null;
         }
       },
-      () => {
-        // Do nothing
-      },
+      () => {},
     );
 
     fileHandler.addListener(async (fileNames) => {
@@ -103,16 +99,17 @@ export class Dtm {
       return null;
     }
 
-    // In-game coords with left-top offset
     const x = Math.floor(width / 2) + coords.x;
     const z = Math.floor(height / 2) + coords.z;
     const elev = this.#data[x + z * width];
     return elev ?? null;
   }
 
-  // `geo` must already be rotated into the XZ ground plane (+Y normal),
-  // i.e. `geo.rotateX(-Math.PI / 2)` has been applied to a fresh
-  // PlaneGeometry before calling this.
+  /**
+   * `geo` must already be rotated into the XZ ground plane (+Y normal).
+   * Callers should apply `geo.rotateX(-Math.PI / 2)` to a fresh
+   * `PlaneGeometry` before calling this.
+   */
   writeY(geo: three.PlaneGeometry) {
     if (!this.#data) return;
 
@@ -145,25 +142,20 @@ export class Dtm {
     const maxX = width - 1;
     const maxZ = height - 1;
 
-    // pos.array layout is interleaved (x, y, z) per vertex. Bypassing
-    // getX/getY/setZ avoids per-call overhead that is significant across
-    // ~4M vertices.
+    // WHY: pos.array is interleaved (x, y, z) per vertex. Bypassing getX/getY/setZ avoids per-call overhead that is significant across roughly 4M vertices.
     const arr = pos.array as Float32Array;
     for (let i = 0, j = 0; i < pos.count; i++, j += 3) {
-      // game axis -> webgl axis: game x -> x, game z(row) -> -z, elevation -> y
+      // WHY: game axis to webgl axis: game x maps to x, game z (row) maps to -z, elevation maps to y.
       const px = arr[j] as number;
       const pz = arr[j + 2] as number;
       let dataX = Math.round((px + halfW) * scaleFactor);
       let dataZ = Math.round((halfH - pz) * scaleFactor);
-      // Plane vertices on the +width/+height edge round to width/height,
-      // one past the last valid DTM index. Clamp to keep sampling in range.
+      // WHY: plane vertices on the +width or +height edge round to width or height, one past the last valid DTM index. Clamp to keep sampling in range.
       if (dataX < 0) dataX = 0;
       else if (dataX > maxX) dataX = maxX;
       if (dataZ < 0) dataZ = 0;
       else if (dataZ > maxZ) dataZ = maxZ;
-      // Elevation byte is in game meters; divide by scaleFactor to convert
-      // to the geometry's local units (which are already shrunk by the same
-      // ratio horizontally).
+      // WHY: elevation byte is in game meters. Divide by scaleFactor to convert to the geometry's local units, which are already shrunk by the same ratio horizontally.
       // deno-lint-ignore no-non-null-assertion
       arr[j + 1] = this.#data[dataX + dataZ * width]! / scaleFactor;
     }

src/index/file-handler.ts

@@ -89,9 +89,7 @@ export class FileHandler {
     doms.files.addEventListener("change", () => {
       if (!doms.files.files) return;
       const files = Array.from(doms.files.files);
-      // webkitdirectory selections expose the chosen directory via the
-      // first file's webkitRelativePath ("<dir>/<file>"); use it so the
-      // Map Name row gets the same immediate feedback as drag-and-drop.
+      // WHY: webkitdirectory selections expose the chosen directory via the first file's webkitRelativePath ("<dir>/<file>"). Use it so the Map Name row gets the same immediate feedback as drag-and-drop.
       const dir = files[0]?.webkitRelativePath.split("/")[0];
       if (dir) this.#setMapName(dir);
       this.#pushFiles(files).catch(printError);
@@ -106,7 +104,7 @@ export class FileHandler {
       this.#setMapName(mapName);
       await this.#pushUrls(
         Array.from(MAP_FILE_NAMES).map((name) => `${mapDir}/${name}`),
-        // Bundled world files are preprocessed. See tools/copy-map-files.ts
+        // WHY: bundled world files are preprocessed by tools/copy-map-files.ts, so no further preprocessing is needed.
         true,
       );
     });
@@ -200,7 +198,6 @@ export class FileHandler {
       throw new Error("Already processing");
     }
     this.#dialogHandler.state = "processing";
-    // Prevent opening the dialog because the dialog will be closed immediately.
     let progression = null;
     if (resourceList.some((r) => !("remove" in r))) {
       progression = this.#dialogHandler.createProgression(

src/index/map-canvas-handler.ts

@@ -6,8 +6,10 @@ import type { Prefab } from "../types/7dtdmap.ts";
 
 interface Doms {
   canvas: HTMLCanvasElement;
-  // Off-DOM mirror of the sign/marker-free composite; its placeholder element
-  // is sampled by the terrain viewer as a texture.
+  /**
+   * Off-DOM mirror of the sign/marker-free composite. Its placeholder
+   * element is sampled by the terrain viewer as a texture.
+   */
   compositeCanvas: HTMLCanvasElement;
   biomesAlpha: HTMLInputElement;
   splat3Alpha: HTMLInputElement;

src/index/marker-coords-display-handler.ts

@@ -14,9 +14,11 @@ interface Doms {
 
 const EMPTY = "E/W: -, N/S: -, Elev: -";
 
-// Hit index is sourced from PrefabsHandler so cursor hover and flag drop
-// agree on "which prefab is here" without each consumer doing its own
-// bookkeeping over PrefabMeshSizes.
+/**
+ * Sources the hit index from `PrefabsHandler` so cursor hover and flag
+ * drop agree on "which prefab is here" without each consumer doing
+ * its own bookkeeping over `PrefabMeshSizes`.
+ */
 export class MarkerCoordsDisplayHandler {
   #doms: Doms;
   #dtmHandler: DtmHandler;

src/index/marker-handler.ts

@@ -10,9 +10,12 @@ interface Doms {
   resetMarker: HTMLButtonElement;
 }
 
-// 2D map input adapter: turns canvas clicks/dblclicks, the reset button, and
-// file loads into MarkerStore updates. The store owns the coordinate state and
-// broadcasts to subscribers; this class only translates DOM events into calls.
+/**
+ * 2D map input adapter. Turns canvas clicks / dblclicks, the reset
+ * button, and file loads into `MarkerStore` updates. The store owns
+ * the coordinate state and broadcasts to subscribers, so this class
+ * only translates DOM events into calls.
+ */
 export class MarkerHandler {
   constructor(
     doms: Doms,
@@ -29,8 +32,7 @@ export class MarkerHandler {
     };
 
     doms.canvas.addEventListener("click", (e) => {
-      // Shift+Click is reserved for prefab page navigation in
-      // PrefabTooltipHandler; the two click roles are mutually exclusive.
+      // WHY: Shift+Click is reserved for prefab page navigation in PrefabTooltipHandler. The two click roles are mutually exclusive.
       if (e.shiftKey) return;
       setFromEvent(e).catch(printError);
     });

src/index/marker-jump-handler.ts

@@ -63,8 +63,7 @@ export class MarkerJumpHandler {
   async #jump(): Promise<void> {
     const coords = this.#store.coords;
     if (!coords) return;
-    // On touch devices the click arrives without a preceding mouseenter, so
-    // position the highlight ourselves before scrolling to it.
+    // WHY: on touch devices the click arrives without a preceding mouseenter, so position the highlight ourselves before scrolling to it.
     await this.#show(coords);
     if (!this.#highlight.visible) return;
     const reduce = matchMedia("(prefers-reduced-motion: reduce)").matches;

src/index/marker-store.ts

@@ -5,9 +5,11 @@ interface EventMessage {
   coords: GameCoords | null;
 }
 
-// Domain state for the map's flag marker. Kept DOM-free so both the 2D canvas
-// input adapter (MarkerHandler) and other input sources (e.g. the terrain
-// viewer's raycast) can drive the same broadcast.
+/**
+ * Domain state for the map's flag marker. Kept DOM-free so both the
+ * 2D canvas input adapter (`MarkerHandler`) and other input sources
+ * (e.g. the terrain viewer's raycast) can drive the same broadcast.
+ */
 export class MarkerStore {
   #coords: GameCoords | null = null;
   #listeners = new events.ListenerManager<EventMessage>();

src/index/prefab-highlight-handler.ts

@@ -37,8 +37,7 @@ export class PrefabHighlightHandler {
     this.#meshSizes = meshSizes;
     this.#highlight = highlight;
 
-    // Nested block rows have no data-x, so closest() resolves them to their
-    // parent prefab row.
+    // WHY: nested block rows have no data-x, so closest() resolves them to their parent prefab row.
     doms.list.addEventListener("mouseover", (event) => {
       const li = coordsLi(event.target);
       if (li === this.#hoveredLi) return;
@@ -61,8 +60,7 @@ export class PrefabHighlightHandler {
       if (!li) return;
       this.#jump(li).catch(printError);
     });
-    // A filter update can replace the hovered row without a mouseout; drop
-    // the highlight instead of leaving it pinned to a stale position.
+    // WHY: a filter update can replace the hovered row without a mouseout, so drop the highlight instead of leaving it pinned to a stale position.
     prefabsHandler.addFilterHeaderListener(() => {
       this.#hoveredLi = null;
       this.#hide();
@@ -71,7 +69,7 @@ export class PrefabHighlightHandler {
 
   async #show(li: HTMLElement): Promise<void> {
     const rect = await this.#computeRect(li);
-    // The hover may have moved on during the awaits.
+    // WHY: the hover may have moved on during the awaits, so re-check identity before applying.
     if (this.#hoveredLi !== li) return;
     if (!rect) {
       this.#hide();
@@ -85,8 +83,7 @@ export class PrefabHighlightHandler {
   }
 
   async #jump(li: HTMLElement): Promise<void> {
-    // On touch devices the click arrives without a preceding mouseover, so
-    // position the highlight ourselves before scrolling to it.
+    // WHY: on touch devices the click arrives without a preceding mouseover, so position the highlight ourselves before scrolling to it.
     this.#hoveredLi = li;
     await this.#show(li);
     if (!this.#highlight.visible) return;