From bcee2e8610ceda0f93ba16978fb61df39e7f149d Mon Sep 17 00:00:00 2001 From: Kris Anderson Date: Thu, 9 Jul 2026 16:27:56 -0400 Subject: [PATCH] fix(highway_3d): _bgPanelKey rejects non-integer panel index; JSDoc bridge fns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _bgPanelKey() treated any non-null panelIndexFor() return as a valid panel id, so a NaN/non-finite index minted a bogus "panelNaN" localStorage key instead of falling back to "main". Gate on Number.isInteger(idx) && idx >= 0. (The camera path is already NaN-safe — panelsMap[NaN] misses and falls through.) - Add a NaN/negative-index case to the resolver tests (drum 22, keys 57, pass). - Convert the camera-bridge helpers' comments to JSDoc (_bgPanelKey, _freeCamFor, _resolveFreeCam, _ssApi across the three plugins) to lift docstring coverage on the changed surface. Comment/robustness only; no behavior change beyond the NaN guard. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Kris Anderson --- plugins/drum_highway_3d/screen.js | 23 +++++++++---- .../tests/camera_bridge.test.js | 7 ++++ plugins/highway_3d/screen.js | 32 +++++++++++++------ plugins/keys_highway_3d/screen.js | 23 +++++++++---- .../tests/camera_bridge.test.js | 7 ++++ 5 files changed, 68 insertions(+), 24 deletions(-) diff --git a/plugins/drum_highway_3d/screen.js b/plugins/drum_highway_3d/screen.js index d2daac0..43b948d 100644 --- a/plugins/drum_highway_3d/screen.js +++ b/plugins/drum_highway_3d/screen.js @@ -1365,15 +1365,24 @@ * Camera Director bridge resolver (pure — exported via createFactory.__test) * ====================================================================== */ - // Resolve the active splitscreen API, defensive on the global-name rename in - // flight (feedBackSplitscreen is canonical; slopsmithSplitscreen is the legacy - // alias). Returns null when splitscreen isn't present. + /** + * The active splitscreen API, defensive on the global-name rename in flight + * (feedBackSplitscreen is canonical; slopsmithSplitscreen is the legacy alias). + * @returns {object|null} the splitscreen API, or null when not present + */ function _ssApi() { return window.feedBackSplitscreen || window.slopsmithSplitscreen || null; } - // Given the splitscreen API, THIS window's per-panel camera map, and the - // global camera, return this panel's camera under splitscreen, else the - // global, else null (Camera Director absent → stock framing). Throw-safe on - // panelIndexFor so a misbehaving splitscreen build can't break framing. + /** + * Resolve the Camera Director camera for a canvas: this panel's camera under + * splitscreen, else the global, else null (Camera Director absent → stock + * framing). Throw-safe on panelIndexFor so a misbehaving splitscreen build + * can't break framing. + * @param {HTMLCanvasElement} canvas this renderer's highway canvas + * @param {object|null} ss the splitscreen API (see _ssApi) + * @param {object|null} panelsMap window.__h3dCamCtlPanels (per-panel cameras by index) + * @param {object|null} globalCam window.__h3dCamCtl (single global camera) + * @returns {object|null} the resolved free-camera bridge, or null + */ function _resolveFreeCam(canvas, ss, panelsMap, globalCam) { if (panelsMap && ss && typeof ss.panelIndexFor === 'function') { try { diff --git a/plugins/drum_highway_3d/tests/camera_bridge.test.js b/plugins/drum_highway_3d/tests/camera_bridge.test.js index 5921849..66b62bb 100644 --- a/plugins/drum_highway_3d/tests/camera_bridge.test.js +++ b/plugins/drum_highway_3d/tests/camera_bridge.test.js @@ -56,6 +56,13 @@ test('_resolveFreeCam: throw-safe on panelIndexFor → falls back to global', () assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g); }); +test('_resolveFreeCam: NaN/invalid panel index → falls back to global', () => { + const { __test } = load(); + const g = { id: 'global' }; + assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => NaN }, { 0: {} }, g), g); + assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => -1 }, { 0: {} }, g), g); +}); + test('_ssApi: null when neither global set; slopsmith alias; feedBack canonical wins', () => { const { window, __test } = load(); assert.equal(__test._ssApi(), null); diff --git a/plugins/highway_3d/screen.js b/plugins/highway_3d/screen.js index 1533bc7..8c36baf 100644 --- a/plugins/highway_3d/screen.js +++ b/plugins/highway_3d/screen.js @@ -2595,24 +2595,36 @@ } const FRET_NUMBER_GHOST_SCOPE_IDS = ['chords', 'all']; + /** + * localStorage panel key for per-panel background settings ('main' or + * 'panel'). Defensive on the splitscreen global-name rename in flight, + * and throw-safe on panelIndexFor — same as _freeCamFor — so a misbehaving + * splitscreen build can't take down background-settings resolution. Only a + * non-negative integer index yields a 'panel' key; anything else (null, + * NaN, negative, non-integer) falls back to 'main' so a bad index can never + * mint a bogus "panelNaN"-style key. + * @param {HTMLCanvasElement} canvas this renderer's highway canvas + * @returns {string} 'main' or 'panel' + */ function _bgPanelKey(canvas) { - // Defensive on the splitscreen global name (rename in flight) AND throw-safe - // on panelIndexFor — same as _freeCamFor — so a misbehaving splitscreen - // build can't take down background-settings resolution. Falls back to 'main'. const ss = window.feedBackSplitscreen || window.slopsmithSplitscreen; let idx = null; if (ss && typeof ss.panelIndexFor === 'function') { try { idx = ss.panelIndexFor(canvas); } catch (e) { idx = null; } } - return (idx == null) ? 'main' : 'panel' + idx; + return (Number.isInteger(idx) && idx >= 0) ? 'panel' + idx : 'main'; } - // Camera Director bridge resolver. Prefers THIS panel's per-panel camera - // under splitscreen (window.__h3dCamCtlPanels[panelIndex]) and falls back to - // the single global (window.__h3dCamCtl); returns null when Camera Director - // is absent → 100% stock framing. Defensive on the splitscreen global name - // (rename in flight: feedBackSplitscreen vs slopsmithSplitscreen). Mirrors - // the panel resolution in _bgPanelKey. + /** + * Camera Director bridge resolver. Prefers THIS panel's per-panel camera under + * splitscreen (window.__h3dCamCtlPanels[panelIndex]) and falls back to the + * single global (window.__h3dCamCtl); returns null when Camera Director is + * absent → 100% stock framing. Defensive on the splitscreen global-name rename + * in flight (feedBackSplitscreen vs slopsmithSplitscreen); throw-safe on + * panelIndexFor. Mirrors the panel resolution in _bgPanelKey. + * @param {HTMLCanvasElement} canvas this renderer's highway canvas + * @returns {object|null} the resolved free-camera bridge, or null + */ function _freeCamFor(canvas) { const map = window.__h3dCamCtlPanels; if (map) { diff --git a/plugins/keys_highway_3d/screen.js b/plugins/keys_highway_3d/screen.js index 238bc8f..3c6d923 100644 --- a/plugins/keys_highway_3d/screen.js +++ b/plugins/keys_highway_3d/screen.js @@ -1604,15 +1604,24 @@ * Camera Director bridge resolver (pure — exported via createFactory.__test) * ====================================================================== */ - // Resolve the active splitscreen API, defensive on the global-name rename in - // flight (feedBackSplitscreen is canonical; slopsmithSplitscreen is the legacy - // alias). Returns null when splitscreen isn't present. + /** + * The active splitscreen API, defensive on the global-name rename in flight + * (feedBackSplitscreen is canonical; slopsmithSplitscreen is the legacy alias). + * @returns {object|null} the splitscreen API, or null when not present + */ function _ssApi() { return window.feedBackSplitscreen || window.slopsmithSplitscreen || null; } - // Given the splitscreen API, THIS window's per-panel camera map, and the - // global camera, return this panel's camera under splitscreen, else the - // global, else null (Camera Director absent → 100% stock framing). Throw-safe - // on panelIndexFor so a misbehaving splitscreen build can't break framing. + /** + * Resolve the Camera Director camera for a canvas: this panel's camera under + * splitscreen, else the global, else null (Camera Director absent → 100% stock + * framing). Throw-safe on panelIndexFor so a misbehaving splitscreen build + * can't break framing. + * @param {HTMLCanvasElement} canvas this renderer's highway canvas + * @param {object|null} ss the splitscreen API (see _ssApi) + * @param {object|null} panelsMap window.__h3dCamCtlPanels (per-panel cameras by index) + * @param {object|null} globalCam window.__h3dCamCtl (single global camera) + * @returns {object|null} the resolved free-camera bridge, or null + */ function _resolveFreeCam(canvas, ss, panelsMap, globalCam) { if (panelsMap && ss && typeof ss.panelIndexFor === 'function') { try { diff --git a/plugins/keys_highway_3d/tests/camera_bridge.test.js b/plugins/keys_highway_3d/tests/camera_bridge.test.js index efaa0c9..a27e8dd 100644 --- a/plugins/keys_highway_3d/tests/camera_bridge.test.js +++ b/plugins/keys_highway_3d/tests/camera_bridge.test.js @@ -56,6 +56,13 @@ test('_resolveFreeCam: throw-safe on panelIndexFor → falls back to global', () assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g); }); +test('_resolveFreeCam: NaN/invalid panel index → falls back to global', () => { + const { __test } = load(); + const g = { id: 'global' }; + assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => NaN }, { 0: {} }, g), g); + assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => -1 }, { 0: {} }, g), g); +}); + test('_ssApi: null when neither global set; slopsmith alias; feedBack canonical wins', () => { const { window, __test } = load(); assert.equal(__test._ssApi(), null);