Skip to content

Fix compont props clone bug#2926

Merged
cptbtptpbcptdtptp merged 5 commits intogalacean:dev/2.0from
cptbtptpbcptdtptp:fix/componentRemap
Mar 18, 2026
Merged

Fix compont props clone bug#2926
cptbtptpbcptdtptp merged 5 commits intogalacean:dev/2.0from
cptbtptpbcptdtptp:fix/componentRemap

Conversation

@cptbtptpbcptdtptp
Copy link
Collaborator

@cptbtptpbcptdtptp cptbtptpbcptdtptp commented Mar 16, 2026

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Other information:

Summary by CodeRabbit

  • Refactor

    • Streamlined and unified cloning/remapping behavior across core and UI so duplication is more consistent.
  • Behavioral Changes

    • More internal references (previously ignored) are now duplicated; UI events, transitions, and certain renderer/state links are preserved on clone.
    • Custom per-component clone hooks reduced, simplifying duplication side-effects.
  • Tests

    • Added extensive test coverage validating cloning and reference remapping across many scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

Caution

Review failed

Pull request was closed or merged during review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Removed srcRoot/targetRoot params from many _cloneTo overrides, added _remap helpers on Entity/Component that delegate to CloneUtils, and refactored CloneManager/ComponentCloner to centralize cloning with early remap, unified ICustomClone shape, and consolidated deep/object cloning logic.

Changes

Cohort / File(s) Summary
Clone core & orchestration
packages/core/src/clone/CloneManager.ts, packages/core/src/clone/ComponentCloner.ts
Centralized clone flow: added early _remap path, unified ICustomClone signatures (optional _cloneTo with optional roots), used optional chaining for custom clone hooks, and consolidated deep/object cloning logic. Review new branching in CloneManager and ComponentCloner interface changes.
Remap helpers
packages/core/src/Component.ts, packages/core/src/Entity.ts, packages/core/src/clone/CloneUtils.ts
Added _remap(srcRoot, targetRoot) on Component/Entity delegating to CloneUtils.remap*; simplified remap implementations to return original when no path and remap components by component-index lookup. Verify remap contracts and call-sites.
_cloneTo signature removals (core & subsystems)
packages/core/src/Renderer.ts, packages/core/src/Camera.ts, packages/core/src/Transform.ts, packages/core/src/animation/Animator.ts, packages/core/src/audio/AudioSource.ts, packages/core/src/mesh/..., packages/core/src/particle/...
Many classes changed _cloneTo(target, srcRoot, targetRoot)_cloneTo(target) and updated super calls. Renderer stopped implementing IComponentCustomClone. Confirm subclass override alignment and cloning behavior.
2D, Text, Sprite renderers
packages/core/src/2d/sprite/SpriteMask.ts, packages/core/src/2d/sprite/SpriteRenderer.ts, packages/core/src/2d/text/TextRenderer.ts
Simplified _cloneTo signatures and super calls; clone bodies still copy sprite/font fields. Check method signatures and external invocations.
Mesh, Skin, SkinnedMesh
packages/core/src/mesh/MeshRenderer.ts, packages/core/src/mesh/Skin.ts, packages/core/src/mesh/SkinnedMeshRenderer.ts
Mesh/SkinnedMesh _cloneTo signatures simplified; Skin removed custom clone/remap logic, added internal _skinMatrices and _updatedManager, changed _bones decorator to @deepClone, and removed IComponentCustomClone usage—review skinning state and decorator impact.
UI components & decorators
packages/ui/src/component/UICanvas.ts, packages/ui/src/component/UITransform.ts, packages/ui/src/component/advanced/Image.ts, packages/ui/src/component/advanced/Text.ts, packages/ui/src/component/advanced/Button.ts, packages/ui/src/component/interactive/UIInteractive.ts, packages/ui/src/component/UIGroup.ts, packages/ui/src/component/UIRenderer.ts
Removed root params from _cloneTo, eliminated some custom remap logic (e.g., camera remap), migrated fields from @ignoreClone to @deepClone, and removed several _cloneTo overrides. Verify decorators now cover previous custom behavior (signals, transitions).
Particle, collider & shape changes
packages/core/src/particle/ParticleGenerator.ts, packages/core/src/physics/shape/ColliderShape.ts, packages/core/src/particle/modules/shape/MeshShape.ts
Removed @ignoreClone from several internal fields (making them included in cloning) and simplified MeshShape _cloneTo. Check cloning surface for newly-included fields.
Tests
tests/src/core/CloneUtils.test.ts
Added an extensive Vitest suite covering remapping, deepClone, signals, arrays, nested objects, cross-entity/script refs, self-references, and edge cases validating remap/clone semantics. Review expectations against new remap behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Requestor as Requestor
    participant CloneMgr as CloneManager
    participant ComponentCloner as ComponentCloner
    participant CloneUtils as CloneUtils
    participant Source as Source
    participant Target as Target

    Requestor->>CloneMgr: start clone(srcRoot, targetRoot)
    CloneMgr->>Source: inspect property/value
    alt Source implements _remap
        Source->>CloneUtils: _remap(srcRoot, targetRoot)
        CloneUtils-->>CloneMgr: remapped object or original
        CloneMgr-->>Target: assign remapped object
    else no _remap
        CloneMgr->>Source: determine cloneMode (primitive/assignment/deep)
        alt primitive/assignment
            CloneMgr-->>Target: direct assign
        else deep/object
            CloneMgr->>ComponentCloner: ensure/allocate target instance
            alt source has copyFrom
                ComponentCloner->>Target: target.copyFrom(source)
            else source has _cloneTo
                ComponentCloner->>Source: source._cloneTo?(target, srcRoot?, targetRoot?)
                Source-->>ComponentCloner: custom clone applied
            else
                ComponentCloner->>Source: recursively clone fields
            end
            ComponentCloner-->>CloneMgr: assign cloned object
        end
    end
    CloneMgr-->>Requestor: cloning complete
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hopped through lines with nimble feet,

I trimmed some params and made clones neat.
I taught each root how to find its twin,
Remapped the paths, let copying begin.
A rabbit's cheer for tidy Git commits!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix compont props clone bug' contains a typo ('compont' instead of 'component') and is vague about what clone bug is being fixed. While it relates to the changeset's focus on cloning behavior, it does not clearly summarize the main change: refactoring the _cloneTo signature and clone architecture. Clarify the title to accurately describe the change (e.g., 'Refactor clone API: simplify _cloneTo signature and introduce _remap') and fix the typo.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 82.63889% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.26%. Comparing base (57fdddb) to head (ee65138).
⚠️ Report is 3 commits behind head on dev/2.0.

Files with missing lines Patch % Lines
packages/core/src/clone/CloneManager.ts 66.21% 25 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           dev/2.0    #2926      +/-   ##
===========================================
- Coverage    77.29%   77.26%   -0.04%     
===========================================
  Files          884      884              
  Lines        96638    96585      -53     
  Branches      9509     9496      -13     
===========================================
- Hits         74697    74626      -71     
- Misses       21777    21795      +18     
  Partials       164      164              
Flag Coverage Δ
unittests 77.26% <82.63%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/src/mesh/Skin.ts (1)

18-27: ⚠️ Potential issue | 🔴 Critical

Cloning Skin now skips the only _skinMatrices initialization path.

_bones is deep-cloned directly, so the bones setter never runs. That setter is the only place _skinMatrices gets sized, which leaves cloned skins without a backing buffer when _updateSkinMatrices() writes into it. rootBone also still participates in skinning but is no longer part of the explicit clone setup, so this type still needs a small post-clone hook.

Possible direction
-  `@ignoreClone`
+  `@deepClone`
   private _rootBone: Entity;
