[codex] preserve Transform replacement dependencies

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<DependentInfo, ComponentConstructor[]>
   ): 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 {

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 = <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,17 @@ export class Entity extends EngineObject {
    * @returns	The component which has been added
    */
   addComponent<T extends ComponentConstructor>(type: T, ...args: ComponentArguments<T>): InstanceType<T> {
+    const needReplaceTransform = Entity._isTransformType(type) && this._transform;
+    if (needReplaceTransform) {
+      ComponentsDependencies._removeCheck(this, <ComponentConstructor>this._transform.constructor, type);
+    }
     ComponentsDependencies._addCheck(this, type);
     const component = new type(this, ...args) as InstanceType<T>;
-    this._components.push(component);
-
-    // @todo: temporary solution
-    if (component instanceof Transform) this._setTransform(component);
+    if (needReplaceTransform) {
+      this._replaceTransform(<Transform>component);
+    } else {
+      this._components.push(component);
+    }
     component._setActive(true, ActiveChangeFlag.All);
     return component;
   }
@@ -422,7 +444,7 @@ export class Entity extends EngineObject {
    */
   clone(): Entity {
     const cloneEntity = this._createCloneEntity();
-    this._parseCloneEntity(this, cloneEntity, this, cloneEntity, new Map<Object, Object>());
+    this._parseCloneEntity(this, cloneEntity, this, cloneEntity, new Map<object, object>());
     return cloneEntity;
   }
 
@@ -477,7 +499,7 @@ export class Entity extends EngineObject {
     target: Entity,
     srcRoot: Entity,
     targetRoot: Entity,
-    deepInstanceMap: Map<Object, Object>
+    deepInstanceMap: Map<object, object>
   ): void {
     const srcChildren = src._children;
     const targetChildren = target._children;
@@ -529,9 +551,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 +788,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();

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 {}

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);