Refactor round 2: fold duplicated command and inspector code
- applyCommands sit on ObjectEditCommand with a revert hook instead of five copies of the same preamble and undo - actionIf replaces six identical Reset/Apply menu blocks, addNameField the three name rows, assetLink the two asset links - requireStructuralObject replaces ten two-line preambles and drops the duplicate holder lookup - __gm.objectsOf had no caller; the SMB remnant in e2e.sh is gone Proven: check.sh 797 tests, full e2e 633 checks, smokes 24/19/20 against the same bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
6471671f7d
commit
ce42d16657
13 files changed
+139
-223
No files matched your search
@@ -4,53 +4,48 @@
|
||||
* instance's deviation again, which is all it takes for the value to follow the template – the
|
||||
* content of a placed container is resolved from it, never copied (see `model/instance.ts`).
|
||||
*/
|
||||
import {
|
||||
captureObject,
|
||||
type InstanceTarget,
|
||||
instanceTargetOf,
|
||||
type ObjectMemo,
|
||||
restoreObject,
|
||||
} from '../model/instance';
|
||||
import type { Asset } from '../model/Asset';
|
||||
import type { InstanceTarget, ObjectRef } from '../model/instance';
|
||||
import { type Axis, type Position, type PropertyValue, setBagValue } from '../model/Properties';
|
||||
import type { Project } from '../model/Project';
|
||||
import { resolveSceneObjectProps } from '../properties/resolve';
|
||||
import { type Command, CommandError } from './Command';
|
||||
import { requireAsset, requireHolder, requireObjectRef, requirePropDef } from './require';
|
||||
import { CommandError } from './Command';
|
||||
import { requireAsset, requirePropDef } from './require';
|
||||
import { ObjectEditCommand } from './sceneObjectCommands';
|
||||
|
||||
const ALL_AXES: readonly Axis[] = ['x', 'y', 'z'];
|
||||
|
||||
function requireTarget(project: Project, holderId: string, objectId: string): InstanceTarget {
|
||||
const target = instanceTargetOf(project, requireHolder(project, holderId), objectId);
|
||||
if (!target) throw new CommandError('That object is not part of a placed container');
|
||||
return target;
|
||||
function requireTarget(ref: ObjectRef): InstanceTarget {
|
||||
if (!ref.target) throw new CommandError('That object is not part of a placed container');
|
||||
return ref.target;
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes the object's value for `key` into its asset and drops the override. For content of a
|
||||
* placed container the template's override goes too, so every instance follows the asset again.
|
||||
*/
|
||||
export class ApplyOverrideToAssetCommand implements Command {
|
||||
export class ApplyOverrideToAssetCommand extends ObjectEditCommand {
|
||||
readonly label: string;
|
||||
private asset: Asset | undefined;
|
||||
private previousAssetValue: PropertyValue | undefined;
|
||||
private previousTemplateValue: PropertyValue | undefined;
|
||||
private memo: ObjectMemo | undefined;
|
||||
private template: InstanceTarget | undefined;
|
||||
private previousTemplateValue: PropertyValue | undefined;
|
||||
|
||||
constructor(
|
||||
private readonly holderId: string,
|
||||
private readonly objectId: string,
|
||||
holderId: string,
|
||||
objectId: string,
|
||||
private readonly key: string,
|
||||
) {
|
||||
super(holderId, objectId);
|
||||
this.label = `Apply ${key} to asset`;
|
||||
}
|
||||
|
||||
execute(project: Project): void {
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
protected override apply(ref: ObjectRef, project: Project): void {
|
||||
const asset = requireAsset(project, ref.object.assetId);
|
||||
requirePropDef(asset, this.key);
|
||||
const value = resolveSceneObjectProps(asset, ref.object)[this.key];
|
||||
this.asset = asset;
|
||||
this.previousAssetValue = asset.props[this.key];
|
||||
this.memo = captureObject(ref);
|
||||
asset.props[this.key] = value!;
|
||||
ref.setOverride(this.key, undefined);
|
||||
this.template = ref.target;
|
||||
@@ -59,126 +54,102 @@ export class ApplyOverrideToAssetCommand implements Command {
|
||||
delete this.template.template.overrides[this.key];
|
||||
}
|
||||
|
||||
undo(project: Project): void {
|
||||
const asset = requireAsset(
|
||||
project,
|
||||
requireObjectRef(project, this.holderId, this.objectId).object.assetId,
|
||||
);
|
||||
protected override revert(): void {
|
||||
if (this.template) setBagValue(this.template.template.overrides, this.key, this.previousTemplateValue);
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
setBagValue(asset.props, this.key, this.previousAssetValue);
|
||||
if (this.asset) setBagValue(this.asset.props, this.key, this.previousAssetValue);
|
||||
}
|
||||
}
|
||||
|
||||
export class ApplyOverrideToContainerCommand implements Command {
|
||||
export class ApplyOverrideToContainerCommand extends ObjectEditCommand {
|
||||
readonly label: string;
|
||||
private previous: PropertyValue | undefined;
|
||||
private memo: ObjectMemo | undefined;
|
||||
private target: InstanceTarget | undefined;
|
||||
|
||||
constructor(
|
||||
private readonly holderId: string,
|
||||
private readonly objectId: string,
|
||||
holderId: string,
|
||||
objectId: string,
|
||||
private readonly key: string,
|
||||
) {
|
||||
super(holderId, objectId);
|
||||
this.label = `Apply ${key} to container`;
|
||||
}
|
||||
|
||||
execute(project: Project): void {
|
||||
const target = requireTarget(project, this.holderId, this.objectId);
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
protected override apply(ref: ObjectRef): void {
|
||||
const target = requireTarget(ref);
|
||||
this.target = target;
|
||||
this.memo = captureObject(ref);
|
||||
this.previous = target.template.overrides[this.key];
|
||||
setBagValue(target.template.overrides, this.key, ref.object.overrides[this.key]);
|
||||
// The deviation is gone the moment the template carries it, so the edit must go with it.
|
||||
ref.setOverride(this.key, target.template.overrides[this.key]);
|
||||
}
|
||||
|
||||
undo(): void {
|
||||
protected override revert(): void {
|
||||
if (this.target) setBagValue(this.target.template.overrides, this.key, this.previous);
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
}
|
||||
}
|
||||
|
||||
export class RevertOverrideToTemplateCommand implements Command {
|
||||
export class RevertOverrideToTemplateCommand extends ObjectEditCommand {
|
||||
readonly label: string;
|
||||
private memo: ObjectMemo | undefined;
|
||||
|
||||
constructor(
|
||||
private readonly holderId: string,
|
||||
private readonly objectId: string,
|
||||
holderId: string,
|
||||
objectId: string,
|
||||
private readonly key: string,
|
||||
) {
|
||||
super(holderId, objectId);
|
||||
this.label = `Revert ${key}`;
|
||||
}
|
||||
|
||||
execute(project: Project): void {
|
||||
const target = requireTarget(project, this.holderId, this.objectId);
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
this.memo = captureObject(ref);
|
||||
ref.setOverride(this.key, target.base.overrides[this.key]);
|
||||
}
|
||||
|
||||
undo(): void {
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
protected override apply(ref: ObjectRef): void {
|
||||
ref.setOverride(this.key, requireTarget(ref).base.overrides[this.key]);
|
||||
}
|
||||
}
|
||||
|
||||
/** Writes the object's local position – one axis, or all – into its container template. */
|
||||
export class ApplyPositionToContainerCommand implements Command {
|
||||
export class ApplyPositionToContainerCommand extends ObjectEditCommand {
|
||||
readonly label: string;
|
||||
private previous: Position = { x: 0, y: 0, z: 0 };
|
||||
private memo: ObjectMemo | undefined;
|
||||
private target: InstanceTarget | undefined;
|
||||
|
||||
constructor(
|
||||
private readonly holderId: string,
|
||||
private readonly objectId: string,
|
||||
holderId: string,
|
||||
objectId: string,
|
||||
private readonly axis?: Axis,
|
||||
) {
|
||||
super(holderId, objectId);
|
||||
this.label = axis ? `Apply ${axis} to container` : 'Apply position to container';
|
||||
}
|
||||
|
||||
execute(project: Project): void {
|
||||
const target = requireTarget(project, this.holderId, this.objectId);
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
protected override apply(ref: ObjectRef): void {
|
||||
const target = requireTarget(ref);
|
||||
this.target = target;
|
||||
this.memo = captureObject(ref);
|
||||
this.previous = { ...target.template.position };
|
||||
for (const axis of this.axis ? [this.axis] : ALL_AXES)
|
||||
target.template.position[axis] = ref.object.position[axis];
|
||||
ref.setPosition(ref.object.position);
|
||||
}
|
||||
|
||||
undo(): void {
|
||||
protected override revert(): void {
|
||||
if (this.target) this.target.template.position = { ...this.previous };
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
}
|
||||
}
|
||||
|
||||
export class RevertPositionToTemplateCommand implements Command {
|
||||
export class RevertPositionToTemplateCommand extends ObjectEditCommand {
|
||||
readonly label: string;
|
||||
private memo: ObjectMemo | undefined;
|
||||
|
||||
constructor(
|
||||
private readonly holderId: string,
|
||||
private readonly objectId: string,
|
||||
holderId: string,
|
||||
objectId: string,
|
||||
private readonly axis?: Axis,
|
||||
) {
|
||||
super(holderId, objectId);
|
||||
this.label = axis ? `Revert ${axis}` : 'Revert position';
|
||||
}
|
||||
|
||||
execute(project: Project): void {
|
||||
const target = requireTarget(project, this.holderId, this.objectId);
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
this.memo = captureObject(ref);
|
||||
protected override apply(ref: ObjectRef): void {
|
||||
const base = requireTarget(ref).base;
|
||||
const position = { ...ref.object.position };
|
||||
for (const axis of this.axis ? [this.axis] : ALL_AXES) position[axis] = target.base.position[axis];
|
||||
for (const axis of this.axis ? [this.axis] : ALL_AXES) position[axis] = base.position[axis];
|
||||
ref.setPosition(position);
|
||||
}
|
||||
|
||||
undo(): void {
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,7 @@ import { removeById } from '../util/list';
|
||||
import { buildAsset } from './assetCommands';
|
||||
import { type Command, CommandError } from './Command';
|
||||
import { nextAssetName } from './naming';
|
||||
import { requireAsset, requireObject, requireHolder, requireStructural } from './require';
|
||||
import { requireAsset, requireHolder, requireStructuralObject } from './require';
|
||||
|
||||
/**
|
||||
* Creates a collider asset sized like the object (sprites: width × height; anything else: schema
|
||||
@@ -25,8 +25,7 @@ export class AddColliderCommand implements Command {
|
||||
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
requireStructural(project, holder, this.targetId);
|
||||
const target = requireObject(project, this.holderId, this.targetId);
|
||||
const target = requireStructuralObject(project, holder, this.targetId);
|
||||
const targetAsset = requireAsset(project, target.assetId);
|
||||
if (targetAsset.type === 'collider') throw new CommandError('A collider cannot have a collider');
|
||||
if (isLiveInstance(project, holder, target))
|
||||
|
||||
@@ -17,7 +17,7 @@ import { removeById, type Removed, restoreAt } from '../util/list';
|
||||
import { buildAsset } from './assetCommands';
|
||||
import { type Command, CommandError } from './Command';
|
||||
import { nextAssetName } from './naming';
|
||||
import { requireObject, requireHolder, requireStructural } from './require';
|
||||
import { requireHolder, requireStructuralObject } from './require';
|
||||
|
||||
/**
|
||||
* Turns the selected objects and their descendants into a container asset placed in their stead, the
|
||||
@@ -39,8 +39,7 @@ export class CreateContainerCommand implements Command {
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
const ids = [...new Set(this.objectIds)];
|
||||
for (const id of ids) requireStructural(project, holder, id);
|
||||
const selected = ids.map((id) => requireObject(project, this.holderId, id));
|
||||
const selected = ids.map((id) => requireStructuralObject(project, holder, id));
|
||||
if (selected.length === 0) throw new CommandError('Select at least one object to create a container');
|
||||
|
||||
const worlds = selected.map((o) => worldPosition(holder, o));
|
||||
@@ -126,8 +125,7 @@ export class UnpackContainerCommand implements Command {
|
||||
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
requireStructural(project, holder, this.objectId);
|
||||
const object = requireObject(project, this.holderId, this.objectId);
|
||||
const object = requireStructuralObject(project, holder, this.objectId);
|
||||
if (!isLiveInstance(project, holder, object))
|
||||
throw new CommandError(`${object.name || 'That object'} is not a placed container`);
|
||||
const created = materializeInstance(project, holder, object);
|
||||
|
||||
@@ -50,6 +50,14 @@ export function requireStructural(project: Project, holder: ObjectTree, objectId
|
||||
throw new CommandError(`${name} comes from a container template; open the container to change it`);
|
||||
}
|
||||
|
||||
/** A stored object the caller may restructure: template content is rejected before the lookup. */
|
||||
export function requireStructuralObject(project: Project, holder: ObjectTree, objectId: string): SceneObject {
|
||||
requireStructural(project, holder, objectId);
|
||||
const object = findObject(holder, objectId);
|
||||
if (!object) throw new CommandError(`Scene object ${objectId} not found`);
|
||||
return object;
|
||||
}
|
||||
|
||||
export function requireBehavior(project: Project, assetId: string): BehaviorAsset {
|
||||
const asset = findAssetOf(project, assetId, 'behavior');
|
||||
if (!asset) throw new CommandError(`Behavior ${assetId} not found`);
|
||||
|
||||
@@ -20,15 +20,15 @@ import {
|
||||
requireObjectRef,
|
||||
requirePropDef,
|
||||
requireHolder,
|
||||
requireStructural,
|
||||
requireStructuralObject,
|
||||
} from './require';
|
||||
|
||||
/**
|
||||
* An edit to one placed object. The base captures the object before the edit and puts it back on
|
||||
* undo, so a subclass only says what changes. `applyCommands` deliberately does not use this: its
|
||||
* commands carry extra undo state whose restore order matters.
|
||||
* undo, so a subclass only says what changes – and, if it wrote somewhere else too, how to take
|
||||
* that back (`revert`).
|
||||
*/
|
||||
abstract class ObjectEditCommand implements Command {
|
||||
export abstract class ObjectEditCommand implements Command {
|
||||
abstract readonly label: string;
|
||||
private memo: ObjectMemo | undefined;
|
||||
|
||||
@@ -39,6 +39,8 @@ abstract class ObjectEditCommand implements Command {
|
||||
|
||||
protected abstract apply(ref: ObjectRef, project: Project): void;
|
||||
|
||||
protected revert(): void {}
|
||||
|
||||
execute(project: Project): void {
|
||||
const ref = requireObjectRef(project, this.holderId, this.objectId);
|
||||
this.memo = captureObject(ref);
|
||||
@@ -46,6 +48,7 @@ abstract class ObjectEditCommand implements Command {
|
||||
}
|
||||
|
||||
undo(): void {
|
||||
this.revert();
|
||||
if (this.memo) restoreObject(this.memo);
|
||||
}
|
||||
}
|
||||
@@ -71,10 +74,7 @@ export class PlaceAssetCommand implements Command {
|
||||
const asset = requireAsset(project, this.assetId);
|
||||
const rejection = placementRejection(project, holder, asset);
|
||||
if (rejection) throw new CommandError(rejection);
|
||||
if (this.parentId) {
|
||||
requireObject(project, this.holderId, this.parentId);
|
||||
requireStructural(project, holder, this.parentId);
|
||||
}
|
||||
if (this.parentId) requireStructuralObject(project, holder, this.parentId);
|
||||
const local = localPositionFor(holder, this.parentId, toPosition(this.position));
|
||||
holder.objects.push(createSceneObject(this.objectId, this.assetId, local, this.parentId));
|
||||
}
|
||||
@@ -99,10 +99,7 @@ export class RemoveSceneObjectsCommand implements Command {
|
||||
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
for (const id of this.objectIds) {
|
||||
requireStructural(project, holder, id);
|
||||
requireObject(project, this.holderId, id);
|
||||
}
|
||||
for (const id of this.objectIds) requireStructuralObject(project, holder, id);
|
||||
this.removed = removeSubtrees(holder, this.objectIds);
|
||||
}
|
||||
|
||||
@@ -226,14 +223,12 @@ export class ReorderObjectCommand implements Command {
|
||||
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
requireStructural(project, holder, this.objectId);
|
||||
const object = requireObject(project, this.holderId, this.objectId);
|
||||
const object = requireStructuralObject(project, holder, this.objectId);
|
||||
if (this.beforeId === this.objectId)
|
||||
throw new CommandError('An object cannot be moved in front of itself');
|
||||
let at = holder.objects.length;
|
||||
if (this.beforeId) {
|
||||
requireStructural(project, holder, this.beforeId);
|
||||
const before = requireObject(project, this.holderId, this.beforeId);
|
||||
const before = requireStructuralObject(project, holder, this.beforeId);
|
||||
if (before.parentId !== object.parentId)
|
||||
throw new CommandError('An object can only be reordered among its own siblings');
|
||||
at = holder.objects.indexOf(before);
|
||||
@@ -271,12 +266,10 @@ export class SetParentCommand implements Command {
|
||||
|
||||
execute(project: Project): void {
|
||||
const holder = requireHolder(project, this.holderId);
|
||||
requireStructural(project, holder, this.objectId);
|
||||
const object = requireObject(project, this.holderId, this.objectId);
|
||||
const object = requireStructuralObject(project, holder, this.objectId);
|
||||
if (this.parentId) {
|
||||
if (this.parentId === this.objectId) throw new CommandError('An object cannot be its own parent');
|
||||
requireStructural(project, holder, this.parentId);
|
||||
if (isLiveInstance(project, holder, requireObject(project, this.holderId, this.parentId)))
|
||||
if (isLiveInstance(project, holder, requireStructuralObject(project, holder, this.parentId)))
|
||||
throw new CommandError('A placed container shows its template; unpack it to put objects inside');
|
||||
if (isSelfOrAncestor(holder, this.objectId, this.parentId))
|
||||
throw new CommandError('An object cannot be parented to one of its own children');
|
||||
|
||||
Reference in new issue
Block a user