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

Jump to bottom

refactor : prefab code optimization and protection #15845

Merged
merged 8 commits into from
Aug 9, 2023
Merged

refactor : prefab code optimization and protection #15845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1449fd1
refactor : prefab code optimization and protection
dogeFu Aug 2, 2023
aba0ff9
remove extra if
dogeFu Aug 2, 2023
992e31f
remove extra param
dogeFu Aug 2, 2023
4262bdb
replace class with interface
dogeFu Aug 2, 2023
992ee28
Update cocos/scene-graph/prefab/utils.ts
dogeFu Aug 2, 2023
3594748
remove command and protect access
dogeFu Aug 7, 2023
1ca17a2
Merge remote-tracking branch 'dogeFu/v3.8.1_expand_failed' into v3.8.…
dogeFu Aug 7, 2023
57523ab
add prefab getter in node.jsb.ts
dogeFu Aug 9, 2023
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
32 changes: 27 additions & 5 deletions cocos/scene-graph/node.jsb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,14 @@ Object.defineProperty(nodeProto, '_siblingIndex', {
},
});

Object.defineProperty(nodeProto, 'prefab', {
configurable: true,
enumerable: true,
get() {
return this._prefab;
},
});

// External classes need to access it through getter/setter
Object.defineProperty(nodeProto, 'siblingIndex', {
configurable: true,
Expand Down Expand Up @@ -1214,11 +1222,25 @@ nodeProto[serializeTag] = function (serializationOutput: SerializationOutput, co
// discard props disallow to synchronize
const isRoot = this._prefab?.root === this;
if (isRoot) {
serializationOutput.writeProperty('_objFlags', this._objFlags);
serializationOutput.writeProperty('_parent', this._parent);
serializationOutput.writeProperty('_prefab', this._prefab);
if (context.customArguments.keepNodeUuid) {
serializationOutput.writeProperty('_id', this._id);
// if B prefab is in A prefab,B can be referenced by component.We should discard it.because B is not the root of prefab
let isNestedPrefab = false;
let parent = this.getParent();
while (parent) {
const nestedRoots = parent._prefab?.nestedPrefabInstanceRoots;
if (nestedRoots && nestedRoots.length > 0) {
// if this node is not in nestedPrefabInstanceRoots,it means this node is not the root of prefab,so it should be discarded.
isNestedPrefab = !nestedRoots.some((root) => root === this);
break;
}
parent = parent.getParent();
}
if (!isNestedPrefab) {
serializationOutput.writeProperty('_objFlags', this._objFlags);
serializationOutput.writeProperty('_parent', this._parent);
serializationOutput.writeProperty('_prefab', this._prefab);
if (context.customArguments.keepNodeUuid) {
serializationOutput.writeProperty('_id', this._id);
}
}
// TODO: editorExtrasTag may be a symbol in the future
serializationOutput.writeProperty(editorExtrasTag, this[editorExtrasTag]);
Expand Down
6 changes: 5 additions & 1 deletion cocos/scene-graph/node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

const idGenerator = new js.IDGenerator('Node');

function getConstructor<T> (typeOrClassName: string | Constructor<T> | AbstractedConstructor<T>): Constructor<T> | AbstractedConstructor<T> | null | undefined {

Check warning on line 53 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

This line has a length of 160. Maximum allowed is 150
if (!typeOrClassName) {
errorID(3804);
return null;
Expand Down Expand Up @@ -291,7 +291,7 @@
return null;
}

protected static _findComponents<T extends Component> (node: Node, constructor: Constructor<T> | AbstractedConstructor<T>, components: Component[]): void {

Check warning on line 294 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

This line has a length of 159. Maximum allowed is 150
const cls = constructor;
const comps = node._components;
// NOTE: internal rtti property
Expand Down Expand Up @@ -333,7 +333,7 @@
protected static _findChildComponents (children: Node[], constructor, components): void {
for (let i = 0; i < children.length; ++i) {
const node = children[i];
Node._findComponents(node, constructor, components);

Check failure on line 336 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Constructor<Component> | AbstractedConstructor<Component>`

Check failure on line 336 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Component[]`
if (node._children.length > 0) {
Node._findChildComponents(node._children, constructor, components);
}
Expand Down Expand Up @@ -363,6 +363,10 @@
*/
@serializable
protected _prefab: PrefabInfo | null = null;
/**
* @engineInternal
*/
public get prefab (): PrefabInfo | null { return this._prefab; }

protected _scene: Scene = null!;

Expand Down Expand Up @@ -1000,8 +1004,8 @@
if (Array.isArray(reqComps)) {
for (let i = 0; i < reqComps.length; i++) {
const reqComp = reqComps[i];
if (!this.getComponent(reqComp)) {

Check failure on line 1007 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Constructor<Component> | AbstractedConstructor<Component>`
this.addComponent(reqComp);

Check failure on line 1008 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Constructor<Component>`
}
}
} else {
Expand Down Expand Up @@ -1086,7 +1090,7 @@
if (component instanceof Component) {
componentInstance = component;
} else {
componentInstance = this.getComponent(component);

Check failure on line 1093 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Constructor<Component> | AbstractedConstructor<Component>`
}
if (componentInstance) {
componentInstance.destroy();
Expand Down Expand Up @@ -1140,7 +1144,7 @@
default:
break;
}
this._eventProcessor.on(type as NodeEventType, callback, target, useCapture);

Check failure on line 1147 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `boolean | undefined`
}

/**
Expand All @@ -1160,7 +1164,7 @@
* ```
*/
public off (type: string, callback?: AnyFunction, target?: unknown, useCapture: any = false): void {
this._eventProcessor.off(type as NodeEventType, callback, target, useCapture);

Check failure on line 1167 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `boolean | undefined`

const hasListeners = this._eventProcessor.hasEventListener(type);
// All listener removed
Expand Down Expand Up @@ -1188,7 +1192,7 @@
* @param target - The target (this object) to invoke the callback, can be null
*/
public once (type: string, callback: AnyFunction, target?: unknown, useCapture?: any): void {
this._eventProcessor.once(type as NodeEventType, callback, target, useCapture);

Check failure on line 1195 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `boolean | undefined`
}

/**
Expand Down Expand Up @@ -1241,7 +1245,7 @@
* @zh 移除目标上的所有注册事件。
* @param target - The target to be searched for all related callbacks
*/
public targetOff (target: string | unknown): void {

Check failure on line 1248 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

'unknown' overrides all other types in this union type
this._eventProcessor.targetOff(target);
// Check for event mask reset
if ((this._eventMask & TRANSFORM_ON) && !this._eventProcessor.hasEventListener(NodeEventType.TRANSFORM_CHANGED)) {
Expand Down Expand Up @@ -1317,7 +1321,7 @@
if (EDITOR && newPrefabInfo) {
if (cloned === newPrefabInfo.root) {
// when instantiate prefab in Editor,should add prefab instance info for root node
EditorExtends.PrefabUtils.addPrefabInstance?.(cloned);

Check failure on line 1324 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint 

Unsafe argument of type `any` assigned to a parameter of type `Node`
// newPrefabInfo.fileId = '';
} else {
// var PrefabUtils = Editor.require('scene://utils/prefab');
Expand Down Expand Up @@ -1806,7 +1810,7 @@
let isNestedPrefab = false;
let parent = this.getParent();
while (parent) {
const nestedRoots = parent?._prefab?.nestedPrefabInstanceRoots;
const nestedRoots = parent._prefab?.nestedPrefabInstanceRoots;
if (nestedRoots && nestedRoots.length > 0) {
// if this node is not in nestedPrefabInstanceRoots,it means this node is not the root of prefab,so it should be discarded.
isNestedPrefab = !nestedRoots.some((root) => root === this);
Expand Down
4 changes: 3 additions & 1 deletion cocos/scene-graph/prefab/prefab-info.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class PrefabInstance {
@type([TargetInfo])
public removedComponents: TargetInfo[] = [];

public targetMap: Record<string, any | Node | Component> = {};
public targetMap: TargetMap = {};

/**
* make sure prefab instance expand only once
Expand Down Expand Up @@ -201,6 +201,8 @@ export class PrefabInstance {
}
}

export interface TargetMap { [k: string]: TargetMap | Node | Component }

@ccclass('cc.PrefabInfo')
export class PrefabInfo {
// the most top node of this prefab in the scene
Expand Down
73 changes: 32 additions & 41 deletions cocos/scene-graph/prefab/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ import { EDITOR, SUPPORT_JIT } from 'internal:constants';
import { cclegacy, errorID, warn, editorExtrasTag } from '../../core';
import { Node } from '../node';
import { Component } from '../component';
import { MountedChildrenInfo, PropertyOverrideInfo, MountedComponentsInfo, TargetInfo } from './prefab-info';
import {
MountedChildrenInfo,
PropertyOverrideInfo,
MountedComponentsInfo,
TargetInfo, TargetMap,
PrefabInstance,
TargetOverrideInfo,
} from './prefab-info';
import { ValueType } from '../../core/value-types';

export * from './prefab-info';

export function createNodeWithPrefab (node: Node): void {
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInfo = (node as any)._prefab;
const prefabInfo = node?.prefab;
minggo marked this conversation as resolved.
Show resolved Hide resolved
if (!prefabInfo) {
return;
}
Expand Down Expand Up @@ -63,7 +68,6 @@ export function createNodeWithPrefab (node: Node): void {
const _id = node.uuid;
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const _prefab = (node as any)._prefab;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
const editorExtras = node[editorExtrasTag];

// instantiate prefab
Expand All @@ -90,16 +94,14 @@ export function createNodeWithPrefab (node: Node): void {
node[editorExtrasTag] = editorExtras;
}

// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
if ((node as any)._prefab) {
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
if (node.prefab) {
// just keep the instance
(node as any)._prefab.instance = _prefab?.instance;
node.prefab.instance = prefabInfo.instance;
}
}

// TODO: more efficient id->Node/Component map
export function generateTargetMap (node: Node, targetMap: any, isRoot: boolean): void {
export function generateTargetMap (node: Node, targetMap: TargetMap, isRoot: boolean): void {
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
if (!targetMap) {
return;
}
Expand All @@ -110,17 +112,13 @@ export function generateTargetMap (node: Node, targetMap: any, isRoot: boolean):

let curTargetMap = targetMap;

// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInstance = (node as any)._prefab?.instance;
const prefabInstance = node.prefab?.instance;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
if (!isRoot && prefabInstance) {
targetMap[prefabInstance.fileId] = {};
curTargetMap = targetMap[prefabInstance.fileId];
curTargetMap = targetMap[prefabInstance.fileId] as TargetMap;
}

// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInfo = (node as any)._prefab;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
const prefabInfo = node.prefab;
if (prefabInfo) {
curTargetMap[prefabInfo.fileId] = node;
}
Expand Down Expand Up @@ -158,7 +156,7 @@ export function getTarget (localID: string[], targetMap: any): Node | Component
return target;
}

export function applyMountedChildren (node: Node, mountedChildren: MountedChildrenInfo[], targetMap: Record<string, any | Node | Component>): void {
export function applyMountedChildren (node: Node, mountedChildren: MountedChildrenInfo[], targetMap: TargetMap): void {
if (!mountedChildren) {
return;
}
Expand All @@ -175,7 +173,7 @@ export function applyMountedChildren (node: Node, mountedChildren: MountedChildr
const localID = childInfo.targetInfo.localID;
if (localID.length > 0) {
for (let i = 0; i < localID.length - 1; i++) {
curTargetMap = curTargetMap[localID[i]];
curTargetMap = curTargetMap[localID[i]] as TargetMap;
}
}
if (childInfo.nodes) {
Expand Down Expand Up @@ -205,7 +203,7 @@ export function applyMountedChildren (node: Node, mountedChildren: MountedChildr
}
}

export function applyMountedComponents (node: Node, mountedComponents: MountedComponentsInfo[], targetMap: Record<string, any | Node | Component>): void {
export function applyMountedComponents (node: Node, mountedComponents: MountedComponentsInfo[], targetMap: TargetMap): void {
if (!mountedComponents) {
return;
}
Expand Down Expand Up @@ -240,7 +238,7 @@ export function applyMountedComponents (node: Node, mountedComponents: MountedCo
}
}

export function applyRemovedComponents (node: Node, removedComponents: TargetInfo[], targetMap: Record<string, any | Node | Component>): void {
export function applyRemovedComponents (node: Node, removedComponents: TargetInfo[], targetMap: TargetMap): void {
if (!removedComponents) {
return;
}
Expand All @@ -261,7 +259,7 @@ export function applyRemovedComponents (node: Node, removedComponents: TargetInf
}
}

export function applyPropertyOverrides (node: Node, propertyOverrides: PropertyOverrideInfo[], targetMap: Record<string, any | Node | Component>): void {
export function applyPropertyOverrides (node: Node, propertyOverrides: PropertyOverrideInfo[], targetMap: TargetMap): void {
if (propertyOverrides.length <= 0) {
return;
}
Expand Down Expand Up @@ -320,18 +318,16 @@ export function applyPropertyOverrides (node: Node, propertyOverrides: PropertyO
}

export function applyTargetOverrides (node: Node): void {
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const targetOverrides = (node as any)._prefab?.targetOverrides;
const targetOverrides = node.prefab?.targetOverrides as TargetOverrideInfo[];
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
if (targetOverrides) {
for (let i = 0; i < targetOverrides.length; i++) {
const targetOverride = targetOverrides[i];

let source: Node | Component | null = targetOverride.source;
const sourceInfo = targetOverride.sourceInfo;
if (sourceInfo) {
// TODO: targetOverride.source is type of `Node | Component`, while `_prefab` does not exist on type 'Component'.
const sourceInstance = targetOverride.source?._prefab?.instance;
const src = targetOverride.source as Node;
const sourceInstance = src?.prefab?.instance;
if (sourceInstance && sourceInstance.targetMap) {
source = getTarget(sourceInfo.localID, sourceInstance.targetMap);
}
Expand All @@ -347,8 +343,8 @@ export function applyTargetOverrides (node: Node): void {
if (!targetInfo) {
continue;
}

const targetInstance = targetOverride.target?._prefab?.instance;
const targetAsNode = targetOverride.target as Node;
const targetInstance = targetAsNode?.prefab?.instance;
if (!targetInstance || !targetInstance.targetMap) {
continue;
}
Expand Down Expand Up @@ -385,10 +381,7 @@ export function applyTargetOverrides (node: Node): void {
}

export function expandPrefabInstanceNode (node: Node, recursively = false): void {
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInfo = (node as any)._prefab;
const prefabInstance = prefabInfo?.instance;
const prefabInstance = node?.prefab?.instance as PrefabInstance;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
if (prefabInstance && !prefabInstance.expanded) {
createNodeWithPrefab(node);
// nested prefab should expand before parent(property override order)
Expand All @@ -400,7 +393,7 @@ export function expandPrefabInstanceNode (node: Node, recursively = false): void
}
}

const targetMap: Record<string, any | Node | Component> = {};
const targetMap = {};
prefabInstance.targetMap = targetMap;
generateTargetMap(node, targetMap, true);
applyMountedChildren(node, prefabInstance.mountedChildren, targetMap);
Expand All @@ -418,17 +411,15 @@ export function expandPrefabInstanceNode (node: Node, recursively = false): void
}

export function expandNestedPrefabInstanceNode (node: Node): void {
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInfo = (node as any)._prefab;
const prefabInfo = node.prefab;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved

if (prefabInfo && prefabInfo.nestedPrefabInstanceRoots) {
prefabInfo.nestedPrefabInstanceRoots.forEach((instanceNode: Node) => {
expandPrefabInstanceNode(instanceNode);
// when expanding the prefab,it's children will be change,so need to apply after expanded
if (!EDITOR) {
applyNodeAndComponentId(instanceNode, (instanceNode as any)._prefab?.instance?.fileId ?? '');
}
// if (!EDITOR) {
// applyNodeAndComponentId(instanceNode, (instanceNode as any)._prefab?.instance?.fileId ?? '');
// }
});
}
}
Expand All @@ -445,7 +436,7 @@ export function applyNodeAndComponentId (prefabInstanceNode: Node, rootId: strin
const child = children[i];
// TODO(PP_Pro): after we support editorOnly tag, we could remove this any type assertion.
// Tracking issue: https://github.com/cocos/cocos-engine/issues/14613
const prefabInfo = (child as any)._prefab!;
const prefabInfo = child.prefab!;
dogeFu marked this conversation as resolved.
Show resolved Hide resolved
const fileId = prefabInfo?.instance ? prefabInfo.instance.fileId : prefabInfo?.fileId;
if (!fileId) continue;
child.id = `${rootId}${fileId}`;
Expand Down
Loading