Two findings from #269. A reference from a container into a placement nested in it was dead from the start, and deleting an asset left references to the objects it took along showing "(missing)". - A reference is stored the way its holder shows the object. In a container tab that is q~x for content of a nested placement q. - referenceCandidates translates it when the container is placed: in placement r, q~x becomes r~q/x. pathOfId and idOfPath in instance.ts do the mapping, objectChoices offers the same spelling the other way. The rule holds at any depth. - Storing q/x as proposed in #285 was dropped. The container tab would then have had to translate its own ids. - No migration. All projects and snapshots hold three references, none of them with ~ or /. - Create container lets a reference to content of a member placement follow it. It was left dead before. - Paste lets such a reference point into the copy. It kept pointing at the original before. Both go through the new renamedReference. - Delete asset records what every holder shows before the cascade and clears every reference to an object that is gone afterwards, with dropReferencesToGone from #269. The row shows None, undo restores the references (#286). Tests: - Gate: 198 files, 2882 tests (2873 + 9), exit 0 - Unit object-references: 26 passed (17 + 9) - e2e 1-17,57: 566 checks passed, 57 has +57i and +57j - Six unit counter-probes fail between 1 and 3 tests each - The browser counter-probe fails 57i and 57j as described in the issues - e2e total 1609 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
214218d1b2
commit
5e21d473a4
9 files changed
+295
-17
No files matched your search
@@ -15,6 +15,7 @@ import { isFontFile } from '../model/font';
|
||||
import { DEFAULT_PALETTE, PIXEL_CANVAS_DEFAULT } from '../model/pixelart';
|
||||
import { isSoundFile } from '../model/sound';
|
||||
import { removeSubtrees } from '../model/hierarchy';
|
||||
import { dropReferencesToGone, shownObjectIds } from '../model/objectRefs';
|
||||
import type { PropertyBag, PropertyValue } from '../model/Properties';
|
||||
import { objectHolders, type Project } from '../model/Project';
|
||||
import {
|
||||
@@ -30,6 +31,7 @@ import { isTextureFile } from '../model/texture';
|
||||
import { cellsWithout } from '../model/tilemap';
|
||||
import { coerceValue, defaultsFor, initialOf, initialPropsFor } from '../properties/PropertySchema';
|
||||
import { type Removed, removeById, removeFirst, restoreAt } from '../util/list';
|
||||
import { Recorder } from '../util/recorder';
|
||||
import { type Command, CommandError } from './Command';
|
||||
import { requireAsset, requirePropDef, requireStateMachine, requireVariablesAsset } from './require';
|
||||
import { SetBagValueCommand } from './SetBagValueCommand';
|
||||
@@ -212,13 +214,15 @@ export class CreateSoundCommand extends ImportFileCommand<SoundAsset> {
|
||||
/**
|
||||
* Deletes an asset and, as a cascade, every scene object referencing it, together with the objects
|
||||
* parented below those – in container templates as well as in scenes. A tile takes the cells it is
|
||||
* painted on along (#268), which would otherwise stay in the file and draw nothing.
|
||||
* painted on along (#268), which would otherwise stay in the file and draw nothing. A reference to
|
||||
* an object that goes is emptied, as when the object is removed by itself (#286).
|
||||
*/
|
||||
export class DeleteAssetCommand implements Command {
|
||||
readonly label = 'Delete asset';
|
||||
private removed?: Removed<Asset>;
|
||||
private removedObjects: { holderId: string; removed: Removed<SceneObject>[] }[] = [];
|
||||
private emptied: { layer: TilemapLayer; cells: Record<string, string> }[] = [];
|
||||
private readonly references = new Recorder();
|
||||
private previousActiveScene = '';
|
||||
|
||||
constructor(private readonly assetId: string) {}
|
||||
@@ -227,6 +231,7 @@ export class DeleteAssetCommand implements Command {
|
||||
const asset = requireAsset(project, this.assetId);
|
||||
this.removedObjects = [];
|
||||
this.emptied = [];
|
||||
const before = shownObjectIds(project);
|
||||
if (asset.type === 'tile')
|
||||
for (const map of project.assets) {
|
||||
if (map.type !== 'tilemap') continue;
|
||||
@@ -245,6 +250,7 @@ export class DeleteAssetCommand implements Command {
|
||||
this.removedObjects.push({ holderId: holder.id, removed: removeSubtrees(holder, roots) });
|
||||
}
|
||||
this.removed = removeFirst(project.assets, (a) => a.id === this.assetId);
|
||||
dropReferencesToGone(project, before, this.references);
|
||||
this.previousActiveScene = project.activeSceneId;
|
||||
if (project.activeSceneId === this.assetId) {
|
||||
const next = project.assets.find((a) => a.type === 'scene');
|
||||
@@ -254,6 +260,7 @@ export class DeleteAssetCommand implements Command {
|
||||
|
||||
undo(project: Project): void {
|
||||
if (!this.removed) return;
|
||||
this.references.revert();
|
||||
restoreAt(project.assets, [this.removed]);
|
||||
const holders = new Map(objectHolders(project).map((h) => [h.id, h]));
|
||||
for (const entry of this.removedObjects) {
|
||||
|
||||
@@ -1,8 +1,14 @@
|
||||
import { newId } from '../ids';
|
||||
import type { ContainerAsset } from '../model/Asset';
|
||||
import { findObject, removeSubtrees, subtreeIds, subtreeRoots, treeOrder } from '../model/hierarchy';
|
||||
import { instanceChildId, type InstanceEdit, isLiveInstance, materializeInstance } from '../model/instance';
|
||||
import { anchorReferences, swapReferences } from '../model/objectRefs';
|
||||
import {
|
||||
instanceChildId,
|
||||
type InstanceEdit,
|
||||
isLiveInstance,
|
||||
materializeInstance,
|
||||
pathOfId,
|
||||
} from '../model/instance';
|
||||
import { anchorReferences, renamedReference, swapReferences } from '../model/objectRefs';
|
||||
import { apply, invert, multiply, translation } from '../model/matrix';
|
||||
import type { Position } from '../model/Properties';
|
||||
import { findAsset, findObjectHolder, type Project } from '../model/Project';
|
||||
@@ -114,12 +120,13 @@ export class CreateContainerCommand implements Command {
|
||||
if (!containerParentId && groupId) container.groupId = groupId;
|
||||
this.removed = removeSubtrees(holder, [...roots]);
|
||||
// A reference to a member follows it (#258): inside the template to its template id, from the
|
||||
// rest of the holder to what the placed container shows of it.
|
||||
const templateIdOf = (ref: string) => templateIds.get(ref);
|
||||
// rest of the holder to what the placed container shows of it – content of a member placement
|
||||
// included (#285).
|
||||
const templateIdOf = (ref: string) => renamedReference(ref, templateIds);
|
||||
swapReferences(asset.objects, templateIdOf, this.references);
|
||||
const shown = (ref: string) => {
|
||||
const moved = templateIdOf(ref);
|
||||
return moved && instanceChildId(this.objectId, moved);
|
||||
return moved && instanceChildId(this.objectId, pathOfId(moved));
|
||||
};
|
||||
swapReferences(holder.objects, shown, this.references);
|
||||
project.assets.push(asset);
|
||||
|
||||
@@ -5,7 +5,7 @@ import { newId } from '../ids';
|
||||
import type { GraphClip, ObjectClip } from '../model/clipboard';
|
||||
import { findGroup } from '../model/hierarchy';
|
||||
import { isLiveInstance } from '../model/instance';
|
||||
import { swapReferences } from '../model/objectRefs';
|
||||
import { renamedReference, swapReferences } from '../model/objectRefs';
|
||||
import { Recorder } from '../util/recorder';
|
||||
import { placementRejection } from '../model/placement';
|
||||
import type { Vec2 } from '../model/Properties';
|
||||
@@ -79,7 +79,7 @@ export class PasteObjectsCommand implements Command {
|
||||
return copy;
|
||||
});
|
||||
// A reference among the copied objects points at the copy; one out of them stays (#258).
|
||||
swapReferences(pasted, (ref) => this.ids.get(ref), new Recorder());
|
||||
swapReferences(pasted, (ref) => renamedReference(ref, this.ids), new Recorder());
|
||||
holder.objects.push(...pasted);
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user