diff --git a/README.md b/README.md index 7be949a3..147c0b96 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,10 @@ shown on the node and editable in the inspector. - **Structs**: several named values travelling as one, so a bundle needs one edge instead of one per part. `Pack` declares its fields as inputs, `Expand` reads them back out as outputs, a behavior port may be declared `struct` – which is how a bundle crosses a call – and a - **variable** may be one, which is how a bundle is stored. Writing a struct variable keeps the + **variable** may be one, which is how a bundle is stored. A `Pack`/`Expand` can take its fields + from a variable that already declares the shape (**From variable**, a struct variable or the item + shape of a list of structs) instead of having them typed again; that adds the fields it does not + have yet as one undo step and leaves the ones it has alone. Writing a struct variable keeps the fields it declares: what arrives fills the ones it names, the rest stay put (`mergeStruct`), so one node can set part of a bundle without erasing it – "the same piece, one row lower" is a `Pack` of that one field. Depth is bounded by the *declaration*, not by convention: a list says @@ -604,7 +607,7 @@ npm run check # typecheck + lint + format:check + tests, what CI would ru # code is lost (tail's is reported), so a red run reads as green npm run build # writes dist/ and regenerates index.html export GM_REMOTE=user@host # optional: run the browser elsewhere; see below -node tools/seed-e2e.mjs && node tools/e2e.mjs # 496 editor checks, screenshots in /tmp/gm-e2e-*.png +node tools/seed-e2e.mjs && node tools/e2e.mjs # 502 editor checks, screenshots in /tmp/gm-e2e-*.png GM_STEPS=20 node tools/e2e.mjs # or 1-12, or 6,20 – a subset while working on one step ./e2e.sh start [GM_STEPS] # the same run detached, see below ./e2e.sh status # live progress, or the last run's result diff --git a/src/core/commands/portCommands.ts b/src/core/commands/portCommands.ts index 1f7ce28f..3f56d3e5 100644 --- a/src/core/commands/portCommands.ts +++ b/src/core/commands/portCommands.ts @@ -71,30 +71,45 @@ function renameParam(site: PortSite, from: string, to: string): void { delete node.params[from]; } -const INVALID_NAME = 'Port names must start with a letter and contain only letters, digits or _'; +// A struct field may be called `exec`, a port may not – so this message has to name that reason. +const INVALID_NAME = + 'Port names must start with a letter, contain only letters, digits or _, and "exec" is taken by the flow port'; -export class AddPortCommand implements Command { +/** Several ports as one step, so seeding a node from a struct is one undo rather than one per field. */ +export class AddPortsCommand implements Command { readonly label: string; + protected readonly ports: readonly DeclaredPort[]; constructor( private readonly assetId: string, private readonly nodeId: string, - private readonly port: DeclaredPort, + ports: readonly DeclaredPort[], ) { - this.label = `Add ${port.key}`; + this.ports = ports.map((p) => ({ ...p })); + this.label = this.ports.length === 1 ? `Add ${this.ports[0]?.key}` : `Add ${this.ports.length} ports`; } execute(project: Project): void { const node = requirePortNode(project, this.assetId, this.nodeId); - if (!isValidPortName(this.port.key)) throw new CommandError(INVALID_NAME); - if ((node.ports ?? []).some((p) => p.key === this.port.key)) - throw new CommandError(`"${this.port.key}" already exists`); - node.ports = [...(node.ports ?? []), { ...this.port }]; + const keys = new Set((node.ports ?? []).map((p) => p.key)); + for (const port of this.ports) { + if (!isValidPortName(port.key)) throw new CommandError(INVALID_NAME); + if (keys.has(port.key)) throw new CommandError(`"${port.key}" already exists`); + keys.add(port.key); + } + node.ports = [...(node.ports ?? []), ...this.ports.map((p) => ({ ...p }))]; } undo(project: Project): void { const node = requirePortNode(project, this.assetId, this.nodeId); - node.ports = (node.ports ?? []).filter((p) => p.key !== this.port.key); + const added = new Set(this.ports.map((p) => p.key)); + node.ports = (node.ports ?? []).filter((p) => !added.has(p.key)); + } +} + +export class AddPortCommand extends AddPortsCommand { + constructor(assetId: string, nodeId: string, port: DeclaredPort) { + super(assetId, nodeId, [port]); } } diff --git a/src/core/graph/index.ts b/src/core/graph/index.ts index 59de1228..59014a6c 100644 --- a/src/core/graph/index.ts +++ b/src/core/graph/index.ts @@ -8,4 +8,5 @@ export * from './nested'; export * from './NodeContext'; export * from './NodeRegistry'; export * from './values'; +export * from './variablePorts'; export { GraphInterpreter, type TraceListener } from './GraphInterpreter'; diff --git a/src/core/graph/variablePorts.ts b/src/core/graph/variablePorts.ts new file mode 100644 index 00000000..ce741e4b --- /dev/null +++ b/src/core/graph/variablePorts.ts @@ -0,0 +1,34 @@ +import type { PropertyDef, StructFieldDef } from '../properties/PropertySchema'; +import { DECLARED_PORT_KINDS, type DeclaredPort, type DeclaredPortKind, type GraphNode } from './Graph'; + +/** Every variable kind is a port kind too; the fallback is for a project file that says otherwise. */ +export function variablePortKind(def: PropertyDef): DeclaredPortKind { + return (DECLARED_PORT_KINDS as readonly string[]).includes(def.kind) + ? (def.kind as DeclaredPortKind) + : 'text'; +} + +/** A color is a number in the value model, and the graph has no port kind of its own for it. */ +export function fieldPortKind(field: StructFieldDef): DeclaredPortKind { + return field.kind === 'color' ? 'number' : field.kind; +} + +/** + * The bundle shape a variable declares, if it declares one: a struct variable holds it directly, a + * list of structs declares the shape of every item. Both are what a Pack builds and an Expand takes + * apart, so both can seed a node's fields. + */ +export function structShapeOf(def: PropertyDef): readonly StructFieldDef[] | undefined { + if (def.kind === 'struct') return def.fields; + if (def.kind === 'list' && def.items === 'struct') return def.fields ?? []; + return undefined; +} + +/** + * The ports `fields` would add to `node`. A key the node already declares is left alone rather than + * retyped: it may carry an edge, and the manual declaration is the one the user typed. + */ +export function missingFieldPorts(node: GraphNode, fields: readonly StructFieldDef[]): DeclaredPort[] { + const taken = new Set((node.ports ?? []).map((p) => p.key)); + return fields.filter((f) => !taken.has(f.key)).map((f) => ({ key: f.key, kind: fieldPortKind(f) })); +} diff --git a/src/editor/inspector/nodeSection.ts b/src/editor/inspector/nodeSection.ts index a2332c8d..d1fda4eb 100644 --- a/src/editor/inspector/nodeSection.ts +++ b/src/editor/inspector/nodeSection.ts @@ -53,7 +53,7 @@ export function buildNodeSection(host: InspectorHost, assetId: string, nodeId: s const problem = callProblem(node, behaviorLookup(host.state.project), assetId); if (problem) addNote(host, problem, 'warning'); if (declaresVariablePorts(node.type)) buildVariablePortSection(host, assetId, node, asset.variables); - else if (def.declaresPorts) buildPortSection(host, assetId, node); + else if (def.declaresPorts) buildPortSection(host, assetId, node, asset.variables); if (def.params.length > 0) addTitle(host, 'SETTINGS'); for (const param of def.params) { diff --git a/src/editor/inspector/portSection.ts b/src/editor/inspector/portSection.ts index a1a4be18..119267e1 100644 --- a/src/editor/inspector/portSection.ts +++ b/src/editor/inspector/portSection.ts @@ -1,5 +1,6 @@ import { AddPortCommand, + AddPortsCommand, nextPortName, RemovePortCommand, RenamePortCommand, @@ -12,6 +13,9 @@ import { type DeclaredPortKind, type GraphNode, isTerminalNode, + missingFieldPorts, + structShapeOf, + variablePortKind, } from '../../core/graph'; import type { PropertyDef } from '../../core/properties/PropertySchema'; import { Button, Dropdown, HStack, Label, Menu, type MenuItem, Spacer, TextField, Theme } from '../../ui'; @@ -97,25 +101,24 @@ export function buildVariablePortSection( } } -/** Every variable kind is a port kind too; the fallback is for a project file that says otherwise. */ -function variablePortKind(def: PropertyDef): DeclaredPortKind { - return (DECLARED_PORT_KINDS as readonly string[]).includes(def.kind) - ? (def.kind as DeclaredPortKind) - : 'text'; -} - /** * The ports a node declares for itself: the signature of a behavior seen as a node, or the fields * of a struct. A start node's are named from the caller's side – what it hands into the graph is * an *input* of the behavior – which is why the side comes from the node type, not from here. */ -export function buildPortSection(host: InspectorHost, assetId: string, node: GraphNode): void { +export function buildPortSection( + host: InspectorHost, + assetId: string, + node: GraphNode, + variables: readonly PropertyDef[] = [], +): void { const onStart = node.type === BEHAVIOR_START; const noun = isTerminalNode(node) ? (onStart ? 'Input' : 'Output') : 'Field'; const base = noun === 'Field' ? 'field' : onStart ? 'in' : 'out'; const header = new HStack({ gap: Theme.gap, align: 'center' }); header.add(new Label(`${noun.toUpperCase()}S`, { style: 'title' }), 1); + if (noun === 'Field') addShapePicker(host, assetId, node, variables, header); const add = new Dropdown(`+ ${noun}`, { variant: 'ghost' }); add.onClick = () => { const items = DECLARED_PORT_KINDS.map((kind) => ({ @@ -164,3 +167,33 @@ export function buildPortSection(host: InspectorHost, assetId: string, node: Gra host.body.add(row); } } + +/** + * Takes the fields of a Pack/Expand from a variable that already declares the shape, instead of + * typing them again. Only offered where there is a shape to copy; a field the node already has + * stays as it is, so this adds and never rewrites. + */ +function addShapePicker( + host: InspectorHost, + assetId: string, + node: GraphNode, + variables: readonly PropertyDef[], + header: HStack, +): void { + const shaped = variables.filter((v) => structShapeOf(v) !== undefined); + if (shaped.length === 0) return; + + const pick = new Dropdown('From variable', { variant: 'ghost' }); + pick.onClick = () => { + const items: MenuItem[] = shaped.map((v) => { + const ports = missingFieldPorts(node, structShapeOf(v) ?? []); + return { + label: v.kind === 'list' ? `${v.key} (item)` : v.key, + disabled: ports.length === 0, + onSelect: () => host.state.execute(new AddPortsCommand(assetId, node.id, ports)), + }; + }); + new Menu(items).openBelow(pick); + }; + header.add(pick); +} diff --git a/tests/core/struct-shape-ports.test.ts b/tests/core/struct-shape-ports.test.ts new file mode 100644 index 00000000..8e3ee0dd --- /dev/null +++ b/tests/core/struct-shape-ports.test.ts @@ -0,0 +1,143 @@ +/** + * Seeding the fields of a Pack/Expand from a variable that already declares the shape. The node + * keeps declaring its own ports – this only copies a shape that exists elsewhere in the behavior. + */ +import { describe, expect, it } from 'vitest'; +import { AddPortCommand, AddPortsCommand } from '../../src/core/commands/portCommands'; +import { + AddStructFieldCommand, + AddVariableCommand, + SetListItemsCommand, +} from '../../src/core/commands/variableCommands'; +import { + type DeclaredPortKind, + fieldPortKind, + missingFieldPorts, + resolveDef, + structShapeOf, +} from '../../src/core/graph'; +import type { BehaviorAsset } from '../../src/core/model/Asset'; +import { behaviorLookup } from '../../src/core/model/behaviorPrograms'; +import type { StructFieldDef } from '../../src/core/properties/PropertySchema'; +import { graphEditor, makeBehavior, makeProject } from '../helpers'; + +const PIECE: readonly StructFieldDef[] = [ + { key: 'shape', kind: 'number' }, + { key: 'name', kind: 'text' }, + { key: 'held', kind: 'boolean' }, + { key: 'tint', kind: 'color' }, + { key: 'cells', kind: 'list', items: 'number' }, +]; + +function behaviorWithPiece() { + const fixture = makeProject(); + const behavior = makeBehavior(fixture, 'B'); + fixture.stack.execute(new AddVariableCommand(behavior.id, 'piece', 'struct')); + for (const field of PIECE) fixture.stack.execute(new AddStructFieldCommand(behavior.id, 'piece', field)); + return { ...fixture, behavior, edit: graphEditor(fixture, behavior) }; +} + +const varOf = (behavior: BehaviorAsset, key: string) => behavior.variables.find((v) => v.key === key)!; + +describe('the shape a variable declares', () => { + it('is the fields of a struct variable and the item fields of a list of structs', () => { + const fixture = behaviorWithPiece(); + const { behavior, stack } = fixture; + stack.execute(new AddVariableCommand(behavior.id, 'shapes', 'list')); + stack.execute(new SetListItemsCommand(behavior.id, 'shapes', 'struct')); + for (const key of ['x', 'y']) + stack.execute(new AddStructFieldCommand(behavior.id, 'shapes', { key, kind: 'number' })); + + expect(structShapeOf(varOf(behavior, 'piece'))?.map((f) => f.key)).toEqual([ + 'shape', + 'name', + 'held', + 'tint', + 'cells', + ]); + expect(structShapeOf(varOf(behavior, 'shapes'))?.map((f) => f.key)).toEqual(['x', 'y']); + }); + + it('is absent for a variable that declares no bundle', () => { + const fixture = behaviorWithPiece(); + const { behavior, stack } = fixture; + stack.execute(new AddVariableCommand(behavior.id, 'score', 'number')); + stack.execute(new AddVariableCommand(behavior.id, 'names', 'list')); + + expect(structShapeOf(varOf(behavior, 'score'))).toBeUndefined(); + expect(structShapeOf(varOf(behavior, 'names'))).toBeUndefined(); + }); + + /** A color is stored as a number and the graph has no port kind for it – every other kind is itself. */ + it('reads a color field as a number port and a list field as a list port', () => { + expect(PIECE.map(fieldPortKind)).toEqual([ + 'number', + 'text', + 'boolean', + 'number', + 'list', + ]); + }); +}); + +describe('seeding a node from that shape', () => { + it('leaves a field the node already declares alone', () => { + const { behavior, stack, edit } = behaviorWithPiece(); + const pack = edit.addNode('PackStruct'); + stack.execute(new AddPortCommand(behavior.id, pack.id, { key: 'shape', kind: 'text' })); + + const ports = missingFieldPorts(pack, structShapeOf(varOf(behavior, 'piece')) ?? []); + expect(ports.map((p) => p.key)).toEqual(['name', 'held', 'tint', 'cells']); + }); + + it('adds every field as one undo step, and the ports become inputs of the Pack', () => { + const fixture = behaviorWithPiece(); + const { behavior, stack, edit } = fixture; + const pack = edit.addNode('PackStruct'); + const ports = missingFieldPorts(pack, structShapeOf(varOf(behavior, 'piece')) ?? []); + stack.execute(new AddPortsCommand(behavior.id, pack.id, ports)); + + const inputs = resolveDef(pack, behavior.graph, behaviorLookup(fixture.project))!.inputs; + expect(inputs.map((p) => `${p.key}:${p.type}`)).toEqual([ + 'shape:number', + 'name:string', + 'held:boolean', + 'tint:number', + 'cells:list', + ]); + + stack.undo(); + expect(pack.ports ?? []).toEqual([]); + }); + + /** All-or-nothing: a clash halfway down must not leave the node with the fields before it. */ + it('adds nothing at all when one of the ports clashes', () => { + const { behavior, stack, edit } = behaviorWithPiece(); + const pack = edit.addNode('PackStruct'); + stack.execute(new AddPortCommand(behavior.id, pack.id, { key: 'held', kind: 'boolean' })); + + expect(() => + stack.execute( + new AddPortsCommand( + behavior.id, + pack.id, + PIECE.map((f) => ({ key: f.key, kind: 'number' })), + ), + ), + ).toThrow('"held" already exists'); + expect((pack.ports ?? []).map((p) => p.key)).toEqual(['held']); + }); + + /** A field may be called `exec`, a port may not – the one name where a shape does not fit a node. */ + it('rejects a shape whose field is called exec, and says why', () => { + const { behavior, stack, edit } = behaviorWithPiece(); + stack.execute(new AddStructFieldCommand(behavior.id, 'piece', { key: 'exec', kind: 'number' })); + const pack = edit.addNode('PackStruct'); + const ports = missingFieldPorts(pack, structShapeOf(varOf(behavior, 'piece')) ?? []); + + expect(() => stack.execute(new AddPortsCommand(behavior.id, pack.id, ports))).toThrow( + 'taken by the flow port', + ); + expect(pack.ports ?? []).toEqual([]); + }); +}); diff --git a/todo-notes/done/expand-und-pack-node.md b/todo-notes/done/expand-und-pack-node.md new file mode 100644 index 00000000..9c4a99a3 --- /dev/null +++ b/todo-notes/done/expand-und-pack-node.md @@ -0,0 +1,21 @@ +beim expand und pack node habe ich mir eigentlich auch vorgestellt, dass man sich eine struct variable auswählt und automatisch die felder erzeugt werden. +wir können das so lassen wie es ist (eigene felder definieren), aber eine helfer auswahl von structs das die felder erstellt wäre gut. + +--- + +**Umgesetzt (2026-09-10).** `From variable ▾` steht in der Kopfzeile des Port-Editors neben +`+ Field ▾`; das Menü listet die Struct-Variablen des Behaviors und legt deren Felder als Ports an. + +Drei Entscheidungen, die die Notiz offen ließ: + +- **Es ergänzt nur, es schreibt nichts um.** Ein Feld, das der Node schon deklariert, bleibt stehen – + es könnte eine Kante tragen. Ist nichts mehr zu ergänzen, ist der Eintrag deaktiviert. +- **Eine Liste aus Structs zählt mit** (`shapes (item)`): sie deklariert die Form *jedes Items*, also + genau das Bündel, das ein `Pack` für sie baut. +- **Ein Color-Feld wird ein Zahlport.** `FIELD_KINDS` ist *keine* Teilmenge von `DECLARED_PORT_KINDS` – + der Graph kennt keine Farbe, im Wertemodell ist sie eine Zahl. + +Zwei Fallen unterwegs, beide gemessen: `AddPortCommand` legt genau **einen** Port an, „alle Felder auf +einmal" wäre sonst in fünf Undo-Schritte zerfallen – daher `AddPortsCommand`, das **alle** Schlüssel +prüft, bevor es einen anhängt (sonst bliebe nach einer Kollision die halbe Liste stehen). +Und ein Struct-Feld darf `exec` heißen, ein Port nicht; die Fehlermeldung sagt jetzt auch warum. diff --git a/todo-notes/expand-und-pack-node.md b/todo-notes/expand-und-pack-node.md deleted file mode 100644 index 53cb980c..00000000 --- a/todo-notes/expand-und-pack-node.md +++ /dev/null @@ -1,2 +0,0 @@ -beim expand und pack node habe ich mir eigentlich auch vorgestellt, dass man sich eine struct variable auswählt und automatisch die felder erzeugt werden. -wir können das so lassen wie es ist (eigene felder definieren), aber eine helfer auswahl von structs das die felder erstellt wäre gut. diff --git a/tools/e2e/23-variable-nodes.mjs b/tools/e2e/23-variable-nodes.mjs index 799cff95..07d16cd2 100644 --- a/tools/e2e/23-variable-nodes.mjs +++ b/tools/e2e/23-variable-nodes.mjs @@ -148,6 +148,82 @@ export async function variableNodes(t) { await sleep(300); check('and undo points it back', (await portsOf('vp-get'))[0].key === 'vx'); + // 23f. A Pack keeps declaring its own fields, but a variable that already carries the shape can + // hand it over. A color field arrives as a number port – the graph has no colors. + await mutate(` + const a = __gm.state.project.assets.find((x) => x.id === 'vp-logic'); + a.variables.push( + ${JSON.stringify( + variable('piece', 'struct', { shape: 0, tint: 0, cells: [] }, undefined, [ + { key: 'shape', kind: 'number' }, + { key: 'tint', kind: 'color' }, + { key: 'cells', kind: 'list', items: 'number' }, + ]), + )}, + ${JSON.stringify( + variable('shapes', 'list', [], 'struct', [ + { key: 'x', kind: 'number' }, + { key: 'y', kind: 'number' }, + ]), + )}, + ); + a.props.piece = { shape: 0, tint: 0, cells: [] }; + a.props.shapes = []; + a.graph.nodes.push(${JSON.stringify(node('vp-pack', 'PackStruct', 560, 400))}); + `); + await sleep(300); + + await select('vp-pack'); + const from = await locate('From variable'); + check('a Pack offers the shapes its behavior declares', Boolean(from), JSON.stringify(from)); + await click(from.x, from.y); + await sleep(200); + await shot('03-from-variable'); + await clickLast('piece'); + await sleep(300); + const seeded = await portsOf('vp-pack'); + check( + 'picking a struct variable declares every field at once, a color among them as a number', + JSON.stringify(seeded) === + '[{"key":"shape","kind":"number"},{"key":"tint","kind":"number"},{"key":"cells","kind":"list"}]', + JSON.stringify(seeded), + ); + check('and the node grew those inputs', Boolean(await portAt('vp-pack', 'cells', false))); + await shot('04-fields-seeded'); + await undo(); + await sleep(300); + check('one undo takes all of them back', (await portsOf('vp-pack')).length === 0); + + // 23g. A list of structs declares the shape of its items, which is the bundle a Pack builds for it. + await click(from.x, from.y); + await sleep(200); + await clickLast('shapes (item)'); + await sleep(300); + const fromItem = await portsOf('vp-pack'); + check( + 'a list of structs offers its item shape too', + fromItem.map((p) => p.key).join(',') === 'x,y', + JSON.stringify(fromItem), + ); + await undo(); + await sleep(300); + + // 23h. A behavior's own inputs are not fields of a bundle, so the helper stays out of that section. + await mutate(` + __gm.state.project.assets.find((x) => x.id === 'vp-logic').graph.nodes.push( + ${JSON.stringify(node('vp-start', 'BehaviorStart', 560, 600))}, + ); + `); + await sleep(200); + await select('vp-start'); + // `locate` throws when the text is absent, which is the wrong shape for "is it there?". + const helper = await json(`__gm.locate('From variable', false)`); + check( + 'a behavior start declares inputs, not fields, and is not offered a shape', + Boolean(await locate('+ Input')) && helper === null, + JSON.stringify(helper), + ); + await mutate(` const p = __gm.state.project; p.assets = p.assets.filter((a) => a.id !== 'vp-logic');