Add Most Farmers Workbench Recipies#54
Merged
The-Code-Monkey merged 5 commits into1.21from Feb 26, 2026
Merged
Conversation
Contributor
📝 WalkthroughIn the beginning — or on a curious afternoon when hobbits and hyperdrives conspired — this branch grows a small, verdant grove in MineTale: the Farmers Workbench and its kin have been planted, wired and given purpose. Release Notes — read aloud beside a campfire on the edge of the galaxy
Notes for reviewers — scrawled in Elvish beside the towel
Verdict — whispered by an ancient gardener with a radio towel
WalkthroughAdds Farmers and Builders workbenches: new blocks, block entities, menus, screens, recipe types/serializers, datagen recipes, many new blocks, and updates to the AbstractWorkbenchContainerMenu, client screen logic (selection memory, craft buttons) and server crafting dispatch to use a recipe-type-aware menu. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Screen as Workbench Screen
participant RecipeBook as ClientRecipeBook
participant Menu as AbstractWorkbenchContainerMenu
participant Network
participant Server as Server Craft Handler
participant BE as Workbench BlockEntity
Player->>Screen: select recipe
Screen->>RecipeBook: store lastKnownSelectedId
Screen->>RecipeBook: resolve RecipeDisplayEntry
Screen->>Screen: canCraft(player, entry, amount)
Screen->>Menu: query networkedNearbyItems
Player->>Screen: click craft button (amount)
Screen->>Network: send CraftRequestPayload(resultItem, amount)
Network->>Server: deliver payload
Server->>Menu: cast menu -> getRecipeType()
Server->>BE: locate recipe by type and validate/consume ingredients
Server->>Player: apply result / sync state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @The-Code-Monkey. The following files were modified: * `src/client/java/com/tcm/MineTale/MineTaleClient.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java` * `src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java` * `src/main/java/com/tcm/MineTale/MineTale.java` * `src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java` * `src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/registry/ModRecipes.java`
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/tcm/MineTale/registry/ModRecipes.java (1)
40-44:⚠️ Potential issue | 🟡 MinorOutdated Javadoc: missing ARMORERS_TYPE and FARMERS_TYPE.
The documentation mentions only furnace, campfire, and workbench recipe types but omits the newly added armorers and farmers types.
📝 Proposed fix
/** * Registers the mod's recipe types and their serializers into the game's built-in registries. * - * Specifically registers the furnace (FURNACE_T1_TYPE), campfire (CAMPFIRE_TYPE), - * and workbench (WORKBENCH_TYPE) recipe types with their corresponding serializers. + * Registers all workbench recipe types (furnace, campfire, workbench, armorers, farmers) + * with their corresponding serializers. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/registry/ModRecipes.java` around lines 40 - 44, The Javadoc in ModRecipes is out of date: it only lists FURNACE_T1_TYPE, CAMPFIRE_TYPE and WORKBENCH_TYPE but omits the newly added ARMORERS_TYPE and FARMERS_TYPE; update the class-level/method Javadoc to mention ARMORERS_TYPE and FARMERS_TYPE alongside the existing FURNACE_T1_TYPE, CAMPFIRE_TYPE and WORKBENCH_TYPE and note their corresponding serializers so the documentation accurately reflects the registered recipe types in ModRecipes.
🧹 Nitpick comments (10)
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java (2)
132-148: Consider using a proper logger instead ofSystem.out.println.The debug statements at lines 141 and 147 use
System.out.println. While this matches the existing pattern in the file (e.g.,hasIngredientAmount), consider refactoring to use a proper logging framework (e.g., SLF4J viaLoggerFactory) with appropriate log levels. This would allow easier filtering and disabling of debug output in production builds.♻️ Optional: Use a logger
Add a logger field at the class level:
private static final org.slf4j.Logger LOGGER = org.slf4j.LoggerFactory.getLogger(WorkbenchWorkbenchScreen.class);Then replace:
- System.out.println("Persistent Selection Success: " + results.get(0)); + LOGGER.debug("Persistent Selection Success: {}", results.get(0)); ... - System.out.println("Request failed: No recipe was ever selected!"); + LOGGER.warn("Request failed: No recipe was ever selected!");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` around lines 132 - 148, Replace the System.out.println debug prints in handleCraftRequest with a proper logger: add a private static final org.slf4j.Logger LOGGER = org.slf4j.LoggerFactory.getLogger(WorkbenchWorkbenchScreen.class) at the class level, then change the two System.out.println calls inside handleCraftRequest (the "Persistent Selection Success: " log and the "Request failed: No recipe was ever selected!" log) to appropriate LOGGER calls (e.g., LOGGER.debug for the success message including the ItemStack, and LOGGER.warn or LOGGER.debug for the failure). Keep all existing logic that reads lastKnownSelectedId, accesses ClientRecipeBook via ClientRecipeBookAccessor, and sends the CraftRequestPayload via ClientPlayNetworking unchanged.
169-184: Comment numbering inconsistency.The inline comments use "// 1.", "// 2.", "// 3." numbering, but line 184 repeats "// 2." when it should be "// 4." (Button Activation Logic). This is a minor readability nit.
📝 Fix comment numbering
- // 2. Button Activation Logic + // 4. Button Activation Logic🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` around lines 169 - 184, Update the misnumbered inline comment: change the duplicate "// 2. Button Activation Logic" to "// 4. Button Activation Logic" to reflect the correct sequence after "// 3." in the block surrounding this.mineTaleRecipeBook, lastKnownSelectedId and selectedEntry (references: RecipeDisplayId, lastKnownSelectedId, ClientRecipeBook/ClientRecipeBookAccessor, RecipeDisplayEntry).src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java (1)
24-24: Remove stray comment.The
// ChestBlockcomment appears to be a leftover note and should be removed.♻️ Proposed fix
-// ChestBlock - public class FarmersWorkbench extends AbstractWorkbench<FarmersWorkbenchEntity> {🤖 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/workbenches/FarmersWorkbench.java` at line 24, Remove the stray comment "// ChestBlock" from the FarmersWorkbench.java file; locate the comment near the top of the FarmersWorkbench class (class name FarmersWorkbench) and delete that orphaned line so the file no longer contains the leftover note.src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java (1)
74-468: Consider extracting a helper method to reduce boilerplate.All moss/grass/dirt recipes follow an identical pattern with only the input block, output block, and recipe name varying. While this is acceptable for data generation code, a helper method could improve maintainability.
♻️ Example helper method
private static void buildSimpleFarmerRecipe( RecipeOutput exporter, RecipeProvider provider, Object input, int inputCount, Block output, String name ) { var builder = new WorkbenchRecipeBuilder(ModRecipes.FARMERS_TYPE, ModRecipes.FARMERS_SERIALIZER); if (input instanceof Block b) { builder.input(b, inputCount); } else if (input instanceof TagKey<?> tag) { // handle tag input } builder.input(ModItems.PLANT_FIBER, 2) .output(output) .time(2) .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) .bookCategory(ModRecipeDisplay.FARMERS_SEARCH) .save(exporter, name); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java` around lines 74 - 468, The file contains many repeated WorkbenchRecipeBuilder calls in FarmerRecipes; extract a helper (e.g., buildSimpleFarmerRecipe) in the FarmerRecipes class that accepts parameters (exporter, provider, input (Block or TagKey/ItemTag with lookup), inputCount, output Block, recipeName) and internally constructs the WorkbenchRecipeBuilder, applies the common .input(ModItems.PLANT_FIBER, 2), .time(2), .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)), .bookCategory(ModRecipeDisplay.FARMERS_SEARCH) and .save(exporter, name), and update each duplicated recipe call sites to invoke this helper instead of repeating the builder code.src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java (1)
91-93: Remove debug logging before release.
System.out.printlndebug statements should be removed or replaced with proper logging (e.g., usingMineTale.LOGGER.debug(...)) for production code.♻️ Proposed fix
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - } + // Debug logging removed - use MineTale.LOGGER.debug() if needed for development🤖 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/workbenches/menu/ArmorersWorkbenchMenu.java` around lines 91 - 93, Remove the System.out.println debug line in ArmorersWorkbenchMenu (the block checking nearbyItems and playerInventory.player.level().isClientSide()) and either delete it or replace it with a proper logger call (e.g., MineTale.LOGGER.debug(...)) so production logging is used instead of stdout; keep the same conditional and message content when converting to MineTale.LOGGER.debug to preserve debug information about nearbyItems.size().src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java (2)
251-283: Remove debug logging before release.Multiple debug
System.out.printlncalls should be removed for production code.♻️ Proposed fix
private boolean hasIngredientAmount(Inventory inventory, Ingredient ingredient, int totalRequired) { - System.out.println("DEBUG: Searching inventory + nearby for " + totalRequired + "..."); if (totalRequired <= 0) return true; int found = 0; // 1. Check Player Inventory for (int i = 0; i < inventory.getContainerSize(); i++) { ItemStack stack = inventory.getItem(i); if (!stack.isEmpty() && ingredient.test(stack)) { found += stack.getCount(); } } // 2. CHECK THE NETWORKED ITEMS FROM CHESTS - // This is the list we sent via the packet! if (this.menu instanceof AbstractWorkbenchContainerMenu workbenchMenu) { for (ItemStack stack : workbenchMenu.getNetworkedNearbyItems()) { if (!stack.isEmpty() && ingredient.test(stack)) { found += stack.getCount(); - System.out.println("DEBUG: Found " + stack.getCount() + " in nearby networked list. Total: " + found); } } } - if (found >= totalRequired) { - System.out.println("DEBUG: Requirement MET with " + found + "/" + totalRequired); - return true; - } - - System.out.println("DEBUG: FAILED. Only found: " + found + "/" + totalRequired); - return false; + return found >= totalRequired; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 251 - 283, Remove the temporary debug System.out.println calls in the hasIngredientAmount method of FarmersWorkbenchScreen (the prints that start with "DEBUG:"). Keep the logic intact (iterating inventory, aggregating counts, iterating workbenchMenu.getNetworkedNearbyItems(), and returning based on found vs totalRequired); simply delete the println lines (or replace them with calls to the project's logger at an appropriate log level if you prefer structured logging) so no stdout debugging remains in production.
132-148: Remove debug logging before release.Multiple
System.out.printlnstatements should be removed or converted to proper logging for production code.♻️ Proposed fix
if (!results.isEmpty()) { - System.out.println("Persistent Selection Success: " + results.get(0)); ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)); return; } } } - System.out.println("Request failed: No recipe was ever selected!"); + // No recipe selected - buttons should be disabled, so this shouldn't normally be reached }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 132 - 148, In handleCraftRequest, remove the System.out.println debug calls (both the "Persistent Selection Success" and "Request failed" prints) and replace them with proper logging using the project's logger (e.g., use a static LOGGER at class scope and call LOGGER.debug(...) for the success path and LOGGER.warn(...) or LOGGER.info(...) for failure), keeping the behavior of ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)) and the null/empty checks around lastKnownSelectedId, ClientRecipeBookAccessor.getKnown(), and resultItems unchanged; add any necessary logger import/field (e.g., private static final Logger LOGGER) if not present.src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java (1)
35-36: Potential mismatch:SimpleContainerDatainitialized withEMPTY_SIZEinstead ofDATA_SIZE.The client-side constructor passes
new SimpleContainerData(EMPTY_SIZE)whereEMPTY_SIZE = 0. While both constants are currently 0, this appears semantically incorrect—SimpleContainerDatarepresents synced data slots, not container inventory size. IfDATA_SIZEchanges in the future, this would cause a mismatch.Suggested fix
public FarmersWorkbenchMenu(int syncId, Inventory playerInventory) { - this(syncId, playerInventory, new SimpleContainerData(EMPTY_SIZE), null); + this(syncId, playerInventory, new SimpleContainerData(DATA_SIZE), null); }🤖 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/workbenches/menu/FarmersWorkbenchMenu.java` around lines 35 - 36, The client-side FarmersWorkbenchMenu constructor is creating the synced data with new SimpleContainerData(EMPTY_SIZE) which semantically should use DATA_SIZE; update the call in the FarmersWorkbenchMenu(int syncId, Inventory playerInventory) constructor to use new SimpleContainerData(DATA_SIZE) instead of EMPTY_SIZE so the menu's synced data size matches the DATA_SIZE constant (verify the other FarmersWorkbenchMenu constructor that accepts a ContainerData param continues to accept and use that data).src/main/java/com/tcm/MineTale/registry/ModBlocks.java (2)
112-175: Consider using specialized block classes for decorative blocks with non-standard behaviors.Many of these new blocks (hanging moss, vines, ivy, rugs, short plants) are registered using the basic
Block::newconstructor. In vanilla Minecraft, similar blocks use specialized classes:
- Vines/Ivy →
VineBlockorMultifaceBlockfor wall-climbing behavior- Hanging plants → Could use properties like
HangingMossBlockor similar for ceiling attachment- Rugs/carpets → Typically use
CarpetBlockfor thin collision boxes- Short plants → Often use
BushBlockorTallGrassBlockfor plant-like behavior (breaking when block below removed, etc.)If these are placeholder registrations for future implementation, this is fine. Otherwise, they may not behave as expected in-game (e.g., vines won't climb walls, rugs will have full-block collision).
Would you like me to help identify which blocks should use which specialized classes, or is this intentional for now?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java` around lines 112 - 175, Several decorative blocks (e.g., VINE, LIANA, WALL_IVY, IVY, RED_HANGING_MOSS, BLUE_HANGING_MOSS, GREEN_MOSS_RUG, BLUE_MOSS_RUG, SHORT_MOSS, SHORT_RED_MOSS, etc.) are registered with Block::new but need specialized block classes to exhibit correct vanilla behaviors; update the register calls in ModBlocks to use appropriate subclasses (e.g., VineBlock or MultifaceBlock for VINE/IVY/WALL_IVY, a hanging/ceiling-attached block class for RED_HANGING_MOSS/BLUE_HANGING_MOSS, CarpetBlock for *_RUG entries, and BushBlock/TallPlantBlock/BushBlock-derived types for SHORT_* plants) and adjust their BlockBehaviour.Properties as required, keeping the same register(...) invocation pattern so other registration plumbing remains unchanged.
79-84: Consider adding.noOcclusion()for this wide workbench block.Looking at
FarmersWorkbench.java, this block is configured as a wide block (IS_WIDE = true). TheFURNACE_WORKBENCH_BLOCK_T1/T2blocks include.noOcclusion()in their properties to ensure proper rendering. Without this, adjacent blocks may incorrectly cull faces.Suggested fix
public static final Block FARMERS_WORKBENCH_BLOCK = register( "farmers_workbench", FarmersWorkbench::new, - BlockBehaviour.Properties.of().sound(SoundType.WOOD), + BlockBehaviour.Properties.of().sound(SoundType.WOOD).noOcclusion(), true );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java` around lines 79 - 84, The block FARMERS_WORKBENCH_BLOCK is a wide block (see FarmersWorkbench and its IS_WIDE = true) but its BlockBehaviour.Properties lacks .noOcclusion(), which can cause face-culling issues; update the register call for "farmers_workbench" (the FARMERS_WORKBENCH_BLOCK constant) to include .noOcclusion() in the BlockBehaviour.Properties chain (matching how FURNACE_WORKBENCH_BLOCK_T1/T2 are configured) so the wide model renders 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/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java`:
- Line 36: Remove the stale TODO comment "// TODO: FarmersWorkbench Not
Implemented" in WorkbenchRecipes (around the FarmersWorkbench recipe block)
since the FarmersWorkbench recipe is already implemented and saved in the
subsequent lines (the FarmersWorkbench recipe creation and save calls); simply
delete the misleading comment so the code/comments reflect the current
implementation.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java`:
- Around line 91-93: Remove the debug System.out.println in FarmersWorkbenchMenu
where it logs nearbyItems size (inside the if checking nearbyItems.isEmpty() and
playerInventory.player.level().isClientSide()); either delete the statement
entirely or replace it with the project's logger at an appropriate level (e.g.,
LOGGER.debug or LOGGER.trace) so production logs aren’t polluted—ensure you
reference the class FarmersWorkbenchMenu and the nearbyItems check when making
the change.
In `@src/main/java/com/tcm/MineTale/MineTale.java`:
- Line 22: Remove the unused import of FarmersWorkbenchMenu from MineTale.java:
since the file now uses AbstractWorkbenchContainerMenu for all instanceof checks
and casting, delete the import statement for
com.tcm.MineTale.block.workbenches.menu.FarmersWorkbenchMenu to clean up unused
imports and avoid warnings.
---
Outside diff comments:
In `@src/main/java/com/tcm/MineTale/registry/ModRecipes.java`:
- Around line 40-44: The Javadoc in ModRecipes is out of date: it only lists
FURNACE_T1_TYPE, CAMPFIRE_TYPE and WORKBENCH_TYPE but omits the newly added
ARMORERS_TYPE and FARMERS_TYPE; update the class-level/method Javadoc to mention
ARMORERS_TYPE and FARMERS_TYPE alongside the existing FURNACE_T1_TYPE,
CAMPFIRE_TYPE and WORKBENCH_TYPE and note their corresponding serializers so the
documentation accurately reflects the registered recipe types in ModRecipes.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 251-283: Remove the temporary debug System.out.println calls in
the hasIngredientAmount method of FarmersWorkbenchScreen (the prints that start
with "DEBUG:"). Keep the logic intact (iterating inventory, aggregating counts,
iterating workbenchMenu.getNetworkedNearbyItems(), and returning based on found
vs totalRequired); simply delete the println lines (or replace them with calls
to the project's logger at an appropriate log level if you prefer structured
logging) so no stdout debugging remains in production.
- Around line 132-148: In handleCraftRequest, remove the System.out.println
debug calls (both the "Persistent Selection Success" and "Request failed"
prints) and replace them with proper logging using the project's logger (e.g.,
use a static LOGGER at class scope and call LOGGER.debug(...) for the success
path and LOGGER.warn(...) or LOGGER.info(...) for failure), keeping the behavior
of ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount))
and the null/empty checks around lastKnownSelectedId,
ClientRecipeBookAccessor.getKnown(), and resultItems unchanged; add any
necessary logger import/field (e.g., private static final Logger LOGGER) if not
present.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Around line 132-148: Replace the System.out.println debug prints in
handleCraftRequest with a proper logger: add a private static final
org.slf4j.Logger LOGGER =
org.slf4j.LoggerFactory.getLogger(WorkbenchWorkbenchScreen.class) at the class
level, then change the two System.out.println calls inside handleCraftRequest
(the "Persistent Selection Success: " log and the "Request failed: No recipe was
ever selected!" log) to appropriate LOGGER calls (e.g., LOGGER.debug for the
success message including the ItemStack, and LOGGER.warn or LOGGER.debug for the
failure). Keep all existing logic that reads lastKnownSelectedId, accesses
ClientRecipeBook via ClientRecipeBookAccessor, and sends the CraftRequestPayload
via ClientPlayNetworking unchanged.
- Around line 169-184: Update the misnumbered inline comment: change the
duplicate "// 2. Button Activation Logic" to "// 4. Button Activation Logic" to
reflect the correct sequence after "// 3." in the block surrounding
this.mineTaleRecipeBook, lastKnownSelectedId and selectedEntry (references:
RecipeDisplayId, lastKnownSelectedId, ClientRecipeBook/ClientRecipeBookAccessor,
RecipeDisplayEntry).
In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java`:
- Around line 74-468: The file contains many repeated WorkbenchRecipeBuilder
calls in FarmerRecipes; extract a helper (e.g., buildSimpleFarmerRecipe) in the
FarmerRecipes class that accepts parameters (exporter, provider, input (Block or
TagKey/ItemTag with lookup), inputCount, output Block, recipeName) and
internally constructs the WorkbenchRecipeBuilder, applies the common
.input(ModItems.PLANT_FIBER, 2), .time(2), .unlockedBy("has_farmers_workbench",
provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)),
.bookCategory(ModRecipeDisplay.FARMERS_SEARCH) and .save(exporter, name), and
update each duplicated recipe call sites to invoke this helper instead of
repeating the builder code.
In `@src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java`:
- Line 24: Remove the stray comment "// ChestBlock" from the
FarmersWorkbench.java file; locate the comment near the top of the
FarmersWorkbench class (class name FarmersWorkbench) and delete that orphaned
line so the file no longer contains the leftover note.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java`:
- Around line 91-93: Remove the System.out.println debug line in
ArmorersWorkbenchMenu (the block checking nearbyItems and
playerInventory.player.level().isClientSide()) and either delete it or replace
it with a proper logger call (e.g., MineTale.LOGGER.debug(...)) so production
logging is used instead of stdout; keep the same conditional and message content
when converting to MineTale.LOGGER.debug to preserve debug information about
nearbyItems.size().
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java`:
- Around line 35-36: The client-side FarmersWorkbenchMenu constructor is
creating the synced data with new SimpleContainerData(EMPTY_SIZE) which
semantically should use DATA_SIZE; update the call in the
FarmersWorkbenchMenu(int syncId, Inventory playerInventory) constructor to use
new SimpleContainerData(DATA_SIZE) instead of EMPTY_SIZE so the menu's synced
data size matches the DATA_SIZE constant (verify the other FarmersWorkbenchMenu
constructor that accepts a ContainerData param continues to accept and use that
data).
In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Around line 112-175: Several decorative blocks (e.g., VINE, LIANA, WALL_IVY,
IVY, RED_HANGING_MOSS, BLUE_HANGING_MOSS, GREEN_MOSS_RUG, BLUE_MOSS_RUG,
SHORT_MOSS, SHORT_RED_MOSS, etc.) are registered with Block::new but need
specialized block classes to exhibit correct vanilla behaviors; update the
register calls in ModBlocks to use appropriate subclasses (e.g., VineBlock or
MultifaceBlock for VINE/IVY/WALL_IVY, a hanging/ceiling-attached block class for
RED_HANGING_MOSS/BLUE_HANGING_MOSS, CarpetBlock for *_RUG entries, and
BushBlock/TallPlantBlock/BushBlock-derived types for SHORT_* plants) and adjust
their BlockBehaviour.Properties as required, keeping the same register(...)
invocation pattern so other registration plumbing remains unchanged.
- Around line 79-84: The block FARMERS_WORKBENCH_BLOCK is a wide block (see
FarmersWorkbench and its IS_WIDE = true) but its BlockBehaviour.Properties lacks
.noOcclusion(), which can cause face-culling issues; update the register call
for "farmers_workbench" (the FARMERS_WORKBENCH_BLOCK constant) to include
.noOcclusion() in the BlockBehaviour.Properties chain (matching how
FURNACE_WORKBENCH_BLOCK_T1/T2 are configured) so the wide model renders
correctly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.javasrc/main/java/com/tcm/MineTale/MineTale.javasrc/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/registry/ModBlockEntities.javasrc/main/java/com/tcm/MineTale/registry/ModBlocks.javasrc/main/java/com/tcm/MineTale/registry/ModItems.javasrc/main/java/com/tcm/MineTale/registry/ModMenuTypes.javasrc/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.javasrc/main/java/com/tcm/MineTale/registry/ModRecipes.java
💤 Files with no reviewable changes (2)
- src/main/java/com/tcm/MineTale/registry/ModItems.java
- src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java
src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java
Outdated
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java (1)
47-55:⚠️ Potential issue | 🟡 MinorRemove stale TODO above the implemented Farmers recipe.
Line 47 says FarmersWorkbench is not implemented, but Lines 48-55 already define and save that recipe. This is now misleading.
✏️ Suggested cleanup
- // TODO: FarmersWorkbench Not Implemented🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java` around lines 47 - 55, Remove the stale TODO comment "// TODO: FarmersWorkbench Not Implemented" that sits immediately above the implemented Farmers recipe in WorkbenchRecipes; keep the existing WorkbenchRecipeBuilder block (the calls using ModRecipes.WORKBENCH_TYPE, ModRecipes.WORKBENCH_SERIALIZER, .input(...), .output(ModBlocks.FARMERS_WORKBENCH_BLOCK), .time(3), .unlockedBy(...), .bookCategory(...), .save(...)) unchanged so the recipe remains intact and the misleading TODO is eliminated.src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java (1)
98-100:⚠️ Potential issue | 🟡 MinorRemove the debug console print before merge.
Line 99 still writes directly to stdout during client-side recipe-book updates; this should be removed or replaced with the project logger at debug/trace level.
Suggested fix
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - }🤖 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/workbenches/menu/FarmersWorkbenchMenu.java` around lines 98 - 100, Remove the stray stdout debug print in FarmersWorkbenchMenu (the System.out.println call that logs nearbyItems.size() during client-side recipe-book updates); either delete the line or replace it with the project's logger at debug/trace level (use the existing logger instance or add one consistent with surrounding classes) so client-side code no longer writes to System.out but uses structured logging instead.
🧹 Nitpick comments (5)
src/main/java/com/tcm/MineTale/MineTale.java (1)
21-21: Consider removing unusedArmorersWorkbenchMenuimport.With the refactor to use
AbstractWorkbenchContainerMenufor all menu handling, theArmorersWorkbenchMenuimport on line 21 also appears unused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/MineTale.java` at line 21, Remove the unused import of ArmorersWorkbenchMenu: locate the import statement referencing ArmorersWorkbenchMenu and delete it since the codebase now uses AbstractWorkbenchContainerMenu for menu handling; ensure no other references to ArmorersWorkbenchMenu remain (search for the class name) before committing.src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java (1)
63-70: Consider adding@Nullableto the return type for consistency.The
recipeTypefield is annotated@Nullable(line 41), butgetRecipeType()doesn't reflect this in its return type. While current concrete implementations always pass non-null types, the annotation mismatch could mislead callers.♻️ Suggested annotation
- public RecipeType<WorkbenchRecipe> getRecipeType() { + public `@Nullable` RecipeType<WorkbenchRecipe> getRecipeType() { return this.recipeType; }🤖 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/workbenches/menu/AbstractWorkbenchContainerMenu.java` around lines 63 - 70, The getter getRecipeType() should reflect that its backing field recipeType is nullable by updating the method signature to return a nullable RecipeType<WorkbenchRecipe> (annotate with `@Nullable`) so callers and static analysis see the contract; locate the getRecipeType() method and add the `@Nullable` annotation to its return type (matching the `@Nullable` on the recipeType field) and ensure imports for the `@Nullable` annotation are present.src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java (1)
99-101: Remove or replace debug logging with proper logger.
System.out.printlndebug statements in production code can cause log pollution. Consider using the mod's logger (MineTale.LOGGER.debug(...)) or removing the statement entirely.♻️ Suggested fix
if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); + // MineTale.LOGGER.debug("Recipe Book accounting for {} stacks from packet", nearbyItems.size()); }🤖 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/workbenches/menu/ArmorersWorkbenchMenu.java` around lines 99 - 101, The code in ArmorersWorkbenchMenu uses System.out.println for debug output when nearbyItems is non-empty on the client; replace this with the mod logger call to avoid console pollution—use MineTale.LOGGER.debug(...) (or remove the statement) and include the same contextual message (e.g., nearbyItems.size()) so the check around nearbyItems and playerInventory.player.level().isClientSide() remains unchanged but the println is swapped for MineTale.LOGGER.debug or removed entirely.src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java (1)
85-272: Consider extracting a helper for repetitive moss recipes.The moss-variant recipes (lines 85-272) share identical structure: 4 moss input + 2 plant fiber → output, time 2, same unlock and category. While functional, a helper method could reduce duplication.
♻️ Optional helper pattern
private static void mossRecipe(RecipeProvider provider, RecipeOutput exporter, Block inputMoss, Block output, String name) { new WorkbenchRecipeBuilder(ModRecipes.FARMERS_TYPE, ModRecipes.FARMERS_SERIALIZER) .input(inputMoss, 4) .input(ModItems.PLANT_FIBER, 2) .output(output) .time(2) .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) .bookCategory(ModRecipeDisplay.FARMERS_SEARCH) .save(exporter, name); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java` around lines 85 - 272, Many moss recipes repeat the same WorkbenchRecipeBuilder pattern; extract a helper (e.g., mossRecipe) that accepts the RecipeProvider (provider), RecipeOutput/exporter, input Block, output Block and recipe name and inside creates the WorkbenchRecipeBuilder with .input(inputMoss, 4), .input(ModItems.PLANT_FIBER, 2), .output(output), .time(2), .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) and .bookCategory(ModRecipeDisplay.FARMERS_SEARCH) then .save(exporter, name); replace each repeated WorkbenchRecipeBuilder block with a call to mossRecipe(provider, exporter, ModBlocks.*_MOSS, ModBlocks.*_OUTPUT, "name") to remove duplication while keeping the same behavior.src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java (1)
139-145: Replace debug logging with proper logger or remove.Multiple
System.out.printlndebug statements here and throughout the class (lines 250, 269, 275, 279) should use the mod's logger for consistency and to allow log-level filtering.♻️ Suggested approach
- System.out.println("Persistent Selection Success: " + results.get(0)); + MineTale.LOGGER.debug("Persistent Selection Success: {}", results.get(0)); ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)); return; } } } - System.out.println("Request failed: No recipe was ever selected!"); + MineTale.LOGGER.debug("Request failed: No recipe was ever selected!");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java` around lines 139 - 145, Replace the System.out.println debug prints in ArmorersWorkbenchScreen (the ones around the persistent selection handling that log "Persistent Selection Success" and "Request failed: No recipe was ever selected!" as well as any other System.out.println occurrences in this class) with the mod's logger calls (e.g., MineTale.LOGGER or your project's LOGGER) using an appropriate level (info/debug/warn) and include contextual data such as results.get(0) and amount in the message; ensure you update all prints in this class (including the other occurrences near the ClientPlayNetworking.send / CraftRequestPayload logic) for consistent log-level filtering or remove them if they are unnecessary.
🤖 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/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Around line 132-135: The code dereferences this.minecraft and
this.minecraft.player (and similarly in the block around the RecipeDisplayEntry
usage) without null checks, which can NPE during world unload/disconnect; before
using this.minecraft, this.minecraft.player, or this.minecraft.level and before
accessing this.lastKnownSelectedId to read the
ClientRecipeBook/getKnown().get(...), add defensive null checks (e.g., if
(this.minecraft == null || this.minecraft.player == null || this.minecraft.level
== null || this.lastKnownSelectedId == null) return; or skip the lookup) so the
ClientRecipeBookAccessor/getKnown().get(this.lastKnownSelectedId) and any
subsequent use of RecipeDisplayEntry are only executed when the client and
player-level are valid.
- Around line 137-147: The code leaves lastKnownSelectedId stale when the
persistent selection lookup fails, causing subsequent craft attempts to always
fail; in the failure branch inside the crafting request method in
WorkbenchWorkbenchScreen (the block that checks if (entry != null) ... else
fallback), reset the remembered selection (e.g., set lastKnownSelectedId to null
or its sentinel value, or call any existing
clearSelection/clearLastKnownSelection helper) before logging "Request failed"
so a stale ID isn't reused on future clicks. Ensure you reference and update the
exact field name lastKnownSelectedId (or the corresponding selection field) in
that method.
In `@src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java`:
- Around line 87-97: Update the Javadoc for getShape to accurately describe that
SHAPE represents the collision shape of a single block (16×16×16) while the
workbench is configured as a multi-block when IS_WIDE = true; mention that the
overall 1×2 footprint is managed by AbstractWorkbench via the TYPE property
(LEFT/RIGHT) and neighbor validation, so the comment should not claim a "1×1
footprint" but instead clarify per-block shape vs. multi-block layout.
---
Duplicate comments:
In `@src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java`:
- Around line 47-55: Remove the stale TODO comment "// TODO: FarmersWorkbench
Not Implemented" that sits immediately above the implemented Farmers recipe in
WorkbenchRecipes; keep the existing WorkbenchRecipeBuilder block (the calls
using ModRecipes.WORKBENCH_TYPE, ModRecipes.WORKBENCH_SERIALIZER, .input(...),
.output(ModBlocks.FARMERS_WORKBENCH_BLOCK), .time(3), .unlockedBy(...),
.bookCategory(...), .save(...)) unchanged so the recipe remains intact and the
misleading TODO is eliminated.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java`:
- Around line 98-100: Remove the stray stdout debug print in
FarmersWorkbenchMenu (the System.out.println call that logs nearbyItems.size()
during client-side recipe-book updates); either delete the line or replace it
with the project's logger at debug/trace level (use the existing logger instance
or add one consistent with surrounding classes) so client-side code no longer
writes to System.out but uses structured logging instead.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java`:
- Around line 139-145: Replace the System.out.println debug prints in
ArmorersWorkbenchScreen (the ones around the persistent selection handling that
log "Persistent Selection Success" and "Request failed: No recipe was ever
selected!" as well as any other System.out.println occurrences in this class)
with the mod's logger calls (e.g., MineTale.LOGGER or your project's LOGGER)
using an appropriate level (info/debug/warn) and include contextual data such as
results.get(0) and amount in the message; ensure you update all prints in this
class (including the other occurrences near the ClientPlayNetworking.send /
CraftRequestPayload logic) for consistent log-level filtering or remove them if
they are unnecessary.
In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java`:
- Around line 85-272: Many moss recipes repeat the same WorkbenchRecipeBuilder
pattern; extract a helper (e.g., mossRecipe) that accepts the RecipeProvider
(provider), RecipeOutput/exporter, input Block, output Block and recipe name and
inside creates the WorkbenchRecipeBuilder with .input(inputMoss, 4),
.input(ModItems.PLANT_FIBER, 2), .output(output), .time(2),
.unlockedBy("has_farmers_workbench",
provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) and
.bookCategory(ModRecipeDisplay.FARMERS_SEARCH) then .save(exporter, name);
replace each repeated WorkbenchRecipeBuilder block with a call to
mossRecipe(provider, exporter, ModBlocks.*_MOSS, ModBlocks.*_OUTPUT, "name") to
remove duplication while keeping the same behavior.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java`:
- Around line 63-70: The getter getRecipeType() should reflect that its backing
field recipeType is nullable by updating the method signature to return a
nullable RecipeType<WorkbenchRecipe> (annotate with `@Nullable`) so callers and
static analysis see the contract; locate the getRecipeType() method and add the
`@Nullable` annotation to its return type (matching the `@Nullable` on the
recipeType field) and ensure imports for the `@Nullable` annotation are present.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java`:
- Around line 99-101: The code in ArmorersWorkbenchMenu uses System.out.println
for debug output when nearbyItems is non-empty on the client; replace this with
the mod logger call to avoid console pollution—use MineTale.LOGGER.debug(...)
(or remove the statement) and include the same contextual message (e.g.,
nearbyItems.size()) so the check around nearbyItems and
playerInventory.player.level().isClientSide() remains unchanged but the println
is swapped for MineTale.LOGGER.debug or removed entirely.
In `@src/main/java/com/tcm/MineTale/MineTale.java`:
- Line 21: Remove the unused import of ArmorersWorkbenchMenu: locate the import
statement referencing ArmorersWorkbenchMenu and delete it since the codebase now
uses AbstractWorkbenchContainerMenu for menu handling; ensure no other
references to ArmorersWorkbenchMenu remain (search for the class name) before
committing.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.javasrc/main/java/com/tcm/MineTale/MineTale.javasrc/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/registry/ModRecipes.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.java
- src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java
- src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java
- src/main/java/com/tcm/MineTale/registry/ModRecipes.java
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java
Show resolved
Hide resolved
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java (1)
101-103:⚠️ Potential issue | 🟡 MinorRemove console debug logging from recipe-content refresh path.
Line 102 logs during recipe book refreshes, which can create noisy logs during normal UI usage.
Suggested cleanup
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - }🤖 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/workbenches/menu/WorkbenchWorkbenchMenu.java` around lines 101 - 103, In WorkbenchWorkbenchMenu, remove the temporary System.out.println(...) used in the recipe-content refresh path (the block checking nearbyItems and this.playerInventory.player.level().isClientSide()); either delete the println entirely or replace it with a proper debug-level logger call (e.g. LOGGER.debug(...)) if you need traceability, ensuring no console System.out output remains during normal UI refreshes.src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java (1)
99-101:⚠️ Potential issue | 🟡 MinorDrop client console debug print from recipe content accounting.
Line 100 writes to stdout during UI refresh cycles and should be removed (or moved behind structured debug logging).
Suggested cleanup
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - }🤖 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/workbenches/menu/ArmorersWorkbenchMenu.java` around lines 99 - 101, Remove the System.out.println debug print in ArmorersWorkbenchMenu (the block that checks nearbyItems.isEmpty() and playerInventory.player.level().isClientSide()); either delete the line entirely or replace it with a structured logger call (e.g., LOGGER.debug(...)) guarded by the logger's debug/isEnabled check and include contextual info (nearbyItems.size()) so debug output is not printed to stdout during UI refresh cycles.
♻️ Duplicate comments (4)
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java (2)
146-146:⚠️ Potential issue | 🟡 MinorClear stale remembered selection when lookup/send fails.
When lookup fails once,
lastKnownSelectedIdremains stale and repeated clicks keep failing until reselection.Suggested fix
- System.out.println("Request failed: No recipe was ever selected!"); + this.lastKnownSelectedId = null; + System.out.println("Request failed: No recipe was ever selected!");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` at line 146, The failure path that currently just prints "Request failed: No recipe was ever selected!" should also clear the stale selection so subsequent clicks can retry: inside the failure handling in WorkbenchWorkbenchScreen (where the println is called), set lastKnownSelectedId to null (or an appropriate sentinel) and update any UI selection state (e.g., selectedRecipeIndex/selectedRecipe) as needed, then log the failure via the existing logger instead of System.out; this ensures lookup/send failures reset lastKnownSelectedId and allow fresh selections to succeed.
131-135:⚠️ Potential issue | 🔴 CriticalGuard client/player/level before dereferencing recipe-book state.
Line 134 and Line 187 can still NPE during transient client state changes (disconnect/world unload). Add a single early guard before recipe-book access in both paths.
Suggested fix
private void handleCraftRequest(int amount) { - // Look at our "Memory" instead of the component - if (this.lastKnownSelectedId != null) { + if (this.minecraft == null || this.minecraft.player == null || this.minecraft.level == null || this.lastKnownSelectedId == null) { + return; + } + // Look at our "Memory" instead of the component + if (this.lastKnownSelectedId != null) { ClientRecipeBook book = this.minecraft.player.getRecipeBook(); RecipeDisplayEntry entry = ((ClientRecipeBookAccessor) book).getKnown().get(this.lastKnownSelectedId); @@ - if (this.lastKnownSelectedId != null && this.minecraft.level != null) { + if (this.lastKnownSelectedId != null + && this.minecraft != null + && this.minecraft.player != null + && this.minecraft.level != null) { ClientRecipeBook book = this.minecraft.player.getRecipeBook(); selectedEntry = ((ClientRecipeBookAccessor) book).getKnown().get(this.lastKnownSelectedId); }Also applies to: 186-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` around lines 131 - 135, Add a null-check guard for the client/player/level before any recipe-book dereference: in handleCraftRequest, before using this.lastKnownSelectedId and calling this.minecraft.player.getRecipeBook(), verify this.minecraft != null && this.minecraft.player != null && this.minecraft.player.getRecipeBook() != null (and optionally this.minecraft.level != null if other logic relies on world state); return early if any are null. Apply the same single-early-guard pattern to the other code path in this class that accesses this.minecraft.player.getRecipeBook() (the branch around the other RecipeDisplayEntry access) so both places avoid NPEs during disconnect/world-unload transitions.src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java (1)
47-47:⚠️ Potential issue | 🟡 MinorRemove stale TODO above implemented Farmers recipe.
Line 47 says Farmers workbench is not implemented, but Lines 48-55 already build and save that recipe.
Suggested cleanup
- // TODO: FarmersWorkbench Not Implemented🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java` at line 47, Remove the stale TODO comment "// TODO: FarmersWorkbench Not Implemented" from the WorkbenchRecipes class (the FarmersWorkbench recipe builder block) because the Farmers workbench recipe is already constructed and saved in the adjacent code; locate the FarmersWorkbench-related recipe creation code in WorkbenchRecipes and delete the TODO line so comments reflect current implementation.src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java (1)
98-100:⚠️ Potential issue | 🟡 MinorRemove debug print from recipe refresh path.
Line 99 still logs nearby packet stack counts via
System.out.println, which is noisy during normal UI use.Suggested cleanup
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - }🤖 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/workbenches/menu/FarmersWorkbenchMenu.java` around lines 98 - 100, Remove the noisy debug print in FarmersWorkbenchMenu: delete the System.out.println call that logs nearbyItems.size() inside the recipe refresh path (the block checking nearbyItems.isEmpty() and this.playerInventory.player.level().isClientSide()). If you prefer retaining diagnostics, replace it with a proper logger call (e.g., LOGGER.debug(...)) guarded by an appropriate debug-level check; otherwise simply remove the println statement.
🧹 Nitpick comments (7)
src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java (1)
74-479: Extract a helper for repeated recipe boilerplate.This section repeats the same builder tail (
time/unlockedBy/bookCategory/save) many times. A helper will reduce drift and typo risk as this list grows.Refactor sketch
+ private static WorkbenchRecipeBuilder farmersBase(RecipeProvider provider) { + return new WorkbenchRecipeBuilder(ModRecipes.FARMERS_TYPE, ModRecipes.FARMERS_SERIALIZER) + .time(2) + .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) + .bookCategory(ModRecipeDisplay.FARMERS_SEARCH); + } + public static void buildRecipes(RecipeProvider provider, RecipeOutput exporter, HolderLookup.Provider lookup) { - new WorkbenchRecipeBuilder(ModRecipes.FARMERS_TYPE, ModRecipes.FARMERS_SERIALIZER) + farmersBase(provider) .input(ModItems.PLANT_FIBER, 20) .input(ItemTags.DIRT, lookup, 2) .input(ModItems.ESSENCE_OF_LIFE, 10) .input(ModBlocks.BAMBOO_LOG) .output(ModBlocks.BAMBOO_PLANTER) - .time(2) - .unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)) - .bookCategory(ModRecipeDisplay.FARMERS_SEARCH) .save(exporter, "bamboo_planter");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java` around lines 74 - 479, Many recipes repeat the same tail calls (time(2).unlockedBy("has_farmers_workbench", provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)).bookCategory(ModRecipeDisplay.FARMERS_SEARCH).save(exporter, name)); extract a small helper method (e.g., finalizeFarmersRecipe or saveFarmersRecipe) that accepts a WorkbenchRecipeBuilder and the recipe name (and exporter) and performs those four calls so each recipe creation becomes new WorkbenchRecipeBuilder(...).input(...).output(...).time/... replaced with builder passed to finalizeFarmersRecipe(builder, exporter, "id"); update all usages to call that helper and keep unique symbols WorkbenchRecipeBuilder, provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK), ModRecipeDisplay.FARMERS_SEARCH and save(exporter, name) intact.src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java (1)
24-24: Remove orphaned comment.The comment
// ChestBlockappears to be a leftover note that doesn't describe anything in the current context. Consider removing it.🧹 Proposed fix
-// ChestBlock - public class FarmersWorkbench extends AbstractWorkbench<FarmersWorkbenchEntity> {🤖 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/workbenches/FarmersWorkbench.java` at line 24, Remove the orphaned inline comment "// ChestBlock" from the FarmersWorkbench.java file; locate it in the FarmersWorkbench class (search for the exact comment text) and delete that leftover line so the class/comment no longer contains an irrelevant note.src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java (2)
143-144: Fix double comment prefix.Line 143 has
// //which appears to be accidental.🧹 Proposed fix
- // // 2. Return the menu as usual + // 2. Return the menu as usual return new FarmersWorkbenchMenu(syncId, playerInventory, this.data, this);🤖 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/workbenches/entity/FarmersWorkbenchEntity.java` around lines 143 - 144, The return statement in FarmersWorkbenchEntity currently has a double comment prefix ("// //") on the preceding line; remove the extra slash so the comment is a single-line comment or delete the stray commented line entirely around the return of new FarmersWorkbenchMenu(syncId, playerInventory, this.data, this); in the FarmersWorkbenchEntity class to clean up the accidental double-comment.
165-175: Remove commented-out code or add TODO explaining intent.The commented-out LIT state check (lines 168-170) appears to be leftover from development. Either remove it or add a TODO comment explaining the planned future behavior.
🧹 Proposed cleanup
`@Override` protected boolean hasFuel() { if (this.level == null) return false; - - // Check if block is lit - // BlockState state = this.level.getBlockState(this.worldPosition); - // boolean isLit = state.hasProperty(BlockStateProperties.LIT) && state.getValue(BlockStateProperties.LIT); - + boolean hasFuelItem = !this.getItem(Constants.FUEL_SLOT).isEmpty(); - return hasFuelItem; }🤖 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/workbenches/entity/FarmersWorkbenchEntity.java` around lines 165 - 175, The hasFuel() method contains commented-out code checking a BlockState LIT property; remove those commented lines or replace them with a concise TODO explaining intended future behavior (e.g., "TODO: consider BlockState LIT check when integrating lighting-based fuel logic") so the purpose is clear; update the method surrounding symbols hasFuel(), this.level, this.worldPosition, and Constants.FUEL_SLOT/getItem(...) accordingly and keep only active code that returns hasFuelItem.src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java (3)
141-148: Remove debug print statements before release.
System.out.printlncalls at lines 141, 147 (and similarly inhasIngredientAmountat lines 272, 291, 297, 301) should be removed or replaced with a proper logging framework with appropriate log levels (e.g.,MineTale.LOGGER.debug(...)).🧹 Example fix for handleCraftRequest
if (!results.isEmpty()) { - System.out.println("Persistent Selection Success: " + results.get(0)); ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)); return; } } } - System.out.println("Request failed: No recipe was ever selected!"); + MineTale.LOGGER.debug("Craft request failed: No recipe selected"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 141 - 148, Remove or replace direct System.out.println calls in FarmersWorkbenchScreen (e.g., inside handleCraftRequest and hasIngredientAmount) with the project's logger at an appropriate log level; specifically, replace the printlns (the "Persistent Selection Success..." and "Request failed..." messages and the debug prints in hasIngredientAmount) with MineTale.LOGGER.debug(...) or remove them if not needed, ensuring you import or reference MineTale.LOGGER consistently and keep the original message text/context in the debug calls.
177-193: Fix inconsistent step numbering in comments.The comments use "1.", "2.", "3.", then "2." again at line 192. This appears to be a copy-paste oversight.
🧹 Proposed fix
// 2. If it's NOT null, remember it! if (currentId != null) { this.lastKnownSelectedId = currentId; } // 3. Use the remembered ID to find the entry for button activation RecipeDisplayEntry selectedEntry = null; if (this.lastKnownSelectedId != null && this.minecraft.level != null) { ClientRecipeBook book = this.minecraft.player.getRecipeBook(); selectedEntry = ((ClientRecipeBookAccessor) book).getKnown().get(this.lastKnownSelectedId); } - // 2. Button Activation Logic + // 4. Button Activation Logic if (selectedEntry != null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 177 - 193, The comment numbering is inconsistent (1., 2., 3., then 2.) around the selection logic; update the inline comments near getSelectedRecipeId(), lastKnownSelectedId, and selectedEntry to use a sequential sequence (e.g., 1., 2., 3., 4. or remove numeric prefixes) so the steps read consistently — specifically change the last "2. Button Activation Logic" comment to "4. Button Activation Logic" (or a non-numbered header) to match the earlier "1.", "2.", "3." steps.
226-240: UseResourceKey<Item>or item registry keys as HashMap keys instead ofHolder<Item>instances.Using
List<Holder<Item>>as a HashMap key relies onHolder.Reference.equals(), which uses identity comparison, not structural equality. The comment is misleading—if the registry returns differentHolder.Referenceinstances for the same item (or ifHolder.Directentries appear), map lookups will fail. Per Minecraft's own guidance, prefer extracting the registry key viaholder.unwrapKey()or using item IDs/ResourceLocations for stable, content-based equality in map operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 226 - 240, The Map keys must use stable registry identifiers rather than Holder identity: change the key construction in the loop that currently builds List<Holder<Item>> (used by aggregatedRequirements and holderToIngredient) to a List<ResourceKey<Item>> (or List<ResourceLocation/ID) by extracting each holder's registry key via holder.unwrapKey().orElseGet(...) and falling back to a stable identifier for direct holders; then use that List<ResourceKey<Item>> as the HashMap key everywhere (including lookups in hasIngredientAmount) so equality/hashCode are content-based and do not depend on Holder.Reference identity.
🤖 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/workbenches/menu/AbstractWorkbenchContainerMenu.java`:
- Around line 41-43: Change the nullable recipeType field in
AbstractWorkbenchContainerMenu to be non-null by contract: remove the `@Nullable`
annotation on the private final RecipeType<WorkbenchRecipe> recipeType, require
a non-null RecipeType parameter in the constructor(s), and validate it on
construction (e.g., Objects.requireNonNull) so the field can never be null;
update any callers/overloads to pass a concrete RecipeType and adjust the code
paths that perform server recipe lookup (the usages around the current lines
68-70 and 90-94) to rely on the guaranteed non-null recipeType rather than
guarding for null.
In `@src/main/java/com/tcm/MineTale/MineTale.java`:
- Around line 89-97: The current guard uses a broad instanceof check against
AbstractWorkbenchContainerMenu (player.containerMenu) which lets unintended
workbench types through; change the boolean isWorkbench to explicitly check for
the two concrete direct-craft container classes that should be allowed (e.g.,
replace the AbstractWorkbenchContainerMenu instanceof test with an instanceof
check for the two specific container classes used for instant direct-craft in
your codebase), and only cast to AbstractWorkbenchContainerMenu
(instanceContainerMenu) after that narrower check passes; use the actual
concrete class names present in the project in place of the placeholder names.
---
Outside diff comments:
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java`:
- Around line 99-101: Remove the System.out.println debug print in
ArmorersWorkbenchMenu (the block that checks nearbyItems.isEmpty() and
playerInventory.player.level().isClientSide()); either delete the line entirely
or replace it with a structured logger call (e.g., LOGGER.debug(...)) guarded by
the logger's debug/isEnabled check and include contextual info
(nearbyItems.size()) so debug output is not printed to stdout during UI refresh
cycles.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java`:
- Around line 101-103: In WorkbenchWorkbenchMenu, remove the temporary
System.out.println(...) used in the recipe-content refresh path (the block
checking nearbyItems and this.playerInventory.player.level().isClientSide());
either delete the println entirely or replace it with a proper debug-level
logger call (e.g. LOGGER.debug(...)) if you need traceability, ensuring no
console System.out output remains during normal UI refreshes.
---
Duplicate comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Line 146: The failure path that currently just prints "Request failed: No
recipe was ever selected!" should also clear the stale selection so subsequent
clicks can retry: inside the failure handling in WorkbenchWorkbenchScreen (where
the println is called), set lastKnownSelectedId to null (or an appropriate
sentinel) and update any UI selection state (e.g.,
selectedRecipeIndex/selectedRecipe) as needed, then log the failure via the
existing logger instead of System.out; this ensures lookup/send failures reset
lastKnownSelectedId and allow fresh selections to succeed.
- Around line 131-135: Add a null-check guard for the client/player/level before
any recipe-book dereference: in handleCraftRequest, before using
this.lastKnownSelectedId and calling this.minecraft.player.getRecipeBook(),
verify this.minecraft != null && this.minecraft.player != null &&
this.minecraft.player.getRecipeBook() != null (and optionally
this.minecraft.level != null if other logic relies on world state); return early
if any are null. Apply the same single-early-guard pattern to the other code
path in this class that accesses this.minecraft.player.getRecipeBook() (the
branch around the other RecipeDisplayEntry access) so both places avoid NPEs
during disconnect/world-unload transitions.
In `@src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java`:
- Line 47: Remove the stale TODO comment "// TODO: FarmersWorkbench Not
Implemented" from the WorkbenchRecipes class (the FarmersWorkbench recipe
builder block) because the Farmers workbench recipe is already constructed and
saved in the adjacent code; locate the FarmersWorkbench-related recipe creation
code in WorkbenchRecipes and delete the TODO line so comments reflect current
implementation.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java`:
- Around line 98-100: Remove the noisy debug print in FarmersWorkbenchMenu:
delete the System.out.println call that logs nearbyItems.size() inside the
recipe refresh path (the block checking nearbyItems.isEmpty() and
this.playerInventory.player.level().isClientSide()). If you prefer retaining
diagnostics, replace it with a proper logger call (e.g., LOGGER.debug(...))
guarded by an appropriate debug-level check; otherwise simply remove the println
statement.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 141-148: Remove or replace direct System.out.println calls in
FarmersWorkbenchScreen (e.g., inside handleCraftRequest and hasIngredientAmount)
with the project's logger at an appropriate log level; specifically, replace the
printlns (the "Persistent Selection Success..." and "Request failed..." messages
and the debug prints in hasIngredientAmount) with MineTale.LOGGER.debug(...) or
remove them if not needed, ensuring you import or reference MineTale.LOGGER
consistently and keep the original message text/context in the debug calls.
- Around line 177-193: The comment numbering is inconsistent (1., 2., 3., then
2.) around the selection logic; update the inline comments near
getSelectedRecipeId(), lastKnownSelectedId, and selectedEntry to use a
sequential sequence (e.g., 1., 2., 3., 4. or remove numeric prefixes) so the
steps read consistently — specifically change the last "2. Button Activation
Logic" comment to "4. Button Activation Logic" (or a non-numbered header) to
match the earlier "1.", "2.", "3." steps.
- Around line 226-240: The Map keys must use stable registry identifiers rather
than Holder identity: change the key construction in the loop that currently
builds List<Holder<Item>> (used by aggregatedRequirements and
holderToIngredient) to a List<ResourceKey<Item>> (or List<ResourceLocation/ID)
by extracting each holder's registry key via holder.unwrapKey().orElseGet(...)
and falling back to a stable identifier for direct holders; then use that
List<ResourceKey<Item>> as the HashMap key everywhere (including lookups in
hasIngredientAmount) so equality/hashCode are content-based and do not depend on
Holder.Reference identity.
In `@src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java`:
- Around line 74-479: Many recipes repeat the same tail calls
(time(2).unlockedBy("has_farmers_workbench",
provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK)).bookCategory(ModRecipeDisplay.FARMERS_SEARCH).save(exporter,
name)); extract a small helper method (e.g., finalizeFarmersRecipe or
saveFarmersRecipe) that accepts a WorkbenchRecipeBuilder and the recipe name
(and exporter) and performs those four calls so each recipe creation becomes new
WorkbenchRecipeBuilder(...).input(...).output(...).time/... replaced with
builder passed to finalizeFarmersRecipe(builder, exporter, "id"); update all
usages to call that helper and keep unique symbols WorkbenchRecipeBuilder,
provider.has(ModBlocks.FARMERS_WORKBENCH_BLOCK), ModRecipeDisplay.FARMERS_SEARCH
and save(exporter, name) intact.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java`:
- Around line 143-144: The return statement in FarmersWorkbenchEntity currently
has a double comment prefix ("// //") on the preceding line; remove the extra
slash so the comment is a single-line comment or delete the stray commented line
entirely around the return of new FarmersWorkbenchMenu(syncId, playerInventory,
this.data, this); in the FarmersWorkbenchEntity class to clean up the accidental
double-comment.
- Around line 165-175: The hasFuel() method contains commented-out code checking
a BlockState LIT property; remove those commented lines or replace them with a
concise TODO explaining intended future behavior (e.g., "TODO: consider
BlockState LIT check when integrating lighting-based fuel logic") so the purpose
is clear; update the method surrounding symbols hasFuel(), this.level,
this.worldPosition, and Constants.FUEL_SLOT/getItem(...) accordingly and keep
only active code that returns hasFuelItem.
In `@src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java`:
- Line 24: Remove the orphaned inline comment "// ChestBlock" from the
FarmersWorkbench.java file; locate it in the FarmersWorkbench class (search for
the exact comment text) and delete that leftover line so the class/comment no
longer contains an irrelevant note.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.javasrc/main/java/com/tcm/MineTale/MineTale.javasrc/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/registry/ModBlockEntities.javasrc/main/java/com/tcm/MineTale/registry/ModBlocks.javasrc/main/java/com/tcm/MineTale/registry/ModItems.javasrc/main/java/com/tcm/MineTale/registry/ModMenuTypes.javasrc/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.javasrc/main/java/com/tcm/MineTale/registry/ModRecipes.java
💤 Files with no reviewable changes (1)
- src/main/java/com/tcm/MineTale/registry/ModItems.java
src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java
Show resolved
Hide resolved
Docstrings generation was requested by @The-Code-Monkey. The following files were modified: * `src/client/java/com/tcm/MineTale/MineTaleClient.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` * `src/main/java/com/tcm/MineTale/MineTale.java` * `src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java` * `src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java` These files were kept as they were: * `src/client/java/com/tcm/MineTale/datagen/recipes/FarmerRecipes.java` * `src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java` * `src/main/java/com/tcm/MineTale/registry/ModRecipes.java`
Feat/add builders workbench
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java (1)
98-102:⚠️ Potential issue | 🟡 MinorRemove this debug incantation before the code journeys to production, dear fellowship member.
This
System.out.printlnis like leaving the Palantír lying about for anyone to trip over - it clutters the logs and serves no purpose in the final deployment. The Vogons would not approve of such untidiness in production code.🧹 Proposed fix to banish the debug statement
List<ItemStack> nearbyItems = this.getNetworkedNearbyItems(); - - if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - } for (ItemStack stack : nearbyItems) {🤖 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/workbenches/menu/WorkbenchWorkbenchMenu.java` around lines 98 - 102, Remove the leftover debug print in WorkbenchWorkbenchMenu: delete the System.out.println(...) line inside the block that checks nearbyItems and playerInventory.player.level().isClientSide() so the method getNetworkedNearbyItems()/the nearbyItems debug branch no longer prints to stdout; keep the surrounding logic intact and, if logging is desired, replace with an appropriate logger call or remove entirely.
♻️ Duplicate comments (2)
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java (2)
136-137:⚠️ Potential issue | 🟠 MajorGuard client state before dereferencing, lest nulls from the void strike.
this.minecraft,this.minecraft.player, andthis.minecraft.levelare still used without full defensive checks in these paths, which can NPE during disconnect/unload transitions.Suggested hardening patch
private void handleCraftRequest(int amount) { + if (this.minecraft == null || this.minecraft.player == null || this.minecraft.level == null) { + return; + } // Look at our "Memory" instead of the component if (this.lastKnownSelectedId != null) { ClientRecipeBook book = this.minecraft.player.getRecipeBook(); RecipeDisplayEntry entry = ((ClientRecipeBookAccessor) book).getKnown().get(this.lastKnownSelectedId); @@ RecipeDisplayEntry selectedEntry = null; - if (this.lastKnownSelectedId != null && this.minecraft.level != null) { + if (this.lastKnownSelectedId != null + && this.minecraft != null + && this.minecraft.player != null + && this.minecraft.level != null) { ClientRecipeBook book = this.minecraft.player.getRecipeBook(); selectedEntry = ((ClientRecipeBookAccessor) book).getKnown().get(this.lastKnownSelectedId); }Also applies to: 140-140, 187-189, 195-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` around lines 136 - 137, Guard against null Minecraft client state before dereferencing in WorkbenchWorkbenchScreen: ensure this.minecraft, this.minecraft.player, and this.minecraft.level are non-null before accessing player.getRecipeBook(), player inventory/recipe operations, or level-dependent logic (e.g., when reading lastKnownSelectedId and using ClientRecipeBookAccessor.getKnown()). Add early-returns or conditional branches in the methods that reference these symbols (including the code path using ClientRecipeBook book = this.minecraft.player.getRecipeBook() and any blocks around lastKnownSelectedId) so the code safely skips or defers work when the client, player, or level is null.
148-148:⚠️ Potential issue | 🟡 MinorClear stale remembered selection on failure.
When lookup fails,
lastKnownSelectedIdremains stale and future clicks keep failing until re-selection; clear it in the failure path.Suggested tidy fix
- System.out.println("Request failed: No recipe was ever selected!"); + this.lastKnownSelectedId = null; + System.out.println("Request failed: No recipe was ever selected!"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` at line 148, On lookup failure in WorkbenchWorkbenchScreen (the branch that currently logs "Request failed: No recipe was ever selected!"), clear the stale selection by resetting lastKnownSelectedId (and any related selection state) so future clicks can re-select; update the failure path that contains the System.out.println to set lastKnownSelectedId = null (or the class's default) and ensure any UI selection flags are cleared as well.
🤖 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/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 269-301: The method hasIngredientAmount is littered with
System.out.println debug prints (e.g., the initial "DEBUG: Searching
inventory..." and messages inside the workbenchMenu loop and the success/failure
logs); remove these printlns before merging to production and either delete them
entirely or replace with a proper logger call at a debug/trace level if you want
to keep them (use your project's logging facility rather than System.out).
Locate the printlns inside hasIngredientAmount and the block that checks menu
instanceof AbstractWorkbenchContainerMenu workbenchMenu and update accordingly,
ensuring no System.out calls remain in production code.
- Around line 133-149: In handleCraftRequest in FarmersWorkbenchScreen remove
the two System.out.println debug statements (the "Persistent Selection Success:
..." and "Request failed: No recipe was ever selected!") and either drop them
entirely or replace them with the project's logging utility at an appropriate
level (e.g., LOGGER.debug or similar) so production builds do not print to
stdout; keep the rest of the method (the ClientPlayNetworking.send(new
CraftRequestPayload(...)) and early return) unchanged.
---
Outside diff comments:
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java`:
- Around line 98-102: Remove the leftover debug print in WorkbenchWorkbenchMenu:
delete the System.out.println(...) line inside the block that checks nearbyItems
and playerInventory.player.level().isClientSide() so the method
getNetworkedNearbyItems()/the nearbyItems debug branch no longer prints to
stdout; keep the surrounding logic intact and, if logging is desired, replace
with an appropriate logger call or remove entirely.
---
Duplicate comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Around line 136-137: Guard against null Minecraft client state before
dereferencing in WorkbenchWorkbenchScreen: ensure this.minecraft,
this.minecraft.player, and this.minecraft.level are non-null before accessing
player.getRecipeBook(), player inventory/recipe operations, or level-dependent
logic (e.g., when reading lastKnownSelectedId and using
ClientRecipeBookAccessor.getKnown()). Add early-returns or conditional branches
in the methods that reference these symbols (including the code path using
ClientRecipeBook book = this.minecraft.player.getRecipeBook() and any blocks
around lastKnownSelectedId) so the code safely skips or defers work when the
client, player, or level is null.
- Line 148: On lookup failure in WorkbenchWorkbenchScreen (the branch that
currently logs "Request failed: No recipe was ever selected!"), clear the stale
selection by resetting lastKnownSelectedId (and any related selection state) so
future clicks can re-select; update the failure path that contains the
System.out.println to set lastKnownSelectedId = null (or the class's default)
and ensure any UI selection flags are cleared as well.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/main/java/com/tcm/MineTale/MineTale.javasrc/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java
- src/client/java/com/tcm/MineTale/MineTaleClient.java
- src/main/java/com/tcm/MineTale/MineTale.java
- src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java
- src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java
- src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java
src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java
Show resolved
Hide resolved
src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java (1)
98-100:⚠️ Potential issue | 🟡 MinorRemove this debug proclamation before journeying to production, dear hoopy frood!
This
System.out.printlnis helpful during your quest through Middle-earth's debugging swamps, but shall only cause noise in the production logs. Replace with a proper logger at DEBUG level or remove entirely.🧹 Proposed fix
if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); }🤖 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/workbenches/menu/WorkbenchWorkbenchMenu.java` around lines 98 - 100, In WorkbenchWorkbenchMenu remove the System.out.println debug line that logs nearbyItems and either delete it or replace it with a proper logger call at DEBUG level; locate the block that checks nearbyItems.isEmpty() and playerInventory.player.level().isClientSide() and change the println to use your project's logging framework (e.g., logger.debug(...)) or remove it entirely so production logs are not polluted.
🧹 Nitpick comments (5)
src/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.java (1)
91-93: Debug logging left behind like breadcrumbs in Mirkwood!Ah, I spy with my little eye a
System.out.printlndebug statement! Most useful during the quest for bug-slaying, but perhaps not something you'd want cluttering the production logs like Smaug's treasure hoard cluttering Erebor.Consider removing this debug output before the final release, or replacing it with a proper logging framework call that can be toggled. Though I must say, 42 debug statements might be the answer to everything.
🧹 Suggested tidying
- if (!nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()) { - System.out.println("DEBUG: Recipe Book is now accounting for " + nearbyItems.size() + " stacks from the packet!"); - } + // Debug logging removed for production🤖 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/workbenches/menu/BuildersWorkbenchMenu.java` around lines 91 - 93, Remove the leftover System.out.println debug statement in BuildersWorkbenchMenu (the block inspecting nearbyItems and this.playerInventory.player.level().isClientSide()). Replace it either by deleting the line or by using the project's logging facility (e.g., a class-level LOGGER.debug/trace) so the message can be toggled off in production; ensure any replacement uses the same conditional (isClientSide()) and references nearbyItems.size() when logging so behavior is identical.src/main/java/com/tcm/MineTale/block/workbenches/entity/BuildersWorkbenchEntity.java (2)
64-77: Javadoc states scanRadius is set to 0.0, but the constructor doesn't explicitly assign it.The documentation at line 67 mentions "Sets the entity's scanRadius to 0.0" yet the constructor only sets
tierandcanPullFromNearby. If the superclass default is 0.0, the Javadoc is technically correct but potentially misleading. Consider either explicitly settingthis.scanRadius = 0.0;or updating the Javadoc to clarify it relies on the inherited default.🤖 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/workbenches/entity/BuildersWorkbenchEntity.java` around lines 64 - 77, The Javadoc claims scanRadius is set to 0.0 but the BuildersWorkbenchEntity constructor doesn't assign it; either explicitly set the field in the constructor (this.scanRadius = 0.0;) or update the Javadoc to state it relies on the superclass default. Locate the BuildersWorkbenchEntity constructor and either add an explicit assignment to the scanRadius field alongside this.tier and this.canPullFromNearby, or change the comment text to remove the assignment claim and note that scanRadius uses the inherited default.
28-62: The ContainerData declares 4 slots but uses none, like a Vogon filing cabinet – bureaucratically present yet functionally empty.The
getCount()method returns 4, yetget(index)only has a default case returning 0. If no synchronised data is needed, consider returning 0 fromgetCount()(matching theDATA_SIZE = 0pattern inBuildersWorkbenchMenu). Alternatively, if future data slots are planned, document the intent.🛠️ Proposed simplification
`@Override` public int getCount() { - return 4; + return 0; }🤖 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/workbenches/entity/BuildersWorkbenchEntity.java` around lines 28 - 62, The ContainerData anonymous instance in BuildersWorkbenchEntity currently returns getCount() == 4 while get(int) always returns 0; update this to be consistent with BuildersWorkbenchMenu's DATA_SIZE by changing getCount() to return 0 if no sync data is needed, or implement explicit index cases in the get(int) (and set(int,int) if required) to expose the intended 4 data slots; touch the ContainerData field named data in BuildersWorkbenchEntity and ensure its getCount() matches BuildersWorkbenchMenu.DATA_SIZE (or vice versa) and document the choice.src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java (1)
38-299: This screen is remarkably similar to FarmersWorkbenchScreen – consider a shared base class.The
BuildersWorkbenchScreenandFarmersWorkbenchScreenshare approximately 90% identical code, differing only in their recipe type, block reference, and category constants. This is like having two copies of "The Lord of the Rings" with only the cover art changed. A shared abstract base class (e.g.,AbstractMineTaleWorkbenchScreen) could reduce maintenance burden and ensure consistent behaviour across all workbench types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java` around lines 38 - 299, BuildersWorkbenchScreen duplicates most logic from FarmersWorkbenchScreen; extract a new AbstractMineTaleWorkbenchScreen base class holding shared fields (mineTaleRecipeBook, lastKnownSelectedId, craftOneBtn/craftTenBtn/craftAllBtn), common lifecycle and helpers (init, handleCraftRequest, render, renderBg, canCraft, hasIngredientAmount, getRecipeBookButtonPosition, createRecipeBookComponent helper) and move duplicated implementations there; make BuildersWorkbenchScreen and FarmersWorkbenchScreen extend this base and implement small protected abstract getters used by the base (e.g., getTexture()/TEXTURE, getTabIcon()/block item for createRecipeBookComponent, getRecipeType()/ModRecipes type, and any menu-specific bindings) so only workbench-specific constants remain in the subclasses and all shared methods reference those abstract accessors (use class names BuildersWorkbenchScreen, FarmersWorkbenchScreen, and AbstractMineTaleWorkbenchScreen to locate code to move/refactor).src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java (1)
218-256: Consider extracting shared crafting logic to reduce duplication across workbench screens.The
canCraftandhasIngredientAmountmethods present in this file appear similar to those found in other workbench screen implementations (such asBuildersWorkbenchScreen). This duplication is like hauling unnecessary luggage through Middle-earth – a burden best left behind. Extracting these methods into a shared utility class or establishing a common base screen class would lighten the load and improve maintainability across all workbench implementations.Manual inspection of
BuildersWorkbenchScreenand other workbench screen classes is needed to confirm the exact extent of duplication and determine the optimal extraction strategy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` around lines 218 - 256, The canCraft method (and its companion hasIngredientAmount) duplicates logic found in other workbench screens like BuildersWorkbenchScreen; extract the shared crafting/inventory-check logic into a single reusable place (either a new utility class e.g., WorkbenchCraftingUtils with static methods or a common abstract base class e.g., AbstractWorkbenchScreen) and update FarmersWorkbenchScreen to call the shared canCraft/hasIngredientAmount implementation; ensure you move any helper details (aggregation of Ingredient holders, holderToIngredient mapping, and Inventory checks) and preserve method signatures used by screens so callers (canCraft, hasIngredientAmount) simply delegate to the new shared implementation.
🤖 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/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Around line 249-281: The method hasIngredientAmount contains several
System.out.println debug calls; remove them (or replace with a proper logger at
debug/trace level) so production logs aren’t polluted—specifically update the
System.out.println usages inside hasIngredientAmount (including the messages
around checking inventory, reporting finds from
workbenchMenu.getNetworkedNearbyItems(), and the final requirement/result
messages) to either be deleted or changed to LOGGER.debug(...) after adding or
using an existing logger in BuildersWorkbenchScreen; ensure no
System.out.println calls remain in hasIngredientAmount.
- Around line 130-146: The two System.out.println debug prints in
handleCraftRequest (the one logging "Persistent Selection Success: " and the
failure "Request failed: No recipe was ever selected!") should be removed or
replaced with a proper logger; locate handleCraftRequest and remove the
System.out.println calls (or swap them for a configured logger call e.g.,
LOGGER.debug/trace) while leaving the logic that builds results and calls
ClientPlayNetworking.send(new CraftRequestPayload(..., amount)) intact, and
ensure no other stray System.out.println usages remain in this method.
In `@src/main/java/com/tcm/MineTale/registry/ModRecipes.java`:
- Around line 43-48: Update the Javadoc in ModRecipes to include BUILDERS_TYPE
in the enumerated list of registered recipe types (alongside FURNACE_T1_TYPE,
CAMPFIRE_TYPE, WORKBENCH_TYPE, ARMORERS_TYPE, and FARMERS_TYPE) and, if
relevant, mention its corresponding serializer (BUILDERS_SERIALIZER) so the
documentation accurately reflects the registrations performed by the ModRecipes
class.
---
Outside diff comments:
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java`:
- Around line 98-100: In WorkbenchWorkbenchMenu remove the System.out.println
debug line that logs nearbyItems and either delete it or replace it with a
proper logger call at DEBUG level; locate the block that checks
nearbyItems.isEmpty() and playerInventory.player.level().isClientSide() and
change the println to use your project's logging framework (e.g.,
logger.debug(...)) or remove it entirely so production logs are not polluted.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Around line 38-299: BuildersWorkbenchScreen duplicates most logic from
FarmersWorkbenchScreen; extract a new AbstractMineTaleWorkbenchScreen base class
holding shared fields (mineTaleRecipeBook, lastKnownSelectedId,
craftOneBtn/craftTenBtn/craftAllBtn), common lifecycle and helpers (init,
handleCraftRequest, render, renderBg, canCraft, hasIngredientAmount,
getRecipeBookButtonPosition, createRecipeBookComponent helper) and move
duplicated implementations there; make BuildersWorkbenchScreen and
FarmersWorkbenchScreen extend this base and implement small protected abstract
getters used by the base (e.g., getTexture()/TEXTURE, getTabIcon()/block item
for createRecipeBookComponent, getRecipeType()/ModRecipes type, and any
menu-specific bindings) so only workbench-specific constants remain in the
subclasses and all shared methods reference those abstract accessors (use class
names BuildersWorkbenchScreen, FarmersWorkbenchScreen, and
AbstractMineTaleWorkbenchScreen to locate code to move/refactor).
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 218-256: The canCraft method (and its companion
hasIngredientAmount) duplicates logic found in other workbench screens like
BuildersWorkbenchScreen; extract the shared crafting/inventory-check logic into
a single reusable place (either a new utility class e.g., WorkbenchCraftingUtils
with static methods or a common abstract base class e.g.,
AbstractWorkbenchScreen) and update FarmersWorkbenchScreen to call the shared
canCraft/hasIngredientAmount implementation; ensure you move any helper details
(aggregation of Ingredient holders, holderToIngredient mapping, and Inventory
checks) and preserve method signatures used by screens so callers (canCraft,
hasIngredientAmount) simply delegate to the new shared implementation.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/BuildersWorkbenchEntity.java`:
- Around line 64-77: The Javadoc claims scanRadius is set to 0.0 but the
BuildersWorkbenchEntity constructor doesn't assign it; either explicitly set the
field in the constructor (this.scanRadius = 0.0;) or update the Javadoc to state
it relies on the superclass default. Locate the BuildersWorkbenchEntity
constructor and either add an explicit assignment to the scanRadius field
alongside this.tier and this.canPullFromNearby, or change the comment text to
remove the assignment claim and note that scanRadius uses the inherited default.
- Around line 28-62: The ContainerData anonymous instance in
BuildersWorkbenchEntity currently returns getCount() == 4 while get(int) always
returns 0; update this to be consistent with BuildersWorkbenchMenu's DATA_SIZE
by changing getCount() to return 0 if no sync data is needed, or implement
explicit index cases in the get(int) (and set(int,int) if required) to expose
the intended 4 data slots; touch the ContainerData field named data in
BuildersWorkbenchEntity and ensure its getCount() matches
BuildersWorkbenchMenu.DATA_SIZE (or vice versa) and document the choice.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.java`:
- Around line 91-93: Remove the leftover System.out.println debug statement in
BuildersWorkbenchMenu (the block inspecting nearbyItems and
this.playerInventory.player.level().isClientSide()). Replace it either by
deleting the line or by using the project's logging facility (e.g., a
class-level LOGGER.debug/trace) so the message can be toggled off in production;
ensure any replacement uses the same conditional (isClientSide()) and references
nearbyItems.size() when logging so behavior is identical.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/client/java/com/tcm/MineTale/MineTaleClient.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/recipes/BuilderRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.javasrc/main/java/com/tcm/MineTale/MineTale.javasrc/main/java/com/tcm/MineTale/block/workbenches/BuildersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/BuildersWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.javasrc/main/java/com/tcm/MineTale/registry/ModBlockEntities.javasrc/main/java/com/tcm/MineTale/registry/ModBlocks.javasrc/main/java/com/tcm/MineTale/registry/ModMenuTypes.javasrc/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.javasrc/main/java/com/tcm/MineTale/registry/ModRecipes.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.java
- src/main/java/com/tcm/MineTale/MineTale.java
- src/client/java/com/tcm/MineTale/MineTaleClient.java
- src/main/java/com/tcm/MineTale/registry/ModBlocks.java
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
Show resolved
Hide resolved
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
Show resolved
Hide resolved
coderabbitai bot
added a commit
that referenced
this pull request
Feb 26, 2026
Docstrings generation was requested by @The-Code-Monkey. * #54 (comment) The following files were modified: * `src/client/java/com/tcm/MineTale/block/workbenches/screen/ArmorersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java` * `src/client/java/com/tcm/MineTale/datagen/recipes/BuilderRecipes.java` * `src/main/java/com/tcm/MineTale/MineTale.java` * `src/main/java/com/tcm/MineTale/block/workbenches/BuildersWorkbench.java` * `src/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.java` * `src/main/java/com/tcm/MineTale/block/workbenches/entity/BuildersWorkbenchEntity.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/CampfireWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/FurnaceWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java` * `src/main/java/com/tcm/MineTale/registry/ModRecipes.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.
Summary by CodeRabbit
New Features
Chores