Implement ingredient list rendering in workbench screen#59
Merged
The-Code-Monkey merged 7 commits into1.21from Mar 12, 2026
Merged
Implement ingredient list rendering in workbench screen#59The-Code-Monkey merged 7 commits into1.21from
The-Code-Monkey merged 7 commits into1.21from
Conversation
Added ingredient rendering logic to BuildersWorkbenchScreen.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughA Most Improbable Augmentation to the Builders' WorkshopHail, wayfarer of code and custodian of improbable fixes! The workbench screens have been touched by both elven craft and bureaucratic absurdity: the UI now shows the recipe ingredients list, computed from grouped requirements and the player's available stores (both pocket and networked chests), and several workbench screens now inherit from a new shared base class to render that list. The Grand Changes
Behavioural Notes
In short: the workbench now proclaims, with the calm assuredness of an ancient mariner and the flourish of an elven minstrel, what materials are required and whether you possess them—from coinpurse to humming distant chest—rendered neatly for the crafting pilgrim. WalkthroughThe workbench screens (Builders, Armorers, Farmers, Workbench) now inherit from a new ModAbstractContainerScreen which provides renderIngredientList(...); each screen's render() was updated to call this when a recipe is selected. ModAbstractContainerScreen implements aggregated ingredient counting and ingredient-list rendering logic. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as BuildersWorkbenchScreen
participant Base as ModAbstractContainerScreen
participant Player as PlayerInventory
participant Network as NearbyNetworkedStorage
participant Renderer as GuiGraphics
UI->>Base: render(selectedRecipe, mouseX, mouseY)
Base->>Player: getAvailableCount(ingredient)
Base->>Network: getAvailableCount(ingredient)
Player-->>Base: counts
Network-->>Base: counts
Base->>Base: aggregate counts, choose display variant, compute progress
Base->>Renderer: renderIngredientList(rows, icons, progress, tooltips)
Renderer-->>UI: ingredient list drawn
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java (1)
204-218: Keep ingredient rows in a steady constellation.Using
HashMapforaggregated/holderToIngmeans row order is unspecified, so list ordering may drift between renders.LinkedHashMapkeeps insertion order and steadies the UI.Proposed refactor
-import java.util.HashMap; +import java.util.HashMap; +import java.util.LinkedHashMap; ... - Map<List<Holder<Item>>, Integer> aggregated = new HashMap<>(); - Map<List<Holder<Item>>, Ingredient> holderToIng = new HashMap<>(); + Map<List<Holder<Item>>, Integer> aggregated = new LinkedHashMap<>(); + Map<List<Holder<Item>>, Ingredient> holderToIng = new LinkedHashMap<>();🤖 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 204 - 218, The UI row order is unstable because aggregated and holderToIng are created as HashMap (unspecified iteration order); change their declarations/instantiations in BuildersWorkbenchScreen to use LinkedHashMap instead (e.g., replace new HashMap<>() with new LinkedHashMap<>() for Map<List<Holder<Item>>, Integer> aggregated and Map<List<Holder<Item>>, Ingredient> holderToIng) so insertion order from reqs.get() is preserved when iterating the aggregated.entrySet().
🤖 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 250-252: The loop in BuildersWorkbenchScreen currently accesses
the non-existent/mapped field this.minecraft.player.getInventory().items; change
it to iterate through the inventory via the public API instead—either loop over
this.minecraft.player.getInventory().getNonEquipmentItems() and apply
ingredient.test(stack) to accumulate stack.getCount(), or use getContainerSize()
and getItem(i) on this.minecraft.player.getInventory() to check each slot;
update the code references in BuildersWorkbenchScreen to remove direct field
access and use one of these public methods.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Around line 204-218: The UI row order is unstable because aggregated and
holderToIng are created as HashMap (unspecified iteration order); change their
declarations/instantiations in BuildersWorkbenchScreen to use LinkedHashMap
instead (e.g., replace new HashMap<>() with new LinkedHashMap<>() for
Map<List<Holder<Item>>, Integer> aggregated and Map<List<Holder<Item>>,
Ingredient> holderToIng) so insertion order from reqs.get() is preserved when
iterating the aggregated.entrySet().
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
Outdated
Show resolved
Hide resolved
Contributor
|
Note Docstrings generation - SKIPPED |
coderabbitai bot
added a commit
that referenced
this pull request
Feb 28, 2026
Docstrings generation was requested by @The-Code-Monkey. * #59 (comment) The following files were modified: * `src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java (1)
203-209: Preserve the ingredient row order, lest the UI go full chaos goblin.
HashMapgives thee no ordering guarantee, so these aggregated rows can drift away from recipe order and feel random.LinkedHashMapkeeps the first-seen order stable with zero extra drama.Suggested tidy fix
-import java.util.HashMap; +import java.util.HashMap; +import java.util.LinkedHashMap; @@ - Map<List<Holder<Item>>, Integer> aggregated = new HashMap<>(); - Map<List<Holder<Item>>, Ingredient> holderToIng = new HashMap<>(); + Map<List<Holder<Item>>, Integer> aggregated = new LinkedHashMap<>(); + Map<List<Holder<Item>>, Ingredient> holderToIng = new LinkedHashMap<>();Also applies to: 217-243
🤖 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 203 - 209, The aggregation uses HashMap which loses insertion order; change the maps declared as aggregated and holderToIng from HashMap to LinkedHashMap so the ingredient rows preserve the original iteration order from reqs.get() (also update the same pattern used in the subsequent block around the code handling lines 217-243), keeping all existing keys/values logic intact but relying on LinkedHashMap to maintain first-seen order.
🤖 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 246-264: The getAvailableCount helper currently sums the player
inventory and workbenchMenu.getNetworkedNearbyItems but omits the workbench’s
own slots, so update getAvailableCount to also iterate the workbench container
when menu is an AbstractWorkbenchContainerMenu (the same place you check
workbenchMenu.getNetworkedNearbyItems) and add any matching stack.getCount()
found there; use safe access (getContainerSize()/getItem(i) or the menu’s
provided container accessors) to read those slots so items already placed in the
bench are counted.
---
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Around line 203-209: The aggregation uses HashMap which loses insertion order;
change the maps declared as aggregated and holderToIng from HashMap to
LinkedHashMap so the ingredient rows preserve the original iteration order from
reqs.get() (also update the same pattern used in the subsequent block around the
code handling lines 217-243), keeping all existing keys/values logic intact but
relying on LinkedHashMap to maintain first-seen order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6079966-f17c-4fbe-aee6-52d12bff2553
📒 Files selected for processing (1)
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java
Outdated
Show resolved
Hide resolved
…m/CodeMonkeysMods/MineTale into Andy/test-inventory-display-items
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (8)
src/client/java/com/tcm/MineTale/block/workbenches/screen/ModAbstractContainerScreen.java (3)
81-86: Minor inconsistency: the player inventory loop lacks theisEmptycheck present in the networked items loop.Don't panic! This won't cause a runtime issue since
ingredient.test(emptyStack)returnsfalse, but for consistency with lines 90-91 (where you check!stack.isEmpty()), you might want to add the same guard here. It's giving "two different hobbits wrote these loops" energy, which is low-key slay but also slightly suss.♻️ Add isEmpty check for consistency
for (int i = 0; i < inv.getContainerSize(); i++) { ItemStack stack = inv.getItem(i); - if (ingredient.test(stack)) { + if (!stack.isEmpty() && ingredient.test(stack)) { found += stack.getCount(); } }🤖 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/ModAbstractContainerScreen.java` around lines 81 - 86, The player-inventory loop in ModAbstractContainerScreen uses inv.getItem(i) and ingredient.test(stack) but lacks the !stack.isEmpty() guard used elsewhere; update the loop that iterates over inv (using inv.getContainerSize(), inv.getItem(i), and variable stack) to first check !stack.isEmpty() before calling ingredient.test(stack) and adding to found so it matches the networked-items loop's behavior and keeps consistency.
57-60: Consider using tick-based timing instead of wall-clock time, friend.By the beard of Gandalf and the infinite improbability of a decent time source, using
System.currentTimeMillis()every render frame is giving me major "not vibing" energy. It works, but in Minecraft's render loop,deltaor a tick counter would be more idiomatic and avoid potential issues with system clock shenanigans (daylight savings, NTP adjustments, etc.). That said, this won't break anything—it's just a bit unchill.🤖 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/ModAbstractContainerScreen.java` around lines 57 - 60, The render uses System.currentTimeMillis() to pick a rotating display item (variants, time, displayStack), which is fragile; replace wall-clock usage with a game tick-based value (e.g., a tick counter or world time) so selection is driven by Minecraft ticks or a screen tick field instead of System.currentTimeMillis(); locate where variants is created (in ModAbstractContainerScreen) and compute the index using something like clientWorld.getTime() or this.screenTick/delta-based counter to pick displayStack deterministically per tick.
89-93: Theinstanceofcheck is redundant given the generic bound, but no cap, it's harmless.Since
T extends AbstractWorkbenchContainerMenu,this.menuis guaranteed to be anAbstractWorkbenchContainerMenu. The check will always pass. You could simplify this, but it's not wrong—just a bit like asking a hobbit if they enjoy second breakfast.♻️ Optional simplification
- if (this.menu instanceof AbstractWorkbenchContainerMenu workbenchMenu) { - for (ItemStack stack : workbenchMenu.getNetworkedNearbyItems()) { - if (ingredient.test(stack)) found += stack.getCount(); - } + for (ItemStack stack : this.menu.getNetworkedNearbyItems()) { + if (ingredient.test(stack)) found += stack.getCount(); }🤖 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/ModAbstractContainerScreen.java` around lines 89 - 93, The instanceof guard in ModAbstractContainerScreen is unnecessary because the class is already parameterized with T extends AbstractWorkbenchContainerMenu, so this.menu is always an AbstractWorkbenchContainerMenu; simplify by removing the conditional and directly cast or use (AbstractWorkbenchContainerMenu) this.menu (or reference this.menu as the generic type) to call getNetworkedNearbyItems and iterate over stacks, using ingredient.test(stack) to accumulate found—update the block around getNetworkedNearbyItems/ingredient.test to eliminate the redundant instanceof and associated pattern variable.src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java (1)
36-36: The@NonNullannotation is only added to this screen, not the others.This import and its usage on the
rendermethod (line 162) creates an inconsistency withFarmersWorkbenchScreen,ArmorersWorkbenchScreen, andWorkbenchWorkbenchScreen, which don't use this annotation. It's giving "one hobbit decided to be extra" vibes. Either apply it consistently across all screens or remove it here for uniformity.🤖 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 36, Remove the inconsistent jspecify NonNull usage from BuildersWorkbenchScreen by deleting the import org.jspecify.annotations.NonNull and removing the `@NonNull` annotation on the render method; alternatively, if you prefer to enforce NonNull across screens, add the same import and `@NonNull` annotation to the render methods in FarmersWorkbenchScreen, ArmorersWorkbenchScreen, and WorkbenchWorkbenchScreen so all screen classes (BuildersWorkbenchScreen.render, FarmersWorkbenchScreen.render, ArmorersWorkbenchScreen.render, WorkbenchWorkbenchScreen.render) are consistent.src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java (2)
221-259: ThecanCraftandhasIngredientAmountmethods are duplicated across all workbench screens.By the infinite improbability of copy-paste, this exact logic appears in
BuildersWorkbenchScreen,ArmorersWorkbenchScreen,WorkbenchWorkbenchScreen, and here. This is giving "Don't Repeat Yourself went on holiday" vibes. Since you've already createdModAbstractContainerScreenas a shared base, consider moving these methods there to reduce duplication.🤖 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 221 - 259, The canCraft(...) and hasIngredientAmount(...) implementations are duplicated across multiple workbench screens (FarmersWorkbenchScreen, BuildersWorkbenchScreen, ArmorersWorkbenchScreen, WorkbenchWorkbenchScreen); move these methods into the shared base class ModAbstractContainerScreen as protected instance methods (preserving signatures: canCraft(Player player, RecipeDisplayEntry entry, int craftCount) and hasIngredientAmount(Inventory inv, Ingredient ing, int requiredAmount)) so all subclasses can reuse them, update imports and visibility, remove the duplicate methods from each concrete screen class, and ensure subclasses call the base implementation where needed (adjust any references to package-private members or fields to be accessible from the base class).
271-301: Remove or replace debugSystem.out.printlnstatements before shipping.These debug prints are lowkey suss for production code. They'll spam the console every frame when a recipe is selected. Consider using a proper logging framework with appropriate log levels, or remove them entirely now that the feature appears to be working.
♻️ Remove debug statements
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 271 - 301, In FarmersWorkbenchScreen, remove all System.out.println debug calls (the prints around the inventory/network search and result checks) and either delete them or replace them with calls to a proper logger at an appropriate log level (e.g., LOGGER.debug or LOGGER.trace) declared in the class; update the code paths that reference inventory, the ingredient test loop, and the block that casts this.menu to AbstractWorkbenchContainerMenu/getNetworkedNearbyItems so the behavior is unchanged aside from removing console spam. Ensure any new logger is a private static final Logger in FarmersWorkbenchScreen and import/use the project's logging facility rather than System.out.src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java (1)
140-146: DebugSystem.out.printlnstatements should be removed.Same as the other screens—these debug prints in
handleCraftRequestwill clutter the console. Consider removing them or using proper logging.🤖 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 140 - 146, Remove the debug System.out.println calls from handleCraftRequest in WorkbenchWorkbenchScreen.java: eliminate the two prints ("Persistent Selection Success: " + results.get(0)) and "Request failed: No recipe was ever selected!" or replace them with proper logger calls if you need runtime visibility; keep the ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)) behavior unchanged and ensure no println remains around that logic.src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java (1)
3-3: Wildcard importjava.util.*is generally discouraged.By the standards of the Galactic Code Style Council, explicit imports are preferred for clarity and to avoid potential namespace conflicts. It's not wrong—just a bit like taking the scenic route through the Restaurant at the End of the Universe when you could've teleported directly. Low priority, but worth noting.
♻️ Consider reverting to explicit imports
-import java.util.*; +import java.util.ArrayList; +import java.util.List;🤖 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` at line 3, Replace the wildcard import "import java.util.*;" with explicit imports for only the java.util types actually used by AbstractWorkbenchContainerMenu (e.g., import java.util.List;, import java.util.Map;, import java.util.Optional;, import java.util.UUID;, import java.util.Collections; — adjust to the real types present). Update the import list manually or run your IDE's "optimize/organize imports" on the AbstractWorkbenchContainerMenu class to produce explicit imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/BuildersWorkbenchScreen.java`:
- Line 36: Remove the inconsistent jspecify NonNull usage from
BuildersWorkbenchScreen by deleting the import org.jspecify.annotations.NonNull
and removing the `@NonNull` annotation on the render method; alternatively, if you
prefer to enforce NonNull across screens, add the same import and `@NonNull`
annotation to the render methods in FarmersWorkbenchScreen,
ArmorersWorkbenchScreen, and WorkbenchWorkbenchScreen so all screen classes
(BuildersWorkbenchScreen.render, FarmersWorkbenchScreen.render,
ArmorersWorkbenchScreen.render, WorkbenchWorkbenchScreen.render) are consistent.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/FarmersWorkbenchScreen.java`:
- Around line 221-259: The canCraft(...) and hasIngredientAmount(...)
implementations are duplicated across multiple workbench screens
(FarmersWorkbenchScreen, BuildersWorkbenchScreen, ArmorersWorkbenchScreen,
WorkbenchWorkbenchScreen); move these methods into the shared base class
ModAbstractContainerScreen as protected instance methods (preserving signatures:
canCraft(Player player, RecipeDisplayEntry entry, int craftCount) and
hasIngredientAmount(Inventory inv, Ingredient ing, int requiredAmount)) so all
subclasses can reuse them, update imports and visibility, remove the duplicate
methods from each concrete screen class, and ensure subclasses call the base
implementation where needed (adjust any references to package-private members or
fields to be accessible from the base class).
- Around line 271-301: In FarmersWorkbenchScreen, remove all System.out.println
debug calls (the prints around the inventory/network search and result checks)
and either delete them or replace them with calls to a proper logger at an
appropriate log level (e.g., LOGGER.debug or LOGGER.trace) declared in the
class; update the code paths that reference inventory, the ingredient test loop,
and the block that casts this.menu to
AbstractWorkbenchContainerMenu/getNetworkedNearbyItems so the behavior is
unchanged aside from removing console spam. Ensure any new logger is a private
static final Logger in FarmersWorkbenchScreen and import/use the project's
logging facility rather than System.out.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/ModAbstractContainerScreen.java`:
- Around line 81-86: The player-inventory loop in ModAbstractContainerScreen
uses inv.getItem(i) and ingredient.test(stack) but lacks the !stack.isEmpty()
guard used elsewhere; update the loop that iterates over inv (using
inv.getContainerSize(), inv.getItem(i), and variable stack) to first check
!stack.isEmpty() before calling ingredient.test(stack) and adding to found so it
matches the networked-items loop's behavior and keeps consistency.
- Around line 57-60: The render uses System.currentTimeMillis() to pick a
rotating display item (variants, time, displayStack), which is fragile; replace
wall-clock usage with a game tick-based value (e.g., a tick counter or world
time) so selection is driven by Minecraft ticks or a screen tick field instead
of System.currentTimeMillis(); locate where variants is created (in
ModAbstractContainerScreen) and compute the index using something like
clientWorld.getTime() or this.screenTick/delta-based counter to pick
displayStack deterministically per tick.
- Around line 89-93: The instanceof guard in ModAbstractContainerScreen is
unnecessary because the class is already parameterized with T extends
AbstractWorkbenchContainerMenu, so this.menu is always an
AbstractWorkbenchContainerMenu; simplify by removing the conditional and
directly cast or use (AbstractWorkbenchContainerMenu) this.menu (or reference
this.menu as the generic type) to call getNetworkedNearbyItems and iterate over
stacks, using ingredient.test(stack) to accumulate found—update the block around
getNetworkedNearbyItems/ingredient.test to eliminate the redundant instanceof
and associated pattern variable.
In
`@src/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.java`:
- Around line 140-146: Remove the debug System.out.println calls from
handleCraftRequest in WorkbenchWorkbenchScreen.java: eliminate the two prints
("Persistent Selection Success: " + results.get(0)) and "Request failed: No
recipe was ever selected!" or replace them with proper logger calls if you need
runtime visibility; keep the ClientPlayNetworking.send(new
CraftRequestPayload(results.get(0), amount)) behavior unchanged and ensure no
println remains around that logic.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java`:
- Line 3: Replace the wildcard import "import java.util.*;" with explicit
imports for only the java.util types actually used by
AbstractWorkbenchContainerMenu (e.g., import java.util.List;, import
java.util.Map;, import java.util.Optional;, import java.util.UUID;, import
java.util.Collections; — adjust to the real types present). Update the import
list manually or run your IDE's "optimize/organize imports" on the
AbstractWorkbenchContainerMenu class to produce explicit imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e7151f5-2bc8-4490-8252-8ac711f6baf1
📒 Files selected for processing (6)
src/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/ModAbstractContainerScreen.javasrc/client/java/com/tcm/MineTale/block/workbenches/screen/WorkbenchWorkbenchScreen.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AbstractWorkbenchContainerMenu.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Line 7: The workflow currently triggers only on push ("on: [push]") so CI
won't run for pull requests; update the workflow trigger to include pull_request
as well (e.g., change the "on" declaration to include both push and
pull_request) so the CI runs for PRs and push events; locate the "on: [push]"
entry in the workflow file and replace it with an "on" configuration that
includes pull_request alongside push.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee3afe6b-5411-43c8-afd4-f644251f33c6
📒 Files selected for processing (1)
.github/workflows/build.yml
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.
Added ingredient rendering logic to BuildersWorkbenchScreen.