diff --git a/src/cameraTilt.test.mjs b/src/cameraTilt.test.mjs index 7d2664c9f..93bbf246e 100644 --- a/src/cameraTilt.test.mjs +++ b/src/cameraTilt.test.mjs @@ -78,6 +78,11 @@ function harness( } const pitchDeg = (camera) => degrees(camera.pitch); +/** Tick until done() holds; a regression fails here instead of hanging. */ +const tickUntil = (h, done, frames = 600) => { + for (let i = 0; i < frames && !done(); i += 1) h.tick(); + assert.ok(done(), `not reached in ${frames} frames`); +}; const headingDeg = (camera) => degrees(Cesium.Math.negativePiToPi(camera.heading)); @@ -171,6 +176,20 @@ test('a requested angle the camera already holds is a no-op', (t) => { assert.equal(getActiveCameraMotion(), null); }); +for (const pitch of [-90, -89]) { + test(`a top-down view asked for ${pitch}° stays put`, (t) => { + // The tool description calls -89 top-down; the clamp must not tilt a + // -90° view up 1° and report it as a move up. + const h = harness(t); + const before = pitchDeg(h.camera); + const result = moveCamera({ motion: 'tilt', pitchDeg: pitch }); + assert.equal(result.ok, true); + assert.equal(result.unchanged, true); + assert.equal(getActiveCameraMotion(), null); + assert.equal(pitchDeg(h.camera), before); + }); +} + test('tilt down from top-down still refuses honestly', (t) => { harness(t); const result = moveCamera({ motion: 'tilt', direction: 'down' }); @@ -335,7 +354,7 @@ test('the pitchDeg and direction check uses the pitch the swing starts from', (t test('after a zoom in, a pivoting tilt never lowers the camera further', (t) => { const h = harness(t, { height: 1000 }); moveCamera({ motion: 'tilt', direction: 'up', mode: 'continuous' }); - while (pitchDeg(h.camera) < -30) h.tick(); + tickUntil(h, () => pitchDeg(h.camera) >= -30); adjustOrbitRange(1 / 2.4); adjustOrbitRange(1 / 2.4); h.tick(); @@ -390,7 +409,7 @@ test('a zoom in during a pivot tilt stops at the clearance', (t) => { // the camera about 107 m up, then at the 80 m zoom floor. const h = harness(t, { height: 400 }); moveCamera({ motion: 'tilt', direction: 'up', mode: 'continuous' }); - while (pitchDeg(h.camera) < -40) h.tick(); + tickUntil(h, () => pitchDeg(h.camera) >= -40); assert.deepEqual(adjustOrbitRange(1 / 2.4), { kind: 'tilt', changed: true }); // At -40° the floor is 150 / sin 40° ≈ 233 m, which the first zoom reached. assert.deepEqual(adjustOrbitRange(1 / 2.4), { kind: 'tilt', changed: false }); diff --git a/src/cameraVerbs.js b/src/cameraVerbs.js index 16888d73e..380a737f0 100644 --- a/src/cameraVerbs.js +++ b/src/cameraVerbs.js @@ -1671,7 +1671,13 @@ export function moveCamera(args = {}, runNavigation = null) { // Judge from the pitch the motion will start at: the pivot's, which at // regional range is a degree or two shallower than the camera's. const { from } = tiltStartFrame(_viewer.camera, { releasesTracking }); - if (Math.abs(tiltToPitch - from) < Cesium.Math.toRadians(0.5)) { + // -89 or -90 means top-down, which a -90° view already is: the clamp + // must not tilt it up 1°. + const atTopDown = tiltToPitch <= PITCH_MIN && from <= PITCH_MIN; + if ( + atTopDown || + Math.abs(tiltToPitch - from) < Cesium.Math.toRadians(0.5) + ) { return { ok: true, action: 'move_camera', diff --git a/src/voice/gevActions.js b/src/voice/gevActions.js index f7573075e..c9bc811a4 100644 --- a/src/voice/gevActions.js +++ b/src/voice/gevActions.js @@ -1302,9 +1302,17 @@ export function createGevActionRunner({ runOptions.signal, deadline, ); - // A later fly-to in the same reply cancels this one: follow it to - // where the camera lands, within the same deadline. - while (status === 'cancelled' && voiceFlight !== flight && current()) { + // 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; status = await waitForVoiceFlight( flight, diff --git a/src/voice/gevActions.test.mjs b/src/voice/gevActions.test.mjs index a464c94bd..14d008933 100644 --- a/src/voice/gevActions.test.mjs +++ b/src/voice/gevActions.test.mjs @@ -570,6 +570,71 @@ test('a motion follows a later fly-to in the same reply that cancels its flight' assert.equal(result.motion, 'orbit'); }); +test('a motion follows a later fly-to whose start cancels its geocode', async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager } = 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 }); + }), + }); + const flyA = runner('fly_to_location', { query: 'Place A' }); + const moved = runner('move_camera', { motion: 'orbit' }); + const movedDone = settledYet(moved); + const flyB = runner('fly_to_location', { query: 'Place B' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + // B takes navigation authority, so A's lookup comes back cancelled. + searches[0].resolve({ cancelled: true }); + assert.equal((await flyA).cancelled, true); + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(movedDone(), false, 'the orbit must follow B, not drop'); + searches[1].resolve({ label: 'Place B', navigationMode: 'city-overview' }); + assert.equal((await flyB).ok, true); + searches[1].options.onComplete(); + const result = await moved; + assert.equal(result.ok, true); + assert.equal(result.motion, 'orbit'); +}); + +test('a motion does not run at a flight that lands while the next place is looked up', async (t) => { + t.after(() => interruptCameraMotion('test-cleanup')); + const { viewer, styleManager, order, 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 movedDone = settledYet(moved); + const flyB = runner('fly_to_location', { query: 'Place B' }); + await new Promise((resolve) => setTimeout(resolve, 0)); + // Deferred navigation leaves A flying while B is looked up, and A lands. + const before = order.length; + flights[0].complete(); + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.equal(movedDone(), false, 'the tilt must wait for B'); + assert.equal( + order.slice(before).includes('cancel'), + false, + 'the tilt must not take navigation and cancel B', + ); + searches[0].resolve({ label: 'Place B', navigationMode: 'city-overview' }); + assert.equal((await flyB).ok, true); + searches[0].options.onComplete(); + const result = await moved; + assert.equal(result.ok, true); + assert.equal(result.motion, 'tilt'); +}); + test('a stale motion does not follow a newer reply\'s fly-to', async () => { const { viewer, styleManager, flights } = tiltAfterFlightHarness(); const runner = createGevActionRunner({