From d915787840b59f37ebe44733dbd6eb91a5c4f45c Mon Sep 17 00:00:00 2001
From: skier233 <39396856+skier233@users.noreply.github.com>
Date: Sun, 23 Feb 2025 22:29:59 -0500
Subject: [PATCH] Fix markers ui bug (#5671)

* Move loadMarkers to separate callback
* Remove async from findColors
---------
Co-authored-by: WithoutPants <53250216+WithoutPants@users.noreply.github.com>
---
 .../components/ScenePlayer/ScenePlayer.tsx    | 126 +++++++++---------
 ui/v2.5/src/components/ScenePlayer/markers.ts |   6 +-
 2 files changed, 67 insertions(+), 65 deletions(-)

diff --git a/ui/v2.5/src/components/ScenePlayer/ScenePlayer.tsx b/ui/v2.5/src/components/ScenePlayer/ScenePlayer.tsx
index a9862b5a9..ea338a7f5 100644
--- a/ui/v2.5/src/components/ScenePlayer/ScenePlayer.tsx
+++ b/ui/v2.5/src/components/ScenePlayer/ScenePlayer.tsx
@@ -693,6 +693,60 @@ export const ScenePlayer: React.FC<IScenePlayerProps> = ({
     _initialTimestamp,
   ]);
 
+  const loadMarkers = useCallback(() => {
+    const player = getPlayer();
+    if (!player) return;
+
+    const markerData = scene.scene_markers.map((marker) => ({
+      title: getMarkerTitle(marker),
+      seconds: marker.seconds,
+      end_seconds: marker.end_seconds ?? null,
+      primaryTag: marker.primary_tag,
+    }));
+
+    const markers = player!.markers();
+    markers.clearMarkers();
+
+    const uniqueTagNames = markerData
+      .map((marker) => marker.primaryTag.name)
+      .filter((value, index, self) => self.indexOf(value) === index);
+
+    // Wait for colors
+    markers.findColors(uniqueTagNames);
+
+    const showRangeTags =
+      !ScreenUtils.isMobile() && (uiConfig?.showRangeMarkers ?? true);
+    const timestampMarkers: IMarker[] = [];
+    const rangeMarkers: IMarker[] = [];
+
+    if (!showRangeTags) {
+      for (const marker of markerData) {
+        timestampMarkers.push(marker);
+      }
+    } else {
+      for (const marker of markerData) {
+        if (marker.end_seconds === null) {
+          timestampMarkers.push(marker);
+        } else {
+          rangeMarkers.push(marker);
+        }
+      }
+    }
+
+    // Add markers in chunks
+    const CHUNK_SIZE = 10;
+    for (let i = 0; i < timestampMarkers.length; i += CHUNK_SIZE) {
+      const chunk = timestampMarkers.slice(i, i + CHUNK_SIZE);
+      requestAnimationFrame(() => {
+        chunk.forEach((m) => markers.addDotMarker(m));
+      });
+    }
+
+    requestAnimationFrame(() => {
+      markers.addRangeMarkers(rangeMarkers);
+    });
+  }, [getPlayer, scene, uiConfig]);
+
   useEffect(() => {
     const player = getPlayer();
     if (!player) return;
@@ -703,74 +757,22 @@ export const ScenePlayer: React.FC<IScenePlayerProps> = ({
       player.poster("");
     }
 
-    function loadMarkers() {
-      const loadMarkersAsync = async () => {
-        const markerData = scene.scene_markers.map((marker) => ({
-          title: getMarkerTitle(marker),
-          seconds: marker.seconds,
-          end_seconds: marker.end_seconds ?? null,
-          primaryTag: marker.primary_tag,
-        }));
+    // Define the event handler outside the useEffect
+    const handleLoadMetadata = () => {
+      loadMarkers();
+    };
 
-        const markers = player!.markers();
-        markers.clearMarkers();
-
-        const uniqueTagNames = markerData
-          .map((marker) => marker.primaryTag.name)
-          .filter((value, index, self) => self.indexOf(value) === index);
-
-        // Wait for colors
-        await markers.findColors(uniqueTagNames);
-
-        const showRangeTags =
-          !ScreenUtils.isMobile() && (uiConfig?.showRangeMarkers ?? true);
-        const timestampMarkers: IMarker[] = [];
-        const rangeMarkers: IMarker[] = [];
-
-        if (!showRangeTags) {
-          for (const marker of markerData) {
-            timestampMarkers.push(marker);
-          }
-        } else {
-          for (const marker of markerData) {
-            if (marker.end_seconds === null) {
-              timestampMarkers.push(marker);
-            } else {
-              rangeMarkers.push(marker);
-            }
-          }
-        }
-
-        // Add markers in chunks
-        const CHUNK_SIZE = 10;
-        for (let i = 0; i < timestampMarkers.length; i += CHUNK_SIZE) {
-          const chunk = timestampMarkers.slice(i, i + CHUNK_SIZE);
-          requestAnimationFrame(() => {
-            chunk.forEach((m) => markers.addDotMarker(m));
-          });
-        }
-
-        requestAnimationFrame(() => {
-          markers.addRangeMarkers(rangeMarkers);
-        });
-      };
-
-      // Call our async function
-      void loadMarkersAsync();
-    }
     // Ensure markers are added after player is fully ready and sources are loaded
     if (player.readyState() >= 1) {
       loadMarkers();
-      return;
     } else {
-      player.on("loadedmetadata", () => {
-        loadMarkers();
-      });
-      return () => {
-        player.off("loadedmetadata", loadMarkers);
-      };
+      player.on("loadedmetadata", handleLoadMetadata);
     }
-  }, [getPlayer, scene, uiConfig]);
+
+    return () => {
+      player.off("loadedmetadata", handleLoadMetadata);
+    };
+  }, [getPlayer, scene, loadMarkers]);
 
   useEffect(() => {
     const player = getPlayer();
diff --git a/ui/v2.5/src/components/ScenePlayer/markers.ts b/ui/v2.5/src/components/ScenePlayer/markers.ts
index 2b232d686..730e09245 100644
--- a/ui/v2.5/src/components/ScenePlayer/markers.ts
+++ b/ui/v2.5/src/components/ScenePlayer/markers.ts
@@ -259,11 +259,11 @@ class MarkersPlugin extends videojs.getPlugin("plugin") {
   }
 
   // Implementing the findColors method
-  async findColors(tagNames: string[]) {
+  findColors(tagNames: string[]) {
     // Compute base hues for each tag
     const baseHues: { [tag: string]: number } = {};
     for (const tag of tagNames) {
-      baseHues[tag] = await this.computeBaseHue(tag);
+      baseHues[tag] = this.computeBaseHue(tag);
     }
 
     // Adjust hues to avoid similar colors
@@ -278,7 +278,7 @@ class MarkersPlugin extends videojs.getPlugin("plugin") {
   // Helper methods translated from Python
 
   // Compute base hue from tag name
-  private async computeBaseHue(tag: string): Promise<number> {
+  private computeBaseHue(tag: string): number {
     const hash = CryptoJS.SHA256(tag);
     const hashHex = hash.toString(CryptoJS.enc.Hex);
     const hashInt = BigInt(`0x${hashHex}`);