Clear references to a template object in every scene on delete (#269)
Deleting an object in a container tab only cleared references inside the template. Placements in scenes and other containers kept their instance edit and showed "(missing)" in the inspector. - shownObjectIds records which objects every holder shows before the delete. dropReferencesToGone compares that with the state after it and clears each reference that no longer resolves. - A reference is resolved from its own place with referenceCandidates, the same way the runtime does. One rule covers instance edits, nested placements and full ids like p~crate. - A cleared instance edit is dropped when it now says the same as the template's own value. Without such a value a marked None stays, as if picked by hand, and Reset hands the row back to the template. - All cleared references come back with a single undo. - References that were already dead in saved projects stay untouched. Findings, not fixed here: - A reference from container D into a nested placement q is stored as q~<id>, but the copy is named r~q/<id> in every placement of D. The runtime finds nothing, so that reference is dead from the start. - Delete asset removes placed objects by cascade but does not clear references to them. Tests: - Unit object-references: 17 passed (13 + 4) - Gate: 198 files, 2873 tests, exit 0 - e2e 1-17,57: 564 checks passed, 57 has 13 (+1 for 57h) - Five unit counter-probes and one browser counter-probe fail as expected - e2e total 1607 is computed, not measured in a full run Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0f6e628616
commit
214218d1b2
6 files changed
+249
-18
No files matched your search
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<ObjectHolder, Set<string>> {
|
||||
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<ObjectHolder, Set<string>>,
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<typeof placedTwice>, 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 });
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
`);
|
||||
}
|
||||
@@ -28,6 +28,7 @@ export const LANES = {
|
||||
vectorArt: -5100,
|
||||
pixelArt: -5400,
|
||||
eyedropper: -5800,
|
||||
containerReference: -6200,
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user