diff --git a/src/cameraTilt.test.mjs b/src/cameraTilt.test.mjs index 93bbf246e..d372d0746 100644 --- a/src/cameraTilt.test.mjs +++ b/src/cameraTilt.test.mjs @@ -185,6 +185,7 @@ for (const pitch of [-90, -89]) { const result = moveCamera({ motion: 'tilt', pitchDeg: pitch }); assert.equal(result.ok, true); assert.equal(result.unchanged, true); + assert.equal(result.pitchDeg, Math.round(before)); assert.equal(getActiveCameraMotion(), null); assert.equal(pitchDeg(h.camera), before); }); diff --git a/src/cameraVerbs.js b/src/cameraVerbs.js index 380a737f0..c1e5c5d1a 100644 --- a/src/cameraVerbs.js +++ b/src/cameraVerbs.js @@ -1682,7 +1682,9 @@ export function moveCamera(args = {}, runNavigation = null) { ok: true, action: 'move_camera', motion, - pitchDeg: Math.round(Cesium.Math.toDegrees(tiltToPitch)), + pitchDeg: Math.round( + Cesium.Math.toDegrees(atTopDown ? from : tiltToPitch), + ), unchanged: true, }; } diff --git a/src/voice/gevActions.js b/src/voice/gevActions.js index c9bc811a4..83cdea367 100644 --- a/src/voice/gevActions.js +++ b/src/voice/gevActions.js @@ -288,9 +288,10 @@ export function createGevActionRunner({ ? { ...SPEECH_BUILDERS, ...speechBuilders } : SPEECH_BUILDERS; const _layerEnabledAt = new Map(); - // The latest voice flight. A tilt in the same reply as "fly to X" waits for - // it to land: run at once, its navigation cancelled the flight, or the - // flight (still geocoding) then replaced the tilt. + // The latest voice flight. A tilt, orbit, pan or rotate in the same reply + // as "fly to X" waits for it to land: run at once, its navigation + // cancelled the flight, or the flight (still geocoding) then replaced the + // motion. The wait follows later fly-tos (voiceFlightTarget). let voiceFlight = null; let analystEngine; // Area handles live for this runner (the page): resolve_area, the @@ -1143,10 +1144,11 @@ export function createGevActionRunner({ runOptions, ); } - // Chained fly-tos in one reply: a grab during the earlier one counts. - const flight = trackVoiceFlight( - voiceFlight && !voiceFlight.done ? voiceFlight.inputCount : undefined, - ); + // Chained fly-tos: a grab during the earlier one counts. The flight + // replaced is the last one that moved, past lookups that found nothing. + const prior = voiceFlightTarget(voiceFlight); + const replaced = prior && !prior.done ? prior : null; + const flight = trackVoiceFlight(replaced?.inputCount, replaced); voiceFlight = flight; const flown = flyToRequestedLocation(viewer, args, { onFlightEnd: flight.settle, @@ -1177,13 +1179,23 @@ export function createGevActionRunner({ if (viewer.trackedEntity) stopAllTracking(viewer, dataManager); }, }); - // No flight started (not found, refused, cancelled): nothing to wait for. - flown.then( - (result) => { - if (result?.ok !== true) flight.settle('failed'); - }, - () => flight.settle('failed'), - ); + // No flight started: nothing to wait for. A lookup that was not found, + // threw, or was cancelled once a newer voice fly-to had begun (which + // takes navigation from it) moved nothing, so a queued motion looks + // past it to the flight it replaced. One cancelled while still the + // latest voice flight lost navigation to something else (a search, a + // share link, Cockpit): the motion then refuses. A flight its own hooks + // already settled (cancelled mid-air) keeps its status. + const notStarted = () => { + if (!flight.done) flight.movedNothing = true; + flight.settle('failed'); + }; + flown.then((result) => { + if (result?.ok === true) return; + if (result?.cancelled && voiceFlight === flight) + flight.settle('failed'); + else notStarted(); + }, notStarted); return flown; } @@ -1287,39 +1299,45 @@ export function createGevActionRunner({ if (name === 'move_camera') { const motion = String(args.motion || '').toLowerCase(); + // A lookup that already found nothing does not hide the last flight + // that moved: running now would cancel that flight mid-air. + const pending = voiceFlightTarget(voiceFlight); if ( VOICE_FLIGHT_WAITING_MOTIONS.has(motion) && - voiceFlight && - !voiceFlight.done + pending && + !pending.done ) { // Bad arguments are refused now, not after waiting on the flight. const argsError = moveCameraArgsError(args); if (argsError) return argsError; const deadline = Date.now() + VOICE_FLIGHT_WAIT_MS; - let flight = voiceFlight; + const start = pending; + let flight = start; let status = await waitForVoiceFlight( flight, runOptions.signal, deadline, ); - // A later fly-to in the same reply replaces this one: follow it to - // where the camera lands, within the same deadline. Its status does - // not matter. A geocode that loses navigation reports failed, and a - // flight left flying while the next place is looked up can still - // land; running there would take navigation and cancel the newer one. - while ( - voiceFlight !== flight && - status !== 'aborted' && - status !== 'timeout' && - current() - ) { - flight = voiceFlight; + // A later fly-to replaces this one: follow it to where the camera + // lands, within the same deadline, whatever this flight reported. A + // geocode that loses navigation reports failed, and a flight left + // flying while the next place is looked up can still land; running + // there would take navigation and cancel the newer one. A later + // lookup that found nothing moved nothing, so the motion acts on the + // last flight it saw that did move the camera. + let landed = flight.movedNothing ? null : { flight, status }; + while (status !== 'aborted' && status !== 'timeout' && current()) { + const target = voiceFlightTarget(voiceFlight, start); + if (!target || target === flight) break; + flight = target; status = await waitForVoiceFlight( flight, runOptions.signal, deadline, ); + if (!flight.movedNothing) landed = { flight, status }; } + if (flight.movedNothing && landed) ({ flight, status } = landed); if (!current()) return { ok: false, action: 'move_camera', cancelled: true }; if (status !== 'arrived') @@ -2939,11 +2957,25 @@ async function frameOverhead(viewer, dataManager, styleManager, args = {}) { ); } -/** A voice flight's end: 'arrived', 'cancelled' or 'failed', settled once. */ -function trackVoiceFlight(inputCount = manualCameraInputCount()) { +let voiceFlightCount = 0; + +/** + * A voice flight's end: 'arrived', 'cancelled' or 'failed', settled once. + * `replaced` is the last flight that moved, if still pending when this one + * began (past lookups that found nothing). It is kept only + * while this one may turn out to have moved nothing (a lookup that found no + * place), so a queued motion can look past it. + */ +function trackVoiceFlight( + inputCount = manualCameraInputCount(), + replaced = null, +) { let resolve; const flight = { done: false, + seq: (voiceFlightCount += 1), + replaced, + movedNothing: false, // Manual input after this means the user took the camera mid-flight. inputCount, ended: new Promise((settle) => { @@ -2952,12 +2984,26 @@ function trackVoiceFlight(inputCount = manualCameraInputCount()) { settle: (status) => { if (flight.done) return; flight.done = true; + if (!flight.movedNothing) flight.replaced = null; resolve(status); }, }; return flight; } +/** + * The flight a motion queued since `start` acts on: the latest one that + * actually started. A lookup that found nothing moved nothing, so look past + * it to the flight it replaced, but never to one older than `start` (a flight + * from an earlier reply). Null when every later lookup found nothing. With + * no `start`, the latest flight that started, however old. + */ +function voiceFlightTarget(latest, start = null) { + let flight = latest; + while (flight?.movedNothing) flight = flight.replaced; + return flight && (!start || flight.seq >= start.seq) ? flight : null; +} + /** * Longest a motion waits for its flight (geocode plus flight, which is about * 2 s) before giving up. The wait also holds the tool output, and on Gemini diff --git a/src/voice/gevActions.test.mjs b/src/voice/gevActions.test.mjs index 14d008933..ece2f92e0 100644 --- a/src/voice/gevActions.test.mjs +++ b/src/voice/gevActions.test.mjs @@ -635,6 +635,162 @@ test('a motion does not run at a flight that lands while the next place is looke assert.equal(result.motion, 'tilt'); }); +// A later fly-to that finds nothing moves nothing: the motion still runs where +// the earlier flight lands, whether the lookup fails before or after that. +for (const order of ['lands first', 'not found first']) { + test(`a motion runs at its flight when a later fly-to finds nothing (${order})`, async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const harness = tiltAfterFlightHarness(); + const { viewer, styleManager, flights } = harness; + const searches = []; + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: (_viewer, query, options) => new Promise((resolve) => { + searches.push({ query, options, resolve }); + }), + }); + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + const moved = runner('move_camera', { motion: 'tilt', pitchDeg: -45 }); + const flyB = runner('fly_to_location', { query: 'Nowhere' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + if (order === 'lands first') flights[0].complete(); + searches[0].resolve(null); + assert.equal((await flyB).ok, false); + if (order !== 'lands first') flights[0].complete(); + const result = await moved; + assert.equal(result.ok, true, result.error); + assert.equal(result.motion, 'tilt'); + }); +} + +test('a motion beside a fly-to that finds nothing never acts at an earlier reply\'s flight', async () => { + const { viewer, styleManager, flights } = tiltAfterFlightHarness(); + const searches = []; + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: (_viewer, query, options) => new Promise((resolve) => { + searches.push({ query, options, resolve }); + }), + }); + // Reply 1: "fly to Austin", still flying. + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + // Reply 2: "fly to Nowhere and tilt". + const flyB = runner('fly_to_location', { query: 'Nowhere' }); + const moved = runner('move_camera', { motion: 'tilt', pitchDeg: -45 }); + await new Promise((resolve) => setTimeout(resolve, 0)); + searches[0].resolve(null); + assert.equal((await flyB).ok, false); + flights[0].complete(); + const result = await moved; + assert.equal(result.ok, false); + assert.match(result.error, /did not arrive/); +}); + +// Two later lookups that both find nothing, in either order, leave the motion +// acting where the first flight landed. +for (const first of [0, 1]) { + test(`a motion looks past two lookups that found nothing (lookup ${first + 1} fails first)`, async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager, flights } = tiltAfterFlightHarness(); + const searches = []; + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: (_viewer, query, options) => new Promise((resolve) => { + searches.push({ query, options, resolve }); + }), + }); + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + const moved = runner('move_camera', { motion: 'orbit' }); + const flyB = runner('fly_to_location', { query: 'Nowhere B' }); + const flyC = runner('fly_to_location', { query: 'Nowhere C' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + searches[first].resolve(null); + searches[1 - first].resolve(null); + assert.equal((await flyB).ok, false); + assert.equal((await flyC).ok, false); + flights[0].complete(); + const result = await moved; + assert.equal(result.ok, true, result.error); + }); +} + +test('a lookup cancelled by a newer one that finds nothing does not hide the flight that landed', async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager, flights } = tiltAfterFlightHarness(); + const searches = []; + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: (_viewer, query, options) => new Promise((resolve) => { + searches.push({ query, options, resolve }); + }), + }); + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + const moved = runner('move_camera', { motion: 'tilt', pitchDeg: -45 }); + const flyB = runner('fly_to_location', { query: 'Place B' }); + const flyC = runner('fly_to_location', { query: 'Nowhere C' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + flights[0].complete(); + // C's start takes navigation from B's lookup; C then finds nothing. + searches[0].resolve({ cancelled: true }); + searches[1].resolve(null); + assert.equal((await flyB).cancelled, true); + assert.equal((await flyC).ok, false); + const result = await moved; + assert.equal(result.ok, true, result.error); +}); + +test('a motion sent after a fly-to that found nothing still waits for the flight before it', async (t) => { + // Serial dispatch: "fly to Austin, fly to Nowhere, tilt", one call at a time. + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager, order, flights } = tiltAfterFlightHarness(); + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: async () => null, + }); + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + assert.equal((await runner('fly_to_location', { query: 'Nowhere' })).ok, false); + const before = order.length; + const moved = runner('move_camera', { motion: 'tilt', pitchDeg: -45 }); + const movedDone = settledYet(moved); + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(movedDone(), false, 'the tilt must wait for the Austin flight'); + assert.equal(order.slice(before).includes('cancel'), false); + flights[0].complete(); + assert.equal((await moved).ok, true); +}); + +test('two lookups that found nothing in a row still leave the motion waiting for the flight', async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager, order, flights } = tiltAfterFlightHarness(); + const runner = createGevActionRunner({ + viewer, + styleManager, + dataManager: { layers: new Map(), getAll: () => [] }, + searchNavigation: async () => null, + }); + assert.equal((await runner('fly_to_location', { locationId: 'austin' })).ok, true); + assert.equal((await runner('fly_to_location', { query: 'Nowhere' })).ok, false); + assert.equal((await runner('fly_to_location', { query: 'Nowhere 2' })).ok, false); + const before = order.length; + const moved = runner('move_camera', { motion: 'orbit' }); + const movedDone = settledYet(moved); + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(movedDone(), false, 'the orbit must wait for the Austin flight'); + assert.equal(order.slice(before).includes('cancel'), false); + flights[0].complete(); + assert.equal((await moved).ok, true); +}); + test('a stale motion does not follow a newer reply\'s fly-to', async () => { const { viewer, styleManager, flights } = tiltAfterFlightHarness(); const runner = createGevActionRunner({