Add back better ping indicator

src/clientSideScene/CameraControls.ts

@@ -975,6 +975,14 @@ export class CameraControls {
         view: this.oldCameraState,
       },
     })
+
+    await this.engineCommandManager.sendSceneCommand({
+      type: 'modeling_cmd_req',
+      cmd_id: uuidv4(),
+      cmd: {
+        type: 'default_camera_get_settings',
+      },
+    })
   }
 
   async saveRemoteCameraState() {

src/components/ModelStateIndicator.tsx

@@ -10,22 +10,8 @@ import {
 } from '@fortawesome/free-solid-svg-icons'
 
 export const ModelStateIndicator = () => {
-  const [commands] = useEngineCommands()
-  const [isDone, setIsDone] = useState<boolean>(false)
-
   const engineStreamState = useSelector(engineStreamActor, (state) => state)
 
-  const lastCommandType = commands[commands.length - 1]?.type
-
-  useEffect(() => {
-    if (lastCommandType === CommandLogType.SetDefaultSystemProperties) {
-      setIsDone(false)
-    }
-    if (lastCommandType === CommandLogType.ExecutionDone) {
-      setIsDone(true)
-    }
-  }, [lastCommandType])
-
   let className = 'w-6 h-6 '
   let icon = <div className={className}></div>
   let dataTestId = 'model-state-indicator'
@@ -38,15 +24,7 @@ export const ModelStateIndicator = () => {
       width="20"
       height="20"
     />
-  } else if (engineStreamState.value === EngineStreamState.Resuming) {
-    className += 'text-secondary'
-    icon = <FontAwesomeIcon
-      data-testid={dataTestId + '-resuming'}
-      icon={faSpinner}
-      width="20"
-      height="20"
-    />
-  } else if (isDone) {
+  } else if (engineStreamState.value === EngineStreamState.Playing) {
     className += 'text-secondary'
     icon = (
         <FontAwesomeIcon
@@ -56,6 +34,14 @@ export const ModelStateIndicator = () => {
           height="20"
         />
     )
+  } else {
+    className += 'text-secondary'
+    icon = <FontAwesomeIcon
+      data-testid={dataTestId + '-resuming'}
+      icon={faSpinner}
+      width="20"
+      height="20"
+    />
   }
 
   return (

src/components/NetworkHealthIndicator.tsx

@@ -86,6 +86,7 @@ export const NetworkHealthIndicator = () => {
     error,
     setHasCopied,
     hasCopied,
+    ping
   } = useNetworkContext()
 
   return (
@@ -129,6 +130,17 @@ export const NetworkHealthIndicator = () => {
             {NETWORK_HEALTH_TEXT[overallState]}
           </p>
         </div>
+        <div
+          className={`flex items-center justify-between p-2 rounded-t-sm`}
+        >
+          <h2 className={`text-xs font-sans font-normal ${overallConnectionStateColor[overallState].icon}`}>Ping</h2>
+          <p
+            data-testid="network"
+            className={`font-bold text-xs uppercase px-2 py-1 rounded-sm ${overallConnectionStateColor[overallState].icon}`}
+          >
+            {ping ?? 'N/A'}
+          </p>
+        </div>
         <ul className="divide-y divide-chalkboard-20 dark:divide-chalkboard-80">
           {Object.keys(steps).map((name) => (
             <li

src/hooks/useNetworkContext.tsx

@@ -25,7 +25,7 @@ export const NetworkContext = createContext<NetworkStatus>({
   error: undefined,
   setHasCopied: (b: boolean) => {},
   hasCopied: false,
-  pingPongHealth: undefined,
+  ping: undefined,
 } as NetworkStatus)
 export const useNetworkContext = () => {
   return useContext(NetworkContext)

src/hooks/useNetworkStatus.tsx

@@ -32,7 +32,7 @@ export interface NetworkStatus {
   error: ErrorType | undefined
   setHasCopied: (b: boolean) => void
   hasCopied: boolean
-  pingPongHealth: undefined | 'OK' | 'TIMEOUT'
+  ping: undefined | number
 }
 
 // Must be called from one place in the application.
@@ -48,9 +48,7 @@ export function useNetworkStatus() {
   const [overallState, setOverallState] = useState<NetworkHealthState>(
     NetworkHealthState.Disconnected
   )
-  const [pingPongHealth, setPingPongHealth] = useState<
-    undefined | 'OK' | 'TIMEOUT'
-  >(undefined)
+  const [ping, setPing] = useState<undefined | number>(undefined)
   const [hasCopied, setHasCopied] = useState<boolean>(false)
 
   const [error, setError] = useState<ErrorType | undefined>(undefined)
@@ -72,12 +70,12 @@ export function useNetworkStatus() {
       !internetConnected
         ? NetworkHealthState.Disconnected
         : hasIssues || hasIssues === undefined
-          ? NetworkHealthState.Issue
-          : pingPongHealth === 'TIMEOUT'
-            ? NetworkHealthState.Weak
-            : NetworkHealthState.Ok
+        ? NetworkHealthState.Issue
+        : ping > (16.6*3) // we consider ping longer than 3 frames as weak
+        ? NetworkHealthState.Weak
+        : NetworkHealthState.Ok
     )
-  }, [hasIssues, internetConnected, pingPongHealth])
+  }, [hasIssues, internetConnected, ping])
 
   useEffect(() => {
     const onlineCallback = () => {
@@ -128,7 +126,8 @@ export function useNetworkStatus() {
 
   useEffect(() => {
     const onPingPongChange = ({ detail: state }: CustomEvent) => {
-      setPingPongHealth(state)
+      console.log(state)
+      setPing(state)
     }
 
     const onConnectionStateChange = ({
@@ -233,6 +232,6 @@ export function useNetworkStatus() {
     error,
     setHasCopied,
     hasCopied,
-    pingPongHealth,
+    ping,
   }
 }

src/lang/std/engineConnection.ts

@@ -20,8 +20,7 @@ import {
 import { reportRejection } from '@src/lib/trap'
 import { binaryToUuid, uuidv4 } from '@src/lib/utils'
 
-// TODO(paultag): This ought to be tweakable.
-const pingIntervalMs = 5_000
+const pingIntervalMs = 1_000
 
 function isHighlightSetEntity_type(
   data: any
@@ -191,8 +190,6 @@ export type EngineConnectionState =
   | State<EngineConnectionStateType.Disconnecting, DisconnectingValue>
   | State<EngineConnectionStateType.Disconnected, void>
 
-export type PingPongState = 'OK' | 'TIMEOUT'
-
 export enum EngineConnectionEvents {
   // Fires for each ping-pong success or failure.
   PingPongChanged = 'ping-pong-changed', // (state: PingPongState) => void
@@ -333,70 +330,17 @@ class EngineConnection extends EventTarget {
       return
     }
 
-    // Without an interval ping, our connection will timeout.
-    // If this.idleMode is true we skip this logic so only reconnect
-    // happens on mouse move
     this.pingIntervalId = setInterval(() => {
-      if (this.idleMode) return
+      // Only start a new ping when the other is fulfilled.
+      if (this.pingPongSpan.ping) { return }
 
-      switch (this.state.type as EngineConnectionStateType) {
-        case EngineConnectionStateType.ConnectionEstablished:
-          // If there was no reply to the last ping, report a timeout and
-          // teardown the connection.
-          if (this.pingPongSpan.ping && !this.pingPongSpan.pong) {
-            this.dispatchEvent(
-              new CustomEvent(EngineConnectionEvents.PingPongChanged, {
-                detail: 'TIMEOUT',
-              })
-            )
-            this.state = {
-              type: EngineConnectionStateType.Disconnecting,
-              value: {
-                type: DisconnectingType.Timeout,
-              },
-            }
-            this.disconnectAll()
+      // Don't start pinging until we're connected.
+      if (this.state.type !== EngineConnectionStateType.ConnectionEstablished) { return }
 
-            // Otherwise check the time between was >= pingIntervalMs,
-            // and if it was, then it's bad network health.
-          } else if (this.pingPongSpan.ping && this.pingPongSpan.pong) {
-            if (
-              Math.abs(
-                this.pingPongSpan.pong.valueOf() -
-                  this.pingPongSpan.ping.valueOf()
-              ) >= pingIntervalMs
-            ) {
-              this.dispatchEvent(
-                new CustomEvent(EngineConnectionEvents.PingPongChanged, {
-                  detail: 'TIMEOUT',
-                })
-              )
-            } else {
-              this.dispatchEvent(
-                new CustomEvent(EngineConnectionEvents.PingPongChanged, {
-                  detail: 'OK',
-                })
-              )
-            }
-          }
-
-          this.send({ type: 'ping' })
-          this.pingPongSpan.ping = new Date()
-          this.pingPongSpan.pong = undefined
-          break
-        case EngineConnectionStateType.Disconnecting:
-        case EngineConnectionStateType.Disconnected:
-          // We will do reconnection elsewhere, because we basically need
-          // to destroy this EngineConnection, and this setInterval loop
-          // lives inside it. (lee) I might change this in the future so it's
-          // outside this class.
-          break
-        default:
-          if (this.isConnecting()) break
-          // Means we never could do an initial connection. Reconnect everything.
-          if (!this.pingPongSpan.ping)
-            this.connect({ reconnect: false }).catch(reportRejection)
-          break
+      this.send({ type: 'ping' })
+      this.pingPongSpan = {
+        ping: new Date(),
+        pong: undefined
       }
     }, pingIntervalMs)
 
@@ -1054,6 +998,14 @@ class EngineConnection extends EventTarget {
           switch (resp.type) {
             case 'pong':
               this.pingPongSpan.pong = new Date()
+              this.dispatchEvent(
+                new CustomEvent(EngineConnectionEvents.PingPongChanged, {
+                  detail: Math.min(999, Math.floor(this.pingPongSpan.pong - this.pingPongSpan.ping)),
+                })
+              )
+              // Clear the initial ping so our interval ping loop can fire again
+              // But only after using it above!
+              this.pingPongSpan.ping = undefined
               break
 
             case 'modeling_session_data':