feat(loader): support path-based cross-prefab entity/component refs#2927
Merged
cptbtptpbcptdtptp merged 11 commits intogalacean:dev/2.0from Mar 18, 2026
Merged
feat(loader): support path-based cross-prefab entity/component refs#2927cptbtptpbcptdtptp merged 11 commits intogalacean:dev/2.0from
cptbtptpbcptdtptp merged 11 commits intogalacean:dev/2.0from
Conversation
WalkthroughReplaces ID-based entity/component references with path-based arrays, updates schema types and parser logic to resolve entities via numeric paths, moves rootIds population to a dedicated pass, and adds tests for cross-prefab component resolution and clone independence. Changes
Sequence DiagramsequenceDiagram
participant Parser as Parser (ReflectionParser)
participant Context as ParseContext
participant Entities as EntityStore
participant Loader as Loader
participant Component as ComponentInstance
Parser->>Context: receive IComponentRef(entityPath, componentType, componentIndex)
Parser->>Entities: _resolveEntityByPath(entityPath)
loop traverse indexes
Entities->>Entities: access child at index
end
Entities-->>Parser: targetEntity (or null)
Parser->>Loader: getClass(componentType)
Loader-->>Parser: ComponentClass (or null)
Parser->>targetEntity: getComponents(ComponentClass, [])
targetEntity-->>Parser: components[]
Parser->>Component: components[componentIndex] (or null)
Component-->>Parser: resolved component instance (or null)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
luzhuang
changed the title
luzhuang
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2927 +/- ##
===========================================
+ Coverage 77.09% 77.30% +0.20%
===========================================
Files 884 884
Lines 96585 96609 +24
Branches 9499 9505 +6
===========================================
+ Hits 74462 74681 +219
+ Misses 21959 21764 -195
Partials 164 164
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…mponent resolution
IComponentRef: { ownerId, componentId } → { entityPath, componentType, componentIndex }
IEntityRef: { entityId } → { entityPath }
entityPath navigates the resolved entity tree from the serialization root,
naturally crossing prefab instance boundaries without scope switching or
context isolation. Reverts v1 prefabInstanceContexts approach.
…tching avoid alias registration or late registration overwriting the reverse class name, which causes ref resolution to fail silently
luzhuang
changed the title
…lution replace manual _components iteration with entity.getComponents(type, [])
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/loader/src/resource-deserialize/resources/parser/ReflectionParser.ts (1)
212-218: Consider tightening ref type guards for malformed payloads.
_isEntityRef/_isComponentRefcurrently only gate onentityPatharray presence andcomponentTypepresence. ValidatingcomponentType/componentIndexshape would reduce false-positive matching in mixed object payloads.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loader/src/resource-deserialize/resources/parser/ReflectionParser.ts` around lines 212 - 218, The current guards in _isEntityRef and _isComponentRef are too loose and can misclassify malformed payloads; tighten them by validating shapes: in both _isEntityRef and _isComponentRef ensure value["entityPath"] is an Array and its entries are of an expected type (e.g., every entry is a string/number and the array is non-empty), and for _isComponentRef additionally assert that componentType is of the expected primitive type (e.g., string) and, if present, componentIndex is a finite number; for _isEntityRef require that componentType is strictly undefined and componentIndex is absent or invalid. Use the existing method names (_isEntityRef, _isComponentRef) to locate and replace the simple checks with these stronger validations.
🤖 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/loader/src/resource-deserialize/resources/parser/ReflectionParser.ts`:
- Around line 189-198: The _resolveEntityByPath function currently only checks
upper bounds and can return undefined; update it to validate that entityPath is
an array of finite non-negative integers and that each index is an integer >= 0
before using it. Specifically, in _resolveEntityByPath verify
Number.isInteger(entityPath[0]) && entityPath[0] >= 0 && entityPath[0] <
rootIds.length before getting rootIds[entityPath[0]]; inside the loop validate
Number.isInteger(entityPath[i]) && entityPath[i] >= 0 && entityPath[i] <
entity.children.length and that entity is non-null before indexing; always
return null on any malformed input instead of allowing undefined to slip
through.
In `@tests/src/loader/PrefabResource.test.ts`:
- Around line 78-81: Wrap the temporary registration into
engine.resourceManager._objectPool around the PrefabParser.parse call in a
try/finally: after assigning
engine.resourceManager._objectPool["dice.prefab"]=nestedPrefab, run assertions
inside the try block and in the finally block remove the key (delete
engine.resourceManager._objectPool["dice.prefab"]) to guarantee cleanup; apply
the same try/finally pattern to the other spots that set
engine.resourceManager._objectPool["dice.prefab"] (the later occurrences in this
test) so the pool is always cleaned even if assertions throw.
---
Nitpick comments:
In
`@packages/loader/src/resource-deserialize/resources/parser/ReflectionParser.ts`:
- Around line 212-218: The current guards in _isEntityRef and _isComponentRef
are too loose and can misclassify malformed payloads; tighten them by validating
shapes: in both _isEntityRef and _isComponentRef ensure value["entityPath"] is
an Array and its entries are of an expected type (e.g., every entry is a
string/number and the array is non-empty), and for _isComponentRef additionally
assert that componentType is of the expected primitive type (e.g., string) and,
if present, componentIndex is a finite number; for _isEntityRef require that
componentType is strictly undefined and componentIndex is absent or invalid. Use
the existing method names (_isEntityRef, _isComponentRef) to locate and replace
the simple checks with these stronger validations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 55891e78-5d00-48c8-a1df-abf25a59c091
📒 Files selected for processing (3)
packages/loader/src/resource-deserialize/resources/parser/ReflectionParser.tspackages/loader/src/resource-deserialize/resources/schema/BasicSchema.tstests/src/loader/PrefabResource.test.ts
Comment on lines
+189
to
+198
| private _resolveEntityByPath(entityPath: number[]): Entity | null { | ||
| const { rootIds, entityMap } = this._context; | ||
| if (!entityPath.length || entityPath[0] >= rootIds.length) return null; | ||
| let entity = entityMap.get(rootIds[entityPath[0]]); | ||
| for (let i = 1; i < entityPath.length; i++) { | ||
| if (!entity || entityPath[i] >= entity.children.length) return null; | ||
| entity = entity.children[entityPath[i]]; | ||
| } | ||
| return entity; | ||
| } |
There was a problem hiding this comment.
Harden _resolveEntityByPath index validation and null contract.
Line 191 only checks upper bounds. Negative/non-integer indices can slip through, and Line 198 can return undefined (not null) on malformed input.
Suggested fix
private _resolveEntityByPath(entityPath: number[]): Entity | null {
const { rootIds, entityMap } = this._context;
- if (!entityPath.length || entityPath[0] >= rootIds.length) return null;
- let entity = entityMap.get(rootIds[entityPath[0]]);
+ const isValidIndex = (index: number, length: number) =>
+ Number.isInteger(index) && index >= 0 && index < length;
+
+ if (!entityPath.length || !isValidIndex(entityPath[0], rootIds.length)) return null;
+
+ let entity = entityMap.get(rootIds[entityPath[0]]);
+ if (!entity) return null;
+
for (let i = 1; i < entityPath.length; i++) {
- if (!entity || entityPath[i] >= entity.children.length) return null;
+ if (!isValidIndex(entityPath[i], entity.children.length)) return null;
entity = entity.children[entityPath[i]];
+ if (!entity) return null;
}
- return entity;
+ return entity ?? null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/loader/src/resource-deserialize/resources/parser/ReflectionParser.ts`
around lines 189 - 198, The _resolveEntityByPath function currently only checks
upper bounds and can return undefined; update it to validate that entityPath is
an array of finite non-negative integers and that each index is an integer >= 0
before using it. Specifically, in _resolveEntityByPath verify
Number.isInteger(entityPath[0]) && entityPath[0] >= 0 && entityPath[0] <
rootIds.length before getting rootIds[entityPath[0]]; inside the loop validate
Number.isInteger(entityPath[i]) && entityPath[i] >= 0 && entityPath[i] <
entity.children.length and that entity is non-null before indexing; always
return null on any malformed input instead of allowing undefined to slip
through.
Comment on lines
+78
to
+81
| const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData); | ||
| // @ts-ignore — 注册到 resourceManager 使 getResourceByRef 可返回 | ||
| engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab; | ||
|
|
There was a problem hiding this comment.
Guard _objectPool cleanup with try/finally to avoid test pollution.
If an assertion throws before the tail cleanup lines, "dice.prefab" remains in _objectPool and can affect later tests.
Suggested pattern
const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData);
// `@ts-ignore`
engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab;
-
-const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData);
-const root = outerPrefab.instantiate();
-const script = root.getComponent(DiceScript);
-
-expect(script.skinMesh).not.toBeNull();
-...
-root.destroy();
-// `@ts-ignore`
-delete engine.resourceManager._objectPool["dice.prefab"];
+try {
+ const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData);
+ const root = outerPrefab.instantiate();
+ const script = root.getComponent(DiceScript);
+
+ expect(script.skinMesh).not.toBeNull();
+ ...
+ root.destroy();
+} finally {
+ // `@ts-ignore`
+ delete engine.resourceManager._objectPool["dice.prefab"];
+}Also applies to: 123-126, 139-142, 204-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/src/loader/PrefabResource.test.ts` around lines 78 - 81, Wrap the
temporary registration into engine.resourceManager._objectPool around the
PrefabParser.parse call in a try/finally: after assigning
engine.resourceManager._objectPool["dice.prefab"]=nestedPrefab, run assertions
inside the try block and in the finally block remove the key (delete
engine.resourceManager._objectPool["dice.prefab"]) to guarantee cleanup; apply
the same try/finally pattern to the other spots that set
engine.resourceManager._objectPool["dice.prefab"] (the later occurrences in this
test) so the pool is always cleaned even if assertions throw.
Collaborator
|
更新：重新检查当前 PR 后，撤回我之前这条评论。
这条评论保留为撤回说明，避免误导后续 review。 |
…mpletion order move rootIds construction from _parseEntity/_parsePrefab (where prefab async loading causes nondeterministic push order) to _parseEntities Promise.all callback, iterating entitiesConfig in original order
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/src/loader/PrefabResource.test.ts (1)
78-81:⚠️ Potential issue | 🟡 MinorGuard
_objectPoolregistration withtry/finallyto avoid leaked global test state.On Line 80 and Line 141, the temporary pool injection is not exception-safe. If an assertion throws before Line 125/Line 207,
"dice.prefab"remains registered and can pollute later tests.Proposed fix
const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData); // `@ts-ignore` — register to resourceManager so getResourceByRef can find it engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab; - -const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); -const root = outerPrefab.instantiate(); -const script = root.getComponent(DiceScript); - -expect(script.skinMesh).not.toBeNull(); -expect(script.skinMesh).toBeInstanceOf(MeshRenderer); -expect(script.skinMesh.entity.name).toBe("dice"); - -root.destroy(); -// `@ts-ignore` -delete engine.resourceManager._objectPool["dice.prefab"]; +try { + const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); + const root = outerPrefab.instantiate(); + const script = root.getComponent(DiceScript); + + expect(script.skinMesh).not.toBeNull(); + expect(script.skinMesh).toBeInstanceOf(MeshRenderer); + expect(script.skinMesh.entity.name).toBe("dice"); + + root.destroy(); +} finally { + // `@ts-ignore` + delete engine.resourceManager._objectPool["dice.prefab"]; +}const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData); // `@ts-ignore` engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab; - -const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); -const instance1 = outerPrefab.instantiate(); -const instance2 = outerPrefab.instantiate(); -... -instance1.destroy(); -instance2.destroy(); -// `@ts-ignore` -delete engine.resourceManager._objectPool["dice.prefab"]; +try { + const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); + const instance1 = outerPrefab.instantiate(); + const instance2 = outerPrefab.instantiate(); + ... + instance1.destroy(); + instance2.destroy(); +} finally { + // `@ts-ignore` + delete engine.resourceManager._objectPool["dice.prefab"]; +}Also applies to: 123-126, 139-142, 204-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/loader/PrefabResource.test.ts` around lines 78 - 81, The test temporarily injects nestedPrefab into engine.resourceManager._objectPool["dice.prefab"] without exception safety; wrap the registration and the subsequent assertions in a try/finally (or similar teardown) so you always remove the temporary entry from _objectPool, e.g. set engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab before assertions and in finally delete engine.resourceManager._objectPool["dice.prefab"]; apply the same try/finally pattern to the other occurrences around the PrefabParser.parse / nestedPrefab usage at the referenced spots so the pool is never left polluted if an assertion throws.
🤖 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/loader/PrefabResource.test.ts`:
- Around line 65-209: Add two regression tests to PrefabResource.test.ts: one
that simulates concurrent async loading of multiple prefab roots to assert
entityPath[0] resolves to the correct root regardless of load order (use
PrefabParser.parse to create two prefab resources, register them in
engine.resourceManager._objectPool, instantiate outer prefab that references
both roots via entityPath and assert resolved components on DiceScript), and a
second that verifies componentIndex/entityPath semantics when
removedEntities/removedComponents are present (create a prefab with
removedEntities/removedComponents entries, parse via PrefabParser.parse,
instantiate and assert that component resolution on DiceScript uses pre-removal
indexing semantics — i.e., indices refer to original pre-removed layout — and
that removed items are not returned as resolved targets); reference
PrefabParser.parse, DiceScript, entityPath, componentIndex, removedEntities, and
removedComponents when adding the assertions.
---
Duplicate comments:
In `@tests/src/loader/PrefabResource.test.ts`:
- Around line 78-81: The test temporarily injects nestedPrefab into
engine.resourceManager._objectPool["dice.prefab"] without exception safety; wrap
the registration and the subsequent assertions in a try/finally (or similar
teardown) so you always remove the temporary entry from _objectPool, e.g. set
engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab before
assertions and in finally delete
engine.resourceManager._objectPool["dice.prefab"]; apply the same try/finally
pattern to the other occurrences around the PrefabParser.parse / nestedPrefab
usage at the referenced spots so the pool is never left polluted if an assertion
throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4e6df58f-18ba-404a-bb75-eeafcadc63fb
📒 Files selected for processing (1)
tests/src/loader/PrefabResource.test.ts
Comment on lines
+65
to
+209
| describe("Cross-prefab IComponentRef (path-based)", () => { | ||
| it("should resolve component ref inside nested prefab instance via entityPath", async () => { | ||
| // 1. nested prefab (dice.prefab): single root entity with MeshRenderer | ||
| const nestedPrefabData: IHierarchyFile = { | ||
| entities: [ | ||
| { | ||
| id: "0", | ||
| name: "diceRoot", | ||
| components: [{ id: "mesh-comp", class: "MeshRenderer" }], | ||
| children: [] | ||
| } | ||
| ] | ||
| }; | ||
| const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData); | ||
| // @ts-ignore — register to resourceManager so getResourceByRef can find it | ||
| engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab; | ||
|
|
||
| // 2. outer prefab (DiceNode.prefab) | ||
| // resolved tree: DiceNode(root=[0]) -> [0]dice(nested) | ||
| // skinMesh crosses nested prefab boundary via entityPath | ||
| const outerPrefabData: IHierarchyFile = { | ||
| entities: [ | ||
| { | ||
| id: "root", | ||
| name: "DiceNode", | ||
| components: [ | ||
| { | ||
| id: "script-comp", | ||
| class: "DiceScript", | ||
| props: { | ||
| skinMesh: { entityPath: [0, 0], componentType: "MeshRenderer", componentIndex: 0 } | ||
| } | ||
| } | ||
| ], | ||
| children: ["dice-inst"] | ||
| }, | ||
| { | ||
| id: "dice-inst", | ||
| name: "dice", | ||
| parent: "root", | ||
| components: [], | ||
| assetUrl: "dice.prefab", | ||
| modifications: [], | ||
| removedEntities: [], | ||
| removedComponents: [] | ||
| } | ||
| ] | ||
| }; | ||
|
|
||
| const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); | ||
| const root = outerPrefab.instantiate(); | ||
| const script = root.getComponent(DiceScript); | ||
|
|
||
| // skinMesh should point to the MeshRenderer inside nested prefab instance | ||
| expect(script.skinMesh).not.toBeNull(); | ||
| expect(script.skinMesh).toBeInstanceOf(MeshRenderer); | ||
| expect(script.skinMesh.entity.name).toBe("dice"); | ||
|
|
||
| root.destroy(); | ||
| // @ts-ignore | ||
| delete engine.resourceManager._objectPool["dice.prefab"]; | ||
| }); | ||
|
|
||
| it("should resolve both local and cross-prefab refs, and survive clone independently", async () => { | ||
| const nestedPrefabData: IHierarchyFile = { | ||
| entities: [ | ||
| { | ||
| id: "0", | ||
| name: "diceRoot", | ||
| components: [{ id: "mesh-comp", class: "MeshRenderer" }], | ||
| children: [] | ||
| } | ||
| ] | ||
| }; | ||
| const nestedPrefab = await PrefabParser.parse(engine, "dice.prefab", nestedPrefabData); | ||
| // @ts-ignore | ||
| engine.resourceManager._objectPool["dice.prefab"] = nestedPrefab; | ||
|
|
||
| // resolved tree: DiceNode(root=[0]) -> [0]numCube, [1]dice(nested) | ||
| const outerPrefabData: IHierarchyFile = { | ||
| entities: [ | ||
| { | ||
| id: "root", | ||
| name: "DiceNode", | ||
| components: [ | ||
| { | ||
| id: "script-comp", | ||
| class: "DiceScript", | ||
| props: { | ||
| numMesh: { entityPath: [0, 0], componentType: "MeshRenderer", componentIndex: 0 }, | ||
| skinMesh: { entityPath: [0, 1], componentType: "MeshRenderer", componentIndex: 0 } | ||
| } | ||
| } | ||
| ], | ||
| children: ["num-cube", "dice-inst"] | ||
| }, | ||
| { | ||
| id: "num-cube", | ||
| name: "numCube", | ||
| parent: "root", | ||
| components: [{ id: "num-mesh-comp", class: "MeshRenderer" }] | ||
| }, | ||
| { | ||
| id: "dice-inst", | ||
| name: "dice", | ||
| parent: "root", | ||
| components: [], | ||
| assetUrl: "dice.prefab", | ||
| modifications: [], | ||
| removedEntities: [], | ||
| removedComponents: [] | ||
| } | ||
| ] | ||
| }; | ||
|
|
||
| const outerPrefab = await PrefabParser.parse(engine, "DiceNode.prefab", outerPrefabData); | ||
|
|
||
| const instance1 = outerPrefab.instantiate(); | ||
| const instance2 = outerPrefab.instantiate(); | ||
|
|
||
| const script1 = instance1.getComponent(DiceScript); | ||
| const script2 = instance2.getComponent(DiceScript); | ||
|
|
||
| // instance1: numMesh -> numCube's MeshRenderer, skinMesh -> dice's MeshRenderer | ||
| const numMesh1 = instance1.children[0].getComponent(MeshRenderer); | ||
| const skinMesh1 = instance1.children[1].getComponent(MeshRenderer); | ||
| expect(script1.numMesh).toBe(numMesh1); | ||
| expect(script1.skinMesh).toBe(skinMesh1); | ||
|
|
||
| // instance2: independent refs | ||
| const numMesh2 = instance2.children[0].getComponent(MeshRenderer); | ||
| const skinMesh2 = instance2.children[1].getComponent(MeshRenderer); | ||
| expect(script2.numMesh).toBe(numMesh2); | ||
| expect(script2.skinMesh).toBe(skinMesh2); | ||
|
|
||
| // refs across instances are independent | ||
| expect(script1.numMesh).not.toBe(script2.numMesh); | ||
| expect(script1.skinMesh).not.toBe(script2.skinMesh); | ||
|
|
||
| instance1.destroy(); | ||
| instance2.destroy(); | ||
| // @ts-ignore | ||
| delete engine.resourceManager._objectPool["dice.prefab"]; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add regression tests for the two known parser edge cases before merge.
This suite is good for nested/cross-prefab refs, but it still misses the two explicitly raised risks:
- top-level
entityPath[0]resolution when multiple prefab roots complete async in different orders, and componentIndex/entityPathbehavior whenremovedEntitiesorremovedComponentsare applied.
Please add tests that lock in intended semantics (especially pre-removal vs post-removal interpretation).
Suggested test skeleton
+it("should resolve top-level entityPath deterministically with multiple async prefab roots", async () => {
+ // Arrange: two top-level prefab instances with controllable async completion order.
+ // Assert: entityPath[0] binds by serialized order (or documented contract), not completion timing.
+});
+
+it("should define componentIndex/entityPath semantics with removedEntities/removedComponents", async () => {
+ // Arrange: target component/entity preceded by removable siblings.
+ // Assert: binding semantics stay consistent with documented behavior.
+});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/src/loader/PrefabResource.test.ts` around lines 65 - 209, Add two
regression tests to PrefabResource.test.ts: one that simulates concurrent async
loading of multiple prefab roots to assert entityPath[0] resolves to the correct
root regardless of load order (use PrefabParser.parse to create two prefab
resources, register them in engine.resourceManager._objectPool, instantiate
outer prefab that references both roots via entityPath and assert resolved
components on DiceScript), and a second that verifies componentIndex/entityPath
semantics when removedEntities/removedComponents are present (create a prefab
with removedEntities/removedComponents entries, parse via PrefabParser.parse,
instantiate and assert that component resolution on DiceScript uses pre-removal
indexing semantics — i.e., indices refer to original pre-removed layout — and
that removed items are not returned as resolved targets); reference
PrefabParser.parse, DiceScript, entityPath, componentIndex, removedEntities, and
removedComponents when adding the assertions.
GuoLei1990
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature / bug fix for loader prefab parsing.
What is the current behavior? (You can also link to an open issue here)
ReflectionParsercurrently resolves:IEntityRefvia{ entityId }lookup inParserContext.entityMapIComponentRefvia{ ownerId, componentId }lookup inParserContext.componentsThat model breaks down for nested prefab references because the actual target lives inside the instantiated entity tree rather than in a flat id map that naturally crosses prefab boundaries.
What is the new behavior (if this is a feature change)?
This PR switches entity/component refs in resource-deserialize schema from id-based lookup to path-based lookup:
IEntityRef: { entityId } -> { entityPath }IComponentRef: { ownerId, componentId } -> { entityPath, componentType, componentIndex }ReflectionParsernow:entityPathagainst the parsed entity treeIComponentRefby locating the target entity first, then selecting the Nth component of the requested registered typeLoader.getClass(componentType)plusinstanceofmatching instead of reverse class-name lookup, so alias/overwrite cases inLoader.registerClass()do not break ref resolutionThis lets refs naturally cross nested prefab boundaries without
prefabInstanceContexts, scope switching, or fallback chains.Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Yes, for serialized resource producers.
Assets that emit
IEntityRef/IComponentRefin loader resource data need to output the new path-based shape:IEntityRefmust emit{ entityPath }IComponentRefmust emit{ entityPath, componentType, componentIndex }Other information:
dev/2.0already contains the clone-side remap fix from #2926. This PR focuses on the remaining parser/schema side.Validation:
pnpm buildpnpm exec vitest run tests/src/loader/PrefabResource.test.ts --reporter=verboseSummary by CodeRabbit
Refactor
Tests