From 19902a9f02429ecf9c1ab992b6a63f5288e6c46f Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sun, 18 Oct 2026 18:27:57 +0200 Subject: [PATCH] Keep groups, comments and silent music in view for the agent (#429) A live test of the music areas built a pattern, a song and a composition that stayed silent: no instrument at first, and later notes outside the C4 to B4 range of a new Notes node. The agent got no hint of either. The same test showed that new nodes left groups behind, that a comment could end up under other nodes, and that an end node like Output stayed left of the devices put in front of it. - get_asset lists under silent what of a pattern, song or composition is not heard. That is a missing instrument, an instrument that sounds no note, or notes outside what its Notes nodes take in, in a song after the transpose. - soundingNotes moves from the pattern tab to src/core/instrument/compile.ts. The tab and the agent use the same rule. - When new nodes go between two that have no room, everything from the fed node on moves right together. This holds for Arrange with a selection too, so no link runs backwards. - A new node linked to a node in a group joins that group, and the group grows in the same undo step. A group that would take in a foreign node stays as it is. - Arrange takes groups and comments along: a group whose nodes all moved the same keeps its size, otherwise it is fitted anew. The rules live in the new src/editor/graph/annotationFollow.ts. - add_comment and fit_group put a comment below its nodes when a node or a group title is in the way above. lint_graph counts covered comments as coveredNotes. - add_notes, get_asset, lint_graph and the relay instructions name the default range and how to mend a covered comment. Tests: - graph-follow (new, 12), agent-edit, agent-music (11) and the other agent, graph, pattern and relay tests green - 10 deliberate breakages, each one red in its own test - e2e 71: 25/25 - Gate: CHECK EXIT 0, 324 files, 5002 tests Co-Authored-By: Claude Opus 5.5 --- src/core/instrument/compile.ts | 9 + src/editor/agent/catalog/graphs.ts | 39 +++-- src/editor/agent/catalog/music.ts | 5 +- src/editor/agent/editPlan.ts | 4 +- src/editor/agent/readTools.ts | 98 ++++++++++- src/editor/agent/tools.json | 4 +- src/editor/graph/annotationDrag.ts | 37 +++- src/editor/graph/annotationFollow.ts | 141 +++++++++++++++ src/editor/graph/arrange.ts | 239 +++++++++++++++++-------- src/editor/graph/graphLayout.ts | 38 +++- src/editor/pattern/PatternEditor.ts | 14 +- tests/editor/agent-edit.test.ts | 46 +++++ tests/editor/agent-music.test.ts | 99 +++++++++++ tests/editor/graph-arrange.test.ts | 4 +- tests/editor/graph-follow.test.ts | 250 +++++++++++++++++++++++++++ tools/mcp/relay.mjs | 15 +- 16 files changed, 923 insertions(+), 119 deletions(-) create mode 100644 src/editor/graph/annotationFollow.ts create mode 100644 tests/editor/graph-follow.test.ts diff --git a/src/core/instrument/compile.ts b/src/core/instrument/compile.ts index 39ea8395..ac74d909 100644 --- a/src/core/instrument/compile.ts +++ b/src/core/instrument/compile.ts @@ -477,6 +477,15 @@ export function playedNotes(graph: Graph): Set { return notes; } +/** Which notes are heard: inside a Notes node's range, and where they lead to something that sounds. */ +export function soundingNotes(asset: InstrumentAsset, sources: InstrumentSources): Set { + return new Set( + [...playedNotes(asset.graph)].filter( + (note) => instrumentBuildFor(asset, note, sources).layers.length > 0, + ), + ); +} + /** The notes a Notes node with single notes has given a name of their own (a drum kit's). */ export function namedNotes(graph: Graph): Map { const names = new Map(); diff --git a/src/editor/agent/catalog/graphs.ts b/src/editor/agent/catalog/graphs.ts index 6d408be4..10ad8892 100644 --- a/src/editor/agent/catalog/graphs.ts +++ b/src/editor/agent/catalog/graphs.ts @@ -46,7 +46,7 @@ import type { PropertyBag, PropertyValue } from '../../../core/model/Properties' import { findGraphAsset, type Project } from '../../../core/model/Project'; import { DECLARED_PORT_KINDS, type DeclaredPortKind } from '../../../core/properties/kinds'; import { coerceValue } from '../../../core/properties/PropertySchema'; -import { groupBoxAround, noteBoxAbove } from '../../graph/annotationDrag'; +import { freeNoteBox, groupBoxAround } from '../../graph/annotationDrag'; import { arrangeCommand, layoutOf } from '../../graph/graphLayout'; import { type PaletteEntry, paletteEntries } from '../../graph/paletteEntries'; import { pickedPortChoices } from '../../graph/portChoices'; @@ -94,7 +94,9 @@ export const GRAPH_COMMANDS: readonly CatalogCommand[] = [ description: "Adds a node to a graph. `node` is an entry of catalog topic 'nodes' for that graph (its id, e.g. " + '"OnKeyHeld", "Move", or "RunBehavior:"). Leave x and y out: when the call has run, the editor ' + - 'arranges the nodes added without a place, flow left to right, and leaves the others where they are.', + 'arranges the nodes added without a place, flow left to right, next to what they link to and into its ' + + 'group. The others stay where they are, but for those right of new nodes put between: they move right ' + + 'to make room.', args: object( { asset: GRAPH, @@ -124,8 +126,8 @@ export const GRAPH_COMMANDS: readonly CatalogCommand[] = [ name: 'arrange_graph', description: 'Arranges nodes of a graph as the Arrange button does: flow left to right, the order that crosses the ' + - 'fewest links, no overlaps. Without nodes it arranges the whole graph – moving what the user placed, so ' + - 'only do that when asked.', + 'fewest links, no overlaps; groups fit round their nodes again and comments go with them. Without nodes ' + + 'it arranges the whole graph – moving what the user placed, so only do that when asked.', args: object({ asset: GRAPH, nodes: ids('The nodes to arrange; the rest stay.') }, ['asset']), build: (args, context) => { const assetId = text(args, 'asset'); @@ -285,7 +287,8 @@ export const GRAPH_COMMANDS: readonly CatalogCommand[] = [ { name: 'add_comment', description: - 'Puts a comment just above nodes, saying what they do; it follows them as the call arranges them.', + 'Puts a comment just above nodes, saying what they do – or below them where a node or a group stands ' + + 'above; it follows them as the call arranges them.', args: object( { asset: GRAPH, nodes: ids('The nodes it is about.'), text: { type: 'string' }, color: COLOR }, ['asset', 'nodes', 'text'], @@ -295,12 +298,13 @@ export const GRAPH_COMMANDS: readonly CatalogCommand[] = [ }, { name: 'fit_group', - description: 'Fits a group (or puts a comment back above) to the nodes given.', + description: + 'Fits a group to the nodes given, or puts a comment back above them (below where that is taken).', args: object({ asset: GRAPH, id: ANNOTATION, nodes: ids('The nodes.') }, ['asset', 'id', 'nodes']), build: (args, context) => { const assetId = text(args, 'asset'); const { kind } = requireAnnotationOf(context.project, assetId, text(args, 'id')); - const box = boxAt(kind, context, assetId, listOf(args, 'nodes')); + const box = boxAt(kind, context, assetId, listOf(args, 'nodes'), text(args, 'id')); return { command: new SetAnnotationBoxCommand(assetId, kind, text(args, 'id'), box) }; }, shows: theNodes, @@ -607,19 +611,30 @@ function colorOf(value: string): number | null { return value === '' ? null : hexColor(value); } -/** Where a group or comment on these nodes goes, by the nodes as they stand now. */ +/** + * Where a group or comment on these nodes goes, by the nodes as they stand now: a comment where it + * lies on nothing else. `self` is the one being fitted again. + */ export function boxAt( kind: AnnotationKind, context: BuildContext, assetId: string, nodeIds: readonly string[], + self?: string, ) { requireNodes(context.project, assetId, nodeIds); const wanted = new Set(nodeIds); - const boxes = (layoutOf(context.project, assetId, context.measure(assetId))?.nodes ?? []).filter((node) => - wanted.has(node.id), - ); - const box = kind === 'group' ? groupBoxAround(boxes) : noteBoxAbove(boxes); + const layout = layoutOf(context.project, assetId, context.measure(assetId)); + const boxes = (layout?.nodes ?? []).filter((node) => wanted.has(node.id)); + const others = (layout?.boxes ?? []).filter((box) => box.id !== self); + const box = + kind === 'group' + ? groupBoxAround(boxes) + : freeNoteBox(boxes, { + nodes: (layout?.nodes ?? []).filter((node) => !wanted.has(node.id)), + groups: others.filter((other) => other.kind === 'group'), + notes: others.filter((other) => other.kind === 'note'), + }); if (!box) throw new CommandError('None of those nodes is drawn'); return box; } diff --git a/src/editor/agent/catalog/music.ts b/src/editor/agent/catalog/music.ts index 5f42229c..af835210 100644 --- a/src/editor/agent/catalog/music.ts +++ b/src/editor/agent/catalog/music.ts @@ -77,7 +77,10 @@ export const MUSIC_COMMANDS: readonly CatalogCommand[] = [ description: `Adds notes to a pattern; with replace, they take the place of all its notes. Time is in ticks, ${TICKS_PER_QUARTER} ` + 'to a quarter note; how many bars long the pattern is, its meter, grid, tempo and instrument are properties. ' + - 'A note has to start before the end of the pattern.', + 'A note has to start before the end of the pattern. A pattern plays nothing until its property instrument ' + + 'names an Instrument asset, here or wherever it plays, and of its notes only those the instrument sounds: ' + + 'inside the range of a Notes node, C4 to B4 in a new one, after the transpose of a song item. ' + + 'get_asset says under silent what is not heard.', args: object( { asset: PATTERN, diff --git a/src/editor/agent/editPlan.ts b/src/editor/agent/editPlan.ts index 1701c721..96d7db2c 100644 --- a/src/editor/agent/editPlan.ts +++ b/src/editor/agent/editPlan.ts @@ -94,7 +94,7 @@ export class AgentEdit implements Command { } // A group made here holds its nodes, which arranging sees through; a comment sits above them. for (const [assetId, nodes] of unplaced) { - const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId)); + const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId), true); if (arranged) run(arranged); } for (const frame of frames) { @@ -147,7 +147,7 @@ function refit(frame: Frame, context: BuildContext): Command | null { const found = holder ? findAnnotation(holder.graph, frame.id) : null; const present = frame.nodes.filter((nodeId) => holder?.graph.nodes.some((node) => node.id === nodeId)); if (!found || present.length === 0) return null; - const box = normalizeBox(boxAt(frame.kind, context, frame.asset, present)); + const box = normalizeBox(boxAt(frame.kind, context, frame.asset, present, frame.id)); const { x, y, w, h } = found.annotation; if (box.x === x && box.y === y && box.w === w && box.h === h) return null; return new SetAnnotationBoxCommand(frame.asset, frame.kind, frame.id, box); diff --git a/src/editor/agent/readTools.ts b/src/editor/agent/readTools.ts index 8a1d836b..d9300d16 100644 --- a/src/editor/agent/readTools.ts +++ b/src/editor/agent/readTools.ts @@ -1,15 +1,22 @@ import { type GraphAnnotation, type GraphNode, resolveDef } from '../../core/graph'; import type { PortDef } from '../../core/graph/NodeRegistry'; import { animationTargets, targetDefs } from '../../core/model/animation'; -import type { AnimationAsset, Asset, PixelArtAsset } from '../../core/model/Asset'; +import type { + AnimationAsset, + Asset, + InstrumentAsset, + PatternAsset, + PixelArtAsset, +} from '../../core/model/Asset'; import { ASSET_TYPE_LABELS, ASSET_TYPES, type AssetType, isPlaceableType } from '../../core/model/AssetTypes'; import { graphLookup } from '../../core/model/graphLookup'; import { isTemplateContent, resolveObjects } from '../../core/model/instance'; -import { findAsset, findGraphAsset, type Project } from '../../core/model/Project'; +import { findAsset, findAssetOf, findGraphAsset, type Project } from '../../core/model/Project'; import { audioSeconds } from '../../core/model/composition'; +import { instrumentSourcesOf, soundingNotes } from '../../core/instrument/compile'; import { noteName } from '../../core/instrument/notes'; -import { patternLength } from '../../core/music/pattern'; -import { songLength } from '../../core/music/song'; +import { patternInstrument, patternLength, patternSequence } from '../../core/music/pattern'; +import { itemNotes, songLength } from '../../core/music/song'; import { TICKS_PER_QUARTER } from '../../core/music/time'; import { CLIP_ROWS } from '../../core/properties/sounds'; import { VIDEO_CLIP_ROWS } from '../../core/properties/videos'; @@ -93,6 +100,8 @@ export function getAsset(project: Project, assetId: string, readCel?: CelLookup) plain.ticks = { perQuarter: TICKS_PER_QUARTER, length: patternLength(asset) }; } if (asset.type === 'song') plain.ticks = { perQuarter: TICKS_PER_QUARTER, length: songLength(asset) }; + const silent = silentParts(project, asset); + if (silent.length > 0) plain.silent = silent; if (asset.type === 'sound' || asset.type === 'video') { plain.clipProperties = (asset.type === 'sound' ? CLIP_ROWS : VIDEO_CLIP_ROWS).map(propertyRow); } @@ -105,6 +114,87 @@ export function getAsset(project: Project, assetId: string, readCel?: CelLookup) return plain; } +/** + * What of a pattern, song or composition is not heard – nothing else says so: all of it for want of + * an instrument, or the notes its instrument does not sound, outside the range of its Notes nodes. + */ +function silentParts(project: Project, asset: Asset): string[] { + const sources = instrumentSourcesOf(project); + const instrumentOf = (pattern: PatternAsset, instead = '') => + findAssetOf(project, instead || patternInstrument(pattern.props), 'instrument'); + const fix = 'set the property instrument of the pattern to an Instrument asset'; + /** The notes of `notes` the instrument does not sound, and what it sounds instead; '' if it sounds all. */ + const unheard = (notes: number[], instrument: InstrumentAsset): string => { + const sounding = soundingNotes(instrument, sources); + const missing = [...new Set(notes)].filter((note) => !sounding.has(note)).sort((a, b) => a - b); + if (missing.length === 0) return ''; + const heard = sounding.size > 0 ? `sounds only ${noteRanges(sounding)}` : 'sounds no note at all'; + const mend = + sounding.size > 0 + ? 'widen "from" and "to" of a Notes node in it' + : 'get_graph on it shows what it lacks, like the sound of a Sample'; + return `${missing.map(noteName).join(', ')} ${missing.length === 1 ? 'is' : 'are'} not heard: ${instrument.name} ${heard}. To hear them, ${mend}`; + }; + if (asset.type === 'pattern') { + const instrument = instrumentOf(asset); + if (!instrument) return [`It has no instrument and plays nothing: ${fix}.`]; + const notes = unheard( + patternSequence(asset).notes.map((n) => n.note), + instrument, + ); + return notes ? [`${notes}, or move the notes.`] : []; + } + if (asset.type === 'song') { + return asset.items.flatMap((item) => { + const pattern = item.kind === 'pattern' ? findAssetOf(project, item.asset, 'pattern') : undefined; + if (!pattern) return []; + const instrument = instrumentOf(pattern, item.instrument); + if (!instrument) + return [ + `Item ${item.id} plays ${pattern.name} without an instrument, so nothing: ${fix}, or the item's.`, + ]; + const notes = unheard( + itemNotes(pattern, item).map((n) => n.note), + instrument, + ); + return notes + ? [ + `Item ${item.id} plays ${pattern.name}, transposed by ${item.transpose}. ${notes}, or change the transpose.`, + ] + : []; + }); + } + if (asset.type === 'composition') { + return asset.items.flatMap((item) => { + if (item.audio?.kind === 'pattern') { + const pattern = findAssetOf(project, item.audio.asset, 'pattern'); + if (!pattern) return []; + return silentParts(project, pattern).length > 0 + ? [`Item ${item.id} plays ${pattern.name}, which is not all heard – get_asset on it says why.`] + : []; + } + const song = item.audio?.kind === 'song' ? findAssetOf(project, item.audio.asset, 'song') : undefined; + return song && silentParts(project, song).length > 0 + ? [`Item ${item.id} plays ${song.name}, where not all is heard – get_asset on it says which.`] + : []; + }); + } + return []; +} + +/** Notes as the runs they make: "C4 to B4, C6". */ +function noteRanges(notes: Set): string { + const sorted = [...notes].sort((a, b) => a - b); + const runs: string[] = []; + for (let i = 0; i < sorted.length;) { + let j = i; + while (j + 1 < sorted.length && sorted[j + 1] === sorted[j]! + 1) j++; + runs.push(i === j ? noteName(sorted[i]!) : `${noteName(sorted[i]!)} to ${noteName(sorted[j]!)}`); + i = j + 1; + } + return runs.join(', '); +} + /** What the rows of an animation may aim at – the root as "", the objects inside it – and what each offers. */ function animationTargetRows(project: Project, animation: AnimationAsset): object[] { const root = findAsset(project, animation.root); diff --git a/src/editor/agent/tools.json b/src/editor/agent/tools.json index 1ab73f8a..0e2061e9 100644 --- a/src/editor/agent/tools.json +++ b/src/editor/agent/tools.json @@ -44,7 +44,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, the rows a clip of a sound or video has (clipProperties), how many seconds a loaded sound lasts. 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, 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. A graph is only counted here; read it with get_graph.", "inputSchema": { "type": "object", "properties": { "asset": { "type": "string", "description": "Id of the asset." } }, @@ -68,7 +68,7 @@ { "name": "lint_graph", "title": "Check how a graph reads", - "description": "Counts what makes a graph hard to read, as the user sees it: nodes on top of each other (overlaps), links that cross (crossings), links running over a node that is not theirs (throughNodes), links running right to left (backwards), and the longest links, with a few ids of each. Aim for 0 overlaps, 0 throughNodes and 0 backwards, and for no more crossings than the links need: two links leaving one node far apart, like exec and dt, cross what comes in between them. To mend: the command arrange_graph, or a second Get or Owner node next to a far-off user instead of one long link.", + "description": "Counts what makes a graph hard to read, as the user sees it: nodes on top of each other (overlaps), links that cross (crossings), links running over a node that is not theirs (throughNodes), links running right to left (backwards), comments a node lies on (coveredNotes), and the longest links, with a few ids of each. Aim for 0 overlaps, 0 throughNodes, 0 backwards and 0 coveredNotes, and for no more crossings than the links need: two links leaving one node far apart, like exec and dt, cross what comes in between them. To mend: the command arrange_graph, or a second Get or Owner node next to a far-off user instead of one long link; a covered comment goes back by its nodes with fit_group.", "inputSchema": { "type": "object", "properties": { "asset": { "type": "string", "description": "Id of the asset holding the graph." } }, diff --git a/src/editor/graph/annotationDrag.ts b/src/editor/graph/annotationDrag.ts index c97c8772..df009833 100644 --- a/src/editor/graph/annotationDrag.ts +++ b/src/editor/graph/annotationDrag.ts @@ -20,7 +20,7 @@ import { type Box, boundsOf, boxesCentredIn, type Point } from '../framing'; /** Height of a group's title bar – the same as a node's, so the two line up in the eye. */ export const ANNOTATION_TITLE_HEIGHT = 24; /** Room a group made around nodes leaves between them and its edge. */ -const GROUP_MARGIN = 20; +export const GROUP_MARGIN = 20; /** A comment as it starts: wide enough for a sentence, tall enough for three lines. */ export const NEW_NOTE_SIZE = { w: 220, h: 96 }; @@ -37,7 +37,7 @@ export function groupBoxAround(boxes: readonly Box[]): AnnotationBox | null { } /** Between a comment and the nodes below it: clear of the title bar of a group around them, with air. */ -const NOTE_GAP = GROUP_MARGIN * 2 + ANNOTATION_TITLE_HEIGHT; +export const NOTE_GAP = GROUP_MARGIN * 2 + ANNOTATION_TITLE_HEIGHT; /** A comment on a set of node boxes: just above them, as wide as they are or a new comment is. */ export function noteBoxAbove(boxes: readonly Box[]): AnnotationBox | null { @@ -52,6 +52,39 @@ export function noteBoxAbove(boxes: readonly Box[]): AnnotationBox | null { }; } +/** What else a graph holds, which a comment should neither hide nor be hidden by. */ +export interface NoteSurroundings { + nodes: readonly Omit[]; + groups: readonly Omit[]; + notes: readonly Omit[]; +} + +/** + * A comment on a set of node boxes where it lies on nothing else: above them, else below them, clear + * of a group around them the same way. Inside another group is free, but not over its title bar. Above + * when neither is free – `lintLayout` counts a comment a node lies on. + */ +export function freeNoteBox(boxes: readonly Box[], around: NoteSurroundings): AnnotationBox | null { + const above = noteBoxAbove(boxes); + const bounds = boundsOf(boxes); + if (!above || !bounds) return null; + const below = { ...above, y: bounds.maxY + GROUP_MARGIN * 2 }; + const meets = (a: Omit, b: Omit) => + a.x < b.x + b.w && b.x < a.x + a.w && a.y < b.y + b.h && b.y < a.y + a.h; + const free = (box: AnnotationBox) => + !around.nodes.some((node) => meets(node, box)) && + !around.notes.some((note) => meets(note, box)) && + around.groups.every( + (group) => + !meets(group, box) || + (box.x >= group.x && + box.x + box.w <= group.x + group.w && + box.y >= group.y + ANNOTATION_TITLE_HEIGHT && + box.y + box.h <= group.y + group.h), + ); + return [above, below].find(free) ?? above; +} + /** A box as the drag found it. */ export interface BoxSeed { id: string; diff --git a/src/editor/graph/annotationFollow.ts b/src/editor/graph/annotationFollow.ts new file mode 100644 index 00000000..03b29ba3 --- /dev/null +++ b/src/editor/graph/annotationFollow.ts @@ -0,0 +1,141 @@ +import type { AnnotationBox } from '../../core/commands/annotationCommands'; +import { type Box, boundsOf, boxesCentredIn } from '../framing'; +import { GROUP_MARGIN, groupBoxAround, NOTE_GAP } from './annotationDrag'; +import type { LayoutBox, LayoutGraph, LayoutNode } from './arrange'; + +/** + * Groups and comments going along when nodes are arranged (#429). A group holds the nodes whose + * centre lies in it and the groups and comments inside it: moved alike, it moves with them, else it + * is fitted round them again. A comment goes with what it stands just above – a group or the top + * row of nodes – else with what it stands just below, else with the nodes of the group it lies in. + * New nodes linked into a group join it, unless it would then take in nodes not its own. + */ + +type Spot = { x: number; y: number }; + +/** How far from a comment what it is about may begin. */ +const REACH = NOTE_GAP + GROUP_MARGIN; +/** How far a comment may lie on what it is about. */ +const SLACK = 8; + +/** + * The groups new nodes are to join: every one round the node that stays which their first link goes + * to – the node they are put next to – while none of them stands in a group yet. + */ +export function adoptingGroups(graph: LayoutGraph, newIds: ReadonlySet): string[] { + const groups = graph.boxes.filter((box) => box.kind === 'group'); + if (graph.nodes.some((node) => newIds.has(node.id) && groups.some((group) => holds(group, node)))) + return []; + const edge = graph.edges.find((e) => newIds.has(e.from.node) !== newIds.has(e.to.node)); + const anchorId = edge && (newIds.has(edge.from.node) ? edge.to.node : edge.from.node); + const anchor = graph.nodes.find((node) => node.id === anchorId); + return anchor ? groups.filter((group) => holds(group, anchor)).map((group) => group.id) : []; +} + +/** Where every group and comment goes once the nodes stand at `spots` (those left out stay). */ +export function followedBoxes( + graph: LayoutGraph, + spots: ReadonlyMap, + joining: { groups: readonly string[]; nodes: readonly string[] } = { groups: [], nodes: [] }, +): Map { + const after = (node: LayoutNode): Box => ({ ...node, ...(spots.get(node.id) ?? {}) }); + const newcomers = graph.nodes.filter((node) => joining.nodes.includes(node.id)); + const done = new Map(); + const busy = new Set(); + const resolve = (box: LayoutBox): AnnotationBox => { + const known = done.get(box.id); + if (known) return known; + if (busy.has(box.id)) return plain(box); + busy.add(box.id); + const now = box.kind === 'group' ? groupAfter(box) : noteAfter(box); + done.set(box.id, now); + return now; + }; + const groupAfter = (group: LayoutBox): AnnotationBox => { + const members = graph.nodes.filter((node) => holds(group, node)); + const inner = graph.boxes.filter( + (box) => box.id !== group.id && area(box) < area(group) && holds(group, box), + ); + const innerNow = inner.map(resolve); + const held = [...members.map(after), ...innerNow]; + if (joining.groups.includes(group.id) && newcomers.length > 0) { + const grown = groupBoxAround([...held, ...newcomers.map(after)].map(boxed)); + const others = graph.nodes.filter((node) => !members.includes(node) && !newcomers.includes(node)); + if (grown && boxesCentredIn(others.map(after), grown).length === 0) return grown; + } + const shift = sameShift([ + ...members.map((node) => delta(node, after(node))), + ...inner.map((box, i) => delta(box, innerNow[i]!)), + ]); + if (shift) return moved(group, shift); + return groupBoxAround(held.map(boxed)) ?? plain(group); + }; + const noteAfter = (note: LayoutBox): AnnotationBox => { + const anchor = anchorOf(graph, note); + if (!anchor) return plain(note); + if ('box' in anchor) { + const now = resolve(anchor.box); + const edge = (b: AnnotationBox) => (anchor.below ? b.y : b.y + b.h); + return moved(note, { x: now.x - anchor.box.x, y: edge(now) - edge(anchor.box) }); + } + const before = boundsOf(anchor.nodes)!; + const now = boundsOf(anchor.nodes.map(after))!; + return moved(note, { + x: now.minX - before.minX, + y: anchor.below ? now.minY - before.minY : now.maxY - before.maxY, + }); + }; + for (const box of graph.boxes) resolve(box); + return done; +} + +type Anchor = { box: LayoutBox; below: boolean } | { nodes: LayoutNode[]; below: boolean }; + +/** What a comment is about, by where it stands now. */ +function anchorOf(graph: LayoutGraph, note: LayoutBox): Anchor | null { + const across = (thing: Omit) => thing.x < note.x + note.w && note.x < thing.x + thing.w; + const groups = graph.boxes.filter((box) => box.kind === 'group' && box.id !== note.id && across(box)); + const nodes = graph.nodes.filter(across); + const bottom = note.y + note.h; + const below = (thing: Omit) => thing.y >= bottom - SLACK && thing.y <= bottom + REACH; + const above = (thing: Omit) => + thing.y + thing.h <= note.y + SLACK && thing.y + thing.h >= note.y - REACH; + for (const [side, near, key] of [ + [true, below, (thing: Omit) => thing.y], + [false, above, (thing: Omit) => -(thing.y + thing.h)], + ] as const) { + const things = [...groups, ...nodes].filter(near).sort((a, b) => key(a) - key(b)); + const first = things[0]; + if (!first) continue; + if ('kind' in first) return { box: first, below: side }; + return { nodes: nodes.filter(near), below: side }; + } + const around = graph.boxes + .filter((box) => box.kind === 'group' && holds(box, note)) + .sort((a, b) => area(a) - area(b))[0]; + const members = around ? graph.nodes.filter((node) => holds(around, node)) : []; + return members.length > 0 ? { nodes: members, below: true } : null; +} + +/** The centre rule of a group, as a drag uses it. */ +function holds(group: Omit, thing: Omit): boolean { + const cx = thing.x + thing.w / 2; + const cy = thing.y + thing.h / 2; + return cx >= group.x && cx <= group.x + group.w && cy >= group.y && cy <= group.y + group.h; +} + +const area = (box: Omit) => box.w * box.h; +const plain = ({ x, y, w, h }: AnnotationBox): AnnotationBox => ({ x, y, w, h }); +const boxed = (box: AnnotationBox, i: number): Box => ({ id: String(i), ...plain(box) }); +const delta = (from: Spot, to: Spot): Spot => ({ x: to.x - from.x, y: to.y - from.y }); +const moved = (box: AnnotationBox, by: Spot): AnnotationBox => ({ + ...plain(box), + x: box.x + by.x, + y: box.y + by.y, +}); + +/** The one shift all of these made, or null when they moved apart; none for nothing at all. */ +function sameShift(shifts: readonly Spot[]): Spot | null { + const first = shifts[0] ?? { x: 0, y: 0 }; + return shifts.every((s) => s.x === first.x && s.y === first.y) ? first : null; +} diff --git a/src/editor/graph/arrange.ts b/src/editor/graph/arrange.ts index e02d3fb7..d8e3bb18 100644 --- a/src/editor/graph/arrange.ts +++ b/src/editor/graph/arrange.ts @@ -27,11 +27,23 @@ export interface LayoutEdge { exec: boolean; } +/** A group or a comment as drawn. */ +export interface LayoutBox extends Box { + kind: 'group' | 'note'; +} + export interface LayoutGraph { nodes: LayoutNode[]; edges: LayoutEdge[]; /** The groups and notes, which nothing arranged may be put over. */ - boxes: Box[]; + boxes: LayoutBox[]; +} + +export interface ArrangeOptions { + /** Groups the arranged nodes may go into: those that take them in (see `annotationFollow`). */ + open?: ReadonlySet; + /** The nodes are new and stand nowhere yet: next to what they link to is tried before where they are. */ + fresh?: boolean; } export const COLUMN_GAP = 72; @@ -64,10 +76,14 @@ interface Hop { weight: number; } -/** The new top-left corners of the movable nodes (all of them, by default). */ +/** + * The new top-left corners of the movable nodes (all of them, by default), and of the nodes that + * stay but move right to make room for them. + */ export function arrange( graph: LayoutGraph, movable?: ReadonlySet, + options: ArrangeOptions = {}, ): Map { const moving = graph.nodes.filter((node) => !movable || movable.has(node.id)); const placed = new Map(); @@ -83,7 +99,7 @@ export function arrange( for (const [id, spot] of spots) placed.set(id, { x: spot.x, y: spot.y + top }); top += height + PART_GAP; } - return settle(graph, moving, placed); + return settle(graph, moving, placed, options); } /** Groups of nodes that links join, in the order their first node has in the graph. */ @@ -372,17 +388,25 @@ function keepApart(wanted: readonly { y: number; weight: number }[], heights: re return ys.map(Math.round); } +/** Staying nodes and boxes from `cut` rightwards move `by` to the right, to make room. */ +interface Push { + cut: number; + by: number; +} + /** * Moves the arrangement as one to where the nodes were. Where that would land on a node that stays, * on a group or note, or turn a link to what stays backwards, it goes next to the node it links to – * a value it hands on left of the node taking it, one it takes right of the node giving it, where its - * links cross the fewest others and else as near the port's height as it fits – and else right of - * everything. + * links cross the fewest others and else as near the port's height as it fits. New nodes try that + * first. Nodes put between ones that stay, with no room there, push what they feed and everything + * right of it further right; else they go right of everything. */ function settle( graph: LayoutGraph, moving: readonly LayoutNode[], placed: Map, + { open, fresh }: ArrangeOptions, ): Map { const ids = new Set(moving.map((node) => node.id)); const staying = graph.nodes.filter((node) => !ids.has(node.id)); @@ -391,89 +415,149 @@ function settle( node.x + node.w / 2 < box.x + box.w && node.y + node.h / 2 > box.y && node.y + node.h / 2 < box.y + box.h; - // A group that held what is arranged would only stand in its way. - const obstacles: Omit[] = [ - ...staying, - ...graph.boxes.filter((box) => !moving.some((node) => inside(box, node))), - ]; - const at = (dx: number, dy: number) => - moving.map((node) => { - const spot = placed.get(node.id)!; - return { x: spot.x + dx, y: spot.y + dy, w: node.w, h: node.h }; - }); - const hits = (boxes: readonly Omit[], room = 0) => - boxes.some((box) => obstacles.some((o) => overlap(grown(box, room), o))); - const across = graph.edges.filter((edge) => ids.has(edge.from.node) !== ids.has(edge.to.node)); + // A group that held what is arranged, or takes it in, would only stand in its way. + const boxes = graph.boxes.filter((box) => !open?.has(box.id) && !moving.some((node) => inside(box, node))); const byId = new Map(graph.nodes.map((node) => [node.id, node])); - const boxAt = (id: string, dx: number, dy: number) => { - const node = byId.get(id)!; - const spot = placed.get(id); - return spot ? { x: spot.x + dx, y: spot.y + dy, w: node.w } : node; - }; - const backwards = (dx: number, dy: number) => - across.some((edge) => { - const from = boxAt(edge.from.node, dx, dy); - return from.x + from.w > boxAt(edge.to.node, dx, dy).x; - }); + const across = graph.edges.filter((edge) => ids.has(edge.from.node) !== ids.has(edge.to.node)); const linked = graph.edges.filter((edge) => ids.has(edge.from.node) || ids.has(edge.to.node)); - const still = curvesOf( - graph.edges.filter((edge) => !linked.includes(edge)), - byId, - ); - const crossingsAt = (dx: number, dy: number) => { - const moved = new Map(byId); - for (const node of moving) { - const spot = placed.get(node.id)!; - moved.set(node.id, { ...node, x: spot.x + dx, y: spot.y + dy }); - } - const mine = curvesOf(linked, moved); - let count = 0; - mine.forEach((a, i) => { - for (const b of [...still, ...mine.slice(i + 1)]) if (curvesCross(a, b)) count += 1; - }); - return count; - }; - const left = Math.min(...moving.map((node) => node.x)); - const top = Math.min(...moving.map((node) => node.y)); - let shift = { x: left, y: top }; - if (hits(at(left, top)) || backwards(left, top)) { - shift = besideLink( - across, - ids, - byId, - placed, - (dx, dy) => !hits(at(dx, dy), ROW_GAP) && !backwards(dx, dy), - crossingsAt, - ) ?? { - x: Math.max(...obstacles.map((o) => o.x + o.w)) + BESIDE_GAP, - y: Math.min(...(across.length > 0 ? linkedStaying(across, ids, byId) : obstacles).map((o) => o.y)), + const view = (push: Push | null) => { + const pushed = >(box: T): T => + push && box.x >= push.cut ? { ...box, x: box.x + push.by } : box; + const now = new Map(graph.nodes.map((node) => [node.id, ids.has(node.id) ? node : pushed(node)])); + const obstacles = [...staying.map((node) => now.get(node.id)!), ...boxes.map(pushed)]; + const still = curvesOf( + graph.edges.filter((edge) => !linked.includes(edge)), + now, + ); + const shifted = (dx: number, dy: number) => { + const moved = new Map(now); + for (const node of moving) { + const spot = placed.get(node.id)!; + moved.set(node.id, { ...node, x: spot.x + dx, y: spot.y + dy }); + } + return moved; }; + const clear = (dx: number, dy: number, room: number) => { + const moved = shifted(dx, dy); + const hits = moving.some((node) => obstacles.some((o) => overlap(grown(moved.get(node.id)!, room), o))); + const backwards = across.some((edge) => { + const from = moved.get(edge.from.node)!; + return from.x + from.w > moved.get(edge.to.node)!.x; + }); + return !hits && !backwards; + }; + return { + obstacles, + clear, + fits: (dx: number, dy: number) => clear(dx, dy, ROW_GAP), + crossingsAt: (dx: number, dy: number) => { + const mine = curvesOf(linked, shifted(dx, dy)); + let count = 0; + mine.forEach((a, i) => { + for (const b of [...still, ...mine.slice(i + 1)]) if (curvesCross(a, b)) count += 1; + }); + return count; + }, + }; + }; + const plain = view(null); + const here = { + x: Math.min(...moving.map((node) => node.x)), + y: Math.min(...moving.map((node) => node.y)), + }; + const staysHere = plain.clear(here.x, here.y, 0); + let push: Push | null = null; + let shift = + !fresh && staysHere ? here : besideLink(across, ids, byId, placed, plain.fits, plain.crossingsAt); + if (!shift && staysHere) shift = here; + if (!shift) { + const room = makeRoom(across, ids, byId, placed, view); + if (room) ({ shift, push } = room); } + shift ??= { + x: Math.max(...plain.obstacles.map((o) => o.x + o.w)) + BESIDE_GAP, + y: Math.min(...(across.length > 0 ? linkedStaying(across, ids, byId) : plain.obstacles).map((o) => o.y)), + }; for (const spot of placed.values()) { spot.x = Math.round(spot.x + shift.x); spot.y = Math.round(spot.y + shift.y); } + if (push) + for (const node of staying) + if (node.x >= push.cut) placed.set(node.id, { x: Math.round(node.x + push.by), y: node.y }); return placed; } /** How far up or down from the linked node's height a place next to it is looked for, in rows. */ const BESIDE_ROWS = 40; +type Fits = (dx: number, dy: number) => boolean; +type CrossingsAt = (dx: number, dy: number) => number; + /** - * The shift that puts the arrangement next to the node its first link to what stays goes to: of the - * places where `fits`, the one whose links cross the fewest others, and of those the nearest to the - * two ports of that link lying level; null when none fits. + * The shift that puts the arrangement next to the node its first link to what stays goes to; null + * when none fits. */ function besideLink( across: readonly LayoutEdge[], ids: ReadonlySet, byId: ReadonlyMap, placed: ReadonlyMap, - fits: (dx: number, dy: number) => boolean, - crossingsAt: (dx: number, dy: number) => number, + fits: Fits, + crossingsAt: CrossingsAt, ): { x: number; y: number } | null { const edge = across[0]; if (!edge) return null; + const handsOn = ids.has(edge.from.node); + const other = byId.get(handsOn ? edge.to.node : edge.from.node)!; + const spots = spotsOf(placed, byId); + const x = handsOn + ? other.x - COLUMN_GAP - Math.max(...spots.map((spot) => spot.x + spot.w)) + : other.x + other.w + COLUMN_GAP - Math.min(...spots.map((spot) => spot.x)); + return nearestRow(x, levelWith(edge, ids, byId, placed), fits, crossingsAt); +} + +/** + * Room for an arrangement between the nodes that feed it and those it feeds, when there is none: + * right of what feeds it, with what it feeds – and every node or box from there on – pushed right + * far enough. Null when it is not put between them, or one that feeds it stands that far right. + */ +function makeRoom( + across: readonly LayoutEdge[], + ids: ReadonlySet, + byId: ReadonlyMap, + placed: ReadonlyMap, + view: (push: Push) => { fits: Fits; crossingsAt: CrossingsAt }, +): { shift: { x: number; y: number }; push: Push } | null { + const into = across.filter((edge) => ids.has(edge.to.node)); + const sources = into.map((edge) => byId.get(edge.from.node)!); + const sinks = across.filter((edge) => ids.has(edge.from.node)).map((edge) => byId.get(edge.to.node)!); + if (sources.length === 0 || sinks.length === 0) return null; + const cut = Math.min(...sinks.map((node) => node.x)); + if (sources.some((node) => node.x >= cut)) return null; + const spots = spotsOf(placed, byId); + const x = + Math.max(...sources.map((node) => node.x + node.w)) + COLUMN_GAP - Math.min(...spots.map((s) => s.x)); + const push = { cut, by: Math.max(0, x + Math.max(...spots.map((s) => s.x + s.w)) + COLUMN_GAP - cut) }; + const { fits, crossingsAt } = view(push); + const shift = nearestRow(x, levelWith(into[0]!, ids, byId, placed), fits, crossingsAt); + return shift && { shift, push }; +} + +function spotsOf( + placed: ReadonlyMap, + byId: ReadonlyMap, +) { + return [...placed].map(([id, spot]) => ({ ...spot, w: byId.get(id)!.w })); +} + +/** The shift down that lays the two ports of a link between what moves and what stays level. */ +function levelWith( + edge: LayoutEdge, + ids: ReadonlySet, + byId: ReadonlyMap, + placed: ReadonlyMap, +): number { const handsOn = ids.has(edge.from.node); const mineId = handsOn ? edge.from.node : edge.to.node; const mine = placed.get(mineId)!; @@ -481,11 +565,16 @@ function besideLink( const other = byId.get(handsOn ? edge.to.node : edge.from.node)!; const minePort = handsOn ? mineNode.outputs[edge.from.port] : mineNode.inputs[edge.to.port]; const otherPort = handsOn ? other.inputs[edge.to.port] : other.outputs[edge.from.port]; - const spots = [...placed].map(([id, spot]) => ({ ...spot, w: byId.get(id)!.w })); - const x = handsOn - ? other.x - COLUMN_GAP - Math.max(...spots.map((spot) => spot.x + spot.w)) - : other.x + other.w + COLUMN_GAP - Math.min(...spots.map((spot) => spot.x)); - const y = other.y + (otherPort ?? other.h / 2) - mine.y - (minePort ?? mineNode.h / 2); + return other.y + (otherPort ?? other.h / 2) - mine.y - (minePort ?? mineNode.h / 2); +} + +/** Of the rows round `y` where it fits, the one whose links cross the fewest others, and of those the nearest. */ +function nearestRow( + x: number, + y: number, + fits: Fits, + crossingsAt: CrossingsAt, +): { x: number; y: number } | null { let best: { x: number; y: number; crossings: number } | null = null; for (let row = 0; row <= BESIDE_ROWS && best?.crossings !== 0; row++) { for (const dy of row === 0 ? [0] : [-row * ROW_GAP, row * ROW_GAP]) { @@ -525,9 +614,11 @@ export interface GraphLint { throughNodes: number; /** Links that run right to left. */ backwards: number; + /** Comments a node lies on, which hides what they say. */ + coveredNotes: number; longest: { edge: string; length: number }[]; /** A few of each kind, with the ids, so they can be found. */ - examples: { kind: 'overlap' | 'crossing' | 'through' | 'backwards'; ids: string[] }[]; + examples: { kind: 'overlap' | 'crossing' | 'through' | 'backwards' | 'covered'; ids: string[] }[]; } const CURVE_STEPS = 16; @@ -583,6 +674,13 @@ export function lintLayout(graph: LayoutGraph): GraphLint { example('through', [edge.id, node.id]); } } + let coveredNotes = 0; + for (const note of graph.boxes.filter((box) => box.kind === 'note')) { + const on = graph.nodes.find((node) => overlap(node, note)); + if (!on) continue; + coveredNotes += 1; + example('covered', [note.id, on.id]); + } const longest = curves .map(({ edge, start, end }) => ({ edge: edge.id, @@ -597,6 +695,7 @@ export function lintLayout(graph: LayoutGraph): GraphLint { crossings, throughNodes, backwards, + coveredNotes, longest, examples, }; diff --git a/src/editor/graph/graphLayout.ts b/src/editor/graph/graphLayout.ts index 5b861f01..8bf96189 100644 --- a/src/editor/graph/graphLayout.ts +++ b/src/editor/graph/graphLayout.ts @@ -1,10 +1,19 @@ +import { normalizeBox, SetAnnotationBoxCommand } from '../../core/commands/annotationCommands'; import { oneStep } from '../../core/commands/CompoundCommand'; import type { Command } from '../../core/commands/Command'; import { MoveNodeCommand } from '../../core/commands/graphCommands'; -import { drawnDef, type Graph, type GraphNode, type NodeDef, resolveDef } from '../../core/graph'; +import { + ANNOTATION_KINDS, + annotationsOf, + drawnDef, + type Graph, + type GraphNode, + type NodeDef, + resolveDef, +} from '../../core/graph'; import { graphLookup } from '../../core/model/graphLookup'; import { findGraphAsset, type Project } from '../../core/model/Project'; -import { annotationBoxes } from './annotationDrag'; +import { adoptingGroups, followedBoxes } from './annotationFollow'; import { arrange, type LayoutGraph } from './arrange'; import { estimatedShape, type NodeShape } from './nodeBox'; @@ -37,27 +46,42 @@ export function layoutOf( to: edge.to, exec: defs.get(edge.from.node)!.outputs.find((p) => p.key === edge.from.port)?.type === 'exec', })); - return { nodes, edges, boxes: annotationBoxes(graph) }; + const boxes = ANNOTATION_KINDS.flatMap((kind) => + annotationsOf(graph, kind).map(({ id, x, y, w, h }) => ({ id, kind, x, y, w, h })), + ); + return { nodes, edges, boxes }; } /** - * The moves that arrange these nodes of a graph (all of them without a list), as one undo step; - * null when none of them has to move. + * The moves that arrange these nodes of a graph (all of them without a list), with the groups and + * comments going along, as one undo step; null when nothing has to move. `fresh` nodes are new ones + * the agent gave no place: put next to what they link to, they join its group. */ export function arrangeCommand( project: Project, assetId: string, nodeIds: readonly string[] | null, measure?: MeasureNode, + fresh = false, ): Command | null { const layout = layoutOf(project, assetId, measure); if (!layout) return null; - const spots = arrange(layout, nodeIds ? new Set(nodeIds) : undefined); + const movable = nodeIds ? new Set(nodeIds) : undefined; + const joining = fresh && movable ? adoptingGroups(layout, movable) : []; + const spots = arrange(layout, movable, { open: new Set(joining), fresh }); + const boxes = followedBoxes(layout, spots, { groups: joining, nodes: nodeIds ?? [] }); const moves = layout.nodes.flatMap((node) => { const spot = spots.get(node.id); return spot && (spot.x !== node.x || spot.y !== node.y) ? [new MoveNodeCommand(assetId, node.id, spot.x, spot.y)] : []; }); - return moves.length > 0 ? oneStep(moves.length === 1 ? 'Arrange node' : 'Arrange nodes', moves) : null; + const refits = layout.boxes.flatMap((box) => { + const now = normalizeBox(boxes.get(box.id) ?? box); + return now.x !== box.x || now.y !== box.y || now.w !== box.w || now.h !== box.h + ? [new SetAnnotationBoxCommand(assetId, box.kind, box.id, now)] + : []; + }); + const steps = [...moves, ...refits]; + return steps.length > 0 ? oneStep(moves.length === 1 ? 'Arrange node' : 'Arrange nodes', steps) : null; } diff --git a/src/editor/pattern/PatternEditor.ts b/src/editor/pattern/PatternEditor.ts index 36f0cab8..d31cf047 100644 --- a/src/editor/pattern/PatternEditor.ts +++ b/src/editor/pattern/PatternEditor.ts @@ -1,10 +1,5 @@ import { SetAssetPropertyCommand } from '../../core/commands/assetCommands'; -import { - instrumentBuildFor, - instrumentSourcesOf, - namedNotes, - playedNotes, -} from '../../core/instrument/compile'; +import { instrumentSourcesOf, namedNotes, soundingNotes } from '../../core/instrument/compile'; import { ghostNotes, placeOf } from '../../core/music/ghosts'; import { GRID_OPTIONS, @@ -263,11 +258,8 @@ export class PatternEditor extends AssetTabEditor { ? findAssetOf(this.state.project, patternInstrument(asset.props), 'instrument') : undefined; if (instrument) { - const sources = instrumentSourcesOf(this.state.project); - const sounding = [...playedNotes(instrument.graph)].filter( - (note) => instrumentBuildFor(instrument, note, sources).layers.length > 0, - ); - this.canvas.setInstrument(new Set(sounding), namedNotes(instrument.graph)); + const sounding = soundingNotes(instrument, instrumentSourcesOf(this.state.project)); + this.canvas.setInstrument(sounding, namedNotes(instrument.graph)); } else this.canvas.setInstrument(new Set(), new Map()); this.syncGhosts(asset); this.bakeButton.sync(); diff --git a/tests/editor/agent-edit.test.ts b/tests/editor/agent-edit.test.ts index ca80e141..d2f9eb04 100644 --- a/tests/editor/agent-edit.test.ts +++ b/tests/editor/agent-edit.test.ts @@ -195,6 +195,52 @@ describe('run_commands', () => { expect(text(unknown)).toContain('Step 1 (arrange_graph): Jump has no node nope'); }); + it('puts a new node linked into a group into it, the group growing in the same undo step', async () => { + const fixture = makeSpriteScene(); + const behavior = makeBehavior(fixture, 'Jump'); + const { host } = hostFor(fixture); + const link = (from: string, fromPort: string, to: string, toPort: string) => ({ + command: 'connect', + args: { asset: behavior.id, from: { node: from, port: fromPort }, to: { node: to, port: toPort } }, + }); + await runTool( + 'run_commands', + { + steps: [ + { command: 'add_node', args: { asset: behavior.id, node: 'OnStart' }, as: 'start' }, + { command: 'add_node', args: { asset: behavior.id, node: 'Move' }, as: 'move' }, + link('$start', 'exec', '$move', 'exec'), + { command: 'add_group', args: { asset: behavior.id, nodes: ['$start', '$move'], title: 'Jump' } }, + ], + }, + host, + ); + const [start, move] = behavior.graph.nodes; + const before = { ...behavior.graph.groups![0]! }; + const result = await runTool( + 'run_commands', + { + steps: [ + { command: 'add_node', args: { asset: behavior.id, node: 'Owner' }, as: 'owner' }, + link('$owner', 'object', move!.id, 'object'), + ], + }, + host, + ); + expect(result.isError).toBeFalsy(); + const owner = behavior.graph.nodes[2]!; + const group = behavior.graph.groups![0]!; + expect(owner.x + NODE_WIDTH + COLUMN_GAP).toBe(move!.x); + expect(owner.x + NODE_WIDTH / 2).toBeGreaterThan(group.x); + expect(owner.y).toBeGreaterThan(group.y); + expect(owner.y).toBeLessThan(group.y + group.h); + // Under the start, where the group had room already. + expect(group.h).toBeGreaterThanOrEqual(before.h); + expect([start!.x, start!.y]).toEqual([behavior.graph.nodes[0]!.x, behavior.graph.nodes[0]!.y]); + await runTool('undo', {}, host); + expect(behavior.graph.groups![0]).toEqual(before); + }); + it('changes nothing when a step fails, and names the step and why', async () => { const fixture = makeSpriteScene(); const { host } = hostFor(fixture); diff --git a/tests/editor/agent-music.test.ts b/tests/editor/agent-music.test.ts index 50be90e7..59cde8f1 100644 --- a/tests/editor/agent-music.test.ts +++ b/tests/editor/agent-music.test.ts @@ -423,3 +423,102 @@ describe('compositions', () => { expect(last.text).toContain('keeps at least one track'); }); }); + +describe('silent music', () => { + it('get_asset says what plays nothing for want of an instrument, until it has one', async () => { + const fixture = makeSpriteScene(); + const { Riff, Theme, Cut, Lead } = await made(fixture, { + Riff: 'pattern', + Theme: 'song', + Cut: 'composition', + Lead: 'instrument', + }); + const track = byId(fixture, Theme).tracks[0]!.id; + const lane = byId(fixture, Cut).tracks[0]!.id; + const built = await run(fixture, [ + { command: 'add_song_item', args: { asset: Theme, track, play: Riff, tick: 0 }, as: 'item' }, + { command: 'add_composition_item', args: { asset: Cut, track: lane, place: Riff, start: 0 } }, + ]); + expect(built.result.isError).toBeUndefined(); + const item = byId(fixture, Theme).items[0]!.id; + expect((await read(fixture, Riff)).silent).toEqual([ + 'It has no instrument and plays nothing: set the property instrument of the pattern to an Instrument asset.', + ]); + expect((await read(fixture, Theme)).silent).toEqual([expect.stringContaining(`Item ${item} plays Riff`)]); + expect((await read(fixture, Cut)).silent).toEqual([expect.stringContaining('which is not all heard')]); + + // The item's own instrument is enough for the song; the pattern's for all of them. + await run(fixture, [{ command: 'change_song_item', args: { asset: Theme, item, instrument: Lead } }]); + expect((await read(fixture, Theme)).silent).toBeUndefined(); + expect((await read(fixture, Riff)).silent).toHaveLength(1); + await run(fixture, [ + { command: 'set_asset_property', args: { asset: Riff, key: 'instrument', value: Lead } }, + ]); + for (const asset of [Riff, Theme, Cut]) expect((await read(fixture, asset)).silent).toBeUndefined(); + }); + + it('names the notes outside what the instrument sounds, after the transpose, until its range takes them in', async () => { + const fixture = makeSpriteScene(); + const { Riff, Theme, Cut, Lead } = await made(fixture, { + Riff: 'pattern', + Theme: 'song', + Cut: 'composition', + Lead: 'instrument', + }); + const node = (type: string) => + ( + fixture.project.assets.find((a) => a.id === Lead) as { + graph: { nodes: { id: string; type: string }[] }; + } + ).graph.nodes.find((n) => n.type === type)!.id; + const track = byId(fixture, Theme).tracks[0]!.id; + const lane = byId(fixture, Cut).tracks[0]!.id; + const built = await run(fixture, [ + { command: 'set_asset_property', args: { asset: Riff, key: 'instrument', value: Lead } }, + { + command: 'add_notes', + args: { + asset: Riff, + notes: [ + { tick: 0, length: 480, note: 'C4' }, + { tick: 480, length: 480, note: 'C5' }, + ], + }, + }, + { command: 'add_song_item', args: { asset: Theme, track, play: Riff, tick: 0, transpose: -12 } }, + { command: 'add_composition_item', args: { asset: Cut, track: lane, place: Riff, start: 0 } }, + ]); + expect(built.result.isError).toBeUndefined(); + const item = byId(fixture, Theme).items[0]!.id; + + // A new instrument's Sample waits for its sound: no note is heard, and get_graph says why. + expect((await read(fixture, Riff)).silent).toEqual([ + 'C4, C5 are not heard: Lead sounds no note at all. To hear them, get_graph on it shows what it lacks, like the sound of a Sample, or move the notes.', + ]); + + const sound = addSound(fixture); + await run(fixture, [ + { + command: 'set_node_param', + args: { asset: Lead, node: node('InstrumentSample'), key: 'sound', value: sound }, + }, + ]); + expect((await read(fixture, Riff)).silent).toEqual([ + 'C5 is not heard: Lead sounds only C4 to B4. To hear them, widen "from" and "to" of a Notes node in it, or move the notes.', + ]); + // An octave down, C5 comes in and C4 drops out: the song goes by the notes as they sound. + expect((await read(fixture, Theme)).silent).toEqual([ + `Item ${item} plays Riff, transposed by -12. C3 is not heard: Lead sounds only C4 to B4. To hear them, widen "from" and "to" of a Notes node in it, or change the transpose.`, + ]); + expect((await read(fixture, Cut)).silent).toEqual([ + expect.stringContaining('Riff, which is not all heard'), + ]); + + const notes = node('InstrumentNotes'); + await run(fixture, [ + { command: 'set_node_param', args: { asset: Lead, node: notes, key: 'from', value: '48' } }, + { command: 'set_node_param', args: { asset: Lead, node: notes, key: 'to', value: '72' } }, + ]); + for (const asset of [Riff, Theme, Cut]) expect((await read(fixture, asset)).silent).toBeUndefined(); + }); +}); diff --git a/tests/editor/graph-arrange.test.ts b/tests/editor/graph-arrange.test.ts index e8cf8cc1..57f06314 100644 --- a/tests/editor/graph-arrange.test.ts +++ b/tests/editor/graph-arrange.test.ts @@ -141,7 +141,7 @@ describe('arrange', () => { node('new-b', ['exec'], [], 30, 20), ], edges: [edge('kept', 'exec', 'new-a', 'exec'), edge('new-a', 'exec', 'new-b', 'exec')], - boxes: [{ id: 'group', x: 200, y: -40, w: 300, h: 200 }], + boxes: [{ id: 'group', kind: 'group', x: 200, y: -40, w: 300, h: 200 }], }; const result = placed(graph, new Set(['new-a', 'new-b'])); const kept = result.nodes.find((n) => n.id === 'kept')!; @@ -226,7 +226,7 @@ describe('arrange', () => { const graph: LayoutGraph = { nodes: [node('a', [], ['exec'], 220, 0), node('b', ['exec'], [], 30, 20)], edges: [edge('a', 'exec', 'b', 'exec')], - boxes: [{ id: 'group', x: 0, y: -40, w: 700, h: 300 }], + boxes: [{ id: 'group', kind: 'group', x: 0, y: -40, w: 700, h: 300 }], }; const result = placed(graph); expect(Math.min(...result.nodes.map((n) => n.x))).toBe(30); diff --git a/tests/editor/graph-follow.test.ts b/tests/editor/graph-follow.test.ts new file mode 100644 index 00000000..f58ffa95 --- /dev/null +++ b/tests/editor/graph-follow.test.ts @@ -0,0 +1,250 @@ +/** + * Groups and comments going along when nodes are arranged (#429), new nodes joining the group of + * what they link to, room made for nodes put between, and comments kept clear of other nodes. + */ +import { describe, expect, it } from 'vitest'; +import { freeNoteBox, groupBoxAround, noteBoxAbove } from '../../src/editor/graph/annotationDrag'; +import { adoptingGroups, followedBoxes } from '../../src/editor/graph/annotationFollow'; +import { + arrange, + COLUMN_GAP, + type LayoutBox, + type LayoutEdge, + type LayoutGraph, + type LayoutNode, + lintLayout, +} from '../../src/editor/graph/arrange'; + +function node(id: string, inputs: string[], outputs: string[], x = 0, y = 0): LayoutNode { + const rows = (keys: string[]) => Object.fromEntries(keys.map((key, i) => [key, 30 + i * 20 + 10])); + return { + id, + x, + y, + w: 176, + h: 36 + Math.max(inputs.length, outputs.length) * 20, + inputs: rows(inputs), + outputs: rows(outputs), + }; +} + +let edgeCount = 0; +function edge(from: string, fromPort: string, to: string, toPort: string): LayoutEdge { + edgeCount += 1; + return { + id: `e${edgeCount}`, + from: { node: from, port: fromPort }, + to: { node: to, port: toPort }, + exec: fromPort === 'exec', + }; +} + +/** Arranges as the editor does: the nodes, then the groups and comments after them. */ +function placed(graph: LayoutGraph, movable?: ReadonlySet, fresh = false): LayoutGraph { + const joining = fresh && movable ? adoptingGroups(graph, movable) : []; + const spots = arrange(graph, movable, { open: new Set(joining), fresh }); + const boxes = followedBoxes(graph, spots, { groups: joining, nodes: [...(movable ?? [])] }); + return { + ...graph, + nodes: graph.nodes.map((n) => ({ ...n, ...(spots.get(n.id) ?? {}) })), + boxes: graph.boxes.map((b) => ({ ...b, ...boxes.get(b.id)! })), + }; +} + +const at = (graph: LayoutGraph, id: string) => graph.nodes.find((n) => n.id === id)!; +const box = (graph: LayoutGraph, id: string) => graph.boxes.find((b) => b.id === id)!; +const centredIn = (b: LayoutBox, n: LayoutNode) => + n.x + n.w / 2 >= b.x && n.x + n.w / 2 <= b.x + b.w && n.y + n.h / 2 >= b.y && n.y + n.h / 2 <= b.y + b.h; + +/** The walking graph of #429 in its group, with the comment above it, as the user has it. */ +function walking(): LayoutGraph { + const nodes = [ + node('update', [], ['exec', 'dt'], 0, 0), + node('owner', [], ['object'], 0, 100), + node('axis', [], ['x', 'y', 'active'], 0, 180), + node('move', ['exec', 'object', 'dx', 'dy', 'scale', 'dt'], ['exec'], 248, 0), + ]; + const group = { id: 'group', kind: 'group' as const, ...groupBoxAround(nodes)! }; + const note = { id: 'note', kind: 'note' as const, ...noteBoxAbove(nodes)! }; + return { + nodes, + edges: [ + edge('update', 'exec', 'move', 'exec'), + edge('update', 'dt', 'move', 'dt'), + edge('owner', 'object', 'move', 'object'), + edge('axis', 'x', 'move', 'dx'), + edge('axis', 'y', 'move', 'dy'), + ], + boxes: [group, note], + }; +} + +describe('new nodes and groups', () => { + it('puts a new node feeding one in a group into that group, which grows round it', () => { + const graph = walking(); + graph.nodes.push(node('vars', [], ['speed'], 508, 0)); + graph.edges.push(edge('vars', 'speed', 'move', 'scale')); + expect(adoptingGroups(graph, new Set(['vars']))).toEqual(['group']); + const result = placed(graph, new Set(['vars']), true); + const group = box(result, 'group'); + for (const n of result.nodes) expect(centredIn(group, n)).toBe(true); + // It grew down for the new node only; the nodes there stay, and the comment stays above it. + expect(group.y).toBe(box(graph, 'group').y); + expect(group.h).toBeGreaterThan(box(graph, 'group').h); + expect(box(result, 'note')).toEqual(box(graph, 'note')); + expect(lintLayout(result)).toMatchObject({ overlaps: 0, backwards: 0, coveredNotes: 0, crossings: 4 }); + }); + + it('leaves a group as it is when growing round the new node would take in a node not its own', () => { + const graph = walking(); + graph.nodes.push(node('vars', [], ['speed'], 508, 0)); + graph.edges.push(edge('vars', 'speed', 'move', 'scale')); + // A stranger right under the group, where the new node would go. + const group = box(graph, 'group'); + graph.nodes.push(node('stranger', [], [], 0, group.y + group.h + 8)); + const result = placed(graph, new Set(['vars']), true); + expect(box(result, 'group')).toEqual(group); + }); + + it('puts a new node taking from one in a group next to it, not where it waited far off', () => { + const graph = walking(); + graph.nodes.push(node('far', [], [], 2000, 400)); + // Where the agent's new node waits: right of everything. + graph.nodes.push(node('after', ['exec'], [], 2260, 0)); + graph.edges.push(edge('move', 'exec', 'after', 'exec')); + const result = placed(graph, new Set(['after']), true); + const move = at(result, 'move'); + expect(at(result, 'after').x).toBe(move.x + move.w + COLUMN_GAP); + expect(centredIn(box(result, 'group'), at(result, 'after'))).toBe(true); + expect(centredIn(box(result, 'group'), at(result, 'far'))).toBe(false); + }); + + it('takes nodes in only while none of the new ones stands in a group already', () => { + const graph = walking(); + graph.nodes.push(node('vars', [], ['speed'], 40, 60)); + graph.edges.push(edge('vars', 'speed', 'move', 'scale')); + expect(adoptingGroups(graph, new Set(['vars']))).toEqual([]); + }); +}); + +describe('arranging with groups and comments', () => { + it('fits a group round its nodes again after Arrange, with its comment going along', () => { + const graph = walking(); + // In one row, as the agent leaves new nodes, and grouped like that. + graph.nodes.forEach((n, i) => Object.assign(n, { x: i * 260, y: 0 })); + graph.boxes[0] = { id: 'group', kind: 'group', ...groupBoxAround(graph.nodes)! }; + graph.boxes[1] = { id: 'note', kind: 'note', ...noteBoxAbove(graph.nodes)! }; + const result = placed(graph); + const group = box(result, 'group'); + expect(group).toMatchObject(groupBoxAround(result.nodes)!); + expect(group.w).toBeLessThan(box(graph, 'group').w / 2); + const note = box(result, 'note'); + // Just above the group, as far from it as before. + expect(group.y - (note.y + note.h)).toBe( + box(graph, 'group').y - (box(graph, 'note').y + box(graph, 'note').h), + ); + expect(lintLayout(result).coveredNotes).toBe(0); + }); + + it('moves a group whose nodes all moved alike as it is, keeping its size', () => { + const graph: LayoutGraph = { + nodes: [node('a', [], ['exec'], 0, 0), node('b', ['exec'], [], 248, 0)], + edges: [edge('a', 'exec', 'b', 'exec')], + boxes: [{ id: 'roomy', kind: 'group', x: -100, y: -100, w: 700, h: 400 }], + }; + const spots = new Map([ + ['a', { x: 50, y: 30 }], + ['b', { x: 298, y: 30 }], + ]); + expect(followedBoxes(graph, spots).get('roomy')).toEqual({ x: -50, y: -70, w: 700, h: 400 }); + }); +}); + +describe('room for nodes put between', () => { + /** A fresh instrument: Notes and Output close together, and the chain the agent puts between. */ + function instrument(): LayoutGraph { + return { + nodes: [ + node('notes', [], ['note'], 0, 0), + node('output', ['audio'], [], 248, 0), + node('beside', [], [], 400, 200), + node('chip', ['note'], ['audio'], 600, 0), + node('env', ['audio'], ['audio'], 860, 0), + node('delay', ['audio'], ['audio'], 1120, 0), + ], + edges: [ + edge('notes', 'note', 'chip', 'note'), + edge('chip', 'audio', 'env', 'audio'), + edge('env', 'audio', 'delay', 'audio'), + edge('delay', 'audio', 'output', 'audio'), + ], + boxes: [{ id: 'about', kind: 'note', x: 248, y: -120, w: 220, h: 96 }], + }; + } + + it('pushes what they feed, and everything right of it, far enough right', () => { + const graph = instrument(); + const result = placed(graph, new Set(['chip', 'env', 'delay']), true); + expect(lintLayout(result)).toMatchObject({ overlaps: 0, backwards: 0, crossings: 0 }); + const notes = at(result, 'notes'); + const delay = at(result, 'delay'); + expect(at(result, 'chip').x).toBe(notes.x + notes.w + COLUMN_GAP); + expect(at(result, 'output').x).toBe(delay.x + delay.w + COLUMN_GAP); + expect([notes.x, notes.y]).toEqual([0, 0]); + // What stood right of the output moved by as much, and so did the comment on it. + const by = at(result, 'output').x - 248; + expect(at(result, 'beside')).toMatchObject({ x: 400 + by, y: 200 }); + expect(box(result, 'about')).toMatchObject({ x: 248 + by, y: -120 }); + }); + + it('pushes nothing where there is room between already', () => { + const graph = instrument(); + at(graph, 'output').x = 1500; + const result = placed(graph, new Set(['chip', 'env', 'delay']), true); + expect(at(result, 'output').x).toBe(1500); + expect(at(result, 'beside').x).toBe(400); + expect(lintLayout(result)).toMatchObject({ overlaps: 0, backwards: 0 }); + }); +}); + +describe('freeNoteBox', () => { + const targets = [{ id: 'a', x: 0, y: 200, w: 176, h: 76 }]; + const above = noteBoxAbove(targets)!; + const empty = { nodes: [], groups: [], notes: [] }; + + it('puts a comment above its nodes while that is free', () => { + expect(freeNoteBox(targets, empty)).toEqual(above); + }); + + it('puts it below them where a node stands above, and above again when below is taken too', () => { + const over = { x: 0, y: above.y + 10, w: 176, h: 40 }; + const below = freeNoteBox(targets, { ...empty, nodes: [over] })!; + expect(below.y).toBe(200 + 76 + 40); + const under = { x: 0, y: below.y + 10, w: 176, h: 40 }; + expect(freeNoteBox(targets, { ...empty, nodes: [over, under] })).toEqual(above); + }); + + it('may lie inside a group, but not over its title bar or across its edge', () => { + const roomy = { x: -50, y: above.y - 50, w: 400, h: 500 }; + expect(freeNoteBox(targets, { ...empty, groups: [roomy] })).toEqual(above); + const titled = { ...roomy, y: above.y - 10 }; + expect(freeNoteBox(targets, { ...empty, groups: [titled] })!.y).toBe(200 + 76 + 40); + }); +}); + +describe('lintLayout', () => { + it('counts a comment a node lies on', () => { + const graph: LayoutGraph = { + nodes: [node('a', [], [], 0, 0), node('b', [], [], 0, 300)], + edges: [], + boxes: [ + { id: 'hidden', kind: 'note', x: 50, y: 20, w: 220, h: 96 }, + { id: 'clear', kind: 'note', x: 0, y: 150, w: 220, h: 96 }, + { id: 'group', kind: 'group', x: -20, y: -44, w: 216, h: 120 }, + ], + }; + const lint = lintLayout(graph); + expect(lint.coveredNotes).toBe(1); + expect(lint.examples).toContainEqual({ kind: 'covered', ids: ['hidden', 'a'] }); + }); +}); diff --git a/tools/mcp/relay.mjs b/tools/mcp/relay.mjs index a8a25522..e7387ed1 100644 --- a/tools/mcp/relay.mjs +++ b/tools/mcp/relay.mjs @@ -39,12 +39,15 @@ const INSTRUCTIONS = [ 'one undo step, and a step can name what an earlier one made ("as": "x", then "$x"). Nodes for a', 'graph come from catalog topic "nodes". A Variables node reads or writes a variable once add_port', 'names it. A behavior acts on the object it is placed under: place it with parent set to that object.', - 'Give new nodes no x and y: the editor arranges them when the call ends, and leaves the nodes', - 'already there where they are. Keep a graph readable: a value needed far away comes from a second', - 'Get or Owner node next to its user, not over a long link. Check with lint_graph, the crossings', - 'too: when there are more than the links need, arrange_graph orders the whole graph for the', - 'fewest. Put nodes that belong together into a group with add_group, and say what they do with', - 'add_comment.', + 'Give new nodes no x and y: the editor arranges them when the call ends, next to what they link to', + 'and into its group; the nodes already there stay, or move right to make room. Keep a graph', + 'readable: a value needed far away comes from a second Get or Owner node next to its user, not', + 'over a long link. Check with lint_graph, the crossings too: when there are more than the links', + 'need, arrange_graph orders the whole graph for the fewest. Put nodes that belong together into a', + 'group with add_group, and say what they do with add_comment; a comment a node lies on', + '(coveredNotes) goes back above its nodes with fit_group. Music plays nothing without an', + 'instrument, and an instrument sounds only the notes its Notes nodes take in, C4 to B4 in a new', + 'one: get_asset lists under silent what of a pattern, song or composition is not heard.', 'The editor follows your edits: after run_commands the user sees the graph, scene or asset it worked', 'on, with what it touched selected. Use open and select to show anything else.', 'Test what you built yourself: play, then play_input sends keys and mouse clicks into the game,',