From 56425fca6a110794df52fe0b3211c498ef7ca738 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Mon, 19 Oct 2026 18:44:58 +0200 Subject: [PATCH] Let a song carry named markers on a strip in the song tab (#338) Stage 1 of #338. Game sync needs named places in a song: an event when the playhead passes one, and a switch that jumps to one. This stage adds the markers themselves. Games do nothing with them yet. - A song has markers, each with id, name and tick. A song without markers stores nothing for them. Two markers may share tick and name, since nodes will name them by id. - Markers change neither the sound nor the bake fingerprint. - The song tab draws a strip under the ruler. A marker has a green flag with its name and a faint line through the tracks. Tracks and lanes sit 18 px lower. The ruler of the pattern tab is unchanged. - Double click on an empty place adds a marker on the grid, with Alt or Shift on the exact tick. Dragging moves it in one undo step. - A click on a marker sets the start of playback there. A double click renames it in the name dialog of the hierarchy groups. - The context menu offers Play from here, Rename and Delete marker, on an empty place Add marker. A tooltip shows the full name. The wheel over the strip zooms time like the ruler. - New agent commands add_song_marker, change_song_marker and remove_song_marker. get_asset lists them under markers. Tests: - song-markers in core 6 and editor 4, 1 agent test, with the touched song, agent and MCP tests 175/175 - 9 counter-probes, all compile, all red - e2e 65 on atx: ALL PASSED, 70 checks, 3 new in 65g3. Counter-probe where a dragged marker is not taken over: 65g3 red - gate on atx: CHECK EXIT 0, 332 files, 5093 tests Co-Authored-By: Claude Opus 5.5 --- src/core/commands/songCommands.ts | 31 ++++ src/core/model/Asset.ts | 3 + src/core/music/markers.ts | 57 +++++++ src/core/store/serialize.ts | 3 + src/editor/agent/catalog/music.ts | 61 ++++++- src/editor/agent/tools.json | 2 +- src/editor/debug/songHooks.ts | 12 +- src/editor/song/SongTimeline.ts | 256 +++++++++++++++++++++++++++--- src/editor/song/markerView.ts | 51 ++++++ tests/core/song-markers.test.ts | 103 ++++++++++++ tests/editor/agent-music.test.ts | 39 +++++ tests/editor/song-markers.test.ts | 59 +++++++ tools/e2e/65-song.mjs | 71 +++++++++ 13 files changed, 718 insertions(+), 30 deletions(-) create mode 100644 src/core/music/markers.ts create mode 100644 src/editor/song/markerView.ts create mode 100644 tests/core/song-markers.test.ts create mode 100644 tests/editor/song-markers.test.ts diff --git a/src/core/commands/songCommands.ts b/src/core/commands/songCommands.ts index 67beddb1..9504ec42 100644 --- a/src/core/commands/songCommands.ts +++ b/src/core/commands/songCommands.ts @@ -8,6 +8,7 @@ import type { EdgeSend, Graph } from '../graph/Graph'; import type { Asset, SongAsset } from '../model/Asset'; import { findGraphAsset, type Project } from '../model/Project'; import { settleEmbedded } from '../music/embedded'; +import { heldSongMarkers, type SongMarker } from '../music/markers'; import { routedKind, sendsTapped } from '../music/projectRouting'; import { heldSend, heldSongGraph } from '../music/routing'; import { heldSongItems, heldSongTracks, type SongItem, type SongTrack } from '../music/song'; @@ -75,6 +76,36 @@ export class SetSongCommand implements Command { } } +/** Writes the markers of a song whole (#338): one gesture, one step to undo. */ +export class SetSongMarkersCommand implements Command { + private previous: SongMarker[] | undefined; + + constructor( + private readonly assetId: string, + private readonly markers: readonly SongMarker[], + readonly label = 'Edit markers', + ) {} + + /** Nothing to do when the song already has exactly these markers – no empty undo step. */ + static changes(asset: SongAsset, markers: readonly SongMarker[]): boolean { + return JSON.stringify(asset.markers ?? []) !== JSON.stringify(heldSongMarkers(markers)); + } + + execute(project: Project): void { + const asset = requireSong(project, this.assetId); + this.previous = asset.markers; + const held = heldSongMarkers(this.markers); + if (held.length > 0) asset.markers = held; + else delete asset.markers; + } + + undo(project: Project): void { + const asset = requireSong(project, this.assetId); + if (this.previous) asset.markers = this.previous; + else delete asset.markers; + } +} + /** * How loud a link of a song's routing sends, and where it taps its track (#350); the same for a link * of the master routing (#351), where only a bus and a song may tap anywhere but after their level. diff --git a/src/core/model/Asset.ts b/src/core/model/Asset.ts index 39a2f4ca..0682bdf5 100644 --- a/src/core/model/Asset.ts +++ b/src/core/model/Asset.ts @@ -1,5 +1,6 @@ import type { Graph } from '../graph/Graph'; import type { Bake } from '../music/bake'; +import type { SongMarker } from '../music/markers'; import type { PatternNote } from '../music/pattern'; import type { SongItem, SongTrack } from '../music/song'; import type { LoopRange } from '../music/sequence'; @@ -490,6 +491,8 @@ export interface SongAsset extends AssetBase { tracks: SongTrack[]; items: SongItem[]; loop?: LoopRange; + /** Named spots on its time (#338); left out without one. */ + markers?: SongMarker[]; graph: Graph; bake?: Bake; } diff --git a/src/core/music/markers.ts b/src/core/music/markers.ts new file mode 100644 index 00000000..c0f4f227 --- /dev/null +++ b/src/core/music/markers.ts @@ -0,0 +1,57 @@ +/** + * Markers of a song (#338): named spots on its time, where an event fires in the game and where a + * switch to the song can start. A marker sits on a whole tick; two may share one, and so may their + * names – a node names a marker by its id. + */ +import { newId } from '../ids'; + +export interface SongMarker { + id: string; + name: string; + tick: number; +} + +const finite = (value: unknown) => (typeof value === 'number' && Number.isFinite(value) ? value : NaN); + +/** Held to what a marker may be: a whole tick from 0, a name, an id no other has; in the order of their ticks. */ +export function heldSongMarkers(raw: unknown): SongMarker[] { + const ids = new Set(); + const markers: SongMarker[] = []; + for (const entry of Array.isArray(raw) ? (raw as unknown[]) : []) { + if (typeof entry !== 'object' || entry === null) continue; + const m = entry as Record; + const tick = Math.round(finite(m.tick)); + if (!(tick >= 0)) continue; + let id = typeof m.id === 'string' && m.id ? m.id : newId(); + while (ids.has(id)) id = newId(); + ids.add(id); + const name = typeof m.name === 'string' ? m.name.trim() : ''; + markers.push({ id, name: name || `Marker ${markers.length + 1}`, tick }); + } + return markers.sort((a, b) => a.tick - b.tick); +} + +/** "Marker 3": the first number no marker is called by yet. */ +export function nextMarkerName(markers: readonly SongMarker[]): string { + const taken = new Set(markers.map((m) => m.name)); + let n = 1; + while (taken.has(`Marker ${n}`)) n++; + return `Marker ${n}`; +} + +export function newMarker(markers: readonly SongMarker[], tick: number, name?: string): SongMarker { + return { id: newId(), name: name?.trim() || nextMarkerName(markers), tick: Math.max(0, Math.round(tick)) }; +} + +/** The markers with one of them changed; the others as they are. */ +export function changedMarker( + markers: readonly SongMarker[], + id: string, + change: Partial>, +): SongMarker[] { + return markers.map((m) => (m.id === id ? { ...m, ...change } : m)); +} + +export function withoutMarker(markers: readonly SongMarker[], id: string): SongMarker[] { + return markers.filter((m) => m.id !== id); +} diff --git a/src/core/store/serialize.ts b/src/core/store/serialize.ts index f1678f03..31ca77ba 100644 --- a/src/core/store/serialize.ts +++ b/src/core/store/serialize.ts @@ -25,6 +25,7 @@ import { heldBake } from '../music/bake'; import { heldStream } from '../music/streams'; import { heldPatternNotes } from '../music/pattern'; import { isEmbeddable } from '../music/embedded'; +import { heldSongMarkers } from '../music/markers'; import { heldSongItems, heldSongTracks } from '../music/song'; import { heldSend, heldSongGraph } from '../music/routing'; import { isMasterRoutingDef, isSongRoutingDef } from '../music/routingNodes'; @@ -348,6 +349,7 @@ export function deserializeProject(raw: unknown, fallbackId = ''): Project { const loop = readLoop(a.loop); const bake = heldBake(a.bake); const tracks = heldSongTracks(a.tracks); + const markers = heldSongMarkers(a.markers); assets.push({ id, type, @@ -356,6 +358,7 @@ export function deserializeProject(raw: unknown, fallbackId = ''): Project { tracks, items: heldSongItems(a.items, tracks), ...(loop && { loop }), + ...(markers.length > 0 && { markers }), graph: heldSongGraph(normalizeGraph(a.graph, isSongRoutingDef), tracks), ...(bake && { bake }), }); diff --git a/src/editor/agent/catalog/music.ts b/src/editor/agent/catalog/music.ts index b7eddd02..204b0aa2 100644 --- a/src/editor/agent/catalog/music.ts +++ b/src/editor/agent/catalog/music.ts @@ -9,7 +9,7 @@ import { oneStep } from '../../../core/commands/CompoundCommand'; import { ReplaceGraphCommand } from '../../../core/commands/graphCommands'; import { SetLoopCommand, SetPatternNotesCommand } from '../../../core/commands/patternCommands'; import { requireAsset, requireEdge, requirePattern, requireSong } from '../../../core/commands/require'; -import { SetSendCommand, SetSongCommand } from '../../../core/commands/songCommands'; +import { SetSendCommand, SetSongCommand, SetSongMarkersCommand } from '../../../core/commands/songCommands'; import type { SendTap } from '../../../core/graph/Graph'; import { newId } from '../../../core/ids'; import { @@ -41,6 +41,7 @@ import { laneRange, paramOf, } from '../../../core/music/automation'; +import { changedMarker, newMarker, type SongMarker, withoutMarker } from '../../../core/music/markers'; import { DEFAULT_VELOCITY, patternLength, type PatternNote } from '../../../core/music/pattern'; import { SEND_TAPS } from '../../../core/music/routing'; import { @@ -81,6 +82,7 @@ import { const PATTERN = id('Id of the pattern asset.'); const SONG = id('Id of the song asset.'); const TRACK = id('Id of the track, as get_asset lists it under tracks.'); +const MARKER = id('Id of the marker, as get_asset lists it under markers.'); const ITEM = id('Id of the item, as get_asset lists it under items.'); const INDEX = whole('Place in the list of tracks, from 0 (the top).', 0); const TICK = whole(`Ticks from the start; ${TICKS_PER_QUARTER} to a quarter note.`, 0); @@ -397,6 +399,57 @@ export const MUSIC_COMMANDS: readonly CatalogCommand[] = [ }, shows: theTab, }, + { + name: 'add_song_marker', + description: + 'Adds a marker to a song: a named spot on its time, drawn on the strip under the ruler of the song tab. ' + + 'Markers may share a tick and a name; a marker is known by its id.', + args: object({ asset: SONG, tick: TICK, name: id('Its name; "Marker " where left out.') }, [ + 'asset', + 'tick', + ]), + build: (args, { project }) => { + const song = requireSong(project, text(args, 'asset')); + const markers = song.markers ?? []; + const name = typeof args.name === 'string' ? args.name : undefined; + const marker = newMarker(markers, numberOf(args, 'tick'), name); + return { + command: new SetSongMarkersCommand(song.id, [...markers, marker], 'Add marker'), + made: { id: marker.id }, + }; + }, + shows: theTab, + }, + { + name: 'change_song_marker', + description: 'Renames a marker of a song or moves it to another tick. Give only what changes.', + args: object({ asset: SONG, marker: MARKER, name: id('Its name.'), tick: TICK }, ['asset', 'marker']), + build: (args, { project }) => { + const song = requireSong(project, text(args, 'asset')); + const marker = requireMarker(song, text(args, 'marker')); + const change: { name?: string; tick?: number } = {}; + if (typeof args.name === 'string') { + if (!args.name.trim()) throw new CommandError('A marker needs a name'); + change.name = args.name.trim(); + } + if (typeof args.tick === 'number') change.tick = numberOf(args, 'tick'); + const markers = changedMarker(song.markers ?? [], marker.id, change); + return { command: new SetSongMarkersCommand(song.id, markers, 'Change marker') }; + }, + shows: theTab, + }, + { + name: 'remove_song_marker', + description: 'Removes a marker of a song.', + args: object({ asset: SONG, marker: MARKER }, ['asset', 'marker']), + build: (args, { project }) => { + const song = requireSong(project, text(args, 'asset')); + const marker = requireMarker(song, text(args, 'marker')); + const markers = withoutMarker(song.markers ?? [], marker.id); + return { command: new SetSongMarkersCommand(song.id, markers, 'Delete marker') }; + }, + shows: theTab, + }, { name: 'add_song_item', description: @@ -690,6 +743,12 @@ function requireTrack(song: SongAsset, trackId: string): SongTrack { return track; } +function requireMarker(song: SongAsset, markerId: string): SongMarker { + const marker = song.markers?.find((m) => m.id === markerId); + if (!marker) throw new CommandError(`${song.name} has no marker ${markerId}`); + return marker; +} + /** A pitch the agent wrote as a MIDI number or a name like "C#4". */ export function pitchOf(value: unknown): number { const number = diff --git a/src/editor/agent/tools.json b/src/editor/agent/tools.json index b33f77bc..74077ee5 100644 --- a/src/editor/agent/tools.json +++ b/src/editor/agent/tools.json @@ -59,7 +59,7 @@ { "name": "get_asset", "title": "Read an asset", - "description": "One asset as stored: its property values, the rows of its properties (key, kind, default), the objects of a scene or container with their names and positions (the content of a placed container too, marked fromContainer), what the rows of an animation may aim at (targets), the cells of a small tilemap, the pixels of small pixel art as rows of palette indices (colors as \"#rrggbb\"), the notes of a pattern with their names and how many ticks it is long, on each track of a song its automation lanes and the parameters a lane can move (params), the rows a clip of a sound or video has (clipProperties), how many seconds a loaded sound lasts, and under silent what of a pattern, song or composition is not heard: all of it without an instrument, or the notes its instrument does not sound. Under usedBy: every asset naming it or one of its frames or clips, with the objects, nodes, items or keys in there that do (none for a property of the asset itself), and the project's start scene, master FX or Master routing; empty when nothing uses it. A graph is only counted here; read it with get_graph.", + "description": "One asset as stored: its property values, the rows of its properties (key, kind, default), the objects of a scene or container with their names and positions (the content of a placed container too, marked fromContainer), what the rows of an animation may aim at (targets), the cells of a small tilemap, the pixels of small pixel art as rows of palette indices (colors as \"#rrggbb\"), the notes of a pattern with their names and how many ticks it is long, on each track of a song its automation lanes and the parameters a lane can move (params), the markers of a song with their ids, names and ticks, the rows a clip of a sound or video has (clipProperties), how many seconds a loaded sound lasts, and under silent what of a pattern, song or composition is not heard: all of it without an instrument, or the notes its instrument does not sound. Under usedBy: every asset naming it or one of its frames or clips, with the objects, nodes, items or keys in there that do (none for a property of the asset itself), and the project's start scene, master FX or Master routing; empty when nothing uses it. A graph is only counted here; read it with get_graph.", "inputSchema": { "type": "object", "properties": { "asset": { "type": "string", "description": "Id of the asset." } }, diff --git a/src/editor/debug/songHooks.ts b/src/editor/debug/songHooks.ts index ebc5d471..9af2e20c 100644 --- a/src/editor/debug/songHooks.ts +++ b/src/editor/debug/songHooks.ts @@ -13,12 +13,17 @@ export function songHooks({ state, editor }: HookContext) { current: () => songEditor.currentAssetId, info: () => songEditor.shownInfo, rect: () => songEditor.canvasRect(), - /** The tracks, items and loop as the model holds them. */ + /** The tracks, items, loop and markers as the model holds them. */ model: (assetId: string) => { const asset = findAssetOf(state.project, assetId, 'song'); return asset ? (JSON.parse( - JSON.stringify({ tracks: asset.tracks, items: asset.items, loop: asset.loop ?? null }), + JSON.stringify({ + tracks: asset.tracks, + items: asset.items, + loop: asset.loop ?? null, + markers: asset.markers ?? [], + }), ) as unknown) : null; }, @@ -30,6 +35,9 @@ export function songHooks({ state, editor }: HookContext) { headAt: (track: number, part: HeadPart, share?: number) => timeline.headPoint(track, part, share), rulerAt: (tick: number) => timeline.rulerPoint(tick), loopAt: (tick: number) => timeline.loopPoint(tick), + /** The markers (#338): where a tick of their strip is on screen, and each as drawn. */ + markerAt: (tick: number) => timeline.markerPoint(tick), + markers: () => timeline.markersShown(), /** The automation lanes (#337): where a value of one is drawn, its head, and the rows as shown. */ automationAt: (track: number, target: AutomationTarget, tick: number, value: number) => timeline.automationPoint(track, target, tick, value), diff --git a/src/editor/song/SongTimeline.ts b/src/editor/song/SongTimeline.ts index 2b08dd28..ee15c081 100644 --- a/src/editor/song/SongTimeline.ts +++ b/src/editor/song/SongTimeline.ts @@ -1,4 +1,10 @@ -import { Container, type FederatedPointerEvent, type FederatedWheelEvent, Graphics } from 'pixi.js'; +import { + CanvasTextMetrics, + Container, + type FederatedPointerEvent, + type FederatedWheelEvent, + Graphics, +} from 'pixi.js'; import { CreateAssetCommand, MakeProjectAssetCommand } from '../../core/commands/assetCommands'; import { CompoundCommand } from '../../core/commands/CompoundCommand'; import { @@ -17,7 +23,7 @@ import { } from '../../core/music/automation'; import { trackParams } from '../../core/music/trackParams'; import { SetLoopCommand } from '../../core/commands/patternCommands'; -import { SetSongCommand, type SongParts } from '../../core/commands/songCommands'; +import { SetSongCommand, SetSongMarkersCommand, type SongParts } from '../../core/commands/songCommands'; import { embeddedCopy, embeddedName, @@ -27,6 +33,7 @@ import { listedAssets, songClockProps, } from '../../core/music/embedded'; +import { changedMarker, newMarker, type SongMarker, withoutMarker } from '../../core/music/markers'; import { PATTERN_BARS_KEY, PATTERN_INSTRUMENT_KEY, patternLength } from '../../core/music/pattern'; import { songRoutes, trackGates } from '../../core/music/routing'; import type { LoopRange } from '../../core/music/sequence'; @@ -114,6 +121,16 @@ import { trackLanes, trackRows, } from './laneView'; +import { GroupNameDialog } from '../panels/GroupNameDialog'; +import { + MARKER_FLAG, + MARKER_STRIP, + markerAt, + markerNameX, + type MarkerSpan, + markerSpans, + SONG_TOP, +} from './markerView'; import { openParamLaneDialog, ParamLaneDialog } from './ParamLaneDialog'; import { drawMeter } from './meterDraw'; import { @@ -187,6 +204,8 @@ const SOLO_COLOR = 0xc9c23a; const FX_COLOR = 0x8fb8de; /** The way and the points of an automation lane (#337), and a fader its lane rules. */ const AUTOMATION_COLOR = 0xe06c9f; +/** A marker of the song (#338): its flag on the strip and its line down the tracks. */ +const MARKER_COLOR = 0x4fc3b0; /** Height of an item's name strip; what it plays is drawn below. */ const LABEL_STRIP = 15; /** The letter on a lit mute, solo or FX button. */ @@ -203,7 +222,7 @@ interface Placed { dark?: boolean; } -type Area = 'corner' | 'ruler' | 'heads' | 'lanes'; +type Area = 'corner' | 'ruler' | 'markers' | 'heads' | 'lanes'; /** * What a press with Shift did to the picked items: `added` the item, or found it picked – then a @@ -222,6 +241,8 @@ type Gesture = edge: 'start' | 'end' | null; } | { kind: 'cursor'; tick: number } + /** A marker (#338) dragged along the strip; a click that does not move it starts playing there. */ + | { kind: 'marker'; id: string; tick: number; pointerX: number; moved: boolean } | { kind: 'empty'; start: Vec2; current: Vec2; active: boolean; add: boolean } | { kind: 'move'; @@ -304,7 +325,17 @@ export class SongTimeline extends Widget { private readonly headLabels = new Container(); private readonly headTexts = new TextPool(this.headLabels); private readonly ruler = new TimeRuler(); - private readonly clips = { lanes: new Graphics(), heads: new Graphics(), ruler: new Graphics() }; + /** The strip of the song's markers (#338), under the ruler. */ + private readonly markersLayer = new Container(); + private readonly markersG = new Graphics(); + private readonly markerLabels = new Container(); + private readonly markerTexts = new TextPool(this.markerLabels); + private readonly clips = { + lanes: new Graphics(), + heads: new Graphics(), + ruler: new Graphics(), + markers: new Graphics(), + }; private readonly hint = crispText({ text: '', style: textStyle('muted') }); private view: SongView = { panX: 0, pxPerTick: 0.02, panY: 0, trackHeight: TRACK_HEIGHT }; /** A zoom or a Fit shown as a transition (#377), the time and the tracks each on their own. */ @@ -363,11 +394,13 @@ export class SongTimeline extends Widget { this.focusable = true; this.lanesLayer.addChild(this.lanesG, this.automationG, this.itemsG, this.itemLabels, this.overlay); this.headsLayer.addChild(this.headsG, this.headLabels); + this.markersLayer.addChild(this.markersG, this.markerLabels); this.addChild( this.bg, this.lanesLayer, this.headsLayer, this.ruler, + this.markersLayer, this.hint, this.outLabel, this.outPeak, @@ -376,6 +409,8 @@ export class SongTimeline extends Widget { this.lanesLayer.mask = this.clips.lanes; this.headsLayer.mask = this.clips.heads; this.ruler.mask = this.clips.ruler; + this.markersLayer.mask = this.clips.markers; + this.markersLayer.eventMode = 'none'; this.hint.eventMode = 'none'; this.outLabel.eventMode = 'none'; this.outPeak.eventMode = 'none'; @@ -663,7 +698,7 @@ export class SongTimeline extends Widget { const at = this.getGlobalPosition(); return { x: at.x + HEAD_WIDTH + tickToX(this.view, tick), - y: at.y + RULER_HEIGHT + trackTop(this.view, this.rowOfIndex(track)) + this.view.trackHeight / 2, + y: at.y + SONG_TOP + trackTop(this.view, this.rowOfIndex(track)) + this.view.trackHeight / 2, }; } @@ -676,7 +711,7 @@ export class SongTimeline extends Widget { const at = this.getGlobalPosition(); return { x: at.x + HEAD_WIDTH + tickToX(this.view, tick), - y: at.y + RULER_HEIGHT + trackTop(this.view, row) + laneY(kind, value, this.view.trackHeight), + y: at.y + SONG_TOP + trackTop(this.view, row) + laneY(kind, value, this.view.trackHeight), }; } @@ -688,7 +723,7 @@ export class SongTimeline extends Widget { const at = this.getGlobalPosition(); return { x: at.x + HEAD_WIDTH / 2, - y: at.y + RULER_HEIGHT + trackTop(this.view, row) + this.view.trackHeight / 2, + y: at.y + SONG_TOP + trackTop(this.view, row) + this.view.trackHeight / 2, }; } @@ -712,7 +747,7 @@ export class SongTimeline extends Widget { const box = itemBox(this.view, row, item); const at = this.getGlobalPosition(); const x = part === 'start' ? box.x + 2 : part === 'end' ? box.x + box.w - 2 : box.x + box.w / 2; - return { x: at.x + HEAD_WIDTH + x, y: at.y + RULER_HEIGHT + box.y + box.h / 2 }; + return { x: at.x + HEAD_WIDTH + x, y: at.y + SONG_TOP + box.y + box.h / 2 }; } /** The meters of these tracks and Out a frame on (#359), with the loudest sample on each side since. */ @@ -770,7 +805,7 @@ export class SongTimeline extends Widget { const box = trackMeterBox(this.view.trackHeight); return { x: at.x + box.x + box.w / 2, - y: at.y + RULER_HEIGHT + trackTop(this.view, this.rowOfIndex(track)) + box.y + box.h / 2, + y: at.y + SONG_TOP + trackTop(this.view, this.rowOfIndex(track)) + box.y + box.h / 2, }; } @@ -781,7 +816,7 @@ export class SongTimeline extends Widget { const slider = part === 'volume' || part === 'pan'; return { x: at.x + box.x + (slider ? box.w * share : box.w / 2), - y: at.y + RULER_HEIGHT + trackTop(this.view, this.rowOfIndex(track)) + box.y + box.h / 2, + y: at.y + SONG_TOP + trackTop(this.view, this.rowOfIndex(track)) + box.y + box.h / 2, }; } @@ -790,6 +825,25 @@ export class SongTimeline extends Widget { return { x: at.x + HEAD_WIDTH + tickToX(this.view, tick), y: at.y + RULER_HEIGHT - 6 }; } + /** A tick on the strip of the song's markers (#338). */ + markerPoint(tick: number): Vec2 { + const at = this.getGlobalPosition(); + return { x: at.x + HEAD_WIDTH + tickToX(this.view, tick), y: at.y + RULER_HEIGHT + MARKER_STRIP / 2 }; + } + + /** The markers as drawn, a dragged one where it would land, with where each is on screen. */ + markersShown(): (SongMarker & { x: number; w: number })[] { + const asset = this.asset(); + if (!asset) return []; + const at = this.getGlobalPosition(); + const markers = this.shownMarkers(asset); + return this.spansOf(markers).map((span, i) => ({ + ...markers[i]!, + x: at.x + HEAD_WIDTH + span.x, + w: span.w, + })); + } + /** A tick on the strip of the ruler that holds the loop. */ loopPoint(tick: number): Vec2 { const at = this.getGlobalPosition(); @@ -840,7 +894,7 @@ export class SongTimeline extends Widget { id: item.id, label: this.labelOf(item), content: this.contentOf(item), - box: { x: at.x + HEAD_WIDTH + box.x, y: at.y + RULER_HEIGHT + box.y, w: box.w, h: box.h }, + box: { x: at.x + HEAD_WIDTH + box.x, y: at.y + SONG_TOP + box.y, w: box.w, h: box.h }, seams: this.seamsOf(item), }, ]; @@ -854,7 +908,7 @@ export class SongTimeline extends Widget { } private get lanesHeight(): number { - return Math.max(0, this.h - RULER_HEIGHT); + return Math.max(0, this.h - SONG_TOP); } private asset(): SongAsset | null { @@ -966,12 +1020,13 @@ export class SongTimeline extends Widget { private areaAt(p: Vec2): Area { const left = p.x < HEAD_WIDTH; if (p.y < RULER_HEIGHT) return left ? 'corner' : 'ruler'; + if (p.y < SONG_TOP) return left ? 'corner' : 'markers'; return left ? 'heads' : 'lanes'; } /** A point of the widget in the lanes' own pixels. */ private inLanes(p: Vec2): Vec2 { - return { x: p.x - HEAD_WIDTH, y: p.y - RULER_HEIGHT }; + return { x: p.x - HEAD_WIDTH, y: p.y - SONG_TOP }; } private onPointerDown(e: FederatedPointerEvent): void { @@ -1001,6 +1056,7 @@ export class SongTimeline extends Widget { } if (e.button === 2) { if (area === 'ruler') this.openRulerMenu(asset, Math.max(0, tick), e.global); + if (area === 'markers') this.openMarkerMenu(asset, g.x, tick, e.global); const lane = area === 'lanes' ? this.laneAt(asset, g.y) : null; if (lane) this.openLaneMenu(lane, g, e.global); else if (area === 'lanes') this.openItemMenu(asset, g, e.global); @@ -1025,10 +1081,27 @@ export class SongTimeline extends Widget { kind: 'cursor', tick: this.cursorTick(tick, songGrid(asset.props), e.altKey || e.shiftKey), }; - } else this.pressLanes(asset, e, g); + } else if (area === 'markers') this.pressMarkers(asset, e, g.x, tick); + else this.pressLanes(asset, e, g); this.invalidate(); } + /** + * A press on the strip of the markers (#338): on a marker it takes it to drag, a second one names + * it; a second press on nothing adds one there – on the grid, with Alt or Shift on the tick. + */ + private pressMarkers(asset: SongAsset, e: FederatedPointerEvent, x: number, tick: number): void { + const id = markerAt(this.spansOf(asset.markers ?? []), x); + const marker = asset.markers?.find((m) => m.id === id); + if (!marker) { + if (pressCount(e, `song-markers:${asset.id}`) === 2) + this.addMarker(asset, this.cursorTick(tick, songGrid(asset.props), e.altKey || e.shiftKey)); + return; + } + if (pressCount(e, `song-marker:${marker.id}`) === 2) return this.renameMarker(marker); + this.gesture = { kind: 'marker', id: marker.id, tick: marker.tick, pointerX: x, moved: false }; + } + private pressHead(asset: SongAsset, e: FederatedPointerEvent, y: number, x: number): void { const rows = this.rows(asset); const at = trackAtY(this.view, rows.length, y); @@ -1233,6 +1306,11 @@ export class SongTimeline extends Widget { case 'cursor': gesture.tick = this.cursorTick(tick, grid, free); break; + case 'marker': + if (!gesture.moved && Math.abs(g.x - gesture.pointerX) < DRAG_THRESHOLD) return; + gesture.moved = true; + gesture.tick = this.cursorTick(tick, grid, free); + break; case 'empty': { gesture.current = g; const span = Math.max(Math.abs(g.x - gesture.start.x), Math.abs(g.y - gesture.start.y)); @@ -1325,6 +1403,18 @@ export class SongTimeline extends Widget { case 'cursor': this.setStart(gesture.tick); break; + case 'marker': { + const marker = asset.markers?.find((m) => m.id === gesture.id); + if (!marker) break; + if (!gesture.moved) this.setStart(marker.tick); + else if (gesture.tick !== marker.tick) + this.commitMarkers( + asset, + changedMarker(asset.markers!, marker.id, { tick: gesture.tick }), + 'Move marker', + ); + break; + } case 'empty': { const caught = gesture.active ? itemsWithin(this.view, asset.items, this.rowsOfTracks(asset), gesture.start, gesture.current) @@ -1404,7 +1494,7 @@ export class SongTimeline extends Widget { const inside = local.x >= HEAD_WIDTH && local.x <= this.w && - local.y >= RULER_HEIGHT && + local.y >= SONG_TOP && local.y <= this.h && xToTick(this.view, g.x) >= 0; const row = this.trackAt(asset, g.y); @@ -1435,7 +1525,9 @@ export class SongTimeline extends Widget { * up. A point of a lane (#337) says its value. */ private followHover(asset: SongAsset, local: Vec2, g: Vec2, global: Vec2): void { - const inside = local.x >= HEAD_WIDTH && local.x <= this.w && local.y >= RULER_HEIGHT && local.y <= this.h; + if (local.x <= this.w && this.areaAt(local) === 'markers') + return this.followMarkerHover(asset, g, global); + const inside = local.x >= HEAD_WIDTH && local.x <= this.w && local.y >= SONG_TOP && local.y <= this.h; const lane = inside ? this.laneAt(asset, g.y) : null; const point = lane && @@ -1472,6 +1564,30 @@ export class SongTimeline extends Widget { this.invalidate(); } + /** A marker under the pointer says its whole name and can be dragged; the strip off one says how to add one. */ + private followMarkerHover(asset: SongAsset, g: Vec2, global: Vec2): void { + const spans = this.spansOf(asset.markers ?? []); + const id = markerAt(spans, g.x); + const marker = asset.markers?.find((m) => m.id === id); + const span = spans.find((s) => s.id === id); + const at = this.getGlobalPosition(); + const anchor = span + ? { x: at.x + HEAD_WIDTH + span.x, y: at.y + RULER_HEIGHT, w: span.w, h: MARKER_STRIP } + : { x: global.x - 4, y: global.y - 4, w: 8, h: 8 }; + tooltips.point( + this, + marker ? `marker:${marker.id}` : 'markers', + () => ({ text: marker ? marker.name : 'Double-click to add a marker', anchor }), + { x: global.x, y: global.y }, + ); + const cursor = marker ? 'ew-resize' : 'default'; + if (this.cursor !== cursor) this.cursor = cursor; + if (this.hover) { + this.hover = null; + this.invalidate(); + } + } + /** Where a paste goes with the pointer over the lanes: the grid line before it and its track; else null. */ private pasteSpot(asset: SongAsset): { tick: number; row: number } | null { const p = this.pointer; @@ -1498,6 +1614,51 @@ export class SongTimeline extends Widget { if (SetSongCommand.changes(asset, parts)) this.state.execute(new SetSongCommand(asset.id, parts, label)); } + private commitMarkers(asset: SongAsset, markers: readonly SongMarker[], label: string): void { + if (SetSongMarkersCommand.changes(asset, markers)) + this.state.execute(new SetSongMarkersCommand(asset.id, markers, label)); + } + + /** A new marker at a tick, named at once. */ + private addMarker(asset: SongAsset, tick: number): void { + const markers = asset.markers ?? []; + this.commitMarkers(asset, [...markers, newMarker(markers, tick)], 'Add marker'); + } + + private renameMarker(marker: SongMarker): void { + GroupNameDialog.open( + new GroupNameDialog('Rename marker', marker.name, 'Rename', (name) => { + const now = this.asset(); + if (now?.markers?.some((m) => m.id === marker.id)) + this.commitMarkers(now, changedMarker(now.markers, marker.id, { name }), 'Rename marker'); + }), + ); + } + + /** The menu of the marker under a right-click on the strip; off one, a marker to add there. */ + private openMarkerMenu(asset: SongAsset, x: number, tick: number, global: Vec2): void { + const id = markerAt(this.spansOf(asset.markers ?? []), x); + const marker = asset.markers?.find((m) => m.id === id); + if (!marker) { + const at = this.cursorTick(tick, songGrid(asset.props), false); + new Menu([{ label: 'Add marker', onSelect: () => this.addMarker(asset, at) }]).open(global.x, global.y); + return; + } + new Menu([ + { label: 'Play from here', onSelect: () => this.setStart(marker.tick) }, + { label: 'Rename…', onSelect: () => this.renameMarker(marker) }, + { separator: true }, + { + label: 'Delete marker', + danger: true, + onSelect: () => { + const now = this.asset(); + if (now) this.commitMarkers(now, withoutMarker(now.markers ?? [], marker.id), 'Delete marker'); + }, + }, + ]).open(global.x, global.y); + } + private openRulerMenu(asset: SongAsset, tick: number, global: Vec2): void { const bar = ticksPerBar(songMeter(asset.props)); const first = snapDown(tick, bar); @@ -1659,8 +1820,7 @@ export class SongTimeline extends Widget { const kind = laneKindOf(track, target); const at = this.getGlobalPosition(); const x = at.x + HEAD_WIDTH + tickToX(this.view, point.tick); - const y = - at.y + RULER_HEIGHT + trackTop(this.view, row) + laneY(kind, point.value, this.view.trackHeight); + const y = at.y + SONG_TOP + trackTop(this.view, row) + laneY(kind, point.value, this.view.trackHeight); return { text: laneValueText(kind, point.value), anchor: { x: x - 4, y: y - 4, w: 8, h: 8 } }; } @@ -1793,7 +1953,7 @@ export class SongTimeline extends Widget { const g = this.inLanes({ x: local.x, y: local.y }); const factor = zoomFactor(e.deltaY); const tracks = area === 'heads'; - if (!tracks && area !== 'ruler' && area !== 'lanes') return; + if (!tracks && area !== 'ruler' && area !== 'markers' && area !== 'lanes') return; (tracks ? this.trackTween : this.timeTween).change(() => { const view = tracks ? zoomTracks(this.view, factor, g.y) @@ -1874,6 +2034,19 @@ export class SongTimeline extends Widget { return songLoop(asset); } + /** The markers as a drag on the strip would leave them. */ + private shownMarkers(asset: SongAsset): SongMarker[] { + const g = this.gesture; + const markers = asset.markers ?? []; + if (g?.kind !== 'marker' || !g.moved) return markers; + return changedMarker(markers, g.id, { tick: g.tick }).sort((a, b) => a.tick - b.tick); + } + + private spansOf(markers: readonly SongMarker[]): MarkerSpan[] { + const style = textStyle('small'); + return markerSpans(this.view, markers, (name) => CanvasTextMetrics.measureText(name, style).width); + } + /** Where playing starts, as a drag on the ruler would leave it. */ private shownStart(): number { return this.gesture?.kind === 'cursor' ? this.gesture.tick : this.startTick; @@ -1925,16 +2098,18 @@ export class SongTimeline extends Widget { const lh = this.lanesHeight; this.bg.clear().rect(0, 0, w, h).fill(c.viewport); this.bg.rect(0, 0, HEAD_WIDTH, h).fill(c.panel); - this.bg.rect(0, 0, w, RULER_HEIGHT).fill(c.panelHeader); + this.bg.rect(0, 0, w, SONG_TOP).fill(c.panelHeader); this.bg.rect(HEAD_WIDTH - 1, 0, 1, h).fill(c.border); - this.clips.lanes.clear().rect(HEAD_WIDTH, RULER_HEIGHT, lw, lh).fill(0xffffff); + this.clips.lanes.clear().rect(HEAD_WIDTH, SONG_TOP, lw, lh).fill(0xffffff); this.clips.heads .clear() - .rect(0, RULER_HEIGHT, HEAD_WIDTH - 1, lh) + .rect(0, SONG_TOP, HEAD_WIDTH - 1, lh) .fill(0xffffff); this.clips.ruler.clear().rect(HEAD_WIDTH, 0, lw, RULER_HEIGHT).fill(0xffffff); - this.lanesLayer.position.set(HEAD_WIDTH, RULER_HEIGHT); - this.headsLayer.position.set(0, RULER_HEIGHT); + this.clips.markers.clear().rect(HEAD_WIDTH, RULER_HEIGHT, lw, MARKER_STRIP).fill(0xffffff); + this.markersLayer.position.set(HEAD_WIDTH, RULER_HEIGHT); + this.lanesLayer.position.set(HEAD_WIDTH, SONG_TOP); + this.headsLayer.position.set(0, SONG_TOP); this.ruler.position.set(HEAD_WIDTH, 0); const asset = this.asset(); @@ -1942,14 +2117,16 @@ export class SongTimeline extends Widget { this.automationG.clear(); this.itemsG.clear(); this.overlay.clear(); + this.markersG.clear(); this.headsG.clear(); this.outLabel.visible = this.outPeak.visible = asset !== null; if (!asset) { this.itemTexts.show([]); this.headTexts.show([]); this.ruler.visible = false; + this.markerTexts.show([]); this.hint.text = 'No song selected – pick one in the Assets panel or create one.'; - this.hint.position.set(HEAD_WIDTH + Theme.padding * 2, RULER_HEIGHT + Theme.padding * 2); + this.hint.position.set(HEAD_WIDTH + Theme.padding * 2, SONG_TOP + Theme.padding * 2); this.hint.visible = true; return; } @@ -1968,6 +2145,7 @@ export class SongTimeline extends Widget { start: this.shownStart(), playhead: this.playhead, }); + this.drawMarkers(asset, lw, lh); const g = this.gesture; if (g?.kind === 'empty' && g.active) drawBand(this.overlay, g.start, g.current); if (this.drop) { @@ -1993,10 +2171,36 @@ export class SongTimeline extends Widget { this.hint.visible = this.hint.text !== ''; this.hint.position.set( HEAD_WIDTH + Theme.padding * 2, - RULER_HEIGHT + trackTop(this.view, rows.length) + Theme.padding * 2, + SONG_TOP + trackTop(this.view, rows.length) + Theme.padding * 2, ); } + /** The markers (#338): a flag and the name on their strip, and a faint line down the tracks. */ + private drawMarkers(asset: SongAsset, lw: number, lh: number): void { + const g = this.markersG; + g.rect(0, MARKER_STRIP - 1, lw, 1).fill(Theme.colors.border); + const markers = this.shownMarkers(asset); + const names = new Map(markers.map((m) => [m.id, m.name])); + const dragged = this.gesture?.kind === 'marker' && this.gesture.moved ? this.gesture.id : null; + const spans = this.spansOf(markers).filter((span) => span.x + span.w >= 0 && span.x - MARKER_FLAG <= lw); + for (const span of spans) { + const x = Math.round(span.x); + const alpha = span.id === dragged ? 1 : 0.85; + g.rect(x, 0, 1, MARKER_STRIP - 1).fill({ color: MARKER_COLOR, alpha }); + g.moveTo(x, 1) + .lineTo(x + MARKER_FLAG, 5) + .lineTo(x, 9) + .closePath() + .fill({ color: MARKER_COLOR, alpha }); + if (x >= 0 && x <= lw) + this.overlay.rect(x, 0, 1, lh).fill({ color: MARKER_COLOR, alpha: span.id === dragged ? 0.6 : 0.3 }); + } + const style = textStyle('small'); + this.markerTexts + .show(spans.map((span) => truncate(names.get(span.id)!, style, span.x + span.w - markerNameX(span)))) + .forEach((text, i) => text.position.set(Math.round(markerNameX(spans[i]!)), 2)); + } + /** * A band per track, the picked one lit, a darker one for each of its open automation lanes (#337), * and the bar, beat and grid lines across them. diff --git a/src/editor/song/markerView.ts b/src/editor/song/markerView.ts new file mode 100644 index 00000000..4de03511 --- /dev/null +++ b/src/editor/song/markerView.ts @@ -0,0 +1,51 @@ +import type { SongMarker } from '../../core/music/markers'; +import { RULER_HEIGHT, type TimeView, tickToX } from '../music/timeView'; + +/** The strip under the song's ruler that holds its markers (#338). */ +export const MARKER_STRIP = 18; +/** Where the tracks of the Song tab start: below the ruler and its markers. */ +export const SONG_TOP = RULER_HEIGHT + MARKER_STRIP; +/** How far either side of its line a press still takes a marker. */ +export const MARKER_REACH = 5; +/** The flag at the head of a marker's line, and the room between it and the name. */ +export const MARKER_FLAG = 8; +const NAME_GAP = 3; + +/** Where a marker is drawn on the strip: its line at `x`, flag and name over `w` pixels right of it. */ +export interface MarkerSpan { + id: string; + x: number; + w: number; +} + +/** + * Each marker as drawn on the strip: its flag and as much of its name as there is room for before + * the next one. `measure` gives the width of a name in pixels. + */ +export function markerSpans( + view: TimeView, + markers: readonly SongMarker[], + measure: (name: string) => number, +): MarkerSpan[] { + return markers.map((marker, i) => { + const x = tickToX(view, marker.tick); + const next = markers[i + 1]; + const room = next ? tickToX(view, next.tick) - x - 2 : Infinity; + const w = Math.max(MARKER_FLAG, Math.min(MARKER_FLAG + NAME_GAP + measure(marker.name), room)); + return { id: marker.id, x, w }; + }); +} + +/** Where a marker's name starts, right of its flag. */ +export function markerNameX(span: MarkerSpan): number { + return span.x + MARKER_FLAG + NAME_GAP; +} + +/** The marker under `x` on the strip, the last drawn first: near its line, or on its flag and name. */ +export function markerAt(spans: readonly MarkerSpan[], x: number): string | null { + for (let i = spans.length - 1; i >= 0; i--) { + const span = spans[i]!; + if (x >= span.x - MARKER_REACH && x <= span.x + Math.max(span.w, MARKER_REACH)) return span.id; + } + return null; +} diff --git a/tests/core/song-markers.test.ts b/tests/core/song-markers.test.ts new file mode 100644 index 00000000..7f404c09 --- /dev/null +++ b/tests/core/song-markers.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest'; +import { CreateAssetCommand } from '../../src/core/commands/assetCommands'; +import { SetSongMarkersCommand } from '../../src/core/commands/songCommands'; +import type { SongAsset } from '../../src/core/model/Asset'; +import { + changedMarker, + heldSongMarkers, + newMarker, + nextMarkerName, + withoutMarker, +} from '../../src/core/music/markers'; +import { makeProject, roundTrip } from '../helpers'; + +function makeSong() { + const f = makeProject(); + const create = new CreateAssetCommand('song', 'Tune'); + f.stack.execute(create); + const song = () => f.project.assets.find((a) => a.id === create.asset.id) as SongAsset; + return { ...f, song }; +} + +describe('song markers (#338)', () => { + it('holds markers to whole ticks from 0, a name and ids of their own, in the order of their ticks', () => { + const held = heldSongMarkers([ + { id: 'b', name: ' Boss ', tick: 3840.4 }, + { id: 'a', name: 'Intro', tick: 0 }, + { id: 'a', name: '', tick: 1920 }, + { id: 'c', name: 'Before', tick: -1 }, + { id: 'd', name: 'Nowhere' }, + 'no marker', + null, + ]); + expect(held.map((m) => [m.name, m.tick])).toEqual([ + ['Intro', 0], + ['Marker 3', 1920], + ['Boss', 3840], + ]); + expect(new Set(held.map((m) => m.id)).size).toBe(3); + expect(held.find((m) => m.tick === 1920)!.id).not.toBe('a'); + expect(heldSongMarkers(undefined)).toEqual([]); + }); + + it('keeps markers on one tick in the order they came', () => { + const held = heldSongMarkers([ + { id: 'x', name: 'Second', tick: 960 }, + { id: 'y', name: 'First', tick: 0 }, + { id: 'z', name: 'Third', tick: 960 }, + ]); + expect(held.map((m) => m.name)).toEqual(['First', 'Second', 'Third']); + }); + + it('names a new marker by the first number not taken', () => { + expect(nextMarkerName([])).toBe('Marker 1'); + const markers = heldSongMarkers([ + { name: 'Marker 1', tick: 0 }, + { name: 'Marker 3', tick: 10 }, + ]); + expect(nextMarkerName(markers)).toBe('Marker 2'); + const made = newMarker(markers, 479.6); + expect(made).toMatchObject({ name: 'Marker 2', tick: 480 }); + expect(newMarker(markers, -5, ' Drop ')).toMatchObject({ name: 'Drop', tick: 0 }); + }); + + it('changes or takes away one marker and leaves the others', () => { + const markers = heldSongMarkers([ + { id: 'a', name: 'A', tick: 0 }, + { id: 'b', name: 'B', tick: 960 }, + ]); + expect(changedMarker(markers, 'b', { tick: 1920 })).toEqual([markers[0], { ...markers[1], tick: 1920 }]); + expect(changedMarker(markers, 'a', { name: 'Start' })[0]).toEqual({ id: 'a', name: 'Start', tick: 0 }); + expect(withoutMarker(markers, 'a')).toEqual([markers[1]]); + }); + + it('writes the markers as one step, leaves the key out without one and takes it back on undo', () => { + const f = makeSong(); + expect(f.song().markers).toBeUndefined(); + const one = [{ id: 'm', name: 'Drop', tick: 1920.2 }]; + expect(SetSongMarkersCommand.changes(f.song(), one)).toBe(true); + f.stack.execute(new SetSongMarkersCommand(f.song().id, one, 'Add marker')); + expect(f.song().markers).toEqual([{ id: 'm', name: 'Drop', tick: 1920 }]); + expect(SetSongMarkersCommand.changes(f.song(), one)).toBe(false); + f.stack.execute(new SetSongMarkersCommand(f.song().id, [], 'Delete marker')); + expect('markers' in f.song()).toBe(false); + f.stack.undo(); + expect(f.song().markers).toEqual([{ id: 'm', name: 'Drop', tick: 1920 }]); + f.stack.undo(); + expect('markers' in f.song()).toBe(false); + }); + + it('keeps the markers through saving and loading', () => { + const f = makeSong(); + const markers = [ + { id: 'a', name: 'Intro', tick: 0 }, + { id: 'b', name: 'Boss', tick: 7680 }, + ]; + f.stack.execute(new SetSongMarkersCommand(f.song().id, markers)); + const back = roundTrip(f.project).assets.find((a) => a.id === f.song().id) as SongAsset; + expect(back.markers).toEqual(markers); + const none = makeSong(); + const plain = roundTrip(none.project).assets.find((a) => a.id === none.song().id) as SongAsset; + expect('markers' in plain).toBe(false); + }); +}); diff --git a/tests/editor/agent-music.test.ts b/tests/editor/agent-music.test.ts index ebff2c58..20ae7d82 100644 --- a/tests/editor/agent-music.test.ts +++ b/tests/editor/agent-music.test.ts @@ -224,6 +224,45 @@ describe('songs', () => { expect(noFx.text).toContain('is no Sound FX asset'); }); + it('adds, renames, moves and removes markers of a song (#338), and get_asset lists them', async () => { + const fixture = makeSpriteScene(); + const { Theme } = await made(fixture, { Theme: 'song' }); + const song = () => byId(fixture, Theme); + const done = await run(fixture, [ + { command: 'add_song_marker', args: { asset: Theme, tick: 7680, name: 'Boss' }, as: 'boss' }, + { command: 'add_song_marker', args: { asset: Theme, tick: 0 } }, + { command: 'change_song_marker', args: { asset: Theme, marker: '$boss', tick: 3840 } }, + ]); + expect(done.result.isError).toBeUndefined(); + expect(song().markers!.map(({ name, tick }) => [name, tick])).toEqual([ + ['Marker 1', 0], + ['Boss', 3840], + ]); + const listed = (await read(fixture, Theme)).markers as { id: string; name: string; tick: number }[]; + expect(listed.map((m) => m.name)).toEqual(['Marker 1', 'Boss']); + + const boss = song().markers![1]!.id; + const blank = await run(fixture, [ + { command: 'change_song_marker', args: { asset: Theme, marker: boss, name: ' ' } }, + ]); + expect(blank.text).toContain('A marker needs a name'); + const renamed = await run(fixture, [ + { command: 'change_song_marker', args: { asset: Theme, marker: boss, name: 'Fight' } }, + ]); + expect(renamed.result.isError).toBeUndefined(); + expect(song().markers![1]).toEqual({ id: boss, name: 'Fight', tick: 3840 }); + + const gone = await run(fixture, [ + { command: 'remove_song_marker', args: { asset: Theme, marker: boss } }, + ]); + expect(gone.result.isError).toBeUndefined(); + expect(song().markers!.map((m) => m.name)).toEqual(['Marker 1']); + const missing = await run(fixture, [ + { command: 'remove_song_marker', args: { asset: Theme, marker: boss } }, + ]); + expect(missing.text).toContain(`Theme has no marker ${boss}`); + }); + it('sets the volume lane of a track in tick order with its easings, refuses what is out of range, and clears it with []', async () => { const fixture = makeSpriteScene(); const { Theme, Bend } = await made(fixture, { Theme: 'song', Bend: 'curve' }); diff --git a/tests/editor/song-markers.test.ts b/tests/editor/song-markers.test.ts new file mode 100644 index 00000000..9ba76e49 --- /dev/null +++ b/tests/editor/song-markers.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { heldSongMarkers } from '../../src/core/music/markers'; +import { + MARKER_FLAG, + MARKER_REACH, + MARKER_STRIP, + markerAt, + markerNameX, + markerSpans, + SONG_TOP, +} from '../../src/editor/song/markerView'; +import { RULER_HEIGHT } from '../../src/editor/music/timeView'; + +/** A tick a pixel, from x 100; every name 6 pixels a letter. */ +const VIEW = { panX: 100, pxPerTick: 1 }; +const measure = (name: string) => name.length * 6; + +describe('the marker strip of the song tab (#338)', () => { + it('lies between the ruler and the tracks', () => { + expect(SONG_TOP).toBe(RULER_HEIGHT + MARKER_STRIP); + }); + + it('draws a marker at its tick, as wide as its flag and name, but not into the next one', () => { + const markers = heldSongMarkers([ + { id: 'a', name: 'Intro', tick: 0 }, + { id: 'b', name: 'A long name', tick: 40 }, + { id: 'c', name: '', tick: 300 }, + ]); + const spans = markerSpans(VIEW, markers, measure); + expect(spans.map((s) => s.x)).toEqual([100, 140, 400]); + // Room to the next one is 40 pixels, less 2: the name of "Intro" (30) and the flag fit just so. + expect(spans[0]!.w).toBe(38); + expect(spans[1]!.w).toBe(MARKER_FLAG + 3 + measure('A long name')); + expect(spans[2]!.w).toBe(MARKER_FLAG + 3 + measure('Marker 3')); + expect(markerNameX(spans[0]!)).toBe(100 + MARKER_FLAG + 3); + }); + + it('keeps the flag of a marker with another one close behind it', () => { + const markers = heldSongMarkers([ + { id: 'a', name: 'Close', tick: 0 }, + { id: 'b', name: 'By', tick: 2 }, + ]); + expect(markerSpans(VIEW, markers, measure)[0]!.w).toBe(MARKER_FLAG); + }); + + it('takes the marker near its line or on its flag and name, the later one where they overlap', () => { + const markers = heldSongMarkers([ + { id: 'a', name: 'Intro', tick: 0 }, + { id: 'b', name: 'B', tick: 6 }, + ]); + const spans = markerSpans(VIEW, markers, measure); + expect(markerAt(spans, 100 - MARKER_REACH)).toBe('a'); + expect(markerAt(spans, 100 - MARKER_REACH - 1)).toBeNull(); + expect(markerAt(spans, 103)).toBe('b'); + expect(markerAt(spans, 106 + MARKER_FLAG + 3 + 6)).toBe('b'); + expect(markerAt(spans, 106 + MARKER_FLAG + 3 + 7)).toBeNull(); + expect(markerAt([], 100)).toBeNull(); + }); +}); diff --git a/tools/e2e/65-song.mjs b/tools/e2e/65-song.mjs index 8ff79493..7e3141bf 100644 --- a/tools/e2e/65-song.mjs +++ b/tools/e2e/65-song.mjs @@ -628,6 +628,77 @@ export async function song(t) { ); await evaluate("__gm.state.notice = ''"); + // 65g3. Markers (#338) on their strip under the ruler. A double-click there adds one on the grid, + // a drag moves it as one step, a click starts playing at it, a double-click names it, resting + // on it says its name, and its menu takes it away. + const songMarkers = async () => (await model()).markers; + const markerAt = (tick) => json(`__gm.song.markerAt(${tick})`); + p = await markerAt(BAR + 100); + await doubleClick(p.x, p.y); + await sleep(100); + const added = await songMarkers(); + p = await markerAt(BAR); + to = await markerAt(3 * BAR + 50); + await drag(p.x, p.y, to.x, to.y); + await sleep(100); + const movedMarkers = await songMarkers(); + const movedLabel = await json('__gm.state.commands.undoLabel'); + await press('z', { ctrl: true }); + await sleep(100); + const markerUndone = await songMarkers(); + await clickToolbar('redo'); + await sleep(100); + const markerDrawn = (await json('__gm.song.markers()'))[0]; + const rulerX = (await json(`__gm.song.rulerAt(${3 * BAR})`)).x; + check( + 'a double-click on the strip adds a marker on the bar, a drag moves it as one step, drawn at its tick', + added.length === 1 && + added[0].name === 'Marker 1' && + added[0].tick === BAR && + movedMarkers[0].tick === 3 * BAR && + movedLabel === 'Move marker' && + markerUndone[0].tick === BAR && + (await songMarkers())[0].tick === 3 * BAR && + Math.abs(markerDrawn?.x - rulerX) < 1, + JSON.stringify({ added, movedMarkers, movedLabel, markerUndone, markerDrawn, rulerX }), + ); + p = await markerAt(3 * BAR); + await click(p.x, p.y); + await sleep(100); + const startAtMarker = (await json('__gm.song.view()')).start; + await sleep(500); + await doubleClick(p.x, p.y); + await sleep(150); + const nameDialog = await json('__gm.groupDialogOpen()'); + await t.typeText('Boss'); + await press('Enter'); + await sleep(100); + const named = (await songMarkers())[0]; + // The presses left the marker's sign quiet: the pointer goes off the strip and back. + to = await json(`__gm.song.rulerAt(${3 * BAR})`); + await mouse('mouseMoved', to.x, to.y); + await mouse('mouseMoved', p.x + 5, p.y); + await mouse('mouseMoved', p.x + 6, p.y); + await t.waitUntil('__gm.tooltip() !== null', { label: 'the sign over a marker' }); + const markerSign = await json('__gm.tooltip()'); + check( + 'a click on a marker starts playing there, a double-click names it, and resting on it says the name', + startAtMarker === 3 * BAR && nameDialog === true && named?.name === 'Boss' && markerSign?.text === 'Boss', + JSON.stringify({ startAtMarker, nameDialog, named, markerSign }), + ); + await click(p.x, p.y, 'right'); + await sleep(150); + const markerRows = await json('__gm.menuRows()'); + await clickText('Delete marker'); + await sleep(100); + const markersLeft = await model(); + check( + "a marker's menu plays from it, renames it and takes it away", + ['Play from here', 'Rename…', 'Delete marker'].every((row) => markerRows?.includes(row)) && + markersLeft.markers.length === 0, + JSON.stringify({ markerRows, markers: markersLeft.markers }), + ); + // 65h. Space plays from the start: the pattern a bar into itself at 2 s a bar, the sound as a // clip round and round, each on the bus of its track. p = await json('__gm.song.rulerAt(0)');