mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-06 11:24:00 +00:00
fix(director): keep scene results and save status accurate (#971)
- Guard against legacy pack bootstrap during unreadable storage recovery - Report failed saves honestly across captures, updates, and imports - Resolve default scene id correctly when advancing past a specific shot - Count unique scenes executed rather than queued shots in run telemetry - Return run failure status when playback throws instead of false success - Pin five regression scenarios in src/scenes/director971.test.mjs
This commit is contained in:
1 parent
3ec31e599e
commit
2b6e45e3ef
3 files changed
+233
-15
No files matched your search
@@ -32,6 +32,7 @@
|
|||||||
"scripts/build-admin-packs.mjs",
|
"scripts/build-admin-packs.mjs",
|
||||||
"scripts/qa-admin-outlines.mjs",
|
"scripts/qa-admin-outlines.mjs",
|
||||||
"src/data/inputOwnership.test.mjs",
|
"src/data/inputOwnership.test.mjs",
|
||||||
|
"src/scenes/director971.test.mjs",
|
||||||
"docs/CODE-BOUNDARIES.md",
|
"docs/CODE-BOUNDARIES.md",
|
||||||
"src/main.js",
|
"src/main.js",
|
||||||
"docs/APPLICATION.md",
|
"docs/APPLICATION.md",
|
||||||
|
|||||||
+22
-15
@@ -183,6 +183,7 @@ export class SceneDirector {
|
|||||||
...this._presentation,
|
...this._presentation,
|
||||||
hasRun: !!this._lastRunJson,
|
hasRun: !!this._lastRunJson,
|
||||||
}));
|
}));
|
||||||
|
if (!this._storageReadError) {
|
||||||
this._bootstrapLegacyShotPacks();
|
this._bootstrapLegacyShotPacks();
|
||||||
// Upgrade only already-installed packs; unrelated Scenes are untouched.
|
// Upgrade only already-installed packs; unrelated Scenes are untouched.
|
||||||
for (const scene of this._project.scenes) {
|
for (const scene of this._project.scenes) {
|
||||||
@@ -200,6 +201,7 @@ export class SceneDirector {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
this._initUI();
|
this._initUI();
|
||||||
this._visibilityUnsubscribe =
|
this._visibilityUnsubscribe =
|
||||||
this.dataManager.subscribeVisibilityRequests?.((change) => {
|
this.dataManager.subscribeVisibilityRequests?.((change) => {
|
||||||
@@ -316,13 +318,14 @@ export class SceneDirector {
|
|||||||
this._toastStorageError(
|
this._toastStorageError(
|
||||||
'Scene not saved — existing saved project could not be read. Export edits or import a valid file.',
|
'Scene not saved — existing saved project could not be read. Export edits or import a valid file.',
|
||||||
);
|
);
|
||||||
return;
|
return false;
|
||||||
}
|
}
|
||||||
this._project.updatedAt = new Date().toISOString();
|
this._project.updatedAt = new Date().toISOString();
|
||||||
try {
|
try {
|
||||||
const payload = JSON.stringify(this._project);
|
const payload = JSON.stringify(this._project);
|
||||||
parseSceneDocument(payload);
|
parseSceneDocument(payload);
|
||||||
localStorage.setItem(STORAGE_KEY, payload);
|
localStorage.setItem(STORAGE_KEY, payload);
|
||||||
|
return true;
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
// Private browsing / block-all-cookies / quota-exceeded throws here. The
|
// Private browsing / block-all-cookies / quota-exceeded throws here. The
|
||||||
// in-memory project stays usable this session, but persistence failed —
|
// in-memory project stays usable this session, but persistence failed —
|
||||||
@@ -336,6 +339,7 @@ export class SceneDirector {
|
|||||||
? `Scene not saved — ${e.message}`
|
? `Scene not saved — ${e.message}`
|
||||||
: undefined,
|
: undefined,
|
||||||
);
|
);
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -889,12 +893,12 @@ export class SceneDirector {
|
|||||||
}
|
}
|
||||||
this._selectedSceneId = scene.id;
|
this._selectedSceneId = scene.id;
|
||||||
this._selectedShotId = appendedShots[0]?.id || this._selectedShotId;
|
this._selectedShotId = appendedShots[0]?.id || this._selectedShotId;
|
||||||
this._saveProject();
|
const saved = this._saveProject();
|
||||||
if (render) {
|
if (render) {
|
||||||
this._renderSceneSelect();
|
this._renderSceneSelect();
|
||||||
this._renderShotList();
|
this._renderShotList();
|
||||||
}
|
}
|
||||||
if (announce) {
|
if (announce && saved) {
|
||||||
this._updateStatus(
|
this._updateStatus(
|
||||||
marker
|
marker
|
||||||
? `Updated ${patchedShotCount} ${patchedShotCount === 1 ? 'shot' : 'shots'}: ${recipe.title}`
|
? `Updated ${patchedShotCount} ${patchedShotCount === 1 ? 'shot' : 'shots'}: ${recipe.title}`
|
||||||
@@ -939,9 +943,9 @@ export class SceneDirector {
|
|||||||
|
|
||||||
scene.shots.push(shot);
|
scene.shots.push(shot);
|
||||||
this._selectedShotId = shot.id;
|
this._selectedShotId = shot.id;
|
||||||
this._saveProject();
|
const saved = this._saveProject();
|
||||||
this._shotOutcome('shot-captured', scene, shot);
|
this._shotOutcome('shot-captured', scene, shot);
|
||||||
this._updateStatus(`Captured: ${scene.title} / ${shot.title}`);
|
if (saved) this._updateStatus(`Captured: ${scene.title} / ${shot.title}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -967,9 +971,9 @@ export class SceneDirector {
|
|||||||
shot.visual = this.styleManager.getVisualState();
|
shot.visual = this.styleManager.getVisualState();
|
||||||
shot.layers = this._captureLayerStates();
|
shot.layers = this._captureLayerStates();
|
||||||
|
|
||||||
this._saveProject();
|
const saved = this._saveProject();
|
||||||
this._shotOutcome('shot-updated', scene, shot);
|
this._shotOutcome('shot-updated', scene, shot);
|
||||||
this._updateStatus(`Updated: ${scene.title} / ${shot.title}`);
|
if (saved) this._updateStatus(`Updated: ${scene.title} / ${shot.title}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -1686,13 +1690,13 @@ export class SceneDirector {
|
|||||||
) {
|
) {
|
||||||
if (this._running) return { started: false, reason: 'already-running' };
|
if (this._running) return { started: false, reason: 'already-running' };
|
||||||
|
|
||||||
let queue = this._buildPlaybackQueue(
|
const startSceneId =
|
||||||
sceneId || this._selectedSceneId || this._project.scenes[0]?.id,
|
sceneId || this._selectedSceneId || this._project.scenes[0]?.id;
|
||||||
{ single },
|
let queue = this._buildPlaybackQueue(startSceneId, { single });
|
||||||
);
|
|
||||||
if (afterShotId !== null) {
|
if (afterShotId !== null) {
|
||||||
const index = queue.findIndex(
|
const index = queue.findIndex(
|
||||||
({ scene, shot }) => scene.id === sceneId && shot.id === afterShotId,
|
({ scene, shot }) =>
|
||||||
|
scene.id === startSceneId && shot.id === afterShotId,
|
||||||
);
|
);
|
||||||
if (index < 0) return { started: false, reason: 'shot-not-found' };
|
if (index < 0) return { started: false, reason: 'shot-not-found' };
|
||||||
queue = queue.slice(index + 1);
|
queue = queue.slice(index + 1);
|
||||||
@@ -1764,7 +1768,7 @@ export class SceneDirector {
|
|||||||
title: 'Editable Scene Run',
|
title: 'Editable Scene Run',
|
||||||
startedAt: new Date().toISOString(),
|
startedAt: new Date().toISOString(),
|
||||||
estimatedDurationSec,
|
estimatedDurationSec,
|
||||||
scenesRun: queue.length,
|
scenesRun: new Set(queue.map(({ scene }) => scene.id)).size,
|
||||||
events: [],
|
events: [],
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -1772,6 +1776,7 @@ export class SceneDirector {
|
|||||||
this._logEvent('scene_run_start', { count: queue.length });
|
this._logEvent('scene_run_start', { count: queue.length });
|
||||||
this._setPlaybackKeyboardEnabled(true);
|
this._setPlaybackKeyboardEnabled(true);
|
||||||
|
|
||||||
|
let runResult = { started: true, shots: queue.length };
|
||||||
try {
|
try {
|
||||||
await playSceneQueue(queue, {
|
await playSceneQueue(queue, {
|
||||||
token,
|
token,
|
||||||
@@ -1786,9 +1791,11 @@ export class SceneDirector {
|
|||||||
this._logEvent('scene_run_error', {
|
this._logEvent('scene_run_error', {
|
||||||
message: error.message || 'unknown error',
|
message: error.message || 'unknown error',
|
||||||
});
|
});
|
||||||
|
runResult = { started: false, reason: 'run-failed' };
|
||||||
} finally {
|
} finally {
|
||||||
this._finishRun();
|
this._finishRun();
|
||||||
}
|
}
|
||||||
|
return runResult;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -2014,9 +2021,9 @@ export class SceneDirector {
|
|||||||
this._selectedShotId =
|
this._selectedShotId =
|
||||||
selection?.shotId || project.scenes[0]?.shots[0]?.id || null;
|
selection?.shotId || project.scenes[0]?.shots[0]?.id || null;
|
||||||
this._loadedSceneId = null;
|
this._loadedSceneId = null;
|
||||||
this._saveProject();
|
const saved = this._saveProject();
|
||||||
this._publish({ type: 'project-imported', project });
|
this._publish({ type: 'project-imported', project });
|
||||||
this._updateStatus(`Imported ${file.name}`);
|
if (saved) this._updateStatus(`Imported ${file.name}`);
|
||||||
return true;
|
return true;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (
|
if (
|
||||||
|
|||||||
@@ -0,0 +1,210 @@
|
|||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import test from 'node:test';
|
||||||
|
import { SceneDirector } from './director.js';
|
||||||
|
|
||||||
|
const STORAGE_KEY = 'godsEyeView.sceneProject.v2';
|
||||||
|
const CHECKPOINT_KEY = 'godsEyeView.sceneProject.checkpoint.v1';
|
||||||
|
const pose = {
|
||||||
|
lat: 10,
|
||||||
|
lon: 20,
|
||||||
|
alt: 500000,
|
||||||
|
heading: 0,
|
||||||
|
pitch: -40,
|
||||||
|
roll: 0,
|
||||||
|
};
|
||||||
|
const shot = (id) => ({
|
||||||
|
id,
|
||||||
|
title: id,
|
||||||
|
durationSec: 0.2,
|
||||||
|
holdSec: 0,
|
||||||
|
camera: pose,
|
||||||
|
visual: { style: 'normal' },
|
||||||
|
layers: {},
|
||||||
|
});
|
||||||
|
const project = JSON.stringify({
|
||||||
|
version: 3,
|
||||||
|
scenes: [{ id: 's', title: 'S', shots: [shot('a'), shot('b')] }],
|
||||||
|
});
|
||||||
|
|
||||||
|
function classList() {
|
||||||
|
return {
|
||||||
|
add() {},
|
||||||
|
remove() {},
|
||||||
|
toggle() {},
|
||||||
|
contains: () => false,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function installDocument() {
|
||||||
|
globalThis.document = {
|
||||||
|
getElementById: () => null,
|
||||||
|
createElement: () => ({
|
||||||
|
classList: classList(),
|
||||||
|
style: {},
|
||||||
|
dataset: {},
|
||||||
|
appendChild() {},
|
||||||
|
remove() {},
|
||||||
|
click() {},
|
||||||
|
addEventListener() {},
|
||||||
|
removeEventListener() {},
|
||||||
|
setAttribute() {},
|
||||||
|
}),
|
||||||
|
addEventListener() {},
|
||||||
|
removeEventListener() {},
|
||||||
|
body: { classList: classList(), appendChild() {} },
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function make(initial, { styleOverrides = {}, dataOverrides = {} } = {}) {
|
||||||
|
const stored = new Map(Object.entries(initial));
|
||||||
|
const storage = {
|
||||||
|
stored,
|
||||||
|
full: false,
|
||||||
|
getItem: (key) => stored.get(key) ?? null,
|
||||||
|
setItem(key, value) {
|
||||||
|
if (this.full)
|
||||||
|
throw new DOMException('quota exceeded', 'QuotaExceededError');
|
||||||
|
stored.set(key, String(value));
|
||||||
|
},
|
||||||
|
removeItem: (key) => stored.delete(key),
|
||||||
|
};
|
||||||
|
globalThis.localStorage = storage;
|
||||||
|
const viewer = {
|
||||||
|
camera: {
|
||||||
|
flyTo: (options) => Promise.resolve().then(() => options.complete?.()),
|
||||||
|
cancelFlight() {},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const style = {
|
||||||
|
runImmediateNavigation: (_name, run) => run(),
|
||||||
|
getCameraState: () => pose,
|
||||||
|
getVisualState: () => ({ style: 'normal' }),
|
||||||
|
setRecordingMode() {},
|
||||||
|
applyVisualState: async () => true,
|
||||||
|
...styleOverrides,
|
||||||
|
};
|
||||||
|
const data = {
|
||||||
|
getAll: () => [],
|
||||||
|
getLayerParams: () => null,
|
||||||
|
setEnabled: async () => true,
|
||||||
|
setLayerParams: () => true,
|
||||||
|
...dataOverrides,
|
||||||
|
};
|
||||||
|
return { director: new SceneDirector(viewer, style, data), storage };
|
||||||
|
}
|
||||||
|
|
||||||
|
function observeStatus(director) {
|
||||||
|
let status = '';
|
||||||
|
director.subscribe(({ state }) => {
|
||||||
|
status = state.status;
|
||||||
|
});
|
||||||
|
return () => status;
|
||||||
|
}
|
||||||
|
|
||||||
|
test('SceneDirector keeps recovery data and failed-save status accurate (#971)', async (t) => {
|
||||||
|
const priorDocument = globalThis.document;
|
||||||
|
const priorStorage = globalThis.localStorage;
|
||||||
|
const priorWarn = console.warn;
|
||||||
|
console.warn = () => {};
|
||||||
|
installDocument();
|
||||||
|
t.after(() => {
|
||||||
|
console.warn = priorWarn;
|
||||||
|
if (priorDocument === undefined) delete globalThis.document;
|
||||||
|
else globalThis.document = priorDocument;
|
||||||
|
if (priorStorage === undefined) delete globalThis.localStorage;
|
||||||
|
else globalThis.localStorage = priorStorage;
|
||||||
|
});
|
||||||
|
|
||||||
|
let made = make({
|
||||||
|
[STORAGE_KEY]: '{"version":99,"scenes":[]}',
|
||||||
|
[CHECKPOINT_KEY]: 'EARLIER-CHECKPOINT',
|
||||||
|
});
|
||||||
|
assert.equal(
|
||||||
|
made.storage.stored.get(STORAGE_KEY),
|
||||||
|
'{"version":99,"scenes":[]}',
|
||||||
|
);
|
||||||
|
assert.equal(made.storage.stored.get(CHECKPOINT_KEY), 'EARLIER-CHECKPOINT');
|
||||||
|
await made.director.destroy();
|
||||||
|
|
||||||
|
made = make({ [STORAGE_KEY]: project });
|
||||||
|
const status = observeStatus(made.director);
|
||||||
|
made.storage.full = true;
|
||||||
|
|
||||||
|
made.director.captureShot();
|
||||||
|
assert.match(status(), /^Scene not saved/);
|
||||||
|
|
||||||
|
made.director.updateSelectedShot();
|
||||||
|
assert.match(status(), /^Scene not saved/);
|
||||||
|
|
||||||
|
assert.equal(
|
||||||
|
await made.director.importProjectFile(
|
||||||
|
{ name: 'fixture.json' },
|
||||||
|
{ prepared: { project: JSON.parse(project), assets: [] } },
|
||||||
|
),
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
assert.match(status(), /^Scene not saved/);
|
||||||
|
await made.director.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('SceneDirector returns results, resolves implicit ids, and counts scenes', async (t) => {
|
||||||
|
const priorDocument = globalThis.document;
|
||||||
|
const priorStorage = globalThis.localStorage;
|
||||||
|
installDocument();
|
||||||
|
t.after(() => {
|
||||||
|
if (priorDocument === undefined) delete globalThis.document;
|
||||||
|
else globalThis.document = priorDocument;
|
||||||
|
if (priorStorage === undefined) delete globalThis.localStorage;
|
||||||
|
else globalThis.localStorage = priorStorage;
|
||||||
|
});
|
||||||
|
|
||||||
|
const made = make({ [STORAGE_KEY]: project });
|
||||||
|
const options = { single: true, preview: false };
|
||||||
|
assert.deepEqual(await made.director.startScene('s', options), {
|
||||||
|
started: true,
|
||||||
|
shots: 2,
|
||||||
|
});
|
||||||
|
assert.equal(made.director._lastRun.scenesRun, 1);
|
||||||
|
assert.deepEqual(
|
||||||
|
await made.director.startScene(undefined, {
|
||||||
|
...options,
|
||||||
|
afterShotId: 'a',
|
||||||
|
}),
|
||||||
|
{ started: true, shots: 1 },
|
||||||
|
);
|
||||||
|
await made.director.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('SceneDirector does not report caught failures as success', async (t) => {
|
||||||
|
const priorDocument = globalThis.document;
|
||||||
|
const priorStorage = globalThis.localStorage;
|
||||||
|
const priorWarn = console.warn;
|
||||||
|
console.warn = () => {};
|
||||||
|
installDocument();
|
||||||
|
t.after(() => {
|
||||||
|
console.warn = priorWarn;
|
||||||
|
if (priorDocument === undefined) delete globalThis.document;
|
||||||
|
else globalThis.document = priorDocument;
|
||||||
|
if (priorStorage === undefined) delete globalThis.localStorage;
|
||||||
|
else globalThis.localStorage = priorStorage;
|
||||||
|
});
|
||||||
|
|
||||||
|
const made = make(
|
||||||
|
{ [STORAGE_KEY]: project },
|
||||||
|
{
|
||||||
|
styleOverrides: {
|
||||||
|
applyVisualState: async () => {
|
||||||
|
throw new Error('visual failed');
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
assert.deepEqual(
|
||||||
|
await made.director.startScene('s', {
|
||||||
|
single: true,
|
||||||
|
preview: false,
|
||||||
|
}),
|
||||||
|
{ started: false, reason: 'run-failed' },
|
||||||
|
);
|
||||||
|
await made.director.destroy();
|
||||||
|
});
|
||||||
Reference in new issue
Block a user