From c6f907b4e086759f3a41c6960987d60a7caffd9e Mon Sep 17 00:00:00 2001 From: root Date: Wed, 9 Sep 2026 20:40:56 +0200 Subject: [PATCH] Make a call node say why it does nothing, and easier to place A behavior node that cannot run looked exactly like a healthy one: no start node, no behavior picked, a deleted behavior or a self-call were all silently skipped. The call still had to be forgiving at runtime -- a dangling reference must not abort a game -- so the fix is diagnosis, not an exception: describeProblem() names the one reason, the node draws it in red under its title, and the inspector repeats it where you pick the behavior. A missing end node is deliberately not a problem: a call is always synchronous, and the end node only decides which values come back. Two ways in: a behavior dragged from the asset panel lands in the graph under the pointer, and double-clicking a call node opens the graph it runs. Both reuse machinery that already existed -- the asset panel already starts the drag, and list views already report double clicks. The e2e module needed its assets at the head of the asset list: after 18 modules the list is long enough that a newly appended row scrolls out of view, and the press then lands on the panel behind it, starting no drag at all. That looked exactly like a rejected drop and cost two full runs. Co-Authored-By: Claude Opus 5 --- README.md | 9 +- src/core/graph/nested.ts | 23 +++- src/editor/debugHook.ts | 5 +- src/editor/graph/GraphCanvas.ts | 99 +++++++++++++++- src/editor/graph/NodeView.ts | 28 ++++- src/editor/inspector/host.ts | 6 +- src/editor/inspector/nodeSection.ts | 5 +- src/ui/Theme.ts | 4 + tests/core/nested-behavior.test.ts | 67 ++++++++++- todo-notes/done/nexted-behavior-ergaenzung.md | 109 ++++++++++++++++++ todo-notes/nexted-behavior-ergaenzung.md | 6 - tools/e2e/19-nested-behavior.mjs | 90 ++++++++++++++- 12 files changed, 427 insertions(+), 24 deletions(-) create mode 100644 todo-notes/done/nexted-behavior-ergaenzung.md delete mode 100644 todo-notes/nexted-behavior-ergaenzung.md diff --git a/README.md b/README.md index 10086bcf..ae97195e 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,13 @@ shown on the node and editable in the inspector. call repeatable and keeps a nested behavior free of hidden state – for state, use a behavior object in the scene (see *Cross-object state* below). A behavior that would run itself raises an error instead of looping; there is one start and one end node per behavior. + Three ways in and out of a call: **drag** a behavior out of the asset panel onto the graph and it + lands as a call where you let go; **double-click** a call node to open the graph it runs. And a + call that cannot do anything **says so on the node** – no behavior picked, the behavior deleted, + or no start node in it. The last one is the one worth naming: without a start node there is no + way in, so the call is skipped and the flow simply carries on at its `exec` output. A missing + *end* node is not a defect – a call is always synchronous, "done" is the nested flow running out, + and the end node only decides what comes back. - **Cross-object state**: a behavior's variables *are* the properties of its object, so another graph reads and writes them with `Object by name` → `Get property` / `Set property`. That is how the example game keeps one coin counter for the whole run. @@ -507,7 +514,7 @@ npm run format # Prettier over src, tests and tools npm run check # typecheck + lint + format:check + tests, what CI would run npm run build # writes dist/ and regenerates index.html export GM_REMOTE=user@host # where the browser runs; see below -node tools/seed-e2e.mjs && node tools/e2e.mjs # 379 editor checks, screenshots in /tmp/gm-e2e-*.png +node tools/seed-e2e.mjs && node tools/e2e.mjs # 387 editor checks, screenshots in /tmp/gm-e2e-*.png node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs # 24 checks: the example game, played end to end node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs # 17 checks: the Tetris example, played end to end ``` diff --git a/src/core/graph/nested.ts b/src/core/graph/nested.ts index 2c187193..3e85b649 100644 --- a/src/core/graph/nested.ts +++ b/src/core/graph/nested.ts @@ -69,12 +69,29 @@ export function resolveDef(node: GraphNode, lookup: BehaviorLookup): NodeDef | u } /** - * Everything that changes a node's ports, as one string. Views and the inspector rebuild when it + * Why this call cannot do anything, or null if it can. A call is the one node that depends on a + * *different* graph, so it is the one node that can look perfectly healthy and still be dead. + */ +export function callProblem(node: GraphNode, lookup: BehaviorLookup, ownerId = ''): string | null { + if (node.type !== RUN_BEHAVIOR) return null; + const id = String(node.params[BEHAVIOR_PARAM] ?? ''); + if (!id) return 'No behavior picked'; + if (id === ownerId) return 'A behavior cannot run itself'; + const program = lookup(id); + if (!program) return 'That behavior is gone'; + if (!terminalNode(program.graph, BEHAVIOR_START)) return 'No start node – does nothing'; + return null; +} + +/** + * Everything that changes how a node looks, as one string. Views and the inspector rebuild when it * changes – a port list that is derived rather than fixed has no other way of being noticed. */ -export function portSignature(node: GraphNode, lookup: BehaviorLookup): string { +export function portSignature(node: GraphNode, lookup: BehaviorLookup, ownerId = ''): string { const def = resolveDef(node, lookup); if (!def) return 'missing'; const list = (ports: PortDef[]) => ports.map((p) => `${p.key}:${p.type}`).join(','); - return `${node.type}|${def.label}|${list(def.inputs)}|${list(def.outputs)}`; + return `${node.type}|${def.label}|${list(def.inputs)}|${list(def.outputs)}|${ + callProblem(node, lookup, ownerId) ?? '' + }`; } diff --git a/src/editor/debugHook.ts b/src/editor/debugHook.ts index bca7e1bd..7dd70262 100644 --- a/src/editor/debugHook.ts +++ b/src/editor/debugHook.ts @@ -1,7 +1,7 @@ import { type Application, Color, type ColorSource, type Container, Rectangle, Text } from 'pixi.js'; import { resolveObjects } from '../core/model/instance'; import { findObjectHolder } from '../core/model/Project'; -import { Button, ui, Widget } from '../ui'; +import { activeDrag, Button, ui, Widget } from '../ui'; import { textureLibrary } from '../viewport/TextureLibrary'; import type { CentreTab, Editor } from './Editor'; import { currentImagePicker } from './texture/ImagePicker'; @@ -169,6 +169,8 @@ export function installDebugHook(app: Application, state: EditorState, editor: E /** Where a dragged asset would land in the Scene tab, and why not when it may not. */ dropPreview: () => viewport.dropInfo(), /** An asset row by id – two assets can share a name with the container picker's label. */ + /** The panel-to-view drag in flight, if any – a drop that never started looks like a refused one. */ + dragPayload: () => activeDrag(), assetRow: (id: string) => assets.rowGlobal(id), assetRowColor: (id: string) => assets.rowColor(id), gameInfo: () => gameView.debugInfo(), @@ -185,6 +187,7 @@ export function installDebugHook(app: Application, state: EditorState, editor: E graphEditor.canvas.portGlobal(nodeId, port, isOutput), nodeAt: (nodeId: string) => graphEditor.canvas.nodeGlobal(nodeId), nodeLabel: (nodeId: string) => graphEditor.canvas.nodeLabel(nodeId), + nodeProblem: (nodeId: string) => graphEditor.canvas.nodeProblem(nodeId), /** Drawn rectangles on screen: what "fit" has to keep inside `canvasRect`. */ nodeRect: (nodeId: string) => graphEditor.canvas.nodeRect(nodeId), canvasRect: () => graphEditor.canvas.canvasRect(), diff --git a/src/editor/graph/GraphCanvas.ts b/src/editor/graph/GraphCanvas.ts index a128711a..b49c59c4 100644 --- a/src/editor/graph/GraphCanvas.ts +++ b/src/editor/graph/GraphCanvas.ts @@ -8,6 +8,8 @@ import { MoveNodeCommand, } from '../../core/commands/graphCommands'; import { + BEHAVIOR_PARAM, + callProblem, canConnect, defAcceptsFrom, dropTargetPort, @@ -22,12 +24,27 @@ import { portsCompatible, portTypeOf, resolveDef, + RUN_BEHAVIOR, } from '../../core/graph'; import type { BehaviorAsset } from '../../core/model/Asset'; import { behaviorLookup } from '../../core/model/behaviorPrograms'; import { findAsset } from '../../core/model/Project'; import { type Trace, TRACE_FADE } from '../../runtime/systems/BehaviorSystem'; -import { clamp, drawGrid, Menu, type Size, Theme, textStyle, Widget, zoomFactor } from '../../ui'; +import { + activeDrag, + clamp, + type DragPayload, + drawGrid, + endDrag, + Menu, + onDragChange, + type Size, + Theme, + textStyle, + Widget, + zoomFactor, +} from '../../ui'; +import { pressCount } from '../../ui/clicks'; import { removeNodes } from '../actions'; import { type EditorState, nodeSelection } from '../state'; import { type Box, boxesInBand, frameBoxes } from './framing'; @@ -78,6 +95,8 @@ export class GraphCanvas extends Widget { current: { x: number; y: number }; active: boolean; } | null = null; + /** A behavior dragged in from the asset panel, and why it may not land. */ + private incoming: { assetId: string; at: { x: number; y: number }; rejection: string } | null = null; private connecting: { from: PortSide; pointer: { x: number; y: number } } | null = null; /** A finished drag whose palette is still open – drawn like the running one, so the connection * being made stays visible while the node type is chosen. */ @@ -114,6 +133,11 @@ export class GraphCanvas extends Widget { e.preventDefault(); }); + // A drag can end anywhere; without this the preview would outlive it. + onDragChange(() => { + if (!activeDrag()) this.setIncoming(null); + }); + // Only the highlight of the selected node depends on the selection. state.on('selection', () => this.redraw()); state.on('project', () => this.sync()); @@ -187,7 +211,7 @@ export class GraphCanvas extends Widget { this.views.set(node.id, view); this.nodeLayer.addChild(view); } - view.sync(node, asset.graph, asset, this.state.project, def); + view.sync(node, asset.graph, asset, this.state.project, def, callProblem(node, lookup, asset.id)); } for (const [id, view] of this.views) { if (!seen.has(id)) { @@ -302,6 +326,7 @@ export class GraphCanvas extends Widget { this.state.toggleNode(this.assetId, view.node.id); return; } + if (pressCount(e, view.node.id) === 2 && this.openCalledBehavior(view.node)) return; // Clicking a selected node keeps the selection so the whole group can be dragged. if (!this.state.selectedNodeIds.includes(view.node.id)) this.state.select(nodeSelection(this.assetId, [view.node.id])); @@ -314,6 +339,16 @@ export class GraphCanvas extends Widget { this.nodeLayer.addChild(view); // raise } + /** Selecting the asset is what opens it: the graph tab follows the selection. */ + private openCalledBehavior(node: GraphNode): boolean { + if (node.type !== RUN_BEHAVIOR) return false; + const id = String(node.params[BEHAVIOR_PARAM] ?? ''); + const asset = id ? findAsset(this.state.project, id) : undefined; + if (asset?.type !== 'behavior') return false; + this.state.select({ kind: 'asset', assetId: id }); + return true; + } + private selectedNodes(): GraphNode[] { const graph = this.asset()?.graph; if (!graph) return []; @@ -322,6 +357,11 @@ export class GraphCanvas extends Widget { } private onPointerMove(e: FederatedPointerEvent): void { + const dragged = activeDrag(); + if (dragged) { + this.trackIncoming(dragged, e); + return; + } if (this.panning) { this.pan.x = this.panning.panX + (e.global.x - this.panning.startX); this.pan.y = this.panning.panY + (e.global.y - this.panning.startY); @@ -357,7 +397,49 @@ export class GraphCanvas extends Widget { } } + /** + * Follows a behavior dragged in from the asset panel. The node is centred under the pointer, so + * it lands where the ghost was rather than below and right of it. + */ + private trackIncoming(dragged: DragPayload, e: FederatedPointerEvent): void { + const local = this.toLocal(e.global); + const inside = local.x >= 0 && local.x <= this.w && local.y >= 0 && local.y <= this.h; + const asset = dragged.kind === 'asset' ? findAsset(this.state.project, dragged.id) : undefined; + if (!inside || !this.assetId || this.state.mode !== 'edit' || asset?.type !== 'behavior') { + this.setIncoming(null); + return; + } + const g = this.screenToGraph(local.x, local.y); + this.setIncoming({ + assetId: asset.id, + at: { x: Math.round(g.x - NODE_WIDTH / 2), y: Math.round(g.y - TITLE_HEIGHT / 2) }, + rejection: asset.id === this.assetId ? 'A behavior cannot run itself' : '', + }); + } + + private setIncoming(incoming: GraphCanvas['incoming']): void { + if (!incoming && !this.incoming) return; + this.incoming = incoming; + this.redraw(); + } + + private dropIncoming(drop: NonNullable): void { + if (drop.rejection) { + this.state.setNotice(drop.rejection); + return; + } + const entry = this.entries().find((it) => it.id === `${RUN_BEHAVIOR}:${drop.assetId}`); + if (entry) this.addNode(entry, drop.at); + } + private onPointerUp(e: FederatedPointerEvent): void { + const incoming = this.incoming; + if (incoming) { + this.setIncoming(null); + endDrag(); + this.dropIncoming(incoming); + return; + } this.panning = null; const marquee = this.marquee; this.marquee = null; @@ -650,6 +732,15 @@ export class GraphCanvas extends Widget { .fill({ color: c.marquee, alpha: 0.12 }) .stroke({ width: 1, color: c.marquee }); } + const drop = this.incoming; + if (drop) { + const a = this.graphToScreen(drop.at.x, drop.at.y); + const color = drop.rejection ? c.danger : c.nodeSelected; + this.band + .rect(a.x, a.y, NODE_WIDTH * this.zoom, NODE_HEIGHT_ESTIMATE * this.zoom) + .fill({ color, alpha: 0.15 }) + .stroke({ width: 2, color }); + } } /** A port's centre in graph coordinates. */ @@ -746,6 +837,10 @@ export class GraphCanvas extends Widget { return this.views.get(nodeId)?.def.label ?? null; } + nodeProblem(nodeId: string): string | null { + return this.views.get(nodeId)?.problemText ?? null; + } + /** Global screen position of a node's title bar centre. */ nodeGlobal(nodeId: string): { x: number; y: number } | null { const view = this.views.get(nodeId); diff --git a/src/editor/graph/NodeView.ts b/src/editor/graph/NodeView.ts index 1d1726c2..75b3643e 100644 --- a/src/editor/graph/NodeView.ts +++ b/src/editor/graph/NodeView.ts @@ -74,6 +74,8 @@ export class NodeView extends Widget { private readonly ports = new Graphics(); private readonly texts: Text[] = []; private paramLines: string[] = []; + /** Why this node can do nothing; see `callProblem`. */ + private problem: string | null = null; selected = false; hot = false; /** Port a running edge drag would connect to, drawn with a ring so the drop is predictable. */ @@ -111,8 +113,10 @@ export class NodeView extends Widget { asset: BehaviorAsset | null, project: Project | null, def: NodeDef, + problem: string | null = null, ): void { this.def = def; + this.problem = problem; this.paramLines = this.def.params .map((p) => ({ label: p.label, @@ -127,8 +131,7 @@ export class NodeView extends Widget { .filter((p) => edgesInto(graph, node.id, p.key).length > 0) .map((p) => p.key), ); - const height = - TITLE_HEIGHT + PAD + this.paramLines.length * ROW_HEIGHT + this.rowCount() * ROW_HEIGHT + PAD; + const height = this.headerHeight() + this.rowCount() * ROW_HEIGHT + PAD; this.rebuildTexts(); this.resize(NODE_WIDTH, height); this.position.set(node.x, node.y); @@ -138,6 +141,17 @@ export class NodeView extends Widget { return Math.max(this.def.inputs.length, this.def.outputs.length); } + /** The warning line as drawn – checks read what the node really shows, not a recomputation. */ + get problemText(): string | null { + return this.problem; + } + + /** Everything above the first port row. The problem line sits in here, so ports move down with it. */ + private headerHeight(): number { + const lines = this.paramLines.length + (this.problem ? 1 : 0); + return TITLE_HEIGHT + PAD + lines * ROW_HEIGHT; + } + /** Local position of a port's centre. */ portPosition(key: string, isOutput: boolean): { x: number; y: number } | null { const list = isOutput ? this.def.outputs : this.def.inputs; @@ -147,7 +161,7 @@ export class NodeView extends Widget { } private rowY(index: number): number { - return TITLE_HEIGHT + PAD + this.paramLines.length * ROW_HEIGHT + index * ROW_HEIGHT + ROW_HEIGHT / 2; + return this.headerHeight() + index * ROW_HEIGHT + ROW_HEIGHT / 2; } /** Every port with its local centre – inputs on the left edge, outputs on the right. */ @@ -190,6 +204,10 @@ export class NodeView extends Widget { add(line, 'small', PAD + 2, y + 2); y += ROW_HEIGHT; } + if (this.problem) { + add(this.problem, 'warning', PAD + 2, y + 2); + y += ROW_HEIGHT; + } for (let i = 0; i < this.rowCount(); i++) { const input = this.def.inputs[i]; const output = this.def.outputs[i]; @@ -230,10 +248,10 @@ export class NodeView extends Widget { this.box .rect(0, TITLE_HEIGHT - Theme.radius, this.w, Theme.radius) .fill(categoryColor(this.def.category)); - const border = this.hot ? c.hot : this.selected ? c.nodeSelected : c.nodeBorder; + const border = this.hot ? c.hot : this.selected ? c.nodeSelected : this.problem ? c.danger : c.nodeBorder; this.box .roundRect(0.5, 0.5, this.w - 1, this.h - 1, Theme.radius) - .stroke({ width: this.selected || this.hot ? 2 : 1, color: border }); + .stroke({ width: this.selected || this.hot || this.problem ? 2 : 1, color: border }); this.ports.clear(); const r = PORT_RADIUS; diff --git a/src/editor/inspector/host.ts b/src/editor/inspector/host.ts index 5e933eef..8af26d1b 100644 --- a/src/editor/inspector/host.ts +++ b/src/editor/inspector/host.ts @@ -16,7 +16,11 @@ export function addTitle(host: InspectorHost, text: string): void { host.body.add(new Label(text, { style: 'title' })); } -export function addNote(host: InspectorHost, text: string, style: 'muted' | 'small' = 'muted'): void { +export function addNote( + host: InspectorHost, + text: string, + style: 'muted' | 'small' | 'warning' = 'muted', +): void { host.body.add(new Label(text, { style })); } diff --git a/src/editor/inspector/nodeSection.ts b/src/editor/inspector/nodeSection.ts index f6635e5a..9aa5ab30 100644 --- a/src/editor/inspector/nodeSection.ts +++ b/src/editor/inspector/nodeSection.ts @@ -1,5 +1,6 @@ import { RemoveNodeCommand, SetNodeParamCommand } from '../../core/commands/graphCommands'; import { + callProblem, edgesInto, type GraphNode, inlineInputDef, @@ -32,7 +33,7 @@ export function nodeSignature(project: Project, assetId: string, nodeId: string) .map((p) => p.key) : []; const declared = (node.ports ?? []).map((p) => `${p.key}:${p.kind}`).join(','); - return `${portSignature(node, lookup)}|${declared}|${connected.join(',')}|${asset.variables + return `${portSignature(node, lookup, assetId)}|${declared}|${connected.join(',')}|${asset.variables .map((v) => v.key) .join(',')}`; } @@ -48,6 +49,8 @@ export function buildNodeSection(host: InspectorHost, assetId: string, nodeId: s addTitle(host, `${def.category.toUpperCase()} NODE`); host.body.add(new Label(def.label, { style: 'bodyBold' })); addNote(host, def.description); + const problem = callProblem(node, behaviorLookup(host.state.project), assetId); + if (problem) addNote(host, problem, 'warning'); if (isTerminalNode(node)) buildPortSection(host, assetId, node); if (def.params.length > 0) addTitle(host, 'SETTINGS'); diff --git a/src/ui/Theme.ts b/src/ui/Theme.ts index 00c76c21..2d53eebd 100644 --- a/src/ui/Theme.ts +++ b/src/ui/Theme.ts @@ -73,6 +73,7 @@ export type TextStyleToken = | 'muted' | 'override' | 'small' + | 'warning' | 'title' | 'mono' | 'accent' @@ -100,6 +101,9 @@ export function textStyle(token: TextStyleToken = 'body'): TextStyle { case 'small': style = new TextStyle({ ...base, fontSize: Theme.smallFontSize, fill: Theme.colors.textMuted }); break; + case 'warning': + style = new TextStyle({ ...base, fontSize: Theme.smallFontSize, fill: Theme.colors.danger }); + break; case 'title': style = new TextStyle({ ...base, diff --git a/tests/core/nested-behavior.test.ts b/tests/core/nested-behavior.test.ts index 3fee81df..ee2c9dc4 100644 --- a/tests/core/nested-behavior.test.ts +++ b/tests/core/nested-behavior.test.ts @@ -1,13 +1,15 @@ import { describe, expect, it } from 'vitest'; import { CommandError } from '../../src/core/commands/Command'; -import { AddNodeCommand } from '../../src/core/commands/graphCommands'; +import { AddNodeCommand, RemoveNodeCommand } from '../../src/core/commands/graphCommands'; import { AddPortCommand, RemovePortCommand, RenamePortCommand } from '../../src/core/commands/portCommands'; import { BEHAVIOR_END, BEHAVIOR_PARAM, BEHAVIOR_START, + callProblem, type Graph, GraphInterpreter, + portSignature, RUN_BEHAVIOR, resolveDef, terminalNode, @@ -180,6 +182,69 @@ describe('a behavior used as a node', () => { }); }); +describe('a call that cannot do anything says so', () => { + it('names each of the four silent cases, and stays quiet about a healthy call', () => { + const fixture = makeProject(); + const lookup = () => behaviorLookup(fixture.project); + const callee = doubler(fixture); + const caller = makeBehavior(fixture, 'Caller'); + const edit = graphEditor(fixture, caller); + const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: '' }); + const problem = () => callProblem(call, lookup(), caller.id); + + expect(problem()).toBe('No behavior picked'); + call.params[BEHAVIOR_PARAM] = caller.id; + expect(problem()).toBe('A behavior cannot run itself'); + call.params[BEHAVIOR_PARAM] = 'gone'; + expect(problem()).toBe('That behavior is gone'); + call.params[BEHAVIOR_PARAM] = makeBehavior(fixture, 'Empty').id; + expect(problem()).toBe('No start node – does nothing'); + call.params[BEHAVIOR_PARAM] = callee.id; + expect(problem()).toBe(null); + // Only a call can be dead this way; no other node type may be marked. + expect(callProblem(edit.addNode('OnStart'), lookup(), caller.id)).toBe(null); + }); + + it('shows up in what the node view redraws on, or it would be drawn once and never again', () => { + const fixture = makeProject(); + // A callee that declares no ports at all: losing its start node changes nothing about the + // calling node except that it is now dead. Anything with ports would move the signature by + // itself and prove nothing. + const callee = makeBehavior(fixture, 'Bare'); + const start = graphEditor(fixture, callee).addNode(BEHAVIOR_START); + const caller = makeBehavior(fixture, 'Caller'); + const call = graphEditor(fixture, caller).addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }); + const signature = () => portSignature(call, behaviorLookup(fixture.project), caller.id); + + const healthy = signature(); + fixture.stack.execute(new RemoveNodeCommand(callee.id, start.id)); + expect(resolveDef(call, behaviorLookup(fixture.project))!.label).toBe('Bare'); + expect(signature()).not.toBe(healthy); + }); + + it('without an end node the call is still a complete one: it just returns nothing', () => { + const fixture = makeProject(); + const callee = makeBehavior(fixture, 'NoEnd'); + const cEdit = graphEditor(fixture, callee); + const start = cEdit.addNode(BEHAVIOR_START); + cEdit.connect(start, 'exec', cEdit.addNode('Log', { message: 'sub' }), 'exec'); + + const caller = makeBehavior(fixture, 'Caller'); + const edit = graphEditor(fixture, caller); + const onStart = edit.addNode('OnStart'); + const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }); + edit.wire([ + [onStart, 'exec', call, 'exec'], + [call, 'exec', edit.addNode('Log', { message: 'after' }), 'exec'], + ]); + + // A missing end node is not a defect, so nothing is reported. + expect(callProblem(call, behaviorLookup(fixture.project), caller.id)).toBe(null); + // "Done" is the nested flow running out: the sub graph is finished before the caller goes on. + expect(runOnStart(fixture, caller).ctx.logs).toEqual(['sub', 'after']); + }); +}); + describe('declared ports', () => { it('renaming one rewrites the edges in this graph and in every calling one', () => { const fixture = makeProject(); diff --git a/todo-notes/done/nexted-behavior-ergaenzung.md b/todo-notes/done/nexted-behavior-ergaenzung.md new file mode 100644 index 00000000..5db0fb5e --- /dev/null +++ b/todo-notes/done/nexted-behavior-ergaenzung.md @@ -0,0 +1,109 @@ +Ein paar Fragen zu behavior als node in einem graph: + +- wenn es keinen Start oder End node gibt, kann man trotztdem das behaviour in einem graph hinzufügen, aber wie läuft dann der flow? (vorallem wenn es keinen start node gibt) +- ohne end node: woher weiß der parent graph (behavior) dass das child (sub behavior) "fertig" ist? +- drag and drop vom asset inspector wäre auch gut als spwan im graph, ist das möglich? +- doppelklick auf node (type behavior) um von dem asset behavior den graph zu öffnen wäre noch gut + +--- + +## Beantwortet und umgesetzt + +Deine ersten zwei Punkte waren Fragen, die letzten zwei Wuensche. Die Antworten standen schon +im Code — der Mangel war, dass **du sie nicht sehen konntest**. Genau das ist jetzt behoben. + +### 1. Ohne Start-Node: wie laeuft der Flow? + +**Gar nicht — der Aufruf wird uebersprungen, und der Fluss laeuft am `exec`-Ausgang weiter.** +Ohne Start-Node gibt es keinen Eingang in den Sub-Graphen; der Interpreter bricht den Aufruf ab +(`if (!start) return {}`) und macht beim Aufrufer weiter. Dasselbe gilt, wenn gar kein Behavior +gewaehlt ist oder das gewaehlte geloescht wurde. Das ist Absicht: ein toter Verweis soll kein +Spiel abbrechen. + +**Das war aber still, und still ist hier falsch.** Es ist derselbe Fehlgriff wie damals beim +`Run in editor`-Schalter: der Node sah gesund aus und tat nichts. Ein Aufruf, der nichts tun +*kann*, sagt es jetzt selbst — rote Zeile auf dem Node, roter Rand, dieselbe Zeile im Inspector: + +| Lage | was der Node sagt | +|---|---| +| kein Behavior gewaehlt | `No behavior picked` | +| Behavior geloescht | `That behavior is gone` | +| Behavior ohne Start-Node | `No start node – does nothing` | +| Behavior zeigt auf sich selbst | `A behavior cannot run itself` | + +Einfuegen kann man ein Behavior ohne Start-Node weiterhin — das ist der normale Weg beim Bauen +(erst den Aufruf setzen, dann den Sub-Graphen fuellen). Es sieht nur nicht mehr fertig aus. + +### 2. Ohne End-Node: woher weiss der Parent, dass das Child fertig ist? + +**Die Frage stellt sich nicht: ein Aufruf ist immer synchron.** Der aufrufende Node kehrt erst +zurueck, wenn der Fluss im Sub-Graphen ausgelaufen ist — "fertig" ist kein Signal, sondern das +Ende der Ausfuehrung. Der End-Node hat damit nichts zu tun; er entscheidet nur, **welche Werte** +zurueckkommen. Ohne ihn kommt schlicht nichts zurueck, alles andere ist unveraendert. + +Deshalb wird ein fehlender End-Node auch **nicht** als Mangel angezeigt — das waere eine +Warnung vor etwas Erlaubtem. Ein eigener Test haelt das fest: der Sub-Graph loggt `sub`, der +Aufrufer danach `after`, und das Ergebnis ist `['sub', 'after']` — nicht umgekehrt und nicht +verschraenkt. + +### 3. Drag & Drop aus dem Asset-Panel in den Graphen + +**Gebaut.** Ein Behavior aus der Asset-Liste in den Graphen gezogen wird zu einem Aufruf-Node, +und zwar **dort, wo du losgelassen hast** (unter dem Zeiger zentriert, nicht rechts-unter ihm). +Waehrend des Ziehens zeigt ein Umriss die Landestelle. Ins **eigene** Behavior gezogen wird +abgelehnt — mit Meldung, statt einen Node zu erzeugen, der beim Laufen einen Fehler wirft. + +Das war billig, weil der Mechanismus schon existierte: `startDrag` feuert im Asset-Panel +laengst fuer jedes platzierbare Asset, und die Szenen-Ansicht ist bereits ein Drop-Ziel. Neu +ist nur, dass der Graph-Canvas jetzt auch eines ist. + +### 4. Doppelklick auf einen Behavior-Node oeffnet dessen Graph + +**Gebaut.** Doppelklick auf einen Aufruf-Node zeigt den Graphen des Behaviors, das er ausfuehrt. +Auch das nutzt vorhandene Mechanik (`pressCount`, dieselbe Erkennung wie in den Listen); der +Node waehlt einfach das Asset aus, und der Graph-Tab folgt der Selektion. + +Zusammen mit Punkt 1 schliesst sich damit ein Kreis: der Node sagt *"No start node"*, ein +Doppelklick bringt dich in den Graphen, und dort legst du den Start-Node an. + +## Belegt + +``` +PASS a behavior dragged from the asset panel becomes a call node +PASS and it lands under the pointer, not at some default spot +PASS a behavior without a start node can be added just the same +PASS and the node says why it does nothing, instead of failing silently – "No start node – does nothing" +PASS a healthy call carries no such warning +PASS dropping a behavior into its own graph is refused, out loud +PASS double-clicking a call node opens the behavior it runs +``` + +Gegenprobe gefahren: ohne die Diagnose fallen beide neuen Unit-Tests. Der zweite war beim +ersten Versuch wertlos — er wollte zeigen, dass der Node **neu gezeichnet** wird, wenn der +Mangel entsteht, prueft das aber an einem Behavior mit Ports, wo sich die Signatur ohnehin +aendert. Jetzt haengt er an einem Behavior ganz ohne Ports, wo sich wirklich nur der Mangel +aendert. + +| | | +|---|---| +| volle Suite, remote | **ALL PASSED – 387 Checks** (379 + 8 neue), 0 FAIL | +| `npm run check` lokal | gruen, **527 Tests** (+3) | + +## Ein Fehlschlag, der zweimal dasselbe war + +Der erste Drag klappte, die beiden folgenden nicht — und zwar **nur im vollen Lauf**, allein +war das Modul gruen. Ich habe zuerst die Drop-Punkte verdaechtigt (falsch: sie lagen in der +Flaeche) und dann die Reihenfolge der Module (falsch: die letzten drei reichten nicht zum +Reproduzieren). Erst eine Messung in der Check-Nachricht hat es gezeigt: die Asset-Zeile lag +bei **y = 449** statt 293 — nach 18 Modulen ist die Liste so lang, dass die zuletzt +angehaengten Behaviors **aus dem sichtbaren Bereich gescrollt** waren. Der Druck landete auf +dem Panel dahinter, es startete gar kein Drag, und das sah exakt aus wie eine Ablehnung. + +Die Testdaten liegen jetzt am **Anfang** der Liste. `__gm.dragPayload()` als neuer Hook +unterscheidet die beiden Faelle kuenftig direkt: *abgelehnt* hat einen Drag, *nie angekommen* +keinen. + +Ein Reparaturversuch am Test-Hook selbst (`rowGlobal` sollte `null` liefern, wenn der +Scroll-Container die Zeile abschneidet) habe ich **wieder zurueckgenommen** — meine Pruefung +ueber die Masken-Bounds lieferte `null` auch fuer klar sichtbare Zeilen und hat den Lauf +sofort im ersten Modul zerlegt. Lieber ein Hook, der schweigt, als einer, der falsch redet. diff --git a/todo-notes/nexted-behavior-ergaenzung.md b/todo-notes/nexted-behavior-ergaenzung.md deleted file mode 100644 index 82bc1587..00000000 --- a/todo-notes/nexted-behavior-ergaenzung.md +++ /dev/null @@ -1,6 +0,0 @@ -Ein paar Fragen zu behavior als node in einem graph: - -- wenn es keinen Start oder End node gibt, kann man trotztdem das behaviour in einem graph hinzufügen, aber wie läuft dann der flow? (vorallem wenn es keinen start node gibt) -- ohne end node: woher weiß der parent graph (behavior) dass das child (sub behavior) "fertig" ist? -- drag and drop vom asset inspector wäre auch gut als spwan im graph, ist das möglich? -- doppelklick auf node (type behavior) um von dem asset behavior den graph zu öffnen wäre noch gut diff --git a/tools/e2e/19-nested-behavior.mjs b/tools/e2e/19-nested-behavior.mjs index 03fcfcfd..44154b70 100644 --- a/tools/e2e/19-nested-behavior.mjs +++ b/tools/e2e/19-nested-behavior.mjs @@ -74,13 +74,16 @@ export async function nestedBehavior(t) { const scene = __gm.state.activeScene; const proto = scene.objects[0]; const mover = p.assets.find((a) => a.id === 'mover'); + // At the front of the list, not the end: these get dragged out of the asset panel below, and + // by this point in the suite the list is long enough to scroll a row at the end out of reach. const behavior = (id, name, graph) => { const a = structuredClone(mover); Object.assign(a, { id, name, props: { enabled: true, runInEditor: false }, variables: [], graph }); - p.assets.push(a); + p.assets.unshift(a); }; behavior('nb-double', 'Double', ${JSON.stringify(doubleGraph)}); behavior('nb-caller', 'Caller', ${JSON.stringify(callerGraph)}); + behavior('nb-empty', 'Empty', { nodes: [], edges: [] }); const make = (id, assetId, parentId, name) => { const o = structuredClone(proto); Object.assign(o, { @@ -204,12 +207,93 @@ export async function nestedBehavior(t) { await undo(); await sleep(200); - // 19g. Clean up: the two behaviors and the box are gone again. + // 19g. Dragging a behavior out of the asset panel drops a call into the graph, where the + // pointer let go of it. + const newNodeIn = async (before) => + (await asset('nb-caller')).graph.nodes.find((n) => !before.includes(n.id)); + const beforeDrag = await nodeIds(); + // Points taken from the drawing area itself: an offset from the centre can fall outside it, + // and a drop that never arrives looks exactly like one that was refused. + const rect = await json('__gm.graph.canvasRect()'); + const spot = (fx, fy) => ({ + x: Math.round(rect.x + rect.w * fx), + y: Math.round(rect.y + rect.h * fy), + }); + const dropAt = spot(0.25, 0.75); + await t.dragAsset('nb-double', dropAt.x, dropAt.y); + await sleep(300); + const dropped = await newNodeIn(beforeDrag); + check( + 'a behavior dragged from the asset panel becomes a call node', + dropped?.type === 'RunBehavior' && dropped?.params.behavior === 'nb-double', + JSON.stringify(dropped && { type: dropped.type, params: dropped.params }), + ); + const droppedAt = dropped && (await json(`__gm.graph.nodeAt(${JSON.stringify(dropped.id)})`)); + check( + 'and it lands under the pointer, not at some default spot', + Boolean(droppedAt) && Math.hypot(droppedAt.x - dropAt.x, droppedAt.y - dropAt.y) < 20, + `dropped at ${JSON.stringify(droppedAt)}, let go at ${JSON.stringify(dropAt)}`, + ); + + // 19h. The question from the note: a behavior with no start node can still be added – and now it + // says why nothing happens instead of sitting there looking healthy. + const beforeEmpty = await nodeIds(); + const emptyRow = await json(`__gm.assetRow('nb-empty')`); + await t.dragAsset('nb-empty', spot(0.6, 0.75).x, spot(0.6, 0.75).y); + await sleep(300); + const emptyCall = await newNodeIn(beforeEmpty); + check( + 'a behavior without a start node can be added just the same', + emptyCall?.params.behavior === 'nb-empty', + `${JSON.stringify(emptyCall && { type: emptyCall.type, params: emptyCall.params })} row=${JSON.stringify(emptyRow)}`, + ); + const problem = await evaluate(`__gm.graph.nodeProblem(${JSON.stringify(emptyCall?.id ?? '')})`); + check( + 'and the node says why it does nothing, instead of failing silently', + problem === 'No start node – does nothing', + `problem=${JSON.stringify(problem)}`, + ); + check( + 'a healthy call carries no such warning', + (await evaluate(`__gm.graph.nodeProblem('nb-call')`)) === null, + ); + await shot('39-call-problem'); + + // 19i. Dropping a behavior into its own graph is refused – and says so, rather than quietly + // adding a node that would throw the moment it ran. + const beforeSelf = await nodeIds(); + const selfRow = await json(`__gm.assetRow('nb-caller')`); + await t.dragAsset('nb-caller', spot(0.45, 0.5).x, spot(0.45, 0.5).y); + await sleep(300); + const afterSelf = await nodeIds(); + const { notice } = await t.state(); + // `dragging` tells a refusal apart from a drag that never arrived – both leave the graph alone. + const dragging = await json('__gm.dragPayload()'); + check( + 'dropping a behavior into its own graph is refused, out loud', + afterSelf.length === beforeSelf.length && notice === 'A behavior cannot run itself', + `nodes ${beforeSelf.length} → ${afterSelf.length}, notice=${JSON.stringify(notice)} row=${JSON.stringify(selfRow)} dragging=${JSON.stringify(dragging)}`, + ); + + // 19j. Double-clicking a call opens the behavior it runs – the other half of the note. + const callNode = await json(`__gm.graph.nodeAt('nb-call')`); + check('the graph shown is still the caller', (await evaluate('__gm.graph.current()')) === 'nb-caller'); + await t.doubleClick(callNode.x, callNode.y); + await sleep(400); + const opened = await evaluate('__gm.graph.current()'); + check( + 'double-clicking a call node opens the behavior it runs', + opened === 'nb-double', + `current=${JSON.stringify(opened)}`, + ); + + // 19k. Clean up: the three behaviors and the box are gone again. await evaluate(`__gm.state.select(null)`); await mutate(` const p = __gm.state.project; const scene = __gm.state.activeScene; - p.assets = p.assets.filter((a) => a.id !== 'nb-double' && a.id !== 'nb-caller'); + const mine = ['nb-double', 'nb-caller', 'nb-empty']; + p.assets = p.assets.filter((a) => !mine.includes(a.id)); scene.objects = scene.objects.filter((o) => o.id !== 'nb-box' && o.id !== 'nb-logic'); `); await sleep(300);