diff --git a/CHANGELOG.md b/CHANGELOG.md index f511bf044..17e55749f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ## [Unreleased] +- Drive share updates, Location feedback and Scene controls through immutable state snapshots and disposable subscriptions. +- Keep stale lookup/load completions from publishing accepted results and retain shot rows during playback progress updates. +- Export the existing Scene director with explicit playback and editing outcomes. + - Separate UI assembly from standalone engine wiring, with dedicated panel layout, position, notice and recording owners. - Stop pending UI presentation and drag work during disposal; preserve accessible status text when stopping its decoration. - Organize component styles behind the same ordered stylesheet entry and include 3D model controls in the current-state snapshot. diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md index 1fc301747..77bad8983 100644 --- a/docs/CODE-BOUNDARIES.md +++ b/docs/CODE-BOUNDARIES.md @@ -338,3 +338,29 @@ Panel layout, position/drag, notices, recording and deferred UI work have separa owners with synchronous cleanup. Existing scene, share and HUD engines retain their entry points. `ui/styles` loads the ordered stylesheet entry; component files retain the original cascade, including responsive and dock refinements. + + +## UI state and Scene actions + +`StyleManager.subscribeShareState(listener)` supplies the current shareable +visual preferences and subsequent settings changes. The built-in share manager +consumes the same updates. `subscribeLocationSearch(listener)` follows the +current lookup owner across control replacement. `LocationSearch.subscribe` +provides the corresponding per-owner contract. Changes identify `started`, +`found`, `missing`, `failed`, `settled`, and the shell's `reset`; request IDs +belong to their lookup owner. Only current requests publish accepted results. + +`gods-eye-view/scenes` exports `SceneDirector`. Its `subscribe(listener)` supplies +small playback snapshots plus editing outcomes. Scene controls consume these +updates to render the affected presentation; progress does not copy the project +or rebuild shot rows. Project import/export outcomes include the project; +shot editing outcomes include the affected shot and its index before deletion. +Camera, layer sequencing, storage and run-file download retain their existing +owners. Cesium remains an external dependency supplied by the application. + +Each listener receives `{ state, change, revision, initial }` and subscriptions +return an unsubscribe function. Initial state is emitted by default; pass +`{ emitCurrent: false }` to receive only changes. Snapshots and outcomes are +immutable plain data. Reentrant publications retain delivery order; removing a +listener or destroying its owner prevents further queued delivery. These APIs +perform no network requests and discover no additional modules. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 92f38babb..6230bd043 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,19 @@ # God's Eye View Current State +## State and action outcomes + +Share preferences, place lookups and Scene playback expose immutable snapshots +and disposable subscriptions. Share settings drive URL updates; lookup outcomes +drive Location labels and busy/error feedback. Superseded or disposed lookups +cannot publish accepted destinations. Each completion carries its own request +identity so an older completion cannot clear the current search indicator. + +Scene controls consume playback state and editing outcomes from the director. +Progress updates carry a small playback snapshot and preserve shot-row identity; +editing outcomes include a copy of the affected scene or shot. Subscriptions +start with current state, isolate listener failures and stop on disposal. +`gods-eye-view/scenes` exports the same director used by the standalone app. + ## UI shell and component ownership The standalone entry composes the UI with the application's existing layer, diff --git a/package.json b/package.json index e7fa939f5..9ff460106 100644 --- a/package.json +++ b/package.json @@ -140,6 +140,7 @@ "./ui/cockpit/vision": "./src/cockpitVisionPolicy.js", "./ui/scenes": "./src/ui/scenes.js", "./ui/shell": "./src/ui/applicationShell.js", - "./ui/styles": "./src/ui/styles.js" + "./ui/styles": "./src/ui/styles.js", + "./scenes": "./src/scenes/director.js" } } diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index f1ef8b5fe..ae0c7c6a2 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -343,7 +343,8 @@ "src/ui/location.js", "src/ui/locationControls.js", "src/ui/locationSearch.js", - "src/locationStatus.js" + "src/locationStatus.js", + "src/app/stateChannel.js" ], "external": [] }, @@ -518,7 +519,8 @@ "src/ui/visualInput.js", "src/ui/visualPresets.js", "src/worldFocus.js", - "src/ui/uiLifetime.js" + "src/ui/uiLifetime.js", + "src/app/stateChannel.js" ], "external": ["cesium"] }, @@ -548,5 +550,19 @@ "src/ui/styles/provider-settings.css" ], "external": [] + }, + "scenes": { + "exports": ["./scenes"], + "modules": [ + "src/scenes/director.js", + "src/scenes/recipes.js", + "src/scenes/scenePolicy.js", + "src/bloom.js", + "src/app/stateChannel.js", + "src/ui/scenes.js", + "src/ui/sceneControls.js", + "src/ui/scenePresentation.js" + ], + "external": ["cesium"] } } diff --git a/scripts/qa-location-controls.mjs b/scripts/qa-location-controls.mjs index 4c3248ce8..385bfb5bb 100644 --- a/scripts/qa-location-controls.mjs +++ b/scripts/qa-location-controls.mjs @@ -87,6 +87,8 @@ try { await page.evaluate(() => { const lookup = window.__godsEyeView.styleManager._locationLookup; window.__qaSearchRequests = []; + window.__qaLocationState = []; + window.__godsEyeView.styleManager.subscribeLocationSearch((notification) => window.__qaLocationState.push(notification)); lookup.search = (query, options) => new Promise((resolve) => window.__qaSearchRequests.push({ query, options, resolve }), @@ -139,6 +141,14 @@ try { ); }), ); + check('Location subscriptions accept only the current result and retain request identities', await page.evaluate(() => { + const seen = window.__qaLocationState; + const started = seen.filter(({ change }) => change?.type === 'started'); + const found = seen.filter(({ change }) => change?.type === 'found'); + return seen[0].initial && started.length === 2 && found.length === 1 && + found[0].change.requestId === started[1].change.requestId && + found[0].state.destination.label === 'Second landmark, Test city' && Object.isFrozen(found[0].state.destination); + })); check( 'cancelled POI expansion stays closed after a frame', await page.evaluate(async (id) => { diff --git a/scripts/qa-scene-controls.mjs b/scripts/qa-scene-controls.mjs index 81882bcee..26ffc8ac4 100644 --- a/scripts/qa-scene-controls.mjs +++ b/scripts/qa-scene-controls.mjs @@ -39,6 +39,10 @@ try { document.getElementById('loading-screen')?.classList.contains('hidden'), { timeout: 60000 }, ); + await page.evaluate(() => { + window.__qaSceneState = []; + window.__godsEyeView.sceneDirector.subscribe((notification) => window.__qaSceneState.push(notification)); + }); await page.click('[data-collapse-target="scene-panel"]'); await page.click('#scene-new-btn'); await page.waitForFunction( @@ -303,6 +307,13 @@ try { document.querySelectorAll('#scene-select option').length > 0, ), ); + check('Scene subscriptions include current state and the completed native editing actions', await page.evaluate(() => { + const seen = window.__qaSceneState; + const types = new Set(seen.map(({ change }) => change?.type)); + return seen[0].initial && Object.isFrozen(seen[0].state) && + ['scene-created', 'shot-captured', 'shot-renamed', 'shot-updated', 'project-exported', 'project-imported', 'shot-deleted', 'scene-deleted', 'run-event'].every((type) => types.has(type)) && + seen.filter(({ change }) => change?.shot).every(({ change }) => Object.isFrozen(change.shot)); + })); const teardown = await page.evaluate(async () => { const director = window.__godsEyeView.sceneDirector; const controls = director._controls; @@ -320,6 +331,7 @@ try { }, }); const stopping = director.destroy(); + const notificationsAtStop = window.__qaSceneState.length; const stoppedSynchronously = controls.destroyed && controls.removers.length === 0 && @@ -332,6 +344,7 @@ try { await director.destroy(); return { stoppedSynchronously, + stateStopped: window.__qaSceneState.length === notificationsAtStop, sameProject: director._project === project, noLateStatus: document.getElementById('scene-status').textContent === previousStatus, @@ -346,6 +359,7 @@ try { 'Late import completion cannot replace the disposed project or status', teardown.sameProject && teardown.noLateStatus, ); + check('Scene subscriptions stop before late imports settle', teardown.stateStopped); check( 'Scene control interaction produces no uncaught browser errors', errors.length === 0, diff --git a/scripts/qa-ui-disposal.mjs b/scripts/qa-ui-disposal.mjs index a52d5d4b9..7a6a2d486 100644 --- a/scripts/qa-ui-disposal.mjs +++ b/scripts/qa-ui-disposal.mjs @@ -119,6 +119,9 @@ try { setSplitFlapText(feedbackLabel, 'LOAD COMPLETE'); const permanentText = feedbackLabel.querySelector('.gev-flap-text')?.firstChild; + let stateNotifications = 0; + ui.subscribeShareState(() => stateNotifications++, { emitCurrent: false }); + ui.subscribeLocationSearch(() => stateNotifications++, { emitCurrent: false }); try { const disposal = ui.dispose(); const focusBefore = document.activeElement; @@ -139,6 +142,9 @@ try { const noDeferredFocus = document.activeElement === focusBefore; releaseRestoration(); await disposal; + ui._syncShareState(); + ui.subscribeShareState(() => stateNotifications++); + ui.subscribeLocationSearch(() => stateNotifications++); const once = JSON.stringify(counts); await ui.dispose(); window.dispatchEvent(new Event('resize')); @@ -202,6 +208,7 @@ try { ui._lifetime.frames.size === 0 && ui._lifetime.timers.size === 0 && ui._lifetime.removers.size === 0, + stateStopped: stateNotifications === 0, idempotent: once === JSON.stringify(counts), }; } finally { @@ -249,6 +256,7 @@ try { 'shell teardown cancels deferred work and releases listeners', result.shellWorkReleased, ); + check('state subscriptions stop synchronously with UI disposal', result.stateStopped); check('repeated UI disposal is inert', result.idempotent); check( 'disposal and subsequent resize produce no uncaught browser errors', diff --git a/scripts/qa-visual-effects.mjs b/scripts/qa-visual-effects.mjs index e51e5b01f..c1cb91b49 100644 --- a/scripts/qa-visual-effects.mjs +++ b/scripts/qa-visual-effects.mjs @@ -152,9 +152,15 @@ try { 'bloom actions, effect state and saved snapshot agree', await page.evaluate(() => { const manager = window.__godsEyeView.styleManager; + const states = []; + const unsubscribe = manager.subscribeShareState((value) => states.push(value)); const result = manager.setBloom({ enabled: true, intensityPct: 98 }); + unsubscribe(); + const published = states.at(-1); const snapshot = manager.getVisualState(); return ( + states[0].initial && states.length > 1 && Object.isFrozen(published.state.options) && + published.state.bloomEnabled && published.state.options.bloomIntensity === 98 && result.ok && result.bloom.intensityPct === 98 && snapshot.bloom.intensity === 98 && diff --git a/src/app/stateChannel.js b/src/app/stateChannel.js new file mode 100644 index 000000000..df1fe38d5 --- /dev/null +++ b/src/app/stateChannel.js @@ -0,0 +1,83 @@ +/** Copy plain product data so a subscriber cannot mutate its owner's state. */ +function copyState(value, ancestors = new Set()) { + if (value === null) return value; + if (typeof value !== 'object') { + if (['string', 'number', 'boolean', 'undefined'].includes(typeof value)) return value; + throw new TypeError('Product state must contain plain data'); + } + if (ancestors.has(value)) throw new TypeError('Circular product state'); + if (!Array.isArray(value) && Object.getPrototypeOf(value) !== Object.prototype && Object.getPrototypeOf(value) !== null) { + throw new TypeError('Product state must contain plain records'); + } + const next = new Set(ancestors).add(value); + const copy = Array.isArray(value) + ? value.map((item) => copyState(item, next)) + : Object.fromEntries(Object.entries(value).map(([key, item]) => [key, copyState(item, next)])); + return Object.freeze(copy); +} + +/** + * Publish immutable snapshots and ordinary action outcomes synchronously. + * Reentrant publications retain order. New subscribers start at current state; + * removed subscribers and destroyed channels cannot receive queued work. + */ +export function createStateChannel(readSnapshot) { + const listeners = new Set(); + const pending = []; + let revision = 0; + let delivering = false; + let destroyed = false; + let lastSnapshot = null; + const getSnapshot = () => { + if (!destroyed) lastSnapshot = copyState(readSnapshot()); + return lastSnapshot; + }; + const deliver = (entry, notification) => { + try { + entry.listener(notification); + } catch { + console.error('Product state listener failed'); + } + }; + return Object.freeze({ + getSnapshot, + subscribe(listener, { emitCurrent = true } = {}) { + if (typeof listener !== 'function') throw new TypeError('Expected a state listener'); + if (destroyed) return () => {}; + const state = emitCurrent ? getSnapshot() : null; + const entry = { listener, revision }; + listeners.add(entry); + if (emitCurrent) deliver(entry, Object.freeze({ + state, change: null, revision, initial: true, + })); + return () => listeners.delete(entry); + }, + publish(change = null) { + if (destroyed) return false; + pending.push(Object.freeze({ + state: getSnapshot(), change: copyState(change), revision: ++revision, initial: false, + })); + if (delivering) return true; + delivering = true; + try { + while (!destroyed && pending.length) { + const notification = pending.shift(); + for (const entry of [...listeners]) { + if (destroyed || !listeners.has(entry) || entry.revision >= notification.revision) continue; + entry.revision = notification.revision; + deliver(entry, notification); + } + } + } finally { + delivering = false; + } + return true; + }, + destroy() { + if (destroyed) return; + destroyed = true; + listeners.clear(); + pending.length = 0; + }, + }); +} diff --git a/src/app/stateChannel.test.mjs b/src/app/stateChannel.test.mjs new file mode 100644 index 000000000..d084bd5db --- /dev/null +++ b/src/app/stateChannel.test.mjs @@ -0,0 +1,91 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createStateChannel } from './stateChannel.js'; + +test('snapshots and action data are immutable copies of live product state', () => { + const live = { selected: { id: 'a' }, items: ['a'] }; + const channel = createStateChannel(() => live); + const received = []; + channel.subscribe((event) => received.push(event)); + const change = { type: 'selected', item: { id: 'b' } }; + live.selected.id = 'b'; + channel.publish(change); + change.item.id = 'mutated'; + assert.equal(received[0].initial, true); + assert.equal(received[0].state.selected.id, 'a'); + assert.equal(received[1].state.selected.id, 'b'); + assert.equal(received[1].change.item.id, 'b'); + assert.throws(() => received[1].state.items.push('c'), TypeError); + assert.deepEqual(live.items, ['a']); +}); + +test('reentrant updates retain order and a new subscriber never regresses behind its initial snapshot', () => { + let count = 0; + const channel = createStateChannel(() => ({ count })); + const first = [], second = [], joined = []; + channel.subscribe(({ state, initial }) => { + if (initial) return; + first.push(state.count); + if (state.count === 1) { + count = 2; + channel.publish({ type: 'next' }); + channel.subscribe(({ state }) => joined.push(state.count)); + } + }); + channel.subscribe(({ state, initial }) => { if (!initial) second.push(state.count); }); + count = 1; + channel.publish(); + assert.deepEqual(first, [1, 2]); + assert.deepEqual(second, [1, 2]); + assert.deepEqual(joined, [2]); +}); + +test('unsubscribe and destroy during delivery revoke pending callbacks and future reads', () => { + let reads = 0; + const channel = createStateChannel(() => ({ reads: ++reads })); + let late = 0; + let remove; + channel.subscribe(() => { + remove(); + channel.publish(); + channel.destroy(); + }, { emitCurrent: false }); + remove = channel.subscribe(() => late++, { emitCurrent: false }); + channel.publish(); + const stoppedReads = reads; + channel.publish(); + channel.subscribe(() => late++); + channel.destroy(); + remove(); + assert.equal(late, 0); + assert.equal(reads, stoppedReads); +}); + +test('a failed subscriber cannot prevent other controls from receiving an update', () => { + const channel = createStateChannel(() => ({ selected: 'a' })); + const errors = []; + const original = console.error; + console.error = (message) => errors.push(message); + try { + channel.subscribe(() => { throw new Error('consumer failure'); }); + let received = 0; + channel.subscribe(() => received++); + channel.publish(); + assert.equal(received, 2); + assert.equal(errors.length, 2); + } finally { + console.error = original; + channel.destroy(); + } +}); + + +test('an invalid initial snapshot cannot retain a subscriber without a cleanup handle', () => { + let live = { action() {} }; + const channel = createStateChannel(() => live); + let calls = 0; + assert.throws(() => channel.subscribe(() => calls++), /plain data/); + live = { selected: 'a' }; + channel.publish(); + assert.equal(calls, 0); +}); diff --git a/src/cameraHandoff.test.mjs b/src/cameraHandoff.test.mjs index f66f13ed8..b1beee7d1 100644 --- a/src/cameraHandoff.test.mjs +++ b/src/cameraHandoff.test.mjs @@ -29,7 +29,7 @@ function body(source, pattern, label) { function ordered(source, needles, label) { let previous = -1; for (const needle of needles) { - const index = source.indexOf(needle); + const index = source.indexOf(needle, previous + 1); assert.ok(index >= 0, `${label}: missing ${needle}`); assert.ok(index > previous, `${label}: ${needle} is out of order`); previous = index; @@ -247,18 +247,18 @@ test('deferred search releases only after its final authority check', () => { const search = fs.readFileSync(path.join(ROOT, 'src', 'ui', 'locationSearch.js'), 'utf8'); ordered(search, [ 'const authority = this.begin();', - 'this.onStart(authority);', + 'this.onStart?.(authority);', 'await this.search(query', 'beforeFly: () => current() && this.beforeFly(authority)', 'if (!current() || controller.signal.aborted) return;', 'destination?.cancelled', 'finally', - 'this.onSettled(authority)', + 'this.onSettled?.(authority)', ], 'deferred search component'); assert.match(ui, /begin: \(\) => this\._beginDeferredNavigation\('location'\)/); assert.match(ui, /beforeFly: \(generation\) => this\._reassertNavigationHandoff\(generation\)/); assert.match(ui, /isCurrent: \(generation\) =>\s*!this\._disposed && generation === this\._navigationGeneration/); - assert.match(ui, /onSettled: \(generation\) => this\._settleLocationSearchUi\(generation\)/); + assert.match(ui, /change\.type === 'settled'\) this\._settleLocationSearchUi\(change\.generation\)/); assert.doesNotMatch(search.slice(0, search.indexOf('await this.search')), /_releaseFollowCamera/); }); @@ -346,7 +346,7 @@ test('teardown refuses deferred location work before geocoding begins', () => { 'const authority = this.begin();', 'if (authority === false)', 'this.input.blur();', - 'this.onStart(authority);', + 'this.onStart?.(authority);', 'await this.search(query', ], 'disposed search refusal'); assert.match(search, /if \(authority === false\) \{[\s\S]*?return;[\s\S]*?\}\s*this\.controller/); diff --git a/src/cockpitMarkup.test.mjs b/src/cockpitMarkup.test.mjs index 4b370f439..32c184152 100644 --- a/src/cockpitMarkup.test.mjs +++ b/src/cockpitMarkup.test.mjs @@ -242,8 +242,8 @@ test('Cockpit owns a focused shared Display portal and compact Radio controls', css, /body\.scene-playback-mode :is\(#clear-selected-layers, #reset-globe-view\)\s*\{\s*display:\s*none !important;/, ); - assert.match(sceneDirector, /this\._running = true;\s*this\._controls\?\.setPlaybackActive\(true\);/); - assert.match(sceneDirector, /styleManager\.setRecordingMode\(false\);\s*this\._controls\?\.setPlaybackActive\(false\);/); + assert.match(sceneDirector, /this\._running = true;\s*this\._setPlaybackActive\(true\);/); + assert.match(sceneDirector, /styleManager\.setRecordingMode\(false\);\s*this\._setPlaybackActive\(false\);/); assert.match(SceneControls.prototype.setPlaybackActive.toString(), /document\.body\.classList\.toggle\('scene-playback-mode', active\)/); assert.equal((html.match(/id="hud-toggle"/g) || []).length, 1, 'HUD control must have one stateful DOM owner'); assert.equal((html.match(/id="detection-toggle"/g) || []).length, 1, 'Detection control must have one stateful DOM owner'); diff --git a/src/locationReadouts.test.mjs b/src/locationReadouts.test.mjs index 26d078c30..7711de20b 100644 --- a/src/locationReadouts.test.mjs +++ b/src/locationReadouts.test.mjs @@ -11,7 +11,7 @@ const director = fs.readFileSync(path.join(ROOT, 'src', 'scenes', 'director.js') /** Source of the free-text LOCATION search handler (Enter on #location-search). */ function locationSearchHandler() { - const start = ui.indexOf('searchAndFlyTo(this.viewer, query, {'); + const start = ui.indexOf(' _handleLocationSearchState(state, change) {'); assert.ok(start > 0, 'free-text location search handler is missing'); const end = ui.indexOf('_beginWorldJumpTransition() {', start); assert.ok(end > start, 'could not bound the location search handler'); @@ -39,7 +39,7 @@ test('a free-text search records its destination for the LOCATION mini-status', // own refresh would otherwise repaint the readout as "Location: --". assert.match( handler, - /this\._searchedLocationLabel = destination\.label[\s\S]{0,120}?this\._setActiveLocation\(null\);/, + /this\._searchedLocationLabel = state\.destination\.label[\s\S]{0,120}?this\._setActiveLocation\(null\);/, ); assert.match(handler, /this\._updateLocationMiniStatus\(\);/); }); diff --git a/src/scenes/director.js b/src/scenes/director.js index 69af3429f..5a0f15c95 100644 --- a/src/scenes/director.js +++ b/src/scenes/director.js @@ -12,6 +12,7 @@ * State is persisted to localStorage and can be exported/imported as JSON. */ +import { createStateChannel } from '../app/stateChannel.js'; import { SceneControls } from '../ui/scenes.js'; import * as Cesium from 'cesium'; import { SCENE_RECIPES } from './recipes.js'; @@ -315,6 +316,8 @@ export class SceneDirector { this._selectedSceneId = this._project.scenes[0]?.id || null; this._selectedShotId = this._project.scenes[0]?.shots[0]?.id || null; + this._presentation = { status: 'Ready', progress: 0, runtime: '', playbackActive: false, keyboardEnabled: false }; + this._state = createStateChannel(() => ({ ...this.getPlaybackStatus(), ...this._presentation, hasRun: !!this._lastRunJson })); this._initUI(); } @@ -332,6 +335,7 @@ export class SceneDirector { if (this._destroyPromise) return this._destroyPromise; this._destroyed = true; this._controls?.destroy(); + this._state.destroy(); this._destroyPromise = Promise.resolve().then(async () => { this.stopScene('Stopped'); this._loadAbort?.abort(); @@ -394,8 +398,18 @@ export class SceneDirector { * Wire up all scene-panel DOM event listeners and render the initial UI state. * Exits silently if the scene-select element is missing (headless/test mode). */ + /** Immutable playback snapshots and completed editing actions. */ + subscribe(listener, options) { return this._state.subscribe(listener, options); } + + _publish(change) { this._state?.publish(change); } + + _shotOutcome(type, scene, shot, index = scene.shots.indexOf(shot)) { + this._publish({ type, sceneId: scene.id, sceneTitle: scene.title, shot, index }); + } + _initUI() { this._controls = new SceneControls({ + subscribe: (listener) => this.subscribe(listener), read: () => ({ scenes: this._project.scenes, selectedSceneId: this._selectedSceneId, @@ -409,13 +423,13 @@ export class SceneDirector { this._selectedShotId = this._getSelectedScene()?.shots[0]?.id || null; this._renderShotList(); }, - selectShot: (id) => { this._selectedShotId = id; }, + selectShot: (id) => { this._selectedShotId = id; this._publish({ type: 'selection-changed' }); }, renameShot: (sceneId, shotId, title) => { - const { shot } = this._getShot(sceneId, shotId); + const { scene, shot } = this._getShot(sceneId, shotId); if (!shot) return; shot.title = title.trim() || shot.title; this._saveProject(); - this._renderShotList(); + this._shotOutcome('shot-renamed', scene, shot); }, create: (name) => this._createScene(name), deleteScene: () => this._deleteSelectedScene(), @@ -438,7 +452,7 @@ export class SceneDirector { if (!this._project.scenes.some((scene) => scene.id === this._selectedSceneId)) { this._selectedSceneId = this._project.scenes[0]?.id || null; } - this._controls?.renderSceneSelect(); + this._publish({ type: 'scene-options-changed' }); } /** @@ -451,7 +465,7 @@ export class SceneDirector { if (scene?.shots.length && !scene.shots.some((shot) => shot.id === this._selectedShotId)) { this._selectedShotId = scene.shots[0].id; } - this._controls?.renderShotList(); + this._publish({ type: 'shots-changed' }); } /** @@ -488,8 +502,7 @@ export class SceneDirector { this._selectedSceneId = scene.id; this._selectedShotId = null; this._saveProject(); - this._renderSceneSelect(); - this._renderShotList(); + this._publish({ type: 'scene-created', scene }); } /** Delete the currently selected scene after user confirmation. Resets to defaults if empty. */ @@ -506,8 +519,7 @@ export class SceneDirector { this._selectedSceneId = this._project.scenes[0]?.id || null; this._selectedShotId = this._project.scenes[0]?.shots[0]?.id || null; this._saveProject(); - this._renderSceneSelect(); - this._renderShotList(); + this._publish({ type: 'scene-deleted', scene }); } /** @@ -553,7 +565,7 @@ export class SceneDirector { scene.shots.push(shot); this._selectedShotId = shot.id; this._saveProject(); - this._renderShotList(); + this._shotOutcome('shot-captured', scene, shot); this._updateStatus(`Captured: ${scene.title} / ${shot.title}`); } @@ -579,7 +591,7 @@ export class SceneDirector { shot.layers = this._captureLayerStates(); this._saveProject(); - this._renderShotList(); + this._shotOutcome('shot-updated', scene, shot); this._updateStatus(`Updated: ${scene.title} / ${shot.title}`); } @@ -592,10 +604,11 @@ export class SceneDirector { const { scene, shot } = this._getShot(sceneId, shotId); if (!scene || !shot) return; + const index = scene.shots.indexOf(shot); scene.shots = scene.shots.filter((item) => item.id !== shot.id); this._selectedShotId = scene.shots[0]?.id || null; this._saveProject(); - this._renderShotList(); + this._shotOutcome('shot-deleted', scene, shot, index); } /** @@ -646,7 +659,7 @@ export class SceneDirector { if (token.cancelled) return; if (this._loadAbort === controller) this._loadAbort = null; - this._updateStatus(`Loaded: ${scene.title} / ${shot.title}`); + this._shotOutcome('shot-loaded', scene, shot); this._updateRuntime(''); } @@ -750,6 +763,9 @@ export class SceneDirector { return { running: this._running, selectedSceneId: this._selectedSceneId, + selectedShotId: this._selectedShotId, + elapsedMs: this._activeRun ? Math.max(0, Date.now() - Date.parse(this._activeRun.startedAt)) : null, + estimatedDurationMs: this._activeRun ? Math.round(this._activeRun.estimatedDurationSec * 1000) : null, sceneCount: this._project.scenes.length, }; } @@ -806,7 +822,7 @@ export class SceneDirector { // Transition to running state this._running = true; - this._controls?.setPlaybackActive(true); + this._setPlaybackActive(true); this._setButtons(true); this._setProgress(0); @@ -841,7 +857,7 @@ export class SceneDirector { this._startProgressTicker(estimatedDurationSec || 1); this._logEvent('scene_run_start', { count: queue.length }); - this._controls?.setPlaybackKeyboardEnabled(true); + this._setPlaybackKeyboardEnabled(true); try { // Main shot sequencing loop @@ -963,6 +979,7 @@ export class SceneDirector { link.click(); link.remove(); URL.revokeObjectURL(url); + this._publish({ type: 'project-exported', project: this._project }); } /** @@ -979,8 +996,7 @@ export class SceneDirector { this._selectedSceneId = this._project.scenes[0]?.id || null; this._selectedShotId = this._project.scenes[0]?.shots[0]?.id || null; this._saveProject(); - this._renderSceneSelect(); - this._renderShotList(); + this._publish({ type: 'project-imported', project: this._project }); this._updateStatus(`Imported ${file.name}`); } catch { this._updateStatus('Import failed (invalid JSON)'); @@ -1197,14 +1213,14 @@ export class SceneDirector { _finishRun() { clearInterval(this._progressTimer); this._progressTimer = null; - this._controls?.setPlaybackKeyboardEnabled(false); + this._setPlaybackKeyboardEnabled(false); // Covers the error path too: a run that threw mid-shot must not leave a // layer transition running against a director that has stopped watching. this._runAbort?.abort(); this._runAbort = null; this.styleManager.setRecordingMode(false); - this._controls?.setPlaybackActive(false); + this._setPlaybackActive(false); this._updateRuntime(''); this._running = false; @@ -1226,25 +1242,44 @@ export class SceneDirector { * Editing controls are disabled during a run; stop is disabled when idle. * @param {boolean} isRunning */ - _setButtons(isRunning) { this._controls?.setButtons(isRunning); } + _setButtons(isRunning) { this._publish({ type: 'buttons-changed', running: isRunning }); } + + _setPlaybackActive(active) { + this._presentation.playbackActive = active; + this._publish({ type: 'playback-presentation' }); + } + + _setPlaybackKeyboardEnabled(enabled) { + this._presentation.keyboardEnabled = enabled; + this._publish({ type: 'playback-keyboard' }); + } /** * Update the progress bar fill width and label. * @param {number} progress - Value in [0, 1] */ - _setProgress(progress) { this._controls?.setProgress(progress); } + _setProgress(progress) { + this._presentation.progress = progress; + this._publish({ type: 'progress-changed' }); + } /** * Set the status line text in the scene panel. * @param {string} text */ - _updateStatus(text) { this._controls?.updateStatus(text); } + _updateStatus(text) { + this._presentation.status = text; + this._publish({ type: 'status-changed' }); + } /** * Set the runtime label (scene/shot name) and toggle its active class. * @param {string} text - Empty string hides the label */ - _updateRuntime(text) { this._controls?.updateRuntime(text); } + _updateRuntime(text) { + this._presentation.runtime = text; + this._publish({ type: 'runtime-changed' }); + } /** * Append a timestamped telemetry event to the active run log. @@ -1253,6 +1288,7 @@ export class SceneDirector { */ _logEvent(type, payload) { if (!this._activeRun) return; + this._publish({ type: 'run-event', event: type, detail: payload || null }); this._activeRun.events.push({ t: new Date().toISOString(), type, diff --git a/src/scenes/director.test.mjs b/src/scenes/director.test.mjs index 3f2d0c7f4..5dd5be213 100644 --- a/src/scenes/director.test.mjs +++ b/src/scenes/director.test.mjs @@ -645,3 +645,55 @@ test('destroy drains cancelled LOAD work before its viewer can be discarded', as assert.equal((await director.startScene('scene-1')).reason, 'destroyed'); await director.destroy(); }); + +test('Scene snapshots are immutable and editing outcomes retain the affected shot and index', async () => { + const { director, restore } = makeDirector(); + try { + director._activeRun = null; + const seen = []; + const unsubscribe = director.subscribe((notification) => seen.push(notification)); + assert.equal(seen[0].initial, true); + assert.equal(seen[0].state.selectedShotId, 'shot-a'); + assert.ok(Object.isFrozen(seen[0].state)); + director.captureShot(); + const captured = seen.find(({ change }) => change?.type === 'shot-captured'); + assert.equal(captured.change.index, 2); + assert.ok(Object.isFrozen(captured.change.shot.camera)); + const id = captured.change.shot.id; + director.deleteShot('scene-1', id); + const deleted = seen.find(({ change }) => change?.type === 'shot-deleted'); + assert.equal(deleted.change.index, 2); + assert.equal(deleted.change.shot.id, id); + assert.equal(captured.state.selectedShotId, id); + unsubscribe(); + const count = seen.length; + director.captureShot(); + assert.equal(seen.length, count); + await director.destroy(); + } finally { restore(); } +}); + +test('Scene load outcomes exclude superseded and disposed completions', async () => { + const { director, styleManager, restore } = makeDirector(); + try { + director._activeRun = null; + const seen = []; + director.subscribe(({ change }) => { if (change) seen.push(change); }); + const pending = []; + styleManager.applyVisualState = () => new Promise((resolve) => pending.push(resolve)); + const old = director.loadShot('scene-1', 'shot-a'); + const current = director.loadShot('scene-1', 'shot-b'); + pending[1](); + await current; + pending[0](); + await old; + assert.deepEqual(seen.filter((change) => change.type === 'shot-loaded').map((change) => change.shot.id), ['shot-b']); + const late = director.loadShot('scene-1', 'shot-a'); + const disposal = director.destroy(); + const count = seen.length; + pending[2](); + await Promise.all([late, disposal]); + director.subscribe(() => assert.fail('disposed director must not notify')); + assert.equal(seen.length, count); + } finally { restore(); } +}); diff --git a/src/ui/applicationShell.js b/src/ui/applicationShell.js index dc9d7bb7b..e841c37a9 100644 --- a/src/ui/applicationShell.js +++ b/src/ui/applicationShell.js @@ -1,3 +1,4 @@ +import { createStateChannel } from '../app/stateChannel.js'; import { setSplitFlapText } from '../splitFlap.js'; import { UiLifetime } from './uiLifetime.js'; import { RecordingControls } from './recordingControls.js'; @@ -537,6 +538,14 @@ export class StyleManager { ]), ); }); + this._shareState = createStateChannel(() => this._readShareState()); + this._shareState.subscribe(({ state }) => { + this.shareLinkManager.onToggleChange(state.bloomEnabled, state.sharpenEnabled, state.options); + }, { emitCurrent: false }); + this._locationState = createStateChannel(() => this._locationLookup?.getState() || null); + this._locationState.subscribe(({ state, change }) => { + this._handleLocationSearchState(state, change); + }, { emitCurrent: false }); // Parse before panel chrome initializes so every valid share URL starts // from deterministic markup defaults instead of recipient-local panel // preferences. Encoded panel fields are applied after all panels exist. @@ -1893,8 +1902,17 @@ export class StyleManager { }); } + /** Current shareable visual preferences; subscriptions include an initial snapshot. */ + subscribeShareState(listener, options) { + return this._shareState.subscribe(listener, options); + } + _syncShareState() { if (this._disposed) return; + this._shareState.publish({ type: 'settings-changed' }); + } + + _readShareState() { const { getDetectionTuning, isScopeMaskEnabled, @@ -1902,10 +1920,10 @@ export class StyleManager { getScopeTerminusOverride, } = this.services; const detection = this._shareableDetectionState(); - this.shareLinkManager.onToggleChange( - this.bloomEnabled, - this.sharpenEnabled, - { + return { + bloomEnabled: this.bloomEnabled, + sharpenEnabled: this.sharpenEnabled, + options: { bloomIntensity: this._getBloomIntensity(), bloomVersion: BLOOM_SCALE_VERSION, sharpenIntensity: parseInt(this._sharpenSlider?.value || '49', 10), @@ -1929,7 +1947,7 @@ export class StyleManager { : Math.round(getScopeTerminusOverride() * 100), mapStack: this.mapStackController?.getActiveId?.() || 'photoreal', }, - ); + }; } /** @@ -4488,6 +4506,28 @@ export class StyleManager { // ── Location Bar ───────────────────────────── + /** Subscribe to the current location lookup, including replacements of its control owner. */ + subscribeLocationSearch(listener, options) { + return this._locationState.subscribe(listener, options); + } + + _handleLocationSearchState(state, change) { + if (this._disposed || !change) return; + if (change.type === 'started') this._activeLocationSearchGeneration = change.generation; + else if (change.type === 'found') { + this._searchedLocationLabel = state.destination.label || state.query; + this._setActiveLocation(null); + this._currentPoi = null; + this._collapsePOIRow(); + this._updateLocationMiniStatus(); + } else if (change.type === 'missing') this._showToast('Location not found'); + else if (change.type === 'failed') this._showToast('Search failed'); + else if (change.type === 'settled') this._settleLocationSearchUi(change.generation); + else if (change.type === 'reset' && this._activeLocationSearchGeneration !== null) { + this._settleLocationSearchUi(this._activeLocationSearchGeneration); + } + } + /** * Initializes the location bar: renders city pills from CITY_POIS, sets up * QWERTY keyboard navigation for POI selection, wires the search toggle @@ -4497,6 +4537,7 @@ export class StyleManager { _initLocationBar() { const { CITY_POIS, searchAndFlyTo, LocationSearch } = this.services; this._locationControls?.destroy(); + this._locationLookupUnsubscribe?.(); this._locationLookup?.destroy(); this._locationLookup = new LocationSearch({ input: this._locationSearch, @@ -4509,22 +4550,10 @@ export class StyleManager { placeSearch: this.placeSearch, ...options, }), - onStart: (generation) => { - this._activeLocationSearchGeneration = generation; - }, - onResult: (destination, query) => { - this._searchedLocationLabel = destination.label || query; - this._setActiveLocation(null); - this._currentPoi = null; - this._collapsePOIRow(); - this._updateLocationMiniStatus(); - }, - onMissing: () => this._showToast('Location not found'), - onError: (error) => { - console.error('[Search] Geocoding failed:', error); - this._showToast('Search failed'); - }, - onSettled: (generation) => this._settleLocationSearchUi(generation), + onError: (error) => console.error('[Search] Geocoding failed:', error), + }); + this._locationLookupUnsubscribe = this._locationLookup.subscribe(({ initial, change }) => { + this._locationState.publish(initial ? { type: 'reset' } : change); }); this._locationControls = new LocationControls({ elements: { @@ -5234,6 +5263,10 @@ export class StyleManager { this._feedback._globalStatusNotice = null; if (this._globalLoadingStatus) this._globalLoadingStatus.hidden = true; this._disposed = true; + this._shareState.destroy(); + this._locationState.destroy(); + this._locationLookupUnsubscribe?.(); + this._locationLookupUnsubscribe = null; this._lifetime.destroy(); this._recording.destroy(); this._panelPosition.destroy(); diff --git a/src/ui/locationSearch.js b/src/ui/locationSearch.js index b7db9c37c..6361d3731 100644 --- a/src/ui/locationSearch.js +++ b/src/ui/locationSearch.js @@ -1,33 +1,17 @@ +import { createStateChannel } from '../app/stateChannel.js'; + /** One cancellable place lookup at a time, under the caller's camera authority. */ export class LocationSearch { - constructor({ - input, - begin, - isCurrent, - beforeFly, - search, - onStart, - onResult, - onMissing, - onError, - onSettled, - }) { - Object.assign(this, { - input, - begin, - isCurrent, - beforeFly, - search, - onStart, - onResult, - onMissing, - onError, - onSettled, - }); + constructor({ input, begin, isCurrent, beforeFly, search, onStart, onResult, onMissing, onError, onSettled }) { + Object.assign(this, { input, begin, isCurrent, beforeFly, search, onStart, onResult, onMissing, onError, onSettled }); this.controller = null; this.generation = 0; this.destroyed = false; + this.state = { status: 'idle', searching: false, query: '', generation: null, requestId: 0, destination: null, error: null }; + this.channel = createStateChannel(() => this.state); } + getState() { return this.channel.getSnapshot(); } + subscribe(listener, options) { return this.channel.subscribe(listener, options); } async run(query) { query = String(query || '').trim(); if (!query || this.destroyed) return; @@ -41,27 +25,48 @@ export class LocationSearch { const controller = new AbortController(); this.controller = controller; const generation = ++this.generation; - const current = () => - !this.destroyed && - generation === this.generation && - this.isCurrent(authority); - this.onStart(authority); - this.input.classList.add('searching'); + const current = () => !this.destroyed && generation === this.generation && this.isCurrent(authority); + const change = (type) => ({ type, generation: authority, requestId: generation, query }); + this.state = { status: 'searching', searching: true, query, generation: authority, requestId: generation, destination: null, error: null }; try { + this.onStart?.(authority); + if (!current() || controller.signal.aborted) return; + this.input.classList.add('searching'); + this.channel.publish(change('started')); + if (!current() || controller.signal.aborted) return; const destination = await this.search(query, { signal: controller.signal, beforeFly: () => current() && this.beforeFly(authority), }); if (!current() || controller.signal.aborted) return; if (destination?.cancelled) return; - if (destination) this.onResult(destination, query); - else this.onMissing(); + if (destination) { + this.onResult?.(destination, query); + if (!current() || controller.signal.aborted) return; + this.state = { ...this.state, status: 'found', destination: { ...destination } }; + this.channel.publish(change('found')); + } else { + this.onMissing?.(); + if (!current() || controller.signal.aborted) return; + this.state = { ...this.state, status: 'missing' }; + this.channel.publish(change('missing')); + } } catch (error) { if (controller.signal.aborted || !current()) return; - this.onError(error); + this.onError?.(error); + if (!current() || controller.signal.aborted) return; + this.state = { ...this.state, status: 'failed', error: { message: String(error?.message || error) } }; + this.channel.publish(change('failed')); } finally { if (this.controller === controller) this.controller = null; - if (!this.destroyed) this.onSettled(authority); + if (!this.destroyed) { + if (generation === this.generation) this.state = { + ...this.state, searching: false, + status: this.state.status === 'searching' ? 'cancelled' : this.state.status, + }; + this.onSettled?.(authority); + this.channel.publish(change('settled')); + } } } destroy() { @@ -70,5 +75,8 @@ export class LocationSearch { this.generation++; this.controller?.abort(); this.controller = null; + this.state = { ...this.state, status: 'disposed', searching: false }; + this.channel.getSnapshot(); + this.channel.destroy(); } } diff --git a/src/ui/locationSearch.test.mjs b/src/ui/locationSearch.test.mjs index 37b107039..0a460a248 100644 --- a/src/ui/locationSearch.test.mjs +++ b/src/ui/locationSearch.test.mjs @@ -108,3 +108,64 @@ test('empty queries and refused navigation do not issue a lookup', async () => { assert.equal(f.requests.length, 0); assert.ok(f.calls.some((x) => x[0] === 'blur')); }); + + +test('search snapshots retain the current busy owner when an older request settles', async () => { + const f = fixture(); + const events = []; + f.search.subscribe((event) => events.push(event)); + assert.equal(events[0].state.status, 'idle'); + const first = f.search.run('First'); + const second = f.search.run('Second'); + f.requests[0].resolve({ label: 'First' }); + await first; + const settled = events.at(-1); + assert.equal(settled.change.generation, 1); + assert.equal(settled.state.generation, 2); + assert.equal(settled.state.searching, true); + f.requests[1].resolve({ label: 'Second', lat: 1, lng: 2 }); + await second; + const found = events.filter((event) => event.change?.type === 'found'); + assert.equal(found.length, 1); + assert.equal(found[0].state.query, 'Second'); + assert.throws(() => { found[0].state.destination.label = 'Changed'; }, TypeError); + assert.equal(f.search.getState().destination.label, 'Second'); + assert.equal(f.search.getState().searching, false); + const count = events.length; + f.search.destroy(); + assert.equal(f.search.getState().status, 'disposed'); + f.search.subscribe(() => events.push('late')); + assert.equal(events.length, count); +}); + +test('a subscriber that destroys a started lookup prevents the provider from being called', async () => { + const f = fixture(); + f.search.subscribe(({ change }) => { + if (change?.type === 'started') f.search.destroy(); + }); + await f.search.run('Place'); + assert.equal(f.requests.length, 0); + assert.equal(f.search.controller, null); +}); + +test('a reentrant replacement in a result callback cannot publish the obsolete result over the new query', async () => { + const f = fixture(); + let replacement; + const found = []; + f.search.subscribe(({ state, change }) => { + if (change?.type === 'found') found.push(state.query); + }); + f.search.onResult = () => { + f.search.onResult = null; + replacement = f.search.run('Second'); + }; + const first = f.search.run('First'); + f.requests[0].resolve({ label: 'First' }); + await first; + assert.equal(f.search.getState().query, 'Second'); + assert.equal(f.search.getState().searching, true); + assert.deepEqual(found, []); + f.requests[1].resolve({ label: 'Second' }); + await replacement; + assert.deepEqual(found, ['Second']); +}); diff --git a/src/ui/sceneControls.js b/src/ui/sceneControls.js index ec1a99d3b..0abdfb5c3 100644 --- a/src/ui/sceneControls.js +++ b/src/ui/sceneControls.js @@ -10,7 +10,7 @@ import { } from './scenePresentation.js'; export class SceneControls { - constructor({ read, actions, elements = sceneElements() }) { + constructor({ read, actions, subscribe, elements = sceneElements() }) { this.read = read; this.actions = actions; this.elements = elements; @@ -20,8 +20,8 @@ export class SceneControls { this.rowRemovers = []; this.playbackKeyRemover = null; if (!elements.select) return; - this.renderSceneSelect(); - this.renderShotList(); + if (subscribe) this.unsubscribe = subscribe((notification) => this.present(notification)); + else { this.renderSceneSelect(); this.renderShotList(); } this.listen(elements.select, 'change', () => this.run('selectScene', elements.select.value), ); @@ -47,9 +47,29 @@ export class SceneControls { await this.run('import', file); if (!this.destroyed) elements.file.value = ''; }); - this.updateStatus('Ready'); - this.setProgress(0); - this.setButtons(false); + if (!subscribe) { + this.updateStatus('Ready'); + this.setProgress(0); + this.setButtons(false); + } + } + + /** Apply only the presentation affected by an action; selection keeps row identity. */ + present({ state, change, initial }) { + if (this.destroyed) return; + const type = change?.type; + if (initial || ['scene-options-changed', 'scene-created', 'scene-deleted', 'project-imported'].includes(type)) this.renderSceneSelect(); + if (initial || ['shots-changed', 'scene-created', 'scene-deleted', 'shot-captured', 'shot-updated', 'shot-renamed', 'shot-deleted', 'project-imported'].includes(type)) this.renderShotList(); + if (type === 'selection-changed') presentSceneSelection(this.elements.shots, state.selectedShotId); + if (initial || type === 'buttons-changed') this.setButtons(state.running); + if (initial || type === 'progress-changed') this.setProgress(state.progress); + if (initial || type === 'status-changed') this.updateStatus(state.status); + if (initial || type === 'runtime-changed') this.updateRuntime(state.runtime); + if (initial || type === 'playback-presentation') this.setPlaybackActive(state.playbackActive); + if (initial || type === 'playback-keyboard') this.setPlaybackKeyboardEnabled(state.keyboardEnabled); + if (type === 'project-exported') this.updateStatus('Project exported'); + if (type === 'shot-loaded') this.updateStatus(`Loaded: ${change.sceneTitle} / ${change.shot.title}`); + if (type === 'run-event' && change.event === 'shot_start') this.setButtons(state.running); } listen(target, type, callback, removers = this.removers) { @@ -170,6 +190,8 @@ export class SceneControls { if (this.destroyed) return; this.destroyed = true; this.actionGeneration++; + this.unsubscribe?.(); + this.unsubscribe = null; this.setPlaybackKeyboardEnabled(false); for (const remove of [ ...this.removers.splice(0), diff --git a/src/ui/sceneControls.test.mjs b/src/ui/sceneControls.test.mjs index ca6974ded..3a78daae0 100644 --- a/src/ui/sceneControls.test.mjs +++ b/src/ui/sceneControls.test.mjs @@ -1,5 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; +import { createStateChannel } from '../app/stateChannel.js'; import { SceneControls } from './sceneControls.js'; import { SceneDirector } from '../scenes/director.js'; @@ -349,3 +350,27 @@ test('the real director preserves a selected shot label for the following double f.restore(); } }); + + +test('Scene controls consume current state, preserve rows on progress, and unsubscribe on destruction', () => { + const f = fixture(); + try { + f.owner.destroy(); + Object.assign(f.state, { status: 'Ready to resume', progress: 0.25, runtime: '', playbackActive: false, keyboardEnabled: false }); + const channel = createStateChannel(() => f.state); + const owner = new SceneControls({ read: () => f.state, actions: f.actions, elements: f.elements, subscribe: (listener) => channel.subscribe(listener) }); + assert.equal(f.elements.status.textContent, 'Ready to resume'); + const row = f.elements.shots.children[0]; + f.state.progress = 0.5; + channel.publish({ type: 'progress-changed' }); + assert.equal(f.elements.shots.children[0], row); + channel.publish({ type: 'project-exported' }); + assert.equal(f.elements.status.textContent, 'Project exported'); + owner.destroy(); + f.state.status = 'Late'; + channel.publish({ type: 'status-changed' }); + assert.equal(f.elements.status.textContent, 'Project exported'); + assert.equal(owner.unsubscribe, null); + channel.destroy(); + } finally { f.restore(); } +});