diff --git a/README.md b/README.md index dd272371..5ab83beb 100644 --- a/README.md +++ b/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 diff --git a/src/core/model/animation.ts b/src/core/model/animation.ts index f38659c1..b153d15c 100644 --- a/src/core/model/animation.ts +++ b/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); diff --git a/src/editor/animation/RowDialog.ts b/src/editor/animation/RowDialog.ts index aee0c0bc..717fd2cd 100644 --- a/src/editor/animation/RowDialog.ts +++ b/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 { diff --git a/tests/core/animation.test.ts b/tests/core/animation.test.ts index e68b090e..b1d88c1d 100644 --- a/tests/core/animation.test.ts +++ b/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', () => { diff --git a/tools/e2e/56-private-variables.mjs b/tools/e2e/56-private-variables.mjs index 2724a8f3..fb6fd060 100644 --- a/tools/e2e/56-private-variables.mjs +++ b/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'); }