Repository navigation
fix(loader): unknown components no longer spawn ghost nested entities (#808) - #908
Conversation
…#808) `spawnAndLinkNestedEntities` ran for every component entry, including ones that missed the registry and no-op'd in `applyComponent`. Entities nested in an unknown component's arrays were spawned, tracked as scene entities and linked via ChildrenComponent, with no field to patch their ids into. The nested-spawn walk now checks `ComponentApply.isKnownComponent` (engine built-ins + registry) first and skips an unknown component's value before creating anything. The RFC #560 B2 format gate still runs on the skipped items, and a new warn-once (`unknown-component-nested:<name>`) reports dropped entities for any key shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Caqyi5Snr6yKssztYFztzZ
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 101 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c59680635
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// entities have no component to patch their ids into and could | ||
| /// only ever be ghosts (#808). | ||
| pub fn isKnownComponent(name: []const u8) bool { | ||
| const builtins = [_][]const u8{ "Position", "Sprite", "Shape", "Tilemap", "Camera", "Image", "Emitter" }; |
There was a problem hiding this comment.
Use scene.builtins.names here instead of introducing another hand-maintained built-in list. scene/src/builtins.zig is explicitly the single source of truth and is designed so adding a built-in updates every dispatch site; with this duplicate, a future built-in can be implemented in applyComponent yet omitted here, causing its entity-bearing arrays to be classified as unknown and dropped by spawnAndLinkNestedEntities even though the component itself is applied.
Useful? React with 👍 / 👎.
…view) Codex P2: use the single built-in source of truth (#881) instead of a hand-maintained list, so a future built-in can't be applied yet treated as unknown by the nested-spawn gate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Caqyi5Snr6yKssztYFztzZ
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Fixes #808.
Problem
loadEntityInternal(and the prefab-spawn / nested recursion) calledspawnAndLinkNestedEntitiesfor every component entry, including components that miss the registry and no-op inapplyComponent(RFC #596 Axis 4). A typo such as"industry__Romm": { "workstations": [ {...} ] }spawned and tracked real scene entities and attached aChildrenComponentto the parent. No component field existed to receive their ids, so they were unreachable ghosts. The only signal was the generic unknown-component warn-once.Fix (design: nested spawning follows the component's fate)
ComponentApply.isKnownComponent(name)returns true for the engine built-insapplyComponentspecial-cases plus every name in theComponentsregistry.spawnAndLinkNestedEntitieschecks it first. For an unknown component it returns before creating any entity. That one choke point covers all three callers: scene entities, prefab spawn, and nested recursion.{prefab + children}) still runs on the skipped items, so malformed input remains a hardInvalidFormaterror. The existingunified_format_testcase depends on this.unknown-component-nested:<name>fires when entity-like items were dropped. It applies to any key shape, so dropped authored entities are always reported.Tests
test/jsonc/unknown_component_nested_test.zig(registered inbuild.zig):ChildrenComponent, no nested warning).componentsmap: scene-entity count stays 1, noDesk, noChildrenComponent. Bothunknown-component:andunknown-component-nested:fired.error.InvalidFormat.On the base commit, the 3 ghost-count tests fail on their entity-count assertion, which reproduces the bug. With the fix all pass. The other local
zig build testfailures on Windows (preview sockets, storage, and a few compile errors in unrelated tests) are the same set with and without this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01Caqyi5Snr6yKssztYFztzZ
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.