diff --git a/changelog.d/1135-director-save-status-run-results.md b/changelog.d/1135-director-save-status-run-results.md new file mode 100644 index 000000000..aa3c6f0e6 --- /dev/null +++ b/changelog.d/1135-director-save-status-run-results.md @@ -0,0 +1,5 @@ +- Scenes keep the "Scene not saved" status when a capture, update, append or + import cannot be saved, and an unreadable saved project no longer overwrites + the recovery checkpoint. `startScene()` resolves to its documented result, + `afterShotId` works without an explicit scene, and `scenesRun` counts the + distinct scenes queued (djabhi31, #1135). diff --git a/src/scenes/director.js b/src/scenes/director.js index 1a6fed403..000c02681 100644 --- a/src/scenes/director.js +++ b/src/scenes/director.js @@ -189,20 +189,22 @@ export class SceneDirector { ...this._presentation, hasRun: !!this._lastRunJson, })); - this._bootstrapLegacyShotPacks(); - // Upgrade only already-installed packs; unrelated Scenes are untouched. - for (const scene of this._project.scenes) { - for (const marker of [...(scene.appliedShotPacks || [])]) { - const recipe = getSceneAppendRecipeById(marker.id); - if ( - recipe?.expansionFromVersion && - marker.version > 0 && - marker.version <= recipe.expansionFromVersion - ) { - this.appendShotPack(scene.id, marker.id, { - render: false, - announce: false, - }); + if (!this._storageReadError) { + this._bootstrapLegacyShotPacks(); + // Upgrade only already-installed packs; unrelated Scenes are untouched. + for (const scene of this._project.scenes) { + for (const marker of [...(scene.appliedShotPacks || [])]) { + const recipe = getSceneAppendRecipeById(marker.id); + if ( + recipe?.expansionFromVersion && + marker.version > 0 && + marker.version <= recipe.expansionFromVersion + ) { + this.appendShotPack(scene.id, marker.id, { + render: false, + announce: false, + }); + } } } } @@ -324,13 +326,14 @@ export class SceneDirector { '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(); try { const payload = JSON.stringify(this._project); parseSceneDocument(payload); localStorage.setItem(STORAGE_KEY, payload); + return true; } catch (e) { // Private browsing / block-all-cookies / quota-exceeded throws here. The // in-memory project stays usable this session, but persistence failed — @@ -351,6 +354,7 @@ export class SceneDirector { }) : undefined, ); + return false; } } @@ -923,12 +927,12 @@ export class SceneDirector { } this._selectedSceneId = scene.id; this._selectedShotId = appendedShots[0]?.id || this._selectedShotId; - this._saveProject(); + const saved = this._saveProject(); if (render) { this._renderSceneSelect(); this._renderShotList(); } - if (announce) { + if (announce && saved) { this._updateStatus( marker ? tn( @@ -984,14 +988,16 @@ export class SceneDirector { scene.shots.push(shot); this._selectedShotId = shot.id; - this._saveProject(); + const saved = this._saveProject(); this._shotOutcome('shot-captured', scene, shot); - this._updateStatus( - t('Captured: {scene} / {shot}', { - scene: displaySceneTitle(scene), - shot: displayShotTitle(shot.title, shot, scene), - }), - ); + if (saved) { + this._updateStatus( + t('Captured: {scene} / {shot}', { + scene: displaySceneTitle(scene), + shot: displayShotTitle(shot.title, shot, scene), + }), + ); + } } /** @@ -1017,14 +1023,16 @@ export class SceneDirector { shot.visual = this.styleManager.getVisualState(); shot.layers = this._captureLayerStates(); - this._saveProject(); + const saved = this._saveProject(); this._shotOutcome('shot-updated', scene, shot); - this._updateStatus( - t('Updated: {scene} / {shot}', { - scene: displaySceneTitle(scene), - shot: displayShotTitle(shot.title, shot, scene), - }), - ); + if (saved) { + this._updateStatus( + t('Updated: {scene} / {shot}', { + scene: displaySceneTitle(scene), + shot: displayShotTitle(shot.title, shot, scene), + }), + ); + } } /** @@ -1755,13 +1763,13 @@ export class SceneDirector { ) { if (this._running) return { started: false, reason: 'already-running' }; - let queue = this._buildPlaybackQueue( - sceneId || this._selectedSceneId || this._project.scenes[0]?.id, - { single }, - ); + const startSceneId = + sceneId || this._selectedSceneId || this._project.scenes[0]?.id; + let queue = this._buildPlaybackQueue(startSceneId, { single }); if (afterShotId !== null) { 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' }; queue = queue.slice(index + 1); @@ -1834,7 +1842,7 @@ export class SceneDirector { title: 'Editable Scene Run', startedAt: new Date().toISOString(), estimatedDurationSec, - scenesRun: queue.length, + scenesRun: new Set(queue.map(({ scene }) => scene.id)).size, events: [], }; @@ -1842,6 +1850,7 @@ export class SceneDirector { this._logEvent('scene_run_start', { count: queue.length }); this._setPlaybackKeyboardEnabled(true); + let runResult = { started: true, shots: queue.length }; try { await playSceneQueue(queue, { token, @@ -1860,9 +1869,11 @@ export class SceneDirector { this._logEvent('scene_run_error', { message: error.message || 'unknown error', }); + runResult = { started: false, reason: 'run-failed' }; } finally { this._finishRun(); } + return runResult; } /** @@ -2090,9 +2101,9 @@ export class SceneDirector { this._selectedShotId = selection?.shotId || project.scenes[0]?.shots[0]?.id || null; this._loadedSceneId = null; - this._saveProject(); + const saved = this._saveProject(); this._publish({ type: 'project-imported', project }); - this._updateStatus(t('Imported {file}', { file: file.name })); + if (saved) this._updateStatus(t('Imported {file}', { file: file.name })); return true; } catch (error) { if ( diff --git a/src/scenes/director.test.mjs b/src/scenes/director.test.mjs index 95552025f..f72b43dc7 100644 --- a/src/scenes/director.test.mjs +++ b/src/scenes/director.test.mjs @@ -601,8 +601,12 @@ const PROJECT_FIXTURE = { }], }; -/** Stub the browser globals the director touches, headlessly. */ -function installSceneRuntime(project = PROJECT_FIXTURE) { +/** + * Stub the browser globals the director touches, headlessly. `storage` replaces + * the seeded entries (key -> raw string); setting `localStorage.full` makes + * every write throw a quota error. + */ +function installSceneRuntime(project = PROJECT_FIXTURE, { storage } = {}) { const originalDocument = globalThis.document; const originalLocalStorage = globalThis.localStorage; const noopClassList = { add() {}, remove() {}, toggle() {}, contains: () => false }; @@ -614,10 +618,17 @@ function installSceneRuntime(project = PROJECT_FIXTURE) { removeEventListener() {}, body: { classList: noopClassList, appendChild() {} }, }; - const stored = new Map([['godsEyeView.sceneProject.v2', JSON.stringify(project)]]); + const stored = new Map(storage + ? Object.entries(storage) + : [['godsEyeView.sceneProject.v2', JSON.stringify(project)]]); globalThis.localStorage = { + stored, + full: false, getItem: (key) => stored.get(key) ?? null, - setItem: (key, value) => stored.set(key, value), + setItem(key, value) { + if (this.full) throw new DOMException('quota exceeded', 'QuotaExceededError'); + stored.set(key, value); + }, removeItem: (key) => stored.delete(key), }; @@ -718,7 +729,8 @@ async function settle(turns = 8) { /** Build a director over doubles, with the fixture project loaded. */ function makeDirector(options = {}) { - const restore = installSceneRuntime(options.project); + const restore = installSceneRuntime(options.project, { storage: options.storage }); + const storage = globalThis.localStorage; const viewer = fakeViewer(); const styleManager = fakeStyleManager(options.style); const dataManager = fakeDataManager(options.data); @@ -728,7 +740,7 @@ function makeDirector(options = {}) { // Telemetry is only accumulated during a run; observable-failure assertions // need the accumulator without driving a whole run. director._activeRun = { events: [] }; - return { director, viewer, styleManager, dataManager, restore }; + return { director, viewer, styleManager, dataManager, storage, restore }; } test('only Play Shot or a scene run grants transient media authority; LOAD, Stop and replacement revoke it', async () => { @@ -1584,3 +1596,65 @@ test('camera refusal starts no authored frame or playback clock', async () => { assert.deepEqual(viewer.flights, []); } finally { restore(); } }); + +test('an unreadable saved project does not overwrite its recovery checkpoint (#971)', async () => { + const unreadable = '{"version":99,"scenes":[]}'; + const warn = console.warn; + console.warn = () => {}; + const { director, storage, restore } = makeDirector({ storage: { + 'godsEyeView.sceneProject.v2': unreadable, + 'godsEyeView.sceneProject.checkpoint.v1': 'EARLIER-CHECKPOINT', + } }); + try { + assert.equal(storage.stored.get('godsEyeView.sceneProject.v2'), unreadable); + assert.equal(storage.stored.get('godsEyeView.sceneProject.checkpoint.v1'), 'EARLIER-CHECKPOINT'); + } finally { console.warn = warn; await director.destroy(); restore(); } +}); + +test('a capture, update or import that cannot be saved keeps the "Scene not saved" status (#971)', async () => { + const warn = console.warn; + console.warn = () => {}; + const { director, storage, restore } = makeDirector(); + let status = ''; + director.subscribe(({ state }) => { status = state.status; }); + try { + storage.full = true; + director._selectedSceneId = 'scene-1'; + director._selectedShotId = 'shot-a'; + director.captureShot(); + assert.match(status, /^Scene not saved/, 'failed capture keeps the save failure'); + director.updateSelectedShot(); + assert.match(status, /^Scene not saved/, 'failed update keeps the save failure'); + assert.equal(await director.importProjectFile( + { name: 'fixture.json' }, + { prepared: { project: structuredClone(PROJECT_FIXTURE), assets: [] } }, + ), true); + assert.match(status, /^Scene not saved/, 'failed import keeps the save failure'); + } finally { console.warn = warn; await director.destroy(); restore(); } +}); + +test('startScene resolves its result, resumes after a shot without a scene id, and counts distinct scenes (#971)', async () => { + const { director, restore } = makeDirector(); + try { + const options = { single: true, preview: false }; + assert.deepEqual(await director.startScene('scene-1', options), { started: true, shots: 2 }); + assert.equal(director._lastRun.scenesRun, 1, 'one scene with two shots is one scene run'); + assert.deepEqual( + await director.startScene(undefined, { ...options, afterShotId: 'shot-a' }), + { started: true, shots: 1 }, + ); + } finally { await director.destroy(); restore(); } +}); + +test('a caught run failure resolves as run-failed, not success (#971)', async () => { + const { director, styleManager, restore } = makeDirector(); + const warn = console.warn; + console.warn = () => {}; + styleManager.applyVisualState = async () => { throw new Error('visual failed'); }; + try { + assert.deepEqual( + await director.startScene('scene-1', { single: true, preview: false }), + { started: false, reason: 'run-failed' }, + ); + } finally { console.warn = warn; await director.destroy(); restore(); } +});