diff --git a/src/core/music/embedded.ts b/src/core/music/embedded.ts index 1fd3b214..2a467aa4 100644 --- a/src/core/music/embedded.ts +++ b/src/core/music/embedded.ts @@ -1,9 +1,9 @@ /** * What a song keeps to itself (#355): a sound, a pattern or a sound FX made in a song is an asset like * any other, so everything that finds an asset by its id finds it, but its `owner` names the song. - * The lists of assets leave it out, and it has exactly one user in its song: the song itself, a - * track or an item. `settleEmbedded` holds a song to that after every edit, so a split, a duplicate - * or a paste gets a copy of its own and what nothing uses any more goes. + * The lists of assets leave it out. The places of its song may share it (#357): the song itself, its + * tracks and its items. `settleEmbedded` holds a song to that after every edit, so a paste from another + * song gets a copy of its own and what nothing uses any more goes. */ import type { Graph } from '../graph/Graph'; import { SOUND_FX_INPUT, SOUND_FX_OUTPUT } from '../graph/nested'; @@ -185,10 +185,11 @@ export interface Settled { } /** - * Holds the song to one user per embedded asset, writing the places in `tracks` and `items` (fresh - * lists the caller owns): a place naming an asset of the song that an earlier place claimed, or one of - * another song, gets a copy; one naming an asset that is gone but in `spare` – what was copied along - * with items – gets that back. The song's own props are only read: they claim, never copy. + * Holds the song to what it keeps to itself, writing the places in `tracks` and `items` (fresh lists + * the caller owns). The places of the song share its assets (#357), so the clips of one take serve + * many items. A place naming an asset of another song gets a copy, one for all places naming it; one + * naming an asset that is gone but in `spare` – what was copied along with items – gets that back. + * The song's own props are only read: they claim, never copy. */ export function settleEmbedded( project: Project, @@ -199,6 +200,7 @@ export function settleEmbedded( ): Settled { const claimed = new Set(); const added: Asset[] = []; + const copies = new Map>(); const taken = new Set(project.assets.map((a) => a.id)); const own = { ...song.props }; slotsOf(own, tracks, items).forEach((slot, index) => { @@ -207,20 +209,24 @@ export function settleEmbedded( const found = assetNamedBy(project.assets, slot.id) ?? assetNamedBy(added, slot.id); const source = found ?? assetNamedBy(spare, slot.id); if (!source || !slot.types.includes(source.type) || (found && !isEmbedded(found))) return; - if (found?.owner === song.id && !claimed.has(found.id)) { + if (found?.owner === song.id) { claimed.add(found.id); return; } if (index === 0) return; - const fresh = found !== undefined || taken.has(source.id); - const id = fresh ? newId() : source.id; - const copy = ownedBy({ ...structuredClone(source), id }, song); - const ids = new Map([[source.id, id]]); - if (fresh) renewClipIds(copy, ids); - taken.add(id); - added.push(copy); - claimed.add(id); - slot.set(ids.get(slot.id) ?? id); + let ids = copies.get(source.id); + if (!ids) { + const fresh = found !== undefined || taken.has(source.id); + const id = fresh ? newId() : source.id; + const copy = ownedBy({ ...structuredClone(source), id }, song); + ids = new Map([[source.id, id]]); + if (fresh) renewClipIds(copy, ids); + copies.set(source.id, ids); + taken.add(id); + added.push(copy); + claimed.add(id); + } + slot.set(ids.get(slot.id) ?? ids.get(source.id) ?? slot.id); }); const dropped = project.assets.filter((a) => a.owner === song.id && !claimed.has(a.id)).map((a) => a.id); return { added, dropped }; diff --git a/src/editor/inspector/songSection.ts b/src/editor/inspector/songSection.ts index a7f4cf64..9348cb55 100644 --- a/src/editor/inspector/songSection.ts +++ b/src/editor/inspector/songSection.ts @@ -2,8 +2,7 @@ import { SetSongCommand } from '../../core/commands/songCommands'; import type { SongAsset } from '../../core/model/Asset'; import { ASSET_TYPE_LABELS } from '../../core/model/AssetTypes'; import { findAssetOf, type Project } from '../../core/model/Project'; -import { findSound } from '../../core/model/sound'; -import { isEmbedded } from '../../core/music/embedded'; +import { embeddedIn, markedName } from '../../core/music/embedded'; import { assetRefLabel, soundEntries } from '../../core/properties/assetRef'; import type { PropertyDef } from '../../core/properties/PropertySchema'; import { @@ -204,8 +203,9 @@ function buildItem(host: InspectorHost, asset: SongAsset, item: SongItem, picked }); if (def.key === FX_ROW.key) handlers.moreChoices = (current) => ownFxEntries(state, asset.id, current, commit); - if (def.key === 'asset' && item.kind === 'sound') - handlers.moreChoices = (current) => ownSoundEntries(state, current, (id) => state.execute(commit(id))); + if (def.key === 'asset') + handlers.moreChoices = (current) => + keptEntries(state, asset.id, item.kind, current, (id) => state.execute(commit(id))); const row = createFieldRow(host, def, item[def.key as keyof SongItem], handlers); host.fields.set(def.key, row); host.body.add(row.row); @@ -220,15 +220,24 @@ function buildItem(host: InspectorHost, asset: SongAsset, item: SongItem, picked } /** - * The sound the song keeps for the item (#355) and its clips (#357), which no list offers: a take's - * clips are picked here. + * The sounds with their clips (#357) or the patterns the song keeps (#355), which no list offers: + * its items share them, so a take's clips are picked here. */ -function ownSoundEntries(state: EditorState, current: string, pick: (id: string) => void): MenuEntry[] { - const sound = findSound(state.project, current)?.asset; - if (!sound || !isEmbedded(sound)) return []; +function keptEntries( + state: EditorState, + songId: string, + kind: SongItem['kind'], + current: string, + pick: (id: string) => void, +): MenuEntry[] { + const entries = embeddedIn(state.project, songId) + .filter((a) => a.type === kind) + .sort((a, b) => a.name.localeCompare(b.name)) + .flatMap((a) => (a.type === 'sound' ? soundEntries(a) : [{ id: a.id, label: markedName(a) }])); + if (entries.length === 0) return []; return [ { separator: true }, - ...soundEntries(sound).map((entry) => ({ + ...entries.map((entry) => ({ label: entry.label, current: entry.id === current, onSelect: () => pick(entry.id), diff --git a/src/editor/song/ownFx.ts b/src/editor/song/ownFx.ts index a327bcf4..b268f6d2 100644 --- a/src/editor/song/ownFx.ts +++ b/src/editor/song/ownFx.ts @@ -1,14 +1,14 @@ /** * A sound FX of its own for a song, a track or an item (#355): the entries every pick of a sound FX - * in a song ends with. The FX made here is kept by the song and named by that one place, so the - * place letting go of it takes it away (`settleEmbedded`). + * in a song ends with. The FX made here is kept by the song and shared by its places (#357), so the + * last place letting go of it takes it away (`settleEmbedded`). */ import { CreateAssetCommand, MakeProjectAssetCommand } from '../../core/commands/assetCommands'; import type { Command } from '../../core/commands/Command'; import { CompoundCommand } from '../../core/commands/CompoundCommand'; import type { SoundFxAsset } from '../../core/model/Asset'; import { findAssetOf } from '../../core/model/Project'; -import { EMBEDDED_MARK, embeddedName, passThroughFxGraph } from '../../core/music/embedded'; +import { EMBEDDED_MARK, embeddedIn, embeddedName, passThroughFxGraph } from '../../core/music/embedded'; import type { MenuEntry } from '../../ui'; import type { EditorState } from '../state'; @@ -16,8 +16,8 @@ import type { EditorState } from '../state'; export type FxPlace = (fxId: string) => Command; /** - * Its own FX with Edit and Make project asset while the place names one; else Create sound FX, and - * Embed a copy while it names one of the project's. + * The FX the song keeps, which its places share (#357); then Edit and Make project asset while the + * place names one of them, else Create sound FX, and Embed a copy while it names one of the project's. */ export function ownFxEntries( state: EditorState, @@ -28,15 +28,24 @@ export function ownFxEntries( const project = state.project; if (!findAssetOf(project, songId, 'song')) return []; const picked = findAssetOf(project, current, 'soundfx'); + const kept = embeddedIn(project, songId) + .filter((a) => a.type === 'soundfx') + .sort((a, b) => a.name.localeCompare(b.name)) + .map((fx) => ({ + label: `${EMBEDDED_MARK} ${fx.name}`, + current: fx.id === current, + onSelect: () => (fx.id === current ? state.openAsset(fx.id) : state.execute(place(fx.id))), + })); if (picked?.owner === songId) return [ { separator: true }, - { label: `${EMBEDDED_MARK} ${picked.name}`, current: true, onSelect: () => state.openAsset(picked.id) }, + ...kept, { label: 'Edit sound FX', onSelect: () => state.openAsset(picked.id) }, { label: 'Make project asset', onSelect: () => state.execute(new MakeProjectAssetCommand(picked.id)) }, ]; return [ { separator: true }, + ...kept, { label: 'Create sound FX', onSelect: () => createOwnFx(state, songId, place) }, ...(picked ? [{ label: 'Embed a copy', onSelect: () => createOwnFx(state, songId, place, picked) }] : []), ]; diff --git a/tests/core/embedded.test.ts b/tests/core/embedded.test.ts index 7f9bf8f2..9fb0bc6b 100644 --- a/tests/core/embedded.test.ts +++ b/tests/core/embedded.test.ts @@ -1,7 +1,7 @@ /** * What a song keeps to itself (#355): an asset with the song as its owner, left out of every list, - * with one user in the song. Split, duplicate and paste give a place a copy of its own; what nothing - * names goes in the same step, and comes back with an undo. + * shared by the places of the song (#357). A paste from another song gives it a copy of its own; + * what nothing names goes in the same step, and comes back with an undo. */ import { describe, expect, it } from 'vitest'; import { @@ -81,24 +81,50 @@ describe('an embedded asset (#355)', () => { expect(isRowHiddenWhenEmbedded(findAsset(f.project, blipId)!, 'loop')).toBe(false); }); - it('gives the right half of a split a copy of its own, the same one again on redo', () => { + it('is shared by both halves of a split (#357), and stays until the last place lets go', () => { const { f, song, takeId, edit } = setup(); + const before = f.project.assets.length; const left = song().items[0]!; edit([ { ...left, length: BAR }, { ...left, id: 'right', tick: BAR, length: BAR }, ]); - const [a, b] = song().items; - expect(a!.asset).toBe(takeId); - expect(b!.asset).not.toBe(takeId); - const copy = findAsset(f.project, b!.asset)!; - expect([copy.owner, copy.name, copy.type]).toEqual([song().id, 'Take 1', 'sound']); + expect(song().items.map((i) => i.asset)).toEqual([takeId, takeId]); + expect(f.project.assets.length).toBe(before); + edit([song().items[1]!]); + expect(findAsset(f.project, takeId)?.owner).toBe(song().id); + edit([]); + expect(findAsset(f.project, takeId)).toBeUndefined(); f.stack.undo(); - expect(findAsset(f.project, copy.id)).toBeUndefined(); expect(embeddedIn(f.project, song().id).map((x) => x.id)).toEqual([takeId]); - f.stack.redo(); - expect(song().items[1]!.asset).toBe(copy.id); - expect(findAsset(f.project, copy.id)).toBe(copy); + }); + + it('is shared by the song, its tracks and its items, whichever kind (#357)', () => { + const { f, song, songId, track, edit } = setup(); + const fx = new CreateAssetCommand('soundfx', 'Sound FX 1', {}, songId); + const pattern = new CreateAssetCommand('pattern', 'Pattern 1', {}, songId); + f.stack.execute(fx); + f.stack.execute(pattern); + const tracks = song().tracks.map((t) => ({ ...t, fx: fx.asset.id })); + const played = { track, kind: 'pattern' as const, asset: pattern.asset.id, length: BAR }; + const items = [0, BAR].map((tick) => ({ ...newItem({ ...played, tick }), fx: fx.asset.id })); + f.stack.execute(new SetSongCommand(songId, { tracks, items })); + f.stack.execute(new SetAssetPropertyCommand(songId, 'fx', fx.asset.id)); + expect(song().items.map((i) => [i.asset, i.fx])).toEqual([ + [pattern.asset.id, fx.asset.id], + [pattern.asset.id, fx.asset.id], + ]); + expect(song().tracks[0]!.fx).toBe(fx.asset.id); + expect( + embeddedIn(f.project, songId) + .map((a) => a.type) + .sort(), + ).toEqual(['pattern', 'soundfx']); + f.stack.execute(new SetAssetPropertyCommand(songId, 'fx', '')); + edit(song().items.map((i) => ({ ...i, fx: '' }))); + expect(findAsset(f.project, fx.asset.id)?.owner).toBe(songId); + f.stack.execute(new SetSongCommand(songId, { tracks: song().tracks.map((t) => ({ ...t, fx: '' })) })); + expect(findAsset(f.project, fx.asset.id)).toBeUndefined(); }); it('goes with the item that names it, and comes back on undo where it was', () => { @@ -144,6 +170,22 @@ describe('an embedded asset (#355)', () => { expect(findAsset(f.project, takeId)?.owner).toBe(song().id); }); + it('is copied once for all the items of another song that name it (#357)', () => { + const { f, takeId } = setup(); + const other = new CreateAssetCommand('song', 'Other'); + f.stack.execute(other); + const otherSong = other.asset as SongAsset; + const track = otherSong.tracks[0]!.id; + const pasted = [0, BAR].map((tick) => + newItem({ track, kind: 'sound', asset: takeId, tick, length: BAR }), + ); + f.stack.execute(new SetSongCommand(otherSong.id, { items: pasted })); + const named = findAssetOf(f.project, otherSong.id, 'song')!.items.map((i) => i.asset); + expect(named[0]).not.toBe(takeId); + expect(named[1]).toBe(named[0]); + expect(embeddedIn(f.project, otherSong.id).map((a) => a.id)).toEqual([named[0]]); + }); + it('leaves a project asset named by several items alone', () => { const { f, song, blipId, edit, track } = setup(); const before = f.project.assets.length; diff --git a/tests/core/sound-clips.test.ts b/tests/core/sound-clips.test.ts index 133ab71b..0b44cb8d 100644 --- a/tests/core/sound-clips.test.ts +++ b/tests/core/sound-clips.test.ts @@ -172,20 +172,17 @@ describe('a clip in a song (#357)', () => { expect(itemFx(f.project, song().items[0]!)).toBe('some-fx'); }); - it('keeps its take, and a second item on a clip of it gets a copy with its own clip ids', () => { + it('keeps its take, which a second item shares, on the same clip or the whole take', () => { const { f, song, songId, track, takeId, takeClipId } = setup(); expect(embeddedUsedBy(f.project, song().items).map((a) => a.id)).toEqual([takeId]); + const before = f.project.assets.length; const [item] = song().items; - f.stack.execute(new SetSongCommand(songId, { items: [item!, { ...item!, id: 'second', tick: BAR }] })); - const named = song().items[1]!.asset; - const copy = findSound(f.project, named)!; - expect(copy.asset.id).not.toBe(takeId); - expect(copy.asset.owner).toBe(songId); - expect(copy.clip?.name).toBe('Clip 1'); - expect(named).not.toBe(takeClipId); - expect(findSound(f.project, takeClipId)?.asset.id).toBe(takeId); - // The take nothing else names stays. - expect(song().items[0]!.asset).toBe(takeClipId); + const second = { ...item!, id: 'second', tick: BAR }; + f.stack.execute(new SetSongCommand(songId, { items: [item!, second] })); + expect(song().items.map((i) => i.asset)).toEqual([takeClipId, takeClipId]); + f.stack.execute(new SetSongCommand(songId, { items: [item!, { ...second, asset: takeId }] })); + expect(song().items.map((i) => i.asset)).toEqual([takeClipId, takeId]); + expect(f.project.assets.length).toBe(before); expect(song().items.map((i) => i.track)).toEqual([track, track]); }); diff --git a/tests/editor/own-fx.test.ts b/tests/editor/own-fx.test.ts index 798ac1af..876efb85 100644 --- a/tests/editor/own-fx.test.ts +++ b/tests/editor/own-fx.test.ts @@ -77,6 +77,18 @@ describe('a sound FX of its own in a song (#355)', () => { expect(embeddedIn(state.project, songId).map((a) => a.name)).toEqual(['Sound FX 2']); }); + it('is offered to every place of its song, which share it (#357)', () => { + const { state, songId, theme, track, onTrack, onItem } = setup(); + const id = createOwnFx(state, songId, onTrack)!; + const entries = ownFxEntries(state, songId, '', onItem); + expect(labels(entries)).toEqual(['• Sound FX 1', 'Create sound FX']); + const own = entries.find((e) => 'label' in e && e.label === '• Sound FX 1'); + if (own && 'onSelect' in own) own.onSelect(); + expect([track().fx, theme().items[0]!.fx]).toEqual([id, id]); + state.execute(onTrack('')); + expect(findAsset(state.project, id)?.owner).toBe(songId); + }); + it('goes when its track picks another one, and comes back on undo', () => { const { state, songId, track, echoId, onTrack } = setup(); const id = createOwnFx(state, songId, onTrack)!;