Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entity support add other Transform #2456

Merged
merged 16 commits into from
Dec 11, 2024
Merged

Entity support add other Transform #2456

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e95b86d
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
d8df9e0
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
0d57088
feat: udpate code
cptbtptpbcptdtptp Dec 10, 2024
0d004aa
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
b882eab
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
01e0823
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
375e4b7
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
6e635e7
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
406391f
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
4991156
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
08a1b5c
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
a0339f2
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
e83000d
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
90589b0
feat: udpate code
cptbtptpbcptdtptp Dec 11, 2024
cb29b43
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
68fa63a
feat: update code
cptbtptpbcptdtptp Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions packages/core/src/Camera.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ export class Camera extends Component {
@ignoreClone
private _frustumChangeFlag: BoolUpdateFlag;
@ignoreClone
private _transform: Transform;
@ignoreClone
private _isViewMatrixDirty: BoolUpdateFlag;
@ignoreClone
private _isInvViewProjDirty: BoolUpdateFlag;
Expand Down Expand Up @@ -315,7 +313,7 @@ export class Camera extends Component {
this._isViewMatrixDirty.flag = false;

// Ignore scale
const transform = this._transform;
const transform = this._entity.transform;
Matrix.rotationTranslation(transform.worldRotationQuaternion, transform.worldPosition, viewMatrix);
viewMatrix.invert();
return viewMatrix;
Expand Down Expand Up @@ -423,11 +421,9 @@ export class Camera extends Component {
constructor(entity: Entity) {
super(entity);

const transform = this.entity.transform;
this._transform = transform;
this._isViewMatrixDirty = transform.registerWorldChangeFlag();
this._isInvViewProjDirty = transform.registerWorldChangeFlag();
this._frustumChangeFlag = transform.registerWorldChangeFlag();
this._isViewMatrixDirty = entity.registerWorldChangeFlag();
this._isInvViewProjDirty = entity.registerWorldChangeFlag();
this._frustumChangeFlag = entity.registerWorldChangeFlag();
this._renderPipeline = new BasicRenderPipeline(this);
this._addResourceReferCount(this.shaderData, 1);
this._updatePixelViewport();
Expand Down Expand Up @@ -607,7 +603,7 @@ export class Camera extends Component {
const context = engine._renderContext;
const virtualCamera = this._virtualCamera;

const transform = this.entity.transform;
const transform = this._entity.transform;
Matrix.multiply(this.projectionMatrix, this.viewMatrix, virtualCamera.viewProjectionMatrix);
virtualCamera.position.copyFrom(transform.worldPosition);
if (virtualCamera.isOrthographic) {
Expand Down Expand Up @@ -738,7 +734,6 @@ export class Camera extends Component {
this._virtualCamera = null;
this._shaderData = null;
this._frustumChangeFlag = null;
this._transform = null;
this._isViewMatrixDirty = null;
this._isInvViewProjDirty = null;
this._viewport = null;
Expand Down Expand Up @@ -788,7 +783,7 @@ export class Camera extends Component {
private _updateShaderData(): void {
const shaderData = this.shaderData;

const transform = this._transform;
const transform = this.entity.transform;
shaderData.setMatrix(Camera._inverseViewMatrixProperty, transform.worldMatrix);
shaderData.setVector3(Camera._cameraPositionProperty, transform.worldPosition);
shaderData.setVector3(Camera._cameraForwardProperty, transform.worldForward);
Expand All @@ -806,7 +801,7 @@ export class Camera extends Component {
private _getInvViewProjMat(): Matrix {
if (this._isInvViewProjDirty.flag) {
this._isInvViewProjDirty.flag = false;
Matrix.multiply(this._transform.worldMatrix, this._getInverseProjectionMatrix(), this._invViewProjMat);
Matrix.multiply(this._entity.transform.worldMatrix, this._getInverseProjectionMatrix(), this._invViewProjMat);
}
return this._invViewProjMat;
}
Expand Down
80 changes: 78 additions & 2 deletions packages/core/src/ComponentsDependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Component } from "./Component";
import { Entity } from "./Entity";

type ComponentConstructor = new (entity: Entity) => Component;
export type ComponentConstructor = new (entity: Entity) => Component;

/**
* @internal
Expand All @@ -11,6 +11,8 @@ export class ComponentsDependencies {
private static _invDependenciesMap = new Map<ComponentConstructor, ComponentConstructor[]>();

static _dependenciesMap = new Map<ComponentConstructor, DependentInfo>();
static _mutuallyExclusiveMap = new Map<ComponentConstructor, ComponentConstructor[]>();
static _inheritedComponents: ComponentConstructor[] = [];

/**
* @internal
Expand All @@ -31,6 +33,12 @@ export class ComponentsDependencies {
}
}
}
const mutuallyExclusiveComponents = ComponentsDependencies._mutuallyExclusiveMap.get(type);
if (mutuallyExclusiveComponents) {
for (let i = 0, n = mutuallyExclusiveComponents.length; i < n; i++) {
entity.getComponent(mutuallyExclusiveComponents[i])?.destroy();
}
}
type = Object.getPrototypeOf(type);
}
}
Expand All @@ -44,14 +52,33 @@ export class ComponentsDependencies {
if (invDependencies) {
for (let i = 0, len = invDependencies.length; i < len; i++) {
if (entity.getComponent(invDependencies[i])) {
throw `Should remove ${invDependencies[i].name} before adding ${type.name}`;
throw `Should remove ${invDependencies[i].name} before remove ${type.name}`;
}
}
}
type = Object.getPrototypeOf(type);
}
}

/**
* @internal
*/
static _getInheritedComponents(entity: Entity, out: ComponentConstructor[]): void {
out.length = 0;
const components = entity._components;
const inheritedComponents = ComponentsDependencies._inheritedComponents;
const inheritedComponentsLength = inheritedComponents.length;
for (let i = 0, n = components.length; i < n; i++) {
const component = components[i];
for (let j = 0; j < inheritedComponentsLength; j++) {
if (component instanceof inheritedComponents[j]) {
out.push(component.constructor as ComponentConstructor);
break;
}
}
}
}

/**
* @internal
*/
Expand Down Expand Up @@ -81,6 +108,23 @@ export class ComponentsDependencies {
}
}

static _markComponentsMutuallyExclusive(
target: ComponentConstructor,
componentOrComponents: ComponentConstructor | ComponentConstructor[]
): void {
const components = Array.isArray(componentOrComponents) ? componentOrComponents : [componentOrComponents];
const map = ComponentsDependencies._mutuallyExclusiveMap;
const preComponents = map.get(target);
if (preComponents) {
for (let i = 0, n = components.length; i < n; i++) {
const component = components[i];
!preComponents.includes(component) && preComponents.push(component);
}
} else {
map.set(target, components);
}
}

private constructor() {}
}

Expand Down Expand Up @@ -110,6 +154,38 @@ export function dependentComponents(
};
}

/**
* Declare mutually exclusive components in an entity.
* @param component - component
*/
export function mutuallyExclusiveComponents(component: ComponentConstructor);

/**
* Declare mutually exclusive components in an entity.
* @param components - components
*/
export function mutuallyExclusiveComponents(components: ComponentConstructor[]);

export function mutuallyExclusiveComponents(componentOrComponents: ComponentConstructor | ComponentConstructor[]) {
const components = Array.isArray(componentOrComponents) ? componentOrComponents : [componentOrComponents];
return function <T extends ComponentConstructor>(target: T): void {
ComponentsDependencies._markComponentsMutuallyExclusive(target, components);
for (let i = 0, n = components.length; i < n; i++) {
ComponentsDependencies._markComponentsMutuallyExclusive(components[i], target);
}
};
}

/**
* Declare the components that child need to inherit.
*/
export function inherited() {
return function <T extends ComponentConstructor>(target: T): void {
const inheritedComponents = ComponentsDependencies._inheritedComponents;
inheritedComponents.includes(target) || inheritedComponents.push(target);
};
}

/**
* Dependent mode.
*/
Expand Down
67 changes: 57 additions & 10 deletions packages/core/src/Entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Matrix } from "@galacean/engine-math";
import { ignoreClone } from ".";
import { BoolUpdateFlag } from "./BoolUpdateFlag";
import { Component } from "./Component";
import { ComponentsDependencies } from "./ComponentsDependencies";
import { ComponentConstructor, ComponentsDependencies } from "./ComponentsDependencies";
import { Engine } from "./Engine";
import { Layer } from "./Layer";
import { Scene } from "./Scene";
import { Script } from "./Script";
import { Transform } from "./Transform";
import { Transform, TransformTransitionState } from "./Transform";
import { UpdateFlagManager } from "./UpdateFlagManager";
import { ReferResource } from "./asset/ReferResource";
import { EngineObject } from "./base";
import { ComponentCloner } from "./clone/ComponentCloner";
Expand All @@ -17,6 +19,11 @@ import { DisorderedArray } from "./utils/DisorderedArray";
* Entity, be used as components container.
*/
export class Entity extends EngineObject {
/** @internal */
static _inheritedComponents: ComponentConstructor[] = [];
/** @internal */
static _matrix: Matrix = new Matrix();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Potential naming conflict with _inheritedComponents

Both Entity and ComponentsDependencies declare a static property named _inheritedComponents, which can lead to confusion and bugs due to naming conflicts. Consider renaming one of them for clarity.

Apply this diff to rename Entity's property:

-static _inheritedComponents: ComponentConstructor[] = [];
+static _entityInheritedComponents: ComponentConstructor[] = [];

And update all references to Entity._inheritedComponents accordingly.

Committable suggestion skipped: line range outside the PR's diff.


/**
* @internal
*/
Expand Down Expand Up @@ -73,8 +80,6 @@ export class Entity extends EngineObject {
name: string;
/** The layer the entity belongs to. */
layer: Layer = Layer.Layer0;
/** Transform component. */
readonly transform: Transform;

/** @internal */
_isActiveInHierarchy: boolean = false;
Expand All @@ -94,14 +99,40 @@ export class Entity extends EngineObject {
_isActive: boolean = true;
/** @internal */
_siblingIndex: number = -1;

/** @internal */
_isTemplate: boolean = false;
/** @internal */
@ignoreClone
_updateFlagManager: UpdateFlagManager = new UpdateFlagManager();

private _transform: Transform;
private _transformTransitionState: TransformTransitionState;
private _templateResource: ReferResource;
private _parent: Entity = null;
private _activeChangedComponents: Component[];

/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}

set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
pre._generateTransitionState(state);
}
if (value) {
state && value._applyTransitionState(state);
}
this._transform = value;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling to transform setter

The transform state transition logic needs validation and error handling.

Apply this diff to improve robustness:

set transform(value: Transform) {
  const pre = this._transform;
  if (value !== pre) {
+   if (value && !(value instanceof Transform)) {
+     throw new Error("Value must be an instance of Transform");
+   }
    let state = this._transformTransitionState;
    if (pre) {
      state ||= this._transformTransitionState = {};
+     try {
        pre._generateTransitionState(state);
+     } catch (e) {
+       console.error("Failed to generate transition state:", e);
+     }
    }
    if (value) {
+     try {
        state && value._applyTransitionState(state);
+     } catch (e) {
+       console.error("Failed to apply transition state:", e);
+     }
    }
    this._transform = value;
  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}
set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
pre._generateTransitionState(state);
}
if (value) {
state && value._applyTransitionState(state);
}
this._transform = value;
}
}
/**
* The transform of the entity.
*/
get transform(): Transform {
return this._transform;
}
set transform(value: Transform) {
const pre = this._transform;
if (value !== pre) {
if (value && !(value instanceof Transform)) {
throw new Error("Value must be an instance of Transform");
}
let state = this._transformTransitionState;
if (pre) {
state ||= this._transformTransitionState = {};
try {
pre._generateTransitionState(state);
} catch (e) {
console.error("Failed to generate transition state:", e);
}
}
if (value) {
try {
state && value._applyTransitionState(state);
} catch (e) {
console.error("Failed to apply transition state:", e);
}
}
this._transform = value;
}
}


/**
* Whether to activate locally.
*/
Expand Down Expand Up @@ -193,11 +224,17 @@ export class Entity extends EngineObject {
* Create a entity.
* @param engine - The engine the entity belongs to
*/
constructor(engine: Engine, name?: string) {
constructor(engine: Engine, name?: string, ...components: ComponentConstructor[]) {
super(engine);
this.name = name;
this.transform = this.addComponent(Transform);
this._inverseWorldMatFlag = this.transform.registerWorldChangeFlag();
for (let i = 0, n = components.length; i < n; i++) {
const type = components[i];
if (!Transform.prototype.isPrototypeOf(type.prototype) || !this.transform) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid directly accessing isPrototypeOf from prototype

Using Transform.prototype.isPrototypeOf(type.prototype) may cause issues if isPrototypeOf is overridden. Instead, consider using Object.prototype.isPrototypeOf.call(Transform.prototype, type.prototype) for safety.

Apply this diff to make the change:

- if (!Transform.prototype.isPrototypeOf(type.prototype) || !this.transform) {
+ if (!Object.prototype.isPrototypeOf.call(Transform.prototype, type.prototype) || !this.transform) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!Transform.prototype.isPrototypeOf(type.prototype) || !this.transform) {
if (!Object.prototype.isPrototypeOf.call(Transform.prototype, type.prototype) || !this.transform) {
🧰 Tools
🪛 Biome (1.9.4)

[error] 232-232: Do not access Object.prototype method 'isPrototypeOf' from target object.

(lint/suspicious/noPrototypeBuiltins)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using isPrototypeOf; use instanceof instead

Accessing Object.prototype methods like isPrototypeOf can be unreliable. Use the instanceof operator or === for accurate type checks.

Apply this diff to fix the issue:

-if (!Transform.prototype.isPrototypeOf(type.prototype) || !this.transform) {
+if (!(type.prototype instanceof Transform) || !this.transform) {

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (1.9.4)

[error] 232-232: Do not access Object.prototype method 'isPrototypeOf' from target object.

(lint/suspicious/noPrototypeBuiltins)

this.addComponent(type);
}
}
singlecoder marked this conversation as resolved.
Show resolved Hide resolved
this.transform || this.addComponent(Transform);
this._inverseWorldMatFlag = this.registerWorldChangeFlag();
}

/**
Expand Down Expand Up @@ -394,7 +431,9 @@ export class Entity extends EngineObject {
* @returns The child entity
*/
createChild(name?: string): Entity {
const child = new Entity(this.engine, name);
const inheritedComponents = Entity._inheritedComponents;
ComponentsDependencies._getInheritedComponents(this, inheritedComponents);
const child = new Entity(this.engine, name, ...inheritedComponents);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure correct usage of inherited components in createChild

Passing ...inheritedComponents directly may cause issues if inheritedComponents is modified elsewhere. Consider creating a new array to prevent side effects.

Apply this diff:

-const child = new Entity(this.engine, name, ...inheritedComponents);
+const child = new Entity(this.engine, name, ...inheritedComponents.slice());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const child = new Entity(this.engine, name, ...inheritedComponents);
const child = new Entity(this.engine, name, ...inheritedComponents.slice());

child.layer = this.layer;
child.parent = this;
return child;
Expand Down Expand Up @@ -429,6 +468,14 @@ export class Entity extends EngineObject {
return cloneEntity;
}

/**
* Register world transform change flag.
* @returns Change flag
*/
registerWorldChangeFlag(): BoolUpdateFlag {
return this._updateFlagManager.createFlag(BoolUpdateFlag);
}

/**
* @internal
*/
Expand Down Expand Up @@ -478,7 +525,7 @@ export class Entity extends EngineObject {
for (let i = 0, n = components.length; i < n; i++) {
const sourceComp = components[i];
if (!(sourceComp instanceof Transform)) {
const targetComp = target.addComponent(<new (entity: Entity) => Component>sourceComp.constructor);
const targetComp = target.addComponent(<ComponentConstructor>sourceComp.constructor);
ComponentCloner.cloneComponent(sourceComp, targetComp, srcRoot, targetRoot, deepInstanceMap);
}
}
Expand Down
19 changes: 4 additions & 15 deletions packages/core/src/Renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ export class Renderer extends Component implements IComponentCustomClone {
protected _dirtyUpdateFlag: number = 0;
@ignoreClone
protected _rendererLayer: Vector4 = new Vector4();
@ignoreClone
protected _transform: Transform;

@deepClone
private _shaderData: ShaderData = new ShaderData(ShaderDataGroup.Renderer);
Expand Down Expand Up @@ -172,7 +170,7 @@ export class Renderer extends Component implements IComponentCustomClone {
this._addResourceReferCount(this.shaderData, 1);

this._onTransformChanged = this._onTransformChanged.bind(this);
this._setTransform(entity.transform);
entity._updateFlagManager.addListener(this._onTransformChanged);

shaderData.enableMacro(Renderer._receiveShadowMacro);
shaderData.setVector4(Renderer._rendererLayerProperty, this._rendererLayer);
Expand Down Expand Up @@ -366,7 +364,7 @@ export class Renderer extends Component implements IComponentCustomClone {
protected override _onDestroy(): void {
super._onDestroy();

this._setTransform(null);
this._entity._updateFlagManager.removeListener(this._onTransformChanged);
this._addResourceReferCount(this.shaderData, -1);

const materials = this._materials;
Expand All @@ -392,7 +390,7 @@ export class Renderer extends Component implements IComponentCustomClone {
* @internal
*/
_updateTransformShaderData(context: RenderContext, onlyMVP: boolean, batched: boolean): void {
const worldMatrix = this._transform.worldMatrix;
const worldMatrix = this._entity.transform.worldMatrix;
if (onlyMVP) {
this._updateProjectionRelatedShaderData(context, worldMatrix, batched);
} else {
Expand Down Expand Up @@ -442,7 +440,7 @@ export class Renderer extends Component implements IComponentCustomClone {
Matrix.invert(worldMatrix, normalMatrix);
normalMatrix.transpose();

shaderData.setMatrix(Renderer._localMatrixProperty, this._transform.localMatrix);
shaderData.setMatrix(Renderer._localMatrixProperty, this._entity.transform.localMatrix);
shaderData.setMatrix(Renderer._worldMatrixProperty, worldMatrix);
shaderData.setMatrix(Renderer._mvMatrixProperty, mvMatrix);
shaderData.setMatrix(Renderer._mvInvMatrixProperty, mvInvMatrix);
Expand All @@ -462,15 +460,6 @@ export class Renderer extends Component implements IComponentCustomClone {
}
}

/**
* @internal
*/
protected _setTransform(transform: Transform): void {
this._transform?._updateFlagManager.removeListener(this._onTransformChanged);
transform?._updateFlagManager.addListener(this._onTransformChanged);
this._transform = transform;
}

/**
* @internal
*/
Expand Down
Loading
Loading