From f785fb9ab1ab62374cafea88f1b41b9d1ef19f81 Mon Sep 17 00:00:00 2001 From: Jorge Fritis <120731233+Jafz2001@users.noreply.github.com> Date: Fri, 3 Jul 2026 09:59:34 -0400 Subject: [PATCH] fix(renderer): keep Rig Builder's tone out of the user's manual VST chain (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(renderer): keep Rig Builder's tone out of the user's manual VST chain Rig Builder's chain preloader is always on, so it loads its whole tone (amp / pedals / racks / master pre-post / RB Final Leveler) into the SHARED engine chain. The Audio menu's 'Save Current Chain' and auto-persist captured the LIVE engine via getChainState()/savePreset(), baking those stages into the user's manual chain — so a user who built their own VST chain saw it sprout a full Rig Builder rig they never added. Add aeIsRigBuilderStage() (path under /rig_builder/, 'RB Final Leveler', rs_gear __rb*, or slot master_pre/post) + aeStripRigBuilderFromNativePreset(), and apply them at save (items + native blob), the app-init restore loop, the preset-load path (with an empty-guard), and refreshChain (display filter) so the manual chain only ever holds the user's own processors. Co-Authored-By: Claude Fable 5 * fix(renderer): skip Rig Builder stages in the tone-switch preload paths too Codex review: legacy polluted presets were only sanitized in replaceChainWithPresetBlob(), but the tone-switch preloads load directly from raw preset.items + nativePreset.chain (loadPresetItemsWithState in IIFE 1 and the deliberately-inline copy in IIFE 2). Skip Rig Builder stages by index in both loops — index-skips keep the items/nativeChain alignment for the remaining pairs — and expose the detector as window._aeIsRigBuilderStage for IIFE 2. Co-Authored-By: Claude Fable 5 * fix(renderer): load fully-polluted presets as empty instead of falling back Codex review round 2: the never-empty guard restored the ORIGINAL polluted blob whenever stripping emptied the chain — but a preset that empties completely was 100% Rig Builder's tone, exactly the case the sanitizer exists for. Load the stripped (empty) chain and warn; empty- chain presets are a supported shape. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Jafz2001 Co-authored-by: Claude Fable 5 Co-authored-by: byrongamatos --- src/renderer/screen.js | 161 ++++++++++++++++++++------ tests/audio-chain-persistence.test.js | 107 +++++++++++------ 2 files changed, 197 insertions(+), 71 deletions(-) diff --git a/src/renderer/screen.js b/src/renderer/screen.js index 3844432..b963aa5 100644 --- a/src/renderer/screen.js +++ b/src/renderer/screen.js @@ -1031,17 +1031,17 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; console.warn('[audio-engine] Corrupted slopsmith-signal-chain; starting empty:', e); savedChain = []; } - // Drop Rig Builder plumbing stages from legacy saves. Before - // saveChainStateFromChain learned to skip Rig-Builder-owned chains, a + // Drop Rig Builder stages from legacy saves. Before + // saveChainStateFromChain learned to filter Rig-Builder-owned stages, a // user chain action taken while Rig Builder's default tone was live - // persisted its wrap stages (unit-impulse trim IR, Final Leveler) into + // persisted its stages (amp, unit-impulse trim IR, Final Leveler) into // the saved chain; restoring those as plain processors resurrects a // tone this panel never built. Rewrite the cleaned list back so the // save self-heals. - const _cleaned = savedChain.filter((item) => !isRigBuilderChainStage(item)); + const _cleaned = savedChain.filter((item) => !aeIsRigBuilderStage(item)); if (_cleaned.length !== savedChain.length) { console.info('[audio-engine] Dropped', savedChain.length - _cleaned.length, - 'Rig Builder plumbing stage(s) from the saved chain.'); + 'Rig Builder stage(s) from the saved chain.'); try { localStorage.setItem('slopsmith-signal-chain', JSON.stringify(_cleaned)); } catch (_) {} savedChain = _cleaned; } @@ -1061,30 +1061,65 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; if (savedChain.length > 0) await refreshChain(); } - // Rig Builder plumbing markers. Its default-tone/preview wraps carry a - // 1-sample unit-impulse trim IR and an auto-appended "RB Final Leveler" - // stage — a chain containing either was built by the Rig Builder plugin, - // not by this panel. - const RB_PLUMBING_MARKER = /_rb_unit_impulse|RB Final Leveler/i; - function isRigBuilderChainStage(stage) { - return RB_PLUMBING_MARKER.test(String(stage?.path || '')) - || RB_PLUMBING_MARKER.test(String(stage?.name || '')); + // A chain stage that belongs to Rig Builder's tone (amp / pedals / racks / + // unit-impulse trim IR / master pre-post / the RB Final Leveler), NOT + // something the user added by hand here in the Audio menu. Rig Builder's + // preloader is always on and loads its whole tone into the SHARED engine + // chain; without this guard, "Save Current Chain" (and the auto-persist) + // captured the live engine via getChainState()/savePreset() and baked + // those stages into the user's manual chain — so on reload the user's own + // signal chain sprouted a full Rig Builder amp/pedal/rack rig they never + // added. + function aeIsRigBuilderStage(s) { + if (!s) return false; + const path = String(s.path || s.file || ''); + if (/[\\/]rig_builder[\\/]/i.test(path)) return true; // bundled RB amp/pedal/rack/leveler VST3 + const name = String(s.name || ''); + // Leveler + the 1-sample unit-impulse trim IR match by name or path so + // copies living outside the plugin dir are still recognized. + if (/RB Final Leveler|_rb_unit_impulse/i.test(name) + || /RB Final Leveler|_rb_unit_impulse/i.test(path)) return true; + const gear = String(s.rs_gear || s.rsGear || ''); + if (gear.indexOf('__rb') === 0) return true; // __rb_final_leveler__ / master sentinels + const slot = String(s.slot || ''); + if (slot === 'master_pre' || slot === 'master_post') return true; + return false; + } + + // Strip Rig Builder's stages out of the engine's native preset blob so a + // saved manual preset only carries the user's own processors (with their VST + // param state). The blob is JSON with a `chain` array; a user's manual chain + // is flat (no positional cross-refs) so filtering it is safe. + function aeStripRigBuilderFromNativePreset(nativePreset) { + try { + const isStr = typeof nativePreset === 'string'; + const obj = isStr ? JSON.parse(nativePreset) : nativePreset; + if (obj && Array.isArray(obj.chain)) { + obj.chain = obj.chain.filter(s => !aeIsRigBuilderStage(s)); + } + return isStr ? JSON.stringify(obj) : obj; + } catch (_) { + return nativePreset; // unparseable — leave untouched + } } function saveChainStateFromChain(chain) { - // Never persist a Rig-Builder-owned chain. Rig Builder reloads its - // default tone into the engine on its own schedule (song stop, screen - // leave), so it is routinely the ambient live chain while the user is - // in this panel; snapshotting it here would make the saved chain - // resurrect Rig Builder's tone on the next restore. Keep the last - // panel-built chain instead. - if (Array.isArray(chain) && chain.some(isRigBuilderChainStage)) return; + // Persist only the USER's stages. Rig Builder reloads its default tone + // into the engine on its own schedule (song stop, screen leave), so it + // is routinely the ambient live chain while the user is in this panel; + // snapshotting its stages would make the saved chain resurrect Rig + // Builder's tone on the next restore. Filtering (rather than skipping + // the save entirely) still captures processors the user stacked on top + // of a live Rig Builder tone — e.g. "add NAM while RB's amp is loaded" + // persists just the NAM. const typeMap = { 0: 'VST', 1: 'NAM', 2: 'IR' }; - const items = chain.filter(s => s.type === 0 || s.type === 1 || s.type === 2).map(s => ({ - type: typeMap[s.type] || 'VST', - path: s.path || '', - name: s.name || '', - })); + const items = chain + .filter(s => (s.type === 0 || s.type === 1 || s.type === 2) && !aeIsRigBuilderStage(s)) + .map(s => ({ + type: typeMap[s.type] || 'VST', + path: s.path || '', + name: s.name || '', + })); try { localStorage.setItem('slopsmith-signal-chain', JSON.stringify(items)); } catch (_) {} } @@ -1290,15 +1325,29 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; const chain = await api.getChainState(); container.innerHTML = ''; - if (chain.length === 0) { - container.innerHTML = '
No processors loaded — add a VST, NAM model, or cabinet IR
'; + // List only the user's own processors — Rig Builder's always-on tone is + // loaded into the same engine chain but it isn't part of the manual chain + // the user is building here (see aeIsRigBuilderStage). Rig Builder's + // stages are audible though, so never hide them SILENTLY: a user hearing + // an amp while this panel claims "No processors loaded" reads as a + // phantom tone. Surface a count note instead; the stages themselves are + // managed from the Rig Builder screen. + const all = Array.isArray(chain) ? chain : []; + const visible = all.filter(s => !aeIsRigBuilderStage(s)); + const rbCount = all.length - visible.length; + const rbNote = rbCount > 0 + ? `
+ ${rbCount} Rig Builder tone stage${rbCount === 1 ? '' : 's'} active (managed in Rig Builder)
` + : ''; + + if (visible.length === 0) { + container.innerHTML = '
No processors loaded — add a VST, NAM model, or cabinet IR
' + rbNote; return chain; } const typeNames = { 0: 'VST', 1: 'NAM', 2: 'IR' }; const typeColors = { 0: 'purple', 1: 'orange', 2: 'cyan' }; - for (const slot of chain) { + for (const slot of visible) { const color = typeColors[slot.type] || 'slate'; const div = document.createElement('div'); div.className = `flex items-center gap-3 p-3 rounded bg-slate-800/50 border border-${color}-500/30`; @@ -1318,6 +1367,11 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; `; container.appendChild(div); } + if (rbNote) { + const note = document.createElement('div'); + note.innerHTML = rbNote; + container.appendChild(note.firstChild); + } return chain; } @@ -1748,14 +1802,20 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; const doSave = async () => { const name = input.value.trim(); if (!name) return; - const nativePreset = await api.savePreset(); - if (!nativePreset) return; + const nativePresetRaw = await api.savePreset(); + if (!nativePresetRaw) return; + // Keep Rig Builder's always-on tone out of the user's manual + // preset (see aeIsRigBuilderStage) — both the native blob and the + // item list. + const nativePreset = aeStripRigBuilderFromNativePreset(nativePresetRaw); const chain = await api.getChainState(); - const items = chain.map(s => ({ - type: s.type === 0 ? 'VST' : s.type === 1 ? 'NAM' : 'IR', - path: s.path || '', - name: s.name || '', - })); + const items = chain + .filter(s => !aeIsRigBuilderStage(s)) + .map(s => ({ + type: s.type === 0 ? 'VST' : s.type === 1 ? 'NAM' : 'IR', + path: s.path || '', + name: s.name || '', + })); const gains = captureCurrentGainLevels(); const noiseGate = captureCurrentNoiseGateState(); const tonePolish = captureCurrentTonePolishState(); @@ -2254,6 +2314,10 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; } catch (_) { nativeChain = []; } for (let ci = 0; ci < chainItems.length; ci++) { const item = chainItems[ci]; + // Legacy polluted presets: skip Rig Builder stages a pre-fix build + // baked in (see aeIsRigBuilderStage). Skipping by index keeps the + // items ↔ nativeChain alignment intact for the remaining pairs. + if (aeIsRigBuilderStage(item) || aeIsRigBuilderStage(nativeChain[ci])) continue; let slotId = -1; if (item.type === 'NAM' && item.path) slotId = await api.loadNAMModel(item.path); else if (item.type === 'IR' && item.path) slotId = await api.loadIR(item.path); @@ -2311,7 +2375,23 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; try { await api.clearChain(); - const result = await api.loadPreset(preset.nativePreset); + // Clean any Rig Builder stages a pre-fix build baked into this saved + // preset (see aeIsRigBuilderStage). A preset that empties completely + // was 100% Rig Builder's tone (saved while only RB stages were live) + // — loading it empty is CORRECT: that tone belongs to Rig Builder, + // which reloads it on its own schedule, and falling back to the + // polluted blob would resurrect RB stages into the manual chain. + // Empty-chain presets are a supported shape (see + // songShouldRebuildChain's loadability notes). + const _nativeToLoad = aeStripRigBuilderFromNativePreset(preset.nativePreset); + try { + const before = JSON.parse(typeof preset.nativePreset === 'string' ? preset.nativePreset : '{}'); + const after = JSON.parse(typeof _nativeToLoad === 'string' ? _nativeToLoad : '{}'); + if (Array.isArray(before?.chain) && before.chain.length && Array.isArray(after?.chain) && after.chain.length === 0) { + console.warn(tag + ': preset contained only Rig Builder stages — loading it as an empty chain.'); + } + } catch (_) { /* diagnostics only */ } + const result = await api.loadPreset(_nativeToLoad); // Some JUCE bridges return {success:false} or bare false instead of throwing. if (result === false || (result && result.success === false)) { console.error(tag + ': loadPreset failed:', result?.error || 'unknown error'); @@ -2393,6 +2473,9 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; window._aeApplyPresetTonePolish = applyPresetTonePolish; window._aeLoadDefaultPreset = loadDefaultPreset; window._aeReplaceChainWithPresetBlob = replaceChainWithPresetBlob; + // Shared with IIFE 2's inline preload copy (the two IIFEs deliberately + // don't share scope) so its legacy-preset load skips Rig Builder stages too. + window._aeIsRigBuilderStage = aeIsRigBuilderStage; /** True when the song has tone-switching configured — a resolvable * global / per-song bypass mapping, or Tone Automation with a resolvable @@ -4802,8 +4885,14 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {}; const parsed = JSON.parse(preset.nativePreset || '{}').chain; if (Array.isArray(parsed)) nativeChain = parsed; } catch (_) { nativeChain = []; } + const _isRbStage = window._aeIsRigBuilderStage || (() => false); for (let ci = 0; ci < chainItems.length; ci++) { const item = chainItems[ci]; + // Legacy polluted presets: skip Rig Builder stages a + // pre-fix build baked in. Skipping by index keeps the + // items ↔ nativeChain alignment for remaining pairs + // (mirrors loadPresetItemsWithState in IIFE 1). + if (_isRbStage(item) || _isRbStage(nativeChain[ci])) continue; let slotId = -1; if (item.type === 'NAM' && item.path) slotId = await api.loadNAMModel(item.path); else if (item.type === 'IR' && item.path) slotId = await api.loadIR(item.path); diff --git a/tests/audio-chain-persistence.test.js b/tests/audio-chain-persistence.test.js index 17c6b32..55ff387 100644 --- a/tests/audio-chain-persistence.test.js +++ b/tests/audio-chain-persistence.test.js @@ -1,10 +1,11 @@ -// Guards against the alpha-tester chain-duplication bug: the audio_engine -// panel must (1) never persist a Rig-Builder-owned live chain into -// localStorage, (2) drop Rig Builder plumbing stages from legacy saves on -// restore, and (3) never auto-load (default preset / saved-chain restore) on -// top of an engine that already has a live chain — the native chain survives -// renderer re-evaluations, so an unconditional restore appended an exact -// duplicate of every stage (two amp stages in series = "gain blown out"). +// Guards against the alpha-tester chain-duplication/pollution bugs: the +// audio_engine panel must (1) persist only the USER's stages — never Rig +// Builder's always-on tone stages — into localStorage and named presets, +// (2) drop Rig Builder stages from legacy saves on restore, and (3) never +// auto-load (default preset / saved-chain restore) on top of an engine that +// already has a live chain — the native chain survives renderer +// re-evaluations, so an unconditional restore appended an exact duplicate of +// every stage (two amp stages in series = "gain blown out"). const test = require('node:test'); const assert = require('node:assert/strict'); @@ -15,14 +16,24 @@ const vm = require('node:vm'); const ROOT = path.join(__dirname, '..'); const SCREEN_JS = fs.readFileSync(path.join(ROOT, 'src', 'renderer', 'screen.js'), 'utf8'); -// Brace-balanced extraction of `function NAME(...) { ... }`. +// Brace-balanced extraction of `function NAME(...) { ... }`. Skips past the +// parameter list first (paren-balanced) so a destructured default parameter +// like `{ snapshot = true } = {}` isn't mistaken for the body's opening brace. function extractFunction(src, name) { const sig = `function ${name}(`; const start = src.indexOf(sig); assert.ok(start !== -1, `function '${name}' not found`); - const openBrace = src.indexOf('{', start); + let i = start + sig.length; + let parens = 1; + while (i < src.length && parens > 0) { + if (src[i] === '(') parens++; + else if (src[i] === ')') parens--; + i++; + } + assert.ok(parens === 0, `unbalanced parens in '${name}' signature`); + const openBrace = src.indexOf('{', i); let depth = 1; - let i = openBrace + 1; + i = openBrace + 1; while (i < src.length && depth > 0) { if (src[i] === '{') depth++; else if (src[i] === '}') depth--; @@ -32,14 +43,10 @@ function extractFunction(src, name) { return src.slice(start, i); } -// Extract the marker const + both helpers into one sandbox. function setupSandbox() { - const markerStart = SCREEN_JS.indexOf('const RB_PLUMBING_MARKER'); - assert.ok(markerStart !== -1, 'RB_PLUMBING_MARKER not found'); - const markerDecl = SCREEN_JS.slice(markerStart, SCREEN_JS.indexOf(';', markerStart) + 1); const code = [ - markerDecl, - extractFunction(SCREEN_JS, 'isRigBuilderChainStage'), + extractFunction(SCREEN_JS, 'aeIsRigBuilderStage'), + extractFunction(SCREEN_JS, 'aeStripRigBuilderFromNativePreset'), extractFunction(SCREEN_JS, 'saveChainStateFromChain'), ].join('\n'); const stored = new Map(); @@ -54,20 +61,17 @@ function setupSandbox() { } const RB_CHAIN = [ - { type: 0, path: '/vst/SamplegSBTCL.vst3', name: 'SamplegSBTCL' }, + { type: 0, path: '/plugins/rig_builder/vst/SamplegSBTCL.vst3', name: 'SamplegSBTCL' }, { type: 2, path: '/irs/_rb_unit_impulse.wav', name: '_rb_unit_impulse' }, { type: 0, path: '/vst/RB Final Leveler.vst3', name: 'RB Final Leveler' }, ]; -test('saveChainStateFromChain skips a Rig-Builder-owned live chain', () => { +test('saveChainStateFromChain persists only the user stages of a mixed chain', () => { const { sandbox, stored } = setupSandbox(); - stored.set('slopsmith-signal-chain', '[{"type":"NAM","path":"/nam/amp.nam","name":"amp"}]'); sandbox.saveChainStateFromChain([...RB_CHAIN, { type: 1, path: '/nam/vox.nam', name: 'VOX' }]); - assert.equal( - stored.get('slopsmith-signal-chain'), - '[{"type":"NAM","path":"/nam/amp.nam","name":"amp"}]', - 'the previously saved panel-built chain must be preserved', - ); + assert.deepEqual(JSON.parse(stored.get('slopsmith-signal-chain')), [ + { type: 'NAM', path: '/nam/vox.nam', name: 'VOX' }, + ], 'RB stages stripped, the user NAM stacked on top survives'); }); test('saveChainStateFromChain persists a panel-built chain normally', () => { @@ -83,12 +87,42 @@ test('saveChainStateFromChain persists a panel-built chain normally', () => { ]); }); -test('isRigBuilderChainStage matches plumbing by path or name only', () => { +test('aeIsRigBuilderStage recognizes every Rig Builder stage shape', () => { const { sandbox } = setupSandbox(); - assert.equal(sandbox.isRigBuilderChainStage({ path: '/irs/_rb_unit_impulse.wav' }), true); - assert.equal(sandbox.isRigBuilderChainStage({ name: 'RB Final Leveler' }), true); - assert.equal(sandbox.isRigBuilderChainStage({ path: '/vst/MyAmp.vst3', name: 'MyAmp' }), false); - assert.equal(sandbox.isRigBuilderChainStage(null), false); + const rb = sandbox.aeIsRigBuilderStage; + // Bundled RB gear by plugin-dir path (amps/pedals/racks), both separators. + assert.equal(rb({ path: '/plugins/rig_builder/vst/SamplegSBTCL.vst3' }), true); + assert.equal(rb({ path: 'C:\\plugins\\rig_builder\\vst\\Amp_AT20.vst3' }), true); + // Plumbing by name or path, wherever the file lives. + assert.equal(rb({ path: '/irs/_rb_unit_impulse.wav' }), true); + assert.equal(rb({ name: '_rb_unit_impulse' }), true); + assert.equal(rb({ name: 'RB Final Leveler' }), true); + assert.equal(rb({ path: '/vst/RB Final Leveler.vst3' }), true); + // Backend chain-spec sentinels. + assert.equal(rb({ rs_gear: '__rb_final_leveler__' }), true); + assert.equal(rb({ slot: 'master_pre' }), true); + assert.equal(rb({ slot: 'master_post' }), true); + // User gear is untouched. + assert.equal(rb({ path: '/vst/MyAmp.vst3', name: 'MyAmp' }), false); + assert.equal(rb({ path: '/nam/vox.nam', name: 'VOX' }), false); + assert.equal(rb(null), false); +}); + +test('aeStripRigBuilderFromNativePreset filters the blob chain, keeps user state', () => { + const { sandbox } = setupSandbox(); + const blob = JSON.stringify({ + chain: [ + { type: 0, path: '/plugins/rig_builder/vst/SamplegSBTCL.vst3', state: 'rb' }, + { type: 0, path: '/vst/MyReverb.vst3', state: 'user-params' }, + { type: 0, path: '/vst/x.vst3', slot: 'master_post', state: 'rb' }, + ], + gains: { input: 1 }, + }); + const out = JSON.parse(sandbox.aeStripRigBuilderFromNativePreset(blob)); + assert.deepEqual(out.chain, [{ type: 0, path: '/vst/MyReverb.vst3', state: 'user-params' }]); + assert.deepEqual(out.gains, { input: 1 }, 'non-chain fields pass through'); + // Unparseable blob passes through untouched. + assert.equal(sandbox.aeStripRigBuilderFromNativePreset('not json{'), 'not json{'); }); test('init auto-load is gated on an empty engine chain (re-evaluation guard)', () => { @@ -106,10 +140,13 @@ test('init auto-load is gated on an empty engine chain (re-evaluation guard)', ( ); }); -test('restore drops Rig Builder plumbing stages from legacy saves', () => { - // aeRestoreSavedChain is async and coupled to the api bridge; assert the - // sanitize step is present and rewrites the cleaned save. - const fn = extractFunction(SCREEN_JS, 'aeRestoreSavedChain'); - assert.equal(fn.includes('isRigBuilderChainStage'), true); - assert.equal(fn.includes("localStorage.setItem('slopsmith-signal-chain', JSON.stringify(_cleaned))"), true); +test('restore self-heals legacy saves and preset save/load strip RB stages', () => { + const restore = extractFunction(SCREEN_JS, 'aeRestoreSavedChain'); + assert.equal(restore.includes('aeIsRigBuilderStage'), true); + assert.equal(restore.includes("localStorage.setItem('slopsmith-signal-chain', JSON.stringify(_cleaned))"), true); + // Save Current Chain strips both the native blob and the item list… + assert.equal(SCREEN_JS.includes('aeStripRigBuilderFromNativePreset(nativePresetRaw)'), true); + // …and the preset-load path sanitizes legacy polluted presets. + const load = extractFunction(SCREEN_JS, 'replaceChainWithPresetBlob'); + assert.equal(load.includes('aeStripRigBuilderFromNativePreset(preset.nativePreset)'), true); });