mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
feat(highway_3d): background controls in the player chrome (#1008)
* Add mid-song background picker to player chrome Mount a background style/intensity control in the player's plugin popover so users can switch backgrounds mid-song without leaving for Settings. Uses ref-counting to manage the shared control across multiple renderer instances. The control syncs bidirectionally with settings.html and the settings bus, so changes from either UI stay agreed. Moved _pcAcquire() to after _isReady to avoid acquiring for non-viable (e.g. WebGL2-missing) renderers. * Grey out background controls that current style ignores Add _PC_USES table to track which settings (intensity, reactive) each background style actually consumes. Disable and grey out controls when the active style doesn't use them, preventing user confusion. Updates _pcPaint() to support disabled state with tooltip explanations, and guards click/change handlers against disabled controls. * Add background control tests and changelog entry Document the new background controls feature in the 3D Highway plugin that allows changing the highway background mid-song from the player's Plugin Controls popover. Add a comprehensive test suite for the background control system covering refcounting, settings sync, greying out unsupported controls, and teardown behavior. * Generalize background control refcounting language Update CHANGELOG and test comments to reflect that the 3D highway background control refcounting applies to any multiple renderer instances, not exclusively splitscreen. Change test name and clarify that multi-instance behavior is exercised with stubbed instances, not real splitscreen sessions (whose visualizer does not currently work). * Reorder 3D Highway changelog entry, bump version Moved the 'Background controls in the player' entry to a different position in the Unreleased changelog section. Updated 3D Highway plugin version from 3.32.0 to 3.33.0. * fix: store screen.js and CHANGELOG.md with CRLF to match main The merge of main was run with merge.renormalize=true (needed — this repo has CRLF committed while core.autocrlf=true, so a plain merge sees all 16k lines as changed). That rewrote screen.js and CHANGELOG.md to LF, which autocrlf then stored. main has both as CRLF, so every line differed and GitHub reported 16,428/16,112 for screen.js and refused to render it. Restaged with the CRLF blobs written directly so they are what get stored. No content change; the diff drops to 316/0 and highway_3d_render_order.test.js leaves the diff entirely. Signed-off-by: Kyle <kyle.j.t@live.co.uk> * Unbind screen:changed hook on last release Ensure the highway_3d control removes its screen:changed listener when the last reference is released to avoid listener/closure leaks across plugin reloads. Added a best-effort off() call and clears _pcScreenHook so future acquires re-bind correctly. Tests updated: mock feedBack on/off implemented, helpers added (screenHooks, fireScreenChanged), and a new test verifies the subscription is removed on final _pcRelease and re-subscribed on re-acquire. * fix(highway_3d): show greyed-out reason on hover for disabled bg controls A native-disabled <button>/<input> receives no pointer events, so its `title` tooltip never appears — the "greyed out, says why on hover" affordance was dead in the browser while the tests passed on the swallowed control title. Move the reason onto a non-disabled wrapper and set pointer-events:none on the disabled control so the hover reaches it. Also add aria-disabled so screen readers get the state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Byron Gamatos <xasiklas@gmail.com> --------- Signed-off-by: Kyle <kyle.j.t@live.co.uk> Signed-off-by: Byron Gamatos <xasiklas@gmail.com> Co-authored-by: Byron Gamatos <xasiklas@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kyle
co-authored by
Claude Opus 4.8
Byron Gamatos
Claude Opus 4.8
Byron Gamatos
parent
be49465540
commit
fcdb4867d6
@@ -0,0 +1,370 @@
|
||||
// Player-chrome background control.
|
||||
//
|
||||
// The control mounts a Background picker (style / Reactive / Intensity) into
|
||||
// the player's Plugin Controls popover so the background can be changed
|
||||
// mid-song. Two things about it are easy to get wrong and invisible when they
|
||||
// are:
|
||||
//
|
||||
// * It is REFCOUNTED. Several renderer instances can be live at once (a
|
||||
// splitscreen host creates one per panel), but the settings it writes are
|
||||
// global — N controls would be N ways to set one value, and a leaked
|
||||
// refcount pins a dead control in the UI. The multi-instance behaviour is
|
||||
// exercised here with stubbed instances; it is NOT verified against a real
|
||||
// splitscreen session, whose visualizer does not currently work.
|
||||
// * It GREYS OUT controls the active style ignores. Not every background
|
||||
// style reads `intensity`, and none of them read audio bands under
|
||||
// Butterchurn, so a live-looking knob that does nothing is a real bug.
|
||||
//
|
||||
// screen.js is a single ~16k-line IIFE, so the control cannot be imported. The
|
||||
// self-contained `_pc*` block is sliced out of the real source and evaluated
|
||||
// with its few collaborators stubbed (BG_STYLE_IDS, _bgReadSetting,
|
||||
// _bgSubscribe/_bgUnsubscribe). The slice markers are asserted before use: move
|
||||
// or rename the block and this fails loudly rather than testing nothing.
|
||||
|
||||
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');
|
||||
|
||||
const SCREEN_JS = path.join(__dirname, '..', 'screen.js');
|
||||
const START = ' const _PC_LABELS = {';
|
||||
const END_CRLF = ' /* ======================================================================\r\n * Factory';
|
||||
const END_LF = ' /* ======================================================================\n * Factory';
|
||||
|
||||
// What each style is expected to consume, derived by reading the BG_STYLES
|
||||
// bodies in screen.js — deliberately NOT read from the plugin's own _PC_USES
|
||||
// table, which would only assert that the table equals itself.
|
||||
// intensity: true => the style's build() reads settings.intensity
|
||||
// reactive: true => the style's update() dereferences its `bands` argument
|
||||
// 'butterchurn' is not a BG_STYLES entry at all (mount falls through to
|
||||
// BG_STYLES.off) and drives its own audio tap, so both are false.
|
||||
const EXPECTED_USES = {
|
||||
off: { intensity: false, reactive: false },
|
||||
particles: { intensity: true, reactive: true },
|
||||
silhouettes: { intensity: true, reactive: true },
|
||||
lights: { intensity: true, reactive: true },
|
||||
geometric: { intensity: true, reactive: true },
|
||||
image: { intensity: true, reactive: false },
|
||||
video: { intensity: false, reactive: false },
|
||||
butterchurn: { intensity: false, reactive: false },
|
||||
};
|
||||
|
||||
const BG_STYLE_IDS = ['off', 'particles', 'silhouettes', 'lights', 'geometric', 'butterchurn', 'image', 'video'];
|
||||
|
||||
// Minimal DOM: only what the control touches.
|
||||
function makeDom() {
|
||||
class El {
|
||||
constructor(tag) {
|
||||
this.tagName = String(tag).toUpperCase();
|
||||
this.children = [];
|
||||
this.parentNode = null;
|
||||
this.listeners = {};
|
||||
this.style = { cssText: '' };
|
||||
this.disabled = false;
|
||||
this._on = false;
|
||||
}
|
||||
appendChild(c) { c.parentNode = this; this.children.push(c); return c; }
|
||||
removeChild(c) {
|
||||
const i = this.children.indexOf(c);
|
||||
if (i >= 0) this.children.splice(i, 1);
|
||||
c.parentNode = null;
|
||||
return c;
|
||||
}
|
||||
addEventListener(t, fn) { (this.listeners[t] || (this.listeners[t] = [])).push(fn); }
|
||||
setAttribute(k, v) { this[k] = v; }
|
||||
get isConnected() {
|
||||
let n = this;
|
||||
while (n.parentNode) n = n.parentNode;
|
||||
return n === root;
|
||||
}
|
||||
querySelector(sel) {
|
||||
const m = /^option\[value="(.+)"\]$/.exec(sel);
|
||||
const want = m ? m[1] : null;
|
||||
const walk = (n) => {
|
||||
for (const c of n.children) {
|
||||
if (want != null && c.tagName === 'OPTION' && c.value === want) return c;
|
||||
const r = walk(c);
|
||||
if (r) return r;
|
||||
}
|
||||
return null;
|
||||
};
|
||||
return walk(this);
|
||||
}
|
||||
fire(type) { (this.listeners[type] || []).forEach((fn) => fn()); }
|
||||
}
|
||||
const root = new El('root');
|
||||
const slot = new El('div');
|
||||
root.appendChild(slot);
|
||||
return { El, root, slot };
|
||||
}
|
||||
|
||||
function load({ store: initialStore } = {}) {
|
||||
const src = fs.readFileSync(SCREEN_JS, 'utf8');
|
||||
const start = src.indexOf(START);
|
||||
assert.notEqual(start, -1, 'could not find the _PC_LABELS marker in screen.js');
|
||||
let end = src.indexOf(END_CRLF);
|
||||
if (end === -1) end = src.indexOf(END_LF);
|
||||
assert.notEqual(end, -1, 'could not find the Factory banner marker in screen.js');
|
||||
assert.ok(end > start, 'slice markers found out of order in screen.js');
|
||||
const block = src.slice(start, end);
|
||||
|
||||
const dom = makeDom();
|
||||
const store = Object.assign({
|
||||
style: 'particles',
|
||||
reactive: true,
|
||||
intensity: 0.5,
|
||||
customImageDataUrl: '',
|
||||
customVideoName: '',
|
||||
}, initialStore);
|
||||
|
||||
const bus = {};
|
||||
const listeners = new Set();
|
||||
const emit = (key) => { for (const fn of listeners) fn(key); };
|
||||
const writes = [];
|
||||
const timers = [];
|
||||
|
||||
const sandbox = {
|
||||
console,
|
||||
BG_STYLE_IDS,
|
||||
_bgReadSetting: (_panelKey, key) => store[key],
|
||||
_bgSubscribe: (fn) => listeners.add(fn),
|
||||
_bgUnsubscribe: (fn) => listeners.delete(fn),
|
||||
setTimeout: (fn) => { timers.push(fn); return timers.length; },
|
||||
clearTimeout: () => {},
|
||||
document: {
|
||||
createElement: (t) => new dom.El(t),
|
||||
// The Settings-panel mirror looks these up; absent here so it no-ops.
|
||||
getElementById: () => null,
|
||||
},
|
||||
window: {
|
||||
feedBack: {
|
||||
ui: { playerControlSlot: () => dom.slot },
|
||||
// The real bus is an EventTarget wrapper exposing on/off. Modelled
|
||||
// here so the screen:changed subscription — and its removal — are
|
||||
// observable.
|
||||
on: (ev, fn) => { (bus[ev] || (bus[ev] = [])).push(fn); },
|
||||
off: (ev, fn) => {
|
||||
const l = bus[ev];
|
||||
if (!l) return;
|
||||
const i = l.indexOf(fn);
|
||||
if (i >= 0) l.splice(i, 1);
|
||||
},
|
||||
},
|
||||
h3dBgSetStyle: (v) => { writes.push(['style', v]); store.style = v; emit('style'); },
|
||||
h3dBgSetReactive: (v) => { writes.push(['reactive', v]); store.reactive = v; emit('reactive'); },
|
||||
h3dBgSetIntensity: (v) => { writes.push(['intensity', v]); store.intensity = v; emit('intensity'); },
|
||||
},
|
||||
};
|
||||
sandbox.globalThis = sandbox;
|
||||
|
||||
const api = vm.runInNewContext(
|
||||
block
|
||||
+ '\n({ _pcAcquire, _pcRelease,'
|
||||
+ ' get el() { return _pcEl; },'
|
||||
+ ' get sel() { return _pcSel; },'
|
||||
+ ' get react() { return _pcReactive; },'
|
||||
+ ' get intens() { return _pcIntensity; },'
|
||||
+ ' get refs() { return _pcRefs; } })',
|
||||
sandbox,
|
||||
);
|
||||
const fireScreenChanged = () => (bus['screen:changed'] || []).slice().forEach((fn) => fn());
|
||||
const screenHooks = () => (bus['screen:changed'] || []).length;
|
||||
return { api, dom, store, emit, writes, timers, sandbox, listenerCount: () => listeners.size, fireScreenChanged, screenHooks };
|
||||
}
|
||||
|
||||
test('mounts one control into the player-control slot', () => {
|
||||
const { api, dom } = load();
|
||||
api._pcAcquire();
|
||||
assert.equal(dom.slot.children.length, 1);
|
||||
assert.ok(api.sel, 'style dropdown was not created');
|
||||
assert.equal(api.sel.children.length, BG_STYLE_IDS.length, 'one option per style');
|
||||
});
|
||||
|
||||
test('multiple renderer instances share a single control', () => {
|
||||
const { api, dom } = load();
|
||||
api._pcAcquire();
|
||||
api._pcAcquire();
|
||||
api._pcAcquire();
|
||||
api._pcAcquire();
|
||||
assert.equal(dom.slot.children.length, 1, 'four instances must not mount four controls');
|
||||
assert.equal(api.refs, 4);
|
||||
|
||||
api._pcRelease();
|
||||
api._pcRelease();
|
||||
api._pcRelease();
|
||||
assert.equal(dom.slot.children.length, 1, 'still held by the last instance');
|
||||
api._pcRelease();
|
||||
assert.equal(dom.slot.children.length, 0, 'last release must unmount');
|
||||
assert.equal(api.el, null);
|
||||
});
|
||||
|
||||
test('the last release unbinds the screen:changed hook', () => {
|
||||
const ctl = load();
|
||||
ctl.api._pcAcquire();
|
||||
assert.equal(ctl.screenHooks(), 1, 'acquire should subscribe once');
|
||||
|
||||
ctl.api._pcAcquire();
|
||||
ctl.api._pcRelease();
|
||||
assert.equal(ctl.screenHooks(), 1, 'a partial release must keep the hook');
|
||||
|
||||
ctl.api._pcRelease();
|
||||
assert.equal(ctl.screenHooks(), 0, 'the hook outlived the control');
|
||||
|
||||
// And re-acquiring must re-subscribe exactly once, not zero times (the
|
||||
// bind is guarded on _pcScreenHook, so failing to null it would leave the
|
||||
// control permanently deaf to chrome rebuilds).
|
||||
ctl.api._pcAcquire();
|
||||
assert.equal(ctl.screenHooks(), 1, 're-acquire did not re-subscribe');
|
||||
ctl.api._pcRelease();
|
||||
});
|
||||
|
||||
test('teardown unsubscribes from the settings bus', () => {
|
||||
const ctl = load();
|
||||
ctl.api._pcAcquire();
|
||||
assert.equal(ctl.listenerCount(), 1);
|
||||
ctl.api._pcRelease();
|
||||
assert.equal(ctl.listenerCount(), 0, 'listener leaked after unmount');
|
||||
});
|
||||
|
||||
test('tracks changes made from the Settings page', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
store.style = 'lights';
|
||||
emit('style');
|
||||
assert.equal(api.sel.value, 'lights');
|
||||
});
|
||||
|
||||
test('custom media options stay disabled until something is uploaded', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
assert.equal(api.sel.querySelector('option[value="image"]').disabled, true);
|
||||
store.customImageDataUrl = 'data:image/png;base64,AAAA';
|
||||
emit('customImageDataUrl');
|
||||
assert.equal(api.sel.querySelector('option[value="image"]').disabled, false);
|
||||
assert.equal(api.sel.querySelector('option[value="video"]').disabled, true, 'video is independent');
|
||||
});
|
||||
|
||||
test('re-mounts into a fresh slot when the player chrome is rebuilt', () => {
|
||||
const { api, dom, sandbox, listenerCount } = load();
|
||||
api._pcAcquire();
|
||||
const first = api.el;
|
||||
|
||||
dom.root.removeChild(dom.slot);
|
||||
const fresh = new dom.El('div');
|
||||
dom.root.appendChild(fresh);
|
||||
sandbox.window.feedBack.ui.playerControlSlot = () => fresh;
|
||||
|
||||
api._pcAcquire();
|
||||
assert.equal(fresh.children.length, 1, 'did not remount into the new slot');
|
||||
assert.notEqual(api.el, first, 'stale node was reused');
|
||||
assert.equal(listenerCount(), 1, 'remount must not double-subscribe');
|
||||
});
|
||||
|
||||
test('a host with no player-control slot mounts nothing and does not throw', () => {
|
||||
const { api, dom, sandbox, timers } = load();
|
||||
sandbox.window.feedBack.ui = {};
|
||||
api._pcAcquire();
|
||||
assert.equal(api.el, null);
|
||||
assert.equal(dom.slot.children.length, 0);
|
||||
|
||||
let guard = 0;
|
||||
while (timers.length && guard++ < 100) timers.shift()();
|
||||
assert.ok(guard < 100, 'retry loop did not terminate');
|
||||
});
|
||||
|
||||
test('intensity writes once on release, not on every drag step', () => {
|
||||
const { api, writes } = load();
|
||||
api._pcAcquire();
|
||||
for (const v of ['0.10', '0.20', '0.30', '0.40', '0.50']) {
|
||||
api.intens.value = v;
|
||||
api.intens.fire('input');
|
||||
}
|
||||
assert.equal(writes.filter((w) => w[0] === 'intensity').length, 0,
|
||||
'dragging must not write — every write rebuilds the background scene');
|
||||
api.intens.fire('change');
|
||||
assert.equal(writes.filter((w) => w[0] === 'intensity').length, 1,
|
||||
'releasing must write exactly once');
|
||||
});
|
||||
|
||||
test('the dropdown and Reactive pill drive the real setters', () => {
|
||||
const { api, store, writes } = load();
|
||||
api._pcAcquire();
|
||||
api.sel.value = 'geometric';
|
||||
api.sel.fire('change');
|
||||
assert.equal(store.style, 'geometric');
|
||||
|
||||
const before = store.reactive;
|
||||
api.react.fire('click');
|
||||
assert.equal(store.reactive, !before, 'Reactive pill must toggle');
|
||||
assert.ok(writes.some((w) => w[0] === 'reactive'));
|
||||
});
|
||||
|
||||
test('greys out exactly the controls each style ignores', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
for (const [style, want] of Object.entries(EXPECTED_USES)) {
|
||||
store.style = style;
|
||||
emit('style');
|
||||
assert.equal(!api.intens.disabled, want.intensity, `${style}: intensity enabled-ness`);
|
||||
assert.equal(!api.react.disabled, want.reactive, `${style}: reactive enabled-ness`);
|
||||
}
|
||||
});
|
||||
|
||||
test('an unknown style enables both controls (fails open)', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
store.style = 'some_future_style';
|
||||
emit('style');
|
||||
assert.equal(api.intens.disabled, false);
|
||||
assert.equal(api.react.disabled, false);
|
||||
});
|
||||
|
||||
test('greyed-out controls cannot reach the setters', () => {
|
||||
const { api, store, emit, writes } = load();
|
||||
api._pcAcquire();
|
||||
store.style = 'video'; // uses neither setting
|
||||
emit('style');
|
||||
const before = writes.length;
|
||||
api.intens.fire('change');
|
||||
api.react.fire('click');
|
||||
assert.equal(writes.length, before, 'an inert control must not write');
|
||||
});
|
||||
|
||||
test('greyed-out controls explain themselves on hover', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
store.style = 'butterchurn';
|
||||
emit('style');
|
||||
assert.match(api.react.title, /butterchurn/i);
|
||||
assert.match(api.intens.title, /butterchurn/i);
|
||||
});
|
||||
|
||||
// A native-disabled <button>/<input> fires no pointer events, so its own
|
||||
// `title` never shows on hover. The reason must therefore also sit on the
|
||||
// non-disabled wrapper, and the disabled control must let the hover fall
|
||||
// through (pointer-events:none) — otherwise the "says why on hover" feature is
|
||||
// dead in the browser while these tests pass on the swallowed control title.
|
||||
test('the greyed-out reason reaches a hoverable wrapper', () => {
|
||||
const { api, store, emit } = load();
|
||||
api._pcAcquire();
|
||||
store.style = 'video'; // uses neither setting
|
||||
emit('style');
|
||||
|
||||
assert.match(api.react.parentNode.title, /nothing to adjust/i,
|
||||
'reactive reason must be on the wrapper, not only the disabled pill');
|
||||
assert.equal(api.react.style.pointerEvents, 'none',
|
||||
'disabled pill must pass hover through to its wrapper');
|
||||
|
||||
assert.match(api.intens.parentNode.title, /nothing to adjust/i,
|
||||
'intensity reason must be on the wrapper, not only the disabled slider');
|
||||
assert.equal(api.intens.style.pointerEvents, 'none',
|
||||
'disabled slider must pass hover through to its wrapper');
|
||||
|
||||
// ...and an enabled style clears the wrapper so the control's own title wins.
|
||||
store.style = 'particles';
|
||||
emit('style');
|
||||
assert.equal(api.react.parentNode.title, '');
|
||||
assert.equal(api.intens.parentNode.title, '');
|
||||
assert.equal(api.intens.style.pointerEvents, '');
|
||||
});
|
||||
Reference in New Issue
Block a user