-
Notifications
You must be signed in to change notification settings - Fork 131
Add a whitelist tag for capturing and spawning mobs #1218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a whitelist tag for capturing and spawning mobs #1218
Conversation
WalkthroughAdds a spawner whitelist tag and localization entry, updates powered spawner to prefer the whitelist over the blacklist when blocking spawns, and simplifies soul-vial capture checks by removing boss-specific helpers and giving whitelist precedence. Changes
Sequence Diagram(s)sequenceDiagram
participant Spawner as PoweredSpawner
participant Tags as EIOTags/TagLookup
participant Decision as SpawnDecision
Spawner->>Tags: query SPAWNER_WHITELIST for EntityType
Tags-->>Spawner: yes / no
Spawner->>Tags: query SPAWNER_BLACKLIST for EntityType
Tags-->>Spawner: yes / no
Spawner->>Decision: evaluate (blacklist && !whitelist)
Decision-->>Spawner: allow spawn / block spawn (set DISABLED)
sequenceDiagram
participant Caller as CaptureCheck
participant Tags as EIOTags/TagLookup
participant Result as CapturableStatus
Caller->>Tags: is type in SOUL_VIAL_WHITELIST?
Tags-->>Caller: yes / no
alt whitelist
Caller->>Result: return CAPTURABLE
else not whitelist
Caller->>Tags: is type in SOUL_VIAL_BLACKLIST?
Tags-->>Caller: yes / no
alt blacklisted
Caller->>Result: return BLACKLISTED
else
Caller->>Result: return CAPTURABLE
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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 |
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 (1)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java (1)
41-41: Remove the unusedCapturableStatus.BOSSenum value and update its test.The
BOSSenum value at line 41 is no longer returned by thegetCapturableStatus()method. The current implementation checks theSOUL_VIAL_BLACKLISTtag and returnsBLACKLISTEDfor boss entities instead ofBOSS. Additionally, the testtestGetCapturableStatus_RejectBoss()inEntityCaptureUtilityTests.java:38will fail because it expectsBOSSto be returned forENDER_DRAGON, but the implementation will now returnBLACKLISTEDinstead.Remove the unused enum value and either remove or update the associated test.
🤖 Fix all issues with AI agents
In
@enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java:
- Around line 61-65: The whitelist check in EntityCaptureUtils currently returns
CapturableStatus.CAPTURABLE for types in EIOTags.EntityTypes.SOUL_VIAL_WHITELIST
before performing the serialization check; move or reorder the serialization
capability check to run first (i.e., verify the entity can be
serialized/deserialized using the existing serialization check logic) and only
then allow the whitelist to override other restrictions so that the method
returns CAPTURABLE for whitelisted types only if they pass serialization.
🧹 Nitpick comments (1)
enderio/src/datagen/java/com/enderio/enderio/datagen/common/tags/EIOEntityTagsProvider.java (1)
30-30: Consider multi-line formatting for consistency.The logic correctly mirrors the SOUL_VIAL_BLACKLIST pattern. However, for consistency with the multi-line chaining style used for SOUL_VIAL_BLACKLIST (lines 23-26), you might format this similarly.
♻️ Optional formatting adjustment
- this.tag(EIOTags.EntityTypes.SPAWNER_BLACKLIST).addTag(Tags.EntityTypes.BOSSES).add(EntityType.WARDEN); + this.tag(EIOTags.EntityTypes.SPAWNER_BLACKLIST) + .addTag(Tags.EntityTypes.BOSSES) + .add(EntityType.WARDEN);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
enderio/src/generated/resources/data/enderio/tags/entity_type/soul_vial_blacklist.jsonis excluded by!**/generated/**
📒 Files selected for processing (2)
enderio/src/datagen/java/com/enderio/enderio/datagen/common/tags/EIOEntityTagsProvider.javaenderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java
🧰 Additional context used
🧬 Code graph analysis (2)
enderio/src/datagen/java/com/enderio/enderio/datagen/common/tags/EIOEntityTagsProvider.java (1)
enderio/src/main/java/com/enderio/enderio/foundation/tag/EIOTags.java (2)
EntityTypes(191-202)EIOTags(21-203)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java (1)
enderio/src/main/java/com/enderio/enderio/foundation/tag/EIOTags.java (2)
EIOTags(21-203)EntityTypes(191-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (2)
enderio/src/datagen/java/com/enderio/enderio/datagen/common/tags/EIOEntityTagsProvider.java (2)
32-32: LGTM! Whitelist follows established pattern.The new SPAWNER_WHITELIST tag correctly mirrors the approach used for SOUL_VIAL_WHITELIST, providing an empty tag that users or modpacks can populate to override the blacklist. This completes the whitelist/blacklist pair for spawners as described in the PR objectives.
23-26: Logic is sound for blacklist-with-whitelist-override pattern.Adding the BOSSES tag to the blacklist aligns with the PR's goal of establishing a default deny-list that can be overridden by the whitelist (as implemented in EntityCaptureUtils). The explicit inclusion of WARDEN is defensive programming to ensure it's always blacklisted regardless of tag membership.
Tags.EntityTypes.BOSSES is a valid NeoForge tag, properly imported from net.neoforged.neoforge.common.Tags and consistently used across the codebase including production code (AttractorObeliskBlockEntity) and test code (EntityCaptureUtilityTests).
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java (1)
24-37: Dragon filtering at line 32 contradicts whitelist precedence.The ender dragon is unconditionally filtered out at line 32, which contradicts the documented behaviour in
getCapturableStatus()(lines 64–65) that states "Whitelist takes precedence over all". If the dragon is added toSOUL_VIAL_WHITELIST,getCapturableStatus()will returnCAPTURABLE(line 67), butgetCapturableEntityTypes()will still exclude it. Either the whitelist check should override dragon filtering, or the whitelist comment should be updated to clarify that the ender dragon is a special case.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.javaenderio/src/test/java/com/enderio/enderio/tests/util/EntityCaptureUtilityTests.java
💤 Files with no reviewable changes (1)
- enderio/src/test/java/com/enderio/enderio/tests/util/EntityCaptureUtilityTests.java
🧰 Additional context used
🧬 Code graph analysis (1)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java (1)
enderio/src/main/java/com/enderio/enderio/foundation/tag/EIOTags.java (2)
EIOTags(21-203)EntityTypes(191-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Run Tests
🔇 Additional comments (2)
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java (2)
39-53: Enum simplification is safe and approved.The streamlined
CapturableStatusenum now focuses on three clear states (CAPTURABLE, BLACKLISTED, INCOMPATIBLE), which improves code clarity. The removal of theBOSSenum value and related helpers does not break any existing references—no usage ofCapturableStatus.BOSSorisBlacklistedBossexists elsewhere in the codebase.
59-75: Whitelist precedence logic correctly implements the PR objective.The updated control flow properly prioritises the whitelist over all other checks, allowing previously restricted mobs (such as bosses) to be captured when explicitly whitelisted. The comments clearly explain this design decision.
The removal of the public helper methods
isBlacklistedBoss(Entity)andisBlacklistedBoss(EntityType<?>)is safe; no references to these methods exist elsewhere in the codebase.
getCapturableStatus needs updating to match the logic changes you've made. |
don't allow the ender dragon
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In
`@enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java`:
- Around line 64-73: The comment claims the whitelist "takes precedence over
all" but the code blacklists EntityType.ENDER_DRAGON before checking
EIOTags.EntityTypes.SOUL_VIAL_WHITELIST; to fix, move the Ender Dragon check
(the block returning CapturableStatus.BLACKLISTED for EntityType.ENDER_DRAGON)
so it runs after the whitelist check in EntityCaptureUtils, ensuring the
whitelist (EIOTags.EntityTypes.SOUL_VIAL_WHITELIST) can override the dragon
exception and keep the surrounding comment accurate.
enderio/src/main/java/com/enderio/enderio/foundation/util/EntityCaptureUtils.java
Show resolved
Hide resolved
Rover656
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
Description
Currently, the capture of mobs is still blocked by the Boss tag even if they are in the whitelist. Also added a whitelist for the spawner to match.
TODO
Breaking Changes
List any breaking changes in this section, such as: changed/removed APIs, changed or removed items/blocks or modifications to recipes and gameplay mechanics.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.