From e1b18a212faa0d7f076f63f96d7e949f744b158c Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Tue, 12 May 2026 10:42:13 +0800 Subject: [PATCH 1/4] fix(core): disable cache for SceneLoader to avoid loadScene self-destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scene is a live runtime tree, not an immutable asset. Caching the loaded Scene caused a self-destroy race when loadScene(url) was called with a URL whose cached Scene was the current active scene: 1. resourceManager.load({url}) returned the cached (= currently active) Scene instance 2. SceneManager.loadScene then entered the destroyOldScene branch and destroyed the old scenes — including the one just returned 3. The "new" scene was the same object, now destroyed: rootEntities empty, native PhysicsScene released, screen blank, no error logged The root cause is the cache-vs-construct conflict. A Scene is both an asset (a JSON blob on disk) and a constructed live runtime. Caching the constructed instance and returning it for "load same URL again" violates the user's intent ("reload this level"). Other engines (Unity SceneManager.LoadScene, Cocos director.loadScene, Unreal OpenLevel) all create a fresh Scene per load — none cache. Aligning Galacean to the same convention. `PrimitiveMeshLoader` and `ProjectLoader` already use `useCache: false` for similar reasons (constructed at load time, not immutable assets), so the pattern is established in the codebase. --- packages/loader/src/SceneLoader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; From 63b6b1f08b6fcb88d312ec8942a4e98e54673bb1 Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Tue, 12 May 2026 14:28:14 +0800 Subject: [PATCH 2/4] test(loader): verify SceneLoader useCache is disabled Adds regression tests for SceneLoader cache policy: - SceneLoader.useCache === false (the fix in this PR) - PrimitiveMeshLoader.useCache === false (existing convention, contrast) - Texture2D loader.useCache === true (immutable asset, contrast) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../core/resource/SceneLoaderCache.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 tests/src/core/resource/SceneLoaderCache.test.ts diff --git a/tests/src/core/resource/SceneLoaderCache.test.ts b/tests/src/core/resource/SceneLoaderCache.test.ts new file mode 100644 index 0000000000..cdcbd1dbb4 --- /dev/null +++ b/tests/src/core/resource/SceneLoaderCache.test.ts @@ -0,0 +1,30 @@ +import { AssetType, ResourceManager } from "@galacean/engine-core"; +import "@galacean/engine-loader"; +import { WebGLEngine } from "@galacean/engine"; +import { describe, beforeAll, expect, it } from "vitest"; + +describe("SceneLoader cache policy", () => { + let engine: WebGLEngine; + + beforeAll(async () => { + engine = await WebGLEngine.create({ canvas: document.createElement("canvas") }); + }); + + 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("Texture2D loader should still have useCache enabled (immutable asset)", () => { + const loader = ResourceManager._loaders[AssetType.Texture2D] || ResourceManager._loaders[AssetType.Texture]; + expect(loader).to.not.be.undefined; + expect(loader.useCache).to.eq(true); + }); +}); From c90f726f641e9462b2eeee4672584efbd4ebf7e6 Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Sun, 12 Jul 2026 23:50:23 +0800 Subject: [PATCH 3/4] fix(core): harden loadScene old-scene destruction - Iterate via getLoopArray: Scene.destroy() splices _scenes during the loop, so iterating the live array skips scenes and crashes reading undefined when multiple scenes are active - Skip the scene being activated: concurrent loads of the same url share one in-flight loading promise regardless of useCache, so the second resolution destroyed the scene the first had just activated Co-Authored-By: Claude Fable 5 --- packages/core/src/SceneManager.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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); From d47a55529b14ac2cf40fa4fe39a64235de81da45 Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Sun, 12 Jul 2026 23:50:27 +0800 Subject: [PATCH 4/4] test(loader): cover loadScene fresh-instance and destruction behavior Replace the Texture2D dead lookup (no loader is registered under that key) with AssetType.Texture, add engine teardown, and add behavior regression tests for sequential fresh-instance loads, multi-scene destruction and concurrent same-url loads Co-Authored-By: Claude Fable 5 --- .../core/resource/SceneLoaderCache.test.ts | 76 ++++++++++++++++++- 1 file changed, 72 insertions(+), 4 deletions(-) diff --git a/tests/src/core/resource/SceneLoaderCache.test.ts b/tests/src/core/resource/SceneLoaderCache.test.ts index cdcbd1dbb4..2a9ee568f1 100644 --- a/tests/src/core/resource/SceneLoaderCache.test.ts +++ b/tests/src/core/resource/SceneLoaderCache.test.ts @@ -1,7 +1,7 @@ -import { AssetType, ResourceManager } from "@galacean/engine-core"; +import { AssetPromise, AssetType, ResourceManager, Scene } from "@galacean/engine-core"; import "@galacean/engine-loader"; import { WebGLEngine } from "@galacean/engine"; -import { describe, beforeAll, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; describe("SceneLoader cache policy", () => { let engine: WebGLEngine; @@ -10,6 +10,14 @@ describe("SceneLoader cache policy", () => { 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; @@ -22,9 +30,69 @@ describe("SceneLoader cache policy", () => { expect(loader.useCache).to.eq(false); }); - it("Texture2D loader should still have useCache enabled (immutable asset)", () => { - const loader = ResourceManager._loaders[AssetType.Texture2D] || ResourceManager._loaders[AssetType.Texture]; + 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); + }); + }); });