Don't zoom to fit on resume; make indicator better

src/Toolbar.tsx

@@ -22,6 +22,8 @@ import {
 } from 'lib/toolbar'
 import { isDesktop } from 'lib/isDesktop'
 import { openExternalBrowserIfDesktop } from 'lib/openWindow'
+import { EngineConnectionStateType } from 'lang/std/engineConnection'
+import useEngineStreamContext, { EngineStreamState, EngineStreamTransition } from 'hooks/useEngineStreamContext'
 
 export function Toolbar({
   className = '',
@@ -48,7 +50,7 @@ export function Toolbar({
   }, [engineCommandManager.artifactGraph, context.selectionRanges])
 
   const toolbarButtonsRef = useRef<HTMLUListElement>(null)
-  const { overallState } = useNetworkContext()
+  const { overallState, immediateState } = useNetworkContext()
   const { isExecuting } = useKclContext()
   const { isStreamReady } = useAppState()
 
@@ -56,6 +58,7 @@ export function Toolbar({
     (overallState !== NetworkHealthState.Ok &&
       overallState !== NetworkHealthState.Weak) ||
     isExecuting ||
+    immediateState.type !== EngineConnectionStateType.ConnectionEstablished ||
     !isStreamReady
 
   const currentMode =

src/components/EngineStream.tsx

@@ -34,6 +34,7 @@ export const EngineStream = () => {
   const {
     state: modelingMachineState,
     send: modelingMachineActorSend,
+    context: modelingMachineActorContext,
   } = useModelingContext()
 
   const engineStreamActor = useEngineStreamContext.useActorRef()
@@ -156,19 +157,28 @@ export const EngineStream = () => {
   }, [streamIdleMode])
 
   useEffect(() => {
+    let frameId = undefined
     const frameLoop = () => {
-      if (timeoutStart.current) {
+      // Do not pause if the user is in the middle of an operation
+      if (!modelingMachineState.matches('idle')) {
+        // In fact, stop the timeout, because we don't want to trigger the
+        // pause when we exit the operation.
+        timeoutStart.current = null
+      } else if (timeoutStart.current) {
         const elapsed = Date.now() - timeoutStart.current
         if (elapsed >= IDLE_TIME_MS) {
           timeoutStart.current = null
           engineStreamActor.send({ type: EngineStreamTransition.Pause })
         }
       }
-
+      frameId = window.requestAnimationFrame(frameLoop)
-      window.requestAnimationFrame(frameLoop)
     }
-    frameLoop()
+    frameId = window.requestAnimationFrame(frameLoop)
-  }, [])
+
+    return () => {
+      window.cancelAnimationFrame(frameId)
+    }
+  }, [modelingMachineState])
 
   useEffect(() => {
     if (!streamIdleMode) return 
@@ -199,6 +209,11 @@ export const EngineStream = () => {
       timeoutStart.current = Date.now()
     }
 
+    // It's possible after a reconnect, the user doesn't move their mouse at
+    // all, meaning the timer is not reset to run. We need to set it every
+    // time our effect dependencies change then.
+    timeoutStart.current = Date.now()
+
     window.document.addEventListener('keydown', onAnyInput)
     window.document.addEventListener('keyup', onAnyInput)
     window.document.addEventListener('mousemove', onAnyInput)

src/components/ModelStateIndicator.tsx

@@ -1,15 +1,26 @@
+import { useEffect, useState } from 'react'
 import { useEngineCommands } from './EngineCommands'
 import { CustomIcon } from './CustomIcon'
 import useEngineStreamContext, { EngineStreamState } from 'hooks/useEngineStreamContext'
 
 export const ModelStateIndicator = () => {
   const [commands] = useEngineCommands()
+  const [isDone, setIsDone] = useState<boolean>(false)
 
   const engineStreamActor = useEngineStreamContext.useActorRef()
   const engineStreamState = engineStreamActor.getSnapshot()
 
   const lastCommandType = commands[commands.length - 1]?.type
 
+  useEffect(() => {
+    if (lastCommandType === 'set_default_system_properties') {
+      setIsDone(false)
+    }
+    if (lastCommandType === 'execution-done') {
+      setIsDone(true)
+    }
+  }, [lastCommandType])
+
   let className = 'w-6 h-6 '
   let icon = <div className={className}></div>
   let dataTestId = 'model-state-indicator'
@@ -32,7 +43,7 @@ export const ModelStateIndicator = () => {
         name="parallel"
       />
     )
-  } else if (lastCommandType === 'execution-done') {
+  } else if (isDone) {
     className +=
       'text-secondary'
     icon = (

src/hooks/useEngineStreamContext.ts

@@ -47,7 +47,7 @@ const engineStreamMachine = setup({
       input: {} as EngineStreamContext,
     },
     actions: {
-      [EngineStreamTransition.Play]({ context }) {
+      [EngineStreamTransition.Play]({ context }, params: { zoomToFit: boolean }) {
         const canvas = context.canvasRef.current
         if (!canvas) return false
 
@@ -61,13 +61,13 @@ const engineStreamMachine = setup({
         canvas.style.display = "none"
 
         video.srcObject = mediaStream
-        void video.play().catch((e) => {
+        void sceneInfra.camControls.restoreCameraPosition()
+        .then(() => video.play())
+        .catch((e) => {
             console.warn('Video playing was prevented', e, video)
-        }).then(() => {
-          kclManager.executeCode(true).then(() => {
-            return sceneInfra.camControls.restoreCameraPosition()
-          }).catch(trap)
         })
+        .then(() => kclManager.executeCode(params.zoomToFit))
+        .catch(trap)
       },
       [EngineStreamTransition.Pause]({ context }) {
         const video = context.videoRef.current
@@ -181,7 +181,7 @@ const engineStreamMachine = setup({
           },
           [EngineStreamTransition.Play]: {
             target: EngineStreamState.Playing,
-            actions: [ { type: EngineStreamTransition.Play } ]
+            actions: [ { type: EngineStreamTransition.Play, params: { zoomToFit: true }} ]
           }
         }
       },
@@ -214,7 +214,7 @@ const engineStreamMachine = setup({
           },
           [EngineStreamTransition.Play]: {
             target: EngineStreamState.Playing,
-            actions: [ { type: EngineStreamTransition.Play } ]
+            actions: [ { type: EngineStreamTransition.Play, params: { zoomToFit: false }} ]
           }
         }
       },