Feat/add builders workbench#55
Merged
The-Code-Monkey merged 1 commit intofeat/add-farmers-workbenchfrom Feb 26, 2026
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces two new workbench types (Farmers and Builders) with complete implementations including client-side GUI screens with recipe book integration, server-side block entities with persistence, menu containers, recipe definitions, and registry registrations. Existing workbench systems are generalized to support multiple recipe types via a new RecipeType parameter. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant RecipeBook as Recipe Book
participant Inventory as Inventory System
participant Menu as Workbench Menu
participant Network as Network
participant Server as Server
participant ServerInv as Server Inventory
Client->>RecipeBook: Select recipe
RecipeBook->>Menu: Store selected recipe ID
Client->>Inventory: Check player items
Client->>Inventory: Check nearby networked items
Inventory-->>Client: Available ingredients
Client->>Client: Validate canCraft()
alt Sufficient ingredients
Client->>Network: Send CraftRequestPayload
Network->>Server: Receive craft request
Server->>Server: Lookup recipe by type
Server->>ServerInv: Validate ingredients exist
ServerInv-->>Server: Validation result
Server->>ServerInv: Consume ingredients
Server->>ServerInv: Grant output item
Server->>Network: Broadcast container update
Network->>Client: Update inventory display
else Insufficient ingredients
Client->>Client: Disable craft button
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 - SKIPPED |
coderabbitai bot
added a commit
that referenced
this pull request
Feb 26, 2026
Docstrings generation was requested by @The-Code-Monkey. * #55 (comment) 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/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/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/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/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/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`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (13)
src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java (1)
139-145: Consider removing or gating debugSystem.out.printlnstatements.Multiple debug print statements remain in production code (lines 139, 145, 250, 269, 275, 279). These will clutter logs and impact performance slightly. Consider removing them or using a proper logging framework with configurable log levels.
🔧 Example fix for one location
- System.out.println("Persistent Selection Success: " + results.get(0)); + // Remove or use: MineTale.LOGGER.debug("Persistent Selection Success: {}", results.get(0));Also applies to: 250-250, 269-269, 275-275, 279-279
🤖 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 139 - 145, Replace the debug System.out.println calls in WorkbenchWorkbenchScreen (the printlns around the persistent selection/recipe request logic and the other occurrences) with a proper logging call or remove them entirely; locate the uses of System.out.println in the class (e.g., the message "Persistent Selection Success: " and "Request failed: No recipe was ever selected!" and the other printlns later) and either delete them or swap them for a logger with an appropriate level (e.g., LOGGER.debug or LOGGER.trace) behind a configurable log level so debug output can be enabled in development but omitted in production.src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java (2)
90-92: DebugSystem.out.printlnshould be removed or use proper logging.Same pattern as other menu classes - consider removing or using a logging framework.
🤖 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 90 - 92, Remove the debug System.out.println call in WorkbenchWorkbenchMenu (the block checking nearbyItems and playerInventory.player.level().isClientSide()) and either delete it entirely or replace it with the project's logging framework (e.g., use a class logger such as LOGGER.debug/trace with a clear message and include nearbyItems.size()); ensure you reference WorkbenchWorkbenchMenu and the nearbyItems check so the change targets that exact debug print.
77-97: Consider extractingfillCraftSlotsStackedContentsto base class.The implementation in
WorkbenchWorkbenchMenuandArmorersWorkbenchMenuis nearly identical (accounting for player inventory, container slots, and networked nearby items). This could be extracted toAbstractWorkbenchContainerMenuto reduce duplication.🤖 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 77 - 97, Both WorkbenchWorkbenchMenu.fillCraftSlotsStackedContents and ArmorersWorkbenchMenu implement the same logic (account playerInventory, container slots, and networked nearby items); extract this logic into a protected method on AbstractWorkbenchContainerMenu (e.g., protected void fillCraftSlotsStackedContents(StackedItemContents contents)) and move the body there: call this.playerInventory.fillStackedContents(contents), iterate this.container.getContainerSize() / this.container.getItem(i), then getNetworkedNearbyItems() and accountStack for each. Make sure AbstractWorkbenchContainerMenu exposes/provides accessors or protected fields for playerInventory, container, and getNetworkedNearbyItems() (or make that method protected in the base) so subclasses can call it, then replace the duplicate implementations in WorkbenchWorkbenchMenu and ArmorersWorkbenchMenu with a single call to super.fillCraftSlotsStackedContents(contents).src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java (1)
91-93: DebugSystem.out.printlnshould be removed or use proper logging.Similar to other files in this PR, this debug statement will clutter production logs.
🔧 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!"); + // Consider using a proper logger: MineTale.LOGGER.debug(...) }🤖 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 debug System.out.println in ArmorersWorkbenchMenu (the block checking nearbyItems and playerInventory.player.level().isClientSide()) and either delete it or replace it with the project's logging framework (e.g., use a LOGGER.debug/trace call) so it does not print to stdout in production; locate the statement inside the ArmorersWorkbenchMenu class and update that method accordingly to use the established logger or remove the line entirely.src/main/java/com/tcm/MineTale/registry/ModRecipes.java (1)
43-48: Javadoc is outdated.The method documentation only mentions furnace, campfire, and workbench types, but the method now also registers armorers, farmers, and builders types.
📝 Suggested update
/** * 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, builders) + * 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 43 - 48, The Javadoc on ModRecipes is outdated: update the method/class comment that currently mentions only FURNACE_T1_TYPE, CAMPFIRE_TYPE and WORKBENCH_TYPE to also document the additional registered recipe types and serializers (ARMORER_TYPE, FARMER_TYPE, BUILDER_TYPE and their corresponding serializer constants/registrations). Edit the Javadoc block above the registering method in ModRecipes to list all six recipe types and briefly note that each is registered into the game's registries, referencing the constants FURNACE_T1_TYPE, CAMPFIRE_TYPE, WORKBENCH_TYPE, ARMORER_TYPE, FARMER_TYPE and BUILDER_TYPE so the doc matches the actual registration behavior.src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java (2)
130-146: Remove debug print statements before release.The
System.out.printlncalls on lines 139 and 145 should be removed or converted to proper logging with an appropriate level (e.g.,MineTale.LOGGER.debug()).🧹 Suggested cleanup
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 was 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 130 - 146, In handleCraftRequest(int amount) remove the two System.out.println debug prints and replace them with proper logger calls (use MineTale.LOGGER.debug(...)) or delete them entirely; specifically update the println in the success branch that prints "Persistent Selection Success: " + results.get(0) and the failure branch printing "Request failed: No recipe was ever selected!" to use MineTale.LOGGER.debug with the same or clearer messages so debugging remains available without using System.out.
249-281: Remove debug print statements from ingredient checking.Multiple
System.out.printlndebug calls exist inhasIngredientAmount. These should be removed or converted to proper logging before release.🧹 Suggested cleanup
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 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 249 - 281, The hasIngredientAmount method contains multiple System.out.println debug statements; remove those println calls (or replace them with the appropriate logger debug calls if a logging facility exists) inside hasIngredientAmount and the block that iterates workbenchMenu.getNetworkedNearbyItems so the method no longer prints to stdout in production. Ensure only meaningful logging via the project's logger (e.g., LOGGER.debug(...) if available) is used, and leave the logic and return behavior of hasIngredientAmount and use of workbenchMenu.getNetworkedNearbyItems unchanged.src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java (1)
28-62: Consider simplifyingContainerDatafor slotless workbench.The
ContainerDataimplementation returnsgetCount() = 4but theget()method returns 0 for all indices. SinceFarmersWorkbenchMenuusesDATA_SIZE = 0, this data is unused. Consider simplifying to match the actual usage or documenting why 4 is returned.♻️ Suggested simplification
protected final ContainerData data = new ContainerData() { `@Override` public int get(int index) { - return switch (index) { - default -> 0; - }; + return 0; } `@Override` public void set(int index, int value) { // Not required on WorkbenchEntity } `@Override` public int getCount() { - return 4; + return 0; // Slotless workbench - no synced data } };🤖 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 28 - 62, The ContainerData implementation in FarmersWorkbenchEntity named data exposes getCount() == 4 while get() always returns 0 and FarmersWorkbenchMenu uses DATA_SIZE = 0; update the entity to match actual usage by either removing the unused ContainerData field or changing its getCount() to return 0 (and keep get/set as no-ops) so the workbench is slotless and consistent with FarmersWorkbenchMenu.DATA_SIZE; modify the data field in FarmersWorkbenchEntity (or adjust FarmersWorkbenchMenu.DATA_SIZE if intentional) to ensure the two symbols ContainerData data and FarmersWorkbenchMenu.DATA_SIZE agree.src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java (2)
139-139: Remove debug print statements in handleCraftRequest.These
System.out.printlncalls should be removed or replaced with proper logging for production.🧹 Proposed fix
if (entry != null) { List<ItemStack> results = entry.resultItems(SlotDisplayContext.fromLevel(this.minecraft.level)); 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!"); }Also applies to: 145-145
🤖 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` at line 139, In BuildersWorkbenchScreen.handleCraftRequest remove the debug System.out.println calls (e.g., "Persistent Selection Success: " + results.get(0)) and replace them with proper logging or delete them entirely; locate the println occurrences inside the handleCraftRequest method and either call the class's logger (or add an SLF4J/Minecraft logger) with an appropriate log level (info/debug) and contextual message, or remove the lines if no runtime logging is desired.
249-281: Remove debug print statements in hasIngredientAmount.Multiple debug
System.out.printlncalls will spam the console during normal gameplay. Remove them or use a proper logger with debug level.🧹 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/BuildersWorkbenchScreen.java` around lines 249 - 281, The hasIngredientAmount method contains multiple System.out.println debug statements that should be removed (or replaced with a proper logger at debug level); open the hasIngredientAmount method and delete the println calls that announce "DEBUG: Searching inventory...", "DEBUG: Found ... in nearby networked list...", "DEBUG: Requirement MET...", and "DEBUG: FAILED..." or replace them with calls to a class logger (e.g., LOGGER.debug(...)) guarded by the logger's isDebugEnabled check; the relevant symbols to edit are the hasIngredientAmount(Inventory, Ingredient, int) method, the menu field check (this.menu instanceof AbstractWorkbenchContainerMenu workbenchMenu), and use workbenchMenu.getNetworkedNearbyItems() as-is.src/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.java (1)
91-93: Remove debug print statement before merging.Same issue as in
FarmersWorkbenchMenu- this debug logging will spam the console in production.🧹 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/BuildersWorkbenchMenu.java` around lines 91 - 93, In BuildersWorkbenchMenu remove the temporary System.out.println debug statement that prints nearbyItems size (the line inside the if checking nearbyItems.isEmpty() && this.playerInventory.player.level().isClientSide()); either delete it outright or replace it with a proper logger.debug call consistent with FarmersWorkbenchMenu's approach so production won't be spammed—ensure the nearbyItems/level check remains unchanged and only the Console print is removed or converted to debug logging.src/main/java/com/tcm/MineTale/block/workbenches/menu/FarmersWorkbenchMenu.java (1)
91-93: Remove debug print statement before merging.This
System.out.printlncall will spam the console in production. Consider using a proper logger with a debug level or removing it 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/FarmersWorkbenchMenu.java` around lines 91 - 93, Remove the debug System.out.println in FarmersWorkbenchMenu where it logs nearbyItems size; either delete the line or replace it with a proper logger.debug call (using the project's logging framework) and guard it with the existing condition (nearbyItems.isEmpty() check and playerInventory.player.level().isClientSide()). Update references in FarmersWorkbenchMenu so the log uses a logger instance (e.g., a private static final Logger) and a clear message like "Recipe Book accounting for {} stacks" with the nearbyItems.size() parameter.src/main/java/com/tcm/MineTale/registry/ModBlocks.java (1)
80-92: Consider using_blocksuffix for consistency with other workbenches.Other workbench registrations use the
_blocksuffix (e.g.,campfire_workbench_block,armorers_workbench_block), but these usefarmers_workbenchandbuilders_workbench. This affects resource locations for textures, models, and language files.🎨 Proposed fix for naming consistency
public static final Block FARMERS_WORKBENCH_BLOCK = register( - "farmers_workbench", + "farmers_workbench_block", FarmersWorkbench::new, BlockBehaviour.Properties.of().sound(SoundType.WOOD), true ); public static final Block BUILDERS_WORKBENCH_BLOCK = register( - "builders_workbench", + "builders_workbench_block", BuildersWorkbench::new, BlockBehaviour.Properties.of().sound(SoundType.WOOD), 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 80 - 92, The block registration names for FARMERS_WORKBENCH_BLOCK and BUILDERS_WORKBENCH_BLOCK currently use "farmers_workbench" and "builders_workbench", causing inconsistent resource locations; update the register(...) calls to use "farmers_workbench_block" and "builders_workbench_block" respectively (keep the same constructors FarmersWorkbench::new and BuildersWorkbench::new and existing BlockBehaviour.Properties), and then ensure any corresponding resource keys (textures/models/lang entries) are renamed to match the new registration IDs.
🤖 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 220-230: The code uses the deprecated Ingredient.items() call
inside BuildersWorkbenchScreen to build keys for aggregatedRequirements and
holderToIngredient; update this by replacing direct enumeration with a stable,
non-deprecated keying strategy (e.g., compute a deterministic key from
Ingredient.toDisplay() or a normalized representation produced by testing a
canonical ItemStack) so you no longer rely on Ingredient.items(); specifically,
in the loop that currently references Ingredient.items() and creates
List<Holder<Item>> key, produce a stable key (string or custom object) derived
from Ingredient.toDisplay() or a canonical test-based representation, then use
that key when updating aggregatedRequirements and holderToIngredient and ensure
hasIngredientAmount uses the same keying method.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 220-230: The loop in FarmersWorkbenchScreen that builds keys for
aggregatedRequirements and holderToIngredient uses the deprecated
Ingredient.items(); replace that call with ing.getValues() (which returns the
HolderSet<Item>) and call toList() on it to produce the List<Holder<Item>> key,
then remove the `@SuppressWarnings`("deprecation") annotation; update references
in the same block where aggregatedRequirements.put(...) and
holderToIngredient.putIfAbsent(...) are used so the logic remains unchanged but
the deprecated method is eliminated.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java`:
- Around line 164-175: The hasFuel() method currently calls
this.getItem(Constants.FUEL_SLOT) which can throw IndexOutOfBoundsException
because FarmersWorkbenchMenu uses EMPTY_SIZE = 0 (slotless); change hasFuel() in
FarmersWorkbenchEntity to avoid accessing inventory slots (do not call getItem
with Constants.FUEL_SLOT) — since this workbench doesn't use fuel simply return
true (or otherwise return a constant boolean) and remove or leave commented the
fuel-slot checks; ensure the method signature remains protected boolean
hasFuel() and no inventory access is performed.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Line 139: In BuildersWorkbenchScreen.handleCraftRequest remove the debug
System.out.println calls (e.g., "Persistent Selection Success: " +
results.get(0)) and replace them with proper logging or delete them entirely;
locate the println occurrences inside the handleCraftRequest method and either
call the class's logger (or add an SLF4J/Minecraft logger) with an appropriate
log level (info/debug) and contextual message, or remove the lines if no runtime
logging is desired.
- Around line 249-281: The hasIngredientAmount method contains multiple
System.out.println debug statements that should be removed (or replaced with a
proper logger at debug level); open the hasIngredientAmount method and delete
the println calls that announce "DEBUG: Searching inventory...", "DEBUG: Found
... in nearby networked list...", "DEBUG: Requirement MET...", and "DEBUG:
FAILED..." or replace them with calls to a class logger (e.g.,
LOGGER.debug(...)) guarded by the logger's isDebugEnabled check; the relevant
symbols to edit are the hasIngredientAmount(Inventory, Ingredient, int) method,
the menu field check (this.menu instanceof AbstractWorkbenchContainerMenu
workbenchMenu), and use workbenchMenu.getNetworkedNearbyItems() as-is.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 130-146: In handleCraftRequest(int amount) remove the two
System.out.println debug prints and replace them with proper logger calls (use
MineTale.LOGGER.debug(...)) or delete them entirely; specifically update the
println in the success branch that prints "Persistent Selection Success: " +
results.get(0) and the failure branch printing "Request failed: No recipe was
ever selected!" to use MineTale.LOGGER.debug with the same or clearer messages
so debugging remains available without using System.out.
- Around line 249-281: The hasIngredientAmount method contains multiple
System.out.println debug statements; remove those println calls (or replace them
with the appropriate logger debug calls if a logging facility exists) inside
hasIngredientAmount and the block that iterates
workbenchMenu.getNetworkedNearbyItems so the method no longer prints to stdout
in production. Ensure only meaningful logging via the project's logger (e.g.,
LOGGER.debug(...) if available) is used, and leave the logic and return behavior
of hasIngredientAmount and use of workbenchMenu.getNetworkedNearbyItems
unchanged.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Around line 139-145: Replace the debug System.out.println calls in
WorkbenchWorkbenchScreen (the printlns around the persistent selection/recipe
request logic and the other occurrences) with a proper logging call or remove
them entirely; locate the uses of System.out.println in the class (e.g., the
message "Persistent Selection Success: " and "Request failed: No recipe was ever
selected!" and the other printlns later) and either delete them or swap them for
a logger with an appropriate level (e.g., LOGGER.debug or LOGGER.trace) behind a
configurable log level so debug output can be enabled in development but omitted
in production.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java`:
- Around line 28-62: The ContainerData implementation in FarmersWorkbenchEntity
named data exposes getCount() == 4 while get() always returns 0 and
FarmersWorkbenchMenu uses DATA_SIZE = 0; update the entity to match actual usage
by either removing the unused ContainerData field or changing its getCount() to
return 0 (and keep get/set as no-ops) so the workbench is slotless and
consistent with FarmersWorkbenchMenu.DATA_SIZE; modify the data field in
FarmersWorkbenchEntity (or adjust FarmersWorkbenchMenu.DATA_SIZE if intentional)
to ensure the two symbols ContainerData data and FarmersWorkbenchMenu.DATA_SIZE
agree.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/ArmorersWorkbenchMenu.java`:
- Around line 91-93: Remove the debug System.out.println in
ArmorersWorkbenchMenu (the block checking nearbyItems and
playerInventory.player.level().isClientSide()) and either delete it or replace
it with the project's logging framework (e.g., use a LOGGER.debug/trace call) so
it does not print to stdout in production; locate the statement inside the
ArmorersWorkbenchMenu class and update that method accordingly to use the
established logger or remove the line entirely.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/BuildersWorkbenchMenu.java`:
- Around line 91-93: In BuildersWorkbenchMenu remove the temporary
System.out.println debug statement that prints nearbyItems size (the line inside
the if checking nearbyItems.isEmpty() &&
this.playerInventory.player.level().isClientSide()); either delete it outright
or replace it with a proper logger.debug call consistent with
FarmersWorkbenchMenu's approach so production won't be spammed—ensure the
nearbyItems/level check remains unchanged and only the Console print is removed
or converted to debug logging.
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; either delete the line or replace it with a
proper logger.debug call (using the project's logging framework) and guard it
with the existing condition (nearbyItems.isEmpty() check and
playerInventory.player.level().isClientSide()). Update references in
FarmersWorkbenchMenu so the log uses a logger instance (e.g., a private static
final Logger) and a clear message like "Recipe Book accounting for {} stacks"
with the nearbyItems.size() parameter.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/WorkbenchWorkbenchMenu.java`:
- Around line 90-92: Remove the debug System.out.println call in
WorkbenchWorkbenchMenu (the block checking nearbyItems and
playerInventory.player.level().isClientSide()) and either delete it entirely or
replace it with the project's logging framework (e.g., use a class logger such
as LOGGER.debug/trace with a clear message and include nearbyItems.size());
ensure you reference WorkbenchWorkbenchMenu and the nearbyItems check so the
change targets that exact debug print.
- Around line 77-97: Both WorkbenchWorkbenchMenu.fillCraftSlotsStackedContents
and ArmorersWorkbenchMenu implement the same logic (account playerInventory,
container slots, and networked nearby items); extract this logic into a
protected method on AbstractWorkbenchContainerMenu (e.g., protected void
fillCraftSlotsStackedContents(StackedItemContents contents)) and move the body
there: call this.playerInventory.fillStackedContents(contents), iterate
this.container.getContainerSize() / this.container.getItem(i), then
getNetworkedNearbyItems() and accountStack for each. Make sure
AbstractWorkbenchContainerMenu exposes/provides accessors or protected fields
for playerInventory, container, and getNetworkedNearbyItems() (or make that
method protected in the base) so subclasses can call it, then replace the
duplicate implementations in WorkbenchWorkbenchMenu and ArmorersWorkbenchMenu
with a single call to super.fillCraftSlotsStackedContents(contents).
In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Around line 80-92: The block registration names for FARMERS_WORKBENCH_BLOCK
and BUILDERS_WORKBENCH_BLOCK currently use "farmers_workbench" and
"builders_workbench", causing inconsistent resource locations; update the
register(...) calls to use "farmers_workbench_block" and
"builders_workbench_block" respectively (keep the same constructors
FarmersWorkbench::new and BuildersWorkbench::new and existing
BlockBehaviour.Properties), and then ensure any corresponding resource keys
(textures/models/lang entries) are renamed to match the new registration IDs.
In `@src/main/java/com/tcm/MineTale/registry/ModRecipes.java`:
- Around line 43-48: The Javadoc on ModRecipes is outdated: update the
method/class comment that currently mentions only FURNACE_T1_TYPE, CAMPFIRE_TYPE
and WORKBENCH_TYPE to also document the additional registered recipe types and
serializers (ARMORER_TYPE, FARMER_TYPE, BUILDER_TYPE and their corresponding
serializer constants/registrations). Edit the Javadoc block above the
registering method in ModRecipes to list all six recipe types and briefly note
that each is registered into the game's registries, referencing the constants
FURNACE_T1_TYPE, CAMPFIRE_TYPE, WORKBENCH_TYPE, ARMORER_TYPE, FARMER_TYPE and
BUILDER_TYPE so the doc matches the actual registration behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
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/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/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/BuildersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/FarmersWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/BuildersWorkbenchEntity.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/BuildersWorkbenchMenu.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/block/workbenches/screen/BuildersWorkbenchScreen.java
Show resolved
Hide resolved
src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/entity/FarmersWorkbenchEntity.java
Show resolved
Hide resolved
The-Code-Monkey
changed the base branch from
1.21
to
feat/add-farmers-workbench
The-Code-Monkey
merged commit e9a6514
into
feat/add-farmers-workbench
3 of 4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.