refactor: enhance and fix bugs on asset management

.changeset/sunny-views-pull.md

@@ -0,0 +1,6 @@
+---
+"@jolly-pixel/runtime": minor
+"@jolly-pixel/engine": minor
+---
+
+Enhance and fix bugs with Asset Management

packages/engine/src/systems/asset/Base.ts

@@ -3,10 +3,9 @@ import { parse } from "../../utils/path.ts";
 
 export type AssetTypeName =
   | "unknown"
-  | "texture"
-  | "audio"
   | "model"
   | "font"
+  | "tilemap"
   | (string & {});
 
 export class Asset {

packages/engine/src/systems/asset/Manager.ts

@@ -38,12 +38,16 @@ export class AssetManager {
       asset.type = this.registry.getTypeForExt(asset.longExt);
     }
 
-    this.waiting.enqueue(asset);
-    this.scheduleAutoload(this.context);
+    const path = asset.toString();
+
+    if (!this.assets.has(path)) {
+      this.waiting.enqueue(asset);
+      this.scheduleAutoload(this.context);
+    }
 
     return {
       asset,
-      get: () => this.get<T>(asset.id)
+      get: () => this.get<T>(path)
     };
   }
 
@@ -52,19 +56,19 @@ export class AssetManager {
   }
 
   get<T>(
-    id: string
+    path: string
   ): T {
-    if (this.assets.has(id)) {
-      return this.assets.get(id) as T;
+    if (this.assets.has(path)) {
+      return this.assets.get(path) as T;
     }
 
-    throw new Error(`Asset with id ${id} not found.`);
+    throw new Error(`Asset "${path}" is not yet loaded.`);
   }
 
   scheduleAutoload(
     context: AssetLoaderContext
   ) {
-    if (this.context) {
+    if (context) {
       this.context = context;
     }
 
@@ -100,7 +104,7 @@ export class AssetManager {
       onStart?.(asset);
 
       const result = await loader(asset, context);
-      this.assets.set(asset.id, result);
+      this.assets.set(asset.toString(), result);
     };
 
     const loadingPromises = assets.map((asset) => loadAsset(asset));

packages/engine/src/systems/asset/Queue.ts

@@ -3,20 +3,30 @@ import { Asset } from "./Base.ts";
 
 export class AssetQueue {
   #assets: Asset[] = [];
+  #pendingPaths = new Set<string>();
 
   get size() {
     return this.#assets.length;
   }
 
   enqueue(
     asset: Asset
-  ) {
+  ): boolean {
+    const path = asset.toString();
+    if (this.#pendingPaths.has(path)) {
+      return false;
+    }
+
     this.#assets.push(asset);
+    this.#pendingPaths.add(path);
+
+    return true;
   }
 
   dequeueAll(): Asset[] {
     const assets = [...this.#assets];
     this.#assets = [];
+    this.#pendingPaths.clear();
 
     return assets;
   }

packages/engine/test/systems/asset/AssetManager.spec.ts

@@ -49,7 +49,7 @@ describe("Systems.AssetManager", () => {
     assert.strictEqual(queuedAssets[1].name, "test2");
   });
 
-  test("should get loaded asset by id", async() => {
+  test("should get loaded asset via lazy handle", async() => {
     assetManager.registry.loader(
       { type: "texture", extensions: [".png"] },
       mockLoader
@@ -58,14 +58,26 @@ describe("Systems.AssetManager", () => {
     const lazyAsset = assetManager.load("/test.png");
     await assetManager.loadAssets(assetManager.context);
 
-    const result = assetManager.get(lazyAsset.asset.id);
+    assert.strictEqual(lazyAsset.get(), "loaded-test");
+  });
+
+  test("should get loaded asset by path", async() => {
+    assetManager.registry.loader(
+      { type: "texture", extensions: [".png"] },
+      mockLoader
+    );
+
+    const lazyAsset = assetManager.load("/test.png");
+    await assetManager.loadAssets(assetManager.context);
+
+    const result = assetManager.get(lazyAsset.asset.toString());
     assert.strictEqual(result, "loaded-test");
   });
 
   test("should throw error when getting non-existent asset", () => {
     assert.throws(
-      () => assetManager.get("non-existent-id"),
-      /Asset with id non-existent-id not found/
+      () => assetManager.get("/non-existent.png"),
+      /Asset "\/non-existent\.png" is not yet loaded/
     );
   });
 
@@ -80,11 +92,8 @@ describe("Systems.AssetManager", () => {
 
     await assetManager.loadAssets(assetManager.context);
 
-    const result1 = assetManager.get(asset1.asset.id);
-    const result2 = assetManager.get(asset2.asset.id);
-
-    assert.strictEqual(result1, "loaded-image1");
-    assert.strictEqual(result2, "loaded-image2");
+    assert.strictEqual(asset1.get(), "loaded-image1");
+    assert.strictEqual(asset2.get(), "loaded-image2");
   });
 
   test("should call onStart callback for each asset", async() => {
@@ -136,4 +145,49 @@ describe("Systems.AssetManager", () => {
       () => assetManager.loadAssets(assetManager.context)
     );
   });
+
+  test("should deduplicate assets loaded from the same path", async() => {
+    let loadCount = 0;
+    assetManager.registry.loader(
+      { type: "model", extensions: [".glb"] },
+      (asset, _context) => {
+        loadCount++;
+
+        return Promise.resolve(`loaded-${asset.name}`);
+      }
+    );
+
+    const handle1 = assetManager.load("/models/tree.glb");
+    const handle2 = assetManager.load("/models/tree.glb");
+
+    assert.strictEqual(assetManager.waiting.size, 1);
+
+    await assetManager.loadAssets(assetManager.context);
+
+    assert.strictEqual(loadCount, 1);
+    assert.strictEqual(handle1.get(), "loaded-tree");
+    assert.strictEqual(handle2.get(), "loaded-tree");
+  });
+
+  test("should not re-enqueue an already loaded asset", async() => {
+    let loadCount = 0;
+    assetManager.registry.loader(
+      { type: "model", extensions: [".glb"] },
+      (asset, _context) => {
+        loadCount++;
+
+        return Promise.resolve(`loaded-${asset.name}`);
+      }
+    );
+
+    const handle1 = assetManager.load("/models/tree.glb");
+    await assetManager.loadAssets(assetManager.context);
+
+    // Load the same path again after it is already cached
+    const handle2 = assetManager.load("/models/tree.glb");
+
+    assert.strictEqual(assetManager.waiting.size, 0);
+    assert.strictEqual(loadCount, 1);
+    assert.strictEqual(handle1.get(), handle2.get());
+  });
 });

packages/runtime/src/index.ts

@@ -59,6 +59,7 @@ export async function loadRuntime(
   // Make sure the focus is always on the game canvas wherever we click on the game window
   document.addEventListener("click", () => runtime.canvas.focus());
 
+  let initialized = false;
   try {
     if (loadingDelay > 0) {
       await timers.setTimeout(loadingDelay);
@@ -92,17 +93,30 @@ export async function loadRuntime(
           onStart: loadingComponent.setAsset.bind(loadingComponent)
         }
       );
-      await loadingCompletePromise;
+
+      // loadingCompletePromise resolves when the Three.js manager reports 100%.
+      // Loaders that bypass the manager (e.g. bare fetch) never trigger onProgress,
+      // so we force completion manually rather than hanging indefinitely.
+      if (loadingComplete) {
+        await loadingCompletePromise;
+      }
+      else {
+        loadingComponent.setProgress(1, 1);
+        await timers.setTimeout(100);
+      }
     }
 
     await loadingComponent.complete();
+    runtime.canvas.style.opacity = "1";
+    initialized = true;
   }
   catch (error: any) {
     loadingComponent.error(error);
   }
 
-  runtime.canvas.style.opacity = "1";
-  runtime.start();
+  if (initialized) {
+    runtime.start();
+  }
 }
 
 export { Runtime, type RuntimeOptions };