diff --git a/src/editor/agent/editPlan.ts b/src/editor/agent/editPlan.ts index 1b331694..21a1221e 100644 --- a/src/editor/agent/editPlan.ts +++ b/src/editor/agent/editPlan.ts @@ -96,8 +96,13 @@ export class AgentEdit implements Command { } } // A group made here holds its nodes, which arranging sees through; a comment sits above them. + // Round new nodes alone, it gets room: what else would stand in it goes below it. for (const [assetId, nodes] of unplaced) { - const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId), true); + const blocks = frames + .filter((frame) => frame.asset === assetId && frame.kind === 'group') + .map((frame) => frame.nodes) + .filter((held) => held.length > 0 && held.every((node) => nodes.includes(node))); + const arranged = arrangeCommand(project, assetId, nodes, this.measure(assetId), true, blocks); if (arranged) run(arranged); } for (const frame of frames) { diff --git a/src/editor/graph/arrange.ts b/src/editor/graph/arrange.ts index d8e3bb18..bb6706be 100644 --- a/src/editor/graph/arrange.ts +++ b/src/editor/graph/arrange.ts @@ -1,4 +1,5 @@ import type { Box } from '../framing'; +import { ANNOTATION_TITLE_HEIGHT, GROUP_MARGIN } from './annotationDrag'; import { curvePoint } from './beads'; /** @@ -44,6 +45,11 @@ export interface ArrangeOptions { open?: ReadonlySet; /** The nodes are new and stand nowhere yet: next to what they link to is tried before where they are. */ fresh?: boolean; + /** + * Arranged nodes a group is to be fitted round afterwards: what else would stand in that group goes + * below it, so that it takes in no node not its own and lies on no other such group. + */ + blocks?: readonly (readonly string[])[]; } export const COLUMN_GAP = 72; @@ -86,10 +92,10 @@ export function arrange( options: ArrangeOptions = {}, ): Map { const moving = graph.nodes.filter((node) => !movable || movable.has(node.id)); - const placed = new Map(); - if (moving.length === 0) return placed; + if (moving.length === 0) return new Map(); const ids = new Set(moving.map((node) => node.id)); const edges = graph.edges.filter((edge) => ids.has(edge.from.node) && ids.has(edge.to.node)); + const placed = new Map(); let top = 0; for (const part of parts(moving, edges)) { const partIds = new Set(part.map((node) => node.id)); @@ -99,9 +105,71 @@ export function arrange( for (const [id, spot] of spots) placed.set(id, { x: spot.x, y: spot.y + top }); top += height + PART_GAP; } + clearBlocks(moving, placed, options.blocks ?? []); return settle(graph, moving, placed, options); } +/** How far a group reaches past its nodes: its margin, and its title bar on top. */ +const BLOCK_SIDE = GROUP_MARGIN; +const BLOCK_TOP = GROUP_MARGIN + ANNOTATION_TITLE_HEIGHT; + +/** + * Makes room for the groups of `blocks`: what would stand in one – a node, or the group of another + * block – goes below it, together with everything arranged under it, until nothing does. The flow + * keeps its columns; sets that share a node with an earlier one are left out. + */ +function clearBlocks( + nodes: readonly LayoutNode[], + placed: Map, + blocks: readonly (readonly string[])[], +): void { + const sets: string[][] = []; + for (const set of blocks) { + const present = set.filter((id) => placed.has(id)); + if (present.length > 0 && !present.some((id) => sets.some((other) => other.includes(id)))) + sets.push(present); + } + if (sets.length === 0) return; + const units = [ + ...sets, + ...nodes.filter((node) => !sets.some((set) => set.includes(node.id))).map((node) => [node.id]), + ]; + const byId = new Map(nodes.map((node) => [node.id, node])); + const frameOf = (unit: readonly string[]) => { + const boxes = unit.map((id) => ({ ...byId.get(id)!, ...placed.get(id)! })); + const minX = Math.min(...boxes.map((box) => box.x)); + const minY = Math.min(...boxes.map((box) => box.y)); + const maxX = Math.max(...boxes.map((box) => box.x + box.w)); + const maxY = Math.max(...boxes.map((box) => box.y + box.h)); + const framed = sets.includes(unit as string[]); + const side = framed ? BLOCK_SIDE : 0; + return { + x: minX - side, + y: minY - (framed ? BLOCK_TOP : 0), + w: maxX - minX + 2 * side, + h: maxY - minY + (framed ? BLOCK_TOP + GROUP_MARGIN : 0), + }; + }; + for (let round = 0; round < units.length * sets.length; round++) { + const frames = new Map(units.map((unit) => [unit, frameOf(unit)])); + const crowded = sets + .map((set) => ({ set, frame: frames.get(set)! })) + .sort((a, b) => a.frame.y - b.frame.y) + .find(({ set, frame }) => + units.some((unit) => unit !== set && overlap(grown(frames.get(unit)!, ROW_GAP), frame)), + ); + if (!crowded) return; + const { set, frame } = crowded; + const tops = units + .filter((unit) => unit !== set && overlap(grown(frames.get(unit)!, ROW_GAP), frame)) + .map((unit) => frames.get(unit)!.y); + const from = Math.min(...tops); + const by = frame.y + frame.h + ROW_GAP - from; + for (const unit of units) + if (unit !== set && frames.get(unit)!.y >= from) for (const id of unit) placed.get(id)!.y += by; + } +} + /** Groups of nodes that links join, in the order their first node has in the graph. */ function parts(nodes: readonly LayoutNode[], edges: readonly LayoutEdge[]): LayoutNode[][] { const root = new Map(nodes.map((node) => [node.id, node.id])); diff --git a/src/editor/graph/graphLayout.ts b/src/editor/graph/graphLayout.ts index 8bf96189..7ef0513b 100644 --- a/src/editor/graph/graphLayout.ts +++ b/src/editor/graph/graphLayout.ts @@ -55,7 +55,8 @@ export function layoutOf( /** * 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. + * the agent gave no place: put next to what they link to, they join its group. `blocks` are sets of + * them a new group is fitted round afterwards: what else would stand in it goes below it. */ export function arrangeCommand( project: Project, @@ -63,12 +64,13 @@ export function arrangeCommand( nodeIds: readonly string[] | null, measure?: MeasureNode, fresh = false, + blocks: readonly (readonly string[])[] = [], ): Command | null { const layout = layoutOf(project, assetId, measure); if (!layout) return null; 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 spots = arrange(layout, movable, { open: new Set(joining), fresh, blocks }); const boxes = followedBoxes(layout, spots, { groups: joining, nodes: nodeIds ?? [] }); const moves = layout.nodes.flatMap((node) => { const spot = spots.get(node.id); diff --git a/tests/editor/agent-copy.test.ts b/tests/editor/agent-copy.test.ts index fc88612e..5159f34c 100644 --- a/tests/editor/agent-copy.test.ts +++ b/tests/editor/agent-copy.test.ts @@ -8,8 +8,9 @@ import { hostFor, text } from './agentTestHost'; /** Copying nodes through the agent's tools (#434). */ /** - * Behavior Walk, placed by hand so no group takes in a node not its own: a key held moving its owner, a Variables node reading `speed` (kept), a group round - * key and move, a comment above the Variables node, and a group round the owner alone. + * Behavior Walk, placed by hand: a key held moving its owner, a Variables node reading `speed` + * (kept), a group round key and move, a comment above the Variables node, and a group round the + * owner alone. */ async function walkAndRun(fixture: SceneFixture, host: ToolHost) { const steps = [ diff --git a/tests/editor/agent-edit.test.ts b/tests/editor/agent-edit.test.ts index e5f6853f..1889f2f1 100644 --- a/tests/editor/agent-edit.test.ts +++ b/tests/editor/agent-edit.test.ts @@ -241,6 +241,41 @@ describe('run_commands', () => { expect(behavior.graph.groups![0]).toEqual(before); }); + it('gives a group made round new nodes room, what else would stand in it going below', async () => { + const fixture = makeSpriteScene('Hero'); + const { host } = hostFor(fixture); + const steps = [ + ...walkLeft(fixture), + { command: 'add_group', args: { asset: '$walk', nodes: ['$key', '$move'], title: 'Walk' } }, + { command: 'add_group', args: { asset: '$walk', nodes: ['$owner'], title: 'Who' } }, + ]; + const result = await runTool('run_commands', { steps, label: 'walk' }, host); + expect(result.isError, text(result)).toBeFalsy(); + + const behavior = fixture.project.assets.find((asset) => asset.name === 'Walk')!; + const { graph } = findGraphAsset(fixture.project, behavior.id)!; + const [walk, who] = graph.groups!; + const centred = (box: { x: number; y: number; w: number; h: number }) => + graph.nodes + .filter((node) => { + const cx = node.x + NODE_WIDTH / 2; + const cy = node.y + 20; + return cx >= box.x && cx <= box.x + box.w && cy >= box.y && cy <= box.y + box.h; + }) + .map((node) => node.type); + // Move is taller than the key: before, the Owner beside the key came to stand in the Walk group. + expect(centred(walk!)).toEqual(['OnKeyHeld', 'Move']); + expect(centred(who!)).toEqual(['Owner']); + const apart = + who!.x + who!.w <= walk!.x || + walk!.x + walk!.w <= who!.x || + who!.y + who!.h <= walk!.y || + walk!.y + walk!.h <= who!.y; + expect(apart).toBe(true); + const lint = JSON.parse(text(await runTool('lint_graph', { asset: behavior.id }, host))) as GraphLint; + expect(lint).toMatchObject({ overlaps: 0, throughNodes: 0, backwards: 0, coveredNotes: 0 }); + }); + it('changes nothing when a step fails, and names the step and why', async () => { const fixture = makeSpriteScene(); const { host } = hostFor(fixture);