diff --git a/src/editor/actions.ts b/src/editor/actions.ts index 96fd82e9..6fa44e39 100644 --- a/src/editor/actions.ts +++ b/src/editor/actions.ts @@ -2,7 +2,7 @@ * Editor actions that several places trigger the same way (inspector, hierarchy context * menu, keyboard): they run the command and put the selection where the user expects it. */ -import { RemoveAnnotationCommand } from '../core/commands/annotationCommands'; +import { annotationNoun, RemoveAnnotationCommand } from '../core/commands/annotationCommands'; import { DeleteAssetCommand } from '../core/commands/assetCommands'; import { AddColliderCommand } from '../core/commands/colliderCommands'; import { oneStep } from '../core/commands/CompoundCommand'; @@ -11,6 +11,7 @@ import { RemoveNodeCommand } from '../core/commands/graphCommands'; import { RemoveSceneObjectsCommand } from '../core/commands/sceneObjectCommands'; import type { AssetType } from '../core/model/AssetTypes'; import { isLiveInstance, isTemplateContent } from '../core/model/instance'; +import type { Command } from '../core/commands/Command'; import { findAnnotation } from '../core/graph'; import { findAsset, findGraphAsset, type ObjectHolder, type Project } from '../core/model/Project'; import type { SceneObject } from '../core/model/SceneObject'; @@ -67,6 +68,35 @@ export function removeAnnotation(state: EditorState, assetId: string, annotation return Boolean(found) && state.execute(new RemoveAnnotationCommand(assetId, found!.kind, annotationId)); } +/** + * Deletes everything a graph selection holds – nodes and boxes alike – as one undo step, since one + * band caught them and one keypress asked for them to go. Boxes still take none of their nodes. + */ +export function removeGraphSelection( + state: EditorState, + assetId: string, + nodeIds: readonly string[], + annotationIds: readonly string[], +): boolean { + const graph = findGraphAsset(state.project, assetId)?.graph; + const commands: Command[] = nodeIds.map((id) => new RemoveNodeCommand(assetId, id)); + let noun = ''; + for (const id of annotationIds) { + const found = graph && findAnnotation(graph, id); + if (!found) continue; + noun = annotationNoun(found.kind); + commands.push(new RemoveAnnotationCommand(assetId, found.kind, id)); + } + if (commands.length === 0) return false; + const label = + commands.length === 1 && noun !== '' + ? `Delete ${noun}` + : nodeIds.length === commands.length + ? `Delete ${commands.length} nodes` + : `Delete ${commands.length} items`; + return state.execute(oneStep(label, commands)); +} + /** Asks before deleting an asset, spelling out what goes with it. */ export function confirmDeleteAsset(state: EditorState, assetId: string): void { const asset = findAsset(state.project, assetId); diff --git a/src/editor/debugHook.ts b/src/editor/debugHook.ts index 42c8fe17..4ba88e7c 100644 --- a/src/editor/debugHook.ts +++ b/src/editor/debugHook.ts @@ -336,6 +336,8 @@ export function installDebugHook(app: Application, state: EditorState, editor: E annotations: () => graphEditor.canvas.annotationsDrawn(), /** Where one is grabbed: its title bar, or with `corner` the handle that resizes it. */ annotationAt: (id: string, corner = false) => graphEditor.canvas.annotationGlobal(id, corner), + /** The box as drawn on screen – how a check sees whether a fit brought it into view. */ + annotationRect: (id: string) => graphEditor.canvas.annotationRect(id), dropTarget: () => graphEditor.canvas.pendingDropTarget(), danglingEdge: () => graphEditor.canvas.danglingEdgeVisible(), edgeDrawn: (edgeId: string) => graphEditor.canvas.edgeDrawn(edgeId), diff --git a/src/editor/framing.ts b/src/editor/framing.ts index f0951e1d..2a2977a4 100644 --- a/src/editor/framing.ts +++ b/src/editor/framing.ts @@ -83,6 +83,22 @@ export function boxesInBand(boxes: readonly Box[], band: Band): string[] { .map((b) => b.id); } +/** + * Ids of every box the band encloses whole. This is the rule for groups and comments, not the + * touching one: a group wraps the nodes it holds, so it overlaps every band drawn over any of + * them – and since a group drags its nodes along, catching it by accident would be felt at once. + * Enclosing it is a deliberate gesture, and it is the one the eye reads as "this box, too". + */ +export function boxesWithinBand(boxes: readonly Box[], band: Band): string[] { + const minX = Math.min(band.x1, band.x2); + const maxX = Math.max(band.x1, band.x2); + const minY = Math.min(band.y1, band.y2); + const maxY = Math.max(band.y1, band.y2); + return boxes + .filter((b) => b.x >= minX && b.x + b.w <= maxX && b.y >= minY && b.y + b.h <= maxY) + .map((b) => b.id); +} + /** * Ids of the boxes a group carries: those whose centre lies inside it. The centre, not an overlap, * because a group and the node next to it always touch a little – and because the answer must be diff --git a/src/editor/graph/GraphCanvas.ts b/src/editor/graph/GraphCanvas.ts index 9c86eb79..9fa853e8 100644 --- a/src/editor/graph/GraphCanvas.ts +++ b/src/editor/graph/GraphCanvas.ts @@ -71,12 +71,13 @@ import { Widget, zoomFactor, } from '../../ui'; -import { type EditorState, nodeSelection } from '../state'; +import { annotationSelection, type EditorState, graphSelection, nodeSelection } from '../state'; import { type Box, boundsOf, boxesCentredIn, boxesInBand, + boxesWithinBand, dragOffset, frameBoxes, resizeSize, @@ -142,6 +143,8 @@ export class GraphCanvas extends Widget { private panning: { startX: number; startY: number; panX: number; panY: number } | null = null; private dragging: { items: { nodeId: string; start: { x: number; y: number } }[]; + /** Boxes selected alongside the nodes: a band caught them together, so they travel together. */ + boxes: { id: string; kind: AnnotationKind; start: AnnotationBox }[]; pointer: { x: number; y: number }; moved: boolean; } | null = null; @@ -368,7 +371,10 @@ export class GraphCanvas extends Widget { return; } const id = view.annotation.id; - this.state.select({ kind: 'annotation', assetId: this.assetId, annotationId: id }); + // Grabbing a box that is already part of a wider selection keeps that selection, the way a node + // does: otherwise dragging one of several would silently drop the rest first. + if (!this.state.selectedAnnotationIds.includes(id)) + this.state.select(annotationSelection(this.assetId, [id])); if (e.button === 2) { openAnnotationMenu(this.state, this.assetId, view.kind, id, e.global.x, e.global.y); return; @@ -379,20 +385,71 @@ export class GraphCanvas extends Widget { const pointer = this.screenToGraph(p.x, p.y); const start = boxOf(view.annotation); const resizing = view.handleAt(local.x, local.y); + // A resize carries nothing: membership is re-derived from where the box ends up, so a node the + // corner sweeps over simply belongs to it after. + const along = resizing + ? { boxes: [], nodes: [] } + : this.travellers( + [{ id, kind: view.kind, start }, ...this.selectedBoxes(id)], + this.state.selectedNodeIds, + ); this.annotationDrag = { id, kind: view.kind, mode: resizing ? 'resize' : 'move', start, - // A comment carries nothing, and resizing carries nothing either: membership is re-derived - // from where the box ends up, so a node the corner sweeps over simply belongs to it after. - members: view.kind === 'group' && !resizing ? this.nodeMembers(start) : [], - nested: view.kind === 'group' && !resizing ? this.nestedMembers(id, start) : [], + members: along.nodes, + nested: along.boxes.filter((b) => b.id !== id), pointer, moved: false, }; } + /** The selected groups and comments as drag seeds, `exceptId` left out (it is the grabbed one). */ + private selectedBoxes(exceptId?: string): { id: string; kind: AnnotationKind; start: AnnotationBox }[] { + const graph = this.asset()?.graph; + if (!graph) return []; + const seeds: { id: string; kind: AnnotationKind; start: AnnotationBox }[] = []; + for (const selected of this.state.selectedAnnotationIds) { + if (selected === exceptId) continue; + const found = findAnnotation(graph, selected); + if (found) seeds.push({ id: selected, kind: found.kind, start: boxOf(found.annotation) }); + } + return seeds; + } + + /** + * Everything one move gesture carries: the boxes it starts from, the boxes those hold, and every + * node beneath any of them – each node exactly once, however many groups claim it. Without that + * last part a node inside a group that is itself selected would be offset twice. + */ + private travellers( + seedBoxes: readonly { id: string; kind: AnnotationKind; start: AnnotationBox }[], + seedNodeIds: readonly string[], + ): { + boxes: { id: string; kind: AnnotationKind; start: AnnotationBox }[]; + nodes: { nodeId: string; start: { x: number; y: number } }[]; + } { + const boxes = new Map(seedBoxes.map((b) => [b.id, b])); + for (const seed of seedBoxes) { + if (seed.kind !== 'group') continue; + for (const nested of this.nestedMembers(seed.id, seed.start)) + if (!boxes.has(nested.id)) boxes.set(nested.id, nested); + } + const nodes = new Map(); + const graph = this.asset()?.graph; + for (const id of seedNodeIds) { + const node = graph?.nodes.find((n) => n.id === id); + if (node) nodes.set(id, { nodeId: id, start: { x: node.x, y: node.y } }); + } + for (const box of boxes.values()) { + if (box.kind !== 'group') continue; + for (const member of this.nodeMembers(box.start)) + if (!nodes.has(member.nodeId)) nodes.set(member.nodeId, member); + } + return { boxes: [...boxes.values()], nodes: [...nodes.values()] }; + } + /** The nodes a group at this box carries – the centre rule, against what is drawn right now. */ private nodeMembers(box: AnnotationBox): { nodeId: string; start: { x: number; y: number } }[] { const graph = this.asset()?.graph; @@ -496,11 +553,7 @@ export class GraphCanvas extends Widget { if (!this.assetId) return; const command = new AddAnnotationCommand(this.assetId, kind, box, text); if (this.state.execute(command)) - this.state.select({ - kind: 'annotation', - assetId: this.assetId, - annotationId: command.annotation.id, - }); + this.state.select(annotationSelection(this.assetId, [command.annotation.id])); } /** A group drawn around the selected nodes, with room for the title bar above them. */ @@ -570,13 +623,16 @@ export class GraphCanvas extends Widget { if (this.w === 0 || this.h === 0) return; this.needsFit = false; const asset = this.asset(); - if (!asset || asset.graph.nodes.length === 0) { + // Groups and comments are part of the picture: a fit that framed only the nodes would cut the + // box drawn around them, and a graph holding nothing but a comment would frame the origin. + const boxes = asset ? [...this.nodeBoxes(), ...this.annotationBoxes()] : []; + if (boxes.length === 0) { this.pan = { x: this.w / 2 - NODE_WIDTH / 2, y: this.h / 3 }; this.zoom = 1; this.applyView(); return; } - const framing = frameBoxes(this.nodeBoxes(), { w: this.w, h: this.h }, FIT_MARGIN, { + const framing = frameBoxes(boxes, { w: this.w, h: this.h }, FIT_MARGIN, { min: MIN_ZOOM, max: MAX_ZOOM, }); @@ -585,6 +641,15 @@ export class GraphCanvas extends Widget { this.applyView(); } + /** Every group and comment as the rectangle it is drawn as – a box states its own size. */ + private annotationBoxes(): Box[] { + const graph = this.asset()?.graph; + if (!graph) return []; + return ANNOTATION_KINDS.flatMap((kind) => + annotationsOf(graph, kind).map((a) => ({ id: a.id, ...boxOf(a) })), + ); + } + /** Each node as the rectangle it is actually drawn as (a view knows its own height). */ private nodeBoxes(): Box[] { return (this.asset()?.graph.nodes ?? []).map((node) => { @@ -722,8 +787,10 @@ export class GraphCanvas extends Widget { if (!this.state.selectedNodeIds.includes(view.node.id)) this.state.select(nodeSelection(this.assetId, [view.node.id])); const p = this.toLocal(e.global); + const along = this.travellers(this.selectedBoxes(), this.state.selectedNodeIds); this.dragging = { - items: this.selectedNodes().map((n) => ({ nodeId: n.id, start: { x: n.x, y: n.y } })), + items: along.nodes, + boxes: along.boxes, pointer: this.screenToGraph(p.x, p.y), moved: false, }; @@ -777,6 +844,14 @@ export class GraphCanvas extends Widget { node.x = item.start.x + offset.x; node.y = item.start.y + offset.y; } + // Boxes caught by the same band travel with the nodes, each snapping for itself as a node does. + for (const box of this.dragging.boxes) { + const found = findAnnotation(graph, box.id)?.annotation; + if (!found) continue; + const offset = dragOffset(box.start, dx, dy, step); + found.x = box.start.x + offset.x; + found.y = box.start.y + offset.y; + } this.state.preview(); } else if (this.connecting) { this.connecting.pointer = g; @@ -855,7 +930,7 @@ export class GraphCanvas extends Widget { this.dragging = null; if (drag?.moved && this.assetId) { const graph = this.asset()?.graph; - const moves: MoveNodeCommand[] = []; + const moves: Command[] = []; for (const item of drag.items) { const node = graph?.nodes.find((n) => n.id === item.nodeId); if (!node) continue; @@ -865,6 +940,14 @@ export class GraphCanvas extends Widget { node.y = item.start.y; moves.push(new MoveNodeCommand(this.assetId, item.nodeId, next.x, next.y)); } + // The boxes that came along belong in the same step: one gesture, one undo. + for (const box of drag.boxes) { + const found = graph && findAnnotation(graph, box.id)?.annotation; + if (!found) continue; + const next = boxOf(found); + Object.assign(found, box.start); + moves.push(new SetAnnotationBoxCommand(this.assetId, box.kind, box.id, next)); + } if (moves.length > 0) this.state.execute(oneStep('Move nodes', moves)); } const connecting = this.connecting; @@ -884,14 +967,17 @@ export class GraphCanvas extends Widget { current: { x: number; y: number }; }): void { if (!this.assetId) return; - const ids = boxesInBand(this.nodeBoxes(), { + const band = { x1: marquee.start.x, y1: marquee.start.y, x2: marquee.current.x, y2: marquee.current.y, - }); - // An empty band clears the nodes but keeps the behavior selected, as a plain click does. - this.state.select(nodeSelection(this.assetId, ids) ?? { kind: 'asset', assetId: this.assetId }); + }; + const ids = boxesInBand(this.nodeBoxes(), band); + // A box counts only when the band holds all of it – see `boxesWithinBand`. + const boxIds = boxesWithinBand(this.annotationBoxes(), band); + // An empty band clears the selection but keeps the behavior, as a plain click does. + this.state.select(graphSelection(this.assetId, ids, boxIds) ?? { kind: 'asset', assetId: this.assetId }); } private connect(from: PortSide, to: PortSide): void { @@ -1141,10 +1227,9 @@ export class GraphCanvas extends Widget { return t !== undefined && now - t <= TRACE_FADE; }; const selectedNodes = new Set(this.state.selectedNodeIds); - const selectedAnnotation = - this.state.selection?.kind === 'annotation' ? this.state.selection.annotationId : ''; + const selectedAnnotations = new Set(this.state.selectedAnnotationIds); for (const view of this.annotations.values()) { - view.selected = view.annotation.id === selectedAnnotation; + view.selected = selectedAnnotations.has(view.annotation.id); view.redraw(); } const target = this.dropTarget; @@ -1407,6 +1492,12 @@ export class GraphCanvas extends Widget { return { x: g.x, y: g.y }; } + /** The rectangle a box is drawn as, on screen – what a fit has to bring into view. */ + annotationRect(id: string): Rect | null { + const view = this.annotations.get(id); + return view ? globalRect(view, view.w, view.h) : null; + } + chevronGlobal(nodeId: string): { x: number; y: number } | null { const g = this.views.get(nodeId)?.chevronGlobal(); return g ? { x: g.x, y: g.y } : null; diff --git a/src/editor/graph/graphMenus.ts b/src/editor/graph/graphMenus.ts index 756d54ef..cf68b009 100644 --- a/src/editor/graph/graphMenus.ts +++ b/src/editor/graph/graphMenus.ts @@ -4,7 +4,7 @@ import { CollapseNodesCommand, DisconnectCommand, FoldEdgeCommand } from '../../ import { annotationNoun } from '../../core/commands/annotationCommands'; import type { AnnotationKind, GraphEdge, GraphNode } from '../../core/graph'; import { Menu, type MenuItem } from '../../ui'; -import { removeAnnotation, removeNodes } from '../actions'; +import { removeGraphSelection, removeNodes } from '../actions'; import type { EditorState } from '../state'; /** For the selected nodes; a selection mixing both states offers both directions. */ @@ -49,11 +49,17 @@ export function openAnnotationMenu( x: number, y: number, ): void { + // Right-clicking a box keeps whatever else is selected, so the entry speaks about all of it – + // a menu that silently deleted one of five marked boxes would be the worse surprise. + const held = state.selectedAnnotationIds.includes(id); + const boxes = held ? state.selectedAnnotationIds : [id]; + const nodes = held ? state.selectedNodeIds : []; + const count = boxes.length + nodes.length; new Menu([ { - label: `Delete ${annotationNoun(kind)}`, + label: count > 1 ? `Delete ${count} items` : `Delete ${annotationNoun(kind)}`, danger: true, - onSelect: () => removeAnnotation(state, assetId, id), + onSelect: () => removeGraphSelection(state, assetId, nodes, boxes), }, ]).open(x, y); } diff --git a/src/editor/shortcuts.ts b/src/editor/shortcuts.ts index 2e203f0c..e31585e3 100644 --- a/src/editor/shortcuts.ts +++ b/src/editor/shortcuts.ts @@ -1,6 +1,6 @@ import { RemoveFrameCommand } from '../core/commands/frameCommands'; import { ui } from '../ui'; -import { confirmDeleteAsset, createContainer, removeAnnotation, removeNodes, removeObjects } from './actions'; +import { confirmDeleteAsset, createContainer, removeGraphSelection, removeObjects } from './actions'; import type { EditorState } from './state'; export interface ShortcutActions { @@ -38,9 +38,9 @@ export function installShortcuts(state: EditorState, actions: ShortcutActions): if (event.key === 'Delete' || event.key === 'Backspace') { const s = state.selection; if (s?.kind === 'object') removeObjects(state, s.holderId, s.objectIds); - else if (s?.kind === 'node') removeNodes(state, s.assetId, s.nodeIds); + else if (s?.kind === 'node') removeGraphSelection(state, s.assetId, s.nodeIds, s.annotationIds); else if (s?.kind === 'frame') state.execute(new RemoveFrameCommand(s.assetId, s.frameId)); - else if (s?.kind === 'annotation') removeAnnotation(state, s.assetId, s.annotationId); + else if (s?.kind === 'annotation') removeGraphSelection(state, s.assetId, [], s.annotationIds); else if (s?.kind === 'asset') confirmDeleteAsset(state, s.assetId); return s !== null; } diff --git a/src/editor/state.ts b/src/editor/state.ts index 64897f3b..42157727 100644 --- a/src/editor/state.ts +++ b/src/editor/state.ts @@ -27,12 +27,16 @@ export type Selection = * holds the whole selection, primary included. */ | { kind: 'object'; holderId: string; objectId: string; objectIds: string[] } - /** One or more nodes of a behavior's graph, `nodeId` being the primary one (as with objects). */ - | { kind: 'node'; assetId: string; nodeId: string; nodeIds: string[] } + /** + * One or more nodes of a behavior's graph, `nodeId` being the primary one (as with objects). + * `annotationIds` holds the groups and comments caught alongside them: a rubber band spans + * whatever lies under it, and the two sorts move and die together from then on. + */ + | { kind: 'node'; assetId: string; nodeId: string; nodeIds: string[]; annotationIds: string[] } /** A frame rectangle inside a texture asset (the Texture tab). */ | { kind: 'frame'; assetId: string; frameId: string } - /** One group or comment of a graph – never several, since neither carries the other. */ - | { kind: 'annotation'; assetId: string; annotationId: string } + /** One or more groups or comments of a graph, `annotationId` being the primary one. */ + | { kind: 'annotation'; assetId: string; annotationId: string; annotationIds: string[] } | null; /** @@ -67,9 +71,46 @@ export function objectSelection(holderId: string, objectIds: readonly string[], } /** Builds a node selection; the last id is the primary one unless given explicitly. */ -export function nodeSelection(assetId: string, nodeIds: readonly string[], primary?: string): Selection { +export function nodeSelection( + assetId: string, + nodeIds: readonly string[], + primary?: string, + annotationIds: readonly string[] = [], +): Selection { const picked = pickIds(nodeIds, primary); - return picked && { kind: 'node', assetId, nodeId: picked.primary, nodeIds: picked.ids }; + return ( + picked && { + kind: 'node', + assetId, + nodeId: picked.primary, + nodeIds: picked.ids, + annotationIds: [...new Set(annotationIds)], + } + ); +} + +/** Builds an annotation selection; the last id is the primary one unless given explicitly. */ +export function annotationSelection( + assetId: string, + annotationIds: readonly string[], + primary?: string, +): Selection { + const picked = pickIds(annotationIds, primary); + return picked && { kind: 'annotation', assetId, annotationId: picked.primary, annotationIds: picked.ids }; +} + +/** + * What a rubber band leaves selected: nodes win the kind when it caught any, so the inspector keeps + * showing a node where it did before, and a band over boxes alone still selects them. + */ +export function graphSelection( + assetId: string, + nodeIds: readonly string[], + annotationIds: readonly string[], +): Selection { + return ( + nodeSelection(assetId, nodeIds, undefined, annotationIds) ?? annotationSelection(assetId, annotationIds) + ); } function pickIds(input: readonly string[], primary?: string): { ids: string[]; primary: string } | null { @@ -317,9 +358,25 @@ export class EditorState { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (selection?.kind === 'object' && !selection.objectIds) selection = objectSelection(selection.holderId, [selection.objectId]); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - if (selection?.kind === 'node' && !selection.nodeIds) - selection = nodeSelection(selection.assetId, [selection.nodeId]); + // A graph selection carries two lists, and a short form may leave out either – `{ nodeId }` + // alone, or `{ nodeIds }` without the boxes. Filling both here keeps every reader of them + // from having to ask whether they are there. + if (selection?.kind === 'node') + selection = nodeSelection( + selection.assetId, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + selection.nodeIds ?? [selection.nodeId], + selection.nodeId, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + selection.annotationIds ?? [], + ); + if (selection?.kind === 'annotation') + selection = annotationSelection( + selection.assetId, + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + selection.annotationIds ?? [selection.annotationId], + selection.annotationId, + ); if (sameSelection(this.selection, selection)) return; this.selection = selection; this.emit('selection'); @@ -339,8 +396,13 @@ export class EditorState { const current = s?.kind === 'node' && s.assetId === assetId ? s.nodeIds : []; const held = current.includes(nodeId); const next = held ? current.filter((id) => id !== nodeId) : [...current, nodeId]; + // Boxes caught alongside stay held: the toggle speaks about this node, not about them. + const boxes = s?.kind === 'node' && s.assetId === assetId ? s.annotationIds : []; // Emptied by the toggle, the behavior itself stays selected – the graph must not close. - this.select(nodeSelection(assetId, next, held ? undefined : nodeId) ?? { kind: 'asset', assetId }); + this.select( + nodeSelection(assetId, next, held ? undefined : nodeId, boxes) ?? + annotationSelection(assetId, boxes) ?? { kind: 'asset', assetId }, + ); } get selectedObjectIds(): string[] { @@ -351,6 +413,13 @@ export class EditorState { return this.selection?.kind === 'node' ? this.selection.nodeIds : []; } + /** The groups and comments selected, whether alongside nodes or on their own. */ + get selectedAnnotationIds(): string[] { + const s = this.selection; + if (s?.kind === 'node') return s.annotationIds; + return s?.kind === 'annotation' ? s.annotationIds : []; + } + /** Drops selected targets that no longer exist (after delete/undo). */ private validateSelection(): void { const s = this.selection; @@ -368,16 +437,25 @@ export class EditorState { if (!asset) this.select(null); else { const alive = s.nodeIds.filter((id) => asset.graph.nodes.some((n) => n.id === id)); - if (alive.length !== s.nodeIds.length) - this.select(nodeSelection(s.assetId, alive, s.nodeId) ?? { kind: 'asset', assetId: s.assetId }); + const boxes = s.annotationIds.filter((id) => findAnnotation(asset.graph, id)); + if (alive.length !== s.nodeIds.length || boxes.length !== s.annotationIds.length) + this.select( + nodeSelection(s.assetId, alive, s.nodeId, boxes) ?? + annotationSelection(s.assetId, boxes) ?? { kind: 'asset', assetId: s.assetId }, + ); } } if (s.kind === 'annotation') { const asset = findGraphAsset(this.project, s.assetId); // Same fallback as a node: the graph stays open when the box it held vanished (undo). if (!asset) this.select(null); - else if (!findAnnotation(asset.graph, s.annotationId)) - this.select({ kind: 'asset', assetId: s.assetId }); + else { + const alive = s.annotationIds.filter((id) => findAnnotation(asset.graph, id)); + if (alive.length !== s.annotationIds.length) + this.select( + annotationSelection(s.assetId, alive, s.annotationId) ?? { kind: 'asset', assetId: s.assetId }, + ); + } } if (s.kind === 'frame') { const asset = findAssetOf(this.project, s.assetId, 'texture'); @@ -488,12 +566,21 @@ export function sameSelection(a: Selection, b: Selection): boolean { return ( a.assetId === b.assetId && a.nodeId === b.nodeId && - a.nodeIds.length === b.nodeIds.length && - a.nodeIds.every((id, i) => id === b.nodeIds[i]) + sameIds(a.nodeIds, b.nodeIds) && + sameIds(a.annotationIds, b.annotationIds) ); } if (a.kind === 'frame' && b.kind === 'frame') return a.assetId === b.assetId && a.frameId === b.frameId; if (a.kind === 'annotation' && b.kind === 'annotation') - return a.assetId === b.assetId && a.annotationId === b.annotationId; + return ( + a.assetId === b.assetId && + a.annotationId === b.annotationId && + sameIds(a.annotationIds, b.annotationIds) + ); return false; } + +/** Same ids in the same order – the order carries which one was picked last. */ +function sameIds(a: readonly string[], b: readonly string[]): boolean { + return a.length === b.length && a.every((id, i) => id === b[i]); +} diff --git a/tests/editor/graph-band-selection.test.ts b/tests/editor/graph-band-selection.test.ts new file mode 100644 index 00000000..bffe5fee --- /dev/null +++ b/tests/editor/graph-band-selection.test.ts @@ -0,0 +1,175 @@ +/** + * A rubber band spans a piece of the graph, and what lies in that piece is what it caught – groups + * and comments included. They are not nodes, though: a group wraps the nodes it holds and therefore + * touches every band drawn over any of them, so touching cannot be the rule for a box. The band has + * to hold all of it. From then on both sorts travel and die together, in one undo step. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import { AddAnnotationCommand } from '../../src/core/commands/annotationCommands'; +import { AddNodeCommand } from '../../src/core/commands/graphCommands'; +import { annotationsOf, type Graph } from '../../src/core/graph'; +import { createEmptyProject, findGraphAsset } from '../../src/core/model/Project'; +import { removeGraphSelection } from '../../src/editor/actions'; +import { type Box, boxesInBand, boxesWithinBand } from '../../src/editor/framing'; +import { CreateAssetCommand } from '../../src/core/commands/assetCommands'; +import { annotationSelection, EditorState, graphSelection, sameSelection } from '../../src/editor/state'; + +function fixture() { + const state = new EditorState(); + state.loadProject(createEmptyProject('p', 'P'), 1); + const create = new CreateAssetCommand('behavior', 'B'); + // A rejected command only returns false; an unasserted setup would let assertions pass emptily. + expect(state.execute(create)).toBe(true); + const behaviorId = create.asset.id; + const graphOf = (): Graph => findGraphAsset(state.project, behaviorId)!.graph; + const addNode = (): string => { + const command = new AddNodeCommand(behaviorId, 'Number', 0, 0); + expect(state.execute(command)).toBe(true); + return command.node.id; + }; + const addBox = (kind: 'group' | 'note'): string => { + const command = new AddAnnotationCommand(behaviorId, kind, { x: 0, y: 0, w: 200, h: 120 }, 'T'); + expect(state.execute(command)).toBe(true); + return command.annotation.id; + }; + return { state, behaviorId, graphOf, addNode, addBox }; +} + +const boxes: Box[] = [ + { id: 'group', x: 0, y: 0, w: 300, h: 200 }, + { id: 'note', x: 400, y: 0, w: 100, h: 60 }, +]; + +describe('what a rubber band catches', () => { + it('holds a box only when the band encloses all of it', () => { + // A band across the middle of the group – the gesture one makes to catch the nodes inside it. + const across = { x1: 50, y1: 50, x2: 250, y2: 150 }; + expect(boxesInBand(boxes, across)).toEqual(['group']); + expect(boxesWithinBand(boxes, across)).toEqual([]); + }); + + it('catches the box once the band is drawn around it, whichever way it is drawn', () => { + expect(boxesWithinBand(boxes, { x1: -10, y1: -10, x2: 320, y2: 220 })).toEqual(['group']); + // Right to left and bottom to top mean the same rectangle. + expect(boxesWithinBand(boxes, { x1: 320, y1: 220, x2: -10, y2: -10 })).toEqual(['group']); + }); + + it('leaves out the one that only pokes in, and takes it when the band grows past it', () => { + expect(boxesWithinBand(boxes, { x1: -10, y1: -10, x2: 460, y2: 220 })).toEqual(['group']); + expect(boxesWithinBand(boxes, { x1: -10, y1: -10, x2: 520, y2: 220 })).toEqual(['group', 'note']); + }); +}); + +describe('a selection holding nodes and boxes', () => { + let f: ReturnType; + beforeEach(() => { + f = fixture(); + }); + + it('stays a node selection when the band caught both, so the inspector still shows a node', () => { + const node = f.addNode(); + const group = f.addBox('group'); + f.state.select(graphSelection(f.behaviorId, [node], [group])); + expect(f.state.selection).toMatchObject({ kind: 'node', nodeId: node }); + expect(f.state.selectedNodeIds).toEqual([node]); + expect(f.state.selectedAnnotationIds).toEqual([group]); + }); + + it('becomes a box selection when the band caught boxes alone', () => { + const group = f.addBox('group'); + const note = f.addBox('note'); + f.state.select(graphSelection(f.behaviorId, [], [group, note])); + expect(f.state.selection).toMatchObject({ kind: 'annotation', annotationId: note }); + expect(f.state.selectedAnnotationIds).toEqual([group, note]); + }); + + it('is a different selection once the boxes differ, even with the same nodes', () => { + const node = f.addNode(); + const group = f.addBox('group'); + const without = graphSelection(f.behaviorId, [node], []); + const with_ = graphSelection(f.behaviorId, [node], [group]); + expect(sameSelection(without, graphSelection(f.behaviorId, [node], []))).toBe(true); + expect(sameSelection(without, with_)).toBe(false); + }); + + it('keeps the boxes when a ctrl-click adds a node', () => { + const first = f.addNode(); + const second = f.addNode(); + const group = f.addBox('group'); + f.state.select(graphSelection(f.behaviorId, [first], [group])); + f.state.toggleNode(f.behaviorId, second); + expect(f.state.selectedNodeIds).toEqual([first, second]); + expect(f.state.selectedAnnotationIds).toEqual([group]); + }); + + it('falls back to the boxes when the last node is toggled off, not to the asset', () => { + const node = f.addNode(); + const group = f.addBox('group'); + f.state.select(graphSelection(f.behaviorId, [node], [group])); + f.state.toggleNode(f.behaviorId, node); + expect(f.state.selection).toMatchObject({ kind: 'annotation', annotationId: group }); + }); + + it('accepts the short form a test hook passes for a box, filling in the list', () => { + const group = f.addBox('group'); + f.state.select({ kind: 'annotation', assetId: f.behaviorId, annotationId: group } as never); + expect(f.state.selectedAnnotationIds).toEqual([group]); + }); +}); + +describe('deleting what the band caught', () => { + let f: ReturnType; + beforeEach(() => { + f = fixture(); + }); + + it('takes nodes and boxes in one undo step, and gives both back', () => { + const node = f.addNode(); + const kept = f.addNode(); + const group = f.addBox('group'); + f.state.select(graphSelection(f.behaviorId, [node], [group])); + + expect(removeGraphSelection(f.state, f.behaviorId, [node], [group])).toBe(true); + expect(f.graphOf().nodes.map((n) => n.id)).toEqual([kept]); + expect(annotationsOf(f.graphOf(), 'group')).toHaveLength(0); + + f.state.undo(); + expect(f.graphOf().nodes).toHaveLength(2); + expect(annotationsOf(f.graphOf(), 'group')).toHaveLength(1); + }); + + it('still leaves the nodes a group holds alone – deleting a box is not deleting its content', () => { + const node = f.addNode(); + const group = f.addBox('group'); + removeGraphSelection(f.state, f.behaviorId, [], [group]); + expect(f.graphOf().nodes.map((n) => n.id)).toEqual([node]); + }); + + it('drops a deleted box from a mixed selection but keeps the nodes', () => { + const node = f.addNode(); + const group = f.addBox('group'); + f.state.select(graphSelection(f.behaviorId, [node], [group])); + removeGraphSelection(f.state, f.behaviorId, [], [group]); + expect(f.state.selectedNodeIds).toEqual([node]); + expect(f.state.selectedAnnotationIds).toEqual([]); + }); + + it('keeps the surviving box when one of several goes', () => { + const group = f.addBox('group'); + const note = f.addBox('note'); + f.state.select(annotationSelection(f.behaviorId, [group, note])); + removeGraphSelection(f.state, f.behaviorId, [], [note]); + expect(f.state.selectedAnnotationIds).toEqual([group]); + }); + + it('refuses an empty ask, so no empty step lands on the undo stack', () => { + const node = f.addNode(); + const group = f.addBox('group'); + removeGraphSelection(f.state, f.behaviorId, [node], [group]); + expect(removeGraphSelection(f.state, f.behaviorId, [], [])).toBe(false); + // One undo, not two: the refused ask left nothing behind that a first undo would spend itself on. + f.state.undo(); + expect(f.graphOf().nodes).toHaveLength(1); + expect(annotationsOf(f.graphOf(), 'group')).toHaveLength(1); + }); +}); diff --git a/tools/e2e/36-graph-groups.mjs b/tools/e2e/36-graph-groups.mjs index 0dccf8b3..a9a6714e 100644 --- a/tools/e2e/36-graph-groups.mjs +++ b/tools/e2e/36-graph-groups.mjs @@ -228,4 +228,105 @@ export async function graphGroups(t) { (saved.graph.groups ?? []).length === 0, JSON.stringify({ notes: saved.graph.notes, groups: saved.graph.groups }), ); + // 36j. Fit frames the boxes too (#95). A comment far from every node is the case that tells the + // two answers apart: framing the nodes alone leaves it off screen, and nothing in the + // picture says why. The graph itself never moves – only the view does. + const away = await one('note'); + // Far enough that framing the nodes alone leaves it off screen, near enough that the fit can + // still show everything: the view stops zooming out at 0.25, and past that no fit can succeed. + await mutate(` + const g = __gm.state.project.assets.find((a) => a.id === 'gg-logic').graph; + const note = g.notes.find((n) => n.id === ${JSON.stringify(away.id)}); + note.x = -700; note.y = -500;`); + // The button, not a hook: "click Fit" is the gesture the issue is about. + const fitButton = await locate('Fit'); + await click(fitButton.x, fitButton.y); + const canvas = await json('__gm.graph.canvasRect()'); + const noteRect = await json(`__gm.graph.annotationRect(${JSON.stringify(away.id)})`); + const startRect = await json('__gm.graph.nodeRect("gg-start")'); + const inside = (r) => + r !== null && + r.x >= canvas.x - 1 && + r.y >= canvas.y - 1 && + r.x + r.w <= canvas.x + canvas.w + 1 && + r.y + r.h <= canvas.y + canvas.h + 1; + check( + 'fit brings a far-off comment into view without losing the nodes', + inside(noteRect) && inside(startRect), + JSON.stringify({ canvas, note: noteRect, node: startRect }), + ); + await shot('36-fit-with-comment'); + + // 36k. The rubber band catches boxes as well (#95). Everything is on screen after the fit, so a + // band across the whole canvas must come back holding both sorts at once – and it stays a + // node selection, which is what keeps the inspector showing what it showed before. + await evaluate('__gm.state.select(null)'); + const band = await json(`__gm.graph.annotationRect(${JSON.stringify(away.id)})`); + await drag(canvas.x + 3, canvas.y + 3, canvas.x + canvas.w - 3, canvas.y + canvas.h - 3); + const caught = await json('__gm.state.selection'); + check( + 'a band over the whole graph catches the comment along with the nodes', + caught?.kind === 'node' && + (caught.annotationIds ?? []).includes(away.id) && + caught.nodeIds.includes('gg-start'), + JSON.stringify(caught), + ); + + // 36l. But a band that merely crosses a box leaves it alone – the rule that keeps a group, which + // wraps the nodes it holds, from joining every band drawn over any of them. It starts to the + // left of the comment on empty canvas and stops halfway across it. + await evaluate('__gm.state.select(null)'); + await drag(band.x - 8, band.y + band.h / 2, band.x + band.w / 2, band.y + band.h / 2 + 4); + const grazed = await json('__gm.state.selectedAnnotationIds ?? []'); + const stillThere = await one('note'); + check( + 'a band that only crosses a box does not catch it, and does not move it either', + !grazed.includes(away.id) && stillThere.x === -700 && stillThere.y === -500, + JSON.stringify({ grazed, note: { x: stillThere.x, y: stillThere.y } }), + ); + + // 36m. What one band caught, one drag moves – nodes and boxes by the same offset. The group here + // is the trap: it carries the nodes inside it, and those are selected too, so a naive + // implementation offsets them twice. Equal deltas are what says it did not. + await mutate(` + const g = __gm.state.project.assets.find((a) => a.id === 'gg-logic').graph; + g.groups = [{ id: 'gg-box', x: -20, y: -44, w: 216, h: 320, text: 'Held' }];`); + const fitAgain = await locate('Fit'); + await click(fitAgain.x, fitAgain.y); + const sheet = await json('__gm.graph.canvasRect()'); + await evaluate('__gm.state.select(null)'); + await drag(sheet.x + 3, sheet.y + 3, sheet.x + sheet.w - 3, sheet.y + sheet.h - 3); + const before = { + start: await nodeAt('gg-start'), + far: await nodeAt('gg-far'), + group: (await boxes()).find((a) => a.id === 'gg-box'), + note: await one('note'), + }; + const grip = await json('__gm.graph.nodeAt("gg-far")'); + await drag(grip.x, grip.y, grip.x + 70, grip.y + 50); + const after = { + start: await nodeAt('gg-start'), + far: await nodeAt('gg-far'), + group: (await boxes()).find((a) => a.id === 'gg-box'), + note: await one('note'), + }; + const delta = (key) => ({ x: after[key].x - before[key].x, y: after[key].y - before[key].y }); + const moved = delta('far'); + check( + 'dragging one node moves every caught box and node by the same step, none of them twice', + moved.x > 0 && + moved.y > 0 && + ['start', 'group', 'note'].every((key) => delta(key).x === moved.x && delta(key).y === moved.y), + JSON.stringify({ far: moved, start: delta('start'), group: delta('group'), note: delta('note') }), + ); + + // And one undo takes the whole mixed gesture back, boxes included. + await t.undo(); + check( + 'one undo returns the nodes and the boxes together', + JSON.stringify(await nodeAt('gg-start')) === JSON.stringify(before.start) && + (await boxes()).find((a) => a.id === 'gg-box').x === before.group.x && + (await one('note')).x === before.note.x, + JSON.stringify({ start: await nodeAt('gg-start'), note: (await one('note')).x }), + ); }