fix unreliable channel (#2329)

* fix unreliable channel

* add test for hovering

e2e/playwright/flow-tests.spec.ts

@@ -12,6 +12,7 @@ import {
   TEST_SETTINGS_ONBOARDING_START,
 } from './storageStates'
 import * as TOML from '@iarna/toml'
+import { Coords2d } from 'lang/std/sketch'
 
 /*
 debug helper: unfortunately we do rely on exact coord mouse clicks in a few places
@@ -1264,6 +1265,72 @@ test('ProgramMemory can be serialised', async ({ page }) => {
   })
 })
 
+test('Hovering over 3d features highlights code', async ({ page }) => {
+  const u = getUtils(page)
+  await page.addInitScript(async () => {
+    localStorage.setItem(
+      'persistCode',
+      `const part001 = startSketchOn('-XZ')
+  |> startProfileAt([20, 0], %)
+  |> line([7.13, 4 + 0], %)
+  |> angledLine({ angle: 3 + 0, length: 3.14 + 0 }, %)
+  |> lineTo([20.14 + 0, -0.14 + 0], %)
+  |> xLineTo(29 + 0, %)
+  |> yLine(-3.14 + 0, %, 'a')
+  |> xLine(1.63, %)
+  |> angledLineOfXLength({ angle: 3 + 0, length: 3.14 }, %)
+  |> angledLineOfYLength({ angle: 30, length: 3 + 0 }, %)
+  |> angledLineToX({ angle: 22.14 + 0, to: 12 }, %)
+  |> angledLineToY({ angle: 30, to: 11.14 }, %)
+  |> angledLineThatIntersects({
+        angle: 3.14,
+        intersectTag: 'a',
+        offset: 0
+      }, %)
+  |> tangentialArcTo([13.14 + 0, 13.14], %)
+  |> close(%)
+  |> extrude(5 + 7, %)    
+`
+    )
+  })
+  await page.setViewportSize({ width: 1000, height: 500 })
+  await page.goto('/')
+  await u.waitForAuthSkipAppStart()
+
+  const extrusionTop: Coords2d = [800, 240]
+  const flatExtrusionFace: Coords2d = [960, 160]
+  const arc: Coords2d = [840, 160]
+  const close: Coords2d = [720, 200]
+  const nothing: Coords2d = [600, 200]
+
+  await page.mouse.move(nothing[0], nothing[1])
+  await page.mouse.click(nothing[0], nothing[1])
+
+  await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
+  await page.waitForTimeout(200)
+
+  await page.mouse.move(extrusionTop[0], extrusionTop[1])
+  await expect(page.getByTestId('hover-highlight')).toBeVisible()
+  await page.mouse.move(nothing[0], nothing[1])
+  await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
+
+  await page.mouse.move(arc[0], arc[1])
+  await expect(page.getByTestId('hover-highlight')).toBeVisible()
+  await page.mouse.move(nothing[0], nothing[1])
+  await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
+
+  await page.mouse.move(close[0], close[1])
+  await expect(page.getByTestId('hover-highlight')).toBeVisible()
+  await page.mouse.move(nothing[0], nothing[1])
+  await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
+
+  await page.mouse.move(flatExtrusionFace[0], flatExtrusionFace[1])
+  await expect(page.getByTestId('hover-highlight')).toHaveCount(5) // multiple lines
+  await page.mouse.move(nothing[0], nothing[1])
+  await page.waitForTimeout(100)
+  await expect(page.getByTestId('hover-highlight')).not.toBeVisible()
+})
+
 test("Various pipe expressions should and shouldn't allow edit and or extrude", async ({
   page,
 }) => {

src/hooks/useEngineConnectionSubscriptions.ts

@@ -14,8 +14,8 @@ export function useEngineConnectionSubscriptions() {
       event: 'highlight_set_entity',
       callback: ({ data }) => {
         if (data?.entity_id) {
-          const sourceRange =
+          const sourceRange = engineCommandManager.artifactMap?.[data.entity_id]
-            engineCommandManager.artifactMap?.[data.entity_id]?.range
+            ?.range || [0, 0]
           editorManager.setHighlightRange(sourceRange)
         } else if (
           !editorManager.highlightRange ||

src/lang/std/engineConnection.ts

@@ -542,6 +542,27 @@ class EngineConnection {
             },
           }
         })
+        this.unreliableDataChannel.addEventListener('message', (event) => {
+          const result: UnreliableResponses = JSON.parse(event.data)
+          Object.values(
+            this.engineCommandManager.unreliableSubscriptions[result.type] || {}
+          ).forEach(
+            // TODO: There is only one response that uses the unreliable channel atm,
+            // highlight_set_entity, if there are more it's likely they will all have the same
+            // sequence logic, but I'm not sure if we use a single global sequence or a sequence
+            // per unreliable subscription.
+            (callback) => {
+              if (
+                result.type === 'highlight_set_entity' &&
+                result?.data?.sequence &&
+                result?.data.sequence > this.engineCommandManager.inSequence
+              ) {
+                this.engineCommandManager.inSequence = result.data.sequence
+                callback(result)
+              }
+            }
+          )
+        })
       })
     }
 
@@ -853,7 +874,7 @@ type CommandTypes = Models['ModelingCmd_type']['type'] | 'batch'
 
 type UnreliableResponses = Extract<
   Models['OkModelingCmdResponse_type'],
-  { type: 'highlight_set_entity' }
+  { type: 'highlight_set_entity' | 'camera_drag_move' }
 >
 interface UnreliableSubscription<T extends UnreliableResponses['type']> {
   event: T
@@ -1061,32 +1082,6 @@ export class EngineCommandManager {
         setIsStreamReady(false)
       },
       onConnectionStarted: (engineConnection) => {
-        engineConnection?.pc?.addEventListener('datachannel', (event) => {
-          let unreliableDataChannel = event.channel
-
-          unreliableDataChannel.addEventListener('message', (event) => {
-            const result: UnreliableResponses = JSON.parse(event.data)
-            Object.values(
-              this.unreliableSubscriptions[result.type] || {}
-            ).forEach(
-              // TODO: There is only one response that uses the unreliable channel atm,
-              // highlight_set_entity, if there are more it's likely they will all have the same
-              // sequence logic, but I'm not sure if we use a single global sequence or a sequence
-              // per unreliable subscription.
-              (callback) => {
-                if (
-                  result?.data?.sequence &&
-                  result?.data.sequence > this.inSequence &&
-                  result.type === 'highlight_set_entity'
-                ) {
-                  this.inSequence = result.data.sequence
-                  callback(result)
-                }
-              }
-            )
-          })
-        })
-
         // When the EngineConnection starts a connection, we want to register
         // callbacks into the WebSocket/PeerConnection.
         engineConnection.websocket?.addEventListener('message', (event) => {
@@ -1366,14 +1361,11 @@ export class EngineCommandManager {
     callback,
   }: Subscription<T>): () => void {
     const localUnsubscribeId = uuidv4()
-    const otherEventCallbacks = this.subscriptions[event]
+    if (!this.subscriptions[event]) {
-    if (otherEventCallbacks) {
+      this.subscriptions[event] = {}
-      otherEventCallbacks[localUnsubscribeId] = callback
-    } else {
-      this.subscriptions[event] = {
-        [localUnsubscribeId]: callback,
-      }
     }
+    this.subscriptions[event][localUnsubscribeId] = callback
+
     return () => this.unSubscribeTo(event, localUnsubscribeId)
   }
   private unSubscribeTo(event: ModelTypes, id: string) {
@@ -1384,14 +1376,10 @@ export class EngineCommandManager {
     callback,
   }: UnreliableSubscription<T>): () => void {
     const localUnsubscribeId = uuidv4()
-    const otherEventCallbacks = this.unreliableSubscriptions[event]
+    if (!this.unreliableSubscriptions[event]) {
-    if (otherEventCallbacks) {
+      this.unreliableSubscriptions[event] = {}
-      otherEventCallbacks[localUnsubscribeId] = callback
-    } else {
-      this.unreliableSubscriptions[event] = {
-        [localUnsubscribeId]: callback,
-      }
     }
+    this.unreliableSubscriptions[event][localUnsubscribeId] = callback
     return () => this.unSubscribeToUnreliable(event, localUnsubscribeId)
   }
   private unSubscribeToUnreliable(