From 0d57a2f3c434b9ef2ba856b7c194067811185412 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Fri, 9 Oct 2026 07:33:49 +0200 Subject: [PATCH] Pick a song's sign in one row, and let its patterns keep theirs (#363) A song had two rows for its meter, Beats per bar and Beat, and the patterns it kept to itself took both from it on every change. Their length is counted in bars, so a new sign cut or stretched them and their items. - The song has one row Sign with 1/2, 2/2, 3/2, 1/4 to 6/4 and 5/8 to 9/8, 4/4 by default. It is stored as `sign`. A value not in the list reads as 4/4. - An embedded pattern keeps its own tempo and meter, as a project pattern does. Its inspector shows Tempo, Beats per bar and Beat again and hides only Bus. - A pattern made in a song still starts with the song's tempo and sign, so its first bar fits the item it gets. - A copy (Embed a copy, a paste from another song) keeps its clock. `clockFollowers` and the follow step in `AssetPropertyCommand` are gone. - No load migration. The two songs of the user's project were moved from `beats`/`beat` to `sign` by hand. No other project had a song with a meter of its own. Tests: - Unit song + embedded + pattern: 44 green, 1 new, 4 rewritten. Counter-probes: a copy taking the song's clock and a sign outside the list both turn a test red. - e2e 65 (melektaus-atx): ALL PASSED, 57 checks (1 new: Sign picked from its list in the inspector). - Gate (melektaus-atx): CHECK EXIT 0, 272 files, 4214 tests. After it, `SIGNS` lost its `as const` and the cast went with it. Typecheck, lint and the 44 unit tests ran again, all green. Co-Authored-By: Claude Opus 5.5 --- src/core/commands/assetCommands.ts | 17 ++------- src/core/music/embedded.ts | 31 +++------------- src/core/music/song.ts | 32 ++++++++++++---- src/core/properties/PropertySchema.ts | 17 ++------- tests/core/embedded.test.ts | 53 +++++++++++---------------- tests/core/song.test.ts | 35 +++++++++++++++++- tools/e2e/65-song.mjs | 16 ++++++++ 7 files changed, 109 insertions(+), 92 deletions(-) diff --git a/src/core/commands/assetCommands.ts b/src/core/commands/assetCommands.ts index 39701130..58814888 100644 --- a/src/core/commands/assetCommands.ts +++ b/src/core/commands/assetCommands.ts @@ -12,7 +12,7 @@ import type { VariablesAsset, } from '../model/Asset'; import type { AssetType } from '../model/AssetTypes'; -import { clockFollowers, embeddedIn } from '../music/embedded'; +import { embeddedIn } from '../music/embedded'; import { defaultTracks } from '../music/song'; import { heldSongGraph } from '../music/routing'; import { linearCurve } from '../model/curve'; @@ -21,7 +21,7 @@ import { DEFAULT_PALETTE, PIXEL_CANVAS_DEFAULT } from '../model/pixelart'; import { isSoundFile, renewClipIds } from '../model/sound'; import { removeSubtrees } from '../model/hierarchy'; import { dropReferencesToGone, renamedReference, shownObjectIds, swapReferences } from '../model/objectRefs'; -import { type PropertyBag, type PropertyValue, setBagValue } from '../model/Properties'; +import type { PropertyBag, PropertyValue } from '../model/Properties'; import { objectHolders, type Project } from '../model/Project'; import { type Scope, @@ -547,12 +547,8 @@ export class SetMachineStartCommand implements Command { } } -/** - * An embedded pattern follows the tempo and the meter of its song (#355), in the same undo step. A - * song that picks another sound FX lets go of the one it kept to itself, in that step too. - */ +/** A song that picks another sound FX lets go of the one it kept to itself, in the same undo step. */ abstract class AssetPropertyCommand extends SetBagValueCommand { - private followed: { id: string; previous: PropertyValue | undefined }[] = []; private settled: SetSongCommand | null = null; protected constructor( @@ -566,11 +562,6 @@ abstract class AssetPropertyCommand extends SetBagValueCommand { override execute(project: Project): void { super.execute(project); const asset = requireAsset(project, this.assetId); - this.followed = clockFollowers(project, asset, this.key).map((pattern) => { - const previous = pattern.props[this.key]; - setBagValue(pattern.props, this.key, asset.props[this.key]); - return { id: pattern.id, previous }; - }); this.settled = asset.type === 'song' && this.key === ASSET_FX_KEY ? new SetSongCommand(asset.id, {}) : null; this.settled?.execute(project); @@ -578,8 +569,6 @@ abstract class AssetPropertyCommand extends SetBagValueCommand { override undo(project: Project): void { this.settled?.undo(project); - for (const { id, previous } of this.followed) - setBagValue(requireAsset(project, id).props, this.key, previous); super.undo(project); } diff --git a/src/core/music/embedded.ts b/src/core/music/embedded.ts index 2a467aa4..7efc18b6 100644 --- a/src/core/music/embedded.ts +++ b/src/core/music/embedded.ts @@ -8,7 +8,7 @@ import type { Graph } from '../graph/Graph'; import { SOUND_FX_INPUT, SOUND_FX_OUTPUT } from '../graph/nested'; import { newId } from '../ids'; -import type { Asset, PatternAsset, SongAsset } from '../model/Asset'; +import type { Asset, SongAsset } from '../model/Asset'; import type { AssetType } from '../model/AssetTypes'; import type { PropertyBag } from '../model/Properties'; import { findAsset, findAssetOf, type Project } from '../model/Project'; @@ -20,19 +20,13 @@ import { type SongItem, songMeter, songTempo, type SongTrack } from './song'; export const EMBEDDABLE_TYPES: readonly AssetType[] = ['sound', 'pattern', 'soundfx']; -/** - * What an embedded pattern takes from its song: it is drawn and heard on the song's clock. A pattern - * names these the way a song does, so one key serves both. - */ -export const SONG_CLOCK_KEYS: readonly string[] = [PATTERN_TEMPO_KEY, PATTERN_BEATS_KEY, PATTERN_BEAT_KEY]; - /** * The rows of an embedded asset that do nothing in a song: where it goes and how it repeats is the - * song's, and a pattern's clock is the song's too. + * song's. A pattern keeps its tempo and meter (#363), as one of the project does. */ const HIDDEN_ROWS: Partial> = { sound: ['loop', 'bus'], - pattern: [...SONG_CLOCK_KEYS, 'bus'], + pattern: ['bus'], }; export function isEmbeddable(type: AssetType): boolean { @@ -75,7 +69,7 @@ export function assetTitle(project: Project, asset: Asset): string { return song ? `${song.name} › ${asset.name}` : asset.name; } -/** The tempo and the meter of a song, as a pattern's props. */ +/** The tempo and the meter of a song, as the props a pattern made in it starts with. */ export function songClockProps(props: PropertyBag): PropertyBag { const meter = songMeter(props); return { @@ -85,30 +79,17 @@ export function songClockProps(props: PropertyBag): PropertyBag { }; } -/** Gives an asset that comes to live in `song` its owner, and a pattern the song's clock. */ -function ownedBy(asset: T, song: SongAsset): T { - asset.owner = song.id; - if (asset.type === 'pattern') Object.assign(asset.props, songClockProps(song.props)); - return asset; -} - /** * A copy of a project asset for the song to keep to itself (#355), to bend without touching the rest. * `ids` says what the asset and its clips (#357) are called in the copy. */ export function embeddedCopy(asset: Asset, song: SongAsset): { copy: Asset; ids: Map } { - const copy = ownedBy({ ...structuredClone(asset), id: newId() }, song); + const copy: Asset = { ...structuredClone(asset), id: newId(), owner: song.id }; const ids = new Map([[asset.id, copy.id]]); renewClipIds(copy, ids); return { copy, ids }; } -/** The patterns that follow `key` of an asset: those a song keeps, when the key is part of its clock. */ -export function clockFollowers(project: Project, asset: Asset, key: string): PatternAsset[] { - if (asset.type !== 'song' || !SONG_CLOCK_KEYS.includes(key)) return []; - return embeddedIn(project, asset.id).filter((a): a is PatternAsset => a.type === 'pattern'); -} - /** "Pattern 1", 2, 3 … ("Sound FX 1" …): the first number none of what the song keeps to itself has. */ export function embeddedName(project: Project, songId: string, base: string): string { return nextNumberedName( @@ -218,7 +199,7 @@ export function settleEmbedded( if (!ids) { const fresh = found !== undefined || taken.has(source.id); const id = fresh ? newId() : source.id; - const copy = ownedBy({ ...structuredClone(source), id }, song); + const copy: Asset = { ...structuredClone(source), id, owner: song.id }; ids = new Map([[source.id, id]]); if (fresh) renewClipIds(copy, ids); copies.set(source.id, ids); diff --git a/src/core/music/song.ts b/src/core/music/song.ts index c2496bf7..43f18bf6 100644 --- a/src/core/music/song.ts +++ b/src/core/music/song.ts @@ -56,10 +56,29 @@ export interface SongItem { } export const SONG_TEMPO_KEY = 'tempo'; -export const SONG_BEATS_KEY = 'beats'; -export const SONG_BEAT_KEY = 'beat'; +export const SONG_SIGN_KEY = 'sign'; export const SONG_GRID_KEY = 'grid'; +/** The time signatures a song is written in (#363), one row for both numbers. */ +export const SIGNS: readonly string[] = [ + '1/2', + '2/2', + '3/2', + '1/4', + '2/4', + '3/4', + '4/4', + '5/4', + '6/4', + '5/8', + '6/8', + '7/8', + '8/8', + '9/8', +]; +export const DEFAULT_SIGN = '4/4'; +export const SIGN_OPTIONS = SIGNS.map((sign) => ({ value: sign, label: sign })); + export const DEFAULT_SONG_GRID = 'bar'; export const SONG_GRID_OPTIONS = [{ value: 'bar', label: 'Bar' }, ...GRID_OPTIONS]; /** A new song has this many tracks. */ @@ -78,12 +97,11 @@ export function itemMaxDb(kind: SongItemKind): number { return kind === 'sound' ? SOUND_ITEM_MAX_DB : 0; } +/** The meter of a song's sign; 4/4 where it is not one of `SIGNS`. */ export function songMeter(props: PropertyBag): Meter { - const beats = props[SONG_BEATS_KEY]; - return heldMeter({ - beats: typeof beats === 'number' ? Math.round(beats) : 4, - unit: Number(textOf(props[SONG_BEAT_KEY] ?? '4')), - }); + const sign = textOf(props[SONG_SIGN_KEY] ?? DEFAULT_SIGN); + const [beats, unit] = (SIGNS.includes(sign) ? sign : DEFAULT_SIGN).split('/'); + return heldMeter({ beats: Number(beats), unit: Number(unit) }); } export function songTempo(props: PropertyBag): number { diff --git a/src/core/properties/PropertySchema.ts b/src/core/properties/PropertySchema.ts index 475494eb..988f38a9 100644 --- a/src/core/properties/PropertySchema.ts +++ b/src/core/properties/PropertySchema.ts @@ -118,11 +118,12 @@ import { PATTERN_TEMPO_KEY, } from '../music/pattern'; import { + DEFAULT_SIGN, DEFAULT_SONG_GRID, - SONG_BEAT_KEY, - SONG_BEATS_KEY, + SIGN_OPTIONS, SONG_GRID_KEY, SONG_GRID_OPTIONS, + SONG_SIGN_KEY, SONG_TEMPO_KEY, } from '../music/song'; import { DEFAULT_TEMPO, MAX_TEMPO, MIN_TEMPO } from '../music/time'; @@ -770,17 +771,7 @@ const ASSET_SCHEMAS: Record = { max: MAX_TEMPO, step: 1, }, - { - key: SONG_BEATS_KEY, - label: 'Beats per bar', - kind: 'number', - default: 4, - min: 1, - max: 32, - step: 1, - decimals: 0, - }, - { key: SONG_BEAT_KEY, label: 'Beat', kind: 'enum', default: '4', options: BEAT_OPTIONS }, + { key: SONG_SIGN_KEY, label: 'Sign', kind: 'enum', default: DEFAULT_SIGN, options: SIGN_OPTIONS }, { key: SONG_GRID_KEY, label: 'Grid', diff --git a/tests/core/embedded.test.ts b/tests/core/embedded.test.ts index 9fb0bc6b..037e479a 100644 --- a/tests/core/embedded.test.ts +++ b/tests/core/embedded.test.ts @@ -10,7 +10,6 @@ import { DeleteAssetCommand, DuplicateAssetCommand, MakeProjectAssetCommand, - ResetAssetPropertyCommand, SetAssetPropertyCommand, } from '../../src/core/commands/assetCommands'; import { nextAssetName } from '../../src/core/commands/naming'; @@ -29,7 +28,7 @@ import { songClockProps, } from '../../src/core/music/embedded'; import { patternLength, patternMeter, patternTempo } from '../../src/core/music/pattern'; -import { newItem, type SongItem, songSequence } from '../../src/core/music/song'; +import { newItem, type SongItem, songMeter, songSequence } from '../../src/core/music/song'; import { assetRefChoices } from '../../src/core/properties/assetRef'; import { deserializeProject, serializeProject } from '../../src/core/store/serialize'; import { makeProject } from '../helpers'; @@ -266,7 +265,7 @@ describe('a pattern made in a song (#355)', () => { const s = setup(); const { f, songId } = s; f.stack.execute(new SetAssetPropertyCommand(songId, 'tempo', 140)); - f.stack.execute(new SetAssetPropertyCommand(songId, 'beats', 3)); + f.stack.execute(new SetAssetPropertyCommand(songId, 'sign', '3/4')); const loose = new CreateAssetCommand('pattern', 'Loose'); f.stack.execute(loose); const lead = new CreateAssetCommand('instrument', 'Lead'); @@ -296,40 +295,32 @@ describe('a pattern made in a song (#355)', () => { expect(make().name).toBe('Pattern 2'); }); - it('follows the tempo and the meter of its song in the same undo step, and nothing else', () => { - const { f, songId, loose, make } = withPatterns(); + it('keeps its own tempo, meter and length when the song changes its own (#363)', () => { + const { f, songId, make } = withPatterns(); const own = make(); f.stack.execute(new SetAssetPropertyCommand(songId, 'tempo', 90)); - expect([patternTempo(own.props), patternTempo(loose.props)]).toEqual([90, 120]); - f.stack.execute(new SetAssetPropertyCommand(songId, 'beat', '8')); - expect(patternMeter(own.props)).toEqual({ beats: 3, unit: 8 }); - f.stack.execute(new ResetAssetPropertyCommand(songId, 'beats')); - expect(patternMeter(own.props).beats).toBe(4); - const grid = own.props.grid; - f.stack.execute(new SetAssetPropertyCommand(songId, 'grid', '1/8')); - expect([own.props.grid, findAsset(f.project, songId)?.props.grid]).toEqual([grid, '1/8']); - f.stack.undo(); - f.stack.undo(); - expect(patternMeter(own.props)).toEqual({ beats: 3, unit: 8 }); - f.stack.undo(); - f.stack.undo(); - expect([patternTempo(own.props), patternMeter(own.props).unit]).toEqual([140, 4]); + f.stack.execute(new SetAssetPropertyCommand(songId, 'sign', '6/8')); + expect(songMeter(findAsset(f.project, songId)!.props)).toEqual({ beats: 6, unit: 8 }); + expect([patternTempo(own.props), patternMeter(own.props), patternLength(own)]).toEqual([ + 140, + { beats: 3, unit: 4 }, + 3 * 480, + ]); }); - it('hides its clock and its bus, not its length, instrument or sound FX', () => { - const { loose, make } = withPatterns(); + it('hides only its bus, not its clock, length, instrument or sound FX (#363)', () => { + const { make } = withPatterns(); const own = make(); - const keys = ['tempo', 'beats', 'beat', 'bus', 'bars', 'instrument', 'fx']; + const keys = ['bus', 'tempo', 'beats', 'beat', 'bars', 'instrument', 'fx']; expect(keys.map((k) => isRowHiddenWhenEmbedded(own, k))).toEqual([ true, - true, - true, - true, + false, + false, + false, false, false, false, ]); - expect(isRowHiddenWhenEmbedded(loose, 'tempo')).toBe(false); }); it('plays the instrument of the pattern on its track that starts last before it, else the first after', () => { @@ -351,13 +342,13 @@ describe('a pattern made in a song (#355)', () => { expect(near(0, 'elsewhere')).toBe(''); }); - it("takes the clock of the song it comes to as a copy, a project pattern's or another song's", () => { + it("keeps its own clock as a copy, a project pattern's or another song's (#363)", () => { const { f, song, loose, track, edit } = withPatterns(); const copy = embeddedCopy(loose, song()).copy as PatternAsset; - expect([patternTempo(copy.props), patternTempo(loose.props)]).toEqual([140, 120]); + expect([patternTempo(copy.props), patternMeter(copy.props).beats]).toEqual([120, 4]); const other = new CreateAssetCommand('song', 'Other'); f.stack.execute(other); - const theirs = new CreateAssetCommand('pattern', 'Theirs', { bars: 1 }, other.asset.id); + const theirs = new CreateAssetCommand('pattern', 'Theirs', { bars: 1, beats: 5 }, other.asset.id); f.stack.execute(theirs); edit([ ...song().items, @@ -367,8 +358,8 @@ describe('a pattern made in a song (#355)', () => { expect(named.id).not.toBe(theirs.asset.id); expect([named.owner, patternTempo(named.props), patternMeter(named.props).beats]).toEqual([ song().id, - 140, - 3, + 120, + 5, ]); }); }); diff --git a/tests/core/song.test.ts b/tests/core/song.test.ts index 4bc5feda..ad88bfa9 100644 --- a/tests/core/song.test.ts +++ b/tests/core/song.test.ts @@ -14,6 +14,7 @@ import { songGrid, songLength, songLoop, + songMeter, songSequence, trackItemsBus, } from '../../src/core/music/song'; @@ -72,8 +73,7 @@ describe('the song asset (#334)', () => { expect(f.song().items).toEqual([]); expect(schemaForAsset(f.project, f.song()).map((d) => d.key)).toEqual([ 'tempo', - 'beats', - 'beat', + 'sign', 'grid', 'fx', 'bus', @@ -83,6 +83,37 @@ describe('the song asset (#334)', () => { expect(songGrid(f.song().props)).toBe(160); }); + it('is written in one of the signs from 1/2 to 9/8, else in 4/4 (#363)', () => { + const f = makeSong(); + const sign = schemaForAsset(f.project, f.song()).find((d) => d.key === 'sign'); + expect([sign?.label, sign?.default]).toEqual(['Sign', '4/4']); + expect(sign?.kind === 'enum' && sign.options.map((o) => o.value)).toEqual([ + '1/2', + '2/2', + '3/2', + '1/4', + '2/4', + '3/4', + '4/4', + '5/4', + '6/4', + '5/8', + '6/8', + '7/8', + '8/8', + '9/8', + ]); + expect(songMeter(f.song().props)).toEqual({ beats: 4, unit: 4 }); + f.stack.execute(new SetAssetPropertyCommand(f.song().id, 'sign', '9/8')); + expect([songMeter(f.song().props), songLength(f.song())]).toEqual([{ beats: 9, unit: 8 }, 9 * 240]); + f.stack.execute(new SetAssetPropertyCommand(f.song().id, 'sign', '3/2')); + expect(songMeter(f.song().props)).toEqual({ beats: 3, unit: 2 }); + expect([songMeter({ sign: '7/4' }), songMeter({ sign: 'x' })]).toEqual([ + { beats: 4, unit: 4 }, + { beats: 4, unit: 4 }, + ]); + }); + it('is at least a bar long, else up to the bar line after its last item, and loops all of it', () => { const f = makeSong(); expect(songLength(f.song())).toBe(BAR); diff --git a/tools/e2e/65-song.mjs b/tools/e2e/65-song.mjs index 5cd1ed4f..9f6d5bce 100644 --- a/tools/e2e/65-song.mjs +++ b/tools/e2e/65-song.mjs @@ -347,6 +347,22 @@ export async function song(t) { p = await json('__gm.song.laneAt(300, 3)'); await click(p.x, p.y); const emptyPick = await json('__gm.state.selection'); + // The song is written in one Sign (#363): picked from its list, then back to 4/4. + const signRows = await json("[__gm.inspectorField('Sign'), __gm.inspectorField('Beats per bar')]"); + const pickSign = async (sign) => { + const field = await json("__gm.inspectorField('Sign')"); + await click(field.x, field.y); + await sleep(150); + await clickText(sign); + await sleep(150); + return json(`__gm.state.project.assets.find((a) => a.id === ${JSON.stringify(id)})?.props.sign ?? null`); + }; + const signs = [await pickSign('6/8'), await pickSign('4/4')]; + check( + 'the song is written in one Sign, picked from its list', + signRows[0] !== null && signRows[1] === null && signs.join() === '6/8,4/4', + JSON.stringify({ signRows, signs }), + ); p = await json(`__gm.song.itemAt(${JSON.stringify(clip.id)}, 'body')`); await click(p.x, p.y); await sleep(150);