fix(world): skip prefab components with no registered metadata instead of NPEing - #5349
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughComponent-copying logic now detects missing component metadata, logs errors, and skips unavailable copies in block item creation, block type generation, and temporary entity cleanup. ChangesComponent copy guards
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
engine/src/main/java/org/terasology/engine/world/block/typeEntity/BlockTypeEntityGenerator.java (1)
76-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for missing metadata.
The stated
ManualLabor:ShapingTablereproduction remains pending. Add a test covering block-type generation that verifies the unavailable component is skipped and the entity still builds successfully.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/src/main/java/org/terasology/engine/world/block/typeEntity/BlockTypeEntityGenerator.java` around lines 76 - 86, Add regression coverage for BlockTypeEntityGenerator that uses the ManualLabor:ShapingTable scenario or equivalent missing ComponentMetadata setup. Verify the unavailable component is skipped while the block-type entity is generated successfully, preserving the existing builder behavior for available components.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@engine/src/main/java/org/terasology/engine/world/block/typeEntity/BlockTypeEntityGenerator.java`:
- Around line 76-86: Add regression coverage for BlockTypeEntityGenerator that
uses the ManualLabor:ShapingTable scenario or equivalent missing
ComponentMetadata setup. Verify the unavailable component is skipped while the
block-type entity is generated successfully, preserving the existing builder
behavior for available components.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 33db9f88-b675-4572-bf93-d1be6e107969
📒 Files selected for processing (3)
engine/src/main/java/org/terasology/engine/world/block/items/BlockItemFactory.javaengine/src/main/java/org/terasology/engine/world/block/typeEntity/BlockTypeEntityGenerator.javaengine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java
BenjaminAmos
approved these changes
Jul 29, 2026
3 tasks
…d of NPEing ComponentLibrary.copy() returns null for a component class with no registered metadata (e.g. an optional-dependency module not loaded); these three call sites passed that straight into addComponent() -> NPE. BlockPrefabManager.updateBlock() already guards this; applied the same skip-and-log pattern here. Found via a live crash: ManualLabor:ShapingTable referencing MachineDefinitionComponent with `machines` not loaded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
soloturn
force-pushed
the
soloturn-prefab
branch
from
July 30, 2026 20:09
9a6f71d to
465555f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
engine/src/main/java/org/terasology/engine/world/block/items/BlockItemFactory.java (1)
193-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the unloaded-module regression before merge.
The null guard is appropriate, but the stated
ManualLabor:ShapingTablereproduction withmachinesunloaded is still pending. Add a regression test that exercises this path and verifies block-item creation continues with the remaining components.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/src/main/java/org/terasology/engine/world/block/items/BlockItemFactory.java` around lines 193 - 200, Add a regression test for the BlockItemFactory component-copy path using the ManualLabor:ShapingTable scenario with the machines module unloaded. Verify creation does not fail when ComponentLibrary.copy returns null for an unavailable component and that the resulting block item still contains the remaining valid components.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@engine/src/main/java/org/terasology/engine/world/block/items/BlockItemFactory.java`:
- Around line 193-200: Add a regression test for the BlockItemFactory
component-copy path using the ManualLabor:ShapingTable scenario with the
machines module unloaded. Verify creation does not fail when
ComponentLibrary.copy returns null for an unavailable component and that the
resulting block item still contains the remaining valid components.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 46fd4fc4-21c9-4d7a-aaca-ecc20275d9b5
📒 Files selected for processing (3)
engine/src/main/java/org/terasology/engine/world/block/items/BlockItemFactory.javaengine/src/main/java/org/terasology/engine/world/block/typeEntity/BlockTypeEntityGenerator.javaengine/src/main/java/org/terasology/engine/world/internal/EntityAwareWorldProvider.java
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.
Summary
BlockPrefabManager.updateBlock()was already hardened against in fix: reset Logback once the real log folder is known + upgrade to 1.6.0 #5343; hardened them the same way.Test plan
:engine:compileJavasucceedsengine-testsregression (test+integrationTest): 1729 tests, 0 failures, 0 errorsmachines) — confirm it now logs and skips instead of crashingRelated
josariassurvival) with an optional-dependency module (machines) not loaded.