Follow up: Stream Idle PR (#6238)

* Use a dropdown for stream idle setting

* Add support for undefined values in dropdowns

* Move cache bust to the beginning of engine open for reconnections

* yarn tsc

* Don't setup model feature highlighters until the connection is ready

* Wait 2s to give engine time to serve video, then listen for modeling commands

* Undo teardown

* yarn fmt

* Fix circular module dependency; fmt & lint & tsc

* Fix editor-test waiting for 2 instead of 1

* Increment another 2 numbers ...

---------

Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com>

e2e/playwright/editor-tests.spec.ts

@@ -82,7 +82,7 @@ sketch001 = startSketchOn(XY)
       .poll(() =>
         page.locator('[data-receive-command-type="scene_clear_all"]').count()
       )
-      .toBe(1)
+      .toBe(2)
     await expect
       .poll(() => page.locator('[data-message-type="execution-done"]').count())
       .toBe(2)
@@ -106,7 +106,7 @@ sketch001 = startSketchOn(XY)
     ).toHaveCount(3)
     await expect(
       page.locator('[data-receive-command-type="scene_clear_all"]')
-    ).toHaveCount(1)
+    ).toHaveCount(2)
   })
 
   test('ensure we use the cache, and do not clear on append', async ({
@@ -133,7 +133,7 @@ sketch001 = startSketchOn(XY)
     await u.openDebugPanel()
     await expect(
       page.locator('[data-receive-command-type="scene_clear_all"]')
-    ).toHaveCount(1)
+    ).toHaveCount(2)
     await expect(
       page.locator('[data-message-type="execution-done"]')
     ).toHaveCount(2)
@@ -161,7 +161,7 @@ sketch001 = startSketchOn(XY)
     ).toHaveCount(3)
     await expect(
       page.locator('[data-receive-command-type="scene_clear_all"]')
-    ).toHaveCount(1)
+    ).toHaveCount(2)
   })
 
   test('if you click the format button it formats your code', async ({

src/components/EngineCommands.tsx

@@ -1,6 +1,6 @@
 import { useEffect, useState } from 'react'
 
-import type { CommandLog } from '@src/lang/std/engineConnection'
+import type { CommandLog } from '@src/lang/std/commandLog'
 import { engineCommandManager } from '@src/lib/singletons'
 import { reportRejection } from '@src/lib/trap'
 

src/components/Settings/SettingsFieldInput.tsx

@@ -101,7 +101,10 @@ export function SettingsFieldInput({
               type: `set.${category}.${settingName}`,
               data: {
                 level: settingsLevel,
-                value: e.target.value,
+                // undefined is the only special string due to no way to
+                // encode it in the string-only options.
+                value:
+                  e.target.value === 'undefined' ? undefined : e.target.value,
               },
             } as unknown as EventFrom<WildcardSetEvent>)
           }

src/hooks/useEngineConnectionSubscriptions.ts

@@ -27,6 +27,8 @@ import {
 } from '@src/lib/singletons'
 import { err, reportRejection } from '@src/lib/trap'
 import { getModuleId } from '@src/lib/utils'
+import { engineStreamActor } from '@src/machines/appMachine'
+import { EngineStreamState } from '@src/machines/engineStreamMachine'
 import type {
   EdgeCutInfo,
   ExtrudeFacePlane,
@@ -38,8 +40,11 @@ export function useEngineConnectionSubscriptions() {
   const stateRef = useRef(state)
   stateRef.current = state
 
+  const engineStreamState = engineStreamActor.getSnapshot()
+
   useEffect(() => {
     if (!engineCommandManager) return
+    if (engineStreamState.value !== EngineStreamState.Playing) return
 
     const unSubHover = engineCommandManager.subscribeToUnreliable({
       // Note this is our hover logic, "highlight_set_entity" is the event that is fired when we hover over an entity
@@ -76,9 +81,12 @@ export function useEngineConnectionSubscriptions() {
       unSubHover()
       unSubClick()
     }
-  }, [engineCommandManager, context?.sketchEnginePathId])
+  }, [engineCommandManager, engineStreamState, context?.sketchEnginePathId])
 
   useEffect(() => {
+    if (!engineCommandManager) return
+    if (engineStreamState.value !== EngineStreamState.Playing) return
+
     const unSub = engineCommandManager.subscribeTo({
       event: 'select_with_point',
       callback: state.matches('Sketch no face')
@@ -342,5 +350,5 @@ export function useEngineConnectionSubscriptions() {
         : () => {},
     })
     return unSub
-  }, [state])
+  }, [engineCommandManager, engineStreamState, state])
 }

src/lang/KclSingleton.ts

@@ -19,8 +19,8 @@ import {
 } from '@src/lang/errors'
 import { executeAst, executeAstMock, lintAst } from '@src/lang/langHelpers'
 import { getNodeFromPath, getSettingsAnnotation } from '@src/lang/queryAst'
+import { CommandLogType } from '@src/lang/std/commandLog'
 import type { EngineCommandManager } from '@src/lang/std/engineConnection'
-import { CommandLogType } from '@src/lang/std/engineConnection'
 import { topLevelRange } from '@src/lang/util'
 import type {
   ArtifactGraph,

src/lang/std/commandLog.ts

@@ -0,0 +1,31 @@
+import type { Models } from '@kittycad/lib'
+import type { EngineCommand } from '@src/lang/std/artifactGraph'
+
+export enum CommandLogType {
+  SendModeling = 'send-modeling',
+  SendScene = 'send-scene',
+  ReceiveReliable = 'receive-reliable',
+  ExecutionDone = 'execution-done',
+  ExportDone = 'export-done',
+  SetDefaultSystemProperties = 'set_default_system_properties',
+}
+
+export type CommandLog =
+  | {
+      type: CommandLogType.SendModeling
+      data: EngineCommand
+    }
+  | {
+      type: CommandLogType.SendScene
+      data: EngineCommand
+    }
+  | {
+      type: CommandLogType.ReceiveReliable
+      data: Models['OkWebSocketResponseData_type']
+      id: string
+      cmd_type?: string
+    }
+  | {
+      type: CommandLogType.ExecutionDone
+      data: null
+    }

src/lang/std/engineConnection.ts

@@ -1,15 +1,20 @@
 import type { Models } from '@kittycad/lib'
 import { VITE_KC_API_WS_MODELING_URL, VITE_KC_DEV_TOKEN } from '@src/env'
+import { jsAppSettings } from '@src/lib/settings/settingsUtils'
 import { BSON } from 'bson'
 
 import type { MachineManager } from '@src/components/MachineManagerProvider'
 import type { useModelingContext } from '@src/hooks/useModelingContext'
+import type CodeManager from '@src/lang/codeManager'
 import type { KclManager } from '@src/lang/KclSingleton'
 import type { EngineCommand, ResponseMap } from '@src/lang/std/artifactGraph'
+import type { CommandLog } from '@src/lang/std/commandLog'
+import { CommandLogType } from '@src/lang/std/commandLog'
 import type { SourceRange } from '@src/lang/wasm'
 import { defaultSourceRange } from '@src/lang/wasm'
 import { EXECUTE_AST_INTERRUPT_ERROR_MESSAGE } from '@src/lib/constants'
 import { markOnce } from '@src/lib/performance'
+import type RustContext from '@src/lib/rustContext'
 import type { SettingsViaQueryString } from '@src/lib/settings/settingsTypes'
 import {
   Themes,
@@ -28,8 +33,6 @@ function isHighlightSetEntity_type(
   return data.entity_id && data.sequence
 }
 
-type OkWebSocketResponseData = Models['OkWebSocketResponseData_type']
-
 interface NewTrackArgs {
   conn: EngineConnection
   mediaStream: MediaStream
@@ -658,6 +661,22 @@ class EngineConnection extends EventTarget {
                   detail: { conn: this, mediaStream: this.mediaStream! },
                 })
               )
+
+              setTimeout(() => {
+                // Everything is now connected.
+                this.state = {
+                  type: EngineConnectionStateType.ConnectionEstablished,
+                }
+
+                this.engineCommandManager.inSequence = 1
+
+                this.dispatchEvent(
+                  new CustomEvent(EngineConnectionEvents.Opened, {
+                    detail: this,
+                  })
+                )
+                markOnce('code/endInitialEngineConnect')
+              }, 2000)
               break
             case 'connecting':
               break
@@ -785,18 +804,6 @@ class EngineConnection extends EventTarget {
                 type: ConnectingType.DataChannelEstablished,
               },
             }
-
-            // Everything is now connected.
-            this.state = {
-              type: EngineConnectionStateType.ConnectionEstablished,
-            }
-
-            this.engineCommandManager.inSequence = 1
-
-            this.dispatchEvent(
-              new CustomEvent(EngineConnectionEvents.Opened, { detail: this })
-            )
-            markOnce('code/endInitialEngineConnect')
           }
           this.unreliableDataChannel?.addEventListener(
             'open',
@@ -1269,35 +1276,6 @@ export interface Subscription<T extends ModelTypes> {
   ) => void
 }
 
-export enum CommandLogType {
-  SendModeling = 'send-modeling',
-  SendScene = 'send-scene',
-  ReceiveReliable = 'receive-reliable',
-  ExecutionDone = 'execution-done',
-  ExportDone = 'export-done',
-  SetDefaultSystemProperties = 'set_default_system_properties',
-}
-
-export type CommandLog =
-  | {
-      type: CommandLogType.SendModeling
-      data: EngineCommand
-    }
-  | {
-      type: CommandLogType.SendScene
-      data: EngineCommand
-    }
-  | {
-      type: CommandLogType.ReceiveReliable
-      data: OkWebSocketResponseData
-      id: string
-      cmd_type?: string
-    }
-  | {
-      type: CommandLogType.ExecutionDone
-      data: null
-    }
-
 export enum EngineCommandManagerEvents {
   // engineConnection is available but scene setup may not have run
   EngineAvailable = 'engine-available',
@@ -1398,6 +1376,7 @@ export class EngineCommandManager extends EventTarget {
 
   private onEngineConnectionOpened = () => {}
   private onEngineConnectionClosed = () => {}
+  private onVideoTrackMute = () => {}
   private onDarkThemeMediaQueryChange = (e: MediaQueryListEvent) => {
     this.setTheme(e.matches ? Themes.Dark : Themes.Light).catch(reportRejection)
   }
@@ -1408,6 +1387,8 @@ export class EngineCommandManager extends EventTarget {
   modelingSend: ReturnType<typeof useModelingContext>['send'] =
     (() => {}) as any
   kclManager: null | KclManager = null
+  codeManager?: CodeManager
+  rustContext?: RustContext
 
   // The current "manufacturing machine" aka 3D printer, CNC, etc.
   public machineManager: MachineManager | null = null
@@ -1480,6 +1461,11 @@ export class EngineCommandManager extends EventTarget {
 
     // eslint-disable-next-line @typescript-eslint/no-misused-promises
     this.onEngineConnectionOpened = async () => {
+      await this.rustContext?.clearSceneAndBustCache(
+        { settings: await jsAppSettings() },
+        this.codeManager?.currentFilePath || undefined
+      )
+
       // Set the stream's camera projection type
       // We don't send a command to the engine if in perspective mode because
       // for now it's the engine's default.
@@ -1695,15 +1681,17 @@ export class EngineCommandManager extends EventTarget {
         delete this.pendingCommands[message.request_id || '']
       }) as EventListener)
 
+      this.onVideoTrackMute = () => {
+        console.error('video track mute: check webrtc internals -> inbound rtp')
+      }
+
       this.onEngineConnectionNewTrack = ({
         detail: { mediaStream },
       }: CustomEvent<NewTrackArgs>) => {
-        mediaStream.getVideoTracks()[0].addEventListener('mute', () => {
+        // Engine side had an oopsie (client sent trickle_ice, engine no happy)
-          console.error(
+        mediaStream
-            'video track mute: check webrtc internals -> inbound rtp'
+          .getVideoTracks()[0]
-          )
+          .addEventListener('mute', this.onVideoTrackMute)
-        })
-
         setMediaStream(mediaStream)
       }
       this.engineConnection?.addEventListener(

src/lib/coredump.ts

@@ -5,10 +5,8 @@ import type { OsInfo } from '@rust/kcl-lib/bindings/OsInfo'
 import type { WebrtcStats } from '@rust/kcl-lib/bindings/WebrtcStats'
 
 import type CodeManager from '@src/lang/codeManager'
-import type {
+import type { CommandLog } from '@src/lang/std/commandLog'
-  CommandLog,
+import type { EngineCommandManager } from '@src/lang/std/engineConnection'
-  EngineCommandManager,
-} from '@src/lang/std/engineConnection'
 import { isDesktop } from '@src/lib/isDesktop'
 import type RustContext from '@src/lib/rustContext'
 import screenshot from '@src/lib/screenshot'

src/lib/settings/initialSettings.tsx

@@ -1,4 +1,4 @@
-import { useRef, useState } from 'react'
+import { useRef } from 'react'
 
 import type { CameraOrbitType } from '@rust/kcl-lib/bindings/CameraOrbitType'
 import type { CameraProjectionType } from '@rust/kcl-lib/bindings/CameraProjectionType'
@@ -6,7 +6,6 @@ import type { NamedView } from '@rust/kcl-lib/bindings/NamedView'
 import type { OnboardingStatus } from '@rust/kcl-lib/bindings/OnboardingStatus'
 
 import { CustomIcon } from '@src/components/CustomIcon'
-import { Toggle } from '@src/components/Toggle/Toggle'
 import Tooltip from '@src/components/Tooltip'
 import type { CameraSystem } from '@src/lib/cameraControls'
 import { cameraMouseDragGuards, cameraSystems } from '@src/lib/cameraControls'
@@ -216,104 +215,37 @@ export function createSettings() {
         hideOnLevel: 'project',
         description: 'Save bandwidth & battery',
         validate: (v) =>
-          v === undefined ||
+          String(v) == 'undefined' ||
-          (typeof v === 'number' &&
+          (Number(v) >= 0 && Number(v) <= 60 * MS_IN_MINUTE),
-            v >= 1 * MS_IN_MINUTE &&
+        commandConfig: {
-            v <= 60 * MS_IN_MINUTE),
+          inputType: 'options',
-        Component: ({
+          defaultValueFromContext: (context) =>
-          value: settingValueInStorage,
+            context.app.streamIdleMode.current,
-          updateValue: writeSettingValueToStorage,
+          options: (cmdContext, settingsContext) =>
-        }) => {
+            [
-          const [timeoutId, setTimeoutId] = useState<
+              undefined,
-            ReturnType<typeof setTimeout> | undefined
+              5 * 1000,
-          >(undefined)
+              30 * 1000,
-          const [preview, setPreview] = useState(
+              1 * MS_IN_MINUTE,
-            settingValueInStorage === undefined
+              2 * MS_IN_MINUTE,
-              ? settingValueInStorage
+              5 * MS_IN_MINUTE,
-              : settingValueInStorage / MS_IN_MINUTE
+              15 * MS_IN_MINUTE,
-          )
+              30 * MS_IN_MINUTE,
-          const onChangeRange = (e: React.SyntheticEvent) => {
+              60 * MS_IN_MINUTE,
-            if (
+            ].map((v) => ({
-              !(
+              name:
-                e.isTrusted &&
+                v === undefined
-                'value' in e.currentTarget &&
+                  ? 'Off'
-                e.currentTarget.value
+                  : v < MS_IN_MINUTE
-              )
+                    ? `${Math.floor(v / 1000)} seconds`
-            )
+                    : `${Math.floor(v / MS_IN_MINUTE)} minutes`,
-              return
+              value: v,
-            setPreview(Number(e.currentTarget.value))
+              isCurrent:
-          }
+                v ===
-          const onSaveRange = (e: React.SyntheticEvent) => {
+                settingsContext.app.streamIdleMode[
-            if (preview === undefined) return
+                  cmdContext.argumentsToSubmit.level as SettingsLevel
-            if (
+                ],
-              !(
+            })),
-                e.isTrusted &&
-                'value' in e.currentTarget &&
-                e.currentTarget.value
-              )
-            )
-              return
-            writeSettingValueToStorage(
-              Number(e.currentTarget.value) * MS_IN_MINUTE
-            )
-          }
-
-          return (
-            <div className="flex item-center gap-4 m-0 py-0">
-              <Toggle
-                name="streamIdleModeToggle"
-                offLabel="Off"
-                onLabel="On"
-                checked={settingValueInStorage !== undefined}
-                onChange={(event: React.SyntheticEvent<HTMLInputElement>) => {
-                  if (timeoutId) {
-                    return
-                  }
-                  const isChecked = event.currentTarget.checked
-                  clearTimeout(timeoutId)
-                  setTimeoutId(
-                    setTimeout(() => {
-                      const requested = !isChecked ? undefined : 5
-                      setPreview(requested)
-                      writeSettingValueToStorage(
-                        requested === undefined
-                          ? undefined
-                          : Number(requested) * MS_IN_MINUTE
-                      )
-                      setTimeoutId(undefined)
-                    }, 100)
-                  )
-                }}
-                className="block w-4 h-4"
-              />
-              <div className="flex flex-col grow">
-                <input
-                  type="range"
-                  onChange={onChangeRange}
-                  onMouseUp={onSaveRange}
-                  onKeyUp={onSaveRange}
-                  onPointerUp={onSaveRange}
-                  disabled={preview === undefined}
-                  value={
-                    preview !== null && preview !== undefined ? preview : 5
-                  }
-                  min={1}
-                  max={60}
-                  step={1}
-                  className="block flex-1"
-                />
-                {preview !== undefined && preview !== null && (
-                  <div>
-                    {preview / MS_IN_MINUTE === 60
-                      ? '1 hour'
-                      : preview / MS_IN_MINUTE === 1
-                        ? '1 minute'
-                        : preview + ' minutes'}
-                  </div>
-                )}
-              </div>
-            </div>
-          )
         },
       }),
       allowOrbitInSketchMode: new Setting<boolean>({

src/lib/singletons.ts

@@ -44,7 +44,12 @@ export const kclManager = new KclManager(engineCommandManager, {
 // CYCLIC REF
 editorManager.kclManager = kclManager
 
+// These are all late binding because of their circular dependency.
+// TODO: proper dependency injection.
 engineCommandManager.kclManager = kclManager
+engineCommandManager.codeManager = codeManager
+engineCommandManager.rustContext = rustContext
+
 kclManager.sceneInfraBaseUnitMultiplierSetter = (unit: BaseUnit) => {
   sceneInfra.baseUnit = unit
 }

src/machines/engineStreamMachine.ts

@@ -1,10 +1,4 @@
-import { jsAppSettings } from '@src/lib/settings/settingsUtils'
+import { engineCommandManager, sceneInfra } from '@src/lib/singletons'
-import {
-  codeManager,
-  engineCommandManager,
-  rustContext,
-  sceneInfra,
-} from '@src/lib/singletons'
 import type { MutableRefObject } from 'react'
 import type { ActorRefFrom } from 'xstate'
 import { assign, fromPromise, setup } from 'xstate'
@@ -129,14 +123,6 @@ export const engineStreamMachine = setup({
         await holdOntoVideoFrameInCanvas(video, canvas)
         video.style.display = 'none'
 
-        // Before doing anything else clear the cache
-        // Originally I (lee) had this on the reconnect but it was interfering
-        // with kclManager.executeCode()?
-        await rustContext.clearSceneAndBustCache(
-          { settings: await jsAppSettings() },
-          codeManager.currentFilePath || undefined
-        )
-
         await sceneInfra.camControls.saveRemoteCameraState()
 
         // Make sure we're on the next frame for no flickering between canvas