From b70bf2e267af0b890a614ab3d0f6a7f2945bbb8c Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Thu, 23 Jul 2026 11:33:43 +0800 Subject: [PATCH 1/2] fix(core): preserve Transform replacement dependencies --- packages/core/src/ComponentsDependencies.ts | 8 +- packages/core/src/Entity.ts | 60 +++++++--- tests/src/core/Transform.test.ts | 119 +++++++++++++++++++- tests/src/ui/UITransform.test.ts | 21 +++- 4 files changed, 189 insertions(+), 19 deletions(-) diff --git a/packages/core/src/ComponentsDependencies.ts b/packages/core/src/ComponentsDependencies.ts index 9af8569b12..b76dec5982 100644 --- a/packages/core/src/ComponentsDependencies.ts +++ b/packages/core/src/ComponentsDependencies.ts @@ -36,10 +36,12 @@ export class ComponentsDependencies { /** * @internal */ - static _removeCheck(entity: Entity, type: ComponentConstructor): void { + static _removeCheck(entity: Entity, type: ComponentConstructor, replace?: ComponentConstructor): void { const components = entity._components; const n = components.length; while (type !== Component) { + // The replacement still satisfies this type and all of its base types. + if (replace && (replace === type || replace.prototype instanceof type)) return; let count = 0; for (let i = 0; i < n; i++) { if (components[i] instanceof type && ++count > 1) return; @@ -64,7 +66,7 @@ export class ComponentsDependencies { dependentComponent: ComponentConstructor, map: Map ): void { - let components = map.get(targetInfo); + const components = map.get(targetInfo); if (!components) { map.set(targetInfo, [dependentComponent]); } else { @@ -77,7 +79,7 @@ export class ComponentsDependencies { */ static _addInvDependency(currentComponent: ComponentConstructor, dependentComponent: ComponentConstructor): void { const map = this._invDependenciesMap; - let components = map.get(currentComponent); + const components = map.get(currentComponent); if (!components) { map.set(currentComponent, [dependentComponent]); } else { diff --git a/packages/core/src/Entity.ts b/packages/core/src/Entity.ts index eab8ce5aa2..fd08f0c3c3 100644 --- a/packages/core/src/Entity.ts +++ b/packages/core/src/Entity.ts @@ -22,6 +22,11 @@ import { DisorderedArray } from "./utils/DisorderedArray"; export class Entity extends EngineObject { /** @internal */ static _tempComponentConstructors: ComponentConstructor[] = []; + + private static _isTransformType(type: ComponentConstructor): boolean { + return type === Transform || type.prototype instanceof Transform; + } + /** * @internal */ @@ -236,10 +241,22 @@ export class Entity extends EngineObject { constructor(engine: Engine, name?: string, ...components: ComponentConstructor[]) { super(engine); this.name = name ?? "Entity"; - for (let i = 0, n = components.length; i < n; i++) { - this.addComponent(components[i]); + let transformType: ComponentConstructor = Transform; + const n = components.length; + for (let i = n - 1; i >= 0; i--) { + const componentType = components[i]; + if (Entity._isTransformType(componentType)) { + transformType = componentType; + break; + } + } + this._transform = this.addComponent(transformType); + for (let i = 0; i < n; i++) { + const componentType = components[i]; + if (!Entity._isTransformType(componentType)) { + this.addComponent(componentType); + } } - !this._transform && this.addComponent(Transform); this._inverseWorldMatFlag = this.registerWorldChangeFlag(); } @@ -250,12 +267,16 @@ export class Entity extends EngineObject { * @returns The component which has been added */ addComponent(type: T, ...args: ComponentArguments): InstanceType { + const needReplaceTransform = Entity._isTransformType(type) && this._transform; + if (needReplaceTransform) + ComponentsDependencies._removeCheck(this, this._transform.constructor, type); ComponentsDependencies._addCheck(this, type); const component = new type(this, ...args) as InstanceType; - this._components.push(component); - - // @todo: temporary solution - if (component instanceof Transform) this._setTransform(component); + if (needReplaceTransform) { + this._replaceTransform(component); + } else { + this._components.push(component); + } component._setActive(true, ActiveChangeFlag.All); return component; } @@ -422,7 +443,7 @@ export class Entity extends EngineObject { */ clone(): Entity { const cloneEntity = this._createCloneEntity(); - this._parseCloneEntity(this, cloneEntity, this, cloneEntity, new Map()); + this._parseCloneEntity(this, cloneEntity, this, cloneEntity, new Map()); return cloneEntity; } @@ -477,7 +498,7 @@ export class Entity extends EngineObject { target: Entity, srcRoot: Entity, targetRoot: Entity, - deepInstanceMap: Map + deepInstanceMap: Map ): void { const srcChildren = src._children; const targetChildren = target._children; @@ -529,9 +550,13 @@ export class Entity extends EngineObject { * @internal */ _removeComponent(component: Component): void { - ComponentsDependencies._removeCheck(this, component.constructor as ComponentConstructor); const components = this._components; - components.splice(components.indexOf(component), 1); + const index = components.indexOf(component); + // A replaced Transform is detached from the component slot immediately but + // can still reach here later because object destruction may be deferred. + if (index < 0) return; + ComponentsDependencies._removeCheck(this, component.constructor as ComponentConstructor); + components.splice(index, 1); } /** @@ -762,9 +787,18 @@ export class Entity extends EngineObject { } } - private _setTransform(value: Transform): void { - this._transform?.destroy(); + private _replaceTransform(value: Transform): void { + const previous = this._transform; + value.position.copyFrom(previous.position); + value.rotationQuaternion.copyFrom(previous.rotationQuaternion); + value.scale.copyFrom(previous.scale); + // Keep the unique Transform in the same component slot. Detach the old + // instance before destroy because destroy can be deferred during a frame. + const components = this._components; + const previousIndex = components.indexOf(previous); + components[previousIndex] = value; this._transform = value; + previous.destroy(); const children = this._children; for (let i = 0, n = children.length; i < n; i++) { children[i].transform?._parentChange(); diff --git a/tests/src/core/Transform.test.ts b/tests/src/core/Transform.test.ts index 980edd3572..f96f29afda 100644 --- a/tests/src/core/Transform.test.ts +++ b/tests/src/core/Transform.test.ts @@ -1,4 +1,13 @@ -import { deepClone, Entity, Scene, Script, Transform } from "@galacean/engine-core"; +import { + deepClone, + dependentComponents, + DependentMode, + Entity, + MeshRenderer, + Scene, + Script, + Transform +} from "@galacean/engine-core"; import { Vector2, Vector3 } from "@galacean/engine-math"; import { WebGLEngine } from "@galacean/engine"; import { beforeAll, describe, expect, it } from "vitest"; @@ -112,16 +121,115 @@ describe("Transform test", function () { // Add component const preTransform0 = entity0.transform; + const meshRenderer = entity0.addComponent(MeshRenderer); + const transformIndex = entity0._components.indexOf(preTransform0); entity0.addComponent(SubClassOfTransform); expect(preTransform0.destroyed).to.equal(true); expect(entity0.transform instanceof Transform).to.equal(true); expect(entity0.transform instanceof SubClassOfTransform).to.equal(true); + expect(entity0._components[transformIndex]).to.equal(entity0.transform); + expect(entity0._components.indexOf(meshRenderer)).to.equal(1); + expect(entity0.transform.position).to.deep.include({ x: 1, y: 2, z: 3 }); + expect(entity0.transform.rotation.x).to.be.approximately(0, 1e-6); + expect(entity0.transform.rotation.y).to.be.approximately(45, 1e-6); + expect(entity0.transform.rotation.z).to.be.approximately(0, 1e-6); + expect(entity0.transform.scale).to.deep.include({ x: 1, y: 2, z: 3 }); const preTransform1 = entity1.transform; + const meshRenderer1 = entity1.addComponent(MeshRenderer); + const transformIndex1 = entity1._components.indexOf(preTransform1); entity1.addComponent(Transform); expect(preTransform1.destroyed).to.equal(true); expect(entity1.transform instanceof Transform).to.equal(true); expect(entity1.transform instanceof SubClassOfTransform).to.equal(false); + expect(entity1._components[transformIndex1]).to.equal(entity1.transform); + expect(entity1._components.indexOf(meshRenderer1)).to.equal(1); + expect(entity1.transform.position).to.deep.include({ x: 4, y: 5, z: 6 }); + expect(entity1.transform.rotation.x).to.be.approximately(0, 1e-6); + expect(entity1.transform.rotation.y).to.be.approximately(90, 1e-6); + expect(entity1.transform.rotation.z).to.be.approximately(0, 1e-6); + expect(entity1.transform.scale).to.deep.include({ x: 4, y: 5, z: 6 }); + }); + + it("creates the unique Transform before constructor components", () => { + const entityWithRenderer = new Entity(engine, "entity-with-renderer", MeshRenderer); + expect(entityWithRenderer._components[0]).to.equal(entityWithRenderer.transform); + expect(entityWithRenderer.getComponent(MeshRenderer)).not.to.equal(null); + + const entityWithLateTransform = new Entity( + engine, + "entity-with-late-transform", + MeshRenderer, + Transform, + SubClassOfTransform + ); + const transforms: Transform[] = []; + entityWithLateTransform.getComponents(Transform, transforms); + expect(transforms).to.deep.equal([entityWithLateTransform.transform]); + expect(entityWithLateTransform.transform).to.be.instanceOf(SubClassOfTransform); + expect(entityWithLateTransform._components[0]).to.equal(entityWithLateTransform.transform); + expect(entityWithLateTransform._components[1]).to.be.instanceOf(MeshRenderer); + + const clone = entityWithLateTransform.clone(); + expect(clone.transform).to.be.instanceOf(SubClassOfTransform); + expect(clone._components.map((component) => component.constructor)).to.deep.equal( + entityWithLateTransform._components.map((component) => component.constructor) + ); + }); + + it("keeps the Transform slot unique while destruction is deferred", () => { + const deferredEntity = new Entity(engine, "deferred-transform"); + const previous = deferredEntity.transform; + let replacement: SubClassOfTransform; + + engine._frameInProcess = true; + try { + replacement = deferredEntity.addComponent(SubClassOfTransform); + const transforms: Transform[] = []; + deferredEntity.getComponents(Transform, transforms); + expect(previous.pendingDestroy).to.equal(true); + expect(transforms).to.deep.equal([replacement]); + expect(deferredEntity._components[0]).to.equal(replacement); + } finally { + engine._frameInProcess = false; + previous.destroy(); + } + }); + + it("rolls back Transform replacement when a dependency prevents it", () => { + const dependentEntity = new Entity(engine, "dependent-transform", SubClassOfTransform); + const previous = dependentEntity.transform; + dependentEntity.addComponent(RequiresSubClassOfTransform); + + expect(() => dependentEntity.addComponent(Transform)).to.throw( + "Should remove RequiresSubClassOfTransform before remove SubClassOfTransform" + ); + const transforms: Transform[] = []; + dependentEntity.getComponents(Transform, transforms); + expect(dependentEntity.transform).to.equal(previous); + expect(transforms).to.deep.equal([previous]); + expect(dependentEntity._components[0]).to.equal(previous); + }); + + it("checks dependencies declared by a replacement Transform", () => { + const dependentEntity = new Entity(engine, "check-only-dependent-transform"); + const previous = dependentEntity.transform; + + expect(() => dependentEntity.addComponent(CheckOnlyDependentTransform)).to.throw( + "Should add MeshRenderer before adding CheckOnlyDependentTransform" + ); + expect(dependentEntity.transform).to.equal(previous); + expect(dependentEntity._components).to.deep.equal([previous]); + }); + + it("auto adds dependencies declared by a replacement Transform", () => { + const dependentEntity = new Entity(engine, "auto-add-dependent-transform"); + const replacement = dependentEntity.addComponent(AutoAddDependentTransform); + + expect(dependentEntity.transform).to.equal(replacement); + expect(dependentEntity._components[0]).to.equal(replacement); + expect(dependentEntity._components[1]).to.be.instanceOf(MeshRenderer); + expect(dependentEntity.getComponent(MeshRenderer)).not.to.equal(null); }); it("clone with worldMatrix listener should not produce stale parent cache after reparent", () => { @@ -191,3 +299,12 @@ class SubClassOfTransform extends Transform { @deepClone size: Vector2 = new Vector2(); } + +@dependentComponents(SubClassOfTransform, DependentMode.CheckOnly) +class RequiresSubClassOfTransform extends Script {} + +@dependentComponents(MeshRenderer, DependentMode.CheckOnly) +class CheckOnlyDependentTransform extends Transform {} + +@dependentComponents(MeshRenderer, DependentMode.AutoAdd) +class AutoAddDependentTransform extends Transform {} diff --git a/tests/src/ui/UITransform.test.ts b/tests/src/ui/UITransform.test.ts index 534f0f3927..84ab5487d1 100644 --- a/tests/src/ui/UITransform.test.ts +++ b/tests/src/ui/UITransform.test.ts @@ -1,5 +1,5 @@ -import { WebGLEngine } from "@galacean/engine"; -import { HorizontalAlignmentMode, UICanvas, UITransform, VerticalAlignmentMode } from "@galacean/engine-ui"; +import { Entity, MeshRenderer, WebGLEngine } from "@galacean/engine"; +import { HorizontalAlignmentMode, Image, UICanvas, UITransform, VerticalAlignmentMode } from "@galacean/engine-ui"; import { describe, expect, it } from "vitest"; describe("UITransform", async () => { @@ -404,6 +404,23 @@ describe("UITransform", async () => { }); describe("clone", () => { + it("keeps component mapping after a renderer replaces Transform with UITransform", () => { + const original = new Entity(engine, "clone-transform-replacement"); + original.addComponent(MeshRenderer); + original.addComponent(Image); + + expect(original.transform).to.be.instanceOf(UITransform); + expect(original._components[0]).to.equal(original.transform); + + const cloned = original.clone(); + expect(cloned.transform).to.be.instanceOf(UITransform); + expect(cloned.getComponent(MeshRenderer)).not.to.equal(null); + expect(cloned.getComponent(Image)).not.to.equal(null); + expect(cloned._components.map((component) => component.constructor)).to.deep.equal( + original._components.map((component) => component.constructor) + ); + }); + it("clones basic properties correctly", () => { const parent = root.createChild("clone-parent"); parent.addComponent(UICanvas); From 99c357ada2727c4ea70fbbb331086c5b4cdf824b Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Thu, 23 Jul 2026 11:50:39 +0800 Subject: [PATCH 2/2] style(core): brace Transform replacement check --- packages/core/src/Entity.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/Entity.ts b/packages/core/src/Entity.ts index fd08f0c3c3..4072012761 100644 --- a/packages/core/src/Entity.ts +++ b/packages/core/src/Entity.ts @@ -268,8 +268,9 @@ export class Entity extends EngineObject { */ addComponent(type: T, ...args: ComponentArguments): InstanceType { const needReplaceTransform = Entity._isTransformType(type) && this._transform; - if (needReplaceTransform) + if (needReplaceTransform) { ComponentsDependencies._removeCheck(this, this._transform.constructor, type); + } ComponentsDependencies._addCheck(this, type); const component = new type(this, ...args) as InstanceType; if (needReplaceTransform) {