Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughA Most Improbable Endeavour: The Chicken Coop Roost Logic ImplementationThe Grand SchemeA tidy storm of changes lands in this PR: a grand new poultry mechanism blossoms—an extensive ChickenCoopEntity and the supporting block/registry plumbing—alongside a conscientious cull of stray imports across the codebase. The Roost: ChickenCoopEntity Emerges from the VoidA new block entity, the ChickenCoopEntity, has been introduced. It behaves like a small, slightly magical aviary with a schedule reminiscent of elvish watches and Vogon bureaucracy:
The Block Itself: ChickenCoopBlock AscendsChickenCoopBlock now implements EntityBlock and ties into the new entity:
Registry & Names
The Great Import PurgeAcross many client, datagen and workbench files several unused imports were removed (BlacksmithsWorkbenchMenu, ModTags, Blocks/Items imports, Constants, ArrayList, etc.)—a modest but welcome housekeeping sweep. Summary of Alterations
In brief: this PR brings a sophisticated, time‑aware chicken roost into MineTale—complete with nocturnal sequestration, dawnly release, egg accumulation, and the requisite registry and block‑entity ties—while tidying imports in passing. So long, and thanks for all the eggs. WalkthroughAdds a ChickenCoop block entity with server-side ticking and night/day chicken collection/release, egg tracking, particles/sounds and custom NBT (de)serialization; wires Block → EntityBlock and ticker; registers CHICKEN_COOP_BE and renames CHICKEN_COOP → CHICKEN_COOP_BLOCK; removes several unused imports. Changes
Sequence DiagramsequenceDiagram
participant Level
participant ChickenCoopBlock
participant ChickenCoopEntity
participant NearbyChickens
participant NBT
Level->>ChickenCoopBlock: server tick triggers
ChickenCoopBlock->>ChickenCoopEntity: delegate to ticker (tick)
rect rgba(100, 150, 200, 0.5)
Note over ChickenCoopEntity: Night — collection phase
ChickenCoopEntity->>NearbyChickens: scan radius (up to 6)
NearbyChickens-->>ChickenCoopEntity: matched chicken entities
ChickenCoopEntity->>NBT: serialize and store chicken data
ChickenCoopEntity->>NearbyChickens: remove original entities
end
rect rgba(200, 150, 100, 0.5)
Note over ChickenCoopEntity: Day — release phase
ChickenCoopEntity->>NBT: deserialize stored chicken data
ChickenCoopEntity->>Level: find safe grass spawn positions
ChickenCoopEntity->>Level: spawn chickens at locations
ChickenCoopEntity->>ChickenCoopEntity: clear stored data
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java (1)
49-49: Please ditchSystem.out.printlnin tick path.Use the mod logger with debug level instead. Console spam from game loops is pure goblin energy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` at line 49, Replace the System.out.println call in ChickenCoopEntity (the tick/update method where "Chicken Coop: Attempting to collect chickens!" is printed) with the mod's logger at debug level (e.g., call your mod logger's debug/info method) so the message uses the project's logging framework instead of printing to stdout; update the reference in ChickenCoopEntity (tick/update method) to use the logger instance (or static LOGGER) and remove the println to avoid console spam.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 315-333: The current ChickenCoopEntity input bridge methods list,
listOrEmpty, childrenList and childrenListOrEmpty always return empty
collections causing list fields to be dropped on load; replace these stubs with
implementations that read stored list data from the entity's underlying storage
(the same backing map/field used by get/put operations), decode entries using
the provided Codec<T> in list/listOrEmpty and return a TypedInputList wrapper
that streams/iterates over the decoded elements, and similarly return a
ValueInputList wrapper for childrenList/childrenListOrEmpty that exposes the
stored ValueInput items; ensure the wrappers reflect the actual stored size and
lazily decode entries (or decode once into an immutable list) so round-trip
serialization preserves list contents.
- Around line 35-37: Persist the in-memory state by serializing
storedChickensNbt and isNightMode to the BlockEntity NBT and restoring them on
load: implement/extend writeNbt(CompoundTag) to write a ListTag of CompoundTag
entries for storedChickensNbt and a boolean for isNightMode (call
super.writeNbt), and implement readNbt(CompoundTag) (or fromTag/toTag variants
used in this codebase) to rebuild storedChickensNbt from the ListTag and restore
isNightMode (call super.readNbt), then call markDirty() and sync the block
entity when these values change so they persist across chunk unloads and server
restarts.
In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Around line 374-376: The registry name was changed to "chicken_coop_block"
which will break existing worlds expecting "minetale:chicken_coop"; revert the
registry id in the register(...) call back to the original "chicken_coop" to
preserve compatibility (update the string literal in the CHICKEN_COOP_BLOCK
registration). Locate the register invocation that creates CHICKEN_COOP_BLOCK
(the register("chicken_coop_block", ChickenCoopBlock::new) call) and restore the
original id, and if you must introduce a new name, register it as an alias
rather than replacing the existing registry id.
---
Nitpick comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Line 49: Replace the System.out.println call in ChickenCoopEntity (the
tick/update method where "Chicken Coop: Attempting to collect chickens!" is
printed) with the mod's logger at debug level (e.g., call your mod logger's
debug/info method) so the message uses the project's logging framework instead
of printing to stdout; update the reference in ChickenCoopEntity (tick/update
method) to use the logger instance (or static LOGGER) and remove the println to
avoid console spam.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 350b7ae6-06cc-45c5-abe2-66fc5600b8ca
📒 Files selected for processing (12)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FurnitureWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.javasrc/client/java/com/tcm/MineTale/datagen/ModItemTagProvider.javasrc/client/java/com/tcm/MineTale/datagen/ModModelProvider.javasrc/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.javasrc/main/java/com/tcm/MineTale/block/ChickenCoopBlock.javasrc/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/FurnitureWorkbench.javasrc/main/java/com/tcm/MineTale/registry/ModBlockEntities.javasrc/main/java/com/tcm/MineTale/registry/ModBlocks.java
💤 Files with no reviewable changes (8)
- src/client/java/com/tcm/MineTale/datagen/ModItemTagProvider.java
- src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.java
- src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java
- src/client/java/com/tcm/MineTale/MineTaleClient.java
- src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java
- src/main/java/com/tcm/MineTale/block/workbenches/FurnitureWorkbench.java
- src/client/java/com/tcm/MineTale/block/workbenches/screen/FurnitureWorkbenchScreen.java
- src/client/java/com/tcm/MineTale/datagen/ModModelProvider.java
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java
Outdated
Show resolved
Hide resolved
Added egg count functionality to ChickenCoopEntity, allowing it to store and manage eggs laid by chickens during night mode.
Implement interaction for collecting eggs from the Chicken Coop block.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java (2)
35-37:⚠️ Potential issue | 🔴 CriticalPersist coop state, or the chickens sail into the void between saves.
storedChickensNbt,isNightMode, andeggCountare runtime-only right now; after unload/restart this state can reset. Add proper block-entity NBT save/load overrides so the coop survives dawn like a true Fellowship.Also applies to: 53-58, 132-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 35 - 37, ChickenCoopEntity currently keeps storedChickensNbt, isNightMode, and eggCount only in memory so their values are lost on unload; add NBT persistence by overriding the block-entity serialization methods in ChickenCoopEntity (override writeNbt/readNbt or toTag/fromTag depending on your mappings) to serialize storedChickensNbt as a ListTag of CompoundTag, write eggCount as an int, and isNightMode as a boolean, calling super.then writing/reading these keys (and markDirty()/requestModelData/update clients as needed after load); ensure you correctly reconstruct storedChickensNbt from the ListTag on read and call the matching keys consistently.
340-356:⚠️ Potential issue | 🟠 MajorThese list-input stubs are giga-scuffed: they always return empty.
list(...)/listOrEmpty(...)andchildrenList(...)/childrenListOrEmpty(...)currently discard list data on read, so round-trips can silently lose entity state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 340 - 356, The current ChickenCoopEntity implementations of list, listOrEmpty, childrenList and childrenListOrEmpty always return empty stubs and thus discard persisted list data on reads; replace these stubs so they return the actual stored lists from the entity's internal state (rather than Optional.empty()/empty implementations). Locate the backing storage used for keyed lists/children in ChickenCoopEntity (the field or accessor that holds lists of inputs/children) and make list(key, codec) return Optional.of(the typed list wrapped/decoded with the provided Codec), make listOrEmpty return that typed list or a thin wrapper delegating isEmpty()/stream()/iterator() to the real collection, and do the same for childrenList/childrenListOrEmpty returning ValueInputList wrappers that reflect the actual stored children; ensure you preserve generic typing and streaming/iteration semantics rather than always creating Stream.empty().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.java`:
- Around line 237-259: The code is referencing a non-existent
ChickenCoopBlockEntity and missing several Minecraft imports; change the type
check from ChickenCoopBlockEntity to the actual ChickenCoopEntity used in your
project (e.g., replace "if (level.getBlockEntity(brainPos) instanceof
ChickenCoopBlockEntity be)" with the correct entity class check and variable
name), and add the required imports for ItemInteractionResult, BlockHitResult,
SoundEvents, SoundSource, and Items from the Minecraft API so the method
useWithoutItem compiles; ensure you reference the exact class name
ChickenCoopEntity (or whatever the existing class is) and update any variable
names accordingly.
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Line 69: The file is missing imports for the sound enums used in the playSound
call; add the appropriate imports for SoundEvents and SoundSource at the top of
ChickenCoopEntity (so the reference to SoundEvents.CHICKEN_EGG and
SoundSource.BLOCKS in the playSound(...) call resolves), ensuring you import the
correct Minecraft classes (the ones that define SoundEvents and SoundSource) so
the file compiles.
---
Duplicate comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 35-37: ChickenCoopEntity currently keeps storedChickensNbt,
isNightMode, and eggCount only in memory so their values are lost on unload; add
NBT persistence by overriding the block-entity serialization methods in
ChickenCoopEntity (override writeNbt/readNbt or toTag/fromTag depending on your
mappings) to serialize storedChickensNbt as a ListTag of CompoundTag, write
eggCount as an int, and isNightMode as a boolean, calling super.then
writing/reading these keys (and markDirty()/requestModelData/update clients as
needed after load); ensure you correctly reconstruct storedChickensNbt from the
ListTag on read and call the matching keys consistently.
- Around line 340-356: The current ChickenCoopEntity implementations of list,
listOrEmpty, childrenList and childrenListOrEmpty always return empty stubs and
thus discard persisted list data on reads; replace these stubs so they return
the actual stored lists from the entity's internal state (rather than
Optional.empty()/empty implementations). Locate the backing storage used for
keyed lists/children in ChickenCoopEntity (the field or accessor that holds
lists of inputs/children) and make list(key, codec) return Optional.of(the typed
list wrapped/decoded with the provided Codec), make listOrEmpty return that
typed list or a thin wrapper delegating isEmpty()/stream()/iterator() to the
real collection, and do the same for childrenList/childrenListOrEmpty returning
ValueInputList wrappers that reflect the actual stored children; ensure you
preserve generic typing and streaming/iteration semantics rather than always
creating Stream.empty().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ab3aded-798e-4ef4-bddb-b50ae930182a
📒 Files selected for processing (2)
src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.javasrc/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java (2)
409-425:⚠️ Potential issue | 🔴 Critical
list()is decoding the box, not the contents.
tag.getList(key)gives you anOptional<ListTag>, andlistTag.stream()streams that Optional, not the entries inside theListTag. WithCompoundTag.CODEC,StoredChickensreloads empty, so collected birds vanish on load. Absolutely cursed.In Java, what does `Optional.stream()` return? For `Optional<ListTag>.stream()`, does it iterate the `ListTag` entries or emit the `ListTag` itself as a single element?🛠️ Minimal fix
`@Override` public <T> Optional<TypedInputList<T>> list(String key, Codec<T> codec) { - // Check if the key exists and is a List - if (!tag.contains(key)) return Optional.empty(); - - // In modern mappings, getList only takes the Key. - // It returns the list if found, or an empty one if not. - Optional<ListTag> listTag = tag.getList(key); - - // Map the NBT tags to objects using the codec - List<T> items = listTag.stream() - .map(nbt -> codec.parse(registries.createSerializationContext(NbtOps.INSTANCE), nbt) - .resultOrPartial(System.err::println)) - .flatMap(Optional::stream) // Flattens Optional<T> into the stream - .toList(); - - return Optional.of(new ChickenTypedInputList<>(items)); + return tag.getList(key).map(listTag -> { + List<T> items = listTag.stream() + .map(nbt -> codec.parse(registries.createSerializationContext(NbtOps.INSTANCE), nbt) + .resultOrPartial(System.err::println)) + .flatMap(Optional::stream) + .toList(); + + return new ChickenTypedInputList<>(items); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 409 - 425, The code is streaming the Optional<ListTag> itself instead of the ListTag's entries; change the logic in list(String key, Codec<T> codec) to first check listTag.isPresent(), then call listTag.get().stream() (or otherwise iterate the ListTag's elements) so you map each inner NBT element through codec.parse(...) rather than mapping the Optional; keep the same variables (tag, listTag, codec, registries, NbtOps.INSTANCE) and return Optional.of(new ChickenTypedInputList<>(items)) after collecting the mapped items.
434-448:⚠️ Potential issue | 🟠 MajorThese
*OrEmptyreaders are still returning the void.
listOrEmpty,childrenList, andchildrenListOrEmptyignore the backing tag and hard-code empty views. Any chicken fields that load through those paths will be dropped duringchicken.load(...), so round-tripping is still scuffed.🛠️ First step
`@Override` public <T> TypedInputList<T> listOrEmpty(String key, Codec<T> codec) { - return new TypedInputList<T>() { - `@Override` public boolean isEmpty() { return true; } - `@Override` public Stream<T> stream() { return Stream.empty(); } - `@Override` public java.util.Iterator<T> iterator() { return Stream.<T>empty().iterator(); } - }; + return list(key, codec).orElseGet(() -> new ChickenTypedInputList<>(List.of())); }
childrenList(...)andchildrenListOrEmpty(...)need the same treatment against the storedListTag, rather than unconditional empty wrappers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 434 - 448, The three *OrEmpty methods (listOrEmpty, childrenList, childrenListOrEmpty) currently return unconditional empty wrappers and must instead pull from the stored ListTag backing the entity; update listOrEmpty to look up the ListTag for the given key, return Optional.empty or an empty view only if the tag is missing, otherwise return a TypedInputList that streams/iterates over the ListTag entries decoding each element with the supplied Codec<T>; similarly implement childrenList/childrenListOrEmpty to locate the backing ListTag and return a ValueInputList that wraps each Tag element as a ValueInput (streaming/iterating over the ListTag), preserving the original list items so chicken.load(...) round-trips correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 109-139: The persistent state eggsLaidThisNight is not saved or
restored, causing it to reset after chunk unloads; in saveAdditional and
loadAdditional add symmetric persistence for eggsLaidThisNight: use
ValueOutput.putInt with the key (e.g. "EggsLaidThisNight") when saving inside
saveAdditional alongside eggCount/isNightMode, and read it back in
loadAdditional using ValueInput.getIntOr with a default of 0 so the field
this.eggsLaidThisNight is restored on reload; update the methods saveAdditional
and loadAdditional in ChickenCoopEntity accordingly.
---
Duplicate comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 409-425: The code is streaming the Optional<ListTag> itself
instead of the ListTag's entries; change the logic in list(String key, Codec<T>
codec) to first check listTag.isPresent(), then call listTag.get().stream() (or
otherwise iterate the ListTag's elements) so you map each inner NBT element
through codec.parse(...) rather than mapping the Optional; keep the same
variables (tag, listTag, codec, registries, NbtOps.INSTANCE) and return
Optional.of(new ChickenTypedInputList<>(items)) after collecting the mapped
items.
- Around line 434-448: The three *OrEmpty methods (listOrEmpty, childrenList,
childrenListOrEmpty) currently return unconditional empty wrappers and must
instead pull from the stored ListTag backing the entity; update listOrEmpty to
look up the ListTag for the given key, return Optional.empty or an empty view
only if the tag is missing, otherwise return a TypedInputList that
streams/iterates over the ListTag entries decoding each element with the
supplied Codec<T>; similarly implement childrenList/childrenListOrEmpty to
locate the backing ListTag and return a ValueInputList that wraps each Tag
element as a ValueInput (streaming/iterating over the ListTag), preserving the
original list items so chicken.load(...) round-trips correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b029822-0911-47be-866b-05b36a07c124
📒 Files selected for processing (5)
src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.javasrc/main/java/com/tcm/MineTale/block/ChickenCoopBlock.javasrc/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/registry/ModItems.java
💤 Files with no reviewable changes (2)
- src/main/java/com/tcm/MineTale/registry/ModItems.java
- src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @The-Code-Monkey. The following files were modified: * `src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.java` * `src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java (1)
40-45: The constant's Javadoc hath wandered astray, attaching itself to the constructor like a confused Took.The
MAX_EGGSconstant at line 45 has its documentation comment (/**) immediately followed by the constructor's Javadoc, creating a rather befuddling reading experience. This is a minor formatting oddity—not wrong, merely... aesthetically challenged.📝 Proposed formatting fix
private int eggCount = 0; private int eggsLaidThisNight = 0; + /** Maximum number of eggs the coop can store. */ private static final int MAX_EGGS = 16; - /** + + /** * Creates a ChickenCoopEntity for the given block position and block state. * * `@param` pos the block position for this block entity * `@param` state the block state associated with this block entity */ - public ChickenCoopEntity(BlockPos pos, BlockState state) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 40 - 45, The stray Javadoc opener is attached to the constructor instead of the constant; move the Javadoc (the /** ... */ block) so it directly precedes the MAX_EGGS declaration in the ChickenCoopEntity class (replace the current placement before the constructor with the comment immediately above the line declaring private static final int MAX_EGGS = 16) so the constant has its proper documentation and the ChickenCoopEntity() constructor Javadoc is separate.src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.java (1)
311-314: These.relative()incantations are giving "did absolutely nothing" energy.Behold, these two lines move the position by precisely zero blocks in any direction—a journey as pointless as a Vogon poetry reading, yet somehow less painful:
.relative(facing, 0) // already at z=0 .relative(facing.getClockWise(), 0); // x=1 is center, so we shift back to itThe subtraction on lines 311-312 already computes the origin correctly (which IS the brain position), rendering these subsequent calls utterly redundant. They're not wrong, mind you—just... existentially unnecessary.
🧹 Proposed simplification
// Calculate the origin (Bottom Front Center is our origin in calculateOffset logic) // Based on your calculateOffset, the BFC is at x=1, z=0, y=0. BlockPos brainPos = pos.subtract(calculateOffset(BlockPos.ZERO, facing, - currentPart.getXOffset(), currentPart.getZOffset(), currentPart.getYOffset())) - .relative(facing, 0) // already at z=0 - .relative(facing.getClockWise(), 0); // x=1 is center, so we shift back to it + currentPart.getXOffset(), currentPart.getZOffset(), currentPart.getYOffset()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.java` around lines 311 - 314, The two chained .relative(...) calls on the BlockPos brainPos are redundant because the preceding subtraction with calculateOffset already yields the correct origin; remove the .relative(facing, 0) and .relative(facing.getClockWise(), 0) invocations in the BlockPos brainPos assignment (the line that uses calculateOffset(BlockPos.ZERO, facing, currentPart.getXOffset(), currentPart.getZOffset(), currentPart.getYOffset())) so brainPos is assigned directly from the subtract(...) result, leaving facing, calculateOffset, and currentPart references intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 532-555: The Javadoc block above the add method in the
ChickenTypedOutputList record is not terminated, which causes the add(T value)
implementation to be treated as part of the comment and removes the required
override; close the Javadoc comment (add the missing "*/" before the `@Override`)
so the add method (which calls
codec.encodeStart(...).resultOrPartial(System.err::println).ifPresent(list::add))
is compiled as code and the class properly implements TypedOutputList.add;
ensure the Javadoc remains correctly formed and aligned with the add method
signature.
- Around line 757-769: In list(String key, Codec<T> codec) in ChickenCoopEntity
the code calls listTag.stream() where listTag is an Optional<ListTag>, so you're
streaming the Optional itself instead of the ListTag's elements; fix by
unwrapping the Optional (e.g., use tag.getList(key).map(listTag -> { ... }) or
flatMap) and then call stream() on the resulting ListTag to iterate its
elements, parse each element with
codec.parse(registries.createSerializationContext(NbtOps.INSTANCE),
nbt).resultOrPartial(...), collect the parsed T items into a List<T>, and return
Optional.of(new ChickenTypedInputList<>(items)); keep error handling identical
to childrenList() for reference.
---
Nitpick comments:
In `@src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.java`:
- Around line 311-314: The two chained .relative(...) calls on the BlockPos
brainPos are redundant because the preceding subtraction with calculateOffset
already yields the correct origin; remove the .relative(facing, 0) and
.relative(facing.getClockWise(), 0) invocations in the BlockPos brainPos
assignment (the line that uses calculateOffset(BlockPos.ZERO, facing,
currentPart.getXOffset(), currentPart.getZOffset(), currentPart.getYOffset()))
so brainPos is assigned directly from the subtract(...) result, leaving facing,
calculateOffset, and currentPart references intact.
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 40-45: The stray Javadoc opener is attached to the constructor
instead of the constant; move the Javadoc (the /** ... */ block) so it directly
precedes the MAX_EGGS declaration in the ChickenCoopEntity class (replace the
current placement before the constructor with the comment immediately above the
line declaring private static final int MAX_EGGS = 16) so the constant has its
proper documentation and the ChickenCoopEntity() constructor Javadoc is
separate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2141d444-454c-4881-91b8-b426f6653303
📒 Files selected for processing (2)
src/main/java/com/tcm/MineTale/block/ChickenCoopBlock.javasrc/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java (1)
758-769:⚠️ Potential issue | 🔴 CriticalUnwrap the
Optional<ListTag>before streaming, or the stored chickens still march straight into Mordor.Line 763 is streaming the
Optional, not theListTagcontents. That means the codec tries to parse the whole list as one element, decoding fails, and reloads still drop every stored chicken. This was flagged earlier and the bug is still here.⚒️ Minimal fix
`@Override` public <T> Optional<TypedInputList<T>> list(String key, Codec<T> codec) { if (!tag.contains(key)) return Optional.empty(); - Optional<ListTag> listTag = tag.getList(key); - List<T> items = listTag.stream() + Optional<ListTag> listTag = tag.getList(key); + if (listTag.isEmpty()) return Optional.empty(); + + List<T> items = listTag.get().stream() .map(nbt -> codec.parse(registries.createSerializationContext(NbtOps.INSTANCE), nbt) .resultOrPartial(System.err::println)) .flatMap(Optional::stream) .toList(); return Optional.of(new ChickenTypedInputList<>(items)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java` around lines 758 - 769, The bug is that list(String key, Codec<T> codec) is streaming the Optional<ListTag> instead of the ListTag contents; change the code to unwrap the Optional<ListTag> returned by tag.getList(key) (e.g., using listTag.isPresent()/listTag.map(...) or listTag.ifPresent/flatMap) and stream over the contained ListTag's elements (not the Optional), then parse each element with codec.parse(registries.createSerializationContext(NbtOps.INSTANCE), nbt).resultOrPartial(...), collect to items and return new ChickenTypedInputList<>(items); if the Optional is empty return Optional.empty(). Ensure you reference the existing methods tag.getList, codec.parse, registries.createSerializationContext, and ChickenTypedInputList when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 609-613: The child(...) method assumes the stored tag at key is a
CompoundTag and calls .get() directly, which can throw if the tag exists but is
the wrong type; change child(String key) to mirror childrenList() by verifying
the value is a compound before unwrapping—e.g., check
tag.getCompound(key).isPresent() (or check tag.get(key) is a CompoundTag) and
return Optional.empty() if not present/compound, otherwise construct the
ChickenValueInput with the compound and registries and return it; avoid calling
.get() on the Optional without that presence/type guard.
- Around line 386-390: The store and read methods (e.g., ChickenCoopEntity.store
and ChickenCoopEntity.read, and ChickenValueInput.read) are using
NbtOps.INSTANCE directly which loses registry context for registry-backed
Codecs; change the ops argument to use the registry-aware ops from your
registries by calling registries.createSerializationContext(NbtOps.INSTANCE) (or
the equivalent registry context provider used elsewhere) for both
codec.encodeStart(...) and codec.parse(...), ensuring you pass the same
registry-aware ops used by storeNullable/MapCodec/list paths so registry-backed
values serialize/deserialize consistently.
- Around line 259-265: The current code in ChickenCoopEntity calls
level.addFreshEntity(chicken) and then unconditionally clears storedChickensNbt;
change this to check the boolean result of addFreshEntity(chicken) and only
remove the corresponding NBT entry (and call spawnFeatherParticles) when
addFreshEntity returns true; if addFreshEntity returns false, keep the NBT entry
in storedChickensNbt (or move it to a retry queue) so the chicken data is
preserved, and call this.setChanged() after you mutate storedChickensNbt; look
for the addFreshEntity, storedChickensNbt, spawnFeatherParticles and setChanged
usages in ChickenCoopEntity to make the change.
---
Duplicate comments:
In `@src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.java`:
- Around line 758-769: The bug is that list(String key, Codec<T> codec) is
streaming the Optional<ListTag> instead of the ListTag contents; change the code
to unwrap the Optional<ListTag> returned by tag.getList(key) (e.g., using
listTag.isPresent()/listTag.map(...) or listTag.ifPresent/flatMap) and stream
over the contained ListTag's elements (not the Optional), then parse each
element with codec.parse(registries.createSerializationContext(NbtOps.INSTANCE),
nbt).resultOrPartial(...), collect to items and return new
ChickenTypedInputList<>(items); if the Optional is empty return
Optional.empty(). Ensure you reference the existing methods tag.getList,
codec.parse, registries.createSerializationContext, and ChickenTypedInputList
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 450c0df8-ea1d-499d-b924-1fe0c8c8b008
📒 Files selected for processing (1)
src/main/java/com/tcm/MineTale/block/entity/ChickenCoopEntity.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
1 participant
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.
No description provided.