diff --git a/src/core/commands/variableCommands.ts b/src/core/commands/variableCommands.ts index d213d037..bb521dee 100644 --- a/src/core/commands/variableCommands.ts +++ b/src/core/commands/variableCommands.ts @@ -5,6 +5,7 @@ import { type PropertyValue, type StructValue, } from '../model/Properties'; +import { isKeptVariable, setKeptVariable } from '../model/Asset'; import { overrideSites } from '../model/instance'; import type { Project } from '../model/Project'; import type { VariableKind } from '../properties/kinds'; @@ -19,9 +20,12 @@ import { import { type Removed, removeFirst, restoreAt } from '../util/list'; import { type Command, CommandError, RenameCommand } from './Command'; import { renameVariablePorts } from './portCommands'; -import { requireVariableOwner } from './require'; +import { requireBehavior, requireVariableOwner } from './require'; -/** The variable's default value is written to `props` as well. */ +/** + * The variable's default value is written to `props` as well. On a behavior a new variable starts + * out kept between calls (#229). + */ export class AddVariableCommand implements Command { readonly label: string; readonly def: PropertyDef; @@ -42,12 +46,14 @@ export class AddVariableCommand implements Command { throw new CommandError(`Variable "${this.def.key}" already exists`); asset.variables.push(this.def); asset.props[this.def.key] = this.def.default; + if (asset.type === 'behavior') setKeptVariable(asset, this.def.key, true); } undo(project: Project): void { const asset = requireVariableOwner(project, this.assetId); asset.variables = asset.variables.filter((v) => v.key !== this.def.key); delete asset.props[this.def.key]; + if (asset.type === 'behavior') setKeptVariable(asset, this.def.key, false); } } @@ -88,7 +94,7 @@ function restoreOverrides( /** Removes a variable together with its default and every instance override. */ export class RemoveVariableCommand implements Command { readonly label: string; - private removed?: { def: Removed; value: PropertyValue | undefined }; + private removed?: { def: Removed; value: PropertyValue | undefined; kept: boolean }; private overrides: TakenOverride[] = []; constructor( @@ -102,8 +108,10 @@ export class RemoveVariableCommand implements Command { const asset = requireVariableOwner(project, this.assetId); const def = removeFirst(asset.variables, (v) => v.key === this.key); if (!def) throw new CommandError(`Variable "${this.key}" not found`); - this.removed = { def, value: asset.props[this.key] }; + const kept = asset.type === 'behavior' && isKeptVariable(asset, this.key); + this.removed = { def, value: asset.props[this.key], kept }; delete asset.props[this.key]; + if (asset.type === 'behavior') setKeptVariable(asset, this.key, false); this.overrides = takeOverrides(project, this.assetId, this.key); } @@ -112,6 +120,7 @@ export class RemoveVariableCommand implements Command { const asset = requireVariableOwner(project, this.assetId); restoreAt(asset.variables, [this.removed.def]); if (this.removed.value !== undefined) asset.props[this.key] = this.removed.value; + if (this.removed.kept && asset.type === 'behavior') setKeptVariable(asset, this.key, true); restoreOverrides(project, this.assetId, this.key, this.overrides); } } @@ -135,6 +144,9 @@ export class RenameVariableCommand extends RenameCommand { if (asset.variables.some((v) => v.key === to)) throw new CommandError(`Variable "${to}" already exists`); def.key = to; def.label = to; + // Renamed in place, so the switch keeps its spot in the list. + if (asset.type === 'behavior' && asset.keptVariables) + asset.keptVariables = asset.keptVariables.map((k) => (k === from ? to : k)); if (asset.props[from] !== undefined) { asset.props[to] = asset.props[from]!; delete asset.props[from]; @@ -148,6 +160,32 @@ export class RenameVariableCommand extends RenameCommand { } } +/** Whether a behavior's variable carries on from one `Run behavior` call to the next (#229). */ +export class SetVariableKeptCommand implements Command { + readonly label: string; + private previous = false; + + constructor( + private readonly assetId: string, + private readonly key: string, + private readonly kept: boolean, + ) { + this.label = kept ? `Keep ${key} between calls` : `Reset ${key} every call`; + } + + execute(project: Project): void { + const asset = requireBehavior(project, this.assetId); + if (!asset.variables.some((v) => v.key === this.key)) + throw new CommandError(`Variable "${this.key}" not found`); + this.previous = isKeptVariable(asset, this.key); + setKeptVariable(asset, this.key, this.kept); + } + + undo(project: Project): void { + setKeptVariable(requireBehavior(project, this.assetId), this.key, this.previous); + } +} + const INVALID_NAME = 'Variable names must start with a letter and contain only letters, digits or _'; const INVALID_FIELD = 'Field names must start with a letter and contain only letters, digits or _'; diff --git a/src/core/graph/GraphInterpreter.ts b/src/core/graph/GraphInterpreter.ts index 6b2edb6e..7118747b 100644 --- a/src/core/graph/GraphInterpreter.ts +++ b/src/core/graph/GraphInterpreter.ts @@ -43,21 +43,20 @@ function emptyFrame(graph: Graph): Frame { return { graph, outputCache: new Map(), dataCache: new Map() }; } -/** What a `Run behavior` with Keep state left behind in its callee (#229). */ -interface KeptState { - variables: PropertyBag; - states: Record; -} - /** - * The variables a kept call resumes with: today's declarations, each at the value it was left at - * unless that value is gone or no longer of the declared sort – then at its default. + * The variables a call starts with: today's declarations at their defaults, except that a kept one + * resumes at the value the last call from the same site left – unless that value is gone or no + * longer of the declared sort (#229). */ -export function resumedVariables(defaults: PropertyBag, kept: PropertyBag | undefined): PropertyBag { +export function resumedVariables( + defaults: PropertyBag, + left: PropertyBag | undefined, + kept: readonly string[], +): PropertyBag { const variables: PropertyBag = {}; for (const [key, value] of Object.entries(defaults)) { - const left = kept?.[key]; - variables[key] = left !== undefined && typeof left === typeof value ? left : value; + const was = kept.includes(key) ? left?.[key] : undefined; + variables[key] = was !== undefined && typeof was === typeof value ? was : value; } return variables; } @@ -72,8 +71,8 @@ export class GraphInterpreter { readonly nodeState = new Map>(); /** Assets run as a node so far – the graph editor highlights them from this instance's trace. */ readonly calledBehaviors = new Set(); - /** Kept calls by call site: the chain of `Run behavior` node ids that led there. */ - readonly keptCalls = new Map(); + /** What calls left in their variables, by call site: the chain of `Run behavior` node ids that led there. */ + readonly keptCalls = new Map(); private frame: Frame; private callStack: string[] = []; private callSites: string[] = []; @@ -163,7 +162,7 @@ export class GraphInterpreter { * Runs another behavior from its start node to its end node and returns what the end node was * handed. The nested graph gets its own frame and a fresh copy of its variable defaults, so it is * a call and not an inlining: nothing of it survives the return but the values it hands back – - * unless the call keeps its state, which then waits for the next call from the same site. + * and the variables it keeps, which wait for the next call from the same site. */ private call(call: CallResult, site: string, ctx: NodeContext, depth: number): Record { const program = this.lookup.behavior(call.behaviorId); @@ -172,10 +171,9 @@ export class GraphInterpreter { const start = terminalNode(program.graph, BEHAVIOR_START); if (!start) return {}; const sites = [...this.callSites, site].join('/'); - const kept = call.keep ? this.keptCalls.get(sites) : undefined; - const variables = resumedVariables(program.defaults, kept?.variables); - const states = kept?.states ?? {}; - if (call.keep) this.keptCalls.set(sites, { variables, states }); + const kept = program.kept ?? []; + const variables = resumedVariables(program.defaults, this.keptCalls.get(sites), kept); + if (kept.length > 0) this.keptCalls.set(sites, variables); const outer = this.frame; this.frame = emptyFrame(program.graph); this.frame.outputCache.set(start.id, { ...call.inputs }); @@ -185,16 +183,16 @@ export class GraphInterpreter { this.calledBehaviors.add(call.behaviorId); try { this.trace?.onNode(start.id); - // A call sees nothing of the caller's variables or states. What it does to a local machine is - // therefore not reported; a `global` machine is the run's, so moving that one is a move like - // any other. + // Fresh states every call, and nothing of the caller's variables or states. What it does to a + // local machine therefore dies with the call and is not reported; a `global` machine is the + // run's, so moving that one is a move like any other. this.follow( start, 'exec', { ...ctx, variables, - states, + states: {}, stateChanged: (machineId, from, to) => { if (machineId in ctx.shared.states) ctx.stateChanged(machineId, from, to); }, diff --git a/src/core/graph/NodeRegistry.ts b/src/core/graph/NodeRegistry.ts index e7e2e1b8..40809b8c 100644 --- a/src/core/graph/NodeRegistry.ts +++ b/src/core/graph/NodeRegistry.ts @@ -48,8 +48,6 @@ export interface RepeatResult { export interface CallResult { behaviorId: string; inputs: Record; - /** The called behavior picks up the variables and states this call site left behind (#229). */ - keep?: boolean; } export interface RunResult { diff --git a/src/core/graph/lookup.ts b/src/core/graph/lookup.ts index 4fe5f0f7..673053ad 100644 --- a/src/core/graph/lookup.ts +++ b/src/core/graph/lookup.ts @@ -6,6 +6,8 @@ export interface BehaviorProgram { name: string; graph: Graph; defaults: PropertyBag; + /** The variables a call hands on to the next call from the same site (#229); absent is none. */ + kept?: readonly string[]; } /** One state of a machine: the node id is what a port and the runtime hold, the name is the label. */ diff --git a/src/core/graph/nested.ts b/src/core/graph/nested.ts index 9bf6a322..6c32d38a 100644 --- a/src/core/graph/nested.ts +++ b/src/core/graph/nested.ts @@ -22,7 +22,6 @@ export const VARIABLES = 'Variables'; export const PROPERTIES = 'Properties'; export const BEHAVIOR_PARAM = 'behavior'; -export const KEEP_STATE_PARAM = 'keepState'; export const VARIABLES_PARAM = 'variablesAsset'; /** Whose declarations a `Variables` node reads; empty is the behavior the node sits in. */ diff --git a/src/core/graph/nodes/flow/RunBehavior.ts b/src/core/graph/nodes/flow/RunBehavior.ts index 0affcb18..188e4352 100644 --- a/src/core/graph/nodes/flow/RunBehavior.ts +++ b/src/core/graph/nodes/flow/RunBehavior.ts @@ -1,13 +1,6 @@ import type { NodeDef } from '../../NodeRegistry'; import { textOf } from '../../../model/Properties'; -import { - BEHAVIOR_END, - BEHAVIOR_PARAM, - BEHAVIOR_START, - declaredPorts, - KEEP_STATE_PARAM, - RUN_BEHAVIOR, -} from '../../nested'; +import { BEHAVIOR_END, BEHAVIOR_PARAM, BEHAVIOR_START, declaredPorts, RUN_BEHAVIOR } from '../../nested'; import { exec } from '../shared'; /** @@ -20,15 +13,12 @@ export const RunBehavior: NodeDef = { label: 'Run behavior', category: 'flow', description: - 'Runs another behavior as a step of this one, passing values in and taking results out. Its ' + - 'variables start at their defaults every time, unless Keep state is on: then each object keeps ' + - 'them for this node from one call to the next.', + 'Runs another behavior as a step of this one, passing values in and taking results out. A ' + + 'variable marked Keep in that behavior carries on from the last call of this node on this ' + + 'object; the others start at their defaults every time.', inputs: [exec()], outputs: [exec()], - params: [ - { key: BEHAVIOR_PARAM, label: 'Behavior', kind: 'asset', default: '', assetTypes: ['behavior'] }, - { key: KEEP_STATE_PARAM, label: 'Keep state', kind: 'boolean', default: false }, - ], + params: [{ key: BEHAVIOR_PARAM, label: 'Behavior', kind: 'asset', default: '', assetTypes: ['behavior'] }], derivePorts: ({ node, lookup }) => { const program = lookup.behavior(textOf(node.params[BEHAVIOR_PARAM] ?? '')); return { @@ -39,11 +29,7 @@ export const RunBehavior: NodeDef = { }; }, run: (_ctx, inputs, params) => ({ - call: { - behaviorId: textOf(params[BEHAVIOR_PARAM] ?? ''), - inputs, - keep: params[KEEP_STATE_PARAM] === true, - }, + call: { behaviorId: textOf(params[BEHAVIOR_PARAM] ?? ''), inputs }, next: 'exec', }), }; diff --git a/src/core/model/Asset.ts b/src/core/model/Asset.ts index dd25b6b6..6b89bf5a 100644 --- a/src/core/model/Asset.ts +++ b/src/core/model/Asset.ts @@ -29,13 +29,33 @@ export interface SceneAsset extends AssetBase { objects: SceneObject[]; } -/** `variables` is this asset's own property schema; `props` holds their default values. */ +/** + * `variables` is this asset's own property schema; `props` holds their default values. + * + * `keptVariables` names the variables a `Run behavior` call hands on to the next call from the same + * node on the same object (#229); the others start at their default every call. Absent keeps none, + * so a project from before the switch runs as it did – a variable added since starts out kept. + */ export interface BehaviorAsset extends AssetBase { type: 'behavior'; variables: PropertyDef[]; + keptVariables?: string[]; graph: Graph; } +/** Whether a call of this behavior hands `key` on to the next call (#229). */ +export function isKeptVariable(asset: BehaviorAsset, key: string): boolean { + return asset.keptVariables?.includes(key) === true; +} + +/** Writes the switch for one variable; an empty list is dropped rather than stored. */ +export function setKeptVariable(asset: BehaviorAsset, key: string, on: boolean): void { + const rest = (asset.keptVariables ?? []).filter((k) => k !== key); + const next = on ? [...rest, key] : rest; + if (next.length > 0) asset.keptVariables = next; + else delete asset.keptVariables; +} + /** * Declarations shared by the whole run: a `Variables` node naming this asset reads and writes one * value per key, however many graphs point at it. Nothing is placed, so nothing overrides it – diff --git a/src/core/model/graphLookup.ts b/src/core/model/graphLookup.ts index eb5054e8..7741475f 100644 --- a/src/core/model/graphLookup.ts +++ b/src/core/model/graphLookup.ts @@ -5,14 +5,19 @@ import { findAssetOf, type Project } from './Project'; /** * What a graph may see of the project around it. A behavior's `defaults` are its variable values; - * every call copies them, so a nested behavior never carries state from one call to the next. + * every call starts from a copy of them, except for the variables it keeps (#229). */ export function graphLookup(project: Project): GraphLookup { return { behavior: (assetId) => { const asset = findAssetOf(project, assetId, 'behavior'); if (!asset) return null; - return { name: asset.name, graph: asset.graph, defaults: resolveAssetProps(project, asset) }; + return { + name: asset.name, + graph: asset.graph, + defaults: resolveAssetProps(project, asset), + kept: asset.keptVariables ?? [], + }; }, machine: (assetId) => { const asset = findAssetOf(project, assetId, 'statemachine'); diff --git a/src/core/store/serialize.ts b/src/core/store/serialize.ts index 4ab064e1..8adc27c6 100644 --- a/src/core/store/serialize.ts +++ b/src/core/store/serialize.ts @@ -23,6 +23,7 @@ import type { AnimationKey, AnimationTrack, Asset, + BehaviorAsset, PixelFrame, PixelLayer, StateMachineAsset, @@ -164,14 +165,20 @@ export function deserializeProject(raw: unknown, fallbackId = ''): Project { if (type === 'scene' || type === 'container') { assets.push({ id, type, name, props, objects: normalizeObjects(a.objects, schemaOf) }); } else if (type === 'behavior') { - assets.push({ + const declared = variables.get(id) ?? []; + const behavior: BehaviorAsset = { id, type, name, props, - variables: variables.get(id) ?? [], + variables: declared, graph: normalizeGraph(a.graph), - }); + }; + // Only names that are declared, each once, in declaration order. + const kept = new Set(Array.isArray(a.keptVariables) ? a.keptVariables : []); + const keptVariables = declared.map((v) => v.key).filter((key) => kept.has(key)); + if (keptVariables.length > 0) behavior.keptVariables = keptVariables; + assets.push(behavior); } else if (type === 'variables') { const shared: VariablesAsset = { id, type, name, props, variables: variables.get(id) ?? [] }; // Scope first: a `scene` asset is never persistent, so the switch that cannot apply is dropped. diff --git a/src/editor/inspector/assetSection.ts b/src/editor/inspector/assetSection.ts index 05d3d8e0..f0e353fb 100644 --- a/src/editor/inspector/assetSection.ts +++ b/src/editor/inspector/assetSection.ts @@ -15,9 +15,16 @@ import { RemoveVariableCommand, RenameStructFieldCommand, RenameVariableCommand, + SetVariableKeptCommand, } from '../../core/commands/variableCommands'; import type { Command } from '../../core/commands/Command'; -import type { Asset, BehaviorAsset, StateMachineAsset, VariablesAsset } from '../../core/model/Asset'; +import { + type Asset, + type BehaviorAsset, + isKeptVariable, + type StateMachineAsset, + type VariablesAsset, +} from '../../core/model/Asset'; import { findAsset, type Project } from '../../core/model/Project'; import type { PropertyBag, PropertyValue } from '../../core/model/Properties'; import { readScope, readSharedScope, type Scope, scopeOf, sharedScopeOf } from '../../core/model/Scope'; @@ -56,11 +63,13 @@ export function assetSignature(project: Project, assetId: string): string { // The same for the switch that keeps shared values between runs and for the scope beside it: the // notes, the button and whether the switch is shown at all depend on the two. const kept = asset.type === 'variables' ? `|${isPersistent(asset)}|${sharedScopeOf(asset)}` : ''; + // A behavior's Keep buttons are drawn once per build and read their switch from it (#229). + const keeps = asset.type === 'behavior' ? `|keep:${[...(asset.keptVariables ?? [])].sort().join(',')}` : ''; // Vector art shows its shapes under its rows (#129), so a shape added or renamed is a new structure. const shapes = asset.type === 'vectorart' ? `|${shapeListSignature(asset)}` : ''; const values = resolveAssetProps(project, asset); const rows = schemaForAsset(project, asset).filter((def) => isRowShown(def, values)); - return rows.map(defSignature).join(',') + machine + kept + shapes; + return rows.map(defSignature).join(',') + machine + kept + keeps + shapes; } export function defSignature(def: PropertyDef): string { @@ -207,8 +216,18 @@ function buildVariables(host: InspectorHost, asset: BehaviorAsset | VariablesAss ), ), }; + const keep = + asset.type === 'behavior' + ? { + on: isKeptVariable(asset, def.key), + set: (on: boolean) => run(new SetVariableKeptCommand(asset.id, def.key, on)), + } + : undefined; const value = values[def.key] ?? def.default; - host.fields.set(def.key, buildVariable(host, def, value, assetHandlers(host, asset.id, def), { edit })); + host.fields.set( + def.key, + buildVariable(host, def, value, assetHandlers(host, asset.id, def), { edit, keep }), + ); } } diff --git a/src/editor/inspector/variableRows.ts b/src/editor/inspector/variableRows.ts index 4fcf605a..85e490d2 100644 --- a/src/editor/inspector/variableRows.ts +++ b/src/editor/inspector/variableRows.ts @@ -52,8 +52,15 @@ export interface VariableEditing { addField: (kind: StructFieldKind) => void; } +/** A behavior's switch for one variable: carried on from one `Run behavior` call to the next (#229). */ +export interface VariableKeep { + on: boolean; + set: (on: boolean) => void; +} + export interface VariableOptions { edit?: VariableEditing; + keep?: VariableKeep; /** ↺/↑ on the first row: an override covers a whole struct, so its fields share them. */ actions?: RowActions; } @@ -69,7 +76,7 @@ export function buildVariable( handlers: FieldHandlers, options: VariableOptions = {}, ): FieldRow { - const { edit, actions } = options; + const { edit, keep, actions } = options; const name = nameCell(def.key, 0, edit && ((next) => edit.rename(next))); const head = gridRow(0, markLabel(def.kind), name); let setValue: (next: PropertyValue) => void = () => {}; @@ -79,6 +86,7 @@ export function buildVariable( } else { setValue = addValue(head, createField(host, def, value, handlers)); } + if (keep) head.add(keepButton(keep)); if (edit) head.add(removeButton(edit.remove)); const refreshActions = actions ? attachRowActions(head, actions) : () => {}; host.body.add(head); @@ -95,6 +103,18 @@ export function buildVariable( }; } +/** Lit while the variable is kept; a click flips it. */ +function keepButton(keep: VariableKeep): Button { + const button = new Button('Keep', { + variant: 'ghost', + style: 'small', + toggle: true, + onClick: () => keep.set(!keep.on), + }); + button.setToggled(keep.on); + return button; +} + /** One row per field, each with its own value; returns the setter that pushes a bundle into them. */ function structFieldRows( host: InspectorHost, diff --git a/tests/core/keep-state.test.ts b/tests/core/keep-state.test.ts index 24cb9fa7..e1967b60 100644 --- a/tests/core/keep-state.test.ts +++ b/tests/core/keep-state.test.ts @@ -2,12 +2,17 @@ import { describe, expect, it } from 'vitest'; import { CreateAssetCommand } from '../../src/core/commands/assetCommands'; import { AddNodeCommand } from '../../src/core/commands/graphCommands'; import { AddPortCommand } from '../../src/core/commands/portCommands'; +import { + AddVariableCommand, + RemoveVariableCommand, + RenameVariableCommand, + SetVariableKeptCommand, +} from '../../src/core/commands/variableCommands'; import { BEHAVIOR_END, BEHAVIOR_PARAM, BEHAVIOR_START, GraphInterpreter, - KEEP_STATE_PARAM, MACHINE_PORT, RUN_BEHAVIOR, resumedVariables, @@ -16,6 +21,7 @@ import { } from '../../src/core/graph'; import type { BehaviorAsset } from '../../src/core/model/Asset'; import { graphLookup } from '../../src/core/model/graphLookup'; +import { deserializeProject, serializeProject } from '../../src/core/store/serialize'; import { fakeContext, graphEditor, @@ -28,40 +34,43 @@ import { startWorld, } from '../helpers'; -/** A behavior that counts its calls in its own `count` and hands the new count out as `out1`. */ +/** + * A behavior that counts its calls in `count` and hands the new count out as `out1`. `steps` + * counts the same way but is switched off, so it shows what an unkept variable does meanwhile. + */ function counter(fixture: ProjectFixture): BehaviorAsset { - const callee = makeBehavior(fixture, 'Counter', ['count']); + const callee = makeBehavior(fixture, 'Counter', ['count', 'steps']); + fixture.stack.execute(new SetVariableKeptCommand(callee.id, 'steps', false)); const edit = graphEditor(fixture, callee); const start = edit.addNode(BEHAVIOR_START); const end = edit.addNode(BEHAVIOR_END); fixture.stack.execute(new AddPortCommand(callee.id, end.id, { key: 'out1', kind: 'number' })); - const get = edit.variables(['count']); + fixture.stack.execute(new AddPortCommand(callee.id, end.id, { key: 'out2', kind: 'number' })); + const get = edit.variables(['count', 'steps']); const plus = edit.addNode('Add', { b: 1 }); - const set = edit.variables(['count'], { write: true }); + const plusSteps = edit.addNode('Add', { b: 1 }); + const set = edit.variables(['count', 'steps'], { write: true }); edit.wire([ [start, 'exec', set, 'exec'], [get, 'count', plus, 'a'], + [get, 'steps', plusSteps, 'a'], [plus, 'result', set, 'count'], + [plusSteps, 'result', set, 'steps'], [set, 'exec', end, 'exec'], [plus, 'result', end, 'out1'], + [plusSteps, 'result', end, 'out2'], ]); return callee; } -/** A caller that runs `callee` once per `event` through each of `sites` nodes, storing the results. */ -function callerOf( - fixture: ProjectFixture, - callee: BehaviorAsset, - keep: boolean, - sites = 1, - event = 'OnStart', -) { +/** A caller that runs `callee` once per `event` through each of `sites` nodes, storing `out1`. */ +function callerOf(fixture: ProjectFixture, callee: BehaviorAsset, sites = 1, event = 'OnStart') { const keys = Array.from({ length: sites }, (_, i) => `r${i}`); const caller = makeBehavior(fixture, 'Caller', keys); const edit = graphEditor(fixture, caller); let previous = edit.addNode(event); for (const key of keys) { - const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id, [KEEP_STATE_PARAM]: keep }); + const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }); const store = edit.variables([key], { write: true }); edit.wire([ [previous, 'exec', call, 'exec'], @@ -73,52 +82,87 @@ function callerOf( return caller; } -describe('Run behavior with Keep state (#229)', () => { - it('is off by default, so a call still starts from the defaults every time', () => { - const fixture = makeProject(); - const caller = callerOf(fixture, counter(fixture), false); - const call = caller.graph.nodes.find((n) => n.type === RUN_BEHAVIOR)!; - const added = new AddNodeCommand(caller.id, RUN_BEHAVIOR, 0, 0); - fixture.stack.execute(added); - expect(added.node.params[KEEP_STATE_PARAM]).toBe(false); +/** `out1` and `out2` of three calls from one node. */ +function threeCalls(fixture: ProjectFixture, callee: BehaviorAsset): unknown[][] { + const caller = makeBehavior(fixture, 'Caller', ['r0', 'r1']); + const edit = graphEditor(fixture, caller); + const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }); + const store = edit.variables(['r0', 'r1'], { write: true }); + edit.wire([ + [edit.addNode('OnStart'), 'exec', call, 'exec'], + [call, 'exec', store, 'exec'], + [call, 'out1', store, 'r0'], + [call, 'out2', store, 'r1'], + ]); + const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); + const ctx = fakeContext({ variables: { r0: 0, r1: 0 } }); + const seen: unknown[][] = []; + for (let i = 0; i < 3; i++) { + interpreter.fire('OnStart', ctx); + seen.push([ctx.variables.r0, ctx.variables.r1]); + } + return seen; +} - const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); - const ctx = fakeContext({ variables: { r0: 0 } }); - for (let i = 0; i < 3; i++) interpreter.fire('OnStart', ctx); - expect(ctx.variables.r0).toBe(1); - expect(call.params[KEEP_STATE_PARAM]).toBe(false); +describe('Keep on a behavior variable (#229)', () => { + it('starts on for a variable added to a behavior, and off on a Variables asset', () => { + const fixture = makeProject(); + const behavior = makeBehavior(fixture, 'B', ['count']); + expect(behavior.keptVariables).toEqual(['count']); + const shared = new CreateAssetCommand('variables', 'Shared'); + fixture.stack.execute(shared); + fixture.stack.execute(new AddVariableCommand(shared.asset.id, 'score', 'number')); + expect('keptVariables' in shared.asset).toBe(false); }); - it('resumes the callee’s variables where the last call from this node left them', () => { + it('keeps none in a project saved before the switch existed', () => { const fixture = makeProject(); - const caller = callerOf(fixture, counter(fixture), true); - const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); - const ctx = fakeContext({ variables: { r0: 0 } }); - const seen: unknown[] = []; - for (let i = 0; i < 3; i++) { - interpreter.fire('OnStart', ctx); - seen.push(ctx.variables.r0); - } - expect(seen).toEqual([1, 2, 3]); - // The caller's own bag never learns of `count`: the kept values stay the callee's. - expect('count' in ctx.variables).toBe(false); + const behavior = makeBehavior(fixture, 'Old', ['count']); + const raw = serializeProject(fixture.project) as { assets: Record[] }; + delete raw.assets.find((a) => a.id === behavior.id)!.keptVariables; + const loaded = deserializeProject(raw).assets.find((a) => a.id === behavior.id) as BehaviorAsset; + expect(loaded.keptVariables).toBeUndefined(); + }); + + it('resumes a kept variable from the last call of this node, and restarts an unkept one', () => { + const fixture = makeProject(); + // count is kept and counts 1, 2, 3; steps is not and reads 1 every time. + expect(threeCalls(fixture, counter(fixture))).toEqual([ + [1, 1], + [2, 1], + [3, 1], + ]); + }); + + it('switches off and on as one undo step each', () => { + const fixture = makeProject(); + const callee = counter(fixture); + fixture.stack.execute(new SetVariableKeptCommand(callee.id, 'count', false)); + expect(callee.keptVariables).toBeUndefined(); + fixture.stack.undo(); + expect(callee.keptVariables).toEqual(['count']); + fixture.stack.redo(); + expect(callee.keptVariables).toBeUndefined(); + expect(threeCalls(fixture, callee).map(([count]) => count)).toEqual([1, 1, 1]); }); it('keeps one state per node: two nodes calling the same behavior count on their own', () => { const fixture = makeProject(); - const caller = callerOf(fixture, counter(fixture), true, 2); + const caller = callerOf(fixture, counter(fixture), 2); const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); const ctx = fakeContext({ variables: { r0: 0, r1: 0 } }); interpreter.fire('OnStart', ctx); expect([ctx.variables.r0, ctx.variables.r1]).toEqual([1, 1]); interpreter.fire('OnStart', ctx); expect([ctx.variables.r0, ctx.variables.r1]).toEqual([2, 2]); + // The caller's own bag never learns of `count`: the kept values stay the callee's. + expect('count' in ctx.variables).toBe(false); }); it('keeps one state per object in a running world', () => { const fixture = makeSpriteScene(); const callee = counter(fixture); - const caller = callerOf(fixture, callee, true, 1, 'OnUpdate'); + const caller = callerOf(fixture, callee, 1, 'OnUpdate'); const log = graphEditor(fixture, caller).addNode('Log'); const store = caller.graph.nodes.filter((n) => n.type === 'Variables').at(-1)!; const call = caller.graph.nodes.find((n) => n.type === RUN_BEHAVIOR)!; @@ -134,22 +178,22 @@ describe('Run behavior with Keep state (#229)', () => { expect(started.world.log).toEqual(['1', '1', '2', '2', '3', '3']); }); - it('gives a kept call inside a kept call its own state per outer node', () => { + it('gives a call inside a call its own state per outer node', () => { const fixture = makeProject(); const inner = counter(fixture); - // The middle behavior just passes the counter through, keeping its call to it. + // The middle behavior just passes the counter through. const middle = makeBehavior(fixture, 'Middle'); const edit = graphEditor(fixture, middle); const start = edit.addNode(BEHAVIOR_START); const end = edit.addNode(BEHAVIOR_END); fixture.stack.execute(new AddPortCommand(middle.id, end.id, { key: 'out1', kind: 'number' })); - const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: inner.id, [KEEP_STATE_PARAM]: true }); + const call = edit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: inner.id }); edit.wire([ [start, 'exec', call, 'exec'], [call, 'exec', end, 'exec'], [call, 'out1', end, 'out1'], ]); - const caller = callerOf(fixture, middle, true, 2); + const caller = callerOf(fixture, middle, 2); const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); const ctx = fakeContext({ variables: { r0: 0, r1: 0 } }); interpreter.fire('OnStart', ctx); @@ -158,15 +202,16 @@ describe('Run behavior with Keep state (#229)', () => { expect([ctx.variables.r0, ctx.variables.r1]).toEqual([2, 2]); }); - it('keeps a local state machine of the callee along with its variables', () => { + it('starts a local state machine of the callee over every call, kept variables or not', () => { const fixture = makeProject(); const create = new CreateAssetCommand('statemachine', 'Doors'); fixture.stack.execute(create); const open = new AddNodeCommand(create.asset.id, STATE, 0, 0, { [STATE_NAME_PARAM]: 'Open' }); fixture.stack.execute(open); const machine = create.asset.id; - // No state yet → "first" and put it in Open; in Open → "open". - const callee = makeBehavior(fixture, 'Door'); + // No state yet → "first" and put it in Open; in Open → "open". The kept variable is only + // there to show that keeping one does not keep the machine along with it. + const callee = makeBehavior(fixture, 'Door', ['count']); const edit = graphEditor(fixture, callee); const start = edit.addNode(BEHAVIOR_START); const branch = edit.addNode('StateBranch', { [MACHINE_PORT]: machine }); @@ -179,40 +224,75 @@ describe('Run behavior with Keep state (#229)', () => { [first, 'exec', set, open.node.id], [branch, open.node.id, again, 'exec'], ]); - for (const keep of [false, true]) { - const caller = makeBehavior(fixture, `Caller ${keep}`); - const cEdit = graphEditor(fixture, caller); - cEdit.connect( - cEdit.addNode('OnStart'), - 'exec', - cEdit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id, [KEEP_STATE_PARAM]: keep }), - 'exec', - ); - const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); - const ctx = fakeContext(); - interpreter.fire('OnStart', ctx); - interpreter.fire('OnStart', ctx); - expect(ctx.logs).toEqual(keep ? ['first', 'open'] : ['first', 'first']); - // A local machine of the callee is none of the caller's business either way. - expect(ctx.states).toEqual({}); - } + const caller = makeBehavior(fixture, 'Caller'); + const cEdit = graphEditor(fixture, caller); + cEdit.connect( + cEdit.addNode('OnStart'), + 'exec', + cEdit.addNode(RUN_BEHAVIOR, { [BEHAVIOR_PARAM]: callee.id }), + 'exec', + ); + const interpreter = new GraphInterpreter(caller.graph, graphLookup(fixture.project)); + const ctx = fakeContext(); + interpreter.fire('OnStart', ctx); + interpreter.fire('OnStart', ctx); + expect(ctx.logs).toEqual(['first', 'first']); + // A local machine of the callee is none of the caller's business either. + expect(ctx.states).toEqual({}); }); - it('survives saving and loading', () => { + it('survives saving and loading, and a name that is not declared is dropped', () => { const fixture = makeProject(); - const caller = callerOf(fixture, counter(fixture), true); - const loaded = roundTrip(fixture.project).assets.find((a) => a.id === caller.id) as BehaviorAsset; - expect(loaded.graph.nodes.find((n) => n.type === RUN_BEHAVIOR)!.params[KEEP_STATE_PARAM]).toBe(true); + const callee = counter(fixture); + const raw = serializeProject(fixture.project) as { assets: Record[] }; + raw.assets.find((a) => a.id === callee.id)!.keptVariables = ['count', 'gone', 'count', 7]; + const loaded = deserializeProject(raw).assets.find((a) => a.id === callee.id) as BehaviorAsset; + expect(loaded.keptVariables).toEqual(['count']); + const again = roundTrip(fixture.project).assets.find((a) => a.id === callee.id) as BehaviorAsset; + expect(again.keptVariables).toEqual(['count']); + }); + + it('follows its variable through a rename, and a removal gives it back on undo', () => { + const fixture = makeProject(); + const callee = counter(fixture); + fixture.stack.execute(new RenameVariableCommand(callee.id, 'count', 'hits')); + expect(callee.keptVariables).toEqual(['hits']); + fixture.stack.execute(new RemoveVariableCommand(callee.id, 'hits')); + expect(callee.keptVariables).toBeUndefined(); + fixture.stack.undo(); + expect(callee.keptVariables).toEqual(['hits']); + fixture.stack.undo(); + expect(callee.keptVariables).toEqual(['count']); + }); + + it('is gone from Run behavior, which has no switch of its own any more', () => { + const fixture = makeProject(); + const caller = makeBehavior(fixture, 'Caller'); + const added = new AddNodeCommand(caller.id, RUN_BEHAVIOR, 0, 0); + fixture.stack.execute(added); + expect(Object.keys(added.node.params)).toEqual([BEHAVIOR_PARAM]); }); }); describe('resumedVariables', () => { it('follows today’s declarations: new ones at their default, dropped ones gone', () => { - expect(resumedVariables({ count: 0, fresh: 5 }, { count: 7, gone: 1 })).toEqual({ count: 7, fresh: 5 }); + expect(resumedVariables({ count: 0, fresh: 5 }, { count: 7, gone: 1 }, ['count', 'fresh'])).toEqual({ + count: 7, + fresh: 5, + }); + }); + + it('restarts a variable that is not kept, whatever it was left at', () => { + expect(resumedVariables({ count: 0, steps: 0 }, { count: 7, steps: 4 }, ['count'])).toEqual({ + count: 7, + steps: 0, + }); }); it('falls back to the default where the kept value is of another sort now', () => { - expect(resumedVariables({ count: 0, name: 'a' }, { count: 'seven', name: 'b' })).toEqual({ + expect( + resumedVariables({ count: 0, name: 'a' }, { count: 'seven', name: 'b' }, ['count', 'name']), + ).toEqual({ count: 0, name: 'b', }); @@ -220,7 +300,7 @@ describe('resumedVariables', () => { it('is a fresh copy of the defaults when nothing was kept yet', () => { const defaults = { count: 0 }; - const variables = resumedVariables(defaults, undefined); + const variables = resumedVariables(defaults, undefined, ['count']); expect(variables).toEqual(defaults); expect(variables).not.toBe(defaults); }); diff --git a/tests/core/node-catalog.test.ts b/tests/core/node-catalog.test.ts index faaf9a17..a7b7e4de 100644 --- a/tests/core/node-catalog.test.ts +++ b/tests/core/node-catalog.test.ts @@ -137,18 +137,12 @@ const CASES: Record = { name: 'asks for the named behavior to run, passing its inputs on', inputs: { power: 3 }, params: { behavior: 'b1' }, - call: { behaviorId: 'b1', inputs: { power: 3 }, keep: false }, - next: 'exec', - }, - { - name: 'asks for the call to keep its state when Keep state is on', - params: { behavior: 'b1', keepState: true }, - call: { behaviorId: 'b1', inputs: {}, keep: true }, + call: { behaviorId: 'b1', inputs: { power: 3 } }, next: 'exec', }, { name: 'without a behavior chosen it still continues, calling nothing that exists', - call: { behaviorId: '', inputs: {}, keep: false }, + call: { behaviorId: '', inputs: {} }, next: 'exec', }, ], diff --git a/tools/e2e/19-nested-behavior.mjs b/tools/e2e/19-nested-behavior.mjs index 6f4fb263..ce63ea9c 100644 --- a/tools/e2e/19-nested-behavior.mjs +++ b/tools/e2e/19-nested-behavior.mjs @@ -128,19 +128,37 @@ export async function nestedBehavior(t) { ); await shot('37-nested-behavior'); - // 19c-ii. Keep state (#229) is a checkbox on the node, off until clicked, and one undo step. - const keepState = async () => - (await asset('nb-caller')).graph.nodes.find((n) => n.id === 'nb-call').params.keepState; - const keepField = await json(`__gm.inspectorField('Keep state')`); - const keepBefore = await keepState(); - if (keepField) await click(keepField.x, keepField.y); - const keepClicked = await keepState(); + // 19c-ii. Keep (#229) is a switch on each variable of the called behavior, not on the node: a new + // variable starts kept, a click on its Keep button lets it go, and undo keeps it again. + const kept = async () => JSON.stringify((await asset('nb-double')).keptVariables ?? []); + const nodeKeys = Object.keys((await asset('nb-caller')).graph.nodes.find((n) => n.id === 'nb-call').params); + await t.select({ kind: 'asset', assetId: 'nb-double' }); + await clickText('+ Variable'); + await clickText('Number'); + const keptAdded = await kept(); + const keepButton = await locate('Keep'); + if (keepButton) await click(keepButton.x, keepButton.y); + const keptClicked = await kept(); + // A second click has to flip it back: a button drawn for the old switch would clear it again. + if (keepButton) await click(keepButton.x, keepButton.y); + const keptReclicked = await kept(); + await undo(); + const keptUndone = await kept(); + await undo(); await undo(); check( - 'the call has a Keep state checkbox: off, on by a click, off again after undo', - Boolean(keepField) && keepBefore !== true && keepClicked === true && (await keepState()) !== true, - `field=${JSON.stringify(keepField)} before=${keepBefore} clicked=${keepClicked} undone=${await keepState()}`, + 'a variable has a Keep switch: on when added, off by a click, on by the next, off after undo', + !nodeKeys.includes('keepState') && + keptAdded === '["var1"]' && + Boolean(keepButton) && + keptClicked === '[]' && + keptReclicked === '["var1"]' && + keptUndone === '[]' && + (await kept()) === '[]', + `node=${nodeKeys} added=${keptAdded} button=${JSON.stringify(keepButton)} clicked=${keptClicked} reclicked=${keptReclicked} undone=${keptUndone} after=${await kept()}`, ); + // Back to the call, which is where 19d looks. + await t.select({ kind: 'node', assetId: 'nb-caller', nodeId: 'nb-call', nodeIds: ['nb-call'] }); // 19d. Declaring a port on one behavior reaches the node in the other one, live. This is the // check that a port list derived from another graph is really derived and not copied.