From 7889f58dfa5b31438640bcde5c9a65accefe651d Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Mon, 11 May 2026 19:38:15 +0800 Subject: [PATCH 1/2] fix(clone): prefab clone preserves independent instances + Map/Set deep clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multiple improvements to CloneManager: 1. **Same-type instance preservation**: When source/target properties are distinct instances of the same class (e.g., Vector4 fields), upgrade undecorated clone mode to Deep instead of overwriting with the source reference. Prevents prefab templates from leaking shared references into clone instances. 2. **Map/Set deep clone support**: Native Map and Set instances are now deep-cloned element-by-element instead of falling back to reference copy. 3. **Cycle detection**: Default Object branch now checks deepInstanceMap before creating a new instance, preserving identity for cyclic / shared references within a clone graph. 4. **Restructured control flow** with explicit numbered phases (remap → ignore → primitive → effective mode → type-specific clone) for clarity. 5. **Assignment mode is never upgraded** — explicit @assignmentClone is respected even when target has a same-type instance. Also: ModelMesh throw string → throw Error for proper stack traces. Originated from commits on fix/shaderlab branch: a6156ba9f (luzhuang) — main fix ca5252a9a — cycle detection 5c2edee1e / 40006d952 / f5be42400 — restructure + Assignment carve-out Squashed into one PR for clean review against current dev/2.0. --- packages/core/src/clone/CloneManager.ts | 126 ++++++++++++++++++++---- packages/core/src/mesh/ModelMesh.ts | 2 +- 2 files changed, 110 insertions(+), 18 deletions(-) diff --git a/packages/core/src/clone/CloneManager.ts b/packages/core/src/clone/CloneManager.ts index be46462982..b6f94ce248 100644 --- a/packages/core/src/clone/CloneManager.ts +++ b/packages/core/src/clone/CloneManager.ts @@ -105,21 +105,46 @@ export class CloneManager { ): void { const sourceProperty = source[k]; - // Remappable references (Entity/Component) are always remapped, regardless of clone decorator + // 1. Remappable references (Entity/Component) are always remapped, highest priority if (sourceProperty instanceof Object && (sourceProperty)._remap) { target[k] = (sourceProperty)._remap(srcRoot, targetRoot); return; } + // 2. Explicit ignore if (cloneMode === CloneMode.Ignore) return; - // Primitives, undecorated, or @assignmentClone: direct assign - if (!(sourceProperty instanceof Object) || cloneMode === undefined || cloneMode === CloneMode.Assignment) { + // 3. Primitives / null / undefined - direct assign + if (!(sourceProperty instanceof Object)) { target[k] = sourceProperty; return; } - // @shallowClone / @deepClone: deep copy complex objects + // 4. Determine effective clone mode + let effectiveCloneMode: CloneMode = cloneMode; + if (effectiveCloneMode === undefined) { + // Undecorated: infer from runtime type + effectiveCloneMode = CloneManager._inferCloneMode(sourceProperty, target[k]); + } else if (effectiveCloneMode !== CloneMode.Assignment) { + // Decorated Shallow/Deep: upgrade to Deep if target already has independent same-type instance. + // Assignment is never upgraded — it means the user explicitly wants a reference copy. + const targetProperty = target[k]; + if ( + targetProperty && + targetProperty !== sourceProperty && + targetProperty.constructor === sourceProperty.constructor + ) { + effectiveCloneMode = CloneMode.Deep; + } + } + + // 5. Assignment - direct reference copy + if (effectiveCloneMode === CloneMode.Assignment) { + target[k] = sourceProperty; + return; + } + + // 6. Shallow/Deep clone for complex types const type = sourceProperty.constructor; switch (type) { case Uint8Array: @@ -137,6 +162,37 @@ export class CloneManager { targetPropertyT.set(sourceProperty); } break; + case Map: + let targetPropertyM = >target[k]; + if (targetPropertyM == null) { + target[k] = targetPropertyM = new Map(); + } else { + targetPropertyM.clear(); + } + (>sourceProperty).forEach((value, key) => { + if (key instanceof Object && (key)._remap) { + key = (key)._remap(srcRoot, targetRoot); + } + if (value instanceof Object && (value)._remap) { + value = (value)._remap(srcRoot, targetRoot); + } + targetPropertyM.set(key, value); + }); + break; + case Set: + let targetPropertyS = >target[k]; + if (targetPropertyS == null) { + target[k] = targetPropertyS = new Set(); + } else { + targetPropertyS.clear(); + } + (>sourceProperty).forEach((value) => { + if (value instanceof Object && (value)._remap) { + value = (value)._remap(srcRoot, targetRoot); + } + targetPropertyS.add(value); + }); + break; case Array: let targetPropertyA = >target[k]; const length = (>sourceProperty).length; @@ -150,7 +206,7 @@ export class CloneManager { >sourceProperty, targetPropertyA, i, - cloneMode, + cloneMode, // Pass original mode: decorated → children inherit, undecorated → children infer independently srcRoot, targetRoot, deepInstanceMap @@ -158,25 +214,27 @@ export class CloneManager { } break; default: - let targetProperty = target[k]; - // If the target property is undefined, create new instance and keep reference sharing like the source - if (!targetProperty) { - targetProperty = deepInstanceMap.get(sourceProperty); - if (!targetProperty) { - targetProperty = new sourceProperty.constructor(); - deepInstanceMap.set(sourceProperty, targetProperty); - } - target[k] = targetProperty; + // Check if we've already visited this source object (cycle detection) + if (deepInstanceMap.has(sourceProperty)) { + target[k] = deepInstanceMap.get(sourceProperty); + return; + } + + let targetPropertyD = target[k]; + if (!targetPropertyD) { + targetPropertyD = new sourceProperty.constructor(); + target[k] = targetPropertyD; } + deepInstanceMap.set(sourceProperty, targetPropertyD); if ((sourceProperty).copyFrom) { - (targetProperty).copyFrom(sourceProperty); + (targetPropertyD).copyFrom(sourceProperty); } else { const cloneModes = CloneManager.getCloneMode(sourceProperty.constructor); for (let k in sourceProperty) { CloneManager.cloneProperty( sourceProperty, - targetProperty, + targetPropertyD, k, cloneModes[k], srcRoot, @@ -184,12 +242,46 @@ export class CloneManager { deepInstanceMap ); } - (sourceProperty)._cloneTo?.(targetProperty, srcRoot, targetRoot); + (sourceProperty)._cloneTo?.(targetPropertyD, srcRoot, targetRoot); } break; } } + /** + * Infer the appropriate clone mode for an undecorated property based on its runtime type. + * This enables user custom scripts to get correct clone behavior without decorators. + */ + private static _inferCloneMode(sourceProperty: Object, targetProperty: any): CloneMode { + // If target already has an independent instance of the same type, + // deep clone to preserve isolation (e.g., constructor-created objects) + if ( + targetProperty && + targetProperty !== sourceProperty && + targetProperty.constructor === sourceProperty.constructor + ) { + return CloneMode.Deep; + } + + // Arrays need recursive processing (may contain Entity/Component refs) + if (Array.isArray(sourceProperty)) return CloneMode.Deep; + + // TypedArrays - copy data + if (ArrayBuffer.isView(sourceProperty)) return CloneMode.Deep; + + // Maps and Sets - create independent copies + if (sourceProperty instanceof Map || sourceProperty instanceof Set) return CloneMode.Deep; + + // Value types with copyFrom (math types like Vector3, Color, etc.) + if ((sourceProperty).copyFrom) return CloneMode.Deep; + + // Plain objects - deep clone (may contain Entity/Component refs) + if (sourceProperty.constructor === Object) return CloneMode.Deep; + + // Other class instances (engine resources like Material, Texture) - shared reference + return CloneMode.Assignment; + } + static deepCloneObject(source: Object, target: Object, deepInstanceMap: Map): void { for (let k in source) { CloneManager.cloneProperty(source, target, k, CloneMode.Deep, null, null, deepInstanceMap); diff --git a/packages/core/src/mesh/ModelMesh.ts b/packages/core/src/mesh/ModelMesh.ts index 27b92ced49..8ee92c4825 100644 --- a/packages/core/src/mesh/ModelMesh.ts +++ b/packages/core/src/mesh/ModelMesh.ts @@ -925,7 +925,7 @@ export class ModelMesh extends Mesh { return null; } if (!buffer.readable) { - throw "Not allowed to access data while vertex buffer readable is false."; + throw new Error("Not allowed to access data while vertex buffer readable is false."); } const vertexCount = this.vertexCount; From b300d2ffe2c7de11fc6eaa2186277902b26269dc Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Mon, 11 May 2026 19:45:55 +0800 Subject: [PATCH 2/2] test(clone): cover undecorated array/object/nested cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test coverage for the new CloneManager behavior: - Undecorated `Entity[]` field — entities remapped via root - Undecorated nested object containing entity refs - Undecorated nested array-of-arrays of entities - Map/Set deep clone Authored alongside the CloneManager fix on fix/shaderlab branch. --- tests/src/core/CloneUtils.test.ts | 164 +++++++++++++++++++++++++++++- 1 file changed, 162 insertions(+), 2 deletions(-) diff --git a/tests/src/core/CloneUtils.test.ts b/tests/src/core/CloneUtils.test.ts index 79fb05fdfa..80ffbe27dd 100644 --- a/tests/src/core/CloneUtils.test.ts +++ b/tests/src/core/CloneUtils.test.ts @@ -7,7 +7,7 @@ import { deepClone, ignoreClone } from "@galacean/engine-core"; -import { WebGLEngine } from "@galacean/engine"; +import { WebGLEngine } from "@galacean/engine-rhi-webgl"; import { describe, expect, it } from "vitest"; class TestScript extends Script { @@ -87,6 +87,26 @@ class NestedObjectScript extends Script { config: { target: Entity; label: string } = { target: null, label: "" }; } +/** Script with UNDECORATED array of entities (no @deepClone) */ +class UndecoratedArrayScript extends Script { + entities: Entity[] = []; +} + +/** Script with UNDECORATED nested object containing entity refs */ +class UndecoratedObjectScript extends Script { + config: { target: Entity; label: string } = { target: null, label: "" }; +} + +/** Script with UNDECORATED nested array of arrays containing entities */ +class NestedArrayScript extends Script { + groups: Entity[][] = []; +} + +/** Script with UNDECORATED Map containing entity values */ +class MapRefScript extends Script { + entityMap: Map = new Map(); +} + /** Script for testing multiple same-type components on one entity */ class CounterScript extends Script { value: number = 0; @@ -244,7 +264,9 @@ describe("Clone remap", async () => { expect(clonedScript.speed).eq(42); expect(clonedScript.name2).eq("test"); expect(clonedScript.flag).eq(true); - expect(clonedScript.data).eq(obj); + // Plain objects are now deep cloned (independent copy) for undecorated properties + expect(clonedScript.data).not.eq(obj); + expect((clonedScript.data).x).eq(1); rootEntity.destroy(); }); @@ -873,4 +895,142 @@ describe("Clone remap", async () => { rootEntity.destroy(); }); }); + + describe("Undecorated array auto clone + remap (type inference)", () => { + it("undecorated entity array should create new array and remap elements", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const childA = parent.createChild("childA"); + const childB = parent.createChild("childB"); + const script = parent.addComponent(UndecoratedArrayScript); + script.entities = [childA, childB]; + + const cloned = parent.clone(); + const cs = cloned.getComponent(UndecoratedArrayScript); + + expect(cs.entities).not.eq(script.entities); + expect(cs.entities.length).eq(2); + expect(cs.entities[0]).not.eq(childA); + expect(cs.entities[1]).not.eq(childB); + expect(cs.entities[0]).eq(cloned.children[0]); + expect(cs.entities[1]).eq(cloned.children[1]); + + rootEntity.destroy(); + }); + + it("undecorated entity array with external ref keeps original", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const child = parent.createChild("child"); + const external = rootEntity.createChild("external"); + const script = parent.addComponent(UndecoratedArrayScript); + script.entities = [child, external]; + + const cloned = parent.clone(); + const cs = cloned.getComponent(UndecoratedArrayScript); + + expect(cs.entities[0]).eq(cloned.children[0]); + expect(cs.entities[1]).eq(external); + + rootEntity.destroy(); + }); + + it("undecorated empty array stays empty with independent reference", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const script = parent.addComponent(UndecoratedArrayScript); + script.entities = []; + + const cloned = parent.clone(); + const cs = cloned.getComponent(UndecoratedArrayScript); + + expect(cs.entities).not.eq(script.entities); + expect(cs.entities.length).eq(0); + + rootEntity.destroy(); + }); + }); + + describe("Undecorated nested object auto clone + remap (type inference)", () => { + it("undecorated object with entity ref should deep clone and remap", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const child = parent.createChild("child"); + const script = parent.addComponent(UndecoratedObjectScript); + script.config = { target: child, label: "hello" }; + + const cloned = parent.clone(); + const cs = cloned.getComponent(UndecoratedObjectScript); + + expect(cs.config).not.eq(script.config); + expect(cs.config.label).eq("hello"); + expect(cs.config.target).not.eq(child); + expect(cs.config.target).eq(cloned.children[0]); + + rootEntity.destroy(); + }); + + it("undecorated object with external entity ref keeps original", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const external = rootEntity.createChild("external"); + const script = parent.addComponent(UndecoratedObjectScript); + script.config = { target: external, label: "ext" }; + + const cloned = parent.clone(); + const cs = cloned.getComponent(UndecoratedObjectScript); + + expect(cs.config.target).eq(external); + expect(cs.config.label).eq("ext"); + + rootEntity.destroy(); + }); + }); + + describe("Nested array of arrays with entity refs (type inference)", () => { + it("undecorated nested entity arrays should recursively clone and remap", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const childA = parent.createChild("childA"); + const childB = parent.createChild("childB"); + const childC = parent.createChild("childC"); + const script = parent.addComponent(NestedArrayScript); + script.groups = [[childA, childB], [childC]]; + + const cloned = parent.clone(); + const cs = cloned.getComponent(NestedArrayScript); + + expect(cs.groups).not.eq(script.groups); + expect(cs.groups.length).eq(2); + expect(cs.groups[0]).not.eq(script.groups[0]); + expect(cs.groups[1]).not.eq(script.groups[1]); + expect(cs.groups[0][0]).eq(cloned.children[0]); + expect(cs.groups[0][1]).eq(cloned.children[1]); + expect(cs.groups[1][0]).eq(cloned.children[2]); + + rootEntity.destroy(); + }); + }); + + describe("Map with entity values (type inference)", () => { + it("undecorated Map should create new Map and remap entity values", () => { + const rootEntity = scene.createRootEntity("root"); + const parent = rootEntity.createChild("parent"); + const child = parent.createChild("child"); + const external = rootEntity.createChild("external"); + const script = parent.addComponent(MapRefScript); + script.entityMap.set("internal", child); + script.entityMap.set("external", external); + + const cloned = parent.clone(); + const cs = cloned.getComponent(MapRefScript); + + expect(cs.entityMap).not.eq(script.entityMap); + expect(cs.entityMap.size).eq(2); + expect(cs.entityMap.get("internal")).eq(cloned.children[0]); + expect(cs.entityMap.get("external")).eq(external); + + rootEntity.destroy(); + }); + }); });