Stop offering private variables as new animation rows (#270)

A behavior variable switched private is hidden from the placed object
and from the Properties node, but the row dialog of an animation kept
offering it. Rows that exist already stay as they are, as decided in
the issue.

- offeredDefs in src/core/model/animation.ts is what the dialog offers
  now: what the target can animate, less private variables and less
  what another row animates already. The filter moved there from
  RowDialog.ts, so it can be tested without the editor.
- animatableDefsOf is unchanged. The label and the runtime read it, so
  a row on a variable that went private keeps playing and keeps its
  label.
- Deviation from the proposal: under Change target the row's own
  variable stays on offer, private or not. The dialog would open with
  no property and a disabled button otherwise. Other private variables
  are not offered there either.
- The command does not refuse a private variable. Only the dialog does
  not offer it.

Finding, not fixed: e2e 56 fails when it runs alone, in 56a-ii from
#257 (typing "9" did not arrive: focus is BODY). With 55 before it the
module passes, so a full run does not show it. It was not measured on
HEAD without this change.

Tests:
- Gate: 198 files, 2883 tests (2882 + 1), exit 0
- e2e 55,56: 27 checks passed, 56 has +56e
- e2e 25: 31 checks passed, it uses the same dialog
- Three unit counter-probes each fail the new test
- The browser counter-probe fails 56e, mood shows up in the dialog
- e2e total 1610 is computed, not measured in a full run

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

README.md

@@ -45,7 +45,7 @@ npm run build        # writes dist/ and regenerates index.html; the folder is wh
 The browser suite and the three example games need a machine with a GPU:
 
 ```bash
-node tools/seed-e2e.mjs && node tools/e2e.mjs      # the editor, 1609 checks (GM_STEPS=20 for one step)
+node tools/seed-e2e.mjs && node tools/e2e.mjs      # the editor, 1610 checks (GM_STEPS=20 for one step)
                                                    #   every tool signs in first – see the Accounts page
 node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs
 node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs

src/core/model/animation.ts

@@ -1,14 +1,15 @@
 import { isKindWith } from '../properties/kinds';
 import { type PropertyDef, schemaFor, shaderParamRows } from '../properties/PropertySchema';
 import { shaderOf } from '../properties/shaders';
-import type {
-  AnimationAsset,
-  AnimationKey,
-  AnimationTrack,
-  Asset,
-  EasingCurve,
-  EasingDirection,
-  KeyInterpolation,
+import {
+  type AnimationAsset,
+  type AnimationKey,
+  type AnimationTrack,
+  type Asset,
+  type EasingCurve,
+  type EasingDirection,
+  isPrivateVariable,
+  type KeyInterpolation,
 } from './Asset';
 import type { AssetType } from './AssetTypes';
 import { type CurveLookup, curveValue, NO_CURVES } from './curve';
@@ -480,6 +481,31 @@ export function trackDef(
   return targetDefs(project, animation, track.target)?.find((d) => d.key === track.property);
 }
 
+/**
+ * The rows the dialog offers for the object `target` names: what it can animate, less a behavior's
+ * private variables (#270) and what another row animates already. A row on a private variable keeps
+ * playing – only a new one is not offered. `changing` is the row being aimed elsewhere: its keys
+ * stay, so only properties of its kind are offered, and its own property is, private or not.
+ */
+export function offeredDefs(
+  project: Project,
+  animation: AnimationAsset,
+  target: string,
+  changing: AnimationTrack | null = null,
+): PropertyDef[] {
+  const aimed = aimedAt(project, animation, target);
+  if (!aimed) return [];
+  const kind = changing ? (trackDef(project, animation, changing)?.kind ?? null) : null;
+  return animatableDefsOf(project, aimed.asset, aimed.bag).filter((def) => {
+    if (kind !== null && def.kind !== kind) return false;
+    const own = changing?.target === target && changing.property === def.key;
+    if (isPrivateVariable(aimed.asset, def.key) && !own) return false;
+    return !animation.tracks.some(
+      (t) => t.id !== changing?.id && t.target === target && t.property === def.key,
+    );
+  });
+}
+
 /** How the timeline names what a row aims at: the object (or the root itself) and the property. */
 export function trackLabel(project: Project, animation: AnimationAsset, track: AnimationTrack): string {
   const aimed = aimedAt(project, animation, track.target);

src/editor/animation/RowDialog.ts

@@ -2,7 +2,7 @@ import {
   AddAnimationTrackCommand,
   RetargetAnimationTrackCommand,
 } from '../../core/commands/animationCommands';
-import { animationTargets, targetDefs, trackDef } from '../../core/model/animation';
+import { animationTargets, offeredDefs } from '../../core/model/animation';
 import type { AnimationAsset, AnimationTrack } from '../../core/model/Asset';
 import { findAsset } from '../../core/model/Project';
 import { objectName } from '../../core/model/SceneObject';
@@ -96,22 +96,8 @@ export class RowDialog extends FormDialog {
     return choices;
   }
 
-  /** The properties the picked object offers, less those another row already animates. */
   private properties(): PropertyDef[] {
-    const defs = targetDefs(this.state.project, this.asset, this.target);
-    if (!defs) return [];
-    const was = this.trackKind();
-    return defs.filter((def) => {
-      if (was !== null && def.kind !== was) return false;
-      return !this.asset.tracks.some(
-        (t) => t.id !== this.track?.id && t.target === this.target && t.property === def.key,
-      );
-    });
-  }
-
-  /** The kind the row being changed has; null when its target or property is gone, so it may go anywhere. */
-  private trackKind(): string | null {
-    return this.track ? (trackDef(this.state.project, this.asset, this.track)?.kind ?? null) : null;
+    return offeredDefs(this.state.project, this.asset, this.target, this.track);
   }
 
   private openTargets(): void {

tests/core/animation.test.ts

@@ -27,12 +27,13 @@ import { CommandError } from '../../src/core/commands/Command';
 import { CreateContainerCommand } from '../../src/core/commands/containerCommands';
 import { AddNodeCommand } from '../../src/core/commands/graphCommands';
 import { SetOverrideCommand } from '../../src/core/commands/sceneObjectCommands';
-import { RenameVariableCommand } from '../../src/core/commands/variableCommands';
+import { RenameVariableCommand, SetVariablePrivateCommand } from '../../src/core/commands/variableCommands';
 import {
   animatableDefs,
   animationDuration,
   frameCount,
   isPositionRow,
+  offeredDefs,
   pauseAnimation,
   PLACED_ROOT_ID,
   playAnimation,
@@ -1226,6 +1227,33 @@ describe('behavior variables and shader parameters as rows (#104)', () => {
       f.stack.execute(new RetargetAnimationTrackCommand(walk.id, speed, moverObject, 'enabled')),
     ).toThrow(CommandError);
   });
+
+  it('does not offer a private variable for a new row, and a row on one keeps playing (#270)', () => {
+    const { f, box, mover, moverObject, walk } = rig();
+    const offered = (changing: string | null = null) =>
+      offeredDefs(
+        f.project,
+        current(f, walk),
+        moverObject,
+        changing === null ? null : rowOf(current(f, walk), changing),
+      ).map((d) => d.key);
+    const row = addRow(f, walk, 'speed', ramp, moverObject);
+    expect(offered()).toEqual(['enabled', 'runInEditor', 'runWhilePaused', 'x']);
+    f.stack.execute(new SetVariablePrivateCommand(mover.id, 'x', true));
+    f.stack.execute(new SetVariablePrivateCommand(mover.id, 'speed', true));
+    expect(offered()).toEqual(['enabled', 'runInEditor', 'runWhilePaused']);
+    // The row being changed keeps its own property on offer, and no other private one of its kind.
+    expect(offered(row)).toEqual(['speed']);
+    expect(keysOn(f, walk, moverObject)).toContain('speed');
+    expect(trackLabel(f.project, current(f, walk), rowOf(current(f, walk), row))).toBe('Mover · speed');
+    const { world, frame } = startWorld(f, { animations: true });
+    playAnimation(world.findByObjectId(box)!, walk.id);
+    frame(0.1);
+    frame(0.5);
+    expect(world.findByObjectId(moverObject)!.props.speed).toBeCloseTo(50);
+    f.stack.execute(new SetVariablePrivateCommand(mover.id, 'x', false));
+    expect(offered()).toContain('x');
+  });
 });
 
 describe('the animation nodes', () => {

tools/e2e/56-private-variables.mjs

@@ -2,11 +2,11 @@
  * Private behavior variables (#257), with the real mouse: the lock next to a variable's Keep hides it
  * from the placed object – no row, no override left – and from the `+ Property` menu of a
  * `Properties` node. A port that names it by hand is still a property of the behavior, and in a run
- * it reads the value.
+ * it reads the value. The row dialog of an animation does not offer it either (#270).
  *
  * Everything it adds carries a `pv56` id and goes again at the end.
  */
-import { MAKE_OBJECT } from './lib/scaffold.mjs';
+import { MAKE_OBJECT, openTab } from './lib/scaffold.mjs';
 
 export async function privateVariables(t) {
   const { check, json, evaluate, mutate, click, press, typeText, sleep, shot, select, undo, stop } = t;
@@ -156,12 +156,52 @@ export async function privateVariables(t) {
     JSON.stringify(logged),
   );
 
+  // 56e. The row dialog of an animation over the scene offers hp for the placed behavior, and neither
+  //      private variable (#270) – `mood` has no row, so it is not the "already has a row" rule that
+  //      hides it. The row that aimed at timer before it went private stays, label and all.
+  await mutate(`
+    const p = __gm.state.project;
+    const brain = p.assets.find((a) => a.id === 'pv56-brain');
+    brain.variables.push({ key: 'mood', label: 'mood', kind: 'number', default: 1 });
+    brain.props.mood = 1;
+    brain.privateVariables = ['timer', 'mood'];
+    p.assets.push({
+      id: 'pv56-walk',
+      type: 'animation',
+      name: 'Walk 56',
+      props: { fps: 10, playback: 'loop' },
+      root: ${JSON.stringify(sceneId)},
+      tracks: [{ id: 'pv56-row', target: 'pv56-obj', property: 'timer', keys: [] }],
+    });
+    __gm.state.select({ kind: 'asset', assetId: 'pv56-walk' });
+  `);
+  await openTab(t, 'animation');
+  await sleep(300);
+  await press_(await json('__gm.animation.addRowAt()'), '+ Row');
+  await evaluate("__gm.animation.rowDialogPick('pv56-obj', 'hp')");
+  await sleep(150);
+  const dialog = await json('__gm.animation.rowDialog()');
+  const rows = await json('__gm.animation.rows()');
+  const properties = dialog?.properties ?? [];
+  check(
+    'the row dialog offers hp and no private variable, the row on the private timer stays',
+    properties.includes('hp') &&
+      !properties.includes('mood') &&
+      !properties.includes('timer') &&
+      dialog.ok === true &&
+      JSON.stringify(rows) === JSON.stringify(['Brain 56 · timer']),
+    JSON.stringify({ properties, ok: dialog?.ok, rows }),
+  );
+  await shot('04-row-dialog');
+  await press_(await json("__gm.locate('Cancel', true)"), 'Cancel');
+
   // Clean up.
   await evaluate('__gm.state.select(null)');
   await mutate(`
     const p = __gm.state.project;
     const scene = __gm.state.activeScene;
     scene.objects = scene.objects.filter((o) => o.id !== 'pv56-obj');
-    p.assets = p.assets.filter((a) => a.id !== 'pv56-brain');
+    p.assets = p.assets.filter((a) => a.id !== 'pv56-brain' && a.id !== 'pv56-walk');
   `);
+  await openTab(t, 'scene');
 }