Fix instance lookups that trusted stale data
Two regressions from the template-resolution rework, plus the harness gap that hid them: - `instanceTargetOf` decided from the shape of the id. Unpacking keeps the derived ids, so unpacked content still counted as template content. It now asks whether the object is spelled out in the holder. - The parent picker captured its holder when the row was built. Since `viewOf()` returns a derived copy, the menu listed the objects of that moment (measured: 40 in the model, 17 in the menu). It resolves on open. - `evaluate()` swallowed page exceptions, so a mutate that died half-way let every later check pass or fail for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ead649eec3
commit
f40b8a0523
5 files changed
+45
-6
No files matched your search
@@ -209,6 +209,9 @@ export function instanceTargetOf(
|
||||
holder: ObjectTree,
|
||||
objectId: string,
|
||||
): InstanceTarget | undefined {
|
||||
// Unpacking keeps the derived ids, so the shape of the id alone still looks like content of a
|
||||
// template afterwards. What decides is whether the object is spelled out in the holder.
|
||||
if (!isTemplateContent(holder, objectId)) return undefined;
|
||||
const split = splitInstanceId(objectId);
|
||||
const instance = split && holder.objects.find((o) => o.id === split.instanceId);
|
||||
if (!split || !instance) return undefined;
|
||||
|
||||
@@ -449,11 +449,14 @@ function parentRow(host: InspectorHost, holder: ObjectHolder, object: SceneObjec
|
||||
}
|
||||
const button = new Dropdown(name);
|
||||
button.onClick = () => {
|
||||
// Resolved anew on every open: `holder` is a derived copy of the scene, so one captured when
|
||||
// the row was built lists the objects of that moment, not the ones there now.
|
||||
const current = viewOf(host.state.project, holder.id) ?? holder;
|
||||
const items: MenuItem[] = [
|
||||
{ label: 'None', onSelect: () => host.state.execute(new SetParentCommand(holder.id, object.id, '')) },
|
||||
];
|
||||
for (const { object: candidate, depth } of treeOrder(holder)) {
|
||||
if (isSelfOrAncestor(holder, object.id, candidate.id)) continue;
|
||||
for (const { object: candidate, depth } of treeOrder(current)) {
|
||||
if (isSelfOrAncestor(current, object.id, candidate.id)) continue;
|
||||
const label = `${' '.repeat(depth)}${nameOf(candidate)}`;
|
||||
items.push({
|
||||
label: candidate.id === object.parentId ? `● ${label}` : label,
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
RevertOverrideToTemplateCommand,
|
||||
RevertPositionToTemplateCommand,
|
||||
} from '../../src/core/commands/applyCommands';
|
||||
import { CreateContainerCommand } from '../../src/core/commands/containerCommands';
|
||||
import { CreateContainerCommand, UnpackContainerCommand } from '../../src/core/commands/containerCommands';
|
||||
import { MoveSceneObjectCommand, SetOverrideCommand } from '../../src/core/commands/sceneObjectCommands';
|
||||
import type { ContainerAsset } from '../../src/core/model/Asset';
|
||||
import { instanceChildId, instanceTargetOf } from '../../src/core/model/instance';
|
||||
@@ -50,6 +50,15 @@ describe('template link', () => {
|
||||
// The container object itself is stored, so it has no template behind it.
|
||||
expect(instanceTargetOf(project, scene, instanceId)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('drops the link once the instance is unpacked, even though the ids stay', () => {
|
||||
const { project, stack, sceneId, sceneOf, instanceId, a } = setup();
|
||||
stack.execute(new UnpackContainerCommand(sceneId, instanceId));
|
||||
expect(sceneOf().objects.some((o) => o.id === a)).toBe(true);
|
||||
expect(instanceTargetOf(project, sceneOf(), a)).toBeUndefined();
|
||||
stack.undo();
|
||||
expect(instanceTargetOf(project, sceneOf(), a)?.asset.type).toBe('container');
|
||||
});
|
||||
});
|
||||
|
||||
describe('position apply / revert', () => {
|
||||
|
||||
@@ -393,6 +393,18 @@ export async function containers(t) {
|
||||
);
|
||||
await setTemplate(`tpl.position.x = -135`);
|
||||
await selectObject(redC);
|
||||
// Unpacking keeps the derived ids, so an id that merely *looks* like content of a template must
|
||||
// not freeze the picker any more – the inspector has to read the holder, not the id.
|
||||
const unpackedParentRow = await locate('Parent');
|
||||
await click(unpackedParentRow.x + 55, unpackedParentRow.y);
|
||||
await sleep(150);
|
||||
check(
|
||||
'unpacked content offers the Parent picker like any ordinary object',
|
||||
(await json('__gm.overlayTexts()')).includes('None'),
|
||||
JSON.stringify(await json('__gm.overlayTexts()')),
|
||||
);
|
||||
await press('Escape');
|
||||
await sleep(100);
|
||||
noticeAfter = await notice();
|
||||
const redCRow = await rowAt(redC);
|
||||
await click(redCRow.x, redCRow.y, 'right');
|
||||
|
||||
+15
-3
@@ -78,9 +78,21 @@ async function waitForChrome(port) {
|
||||
|
||||
/** The toolbox handed to a check body – everything a step needs to drive and inspect the editor. */
|
||||
function makeTools({ cdp, projectId, baseUrl, consoleLog, check, shotPrefix }) {
|
||||
const evaluate = async (expression) =>
|
||||
(await cdp.send('Runtime.evaluate', { expression, awaitPromise: true, returnByValue: true })).result
|
||||
.value;
|
||||
// A throwing expression is reported, never swallowed: a mutate that dies half-way leaves the
|
||||
// scene in a state no later check describes, and every one of them then passes or fails for the
|
||||
// wrong reason.
|
||||
const evaluate = async (expression) => {
|
||||
const { result, exceptionDetails } = await cdp.send('Runtime.evaluate', {
|
||||
expression,
|
||||
awaitPromise: true,
|
||||
returnByValue: true,
|
||||
});
|
||||
if (exceptionDetails)
|
||||
throw new Error(
|
||||
`page threw: ${exceptionDetails.exception?.description ?? exceptionDetails.text}\n in: ${expression.slice(0, 200)}`,
|
||||
);
|
||||
return result.value;
|
||||
};
|
||||
/** Evaluates an expression in the page and returns it as plain JSON; `undefined` comes back as null. */
|
||||
const json = async (expression) => JSON.parse(await evaluate(`JSON.stringify((${expression}) ?? null)`));
|
||||
const locate = async (text, last = false) => {
|
||||
|
||||
Reference in new issue
Block a user