diff --git a/README.md b/README.md index ca67bed9..00e8c2e9 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, 1606 checks (GM_STEPS=20 for one step) +node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1607 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/commands/sceneObjectCommands.ts b/src/core/commands/sceneObjectCommands.ts index 40447aef..e229c595 100644 --- a/src/core/commands/sceneObjectCommands.ts +++ b/src/core/commands/sceneObjectCommands.ts @@ -7,7 +7,7 @@ import { type ObjectRef, restoreObject, } from '../model/instance'; -import { swapReferences } from '../model/objectRefs'; +import { dropReferencesToGone, shownObjectIds } from '../model/objectRefs'; import { placementRejection } from '../model/placement'; import { Recorder } from '../util/recorder'; import { @@ -101,8 +101,8 @@ export class PlaceAssetCommand implements Command { } /** - * Removes objects together with everything parented below them, as one undo step. A reference the - * rest of the holder had on one of them (#258) goes with it. + * Removes objects together with everything parented below them, as one undo step. A reference on one + * of them (#258) goes with it – in the holder, and for a container template in every placement (#269). */ export class RemoveSceneObjectsCommand implements Command { readonly label: string; @@ -119,9 +119,9 @@ export class RemoveSceneObjectsCommand implements Command { execute(project: Project): void { const holder = requireHolder(project, this.holderId); for (const id of this.objectIds) requireStructuralObject(project, holder, id); + const before = shownObjectIds(project); this.removed = removeSubtrees(holder, this.objectIds); - const gone = new Set(this.removed.map((r) => r.item.id)); - swapReferences(holder.objects, (ref) => (gone.has(ref) ? '' : undefined), this.references); + dropReferencesToGone(project, before, this.references); } undo(project: Project): void { diff --git a/src/core/model/objectRefs.ts b/src/core/model/objectRefs.ts index 59f802bf..60f06261 100644 --- a/src/core/model/objectRefs.ts +++ b/src/core/model/objectRefs.ts @@ -3,10 +3,18 @@ * empty for none. Inside a container the id is the one the template gives it, so every placement of * the container points at its own copy – which is why a reference is looked up from where it sits. */ -import { findAsset, type Project } from './Project'; +import { findAsset, type ObjectHolder, objectHolders, type Project } from './Project'; import { treeOrder, type ObjectTree } from './hierarchy'; -import { resolveObjects } from './instance'; -import { mapNested, type PropertyBag, type PropertyValue, tagged, tagOf, untag } from './Properties'; +import { instanceChildId, instanceTargetOf, isEmptyEdit, resolveObjects } from './instance'; +import { + mapNested, + type PropertyBag, + type PropertyValue, + sameValue, + tagged, + tagOf, + untag, +} from './Properties'; import { objectName, type SceneObject } from './SceneObject'; import { Recorder } from '../util/recorder'; @@ -72,22 +80,65 @@ export function objectChoices(project: Project, holder: ObjectTree, objectId: st /** * Renames the references the objects' own values and their container edits hold, as `swap` says, - * through `recorder` so an undo takes it back. + * through `recorder` so an undo takes it back. `fromId` is the object the reference sits on. */ export function swapReferences( objects: readonly SceneObject[], - swap: (ref: string) => string | undefined, + swap: (ref: string, fromId: string) => string | undefined, recorder: Recorder, ): void { - const inBag = (bag: PropertyBag) => { + const inBag = (bag: PropertyBag, fromId: string) => { for (const [key, value] of Object.entries(bag)) { - const next = withReferences(value, swap); + const next = withReferences(value, (ref) => swap(ref, fromId)); if (next !== value) recorder.set(bag, key, next); } }; for (const object of objects) { - inBag(object.overrides); - for (const edit of Object.values(object.instanceEdits)) inBag(edit.overrides); + inBag(object.overrides, object.id); + for (const [path, edit] of Object.entries(object.instanceEdits)) + inBag(edit.overrides, instanceChildId(object.id, path)); + } +} + +/** The ids every holder shows, container content included – what a reference there can name. */ +export function shownObjectIds(project: Project): Map> { + return new Map( + objectHolders(project).map((h) => [h, new Set(resolveObjects(project, h).map((o) => o.id))]), + ); +} + +/** + * Empties every reference that named an object shown `before` and gone now – in the holder it was + * removed from and in every placement of a container it was removed from (#269). A container edit + * emptied down to what its template says goes, so the placement follows the template again. + */ +export function dropReferencesToGone( + project: Project, + before: Map>, + recorder: Recorder, +): void { + const edits: { holder: ObjectHolder; object: SceneObject; path: string; was: PropertyBag }[] = []; + for (const [holder, now] of shownObjectIds(project)) { + const was = before.get(holder); + if (!was || [...was].every((id) => now.has(id))) continue; + for (const object of holder.objects) + for (const [path, edit] of Object.entries(object.instanceEdits)) + edits.push({ holder, object, path, was: { ...edit.overrides } }); + const swap = (ref: string, fromId: string) => { + const named = referenceCandidates(fromId, ref).find((id) => was.has(id)); + return named && !now.has(named) ? '' : undefined; + }; + swapReferences(holder.objects, swap, recorder); + } + // Only once every holder is done: the base of a nested placement is the edits of the one around it. + for (const { holder, object, path, was } of edits) { + const edit = object.instanceEdits[path]!; + const emptied = Object.keys(was).filter((key) => edit.overrides[key] !== was[key]); + const base = emptied.length && instanceTargetOf(project, holder, instanceChildId(object.id, path))?.base; + if (!base) continue; + for (const key of emptied) + if (sameValue(edit.overrides[key], base.overrides[key])) recorder.delete(edit.overrides, key); + if (isEmptyEdit(edit)) recorder.delete(object.instanceEdits, path); } } diff --git a/tests/core/object-references.test.ts b/tests/core/object-references.test.ts index bbf109fe..e94d7102 100644 --- a/tests/core/object-references.test.ts +++ b/tests/core/object-references.test.ts @@ -4,6 +4,7 @@ * own copy of it. */ import { describe, expect, it } from 'vitest'; +import { CreateAssetCommand } from '../../src/core/commands/assetCommands'; import { CreateContainerCommand, UnpackContainerCommand } from '../../src/core/commands/containerCommands'; import { PasteObjectsCommand } from '../../src/core/commands/pasteCommands'; import { AddPortsCommand } from '../../src/core/commands/portCommands'; @@ -17,10 +18,10 @@ import { import { AddStructFieldCommand, AddVariableCommand } from '../../src/core/commands/variableCommands'; import type { BehaviorAsset, ContainerAsset } from '../../src/core/model/Asset'; import { copyObjects } from '../../src/core/model/clipboard'; -import { instanceChildId } from '../../src/core/model/instance'; +import { instanceChildId, resolvedHolder } from '../../src/core/model/instance'; import { objectChoices, objectReference, referencedId } from '../../src/core/model/objectRefs'; import { NO_OBJECT } from '../../src/core/model/Properties'; -import { findAsset } from '../../src/core/model/Project'; +import { findAsset, findObjectHolder } from '../../src/core/model/Project'; import { makeItem } from '../../src/core/properties/listItems'; import { coerceValue, variableDef } from '../../src/core/properties/PropertySchema'; import { @@ -249,3 +250,109 @@ describe('removing and pasting', () => { expect(f.object(pasteAlone.rootIds[0]!).overrides.target).toEqual({ $object: f.crate }); }); }); + +describe('removing an object from a container template (#269)', () => { + /** + * Watcher, Crate and Barrel made into a container whose Watcher points at nothing, placed in + * "Level" and in "Level 2", each placement's Watcher pointed at its own Crate. + */ + function placedTwice() { + const f = watching(); + const barrel = place(f, f.spriteId, { x: 20, y: 0 }); + const create = new CreateContainerCommand(f.sceneId, [f.watcherId, f.crate, barrel]); + f.stack.execute(create); + const asset = findAsset(f.project, create.assetId) as ContainerAsset; + const [tWatcher, tCrate, tBarrel] = asset.objects.map((o) => o.id) as [string, string, string]; + f.stack.execute(new SetOverrideCommand(asset.id, tWatcher, 'target', NO_OBJECT)); + const level2 = new CreateAssetCommand('scene', 'Level 2'); + f.stack.execute(level2); + const other = new PlaceAssetCommand(level2.asset.id, asset.id, { x: 0, y: 0 }); + f.stack.execute(other); + const placements = [ + [f.sceneId, create.objectId], + [level2.asset.id, other.objectId], + ] as const; + for (const [sceneId, placed] of placements) + f.stack.execute( + new SetOverrideCommand(sceneId, instanceChildId(placed, tWatcher), 'target', objectReference(tCrate)), + ); + return { ...f, asset, tWatcher, tCrate, tBarrel, level2: level2.asset.id, placements }; + } + + /** What the inspector shows as `target` on the object, container content resolved. */ + function targetOf(f: SceneFixture, holderId: string, objectId: string) { + const holder = resolvedHolder(f.project, findObjectHolder(f.project, holderId)!); + return holder.objects.find((o) => o.id === objectId)!.overrides.target; + } + + /** The edit a placement holds for its Watcher. */ + function editOf(f: ReturnType, sceneId: string, placed: string) { + const holder = findObjectHolder(f.project, sceneId)!; + return holder.objects.find((o) => o.id === placed)!.instanceEdits[f.tWatcher]; + } + + it('empties the placements pointing at it in every scene, and undo brings them back', () => { + const f = placedTwice(); + f.stack.execute(new RemoveSceneObjectsCommand(f.asset.id, [f.tCrate])); + for (const [sceneId, placed] of f.placements) { + expect(targetOf(f, sceneId, instanceChildId(placed, f.tWatcher))).toEqual(NO_OBJECT); + // None is what the template says too, so the placement follows it again. + expect(editOf(f, sceneId, placed)).toBeUndefined(); + } + f.stack.undo(); + for (const [sceneId, placed] of f.placements) { + expect(targetOf(f, sceneId, instanceChildId(placed, f.tWatcher))).toEqual({ $object: f.tCrate }); + expect(editOf(f, sceneId, placed)?.overrides.target).toEqual({ $object: f.tCrate }); + } + }); + + it('leaves a placement at None where the template names something else', () => { + const f = placedTwice(); + f.stack.execute(new SetOverrideCommand(f.asset.id, f.tWatcher, 'target', objectReference(f.tBarrel))); + f.stack.execute(new RemoveSceneObjectsCommand(f.asset.id, [f.tCrate])); + const [sceneId, placed] = f.placements[0]; + expect(targetOf(f, sceneId, instanceChildId(placed, f.tWatcher))).toEqual(NO_OBJECT); + expect(editOf(f, sceneId, placed)?.overrides.target).toEqual(NO_OBJECT); + }); + + it('empties a reference from outside the placement, and leaves one to what stays alone', () => { + const f = placedTwice(); + const [, placed] = f.placements[0]; + const toCrate = place(f, f.behavior.id, { x: 0, y: 0 }); + const toBarrel = place(f, f.behavior.id, { x: 0, y: 0 }); + const crate = objectReference(instanceChildId(placed, f.tCrate)); + const barrel = objectReference(instanceChildId(placed, f.tBarrel)); + f.stack.execute(new SetOverrideCommand(f.sceneId, toCrate, 'target', crate)); + f.stack.execute(new SetOverrideCommand(f.sceneId, toBarrel, 'target', barrel)); + f.stack.execute(new RemoveSceneObjectsCommand(f.asset.id, [f.tCrate])); + expect(f.object(toCrate).overrides.target).toEqual(NO_OBJECT); + expect(f.object(toBarrel).overrides.target).toEqual(barrel); + f.stack.undo(); + expect(f.object(toCrate).overrides.target).toEqual(crate); + }); + + it('reaches placements nested in another container, template and scene alike', () => { + const f = placedTwice(); + const [, placed] = f.placements[0]; + // The placement in "Level" becomes the content of Outer and takes its edit into Outer's template. + const outer = new CreateContainerCommand(f.sceneId, [placed]); + f.stack.execute(outer); + const outerAsset = findAsset(f.project, outer.assetId) as ContainerAsset; + const inner = outerAsset.objects[0]!.id; + const outerPlaced = new PlaceAssetCommand(f.level2, outerAsset.id, { x: 0, y: 0 }); + f.stack.execute(outerPlaced); + const nestedWatcher = instanceChildId(outerPlaced.objectId, `${inner}/${f.tWatcher}`); + f.stack.execute(new SetOverrideCommand(f.level2, nestedWatcher, 'target', objectReference(f.tBarrel))); + + f.stack.execute(new RemoveSceneObjectsCommand(f.asset.id, [f.tCrate])); + expect(targetOf(f, outerAsset.id, instanceChildId(inner, f.tWatcher))).toEqual(NO_OBJECT); + expect(targetOf(f, f.level2, nestedWatcher)).toEqual({ $object: f.tBarrel }); + f.stack.execute(new RemoveSceneObjectsCommand(f.asset.id, [f.tBarrel])); + expect(targetOf(f, f.level2, nestedWatcher)).toEqual(NO_OBJECT); + + f.stack.undo(); + f.stack.undo(); + expect(targetOf(f, outerAsset.id, instanceChildId(inner, f.tWatcher))).toEqual({ $object: f.tCrate }); + expect(targetOf(f, f.level2, nestedWatcher)).toEqual({ $object: f.tBarrel }); + }); +}); diff --git a/tools/e2e/57-object-references.mjs b/tools/e2e/57-object-references.mjs index 0c76e4bc..5673c842 100644 --- a/tools/e2e/57-object-references.mjs +++ b/tools/e2e/57-object-references.mjs @@ -9,6 +9,8 @@ * a right-click and a second press of the eyedropper leave it as it was. From a list item in a * Variables tab it goes to the scene view and comes back with Take. * + * Removing an object in the Container tab empties a reference a placement of it holds (#269). + * * Everything it adds carries an `or57` id and goes again at the end. */ import { LANES, MAKE_OBJECT } from './lib/scaffold.mjs'; @@ -268,6 +270,76 @@ export async function objectReferences(t) { JSON.stringify({ tabShown, inTab, fromList, listCandidate, backIn, crowd }), ); + // 57h. A container placed in the scene, its Aim pointed at its own Crate there (#269). Removing + // the Crate in the Container tab empties that reference too: the row says None, not + // (missing). One undo brings both back. + await t.showTab('scene'); + await mutate(` + const p = __gm.state.project; + const scene = __gm.state.activeScene; + ${MAKE_OBJECT} + const sprite = p.assets.find((a) => a.type === 'sprite'); + p.assets.push({ + id: 'or57-box', + type: 'container', + name: 'Box 57', + props: {}, + objects: [make('or57-t-aim', 'or57-aim', '', 0, 'Aim 57'), make('or57-t-crate', sprite.id, '', 50, 'Crate 57')], + }); + const placed = make('or57-placed', 'or57-box', '', ${LANES.containerReference}, 'Box 57'); + placed.instanceEdits = { + 'or57-t-aim': { position: {}, overrides: { target: { $object: 'or57-t-crate' } } }, + }; + scene.objects.push(placed); + `); + const placedTarget = () => + json( + "__gm.state.activeScene.objects.find((o) => o.id === 'or57-placed')?.instanceEdits['or57-t-aim']?.overrides.target ?? null", + ); + const targetRow = async () => { + const field = await json("__gm.inspectorField('target')"); + const hits = (await json("__gm.locateAll('None')")) ?? []; + return { + none: hits.some((p) => field && p.x >= field.left && Math.abs(p.y - field.y) < 12), + missing: await inInspector('(missing)'), + crate: await inInspector('Crate 57'), + }; + }; + const placedAim = { kind: 'object', holderId: sceneId, objectId: 'or57-placed~or57-t-aim' }; + await select(placedAim); + await sleep(200); + const rowBefore = await targetRow(); + + await select({ kind: 'object', holderId: 'or57-box', objectId: 'or57-t-crate' }); + await sleep(200); + const tabThen = await json('__gm.tab()'); + await press('Delete'); + await sleep(150); + const templateLeft = await json( + "__gm.state.project.assets.find((a) => a.id === 'or57-box').objects.map((o) => o.id)", + ); + const emptied = await placedTarget(); + await t.showTab('scene'); + await select(placedAim); + await sleep(200); + const rowAfter = await targetRow(); + await shot('04-container-reference'); + await undo(); + await sleep(150); + const restored = await placedTarget(); + check( + 'removing an object in the Container tab empties the placement pointing at it, undo brings it back', + rowBefore.crate && + !rowBefore.missing && + tabThen === 'container' && + JSON.stringify(templateLeft) === JSON.stringify(['or57-t-aim']) && + JSON.stringify(emptied) === JSON.stringify({ $object: '' }) && + rowAfter.none && + !rowAfter.missing && + JSON.stringify(restored) === JSON.stringify({ $object: 'or57-t-crate' }), + JSON.stringify({ rowBefore, tabThen, templateLeft, emptied, rowAfter, restored }), + ); + // Clean up. Back to the scene view first: removing Aim 57 closes 57g's Variables tab, and the // editor then shows whatever document earlier steps left open, not the scene view. await t.showTab('scene'); @@ -276,6 +348,6 @@ export async function objectReferences(t) { const p = __gm.state.project; const scene = __gm.state.activeScene; scene.objects = scene.objects.filter((o) => !o.id.startsWith('or57-')); - p.assets = p.assets.filter((a) => a.id !== 'or57-aim'); + p.assets = p.assets.filter((a) => a.id !== 'or57-aim' && a.id !== 'or57-box'); `); } diff --git a/tools/e2e/lib/scaffold.mjs b/tools/e2e/lib/scaffold.mjs index fc8a6cfb..6a801859 100644 --- a/tools/e2e/lib/scaffold.mjs +++ b/tools/e2e/lib/scaffold.mjs @@ -28,6 +28,7 @@ export const LANES = { vectorArt: -5100, pixelArt: -5400, eyedropper: -5800, + containerReference: -6200, }; /**