Let each variable of a behavior choose whether calls keep it (#229)

The first version of #229 put a Keep state switch on the Run behavior
node. The user wanted it where the variable is declared instead. Whether
a value lives on belongs to the variable, not to every call site.

- The node switch keepState is gone again. RunBehavior, CallResult and
  nested.ts are back to what they were before #229.
- BehaviorAsset.keptVariables lists the kept names. It lives on the
  asset and not in PropertyDef, because that is the schema of every
  inspector row.
- A variable added from now on starts with Keep on. Variables in
  existing projects start with Keep off, so old projects run unchanged.
- SetVariableKeptCommand flips the switch as one undo step. Add, remove
  and rename carry the list along. Loading drops names that are not
  declared.
- The interpreter keeps only the listed variables, per Run behavior
  node and per object as before. The others start at their defaults.
- Local state machines in a called behavior start empty on every call
  again, as before #229. Global and scene machines are unchanged.
- The inspector shows a Keep button in each variable row of a behavior.
  Its state is part of the asset signature, so a click redraws the row.
  Without that the button kept its old look until the inspector was
  reopened.

Found: the project Garden of Sacrifices still has keepState = true on
the call Player -> Character Movement. It is ignored now. Direction
there predates the switch, so Keep is off until set by hand.

Tests:
- ./check.sh: 163 files, 2392 tests, CHECK EXIT 0 (before 2388)
- tests/core/keep-state.test.ts rewritten, 15 tests
- e2e 19: ALL PASSED, 35 checks. 19c-ii now clicks the Keep button
  of a new variable: on when added, off by a click, on again, off
  after undo.
- Counter-checks, each fails its own test: keep list ignored (2),
  states kept (1), new variable not kept (8), rename drops the list
  (1), no Keep button (e2e 19c-ii).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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<PropertyDef>; value: PropertyValue | undefined };
+  private removed?: { def: Removed<PropertyDef>; 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 _';
 

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<string, string>;
-}
-
 /**
- * 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<string, Record<string, number>>();
   /** Assets run as a node so far – the graph editor highlights them from this instance's trace. */
   readonly calledBehaviors = new Set<string>();
-  /** Kept calls by call site: the chain of `Run behavior` node ids that led there. */
-  readonly keptCalls = new Map<string, KeptState>();
+  /** What calls left in their variables, by call site: the chain of `Run behavior` node ids that led there. */
+  readonly keptCalls = new Map<string, PropertyBag>();
   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<string, Value> {
     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);
           },

src/core/graph/NodeRegistry.ts

@@ -48,8 +48,6 @@ export interface RepeatResult {
 export interface CallResult {
   behaviorId: string;
   inputs: Record<string, Value>;
-  /** The called behavior picks up the variables and states this call site left behind (#229). */
-  keep?: boolean;
 }
 
 export interface RunResult {

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. */

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. */

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',
   }),
 };

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 –

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');

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.