diff --git a/packages/core/src/SceneManager.ts b/packages/core/src/SceneManager.ts index 92de60d66b..ab27c57061 100644 --- a/packages/core/src/SceneManager.ts +++ b/packages/core/src/SceneManager.ts @@ -94,9 +94,12 @@ export class SceneManager { const scenePromise = this.engine.resourceManager.load({ url, type: AssetType.Scene }); scenePromise.then((scene: Scene) => { if (destroyOldScene) { - const scenes = this._scenes.getArray(); + // Use loop array because `destroy` removes the scene from `_scenes` during iteration, + // and skip `scene` itself because concurrent loads of the same url resolve to the same instance + const scenes = this._scenes.getLoopArray(); for (let i = 0, n = scenes.length; i < n; i++) { - scenes[i].destroy(); + const oldScene = scenes[i]; + oldScene !== scene && oldScene.destroy(); } } this.addScene(scene); diff --git a/packages/loader/src/SceneLoader.ts b/packages/loader/src/SceneLoader.ts index 7c94a82109..43963f5437 100644 --- a/packages/loader/src/SceneLoader.ts +++ b/packages/loader/src/SceneLoader.ts @@ -167,7 +167,7 @@ export function applySceneData( return Promise.all(promises).then(() => {}); } -@resourceLoader(AssetType.Scene, ["scene"], true) +@resourceLoader(AssetType.Scene, ["scene"], false) class SceneLoader extends Loader { load(item: LoadItem, resourceManager: ResourceManager): AssetPromise { const { engine } = resourceManager; diff --git a/tests/src/core/resource/SceneLoaderCache.test.ts b/tests/src/core/resource/SceneLoaderCache.test.ts new file mode 100644 index 0000000000..2a9ee568f1 --- /dev/null +++ b/tests/src/core/resource/SceneLoaderCache.test.ts @@ -0,0 +1,98 @@ +import { AssetPromise, AssetType, ResourceManager, Scene } from "@galacean/engine-core"; +import "@galacean/engine-loader"; +import { WebGLEngine } from "@galacean/engine"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; + +describe("SceneLoader cache policy", () => { + let engine: WebGLEngine; + + beforeAll(async () => { + engine = await WebGLEngine.create({ canvas: document.createElement("canvas") }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + afterAll(() => { + engine.destroy(); + }); + + it("SceneLoader should have useCache disabled", () => { + const sceneLoader = ResourceManager._loaders[AssetType.Scene]; + expect(sceneLoader).to.not.be.undefined; + expect(sceneLoader.useCache).to.eq(false); + }); + + it("PrimitiveMeshLoader should also have useCache disabled (existing convention)", () => { + const loader = ResourceManager._loaders[AssetType.PrimitiveMesh]; + expect(loader).to.not.be.undefined; + expect(loader.useCache).to.eq(false); + }); + + it("Texture loader should still have useCache enabled (immutable asset)", () => { + const loader = ResourceManager._loaders[AssetType.Texture]; + expect(loader).to.not.be.undefined; + expect(loader.useCache).to.eq(true); + }); + + describe("loadScene behavior", () => { + function mockSceneLoad() { + const loader = ResourceManager._loaders[AssetType.Scene]; + return vi.spyOn(loader, "load").mockImplementation( + () => + new AssetPromise((resolve) => { + resolve(new Scene(engine, "mock")); + }) + ); + } + + it("should activate a fresh Scene instance and destroy the old one on sequential loads of the same url", async () => { + mockSceneLoad(); + const sceneManager = engine.sceneManager; + + const first = await sceneManager.loadScene("/mock-sequential.scene"); + const second = await sceneManager.loadScene("/mock-sequential.scene"); + + expect(second).not.toBe(first); + expect(first.destroyed).toBe(true); + expect(second.destroyed).toBe(false); + expect(sceneManager.scenes[0]).toBe(second); + }); + + it("should destroy every old scene when multiple scenes are active", async () => { + mockSceneLoad(); + const sceneManager = engine.sceneManager; + sceneManager.addScene(new Scene(engine, "extra1")); + sceneManager.addScene(new Scene(engine, "extra2")); + const oldScenes = [...sceneManager.scenes]; + expect(oldScenes.length).toBeGreaterThanOrEqual(2); + + const scene = await sceneManager.loadScene("/mock-multi.scene"); + + for (const oldScene of oldScenes) { + expect(oldScene.destroyed).toBe(true); + } + expect(scene.destroyed).toBe(false); + expect(sceneManager.scenes.length).toBe(1); + expect(sceneManager.scenes[0]).toBe(scene); + }); + + it("should not destroy the activated scene when concurrent loads of the same url share one loading promise", async () => { + const loadSpy = mockSceneLoad(); + const sceneManager = engine.sceneManager; + + const [first, second] = await Promise.all([ + sceneManager.loadScene("/mock-concurrent.scene"), + sceneManager.loadScene("/mock-concurrent.scene") + ]); + + // The in-flight loading promise is shared regardless of useCache + expect(loadSpy).toHaveBeenCalledTimes(1); + expect(second).toBe(first); + expect(first.destroyed).toBe(false); + expect(sceneManager.scenes.length).toBe(1); + expect(sceneManager.scenes[0]).toBe(first); + }); + }); +});