mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 19:29:33 +00:00
fix(highways): address review — bg-key alias, drum cam guard, resolver tests
Three review findings on the per-panel camera work: - highway_3d: _bgPanelKey() resolved splitscreen via window.feedBackSplitscreen only, while _freeCamFor() uses the feedBackSplitscreen||slopsmithSplitscreen alias it claims to "mirror". If the rename lands, per-panel background settings would silently stop being per-panel while the camera stayed per-panel. Resolve the alias the same way in _bgPanelKey. - drum_highway_3d: applyCamera()'s "before first positionCamera()" guard tested `_camBaseH == null`, but _camBaseH/_camBaseD were initialized to 0, so the guard never fired (and could apply a base-0 pose for a frame). Initialize to null. - keys + drum: the PR claimed the Camera Director resolver was unit-checked, but nothing exercised it. Extract the resolver into pure, exported helpers (_resolveFreeCam + _ssApi), delegate the per-instance _freeCamFor to them, and add tests/camera_bridge.test.js covering per-panel select, global fallback, null-when-absent, throw-safety, and the slopsmith-alias resolution. Drum 15→21, keys 50→56, all pass; behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Kris Anderson <topkoa@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ff8a638d28
commit
0d4d8229c7
@@ -1361,6 +1361,29 @@
|
|||||||
} catch (_) { /* dispatch unavailable — persisted value applies next init */ }
|
} catch (_) { /* dispatch unavailable — persisted value applies next init */ }
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/* ======================================================================
|
||||||
|
* 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.
|
||||||
|
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.
|
||||||
|
function _resolveFreeCam(canvas, ss, panelsMap, globalCam) {
|
||||||
|
if (panelsMap && ss && typeof ss.panelIndexFor === 'function') {
|
||||||
|
try {
|
||||||
|
const i = ss.panelIndexFor(canvas);
|
||||||
|
if (i != null && panelsMap[i]) return panelsMap[i];
|
||||||
|
} catch (e) { /* ignore */ }
|
||||||
|
}
|
||||||
|
return globalCam || null;
|
||||||
|
}
|
||||||
|
|
||||||
/* ======================================================================
|
/* ======================================================================
|
||||||
* Renderer factory
|
* Renderer factory
|
||||||
* ====================================================================== */
|
* ====================================================================== */
|
||||||
@@ -1414,7 +1437,7 @@
|
|||||||
let _sparkPts = null, _sparkPos = null, _sparkCol = null, _sparkVel = null, _sparkLife = null;
|
let _sparkPts = null, _sparkPos = null, _sparkCol = null, _sparkVel = null, _sparkLife = null;
|
||||||
let _fxLastWall = 0; // wall clock for FX integration (sparks, pulse decay)
|
let _fxLastWall = 0; // wall clock for FX integration (sparks, pulse decay)
|
||||||
let _kickPulse = 0; // kick-hit camera-dip + floor-wash envelope
|
let _kickPulse = 0; // kick-hit camera-dip + floor-wash envelope
|
||||||
let _camBaseH = 0, _camBaseD = 0; // positionCamera's unpulsed pose
|
let _camBaseH = null, _camBaseD = null; // positionCamera's unpulsed pose (null until it first runs; applyCamera's guard depends on this)
|
||||||
let _gaussTex = null; // shared soft-falloff texture for flash quads
|
let _gaussTex = null; // shared soft-falloff texture for flash quads
|
||||||
let _laneFlashQuads = []; // pooled additive quad per hand lane (z=0)
|
let _laneFlashQuads = []; // pooled additive quad per hand lane (z=0)
|
||||||
let _kickFlashQuad = null; // full-width flash quad for the kick bar
|
let _kickFlashQuad = null; // full-width flash quad for the kick bar
|
||||||
@@ -2651,23 +2674,12 @@
|
|||||||
cam.lookAt(0, 0, -AHEAD * TS * 0.45);
|
cam.lookAt(0, 0, -AHEAD * TS * 0.45);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Camera Director bridge resolver. Prefers THIS panel's per-panel camera
|
// Camera Director bridge for THIS panel — delegates to the pure, unit-
|
||||||
// under splitscreen (window.__h3dCamCtlPanels[panelIndex]) and falls back
|
// tested _resolveFreeCam / _ssApi (see the resolver block above the
|
||||||
// to the single global (window.__h3dCamCtl); null when Camera Director is
|
// factory). Reads the live globals: per-panel map __h3dCamCtlPanels →
|
||||||
// absent → 100% stock framing. Defensive on the splitscreen global name
|
// this panel's camera, else the global __h3dCamCtl, else null (stock).
|
||||||
// (rename in flight: feedBackSplitscreen vs slopsmithSplitscreen).
|
|
||||||
function _freeCamFor(canvas) {
|
function _freeCamFor(canvas) {
|
||||||
const map = window.__h3dCamCtlPanels;
|
return _resolveFreeCam(canvas, _ssApi(), window.__h3dCamCtlPanels, window.__h3dCamCtl);
|
||||||
if (map) {
|
|
||||||
const ss = window.feedBackSplitscreen || window.slopsmithSplitscreen;
|
|
||||||
if (ss && typeof ss.panelIndexFor === 'function') {
|
|
||||||
try {
|
|
||||||
const i = ss.panelIndexFor(canvas);
|
|
||||||
if (i != null && map[i]) return map[i];
|
|
||||||
} catch (e) { /* ignore */ }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return window.__h3dCamCtl || null;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Per-frame camera write: static base pose (positionCamera) + kick-pulse Y
|
// Per-frame camera write: static base pose (positionCamera) + kick-pulse Y
|
||||||
@@ -3617,6 +3629,8 @@
|
|||||||
// vm-loaded with no DOM/WebGL; everything here must stay side-effect
|
// vm-loaded with no DOM/WebGL; everything here must stay side-effect
|
||||||
// free to call).
|
// free to call).
|
||||||
window.slopsmithViz_drum_highway_3d.__test = {
|
window.slopsmithViz_drum_highway_3d.__test = {
|
||||||
|
_resolveFreeCam,
|
||||||
|
_ssApi,
|
||||||
_variantForHit,
|
_variantForHit,
|
||||||
_classifyTiming,
|
_classifyTiming,
|
||||||
readFxSettings,
|
readFxSettings,
|
||||||
|
|||||||
@@ -0,0 +1,68 @@
|
|||||||
|
// Camera Director bridge resolver tests: per-panel select, global fallback,
|
||||||
|
// null-when-absent, throw-safety, and the splitscreen global-name alias. Loads
|
||||||
|
// screen.js in a bare vm window and exercises the __test exports (no DOM/WebGL).
|
||||||
|
const { test } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const path = require('node:path');
|
||||||
|
const vm = require('node:vm');
|
||||||
|
|
||||||
|
function load() {
|
||||||
|
const window = {
|
||||||
|
console,
|
||||||
|
location: { protocol: 'http:', host: 'localhost' },
|
||||||
|
slopsmith: {},
|
||||||
|
};
|
||||||
|
window.window = window;
|
||||||
|
window.globalThis = window;
|
||||||
|
const context = vm.createContext(window);
|
||||||
|
const src = fs.readFileSync(path.join(__dirname, '..', 'screen.js'), 'utf8');
|
||||||
|
vm.runInContext(src, context, { filename: 'screen.js' });
|
||||||
|
return { window, __test: window.slopsmithViz_drum_highway_3d.__test };
|
||||||
|
}
|
||||||
|
|
||||||
|
test('_resolveFreeCam: per-panel camera under splitscreen', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const c0 = {}, c1 = {};
|
||||||
|
const ss = { panelIndexFor: (c) => (c === c0 ? 0 : 1) };
|
||||||
|
const map = { 0: { id: 'p0' }, 1: { id: 'p1' } };
|
||||||
|
assert.equal(__test._resolveFreeCam(c0, ss, map, { id: 'g' }).id, 'p0');
|
||||||
|
assert.equal(__test._resolveFreeCam(c1, ss, map, { id: 'g' }).id, 'p1');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: falls back to global when there is no panel map', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => 0 }, null, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: falls back to global when the panel has no map entry', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
const ss = { panelIndexFor: () => 3 }; // index 3 absent from map
|
||||||
|
assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: null when Camera Director is absent (no global)', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
assert.equal(__test._resolveFreeCam({}, null, null, null), null);
|
||||||
|
assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => 0 }, {}, undefined), null);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: throw-safe on panelIndexFor → falls back to global', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
const ss = { panelIndexFor: () => { throw new Error('boom'); } };
|
||||||
|
assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_ssApi: null when neither global set; slopsmith alias; feedBack canonical wins', () => {
|
||||||
|
const { window, __test } = load();
|
||||||
|
assert.equal(__test._ssApi(), null);
|
||||||
|
const legacy = { panelIndexFor: () => 0 };
|
||||||
|
window.slopsmithSplitscreen = legacy;
|
||||||
|
assert.equal(__test._ssApi(), legacy); // legacy alias picked up
|
||||||
|
const current = { panelIndexFor: () => 1 };
|
||||||
|
window.feedBackSplitscreen = current;
|
||||||
|
assert.equal(__test._ssApi(), current); // canonical name takes precedence
|
||||||
|
});
|
||||||
@@ -2596,7 +2596,9 @@
|
|||||||
const FRET_NUMBER_GHOST_SCOPE_IDS = ['chords', 'all'];
|
const FRET_NUMBER_GHOST_SCOPE_IDS = ['chords', 'all'];
|
||||||
|
|
||||||
function _bgPanelKey(canvas) {
|
function _bgPanelKey(canvas) {
|
||||||
const ss = window.feedBackSplitscreen;
|
// Defensive on the splitscreen global name (rename in flight) so per-panel
|
||||||
|
// background settings keep resolving the same panel as _freeCamFor().
|
||||||
|
const ss = window.feedBackSplitscreen || window.slopsmithSplitscreen;
|
||||||
const idx = (ss && typeof ss.panelIndexFor === 'function') ? ss.panelIndexFor(canvas) : null;
|
const idx = (ss && typeof ss.panelIndexFor === 'function') ? ss.panelIndexFor(canvas) : null;
|
||||||
return (idx == null) ? 'main' : 'panel' + idx;
|
return (idx == null) ? 'main' : 'panel' + idx;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1600,6 +1600,29 @@
|
|||||||
_aiRegisteredCount = 0;
|
_aiRegisteredCount = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* ======================================================================
|
||||||
|
* 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.
|
||||||
|
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.
|
||||||
|
function _resolveFreeCam(canvas, ss, panelsMap, globalCam) {
|
||||||
|
if (panelsMap && ss && typeof ss.panelIndexFor === 'function') {
|
||||||
|
try {
|
||||||
|
const i = ss.panelIndexFor(canvas);
|
||||||
|
if (i != null && panelsMap[i]) return panelsMap[i];
|
||||||
|
} catch (e) { /* ignore */ }
|
||||||
|
}
|
||||||
|
return globalCam || null;
|
||||||
|
}
|
||||||
|
|
||||||
/* ======================================================================
|
/* ======================================================================
|
||||||
* Renderer factory
|
* Renderer factory
|
||||||
* ====================================================================== */
|
* ====================================================================== */
|
||||||
@@ -1850,23 +1873,12 @@
|
|||||||
return _rigOut;
|
return _rigOut;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Camera Director bridge resolver. Prefers THIS panel's per-panel camera
|
// Camera Director bridge for THIS panel — delegates to the pure, unit-
|
||||||
// under splitscreen (window.__h3dCamCtlPanels[panelIndex]) and falls back
|
// tested _resolveFreeCam / _ssApi (see the resolver block above the
|
||||||
// to the single global (window.__h3dCamCtl); null when Camera Director is
|
// factory). Reads the live globals: per-panel map __h3dCamCtlPanels →
|
||||||
// absent → 100% stock framing. Defensive on the splitscreen global name
|
// this panel's camera, else the global __h3dCamCtl, else null (stock).
|
||||||
// (rename in flight: feedBackSplitscreen vs slopsmithSplitscreen).
|
|
||||||
function _freeCamFor(canvas) {
|
function _freeCamFor(canvas) {
|
||||||
const map = window.__h3dCamCtlPanels;
|
return _resolveFreeCam(canvas, _ssApi(), window.__h3dCamCtlPanels, window.__h3dCamCtl);
|
||||||
if (map) {
|
|
||||||
const ss = window.feedBackSplitscreen || window.slopsmithSplitscreen;
|
|
||||||
if (ss && typeof ss.panelIndexFor === 'function') {
|
|
||||||
try {
|
|
||||||
const i = ss.panelIndexFor(canvas);
|
|
||||||
if (i != null && map[i]) return map[i];
|
|
||||||
} catch (e) { /* ignore */ }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return window.__h3dCamCtl || null;
|
|
||||||
}
|
}
|
||||||
// Per-key approach glow: a key lights in its pitch-class color ONLY while a
|
// Per-key approach glow: a key lights in its pitch-class color ONLY while a
|
||||||
// note is heading for it, ramping up the closer that note gets to the hit-line.
|
// note is heading for it, ramping up the closer that note gets to the hit-line.
|
||||||
@@ -4005,6 +4017,8 @@
|
|||||||
};
|
};
|
||||||
// Pure data-layer + scoring hooks for headless tests.
|
// Pure data-layer + scoring hooks for headless tests.
|
||||||
window.slopsmithViz_keys_highway_3d.__test = {
|
window.slopsmithViz_keys_highway_3d.__test = {
|
||||||
|
_resolveFreeCam,
|
||||||
|
_ssApi,
|
||||||
beatDurSec,
|
beatDurSec,
|
||||||
flattenNotation,
|
flattenNotation,
|
||||||
keyRange,
|
keyRange,
|
||||||
|
|||||||
@@ -0,0 +1,68 @@
|
|||||||
|
// Camera Director bridge resolver tests: per-panel select, global fallback,
|
||||||
|
// null-when-absent, throw-safety, and the splitscreen global-name alias. Loads
|
||||||
|
// screen.js in a bare vm window and exercises the __test exports (no DOM/WebGL).
|
||||||
|
const { test } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const path = require('node:path');
|
||||||
|
const vm = require('node:vm');
|
||||||
|
|
||||||
|
function load() {
|
||||||
|
const window = {
|
||||||
|
console,
|
||||||
|
location: { protocol: 'http:', host: 'localhost' },
|
||||||
|
slopsmith: {},
|
||||||
|
};
|
||||||
|
window.window = window;
|
||||||
|
window.globalThis = window;
|
||||||
|
const context = vm.createContext(window);
|
||||||
|
const src = fs.readFileSync(path.join(__dirname, '..', 'screen.js'), 'utf8');
|
||||||
|
vm.runInContext(src, context, { filename: 'screen.js' });
|
||||||
|
return { window, __test: window.slopsmithViz_keys_highway_3d.__test };
|
||||||
|
}
|
||||||
|
|
||||||
|
test('_resolveFreeCam: per-panel camera under splitscreen', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const c0 = {}, c1 = {};
|
||||||
|
const ss = { panelIndexFor: (c) => (c === c0 ? 0 : 1) };
|
||||||
|
const map = { 0: { id: 'p0' }, 1: { id: 'p1' } };
|
||||||
|
assert.equal(__test._resolveFreeCam(c0, ss, map, { id: 'g' }).id, 'p0');
|
||||||
|
assert.equal(__test._resolveFreeCam(c1, ss, map, { id: 'g' }).id, 'p1');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: falls back to global when there is no panel map', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => 0 }, null, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: falls back to global when the panel has no map entry', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
const ss = { panelIndexFor: () => 3 }; // index 3 absent from map
|
||||||
|
assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: null when Camera Director is absent (no global)', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
assert.equal(__test._resolveFreeCam({}, null, null, null), null);
|
||||||
|
assert.equal(__test._resolveFreeCam({}, { panelIndexFor: () => 0 }, {}, undefined), null);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_resolveFreeCam: throw-safe on panelIndexFor → falls back to global', () => {
|
||||||
|
const { __test } = load();
|
||||||
|
const g = { id: 'global' };
|
||||||
|
const ss = { panelIndexFor: () => { throw new Error('boom'); } };
|
||||||
|
assert.equal(__test._resolveFreeCam({}, ss, { 0: {} }, g), g);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('_ssApi: null when neither global set; slopsmith alias; feedBack canonical wins', () => {
|
||||||
|
const { window, __test } = load();
|
||||||
|
assert.equal(__test._ssApi(), null);
|
||||||
|
const legacy = { panelIndexFor: () => 0 };
|
||||||
|
window.slopsmithSplitscreen = legacy;
|
||||||
|
assert.equal(__test._ssApi(), legacy); // legacy alias picked up
|
||||||
|
const current = { panelIndexFor: () => 1 };
|
||||||
|
window.feedBackSplitscreen = current;
|
||||||
|
assert.equal(__test._ssApi(), current); // canonical name takes precedence
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user