diff --git a/static/v3/venue-crowd.js b/static/v3/venue-crowd.js index dea8789..fbb2c5d 100644 --- a/static/v3/venue-crowd.js +++ b/static/v3/venue-crowd.js @@ -133,6 +133,11 @@ let _lastStingerAt = -Infinity; let _prevStreak = 0; let _lastAccuracyPct = null; // from perf events; stats:recorded carries none + // Filename of the song song:loaded last reported. An arrangement switch + // re-emits song:loaded for the SAME file (changeArrangement reloads through + // the normal load path), and that must not be mistaken for arriving at the + // venue with a new song — see onSongLoaded. + let _lastSongFile = ''; let _bound = false; function now() { return Date.now(); } @@ -478,10 +483,40 @@ } } - function onSongLoaded() { + // song:loaded for the SAME file is an arrangement switch, not an arrival at + // the venue. changeArrangement() reloads through the normal load path, so + // the event is indistinguishable from a fresh load except by filename. + function isArrangementSwitch(prevFile, nextFile) { + return !!nextFile && nextFile === prevFile; + } + + function onSongLoaded(song) { + const file = String((song && song.filename) || ''); + const sameSong = isArrangementSwitch(_lastSongFile, file); + _lastSongFile = file; + machine.reset(); _prevStreak = 0; _lastAccuracyPct = null; + + // Switching arrangement is NOT arriving at the venue. + // + // changeArrangement() reloads the song through the same path as a fresh + // load, so highway.js emits song:loaded again — same filename, new + // arrangement. Treated as a new song, that replayed the arrival flyover: + // the camera flew in from the back of the room again mid-set, every time + // the player switched from lead to rhythm. The player is already on + // stage; the room should just carry on. + // + // So keep the video pipeline running and only re-sync the mood: the + // performance restarts, so the loop must follow the reset machine (a + // quiet crossfade), never the intro. + if (sameSong) { + if (_venueActive && _manifest && !_introActive) showLoop(machine.current, FADE_MS); + return; + } + + // A genuinely different song — full teardown. // Abort any stinger/pending state from the previous song: its ended // handler must not fade back into the old song's layers. cancelFade(); @@ -651,6 +686,7 @@ bindRuntime, getState, celebrate, + isArrangementSwitch, }; if (root) root.v3VenueCrowd = api; diff --git a/static/v3/venue-scene-3d.js b/static/v3/venue-scene-3d.js index 5dec46b..dc81127 100644 --- a/static/v3/venue-scene-3d.js +++ b/static/v3/venue-scene-3d.js @@ -18,6 +18,30 @@ let _lastMood = 'idle'; let _bound = false; + // The venue belongs to the SONG player and nowhere else. + // + // isVenueViz() only answers "is Venue the selected visualization" — a global + // preference. It says nothing about what is on screen. Other surfaces borrow + // the same highway_3d renderer (Virtuoso runs its practice charts on it), so + // with Venue selected they inherited the venue backdrop: the crowd and the + // stage showed up behind a chromatic exercise. The viz picker is a + // preference for the player; it is not a licence to paint the venue over + // whatever else happens to be using the renderer. + // + // So gate on both: Venue selected AND the player screen is the one showing. + function isPlayerScreen() { + try { + const active = document.querySelector('.screen.active'); + return !!active && active.id === 'player'; + } catch (_) { + return false; + } + } + + function shouldBeActive() { + return isVenueViz() && isPlayerScreen(); + } + function isVenueViz() { if (root && root.v3VenueViz && typeof root.v3VenueViz.isVenueVisualization === 'function') { const sel = root.v3VenueViz.getSelectedVizId @@ -146,7 +170,8 @@ function syncViz(vizId) { const id = String(vizId || ''); - if (id === 'venue') { + // Venue selected is necessary but not sufficient — see shouldBeActive. + if (id === 'venue' && isPlayerScreen()) { activate(); } else { deactivate(); @@ -192,12 +217,19 @@ if (_active) syncInstrumentPov(); }); sm.on('viz:renderer:ready', () => { - if (isVenueViz()) activate(); + if (shouldBeActive()) activate(); else deactivate(); }); sm.on('viz:reverted', () => deactivate()); + // Leaving the player tears the venue down; coming back rebuilds it. + // Without this the backdrop followed the renderer onto every other + // surface that borrows it (Virtuoso's practice highway). + sm.on('screen:changed', () => { + if (shouldBeActive()) activate(); + else deactivate(); + }); } - if (isVenueViz()) activate(); + if (shouldBeActive()) activate(); } function getState() { @@ -234,6 +266,8 @@ activate, deactivate, syncViz, + isPlayerScreen, + shouldBeActive, onAssetsLoaded, onAssetsFailed, onPerformanceState, diff --git a/tests/js/venue_scene_3d.test.js b/tests/js/venue_scene_3d.test.js index 69272d7..b8f61fd 100644 --- a/tests/js/venue_scene_3d.test.js +++ b/tests/js/venue_scene_3d.test.js @@ -208,7 +208,7 @@ test('index.html loads venue deps before venue-scene-3d', () => { assert.ok(vizIdx < moodIdx && moodIdx < sceneIdx); }); -test('syncViz activates only for venue visualization id', () => { +test('syncViz activates only for venue visualization id, and only on the player', () => { global.h3dVenueSceneSetActive = (on) => { global._h3dActive = on; }; global.h3dVenueSceneSetMood = (s) => { global._h3dMood = s; }; global.h3dVenueSceneSetInstrumentPov = () => {}; @@ -216,7 +216,14 @@ test('syncViz activates only for venue visualization id', () => { global.v3VenueViz = venueViz; global.v3VenueInstrumentPov = pov; global.feedBack = { on() {} }; + // The venue is scoped to the song player: selecting Venue is a preference + // for THAT screen, not a licence to paint the venue over anything else that + // borrows the highway_3d renderer (Virtuoso's practice charts did exactly + // that). syncViz therefore needs to know which screen is showing. + const onScreen = (id) => { global.document = { querySelector: (s) => (s === '.screen.active' && id ? { id } : null) }; }; + const prevDoc = global.document; try { + onScreen('player'); venueScene.deactivate(); venueScene.syncViz('highway_3d'); assert.equal(global._h3dActive, false); @@ -224,7 +231,16 @@ test('syncViz activates only for venue visualization id', () => { assert.equal(global._h3dActive, true); assert.equal(venueScene.getState().active, true); assert.equal(venueScene.getState().themeId, 'small-club'); + + // ...and the same call OFF the player must not activate it. + venueScene.deactivate(); + onScreen('virtuoso'); + venueScene.syncViz('venue'); + assert.equal(global._h3dActive, false, + 'Venue selected must NOT paint the venue onto the Virtuoso highway'); + assert.equal(venueScene.getState().active, false); } finally { + global.document = prevDoc; venueScene.deactivate(); delete global.h3dVenueSceneSetActive; delete global.h3dVenueSceneSetMood; diff --git a/tests/js/venue_scope.test.js b/tests/js/venue_scope.test.js new file mode 100644 index 0000000..af9564b --- /dev/null +++ b/tests/js/venue_scope.test.js @@ -0,0 +1,119 @@ +// Two venue bugs reported from a live career session. +// +// 1. Changing arrangement mid-song replayed the venue arrival flyover. The +// camera flew in from the back of the room again, every time the player +// switched lead -> rhythm. changeArrangement() reloads the song through the +// normal load path, so highway.js re-emits `song:loaded` — same filename, +// new arrangement — and the venue could not tell that from a fresh arrival. +// The player is already on stage; the room should just carry on. +// +// 2. With Venue selected, the venue backdrop showed up on the VIRTUOSO highway. +// The venue was gated purely on the viz selection, which is a global +// preference and says nothing about what is on screen. Virtuoso borrows the +// same highway_3d renderer for its practice charts, so it inherited the +// crowd and the stage behind a chromatic exercise. The venue belongs to the +// song player and nowhere else. + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); + +const crowd = require('../../static/v3/venue-crowd.js'); + +// ── 1. arrangement switch is not an arrival ──────────────────────────────── + +test('same filename = arrangement switch (no arrival flyover)', () => { + // changeArrangement() re-emits song:loaded for the song already on stage. + assert.equal(crowd.isArrangementSwitch('song.feedpak', 'song.feedpak'), true); +}); + +test('different filename = a genuinely new song (flyover is correct)', () => { + assert.equal(crowd.isArrangementSwitch('a.feedpak', 'b.feedpak'), false); +}); + +test('first load of the session is an arrival, not a switch', () => { + // No previous song -> the flyover must play. + assert.equal(crowd.isArrangementSwitch('', 'a.feedpak'), false); +}); + +test('a missing filename is never treated as a switch', () => { + // Otherwise a malformed payload would silently suppress the flyover for the + // rest of the session. + assert.equal(crowd.isArrangementSwitch('a.feedpak', ''), false); + assert.equal(crowd.isArrangementSwitch('a.feedpak', undefined), false); + assert.equal(crowd.isArrangementSwitch('', ''), false); +}); + +// ── 2. the venue belongs to the player screen ────────────────────────────── + +const scene = require('../../static/v3/venue-scene-3d.js'); + +// Venue MUST be the selected visualization for these to mean anything: if the +// viz were unset, shouldBeActive() would be false for the wrong reason and the +// virtuoso assertion below would pass vacuously. Force the viz on, so the only +// thing under test is the SCREEN gate. +function withScreen(id, fn) { + const prevDoc = global.document; + const prevViz = global.v3VenueViz; + global.v3VenueViz = { + isVenueVisualization: (v) => String(v) === 'venue', + getSelectedVizId: () => 'venue', + }; + global.document = { + querySelector(sel) { + if (sel !== '.screen.active') return null; + return id ? { id } : null; + }, + }; + try { return fn(); } finally { global.document = prevDoc; global.v3VenueViz = prevViz; } +} + +test('guard: with Venue selected AND on the player, the venue IS active', () => { + // If this ever fails, every "not active" test below is vacuous. + withScreen('player', () => { + assert.equal(scene.shouldBeActive(), true, + 'the screen gate must not break the normal case'); + }); +}); + +test('venue is active on the player screen', () => { + withScreen('player', () => { + assert.equal(scene.isPlayerScreen(), true); + }); +}); + +test('venue is NOT active on the virtuoso screen (the bug)', () => { + withScreen('virtuoso', () => { + assert.equal(scene.isPlayerScreen(), false, + 'Virtuoso borrows the same highway_3d renderer — the venue backdrop ' + + 'must not follow it there'); + assert.equal(scene.shouldBeActive(), false, + 'selecting Venue is a preference for the PLAYER; it is not a licence ' + + 'to paint the venue over whatever else is using the renderer'); + }); +}); + +test('venue is not active on any other screen either', () => { + for (const id of ['v3-home', 'plugin-folder_library', 'settings', 'career']) { + withScreen(id, () => { + assert.equal(scene.shouldBeActive(), false, `venue must not be active on ${id}`); + }); + } +}); + +test('no active screen at all is not the player', () => { + withScreen(null, () => { + assert.equal(scene.isPlayerScreen(), false); + }); +}); + +test('a throwing document does not take the venue down with it', () => { + const prev = global.document; + global.document = { querySelector() { throw new Error('detached'); } }; + try { + assert.equal(scene.isPlayerScreen(), false, 'must fail closed, not throw'); + } finally { + global.document = prev; + } +});