diff --git a/packages/core/src/Entity.ts b/packages/core/src/Entity.ts index 9470d05817..a55158bd35 100644 --- a/packages/core/src/Entity.ts +++ b/packages/core/src/Entity.ts @@ -9,7 +9,7 @@ import { Script } from "./Script"; import { Transform } from "./Transform"; import { UpdateFlagManager } from "./UpdateFlagManager"; import { ReferResource } from "./asset/ReferResource"; -import { EngineObject } from "./base"; +import { EngineObject, Logger } from "./base"; import { CloneUtils } from "./clone/CloneUtils"; import { ComponentCloner } from "./clone/ComponentCloner"; import { ActiveChangeFlag } from "./enums/ActiveChangeFlag"; @@ -212,16 +212,14 @@ export class Entity extends EngineObject { } set siblingIndex(value: number) { - if (this._siblingIndex === -1) { - throw `The entity ${this.name} is not in the hierarchy`; - } - if (this._isRoot) { this._setSiblingIndex(this._scene._rootEntities, value); - } else { + } else if (this._parent) { const parent = this._parent; this._setSiblingIndex(parent._children, value); parent._dispatchModify(EntityModifyFlags.Child, parent); + } else { + Logger.warn(`The entity ${this.name} is not in the hierarchy`); } } @@ -403,6 +401,7 @@ export class Entity extends EngineObject { for (let i = children.length - 1; i >= 0; i--) { const child = children[i]; child._parent = null; + child._siblingIndex = -1; let activeChangeFlag = ActiveChangeFlag.None; child._isActiveInHierarchy && (activeChangeFlag |= ActiveChangeFlag.Hierarchy); @@ -410,8 +409,13 @@ export class Entity extends EngineObject { activeChangeFlag && child._processInActive(activeChangeFlag); Entity._traverseSetOwnerScene(child, null); // Must after child._processInActive(). + + child._setParentChange(); } children.length = 0; + // Dispatch a single `Child` modify event for the whole clear so subscribers + // (e.g. UICanvas) can invalidate their cached hierarchy state once. + this._dispatchModify(EntityModifyFlags.Child, this); } /** diff --git a/packages/core/src/SceneManager.ts b/packages/core/src/SceneManager.ts index 92de60d66b..984850fed4 100644 --- a/packages/core/src/SceneManager.ts +++ b/packages/core/src/SceneManager.ts @@ -94,7 +94,7 @@ export class SceneManager { const scenePromise = this.engine.resourceManager.load({ url, type: AssetType.Scene }); scenePromise.then((scene: Scene) => { if (destroyOldScene) { - const scenes = this._scenes.getArray(); + const scenes = this._scenes.getLoopArray(); for (let i = 0, n = scenes.length; i < n; i++) { scenes[i].destroy(); } diff --git a/tests/src/core/Entity.test.ts b/tests/src/core/Entity.test.ts index 0a428f4b60..ad9cdac5eb 100644 --- a/tests/src/core/Entity.test.ts +++ b/tests/src/core/Entity.test.ts @@ -328,8 +328,29 @@ describe("Entity", async () => { child.parent = parent; const child2 = new Entity(engine, "child2"); child2.parent = parent; + + const parentModifyCount = [0, 0, 0]; + const childModifyCount = [0, 0, 0]; + const child2ModifyCount = [0, 0, 0]; + // @ts-ignore + parent._registerModifyListener((flag: EntityModifyFlags) => ++parentModifyCount[flag]); + // @ts-ignore + child._registerModifyListener((flag: EntityModifyFlags) => ++childModifyCount[flag]); + // @ts-ignore + child2._registerModifyListener((flag: EntityModifyFlags) => ++child2ModifyCount[flag]); + parent.clearChildren(); expect(parent.children.length).eq(0); + + // Parent should receive a single `Child` modify event for the whole clear so + // listeners (e.g. UICanvas) can invalidate their cached state. + expect(parentModifyCount[EntityModifyFlags.Child]).eq(1); + // Each detached child should receive a `Parent` modify event. + expect(childModifyCount[EntityModifyFlags.Parent]).eq(1); + expect(child2ModifyCount[EntityModifyFlags.Parent]).eq(1); + // Sibling index must be reset so the entity is treated as lonely afterwards. + expect(child.siblingIndex).eq(-1); + expect(child2.siblingIndex).eq(-1); }); it("sibling index", () => { const root = scene.createRootEntity(); @@ -395,12 +416,13 @@ describe("Entity", async () => { }; expect(siblingIndexBadFn).to.throw(); - // thorw error when set lonely entity + // setting sibling index on a lonely entity (no parent, not in scene root) warns instead of throwing const entityX = new Entity(engine, "entityX"); - var lonelyBadFn = function () { + var lonelyFn = function () { entityX.siblingIndex = 1; }; - expect(lonelyBadFn).to.throw(); + expect(lonelyFn).not.to.throw(); + expect(entityX.siblingIndex).eq(-1); }); it("isRoot", () => { diff --git a/tests/src/core/Scene.test.ts b/tests/src/core/Scene.test.ts index c932fbd9db..22c3dbbf55 100644 --- a/tests/src/core/Scene.test.ts +++ b/tests/src/core/Scene.test.ts @@ -1,6 +1,6 @@ import { BackgroundMode, Engine, Entity, Scene, TextureFormat, Texture2D } from "@galacean/engine-core"; import { WebGLEngine } from "@galacean/engine"; -import { describe, beforeAll, beforeEach, expect, it } from "vitest"; +import { describe, beforeAll, beforeEach, expect, it, vi } from "vitest"; describe("Scene", () => { let engine: Engine; @@ -202,4 +202,37 @@ describe("Scene", () => { expect(scene.rootEntitiesCount).eq(0); }); }); + + describe("loadScene", () => { + it("destroyOldScene destroys every previous scene during safe iteration", async () => { + // Dedicated engine so destroying old scenes does not disturb the shared one. + const localEngine = await WebGLEngine.create({ canvas: document.createElement("canvas") }); + const sceneManager = localEngine.sceneManager; + + // Engine starts with one default scene; add two more so multiple scenes exist. + const old0 = sceneManager.scenes[0]; + const old1 = new Scene(localEngine, "old1"); + const old2 = new Scene(localEngine, "old2"); + sceneManager.addScene(old1); + sceneManager.addScene(old2); + expect(sceneManager.scenes.length).eq(3); + + // Resolve the load with a fresh scene without hitting real resources. + const newScene = new Scene(localEngine, "newScene"); + vi.spyOn(localEngine.resourceManager, "load").mockReturnValue(Promise.resolve(newScene) as any); + + await sceneManager.loadScene("mock://scene", true); + + // Destroying a scene removes it from `_scenes` mid-loop; iterating the live array + // (`getArray`) would skip scenes, so every previous scene must still be destroyed. + expect(old0.destroyed).eq(true); + expect(old1.destroyed).eq(true); + expect(old2.destroyed).eq(true); + // Only the newly loaded scene remains. + expect(sceneManager.scenes.length).eq(1); + expect(sceneManager.scenes[0]).eq(newScene); + + localEngine.destroy(); + }); + }); });