_cloneTo(target: Skin): void {
  target._skinMatrices = new Float32Array(target._bones.length * 16);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/mesh/Skin.ts` around lines 18 - 27, Cloned Skin instances
miss initialization of the backing buffer because _bones is deep-cloned
(bypassing the bones setter) and rootBone isn't re-hooked; update
Skin._cloneTo(target: Skin) to allocate target._skinMatrices = new
Float32Array(target._bones.length * 16) after cloning bones and ensure the
target.rootBone is set/registered the same way original rootBone is (so
_updateSkinMatrices and skinning use the same rootBone), preserving any
UpdateFlagManager state if needed.
🧹 Nitpick comments (2)
tests/src/core/CloneUtils.test.ts (1)

18-21: Consider engine cleanup after all tests.

The engine is created once and engine.run() is called, but the engine is never destroyed after the test suite completes. While this may not cause issues in the current test environment, adding cleanup ensures resources are properly released.

♻️ Suggested improvement
 describe("CloneUtils", async () => {
   const engine = await WebGLEngine.create({ canvas: document.createElement("canvas") });
   const scene = engine.sceneManager.activeScene;
   engine.run();
+
+  afterAll(() => {
+    engine.destroy();
+  });

You'll need to import afterAll from vitest:

-import { describe, expect, it } from "vitest";
+import { afterAll, describe, expect, it } from "vitest";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/src/core/CloneUtils.test.ts` around lines 18 - 21, The test creates a
persistent WebGLEngine via WebGLEngine.create and calls engine.run() but never
tears it down; add a vitest afterAll hook (import afterAll) that calls the
appropriate teardown method on the engine (e.g., engine.destroy() or
engine.dispose() depending on the API) to stop the engine and release resources
after the test suite; ensure the engine variable is in a scope accessible to
afterAll so the cleanup runs even if tests fail.
packages/ui/src/component/UICanvas.ts (1)

20-20: Unused import: CloneUtils.

The CloneUtils import is no longer used after removing the camera remapping logic. Consider removing it to keep imports clean.

♻️ Proposed fix
 import {
   BoolUpdateFlag,
   Camera,
   CameraModifyFlags,
   Component,
   DependentMode,
   DisorderedArray,
   Entity,
   EntityModifyFlags,
   Logger,
   MathUtil,
   Matrix,
   Ray,
   Vector2,
   Vector3,
   assignmentClone,
   deepClone,
   dependentComponents,
-  ignoreClone,
-  CloneUtils
+  ignoreClone
 } from "@galacean/engine";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/component/UICanvas.ts` at line 20, Remove the unused import
CloneUtils from the UICanvas module: locate the import statement that includes
CloneUtils (symbol: CloneUtils) and delete it, verify there are no remaining
references to CloneUtils in UICanvas (e.g., in functions or JSX) and run the
linter/build to ensure no unresolved identifiers remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/clone/CloneManager.ts`:
- Around line 106-114: In CloneManager.ts adjust the clone logic so the
CloneMode.Ignore check runs before the _remap fast path: when iterating
properties (sourceProperty / source[k]) first test if cloneMode ===
CloneMode.Ignore and return if so, and only afterwards handle the case where
sourceProperty instanceof Object and (<ICustomClone>sourceProperty)._remap by
calling _remap(srcRoot, targetRoot); this ensures `@ignoreClone` fields (e.g.,
Skin._rootBone, UIInteractive._rootCanvas/_group) are not remapped prematurely.

In `@packages/ui/src/component/interactive/UIInteractive.ts`:
- Around line 53-54: The `@deepClone` on UIInteractive._transitions causes new
Transition instances to be created but their internal mutable fields (_normal,
_pressed, _hover, _disabled, _initialValue, _finalValue, _currentValue) are
still shared; fix this by adding proper clone support on the Transition type
(implement a copyFrom() or _cloneTo() on Transition and its subclasses or add
field-level clone decorators to those mutable fields) so deep cloning copies
their internal state, or alternatively remove `@deepClone` on _transitions and
document/accept shallow cloning semantics—update the Transition class (and any
subclasses) accordingly to ensure cloned UIInteractive instances do not share
mutable transition state.

In `@packages/ui/src/component/UICanvas.ts`:
- Around line 416-418: The _cloneTo method on UICanvas no longer remaps the
`@ignoreClone` _camera so cloned UICanvas instances in ScreenSpaceCamera mode keep
a reference to the original camera; restore camera remapping by invoking the
camera's remap (or _remap) on the cloned instance inside UICanvas._cloneTo
(e.g., if target._camera exists call target._camera =
this._camera?._remap(target) or use the same remapping utility previously used),
and remove the now-unused CloneUtils import if you replace its usage; ensure you
reference _cloneTo, UICanvas, _camera, _remap, ScreenSpaceCamera, and CloneUtils
when making the change.

---

Outside diff comments:
In `@packages/core/src/mesh/Skin.ts`:
- Around line 18-27: Cloned Skin instances miss initialization of the backing
buffer because _bones is deep-cloned (bypassing the bones setter) and rootBone
isn't re-hooked; update Skin._cloneTo(target: Skin) to allocate
target._skinMatrices = new Float32Array(target._bones.length * 16) after cloning
bones and ensure the target.rootBone is set/registered the same way original
rootBone is (so _updateSkinMatrices and skinning use the same rootBone),
preserving any UpdateFlagManager state if needed.

---

Nitpick comments:
In `@packages/ui/src/component/UICanvas.ts`:
- Line 20: Remove the unused import CloneUtils from the UICanvas module: locate
the import statement that includes CloneUtils (symbol: CloneUtils) and delete
it, verify there are no remaining references to CloneUtils in UICanvas (e.g., in
functions or JSX) and run the linter/build to ensure no unresolved identifiers
remain.

In `@tests/src/core/CloneUtils.test.ts`:
- Around line 18-21: The test creates a persistent WebGLEngine via
WebGLEngine.create and calls engine.run() but never tears it down; add a vitest
afterAll hook (import afterAll) that calls the appropriate teardown method on
the engine (e.g., engine.destroy() or engine.dispose() depending on the API) to
stop the engine and release resources after the test suite; ensure the engine
variable is in a scope accessible to afterAll so the cleanup runs even if tests
fail.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fcd9300e-e2ff-40a4-9976-abeb02cae213

📥 Commits

Reviewing files that changed from the base of the PR and between 1efbfe2 and 0539040.

📒 Files selected for processing (23)
  • packages/core/src/2d/sprite/SpriteMask.ts
  • packages/core/src/2d/sprite/SpriteRenderer.ts
  • packages/core/src/2d/text/TextRenderer.ts
  • packages/core/src/Camera.ts
  • packages/core/src/Component.ts
  • packages/core/src/Entity.ts
  • packages/core/src/Renderer.ts
  • packages/core/src/Transform.ts
  • packages/core/src/animation/Animator.ts
  • packages/core/src/audio/AudioSource.ts
  • packages/core/src/clone/CloneManager.ts
  • packages/core/src/clone/ComponentCloner.ts
  • packages/core/src/mesh/MeshRenderer.ts
  • packages/core/src/mesh/Skin.ts
  • packages/core/src/mesh/SkinnedMeshRenderer.ts
  • packages/core/src/particle/modules/shape/MeshShape.ts
  • packages/ui/src/component/UICanvas.ts
  • packages/ui/src/component/UITransform.ts
  • packages/ui/src/component/advanced/Button.ts
  • packages/ui/src/component/advanced/Image.ts
  • packages/ui/src/component/advanced/Text.ts
  • packages/ui/src/component/interactive/UIInteractive.ts
  • tests/src/core/CloneUtils.test.ts

Comment on lines +416 to 418
_cloneTo(target: UICanvas): void {
target.renderMode = this._renderMode;
const camera = this._camera;
if (camera) {
target.camera = CloneUtils.remapComponent(srcRoot, targetRoot, camera);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's any other mechanism handling camera remapping for UICanvas
rg -n "_remap|remapComponent" packages/ui/src/component/UICanvas.ts

# Check how camera is used in the cloning context
rg -nA3 "UICanvas.*clone|clone.*UICanvas" packages/

Repository: galacean/engine

Length of output: 310


🏁 Script executed:

# Check the _camera field decoration and surrounding code
sed -n '80,90p' packages/ui/src/component/UICanvas.ts

# Check for CloneUtils import
rg -n "import.*CloneUtils|from.*CloneUtils" packages/ui/src/component/UICanvas.ts

# Check if there's a _remap method elsewhere in UICanvas
ast-grep --pattern '$$$
_remap($_) {
  $$$
}
$$$' packages/ui/src/component/UICanvas.ts

# Look for CloneManager to understand clone behavior
rg -n "ignoreClone" packages/core/src/Clone/CloneManager.ts -A 2 -B 2

Repository: galacean/engine

Length of output: 487


🏁 Script executed:

# Find CloneManager file
fd -t f "CloneManager" packages/

# Check how `@ignoreClone` decorator affects cloning
rg -n "ignoreClone" packages/ -t ts --max-count 10

# Look for the clone implementation and how it handles `@ignoreClone`
rg -n "@ignoreClone" packages/core/src/ -A 3 -B 3 | head -50

# Check if there's camera-specific cloning logic elsewhere
rg -n "camera.*clone|clone.*camera" packages/ui/src/component/UICanvas.ts -i

# Check the import statements in UICanvas.ts
head -30 packages/ui/src/component/UICanvas.ts

Repository: galacean/engine

Length of output: 25490


🏁 Script executed:

# Check if CloneUtils is actually used in UICanvas.ts
rg -n "CloneUtils" packages/ui/src/component/UICanvas.ts

# Read the CloneManager to understand how `@ignoreClone` fields are handled
head -150 packages/core/src/clone/CloneManager.ts

Repository: galacean/engine

Length of output: 5243


🏁 Script executed:

# Check if Camera has a _remap method
rg -n "_remap" packages/core/src/Camera.ts

# Also search for _remap methods in the codebase to understand the pattern
rg -n "^\s*_remap\(" packages/core/src/ -A 3 | head -50

# Look for ICustomClone interface to understand _remap contract
rg -n "ICustomClone" packages/core/src/clone/ -A 5

Repository: galacean/engine

Length of output: 3807


🏁 Script executed:

# Check if Camera has a _remap method
rg -n "_remap" packages/core/src/Camera.ts

# Also verify what happens when a property has `@ignoreClone` decorator
# The behavior should be: if the property itself is marked `@ignoreClone`, it's skipped
# Let's confirm this by looking at the cloneProperty method more carefully
sed -n '100,120p' packages/core/src/clone/CloneManager.ts

# Check the full signature of cloneProperty to understand the flow
sed -n '100,200p' packages/core/src/clone/CloneManager.ts

Repository: galacean/engine

Length of output: 4097


🏁 Script executed:

# Verify all usages of CloneUtils in UICanvas.ts
rg -n "CloneUtils" packages/ui/src/component/UICanvas.ts -B 2 -A 2

# Let's also check the git history or current state - look at the full _cloneTo method
# and surrounding code to understand the context better
sed -n '410,425p' packages/ui/src/component/UICanvas.ts

# Check what the old _cloneTo looked like by searching for references to remapComponent
rg -n "remapComponent" packages/ --max-count 10

Repository: galacean/engine

Length of output: 1122


🏁 Script executed:

# Final check: confirm Camera class doesn't have _remap method
rg -n "_remap" packages/core/src/Camera.ts

# Also verify the render mode behavior - is it intentional that it's `@ignoreClone` yet manually assigned in _cloneTo?
sed -n '80,90p' packages/ui/src/component/UICanvas.ts

Repository: galacean/engine

Length of output: 390


Camera remapping was removed without an alternative mechanism.

The previous camera remapping via CloneUtils.remapComponent() in _cloneTo has been removed. Since _camera is marked @ignoreClone, it is skipped by the automatic cloning mechanism and won't be remapped. Without a _remap method on Camera or a manual assignment in _cloneTo, a cloned UICanvas in ScreenSpaceCamera mode will reference the original camera instead of a cloned one.

Additionally, the CloneUtils import is now unused and can be removed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/component/UICanvas.ts` around lines 416 - 418, The _cloneTo
method on UICanvas no longer remaps the `@ignoreClone` _camera so cloned UICanvas
instances in ScreenSpaceCamera mode keep a reference to the original camera;
restore camera remapping by invoking the camera's remap (or _remap) on the
cloned instance inside UICanvas._cloneTo (e.g., if target._camera exists call
target._camera = this._camera?._remap(target) or use the same remapping utility
previously used), and remove the now-unused CloneUtils import if you replace its
usage; ensure you reference _cloneTo, UICanvas, _camera, _remap,
ScreenSpaceCamera, and CloneUtils when making the change.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
tests/src/core/CloneUtils.test.ts (1)

289-303: undefined path is declared but not asserted in the null/undefined test.

The test name covers both null and undefined, but undefinedEntity is never checked. Add an explicit assertion to lock this behavior.

Minimal assertion addition
 it("null entity/component refs should not crash and remain null", () => {
@@
   expect(cs.nullEntity).eq(null);
+  expect(cs.undefinedEntity).eq(undefined);
   expect(cs.nullRenderer).eq(null);
   expect(cs.someNumber).eq(0);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/src/core/CloneUtils.test.ts` around lines 289 - 303, The test "null
entity/component refs should not crash and remain null" asserts null fields but
omits asserting the undefined field declared on NullRefScript; after obtaining
cs via cloned.getComponent(NullRefScript) add an explicit assertion for the
undefined path (e.g., expect(cs.undefinedEntity).eq(undefined)) inside that test
so the undefined behavior is locked down alongside null checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/src/core/CloneUtils.test.ts`:
- Around line 1-9: The test file has Prettier/ESLint formatting violations in
the import list (Entity, MeshRenderer, Script, Signal, assignmentClone,
deepClone, ignoreClone) and an inline closure body later in the file; reformat
the import block into a single-line or properly multiline style per project
Prettier settings and update the inline closure to use the preferred brace/arrow
formatting (or run the project's formatter). Run the project's formatter/ESLint
--fix (or Prettier) on tests/src/core/CloneUtils.test.ts to apply consistent
formatting and ensure the import list and the inline closure body comply with
lint rules before merging.
- Around line 116-705: The tests leak scene state because each it() calls
rootEntity.destroy() only on the happy path; make teardown failure-safe by
ensuring root entities are always destroyed: either add an afterEach() that
tracks and destroys any created root entities (e.g., collect rootEntity
instances created in each it into a local array) or wrap each test body in a
try/finally that calls rootEntity.destroy() in finally; update references in the
test file CloneUtils.test.ts (look for variables named rootEntity created in
multiple describe/it blocks and the existing rootEntity.destroy() calls) to use
the new tear-down pattern so leaked entities cannot persist between tests.
- Around line 111-115: The test currently awaits WebGLEngine.create inside an
async describe block which runs during test collection; move engine
initialization and scene setup into a beforeAll hook and teardown into afterAll:
create the engine via WebGLEngine.create, assign engine and scene
(engine.sceneManager.activeScene), call engine.run() in beforeAll, and
stop/dispose/cleanup the engine in afterAll so tests use a ready engine; update
references to the shared engine variable declared in the describe scope and
ensure beforeAll/afterAll handle async awaits.

---

Nitpick comments:
In `@tests/src/core/CloneUtils.test.ts`:
- Around line 289-303: The test "null entity/component refs should not crash and
remain null" asserts null fields but omits asserting the undefined field
declared on NullRefScript; after obtaining cs via
cloned.getComponent(NullRefScript) add an explicit assertion for the undefined
path (e.g., expect(cs.undefinedEntity).eq(undefined)) inside that test so the
undefined behavior is locked down alongside null checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0d36fbba-ae4d-4811-986f-cd4fb9a64c24

📥 Commits

Reviewing files that changed from the base of the PR and between 0539040 and 6a92386.

📒 Files selected for processing (1)
  • tests/src/core/CloneUtils.test.ts

Comment on lines +1 to +9
import {
Entity,
MeshRenderer,
Script,
Signal,
assignmentClone,
deepClone,
ignoreClone
} from "@galacean/engine-core";
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Prettier violations are currently failing lint.

The import list formatting (Line 1-9) and inline closure body formatting (Line 544) match the ESLint/prettier errors. Please run formatter or apply the lint fixes before merge.

Also applies to: 544-544

🧰 Tools
🪛 ESLint

[error] 1-9: Replace ⏎··Entity,⏎··MeshRenderer,⏎··Script,⏎··Signal,⏎··assignmentClone,⏎··deepClone,⏎··ignoreClone⏎ with ·Entity,·MeshRenderer,·Script,·Signal,·assignmentClone,·deepClone,·ignoreClone·

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/src/core/CloneUtils.test.ts` around lines 1 - 9, The test file has
Prettier/ESLint formatting violations in the import list (Entity, MeshRenderer,
Script, Signal, assignmentClone, deepClone, ignoreClone) and an inline closure
body later in the file; reformat the import block into a single-line or properly
multiline style per project Prettier settings and update the inline closure to
use the preferred brace/arrow formatting (or run the project's formatter). Run
the project's formatter/ESLint --fix (or Prettier) on
tests/src/core/CloneUtils.test.ts to apply consistent formatting and ensure the
import list and the inline closure body comply with lint rules before merging.

Comment on lines +111 to +115
describe("Clone remap", async () => {
const engine = await WebGLEngine.create({ canvas: document.createElement("canvas") });
const scene = engine.sceneManager.activeScene;
engine.run();

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

In Vitest 2.x, is using an async callback in describe()supported for test definition, or should async setup be done inbeforeAll instead?

💡 Result:

In Vitest 2.x you can technically write describe('x', async () => { ... }) (it’s not considered invalid by Vitest’s own ESLint rule set) [1], and you’ll even see async () => {} used in docs examples (notably under describe.concurrent) [2].

However, you should not rely on await inside describe() to perform async setup that your tests depend on, because describe bodies run during the test collection phase (tests are registered as a side effect of executing the file), and async setup belongs in hooks that run before execution of tests—typically beforeAll for once-per-suite setup [3].

Recommendation: keep describe() for synchronous test definition, and put async setup in beforeAll (or beforeEach if needed). [3]

Sources: [1] [2] [3]


Move engine initialization from describe callback to beforeAll hook.

Line 111 uses an async describe callback. During test collection (when describe bodies execute), the engine creation and assignment are non-blocking—tests can collect and register before the engine is ready. Move const engine = await WebGLEngine.create(...) and scene setup into a beforeAll hook to ensure the engine is initialized before any test runs. Use afterAll for cleanup.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/src/core/CloneUtils.test.ts` around lines 111 - 115, The test currently
awaits WebGLEngine.create inside an async describe block which runs during test
collection; move engine initialization and scene setup into a beforeAll hook and
teardown into afterAll: create the engine via WebGLEngine.create, assign engine
and scene (engine.sceneManager.activeScene), call engine.run() in beforeAll, and
stop/dispose/cleanup the engine in afterAll so tests use a ready engine; update
references to the shared engine variable declared in the describe scope and
ensure beforeAll/afterAll handle async awaits.

Comment on lines +116 to +705
describe("Basic Entity/Component remap", () => {
it("script undecorated Entity ref should remap to cloned entity", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(TestScript);
script.targetEntity = child;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);
const clonedChild = clonedParent.children[0];

expect(clonedScript.targetEntity).not.eq(child);
expect(clonedScript.targetEntity).eq(clonedChild);
expect(clonedScript.targetEntity.name).eq("child");

rootEntity.destroy();
});

it("script undecorated Component ref should remap to cloned component", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const meshRenderer = child.addComponent(MeshRenderer);
const script = parent.addComponent(TestScript);
script.targetRenderer = meshRenderer;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);
const clonedChild = clonedParent.children[0];
const clonedMeshRenderer = clonedChild.getComponent(MeshRenderer);

expect(clonedScript.targetRenderer).not.eq(meshRenderer);
expect(clonedScript.targetRenderer).eq(clonedMeshRenderer);

rootEntity.destroy();
});

it("script ref to entity outside hierarchy should keep original", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const script = parent.addComponent(TestScript);
script.externalEntity = external;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);

expect(clonedScript.externalEntity).eq(external);

rootEntity.destroy();
});

it("script ref to component outside hierarchy should keep original", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const externalMR = external.addComponent(MeshRenderer);
const script = parent.addComponent(TestScript);
script.externalRenderer = externalMR;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);

expect(clonedScript.externalRenderer).eq(externalMR);

rootEntity.destroy();
});

it("deep hierarchy entity ref should remap correctly", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const grandchild = child.createChild("grandchild");
const script = parent.addComponent(TestScript);
script.deepChild = grandchild;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);
const clonedGrandchild = clonedParent.children[0].children[0];

expect(clonedScript.deepChild).not.eq(grandchild);
expect(clonedScript.deepChild).eq(clonedGrandchild);
expect(clonedScript.deepChild.name).eq("grandchild");

rootEntity.destroy();
});

it("script ref to self entity (clone root) should remap", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(TestScript);
script.selfRef = parent;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);

expect(clonedScript.selfRef).not.eq(parent);
expect(clonedScript.selfRef).eq(clonedParent);

rootEntity.destroy();
});

it("primitive and plain object props should not be affected", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(TestScript);
const obj = { x: 1 };
script.speed = 42;
script.name2 = "test";
script.flag = true;
script.data = obj;

const clonedParent = parent.clone();
const clonedScript = clonedParent.getComponent(TestScript);

expect(clonedScript.speed).eq(42);
expect(clonedScript.name2).eq("test");
expect(clonedScript.flag).eq(true);
expect(clonedScript.data).eq(obj);

rootEntity.destroy();
});
});

describe("Multiple and duplicate refs", () => {
it("multiple entity/component refs on same script all remap independently", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const childA = parent.createChild("childA");
const childB = parent.createChild("childB");
const mrA = childA.addComponent(MeshRenderer);
const mrB = childB.addComponent(MeshRenderer);
const script = parent.addComponent(MultiRefScript);
script.entityA = childA;
script.entityB = childB;
script.rendererA = mrA;
script.rendererB = mrB;

const cloned = parent.clone();
const cs = cloned.getComponent(MultiRefScript);

expect(cs.entityA).not.eq(childA);
expect(cs.entityB).not.eq(childB);
expect(cs.entityA.name).eq("childA");
expect(cs.entityB.name).eq("childB");
expect(cs.entityA).eq(cloned.children[0]);
expect(cs.entityB).eq(cloned.children[1]);
expect(cs.rendererA).eq(cloned.children[0].getComponent(MeshRenderer));
expect(cs.rendererB).eq(cloned.children[1].getComponent(MeshRenderer));

rootEntity.destroy();
});

it("two properties referencing the same entity both remap to the same cloned entity", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(DuplicateRefScript);
script.ref1 = child;
script.ref2 = child;

const cloned = parent.clone();
const cs = cloned.getComponent(DuplicateRefScript);

expect(cs.ref1).not.eq(child);
expect(cs.ref1).eq(cs.ref2);
expect(cs.ref1).eq(cloned.children[0]);

rootEntity.destroy();
});
});

describe("Null and undefined refs", () => {
it("null entity/component refs should not crash and remain null", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(NullRefScript);

const cloned = parent.clone();
const cs = cloned.getComponent(NullRefScript);

expect(cs.nullEntity).eq(null);
expect(cs.nullRenderer).eq(null);
expect(cs.someNumber).eq(0);

rootEntity.destroy();
});
});

describe("Sibling entity refs", () => {
it("ref to sibling entity under clone root should remap", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const childA = parent.createChild("childA");
const childB = parent.createChild("childB");
const mrB = childB.addComponent(MeshRenderer);
const script = childA.addComponent(SiblingRefScript);
script.sibling = childB;
script.siblingRenderer = mrB;

const cloned = parent.clone();
const clonedChildA = cloned.children[0];
const clonedChildB = cloned.children[1];
const cs = clonedChildA.getComponent(SiblingRefScript);

expect(cs.sibling).not.eq(childB);
expect(cs.sibling).eq(clonedChildB);
expect(cs.siblingRenderer).not.eq(mrB);
expect(cs.siblingRenderer).eq(clonedChildB.getComponent(MeshRenderer));

rootEntity.destroy();
});
});

describe("Clone decorator interaction with _remap", () => {
it("@assignmentClone entity ref still gets remapped via _remap priority", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(DecoratedRefScript);
script.assignedEntity = child;

const cloned = parent.clone();
const cs = cloned.getComponent(DecoratedRefScript);

expect(cs.assignedEntity).not.eq(child);
expect(cs.assignedEntity).eq(cloned.children[0]);

rootEntity.destroy();
});

it("@ignoreClone entity ref still gets remapped via _remap priority", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(DecoratedRefScript);
script.ignoredEntity = child;

const cloned = parent.clone();
const cs = cloned.getComponent(DecoratedRefScript);

expect(cs.ignoredEntity).not.eq(child);
expect(cs.ignoredEntity).eq(cloned.children[0]);

rootEntity.destroy();
});

it("undecorated entity ref remaps correctly", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(DecoratedRefScript);
script.autoRemapEntity = child;

const cloned = parent.clone();
const cs = cloned.getComponent(DecoratedRefScript);

expect(cs.autoRemapEntity).not.eq(child);
expect(cs.autoRemapEntity).eq(cloned.children[0]);

rootEntity.destroy();
});

it("@ignoreClone entity ref outside hierarchy stays original", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const script = parent.addComponent(DecoratedRefScript);
script.ignoredEntity = external;

const cloned = parent.clone();
const cs = cloned.getComponent(DecoratedRefScript);

expect(cs.ignoredEntity).eq(external);

rootEntity.destroy();
});
});

describe("@deepClone array of entities", () => {
it("deep cloned entity array should remap internal refs", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const childA = parent.createChild("childA");
const childB = parent.createChild("childB");
const script = parent.addComponent(ArrayRefScript);
script.entities = [childA, childB];

const cloned = parent.clone();
const cs = cloned.getComponent(ArrayRefScript);

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("deep cloned 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(ArrayRefScript);
script.entities = [child, external];

const cloned = parent.clone();
const cs = cloned.getComponent(ArrayRefScript);

expect(cs.entities[0]).eq(cloned.children[0]);
expect(cs.entities[1]).eq(external);

rootEntity.destroy();
});

it("deep cloned empty entity array stays empty", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(ArrayRefScript);
script.entities = [];

const cloned = parent.clone();
const cs = cloned.getComponent(ArrayRefScript);

expect(cs.entities).not.eq(script.entities);
expect(cs.entities.length).eq(0);

rootEntity.destroy();
});
});

describe("Component self and cross references", () => {
it("script referencing itself should remap to cloned script", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(SelfComponentRefScript);
script.selfScript = script;

const cloned = parent.clone();
const cs = cloned.getComponent(SelfComponentRefScript);

expect(cs.selfScript).not.eq(script);
expect(cs.selfScript).eq(cs);

rootEntity.destroy();
});

it("script referencing another script on child entity should remap", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const childScript = child.addComponent(TestScript);
const script = parent.addComponent(CrossScriptRefScript);
script.otherScript = childScript;

const cloned = parent.clone();
const cs = cloned.getComponent(CrossScriptRefScript);
const clonedChildScript = cloned.children[0].getComponent(TestScript);

expect(cs.otherScript).not.eq(childScript);
expect(cs.otherScript).eq(clonedChildScript);

rootEntity.destroy();
});

it("script referencing external script should keep original", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const externalScript = external.addComponent(TestScript);
const script = parent.addComponent(CrossScriptRefScript);
script.otherScript = externalScript;

const cloned = parent.clone();
const cs = cloned.getComponent(CrossScriptRefScript);

expect(cs.otherScript).eq(externalScript);

rootEntity.destroy();
});
});

describe("Nested @deepClone object with entity refs", () => {
it("entity ref inside deep cloned plain object should remap", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = parent.addComponent(NestedObjectScript);
script.config = { target: child, label: "hello" };

const cloned = parent.clone();
const cs = cloned.getComponent(NestedObjectScript);

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("entity ref inside deep cloned object pointing outside keeps original", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const script = parent.addComponent(NestedObjectScript);
script.config = { target: external, label: "ext" };

const cloned = parent.clone();
const cs = cloned.getComponent(NestedObjectScript);

expect(cs.config.target).eq(external);
expect(cs.config.label).eq("ext");

rootEntity.destroy();
});
});

describe("Signal clone with structured bindings", () => {
it("@deepClone Signal should not copy closure listeners", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const script = parent.addComponent(SignalScript);
let called = false;
script.onFire.on(() => { called = true; });

const cloned = parent.clone();
const cs = cloned.getComponent(SignalScript);

expect(cs.onFire).not.eq(script.onFire);
cs.onFire.invoke(1);
expect(called).eq(false);

rootEntity.destroy();
});

it("@deepClone Signal should remap structured binding target to cloned hierarchy", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const handlerEntity = parent.createChild("handler");
const handler = handlerEntity.addComponent(ClickHandler);
const script = parent.addComponent(SignalScript);
script.onFire.on(handler, "handleClick");

const cloned = parent.clone();
const cs = cloned.getComponent(SignalScript);
const clonedHandler = cloned.findByName("handler").getComponent(ClickHandler);

cs.onFire.invoke(1);
expect(clonedHandler.callCount).eq(1);
expect(handler.callCount).eq(0);

rootEntity.destroy();
});

it("@deepClone Signal should keep external structured binding target", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const external = rootEntity.createChild("external");
const externalHandler = external.addComponent(ClickHandler);
const script = parent.addComponent(SignalScript);
script.onFire.on(externalHandler, "handleClick");

const cloned = parent.clone();
const cs = cloned.getComponent(SignalScript);

cs.onFire.invoke(1);
expect(externalHandler.callCount).eq(1);

rootEntity.destroy();
});

it("@deepClone Signal should remap structured binding with pre-resolved args", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const handlerEntity = parent.createChild("handler");
const handler = handlerEntity.addComponent(ClickHandler);
const script = parent.addComponent(SignalScript);
script.onFire.on(handler, "handleClickWithPrefix", "myPrefix");

const cloned = parent.clone();
const cs = cloned.getComponent(SignalScript);
const clonedHandler = cloned.findByName("handler").getComponent(ClickHandler);

cs.onFire.invoke(1);
expect(clonedHandler.callCount).eq(1);
expect(clonedHandler.lastPrefix).eq("myPrefix");
expect(handler.callCount).eq(0);

rootEntity.destroy();
});

it("@deepClone Signal should preserve once flag on structured binding", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const handlerEntity = parent.createChild("handler");
const handler = handlerEntity.addComponent(ClickHandler);
const script = parent.addComponent(SignalScript);
script.onFire.once(handler, "handleClick");

const cloned = parent.clone();
const cs = cloned.getComponent(SignalScript);
const clonedHandler = cloned.findByName("handler").getComponent(ClickHandler);

cs.onFire.invoke(1);
expect(clonedHandler.callCount).eq(1);
cs.onFire.invoke(2);
expect(clonedHandler.callCount).eq(1); // once: removed after first call

rootEntity.destroy();
});
});

describe("Clone hierarchy integrity", () => {
it("clone preserves children count and names", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
parent.createChild("a");
parent.createChild("b");
parent.createChild("c");

const cloned = parent.clone();
expect(cloned.children.length).eq(3);
expect(cloned.children[0].name).eq("a");
expect(cloned.children[1].name).eq("b");
expect(cloned.children[2].name).eq("c");

rootEntity.destroy();
});

it("clone of deeply nested hierarchy preserves structure", () => {
const rootEntity = scene.createRootEntity("root");
const a = rootEntity.createChild("a");
const b = a.createChild("b");
const c = b.createChild("c");
const d = c.createChild("d");

const cloned = a.clone();
expect(cloned.children[0].name).eq("b");
expect(cloned.children[0].children[0].name).eq("c");
expect(cloned.children[0].children[0].children[0].name).eq("d");

rootEntity.destroy();
});

it("script on child entity with ref to parent should remap", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const child = parent.createChild("child");
const script = child.addComponent(TestScript);
script.targetEntity = parent;

const cloned = parent.clone();
const clonedChild = cloned.children[0];
const cs = clonedChild.getComponent(TestScript);

expect(cs.targetEntity).not.eq(parent);
expect(cs.targetEntity).eq(cloned);

rootEntity.destroy();
});

it("multiple scripts on different entities with cross-refs all remap correctly", () => {
const rootEntity = scene.createRootEntity("root");
const parent = rootEntity.createChild("parent");
const childA = parent.createChild("childA");
const childB = parent.createChild("childB");

const scriptA = childA.addComponent(TestScript);
scriptA.targetEntity = childB;

const scriptB = childB.addComponent(TestScript);
scriptB.targetEntity = childA;

const cloned = parent.clone();
const clonedA = cloned.children[0];
const clonedB = cloned.children[1];
const csA = clonedA.getComponent(TestScript);
const csB = clonedB.getComponent(TestScript);

expect(csA.targetEntity).eq(clonedB);
expect(csB.targetEntity).eq(clonedA);

rootEntity.destroy();
});
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Cleanup is not failure-safe; leaked entities can cascade into later tests.

rootEntity.destroy() is only called at the happy path end of each test. If an assertion fails earlier, shared scene state leaks into following tests. Use afterEach (or try/finally) for guaranteed teardown.

Suggested hardening pattern
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it } from "vitest";

+const rootsToDestroy: Entity[] = [];
+afterEach(() => {
+  for (const root of rootsToDestroy.splice(0)) root.destroy();
+});

-const rootEntity = scene.createRootEntity("root");
+const rootEntity = scene.createRootEntity("root");
+rootsToDestroy.push(rootEntity);
🧰 Tools
🪛 ESLint

[error] 544-544: Replace ·called·=·true; with ⏎········called·=·true;⏎·····

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/src/core/CloneUtils.test.ts` around lines 116 - 705, The tests leak
scene state because each it() calls rootEntity.destroy() only on the happy path;
make teardown failure-safe by ensuring root entities are always destroyed:
either add an afterEach() that tracks and destroys any created root entities
(e.g., collect rootEntity instances created in each it into a local array) or
wrap each test body in a try/finally that calls rootEntity.destroy() in finally;
update references in the test file CloneUtils.test.ts (look for variables named
rootEntity created in multiple describe/it blocks and the existing
rootEntity.destroy() calls) to use the new tear-down pattern so leaked entities
cannot persist between tests.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/core/src/clone/CloneManager.ts (1)

108-114: ⚠️ Potential issue | 🟠 Major

@ignoreClone is bypassed by the early _remap fast path.

cloneMode === CloneMode.Ignore needs to run before _remap; otherwise ignored Entity/Component refs still get remapped and assigned.

Suggested fix
-    // Remappable references (Entity/Component) are always remapped, regardless of clone decorator
-    if (sourceProperty instanceof Object && (<ICustomClone>sourceProperty)._remap) {
-      target[k] = (<ICustomClone>sourceProperty)._remap(srcRoot, targetRoot);
-      return;
-    }
-
-    if (cloneMode === CloneMode.Ignore) return;
+    if (cloneMode === CloneMode.Ignore) return;
+
+    if (sourceProperty instanceof Object && (<ICustomClone>sourceProperty)._remap) {
+      target[k] = (<ICustomClone>sourceProperty)._remap(srcRoot, targetRoot);
+      return;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/clone/CloneManager.ts` around lines 108 - 114, The early
remap fast path in CloneManager is bypassing the `@ignoreClone` check: move the
cloneMode === CloneMode.Ignore guard to run before the remap branch so ignored
refs are not processed; specifically, in the cloning loop check if cloneMode ===
CloneMode.Ignore for the current property (the same condition that now appears
after the remap branch) before evaluating sourceProperty instanceof Object and
(<ICustomClone>sourceProperty)._remap, and if ignored skip assigning to
target[k] so _remap is not called or applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/clone/CloneManager.ts`:
- Around line 140-159: The Array branch in CloneManager (the `case Array:`
block) ignores the `deepInstanceMap`, causing duplicated shared arrays and
infinite recursion for self-referential arrays; fix it by checking
`deepInstanceMap` for an existing mapped target for the `sourceProperty` before
allocating or reusing `target[k]`, return or assign that mapped instance if
present, and when creating a new array instance assign it into `deepInstanceMap`
immediately (before iterating) so subsequent recursive clones reuse it; ensure
the rest of the loop continues to call `CloneManager.cloneProperty` as before
using the mapped/created `targetPropertyA`.
- Around line 124-161: The switch in CloneManager.cloneProperty (handling typed
arrays, Array, and default) declares lexical variables (targetPropertyT,
targetPropertyA, length, targetProperty) directly in case clauses which violates
noSwitchDeclarations; fix by wrapping each case branch that declares let/const
in its own block { ... } so targetPropertyT, targetPropertyA, length and
targetProperty are properly block-scoped, leaving existing logic
(slicing/setting for TypedArrays, resizing/iterating for Array, and the default
object handling) unchanged.

---

Duplicate comments:
In `@packages/core/src/clone/CloneManager.ts`:
- Around line 108-114: The early remap fast path in CloneManager is bypassing
the `@ignoreClone` check: move the cloneMode === CloneMode.Ignore guard to run
before the remap branch so ignored refs are not processed; specifically, in the
cloning loop check if cloneMode === CloneMode.Ignore for the current property
(the same condition that now appears after the remap branch) before evaluating
sourceProperty instanceof Object and (<ICustomClone>sourceProperty)._remap, and
if ignored skip assigning to target[k] so _remap is not called or applied.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 56072dd0-2e84-43b0-84bd-f855313b4b24

📥 Commits

Reviewing files that changed from the base of the PR and between 6a92386 and 37eabf5.

📒 Files selected for processing (1)
  • packages/core/src/clone/CloneManager.ts

Comment on lines +124 to +161
switch (type) {
case Uint8Array:
case Uint16Array:
case Uint32Array:
case Int8Array:
case Int16Array:
case Int32Array:
case Float32Array:
case Float64Array:
let targetPropertyT = <TypedArray>target[k];
if (targetPropertyT == null || targetPropertyT.length !== (<TypedArray>sourceProperty).length) {
target[k] = (<TypedArray>sourceProperty).slice();
} else {
targetPropertyT.set(<TypedArray>sourceProperty);
}
break;
case Array:
let targetPropertyA = <Array<any>>target[k];
const length = (<Array<any>>sourceProperty).length;
if (targetPropertyA == null) {
target[k] = targetPropertyA = new Array<any>(length);
} else {
targetPropertyA.length = length;
}
for (let i = 0; i < length; i++) {
CloneManager.cloneProperty(
<Array<any>>sourceProperty,
targetPropertyA,
i,
cloneMode,
srcRoot,
targetRoot,
deepInstanceMap
);
}
break;
default:
let targetProperty = <Object>target[k];
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify remaining switch clauses that declare let/const without a block.
rg -nP 'case\s+[^\n]+:\n\s+(let|const)\s' packages/core/src/clone/CloneManager.ts
# Expected after fix: no matches

Repository: galacean/engine

Length of output: 41


🏁 Script executed:

sed -n '124,161p' packages/core/src/clone/CloneManager.ts

Repository: galacean/engine

Length of output: 1252


Wrap case clauses containing lexical declarations with block scope.

Multiple let and const declarations inside case clauses lack block scoping, violating ESLint's noSwitchDeclarations rule:

  • case Uint8Array: through case Float64Array: (line ~133: let targetPropertyT)
  • case Array: (lines ~141–142: let targetPropertyA, const length)
  • default: (line ~160: let targetProperty)

Each case containing let or const must be wrapped with {} to properly scope variables and avoid lint/CI failures.

🧰 Tools
🪛 Biome (2.4.7)

[error] 133-133: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)


[error] 141-141: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)


[error] 142-142: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)


[error] 161-161: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/clone/CloneManager.ts` around lines 124 - 161, The switch
in CloneManager.cloneProperty (handling typed arrays, Array, and default)
declares lexical variables (targetPropertyT, targetPropertyA, length,
targetProperty) directly in case clauses which violates noSwitchDeclarations;
fix by wrapping each case branch that declares let/const in its own block { ...
} so targetPropertyT, targetPropertyA, length and targetProperty are properly
block-scoped, leaving existing logic (slicing/setting for TypedArrays,
resizing/iterating for Array, and the default object handling) unchanged.

Comment on lines +140 to +159
case Array:
let targetPropertyA = <Array<any>>target[k];
const length = (<Array<any>>sourceProperty).length;
if (targetPropertyA == null) {
target[k] = targetPropertyA = new Array<any>(length);
} else {
targetPropertyA.length = length;
}
for (let i = 0; i < length; i++) {
CloneManager.cloneProperty(
<Array<any>>sourceProperty,
targetPropertyA,
i,
cloneMode,
srcRoot,
targetRoot,
deepInstanceMap
);
}
break;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Array cloning skips deepInstanceMap, breaking shared/cyclic graph handling.

case Array always allocates/reuses target[k] directly, but never reads/writes deepInstanceMap. This can duplicate shared arrays and recurse indefinitely on self-referential arrays.

Suggested fix
       case Array:
-        let targetPropertyA = <Array<any>>target[k];
-        const length = (<Array<any>>sourceProperty).length;
-        if (targetPropertyA == null) {
-          target[k] = targetPropertyA = new Array<any>(length);
-        } else {
-          targetPropertyA.length = length;
-        }
+        const sourceArray = <Array<any>>sourceProperty;
+        let targetPropertyA = <Array<any>>target[k];
+        const length = sourceArray.length;
+        if (targetPropertyA == null) {
+          targetPropertyA = <Array<any>>deepInstanceMap.get(sourceArray);
+          if (!targetPropertyA) {
+            targetPropertyA = new Array<any>(length);
+            deepInstanceMap.set(sourceArray, targetPropertyA);
+          }
+          target[k] = targetPropertyA;
+        } else {
+          deepInstanceMap.set(sourceArray, targetPropertyA);
+        }
+        targetPropertyA.length = length;
         for (let i = 0; i < length; i++) {
           CloneManager.cloneProperty(
-            <Array<any>>sourceProperty,
+            sourceArray,
             targetPropertyA,
             i,
             cloneMode,
📝 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
case Array:
let targetPropertyA = <Array<any>>target[k];
const length = (<Array<any>>sourceProperty).length;
if (targetPropertyA == null) {
target[k] = targetPropertyA = new Array<any>(length);
} else {
targetPropertyA.length = length;
}
for (let i = 0; i < length; i++) {
CloneManager.cloneProperty(
<Array<any>>sourceProperty,
targetPropertyA,
i,
cloneMode,
srcRoot,
targetRoot,
deepInstanceMap
);
}
break;
case Array:
const sourceArray = <Array<any>>sourceProperty;
let targetPropertyA = <Array<any>>target[k];
const length = sourceArray.length;
if (targetPropertyA == null) {
targetPropertyA = <Array<any>>deepInstanceMap.get(sourceArray);
if (!targetPropertyA) {
targetPropertyA = new Array<any>(length);
deepInstanceMap.set(sourceArray, targetPropertyA);
}
target[k] = targetPropertyA;
} else {
deepInstanceMap.set(sourceArray, targetPropertyA);
}
targetPropertyA.length = length;
for (let i = 0; i < length; i++) {
CloneManager.cloneProperty(
sourceArray,
targetPropertyA,
i,
cloneMode,
srcRoot,
targetRoot,
deepInstanceMap
);
}
break;
🧰 Tools
🪛 Biome (2.4.7)

[error] 141-141: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)


[error] 142-142: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/clone/CloneManager.ts` around lines 140 - 159, The Array
branch in CloneManager (the `case Array:` block) ignores the `deepInstanceMap`,
causing duplicated shared arrays and infinite recursion for self-referential
arrays; fix it by checking `deepInstanceMap` for an existing mapped target for
the `sourceProperty` before allocating or reusing `target[k]`, return or assign
that mapped instance if present, and when creating a new array instance assign
it into `deepInstanceMap` immediately (before iterating) so subsequent recursive
clones reuse it; ensure the rest of the loop continues to call
`CloneManager.cloneProperty` as before using the mapped/created
`targetPropertyA`.

@cptbtptpbcptdtptp cptbtptpbcptdtptp self-assigned this Mar 17, 2026
@cptbtptpbcptdtptp cptbtptpbcptdtptp added the ignore for release ignore for release label Mar 17, 2026
- Remove ineffective @ignoreClone on Entity/Component refs
- Fix remapComponent to use index instead of constructor

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/core/src/mesh/Skin.ts (1)

18-28: ⚠️ Potential issue | 🔴 Critical

_skinMatrices will be undefined on cloned Skin objects with non-empty bones.

When @deepClone processes the _bones array, it directly modifies target._bones without invoking the bones setter. Since the setter at line 62 is the only place _skinMatrices is initialized (this._skinMatrices = new Float32Array(boneCount * 16)), and _skinMatrices has @ignoreClone, cloned skins will have undefined for _skinMatrices despite having a populated _bones array.

This causes runtime crashes in _updateSkinMatrices() at lines 85, 87, 89 when accessing the undefined array, and in SkinnedMeshRenderer._update() at lines 198 and 204 when passing skin._skinMatrices to GPU operations.

Proposed fix: Change `_skinMatrices` to `@deepClone`

Since _skinMatrices is a calculation cache and its size must match bones.length * 16, deep cloning ensures the target has a correctly-sized buffer:

  /** `@internal` */
- `@ignoreClone`
+ `@deepClone`
  _skinMatrices: Float32Array;

Alternative: Add a _cloneTo method that initializes _skinMatrices based on _bones.length after cloning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/mesh/Skin.ts` around lines 18 - 28, The cloned Skin
instances end up with _skinMatrices undefined because `@deepClone` copies _bones
directly without invoking the bones setter which allocates _skinMatrices; fix by
making _skinMatrices deep-cloned (change its decorator from `@ignoreClone` to
`@deepClone`) so the cloned buffer matches bones.length*16, or alternatively
implement a _cloneTo(target: Skin) method invoked during cloning that
initializes target._skinMatrices = new Float32Array(target._bones.length * 16)
(ensure this runs before any calls to _updateSkinMatrices or
SkinnedMeshRenderer._update which access skin._skinMatrices).
packages/core/src/Renderer.ts (1)

352-357: ⚠️ Potential issue | 🟠 Major

Add transform entity listener rebinding in _cloneTo to restore listener registration after clone.

During component cloning, generic field cloning happens before _cloneTo and assigns _transformEntity via Entity._remap() without triggering _setTransformEntity(). This bypasses the listener registration in _updateFlagManager, leaving the cloned renderer unable to respond to transform changes.

Suggested fix location
  _cloneTo(target: Renderer): void {
    const materials = this._materials;
    for (let i = 0, n = materials.length; i < n; i++) {
      target._setMaterial(i, materials[i]);
    }
    // Rebind transform entity listeners after field cloning
    target._setTransformEntity(target._transformEntity);
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/Renderer.ts` around lines 352 - 357, _cloneTo currently
copies materials via _setMaterial but doesn't rebind transform listeners after
generic field cloning, so the cloned Renderer won't receive transform updates;
after the material loop in _cloneTo, call
target._setTransformEntity(target._transformEntity) to re-register listeners
created in _updateFlagManager (the _transformEntity was set via Entity._remap
previously), ensuring transform change handlers are restored on the clone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/clone/CloneUtils.ts`:
- Around line 17-23: In CloneUtils.remapComponent, avoid blindly indexing into
the target entity's _components array; compute const idx =
srcEntity._components.indexOf(component), call
CloneUtils._getEntityByHierarchyPath(targetRoot, path) to get the target entity,
then check if idx === -1 or targetEntity is falsy or targetEntity._components is
falsy or idx is out of bounds; if any of those are true return the original
component (or a safe fallback) instead of indexing, otherwise return
targetEntity._components[idx] as T. This uses existing helpers
_getEntityHierarchyPath, _getEntityByHierarchyPath, _tempRemapPath and the
_components arrays to locate and safely guard the mapped component.

---

Outside diff comments:
In `@packages/core/src/mesh/Skin.ts`:
- Around line 18-28: The cloned Skin instances end up with _skinMatrices
undefined because `@deepClone` copies _bones directly without invoking the bones
setter which allocates _skinMatrices; fix by making _skinMatrices deep-cloned
(change its decorator from `@ignoreClone` to `@deepClone`) so the cloned buffer
matches bones.length*16, or alternatively implement a _cloneTo(target: Skin)
method invoked during cloning that initializes target._skinMatrices = new
Float32Array(target._bones.length * 16) (ensure this runs before any calls to
_updateSkinMatrices or SkinnedMeshRenderer._update which access
skin._skinMatrices).

In `@packages/core/src/Renderer.ts`:
- Around line 352-357: _cloneTo currently copies materials via _setMaterial but
doesn't rebind transform listeners after generic field cloning, so the cloned
Renderer won't receive transform updates; after the material loop in _cloneTo,
call target._setTransformEntity(target._transformEntity) to re-register
listeners created in _updateFlagManager (the _transformEntity was set via
Entity._remap previously), ensuring transform change handlers are restored on
the clone.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6e433b54-7958-41f8-944d-85496867e67b

📥 Commits

Reviewing files that changed from the base of the PR and between 37eabf5 and 0fc880b.

📒 Files selected for processing (11)
  • packages/core/src/Component.ts
  • packages/core/src/Renderer.ts
  • packages/core/src/Transform.ts
  • packages/core/src/clone/CloneUtils.ts
  • packages/core/src/mesh/Skin.ts
  • packages/core/src/particle/ParticleGenerator.ts
  • packages/core/src/physics/shape/ColliderShape.ts
  • packages/ui/src/component/UICanvas.ts
  • packages/ui/src/component/UIGroup.ts
  • packages/ui/src/component/UIRenderer.ts
  • packages/ui/src/component/interactive/UIInteractive.ts
💤 Files with no reviewable changes (4)
  • packages/ui/src/component/UIGroup.ts
  • packages/core/src/physics/shape/ColliderShape.ts
  • packages/core/src/particle/ParticleGenerator.ts
  • packages/ui/src/component/UIRenderer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ui/src/component/UICanvas.ts

Comment on lines 17 to +23
static remapComponent<T extends Component>(srcRoot: Entity, targetRoot: Entity, component: T): T {
const paths = CloneUtils._tempRemapPath;
const success = CloneUtils._getEntityHierarchyPath(srcRoot, component.entity, paths);
return success
? (CloneUtils._getEntityByHierarchyPath(targetRoot, paths)?.getComponent(
<ComponentConstructor<T>>component.constructor
) as T)
: component;
const path = CloneUtils._tempRemapPath;
const srcEntity = component.entity;
if (!CloneUtils._getEntityHierarchyPath(srcRoot, srcEntity, path)) return component;
return CloneUtils._getEntityByHierarchyPath(targetRoot, path)._components[
srcEntity._components.indexOf(component)
] as T;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard missing component mapping before array index access.

Line 22 can be -1 (or map to an out-of-range slot), which makes Line 21 return undefined cast to T. That can break downstream clone/remap logic at runtime.

💡 Suggested fix
   static remapComponent<T extends Component>(srcRoot: Entity, targetRoot: Entity, component: T): T {
     const path = CloneUtils._tempRemapPath;
     const srcEntity = component.entity;
     if (!CloneUtils._getEntityHierarchyPath(srcRoot, srcEntity, path)) return component;
-    return CloneUtils._getEntityByHierarchyPath(targetRoot, path)._components[
-      srcEntity._components.indexOf(component)
-    ] as T;
+    const componentIndex = srcEntity._components.indexOf(component);
+    if (componentIndex < 0) return component;
+
+    const targetEntity = CloneUtils._getEntityByHierarchyPath(targetRoot, path);
+    return (targetEntity._components[componentIndex] ?? component) as T;
   }
📝 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
static remapComponent<T extends Component>(srcRoot: Entity, targetRoot: Entity, component: T): T {
const paths = CloneUtils._tempRemapPath;
const success = CloneUtils._getEntityHierarchyPath(srcRoot, component.entity, paths);
return success
? (CloneUtils._getEntityByHierarchyPath(targetRoot, paths)?.getComponent(
<ComponentConstructor<T>>component.constructor
) as T)
: component;
const path = CloneUtils._tempRemapPath;
const srcEntity = component.entity;
if (!CloneUtils._getEntityHierarchyPath(srcRoot, srcEntity, path)) return component;
return CloneUtils._getEntityByHierarchyPath(targetRoot, path)._components[
srcEntity._components.indexOf(component)
] as T;
static remapComponent<T extends Component>(srcRoot: Entity, targetRoot: Entity, component: T): T {
const path = CloneUtils._tempRemapPath;
const srcEntity = component.entity;
if (!CloneUtils._getEntityHierarchyPath(srcRoot, srcEntity, path)) return component;
const componentIndex = srcEntity._components.indexOf(component);
if (componentIndex < 0) return component;
const targetEntity = CloneUtils._getEntityByHierarchyPath(targetRoot, path);
return (targetEntity._components[componentIndex] ?? component) as T;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/clone/CloneUtils.ts` around lines 17 - 23, In
CloneUtils.remapComponent, avoid blindly indexing into the target entity's
_components array; compute const idx = srcEntity._components.indexOf(component),
call CloneUtils._getEntityByHierarchyPath(targetRoot, path) to get the target
entity, then check if idx === -1 or targetEntity is falsy or
targetEntity._components is falsy or idx is out of bounds; if any of those are
true return the original component (or a safe fallback) instead of indexing,
otherwise return targetEntity._components[idx] as T. This uses existing helpers
_getEntityHierarchyPath, _getEntityByHierarchyPath, _tempRemapPath and the
_components arrays to locate and safely guard the mapped component.

@cptbtptpbcptdtptp cptbtptpbcptdtptp merged commit 886dda6 into galacean:dev/2.0 Mar 18, 2026
11 of 12 checks passed
GuoLei1990 pushed a commit to hhhhkrx/runtime that referenced this pull request Mar 18, 2026
* fix: compontont props clone
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore for release ignore for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants