diff --git a/CHANGELOG.md b/CHANGELOG.md index 26cda1b7..ec2ec959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `editor_track_session` schema is bumped to v3 (purely additive β€” v2 trees carry no regions and need no migration). Rendering, playback, and build are unchanged and land in later steps. +- **The drums track is now an ordinary mixer / Tracks channel.** Building on the + drums-as-arrangement work below, the drum chart's mixer strip and Tracks mix now + use the same per-arrangement channel address every other part does, instead of a + one-off "drums" slot. Mute / solo / volume on the drums strip behave exactly like + a pitched track's β€” including in multi-track ("play all") playback, where the drum + kit now follows its own strip β€” and the drum grid's guide claps follow that strip + too. With a single drum chart you won't see a difference (its durable track + identity is unchanged, so delete/undo, rename, and pairing all work as before); + this is the wiring that lets *several* drum charts each get their own strip. + - **Drums are now a selectable part in the arrangement switcher.** Pick **"πŸ₯ Drums"** from the part dropdown to open the drum editor β€” exactly like switching to Lead, Rhythm, or Bass. The drum chart is no longer a mode tucked diff --git a/src/audio.js b/src/audio.js index 463e68e4..9db0191d 100644 --- a/src/audio.js +++ b/src/audio.js @@ -31,6 +31,7 @@ import { host } from './host.js'; import { _pickOnsetsPure, _spectralFluxOnsetsPlan, _spectralFluxStep } from './onsets.js'; import { _tourNoteAction } from './tour.js'; import { _rollMidiForNote, _rollPitchCtx, _rollPitchCtxFor, midiToFreq } from './keys.js'; +import { drumArrangementIndex, isDrumArrangement } from './drum-arrangement.js'; import { arrKind } from './instrument.js'; import { _recState } from './midi-record.js'; import { notes } from './notes.js'; @@ -1702,19 +1703,24 @@ function _guidePitchedEvents() { // identical. Drum parts clap (GM percussion is a follow-up). // // The band roster: one entry per mixable part, in strip order β€” the SAME -// keys the mixer panel uses ('arr:' / 'drums'), so the strips and the -// engine can never disagree about who is who. +// `arr:` keys the mixer panel uses (the drums arrangement included), so +// the strips and the engine can never disagree about who is who. function _bandPartsPure(arrangements, drumTab) { const out = []; (arrangements || []).forEach((a, i) => { - // The drums arrangement plays through the `'drums'` band key (from - // drumTab) below, not as an `arr:` part β€” skip it so it doesn't add - // a phantom (empty-note) band entry. + // The drums arrangement is appended below with the drum tab as its + // payload (its own notes are empty) β€” skip the plain arr pass so it + // isn't added twice. if (a && a.type === 'drums') return; if (a) out.push({ key: 'arr:' + i, idx: i, name: a.name || ('Track ' + (i + 1)) }); }); if (drumTab && Array.isArray(drumTab.hits) && drumTab.hits.length) { - out.push({ key: 'drums', idx: -1, name: 'Drums' }); + // The drum part rides its arrangement's own `arr:` channel now + // (PR2b) β€” the SAME key the mixer strip uses, so the strip and the + // engine agree. `idx` points at the drums arrangement so the scheduler + // resolves it; fall back to the legacy key if it isn't materialized. + const di = drumArrangementIndex(arrangements); + out.push({ key: di >= 0 ? 'arr:' + di : 'drums', idx: di, name: 'Drums' }); } return out; } @@ -1966,7 +1972,8 @@ export function _stemCatchupPure(playStartTime, playStartWall, currentTime, rate // playing node, drop its gain node, and β€” the one that bites β€” delete its // 'audio:' entry from S.partMix. That entry is counted by the whole-map // solo rule, so a stale SOLO left behind by a removed stem would silence every -// live track. Mirrors the drum-delete path (delete S.partMix.drums). +// live track. Same hazard the arrangement-delete path guards against by +// renumbering the arr: keys (see _partMixDropArrangementPure). export function _pruneStaleStems(liveIds) { for (const id of [...playingStemSources.keys()]) { if (liveIds.has(id)) continue; @@ -2335,10 +2342,19 @@ function _guideTick() { const target = _ensurePartGain(part.key); if (!target) continue; const arr = part.idx >= 0 ? S.arrangements[part.idx] : null; - // A drum-ENCODED arrangement (created/imported/legacy "Drums" part β€” - // no pitch, so _bandPartPitchedEvents returns []) claps its rhythm - // through this part's gain, else it voices neither GM nor clap and - // goes silent (review #280 follow-up; GM percussion here is a follow-up). + // The drum-grid arrangement (type:"drums") voices real GM percussion + // from the drum tab through this part's gain (review #282). Its own + // notes are empty, so it must be caught BEFORE the clap-notes path + // below. `part.key === 'drums'` is the defensive fallback for an + // un-materialized tab (di < 0 in the roster). + if (part.key === 'drums' || (arr && isDrumArrangement(arr))) { + _drumKitVoicesInWindow(from, to, target, 1); + continue; + } + // A drum-ENCODED pitched part (a legacy "Drums"-named arrangement with + // real notes, not type:"drums") claps its rhythm through this part's + // gain, else it voices neither GM nor clap and goes silent (review + // #280 follow-up; GM percussion here is a follow-up). if (arr && arrKind(arr) === 'drums') { const times = _guideSanitizeTimesPure((arr.notes || []).map(n => n.time)); for (const t of _guideClapTimesInWindowPure(times, from, to)) { @@ -2349,11 +2365,6 @@ function _guideTick() { } continue; } - // The drum-grid sidecar plays real GM percussion (review #282). - if (part.key === 'drums') { - _drumKitVoicesInWindow(from, to, target, 1); - continue; - } const gm = editorGmVoiceFor(_gmKindPure(arrKind(arr))); const ready = gm !== null && gmPresetReady(gm); if (gm !== null && !ready) ensureGmPreset(gm, S.audioCtx); // clap while it loads diff --git a/src/host.js b/src/host.js index eecc27e6..daa66fa2 100644 --- a/src/host.js +++ b/src/host.js @@ -211,8 +211,8 @@ export const host = { /** Mixer strip keys in Tracks-column row order (mixer follows a reorder). */ mixerTrackOrder: () => [], /** - * Per-part strip state BY KEY ('arr:' / 'drums') for band-mode - * MIDI playback: {audible, vol 0..1} with the whole-map solo rule. + * Per-part strip state BY KEY ('arr:', the drums arrangement included) + * for band-mode MIDI playback: {audible, vol 0..1} with the whole-map solo rule. * Owned by src/mixer-panel.js; inert default = every part at unity. */ partStripState: () => ({ audible: true, vol: 1 }), diff --git a/src/mixer-panel.js b/src/mixer-panel.js index 1f0d9f0e..4f7dee6b 100644 --- a/src/mixer-panel.js +++ b/src/mixer-panel.js @@ -7,7 +7,7 @@ // (recording / guide / click) and the edit blip. // // This module owns the CANONICAL per-part mix state, `S.partMix` β€” a map from -// part key ('arr:' for arrangements, 'drums' for the drum tab) to +// part key ('arr:' for arrangements, the drums arrangement included) to // { vol, mute, solo }. Today the only per-part sound is the guide voice (claps // follow the active editing surface), so mute/solo/volume gate and scale the // guide claps for the part being edited; the Parts-gutter M/S/A (Β§2.5) and @@ -25,15 +25,17 @@ // Part mute/solo/volume is SESSION state β€” it resets with the loaded song // (create.js / file-ops.js clear `S.partMix` when they install arrangements). // ════════════════════════════════════════════════════════════════════ +import { drumArrangementIndex } from './drum-arrangement.js'; import { host } from './host.js'; import { S, editGen } from './state.js'; import { _editorEscHtml, setStatus } from './ui.js'; -import { isDrumArrangement } from './drum-arrangement.js'; /* @pure:mixer-panel:start */ -// One strip per part: every arrangement, plus the drum tab as its own strip -// (drums are a song-level sidecar, not an arrangement) β€” the same list shape -// as the Parts view, keyed the way S.currentArr addresses parts (by index). +// One strip per part, keyed the way S.currentArr addresses parts (by index). +// The drums arrangement is an ordinary `type:"drums"` entry in `arrangements` +// now (its strip is `arr:` like any other part β€” no `'drums'` singleton), +// so a single pass over the arrangements covers it. `drumTab` is unused here; +// it stays in the signature so callers match the sibling roster builders. export function _mixerPartsPure(arrangements, drumTab, stems, removedSourceIds, master) { const parts = []; // The master mix leads the audio band as its own channel strip (keyed @@ -56,17 +58,11 @@ export function _mixerPartsPure(arrangements, drumTab, stems, removedSourceIds, parts.push({ key: 'audio:' + id, name: stem.name || id, kind: 'audio' }); } (arrangements || []).forEach((arr, i) => { - // The drums arrangement gets its OWN 'drums' strip below (from drumTab), - // not an 'arr:' strip β€” skip it here so drums don't show twice. - if (isDrumArrangement(arr)) return; parts.push({ key: 'arr:' + i, name: (arr && arr.name) || 'Track ' + (i + 1), }); }); - if (drumTab && Array.isArray(drumTab.hits) && drumTab.hits.length) { - parts.push({ key: 'drums', name: 'Drums' }); - } return parts; } // Fader positions run 0..110: 0..100 is linear to unity, 101..110 adds @@ -140,11 +136,19 @@ export function _mixerPartAudiblePure(partMix, key) { if (key === 'audio:master') return true; return _mixerAnySoloPure(partMix) ? st.solo : true; } +// The mix key of the ACTIVE editing surface: the drums arrangement's channel +// while the drum grid is open (`arr:` β€” currentArr itself stays on a +// pitched arrangement, #337), else the current pitched arrangement. +export function _mixerActivePartKeyPure(drumEditMode, currentArr, drumIdx) { + return (drumEditMode && Number(drumIdx) >= 0) + ? 'arr:' + drumIdx + : 'arr:' + (Number(currentArr) || 0); +} // What the guide-clap scheduler needs for the ACTIVE editing surface: claps // follow the drum grid in drum mode, the current arrangement otherwise, so // that surface's part decides whether (and how loud) the claps sound. -export function _mixerClapStatePure(partMix, drumEditMode, currentArr) { - const key = drumEditMode ? 'drums' : 'arr:' + (Number(currentArr) || 0); +export function _mixerClapStatePure(partMix, drumEditMode, currentArr, drumIdx) { + const key = _mixerActivePartKeyPure(drumEditMode, currentArr, drumIdx); return { audible: _mixerPartAudiblePure(partMix, key), vol: _mixerGainForFaderPure(_mixerPartStatePure(partMix, key).vol), @@ -158,7 +162,7 @@ export function _mixerOpenFromStoredPure(raw) { // The host-hook target audio.js consults per scheduled clap voice. export function _mixerClapState() { - return _mixerClapStatePure(S.partMix, S.drumEditMode, S.currentArr); + return _mixerClapStatePure(S.partMix, S.drumEditMode, S.currentArr, drumArrangementIndex(S.arrangements)); } // Band mode's per-KEY twin (host.partStripState): {audible, vol 0..1} for @@ -199,7 +203,8 @@ function _selectedStripKeyPure() { if (!selected) return ''; if (selected.type === 'audio') return 'audio:' + selected.sourceId; if (selected.type === 'transcription') { - if (selected.targetId === 'drums') return 'drums'; + // The drums arrangement resolves through the same idβ†’index path as any + // other part (its id is 'drums', so targetId 'drums' β†’ its arr:). const idx = (S.arrangements || []) .findIndex((arr, i) => String((arr && arr.id) || ('arr:' + i)) === selected.targetId); return idx >= 0 ? 'arr:' + idx : ''; @@ -263,7 +268,7 @@ export function _mixerMeterPeakPure(key, levels, activeAudioId, activePart) { } function _meterPeakForKey(key, levels) { - const activePart = S.drumEditMode ? 'drums' : 'arr:' + (Number(S.currentArr) || 0); + const activePart = _mixerActivePartKeyPure(S.drumEditMode, S.currentArr, drumArrangementIndex(S.arrangements)); return _mixerMeterPeakPure(key, levels, S.activeAudioSourceId, activePart); } @@ -278,7 +283,7 @@ export function _mixerMeterInputPure(key, levels, activeAudioId, activePart, pla } function _meterInputForKey(key, levels) { - const activePart = S.drumEditMode ? 'drums' : 'arr:' + (Number(S.currentArr) || 0); + const activePart = _mixerActivePartKeyPure(S.drumEditMode, S.currentArr, drumArrangementIndex(S.arrangements)); return _mixerMeterInputPure(key, levels, S.activeAudioSourceId, activePart, host.playAllTracksEnabled()); } diff --git a/src/state.js b/src/state.js index 71791ca5..cd7fa7a6 100644 --- a/src/state.js +++ b/src/state.js @@ -59,8 +59,9 @@ export const S = { partsViewMode: false, drumSel: new Set(), - // Per-part mix state (mixer panel, B6) β€” 'arr:' / 'drums' β†’ - // { vol, mute, solo }. Session-scoped UI state (never the pack): the + // Per-part mix state (mixer panel, B6) β€” 'arr:' (the drums + // arrangement included) β†’ { vol, mute, solo }. Session-scoped UI state + // (never the pack): the // canonical source for part mute/solo/volume that the mixer strips, // the guide-clap gate (via host.partClapState) and the future // Parts-gutter M/S/A all read. Reset when a song is installed. diff --git a/src/track-session.js b/src/track-session.js index 819dd895..5acff9a7 100644 --- a/src/track-session.js +++ b/src/track-session.js @@ -32,7 +32,7 @@ // ════════════════════════════════════════════════════════════════════ import { host } from './host.js'; import { _renameGuardPure } from './arrangement.js'; -import { isDrumArrangement, syncDrumArrangement } from './drum-arrangement.js'; +import { drumArrangementIndex, isDrumArrangement, syncDrumArrangement } from './drum-arrangement.js'; import { _partViewKeyPure } from './keys.js'; import { arrKind, _arrTypeKind } from './instrument.js'; import { _mixerPanelRefresh, _mixerPartStatePure, mixerSetPart, mixerTogglePart } from './mixer-panel.js'; @@ -111,7 +111,8 @@ export function _clickSourcePure(sources) { // The transcription targets: every arrangement plus the drum tab. targetId // is the durable chart-track key (shared with stemLinks); mixKey is the -// session address partMix / band mode speak ('arr:' / 'drums'). +// session address partMix / band mode speak β€” always 'arr:' now, the +// drums arrangement included (addressed by its own index). export function _trackSessionTargetsPure(arrangements, drumTab) { const out = []; const seen = new Set(); @@ -130,7 +131,17 @@ export function _trackSessionTargetsPure(arrangements, drumTab) { out.push({ id, name: String(arr.name || ('Track ' + (index + 1))).slice(0, 120), mixKey: 'arr:' + index }); }); if (drumTab && Array.isArray(drumTab.hits)) { - out.push({ id: DRUM_TARGET_ID, name: String(drumTab.name || 'Drums').slice(0, 120), mixKey: 'drums' }); + // The drums arrangement's mix channel is `arr:` like every other + // part (retiring the `'drums'` mix-key singleton) β€” but its durable + // TARGET id stays 'drums', so the tracks-row / stemLinks / delete / + // rename hooks that key on it are untouched. Fall back to 'drums' only + // if the arrangement hasn't been materialized (defensive; it always is). + const di = drumArrangementIndex(arrangements); + out.push({ + id: DRUM_TARGET_ID, + name: String(drumTab.name || 'Drums').slice(0, 120), + mixKey: di >= 0 ? 'arr:' + di : 'drums', + }); } return out; } @@ -149,6 +160,22 @@ export function _partMixDropArrangementPure(partMix, index) { return out; } +// Inverse of _partMixDropArrangementPure: open a slot at `index` (renumber every +// arr: with n >= index UP one) and drop `strip` in at arr:. Renumbers +// the CURRENT map, so any live mute/solo/volume edits made while the slot was +// gone ride along to their restored key instead of being clobbered. +export function _partMixInsertArrangementPure(partMix, index, strip) { + const out = {}; + for (const [key, value] of Object.entries(partMix && typeof partMix === 'object' ? partMix : {})) { + if (!key.startsWith('arr:')) { out[key] = value; continue; } + const n = Number(key.slice(4)); + if (!Number.isInteger(n)) continue; + out[n >= index ? 'arr:' + (n + 1) : key] = value; + } + if (strip !== undefined) out['arr:' + index] = strip; + return out; +} + // Normalize any persisted/half-trusted tree against the loaded song: drop // rows whose source/target no longer exists, append rows for anything new, // repair parent cycles, and default the tempo guide. Idempotent β€” this is @@ -883,21 +910,41 @@ export class DeleteDrumTabCmd { this._name = rowName; this._tab = S.drumTab; this._dirty = !!S.drumTabDirty; - this._hadMixStrip = !!(S.partMix && ('drums' in S.partMix)); - this._mixStrip = S.partMix ? S.partMix.drums : undefined; + // Snapshot everything the splice renumbers so undo is a TRUE inverse: + // deleting a drums arrangement that sits mid-list shifts every later + // arrangement (and its `arr:` mix strip) down a slot. Undo must + // put drums back in its ORIGINAL slot β€” NOT re-append it last β€” or the + // index-based undo commands, currentArr, and mix keys that ride on the + // old order would all point one arrangement off after the delete is undone. + this._drumIndex = drumArrangementIndex(S.arrangements); + this._drumStrip = (S.partMix && this._drumIndex >= 0) ? S.partMix['arr:' + this._drumIndex] : undefined; + this._prevCurrentArr = S.currentArr; this._links = S.stemLinks; this._tree = S.trackSession; this._selectedTrackId = S.selectedTrackId; this._selectedRegionId = S.selectedRegionId; } exec() { + // Capture the drum slot BEFORE syncDrumArrangement splices it out β€” + // dropping it must renumber the higher pitched arr: keys down a + // slot, same as the pitched-delete path. A bare delete stranded every + // strip after drums (a stranded solo silenced the whole band). + const drumIndex = drumArrangementIndex(S.arrangements); S.drumTab = null; syncDrumArrangement(S); // remove the derived type:"drums" arrangement // Dirty is what ships the explicit `drum_tab: null` removal on the // next save (see _buildSaveBody) β€” without it the backend's // absentβ†’preserve path would resurrect drum_tab.json on reload. S.drumTabDirty = true; - if (S.partMix) delete S.partMix.drums; + // drumIndex is -1 if the drums arrangement was never materialized (the + // 'drums' target row exists off drumTab.hits alone) β€” dropping index -1 + // would renumber EVERY arr: down and corrupt the map, so only renumber + // when a real slot was found. + if (S.partMix && drumIndex >= 0) S.partMix = _partMixDropArrangementPure(S.partMix, drumIndex); + // The splice shifted every arrangement after drums down one slot; follow + // the selection so currentArr stays in bounds (and off drums) while + // drums is gone. Undo restores it from the snapshot. Mirrors the key renumber. + if (drumIndex >= 0 && drumIndex < S.currentArr) S.currentArr -= 1; S.stemLinks = _trackLinksRetargetPure(S.stemLinks, DRUM_TARGET_ID); if (S.selectedTrackId === transcriptionTrackId(DRUM_TARGET_ID)) { S.selectedTrackId = ''; @@ -908,12 +955,25 @@ export class DeleteDrumTabCmd { } rollback() { S.drumTab = this._tab; - syncDrumArrangement(S); // restore the derived type:"drums" arrangement + const arr = syncDrumArrangement(S); // re-materialize (syncDrumArrangement appends LAST) + // Move drums back to its ORIGINAL slot so every later arrangement + // returns to its pre-delete index β€” otherwise index-based undo commands + // (and currentArr) would target the wrong arrangement after this undo. + if (arr && this._drumIndex >= 0) { + const cur = S.arrangements.indexOf(arr); + if (cur !== this._drumIndex) { + S.arrangements.splice(cur, 1); + S.arrangements.splice(this._drumIndex, 0, arr); + } + } S.drumTabDirty = this._dirty; - if (this._hadMixStrip) { - if (!S.partMix) S.partMix = {}; - S.partMix.drums = this._mixStrip; + // Re-open the drum slot and shift the surviving strips back UP β€” the + // inverse of exec's drop. Operates on the CURRENT map, so live mix edits + // made while drums was gone survive the undo (they aren't history commands). + if (this._drumIndex >= 0 && S.partMix) { + S.partMix = _partMixInsertArrangementPure(S.partMix, this._drumIndex, this._drumStrip); } + S.currentArr = this._prevCurrentArr; S.stemLinks = this._links; if (this._selectedTrackId === transcriptionTrackId(DRUM_TARGET_ID) && !S.selectedTrackId) { diff --git a/tests/drum_arrangement.test.mjs b/tests/drum_arrangement.test.mjs index 9d000497..02b8bfba 100644 --- a/tests/drum_arrangement.test.mjs +++ b/tests/drum_arrangement.test.mjs @@ -147,16 +147,19 @@ t('load β†’ save round-trips the pitched arrangements byte-identically', () => { }); // ── no duplicate tracks/band rows for drums ─────────────────────────── -t('_trackSessionTargetsPure: the drums arrangement does NOT add an arr target β€” drums stay the legacy "drums" target', () => { +t('_trackSessionTargetsPure: the drums arrangement mixes on arr: but keeps its durable "drums" target id (PR2b)', () => { const payload = tab(); const S = { arrangements: [gtr('Lead')], drumTab: payload }; - syncDrumArrangement(S); // arrangements = [Lead, drumsArr] + syncDrumArrangement(S); // arrangements = [Lead, drumsArr] β†’ drums is idx 1 const targets = _trackSessionTargetsPure(S.arrangements, S.drumTab); - const drumTargets = targets.filter(x => x.mixKey === 'drums'); - assert.strictEqual(drumTargets.length, 1, 'exactly one drum target'); - assert.strictEqual(drumTargets[0].id, 'drums'); - assert.ok(!targets.some(x => x.mixKey === 'arr:1'), 'the drums arrangement (idx 1) gets NO arr:1 target'); - assert.strictEqual(targets.filter(x => x.mixKey.startsWith('arr:')).length, 1, 'only the pitched part is an arr target'); + const drumTargets = targets.filter(x => x.id === 'drums'); + assert.strictEqual(drumTargets.length, 1, 'exactly one drum target (no duplicate row)'); + assert.strictEqual(drumTargets[0].mixKey, 'arr:1', + 'the drums target now mixes on the drums arrangement index, not the legacy "drums" key'); + assert.ok(!targets.some(x => x.mixKey === 'drums'), 'the legacy "drums" mix-key singleton is retired'); + // Both parts mix on arr: now (the pitched part from the loop; drums from + // the appended target), but only ONE carries the durable "drums" id. + assert.strictEqual(targets.filter(x => x.mixKey.startsWith('arr:')).length, 2, 'both parts mix on arr:'); }); t('_trackSessionTargetsPure: a pitched part NAMED "Drums" still gets its arr target (not filtered)', () => { diff --git a/tests/drum_delete_undo.test.mjs b/tests/drum_delete_undo.test.mjs index 2686c882..56b8bdfe 100644 --- a/tests/drum_delete_undo.test.mjs +++ b/tests/drum_delete_undo.test.mjs @@ -28,6 +28,7 @@ globalThis.document = globalThis.document || { getElementById: () => null }; const { DeleteDrumTabCmd } = await import('../src/track-session.js'); const { EditHistory } = await import('../src/history.js'); const { _buildSaveBody } = await import('../src/file-ops.js'); +const { syncDrumArrangement } = await import('../src/drum-arrangement.js'); const { S } = await import('../src/state.js'); let pass = 0, fail = 0; @@ -47,13 +48,16 @@ function seed() { }], currentArr: 0, beats: [], sections: [], drumTab: tab, drumTabDirty: false, - partMix: { drums: { audible: false, vol: 0.5 }, 'arr:0': { audible: true, vol: 1 } }, + // The drums arrangement materializes at idx 1 (Lead is 0), so its mix + // strip is keyed 'arr:1' now β€” the retired 'drums' singleton (PR2b). + partMix: { 'arr:1': { audible: false, vol: 0.5 }, 'arr:0': { audible: true, vol: 1 } }, stemLinks: { drums: 'Drums_stem', lead: 'Guitar_L' }, trackSession: null, trackHeights: {}, stems: [], sessionDirty: false, history: new EditHistory(), sel: new Set(), }); + syncDrumArrangement(S); // materialize the type:"drums" arrangement (idx 1) return tab; } @@ -62,7 +66,7 @@ t('delete execs as one history command β€” the stack survives', () => { S.history.exec(new DeleteDrumTabCmd('Drums')); assert.strictEqual(S.drumTab, null, 'tab cleared'); assert.strictEqual(S.drumTabDirty, true, 'dirty β†’ the removal ships on the next save'); - assert.strictEqual('drums' in S.partMix, false, 'mixer strip dropped'); + assert.strictEqual('arr:1' in S.partMix, false, 'mixer strip dropped'); assert.strictEqual(S.stemLinks.drums, undefined, 'pairing retargeted away'); assert.strictEqual(S.stemLinks.lead, 'Guitar_L', 'other pairings untouched'); assert.strictEqual(S.history.undo.length, 1, 'the delete IS on the undo stack'); @@ -77,7 +81,7 @@ t('undo restores the very same tab object, flags, strip, and pairing', () => { 'IDENTITY restore β€” older drum commands hold references into these hits'); assert.strictEqual(S.drumTabDirty, false, 'a disk-clean tab returns clean, so an unrelated save does not re-serialize it'); - assert.deepStrictEqual(S.partMix.drums, { audible: false, vol: 0.5 }, 'mixer strip back'); + assert.deepStrictEqual(S.partMix['arr:1'], { audible: false, vol: 0.5 }, 'mixer strip back'); assert.strictEqual(S.stemLinks, linksBefore, 'pairing map reference restored'); assert.strictEqual(S.history.redo.length, 1, 'redo holds the delete'); }); @@ -89,7 +93,7 @@ t('redo re-deletes; a second undo restores again (round-trip stability)', () => S.history.doRedo(); assert.strictEqual(S.drumTab, null); assert.strictEqual(S.drumTabDirty, true); - assert.strictEqual('drums' in S.partMix, false); + assert.strictEqual('arr:1' in S.partMix, false); S.history.doUndo(); assert.strictEqual(S.drumTab, tab); assert.strictEqual(S.drumTabDirty, false); diff --git a/tests/drum_delete_undo_middle.test.mjs b/tests/drum_delete_undo_middle.test.mjs new file mode 100644 index 00000000..ba88f5b8 --- /dev/null +++ b/tests/drum_delete_undo_middle.test.mjs @@ -0,0 +1,153 @@ +/* + * Regression: deleting the drum transcription when it is NOT the last + * arrangement (a pitched arrangement sits after it). + * + * The drums channel is `arr:` (PR2b). Splicing drums out shifts every + * higher pitched arrangement down one slot, so its `arr:` mix key must + * shift down too β€” exactly what the pitched-delete path does via + * _partMixDropArrangementPure. A bare `delete S.partMix['arr:']` + * stranded the pitched strips after drums (lost mute/solo/vol; a stranded solo + * silenced the whole band) and undo then OVERWROTE the stranded strip. + * + * Roster: Lead(arr:0) + Drums(arr:1) + Bass(arr:2). + * + * Run: node tests/drum_delete_undo_middle.test.mjs + */ +import assert from 'node:assert'; + +globalThis.localStorage = globalThis.localStorage || { + getItem: () => null, setItem: () => {}, removeItem: () => {}, +}; +globalThis.document = globalThis.document || { getElementById: () => null }; + +const { DeleteDrumTabCmd } = await import('../src/track-session.js'); +const { EditHistory } = await import('../src/history.js'); +const { syncDrumArrangement, isDrumArrangement } = await import('../src/drum-arrangement.js'); +const { _mixerAnySoloPure } = await import('../src/mixer-panel.js'); +const { S } = await import('../src/state.js'); + +let pass = 0, fail = 0; +function t(name, fn) { + try { fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +function seed() { + const pitched = () => ({ tuning: [0, 0, 0, 0, 0, 0], capo: 0, notes: [], chords: [], chord_templates: [] }); + const tab = { name: 'Drums', version: 1, hits: [{ t: 0.5, lane: 'kick' }] }; + Object.assign(S, { + sessionId: 'sess-1', createMode: false, format: 'sloppak', sloppakForm: 'zip', + filename: 'song.feedpak', title: 'T', artist: 'A', + // Lead at 0; syncDrumArrangement appends drums at 1; Bass added at 2 AFTER. + arrangements: [{ name: 'Lead', ...pitched() }], + currentArr: 0, beats: [], sections: [], + drumTab: tab, drumTabDirty: false, + partMix: { + 'arr:0': { audible: true, vol: 1 }, // Lead + 'arr:1': { audible: false, vol: 0.5 }, // Drums (mute-ish) + }, + stemLinks: {}, trackSession: null, trackHeights: {}, stems: [], + sessionDirty: false, history: new EditHistory(), sel: new Set(), + }); + syncDrumArrangement(S); // drums β†’ arr:1 + // Now add a pitched arrangement AFTER drums so drums is in the middle. + S.arrangements.push({ name: 'Bass', ...pitched() }); // Bass β†’ arr:2 + S.partMix['arr:2'] = { audible: true, vol: 0.8, solo: true }; + return tab; +} + +t('delete drums-in-the-middle renumbers higher pitched strips down', () => { + seed(); + S.history.exec(new DeleteDrumTabCmd('Drums')); + // Bass's arrangement is now index 1; its strip must follow to arr:1. + assert.deepStrictEqual(S.partMix['arr:1'], { audible: true, vol: 0.8, solo: true }, + 'Bass strip renumbered arr:2 β†’ arr:1'); + assert.strictEqual('arr:2' in S.partMix, false, 'no stray arr:2 left behind'); + assert.strictEqual(_mixerAnySoloPure(S.partMix), true, + 'the surviving solo stays on a LIVE strip β€” band not silenced'); +}); + +t('undo restores every strip AND the original arrangement order', () => { + seed(); + S.history.exec(new DeleteDrumTabCmd('Drums')); + S.history.doUndo(); + // True inverse: drums returns to its ORIGINAL middle slot (arr:1) and Bass + // back at arr:2 β€” NOT re-appended last, so index-based undo/mix keys stay valid. + assert.deepStrictEqual(S.partMix['arr:0'], { audible: true, vol: 1 }, 'Lead intact'); + assert.deepStrictEqual(S.partMix['arr:1'], { audible: false, vol: 0.5 }, + 'drums strip restored at its original middle slot'); + assert.deepStrictEqual(S.partMix['arr:2'], { audible: true, vol: 0.8, solo: true }, + 'Bass strip (solo+vol) restored at its original slot'); + assert.strictEqual(isDrumArrangement(S.arrangements[1]), true, 'drums back in the middle'); + assert.strictEqual(S.arrangements[2].name, 'Bass', 'Bass back last β€” order preserved'); +}); + +t('an index-based undo command survives a drums-in-the-middle delete', () => { + seed(); + S.currentArr = 2; // Bass selected (sits AFTER drums) + const bass = S.arrangements[2]; + // Stand-in for MoveNoteCmd et al: they target S.arrangements[arrIdx] by index. + let touched = null; + S.history.exec({ + arrIdx: 2, + exec() { touched = S.arrangements[this.arrIdx]; }, + rollback() { touched = S.arrangements[this.arrIdx]; }, + }); + S.history.exec(new DeleteDrumTabCmd('Drums')); // drums-in-the-middle removed + S.history.doUndo(); // un-delete drums + S.history.doUndo(); // the note command rolls back + assert.strictEqual(touched, bass, + 'index-based command still targets Bass, not the re-appended drums arrangement'); + assert.strictEqual(isDrumArrangement(touched), false, 'never lands on drums'); +}); + +t('currentArr follows the selected pitched arrangement across delete + undo', () => { + seed(); + S.currentArr = 2; // Bass selected β€” sits AFTER drums (arr:1) + S.history.exec(new DeleteDrumTabCmd('Drums')); + // Drums spliced out shifts Bass 2β†’1; currentArr must follow, not dangle + // out of bounds (arrangements.length is now 2). + assert.ok(S.arrangements[S.currentArr] && !isDrumArrangement(S.arrangements[S.currentArr]), + 'currentArr points at a live pitched arrangement, not out of bounds'); + assert.strictEqual(S.arrangements[S.currentArr].name, 'Bass', 'still Bass selected after delete'); + S.history.doUndo(); + // True-inverse undo restores the original slot, so currentArr returns to 2. + assert.strictEqual(S.currentArr, 2, 'currentArr restored to Bass original slot'); + assert.ok(!isDrumArrangement(S.arrangements[S.currentArr]), + 'currentArr is not the drums arrangement'); + assert.strictEqual(S.arrangements[S.currentArr].name, 'Bass', 'still Bass selected after undo'); +}); + +t('a live mix edit made while drums is gone survives the undo', () => { + seed(); + S.history.exec(new DeleteDrumTabCmd('Drums')); // Bass now at arr:1 + // Mixer/fader edits mutate S.partMix in place, outside EditHistory. + S.partMix['arr:1'] = { audible: true, vol: 0.3, solo: false }; // user re-faders Bass + S.history.doUndo(); + // Bass returns to arr:2 carrying the LIVE edit, not the stale seeded value. + assert.deepStrictEqual(S.partMix['arr:2'], { audible: true, vol: 0.3, solo: false }, + 'the post-delete Bass fader edit rode the undo, not clobbered'); + assert.deepStrictEqual(S.partMix['arr:1'], { audible: false, vol: 0.5 }, 'drums strip back'); +}); + +t('delete does not corrupt mix keys when the drums arrangement was never materialized', () => { + const pitched = () => ({ tuning: [0, 0, 0, 0, 0, 0], capo: 0, notes: [], chords: [], chord_templates: [] }); + Object.assign(S, { + sessionId: 'sess-1', createMode: false, format: 'sloppak', sloppakForm: 'zip', + filename: 'song.feedpak', title: 'T', artist: 'A', + arrangements: [{ name: 'Lead', ...pitched() }], // NO drums arrangement materialized + currentArr: 0, beats: [], sections: [], + drumTab: { name: 'Drums', version: 1, hits: [{ t: 0.5, lane: 'kick' }] }, // but drumTab set + drumTabDirty: false, + partMix: { 'arr:0': { audible: true, vol: 1 } }, + stemLinks: {}, trackSession: null, trackHeights: {}, stems: [], + sessionDirty: false, history: new EditHistory(), sel: new Set(), + }); + // drumArrangementIndex is -1 here; a -1 drop would rewrite arr:0 β†’ arr:-1. + S.history.exec(new DeleteDrumTabCmd('Drums')); + assert.deepStrictEqual(S.partMix['arr:0'], { audible: true, vol: 1 }, 'Lead strip untouched'); + assert.strictEqual('arr:-1' in S.partMix, false, 'no corrupt arr:-1 key'); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/tests/midi_playback.test.mjs b/tests/midi_playback.test.mjs index d0e0bab4..5c7d56a0 100644 --- a/tests/midi_playback.test.mjs +++ b/tests/midi_playback.test.mjs @@ -4,8 +4,8 @@ * real mixer over them (per-part gain nodes, whole-map solo rule). * * Pinned here: - * - the band roster uses EXACTLY the mixer panel's strip keys - * ('arr:' / 'drums'), so strips and engine can never disagree; + * - the band roster uses EXACTLY the mixer panel's strip keys (all 'arr:' + * now, the drums arrangement included), so strips and engine can't disagree; * - the per-key strip state applies the DAW rule ACROSS parts (a solo on * one track silences the others' gains, mute always wins); * - the dedupe key is part-scoped (two parts on the same millisecond @@ -32,6 +32,7 @@ const audio = await import('../src/audio.js'); const { editorPlayAllTracksEnabled, editorTogglePlayAllTracks } = audio; const { _mixerPartStripState } = await import('../src/mixer-panel.js'); const { host, setHostHooks } = await import('../src/host.js'); +const { drumArrangementIndex } = await import('../src/drum-arrangement.js'); const { S } = await import('../src/state.js'); let pass = 0, fail = 0; @@ -43,19 +44,26 @@ function t(name, fn) { // The @pure:midi-playback block, sliced (the pures are module-local). import { readFileSync } from 'node:fs'; const src = readFileSync(new URL('../src/audio.js', import.meta.url), 'utf8').replace(/\r\n/g, '\n'); -function slice(name) { +// deps are injected as named args so a sliced function can call a real import +// (`_bandPartsPure` now resolves the drums arrangement's index). +function slice(name, deps = {}) { const at = src.indexOf(`function ${name}`); const body = src.slice(at, src.indexOf('\n}', at) + 2); - return new Function(`return (${body.replace(`function ${name}`, 'function')})`)(); + const keys = Object.keys(deps); + return new Function(...keys, `return (${body.replace(`function ${name}`, 'function')})`)(...keys.map(k => deps[k])); } -const _bandPartsPure = slice('_bandPartsPure'); +const _bandPartsPure = slice('_bandPartsPure', { drumArrangementIndex }); const _bandFiredKeyPure = slice('_bandFiredKeyPure'); -t('the band roster mirrors the mixer strip keys, drums last', () => { +t('the band roster mirrors the mixer strip keys, drums on its own arr:', () => { + // Drums are a type:"drums" arrangement now β€” the roster keys them by that + // arrangement's index (arr:2 here), the SAME key the mixer strip uses. const parts = _bandPartsPure( - [{ name: 'Lead' }, { name: 'Bass' }], + [{ name: 'Lead' }, { name: 'Bass' }, { name: 'Drums', type: 'drums' }], { hits: [{ t: 1 }] }); - assert.deepStrictEqual(parts.map(p => p.key), ['arr:0', 'arr:1', 'drums']); + assert.deepStrictEqual(parts.map(p => p.key), ['arr:0', 'arr:1', 'arr:2'], + 'the drum part rides arr:2, not a "drums" singleton'); + assert.strictEqual(parts[2].idx, 2, 'and resolves to the drums arrangement index'); assert.deepStrictEqual(_bandPartsPure([{ name: 'Lead' }], null).map(p => p.key), ['arr:0'], 'no drum tab = no drums strip'); assert.deepStrictEqual(_bandPartsPure([{ name: 'Lead' }], { hits: [] }).map(p => p.key), ['arr:0'], diff --git a/tests/mixer_panel.test.mjs b/tests/mixer_panel.test.mjs index 80d0a531..5cd17e5b 100644 --- a/tests/mixer_panel.test.mjs +++ b/tests/mixer_panel.test.mjs @@ -66,7 +66,7 @@ globalThis.window = globalThis.window || globalThis; const { _mixerPartsPure, _mixerPartStatePure, _mixerAnySoloPure, _mixerPartAudiblePure, - _mixerClapStatePure, _mixerOpenFromStoredPure, _mixerClapState, + _mixerClapStatePure, _mixerActivePartKeyPure, _mixerOpenFromStoredPure, _mixerClapState, _mixerGainForFaderPure, _mixerFaderLabelPure, _mixerOrderedPartsPure, _mixerPanelRefresh, editorToggleMixerPanel, initMixerPanel, } = await import('../src/mixer-panel.js'); @@ -81,34 +81,23 @@ function t(name, fn) { // ── Pure strip model ───────────────────────────────────────────────── -t('one strip per arrangement, keyed by index, drums appended only with hits', () => { +t('one strip per arrangement, keyed by index; the drums arrangement is an ordinary strip', () => { const arrs = [{ name: 'Lead' }, { name: '' }, null]; assert.deepStrictEqual(_mixerPartsPure(arrs, null), [ { key: 'arr:0', name: 'Lead' }, { key: 'arr:1', name: 'Track 2' }, { key: 'arr:2', name: 'Track 3' }, ]); - assert.deepStrictEqual(_mixerPartsPure([], { hits: [] }), []); - assert.deepStrictEqual(_mixerPartsPure([], { hits: [{ t: 1 }] }), - [{ key: 'drums', name: 'Drums' }]); + // A bare drum tab no longer conjures a 'drums' strip β€” drums are a + // type:"drums" arrangement now (PR2b), so they ride the arrangement pass and + // get an `arr:` strip named for themselves. + assert.deepStrictEqual(_mixerPartsPure([], { hits: [{ t: 1 }] }), []); + assert.deepStrictEqual(_mixerPartsPure( + [{ name: 'Lead' }, { name: 'Kit', type: 'drums' }], { hits: [{ t: 1 }] }), + [{ key: 'arr:0', name: 'Lead' }, { key: 'arr:1', name: 'Kit' }]); assert.deepStrictEqual(_mixerPartsPure(null, null), []); }); -// #336 regression: the drums arrangement (materialized into S.arrangements[]) -// must NOT get an 'arr:' strip β€” it already gets the dedicated 'drums' -// strip from drumTab. Pre-fix it got both β†’ two Drums strips after reload. -t('the drums arrangement is not double-listed β€” exactly one Drums strip', () => { - const arrs = [{ name: 'Lead' }, { name: 'Bass' }, { name: 'Drums', type: 'drums' }]; - const drumTab = { hits: [{ t: 0, p: 'kick' }] }; - const parts = _mixerPartsPure(arrs, drumTab); - assert.deepStrictEqual(parts, [ - { key: 'arr:0', name: 'Lead' }, - { key: 'arr:1', name: 'Bass' }, - { key: 'drums', name: 'Drums' }, - ], 'the type:"drums" arrangement gets no arr:2 strip; only the drums strip'); - assert.strictEqual(parts.filter(p => p.name === 'Drums').length, 1, 'exactly one Drums strip'); - assert.ok(!parts.some(p => p.key === 'arr:2'), 'no arr: strip for the drums arrangement'); -}); t('strip state defaults to audible unity; volume clamps into [0, 110] (+10 dB ceiling)', () => { assert.deepStrictEqual(_mixerPartStatePure({}, 'arr:0'), { vol: 100, mute: false, solo: false }); @@ -177,11 +166,19 @@ t('master is the OUTPUT bus: others solo/mute never silence it, its own mute doe // ── The clap state the guide scheduler consumes ────────────────────── -t('clap state follows the active surface: drums in drum mode, else the current arrangement', () => { - const mix = { 'arr:1': { mute: true }, drums: { vol: 50 } }; - assert.deepStrictEqual(_mixerClapStatePure(mix, false, 1), { audible: false, vol: 100 / 100 }); - assert.deepStrictEqual(_mixerClapStatePure(mix, true, 1), { audible: true, vol: 0.5 }); - assert.deepStrictEqual(_mixerClapStatePure(mix, false, 0), { audible: true, vol: 1 }); +t('clap state follows the active surface: the drums arrangement in drum mode, else the current arrangement', () => { + // Lead is arr:1; the drums arrangement is arr:2 (drumIdx = 2). currentArr + // stays on the pitched part even in drum mode (#337) β€” the clap key is what + // switches to the drums channel. + const mix = { 'arr:1': { mute: true }, 'arr:2': { vol: 50 } }; + assert.deepStrictEqual(_mixerClapStatePure(mix, false, 1, 2), { audible: false, vol: 100 / 100 }, + 'not drum mode β†’ the current (muted) arrangement'); + assert.deepStrictEqual(_mixerClapStatePure(mix, true, 1, 2), { audible: true, vol: 0.5 }, + 'drum mode β†’ the drums arrangement channel (arr:2)'); + assert.deepStrictEqual(_mixerClapStatePure(mix, false, 0, 2), { audible: true, vol: 1 }); + // No drums arrangement materialized (drumIdx = -1) β†’ fall back to currentArr. + assert.deepStrictEqual(_mixerClapStatePure(mix, true, 1, -1), { audible: false, vol: 1 }, + 'drum mode with no drums arrangement β†’ currentArr'); }); t('solo keeps the reference audible (D5): the gate is per-PART, and the host default leaves audio untouched', () => { @@ -197,11 +194,40 @@ t('solo keeps the reference audible (D5): the gate is per-PART, and the host def }); t('_mixerClapState reads live S (the wiring main.js hands to host.partClapState)', () => { - Object.assign(S, { partMix: { 'arr:0': { mute: true, vol: 40 } }, drumEditMode: false, currentArr: 0 }); + Object.assign(S, { arrangements: [{ name: 'Lead' }], partMix: { 'arr:0': { mute: true, vol: 40 } }, drumEditMode: false, currentArr: 0 }); assert.deepStrictEqual(_mixerClapState(), { audible: false, vol: 0.4 }); S.partMix = {}; }); +t('_mixerActivePartKeyPure: drum mode addresses the drums arrangement index; currentArr stays pitched (#337)', () => { + // In drum mode the active clap channel is the drums arrangement (arr:2), + // NOT currentArr β€” which is deliberately left on a pitched part. + assert.strictEqual(_mixerActivePartKeyPure(true, 0, 2), 'arr:2'); + assert.strictEqual(_mixerActivePartKeyPure(true, 1, 2), 'arr:2', 'ignores currentArr in drum mode'); + // Out of drum mode β†’ the current pitched arrangement. + assert.strictEqual(_mixerActivePartKeyPure(false, 1, 2), 'arr:1'); + // No drums arrangement materialized (drumIdx < 0) β†’ fall back to currentArr, + // even with the flag set, so the key is always a real strip. + assert.strictEqual(_mixerActivePartKeyPure(true, 3, -1), 'arr:3'); + assert.strictEqual(_mixerActivePartKeyPure(false, 'junk', 2), 'arr:0', 'a bad currentArr degrades to arr:0'); +}); + +t('the drums mixer strip gates the drum guide clap in drum-edit mode (arr:, live S)', () => { + // The runtime integration: with the drums arrangement materialized at arr:1, + // muting its mixer strip must silence the drum-grid guide claps β€” even though + // currentArr is a pitched part. This is the whole point of PR2b's rekey. + Object.assign(S, { + arrangements: [{ name: 'Lead' }, { name: 'Drums', type: 'drums' }], + drumEditMode: true, currentArr: 0, + partMix: { 'arr:1': { mute: true, vol: 50 } }, + }); + assert.strictEqual(_mixerClapState().audible, false, 'the muted drums strip gates the drum guide clap'); + assert.strictEqual(_mixerClapState().vol, 0.5, 'and its fader scales the clap level'); + S.partMix = {}; + assert.strictEqual(_mixerClapState().audible, true, 'unmuted β†’ the drum guide clap sounds again'); + Object.assign(S, { drumEditMode: false }); +}); + // ── Panel open state: pref round-trip + toggle ─────────────────────── t('open-state pref round-trip', () => { @@ -253,14 +279,14 @@ t('bus faders seed from host.mixUiState on open (incl. master, dB labels)', () = t('strips render one row per part; the refresh is memoized until state changes', () => { Object.assign(S, { - arrangements: [{ name: 'Lead ' }, { name: 'Bass' }], + arrangements: [{ name: 'Lead ' }, { name: 'Bass' }, { name: 'Drums', type: 'drums' }], drumTab: { hits: [{ t: 0.5 }] }, partMix: {}, currentArr: 0, }); editorToggleMixerPanel(true); const html = els['editor-mixer-parts'].innerHTML; assert.ok(html.includes('Lead <Guitar>'), 'part name rendered (escaped)'); assert.ok(html.includes('data-mix-part="arr:1"'), 'second strip'); - assert.ok(html.includes('data-mix-part="drums"'), 'drums strip'); + assert.ok(html.includes('data-mix-part="arr:2"'), 'the drums arrangement renders as an arr: strip'); assert.ok(html.includes('data-mix-act="solo"'), 'solo button'); assert.ok(html.includes('aria-valuetext="+0.0 dB"'), 'fader exposes its dB value to screen readers'); // Memo: same state β†’ no re-render (a sentinel survives the call). diff --git a/tests/stem_engine.test.mjs b/tests/stem_engine.test.mjs index 59f88be6..1b7cd0ab 100644 --- a/tests/stem_engine.test.mjs +++ b/tests/stem_engine.test.mjs @@ -25,12 +25,13 @@ const t = (name, fn) => tests.push([name, fn]); t('the mixer lists stem strips (audio band first), honoring removals', () => { const parts = _mixerPartsPure( - [{ name: 'Lead' }], { hits: [{}] }, + [{ name: 'Lead' }, { name: 'Drums', type: 'drums' }], { hits: [{}] }, [{ id: 'Guitar_L', name: 'Gtr L' }, { id: 'Bass_DI', name: 'Bass' }, { id: 'gone' }], ['gone']); assert.deepStrictEqual(parts.map(p => p.key), - ['audio:Guitar_L', 'audio:Bass_DI', 'arr:0', 'drums'], - 'stems first (removed dropped), then parts, then drums'); + ['audio:Guitar_L', 'audio:Bass_DI', 'arr:0', 'arr:1'], + 'stems first (removed dropped), then parts β€” the drums arrangement is an ordinary arr: strip now'); + assert.strictEqual(parts[3].name, 'Drums', 'the drums strip follows its arrangement name'); assert.strictEqual(parts[0].name, 'Gtr L'); assert.strictEqual(parts[0].kind, 'audio'); });