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 <noreply@anthropic.com>
This commit is contained in:
1 parent
1d8e6ebea0
commit
c6f907b4e0
12 files changed
+427
-24
No files matched your search
@@ -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) ?? ''
|
||||
}`;
|
||||
}
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<GraphCanvas['incoming']>): 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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 }));
|
||||
}
|
||||
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user