From e29345fbf6601fbccda147f12e30ddd26e1cadf0 Mon Sep 17 00:00:00 2001 From: Frank Noirot Date: Mon, 12 Aug 2024 09:54:16 -0400 Subject: [PATCH] Move executeCode out of routeLoaders, into shared space in Stream (#3332) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Move executeCode out of routeLoaders, into shared space in Stream * Update src/components/Stream.tsx Co-authored-by: Jonathan Tran * Remove unused dependency * file switching useEffect should depend on engineConnection * A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu) * Re-run CI * Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)" This reverts commit d97e74a48b48966bb98340e76445efa03a19c54c. * Post-merge fix up --------- Co-authored-by: Jonathan Tran Co-authored-by: github-actions[bot] Co-authored-by: Jess Frazelle Co-authored-by: Kurt Hutten Irev-Dev --- src/components/Stream.tsx | 64 +++++++++++++++++++++++++++++---------- src/lib/routeLoaders.ts | 5 +-- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/components/Stream.tsx b/src/components/Stream.tsx index f3f84aed1..e852b22d3 100644 --- a/src/components/Stream.tsx +++ b/src/components/Stream.tsx @@ -15,6 +15,9 @@ import { EngineConnectionStateType, DisconnectingType, } from 'lang/std/engineConnection' +import { useRouteLoaderData } from 'react-router-dom' +import { PATHS } from 'lib/paths' +import { IndexLoaderData } from 'lib/types' enum StreamState { Playing = 'playing', @@ -32,6 +35,7 @@ export const Stream = () => { const { mediaStream } = useAppStream() const { overallState, immediateState } = useNetworkContext() const [streamState, setStreamState] = useState(StreamState.Unset) + const { file } = useRouteLoaderData(PATHS.FILE) as IndexLoaderData const IDLE = settings.context.app.streamIdleMode.current @@ -39,6 +43,39 @@ export const Stream = () => { overallState === NetworkHealthState.Ok || overallState === NetworkHealthState.Weak + /** + * Execute code and show a "building scene message" + * in Stream.tsx in the meantime. + * + * I would like for this to live somewhere more central, + * but it seems to me that we need the video element ref + * to be able to play the video after the code has been + * executed. If we can find a way to do this from a more + * central place, we can move this code there. + */ + async function executeCodeAndPlayStream() { + kclManager.isFirstRender = true + kclManager.executeCode(true).then(() => { + videoRef.current?.play().catch((e) => { + console.warn('Video playing was prevented', e, videoRef.current) + }) + kclManager.isFirstRender = false + setStreamState(StreamState.Playing) + }) + } + + /** + * Subscribe to execute code when the file changes + * but only if the scene is already ready. + * See onSceneReady for the initial scene setup. + */ + useEffect(() => { + if (engineCommandManager.engineConnection?.isReady() && file?.path) { + console.log('execute on file change') + executeCodeAndPlayStream() + } + }, [file?.path, engineCommandManager.engineConnection]) + useEffect(() => { if ( immediateState.type === EngineConnectionStateType.Disconnecting && @@ -135,26 +172,19 @@ export const Stream = () => { timeoutIdIdleB = setTimeout(teardown, IDLE_TIME_MS) } - const onSceneReady = () => { - kclManager.isFirstRender = true - setStreamState(StreamState.Playing) - kclManager.executeCode(true).then(() => { - videoRef.current?.play().catch((e) => { - console.warn('Video playing was prevented', e, videoRef.current) - }) - kclManager.isFirstRender = false - }) - } - + /** + * Add a listener to execute code and play the stream + * on initial stream setup. + */ engineCommandManager.addEventListener( EngineCommandManagerEvents.SceneReady, - onSceneReady + executeCodeAndPlayStream ) return () => { engineCommandManager.removeEventListener( EngineCommandManagerEvents.SceneReady, - onSceneReady + executeCodeAndPlayStream ) globalThis?.window?.document?.removeEventListener('paste', handlePaste, { capture: true, @@ -185,16 +215,18 @@ export const Stream = () => { } }, [IDLE, streamState]) - // HOT FIX: for https://github.com/KittyCAD/modeling-app/pull/3250 - // TODO review if there's a better way to play the stream again. + /** + * Play the vid + */ useEffect(() => { - if (!kclManager.isFirstRender) + if (!kclManager.isFirstRender) { setTimeout(() => // execute in the next event loop videoRef.current?.play().catch((e) => { console.warn('Video playing was prevented', e, videoRef.current) }) ) + } }, [kclManager.isFirstRender]) useEffect(() => { diff --git a/src/lib/routeLoaders.ts b/src/lib/routeLoaders.ts index a03b168bd..f7af34912 100644 --- a/src/lib/routeLoaders.ts +++ b/src/lib/routeLoaders.ts @@ -12,7 +12,7 @@ import { loadAndValidateSettings } from './settings/settingsUtils' import makeUrlPathRelative from './makeUrlPathRelative' import { sep } from '@tauri-apps/api/path' import { readTextFile } from '@tauri-apps/plugin-fs' -import { codeManager, kclManager } from 'lib/singletons' +import { codeManager } from 'lib/singletons' import { fileSystemManager } from 'lang/std/fileSystemManager' import { getProjectInfo, @@ -105,9 +105,6 @@ export const fileLoader: LoaderFunction = async ({ codeManager.updateCurrentFilePath(current_file_path) codeManager.updateCodeStateEditor(code) - // We don't want to call await on execute code since we don't want to block the UI - kclManager.executeCode(true) - // Set the file system manager to the project path // So that WASM gets an updated path for operations fileSystemManager.dir = project_path