Skip to content

Fixed some of #66's problems#68

Merged
The-Code-Monkey merged 4 commits into1.21from
Blacksmith-an-stuff3
Mar 5, 2026
Merged

Fixed some of #66's problems#68
The-Code-Monkey merged 4 commits into1.21from
Blacksmith-an-stuff3

Conversation

@AnyaPizza
Copy link
Collaborator

No description provided.

@AnyaPizza AnyaPizza self-assigned this Mar 4, 2026
@AnyaPizza AnyaPizza added the enhancement New feature or request label Mar 4, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d1f7e081-3849-40ab-9096-5f6ede5f5ead

📥 Commits

Reviewing files that changed from the base of the PR and between 3ebe2bf and a978de2.

📒 Files selected for processing (1)
  • src/main/java/com/tcm/MineTale/registry/ModBlocks.java

📝 Walkthrough

The Alchemist's Workbench: A Most Improbable Undertaking

Behold! In these bewildering hours a guild of code-smiths hath set forth to mend sundry vexations left by an earlier expedition. From their labours emerges the Alchemist's Workbench and a plenitude of supporting artefacts — strange, useful, and mildly explosive.

The Grand Edifice: The Alchemist's Workbench Materialises

  • New block: AlchemistsWorkbench (IS_WIDE = true, IS_TALL = false). Renders as a model, uses a simple full-block shape, and binds a ticker to its block entity.
  • New block entity: AlchemistsWorkbenchEntity. Persists tier, scanRadius and inventory via Codec, exposes ContainerData, supports nearby-item scanning, save/load persistence, hasFuel check and returns ModRecipes.ALCHEMISTS_TYPE for recipe lookup.

The Apparatus of Interaction: Menu and Screen

  • New menu: AlchemistsWorkbenchMenu. Client & server constructors, binds optional block entity, fills crafting stacked contents from player inventory + nearby items, implements RecipeBookType integration and returns an empty WorkbenchRecipeInput (no internal input slots).
  • New screen: AlchemistsWorkbenchScreen. Extends AbstractRecipeBookScreen; integrates recipe book (ALCHEMISTS_SEARCH), computes toggle/button positions, initialises three craft buttons (Craft, x10, All), stores last selected recipe id, determines canCraft by aggregating ingredient requirements across player inventory and networked nearby items, sends CraftRequestPayloads to server, and renders background/texture & tooltips. Includes debug logging.

Items and Balance

  • New items registered: EMPTY_POTION_BOTTLE, ANTIDOTE, BOOM_POWDER, POPBERRY_BOMB.
  • New crude items: CRUDE_MACE, CRUDE_BUILDERS_HAMMER, CRUDE_HATCHET, CRUDE_BATTLEAXE, CRUDE_DAGGERS, CRUDE_SHORTBOW, CRUDE_SWORD, CRUDE_LONGSWORD.
  • New copper items: COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE, COPPER_LONGSWORD, COPPER_SHORTBOW.
  • Balance tweak: COPPER_MACE — rarity EPIC → UNCOMMON; durability 500 → 300; enchantability 15 → 10.

Recipes — Many a Recipe Awoken

  • New Alchemist recipe type & serializer registered (ALCHEMISTS_TYPE / ALCHEMISTS_SERIALIZER) and RecipeBookCategory (ALCHEMISTS_SEARCH).
  • Alchemist recipes added: Antidote (uses VENOM_SAC, EMPTY_POTION_BOTTLE, PLANT_FIBER, ESSENCE_OF_LIFE) and Popberry Bomb (WILD_BERRY, BOOM_POWDER, PLANT_FIBER).
  • Numerous previously commented recipes are now active:
    • Blacksmith recipes: copper_battleaxe, copper_daggers, crude_longsword, copper_longsword, copper_shortbow, copper_sword.
    • Workbench recipes: many crude and copper tools/weapon recipes (crude_builders_hammer, copper_pickaxe, stone_pickaxe, crude_hatchet, copper_hatchet, crude_battleaxe, crude_mace, crude_daggers, crude_shortbow, crude_sword, etc.).
  • Recipe IDs normalised to lowercase where applicable; unlock conditions and categories set.

Data Generation, Models, Loot & Tags

  • Datagen: ModLangProvider adds translations for the new block and items; ModModelProvider adds flat handheld item models for potion items; ModLootTableProvider adds loot rules for the alchemist workbench mirroring armorer behaviour.
  • Tags: ModBlockTagProvider adds ALCHEMISTS_WORKBENCH_BLOCK to MINEABLE_WITH_PICKAXE.

Registries & Wiring

  • ModBlocks: registers ALCHEMISTS_WORKBENCH_BLOCK (stone sound, registers BlockItem).
  • ModBlockEntities: registers ALCHEMISTS_WORKBENCH_BE.
  • ModMenuTypes: registers ALCHEMISTS_WORKBENCH_MENU.
  • ModRecipes & ModRecipeDisplay: add the alchemists recipe type/serializer and recipe display type/category.

Notes on Code Health & Behaviour

  • Several previously commented-out datagen recipe templates have been enabled (large diffs in recipe generation files).
  • New client-side screen includes debug print statements and a persistent last-known recipe selection for button enablement.
  • No public API method signature removals were detected; multiple new public classes/fields added (workbench, entity, menu, screen, items, recipe types, menu types).

Thus the realm gains an Alchemist's Workbench, new implements, recipes, and the curious ability to craft both cures and combustibles. Whether this soothes the universe's great displeasure remains, per common wisdom, approximately forty-two percent uncertain.

Walkthrough

Adds a complete Alchemists Workbench feature: block, block entity, menu, screen, recipe type/serializer, recipe-book category, items, data-generation (recipes, models, loot, translations), and client registration wiring for the new screen and menu.

Changes

Cohort / File(s) Summary
Core block & entity
src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java, src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
New AlchemistsWorkbench block and AlchemistsWorkbenchEntity: ticker binding, codec, shape/render, persistence (tier, scanRadius, inventory), fuel check, recipe type binding, and menu creation.
Client UI & Screen
src/client/java/com/tcm/MineTale/MineTaleClient.java, src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.java
Registers ALCHEMISTS_WORKBENCH menu screen; adds AlchemistsWorkbenchScreen with recipe-book integration, craft buttons (1/10/all), ingredient feasibility checks against player + nearby items, and networked craft request dispatch.
Menu & registry wiring
src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java, src/main/java/com/tcm/MineTale/registry/ModMenuTypes.java, src/main/java/com/tcm/MineTale/registry/ModBlockEntities.java, src/main/java/com/tcm/MineTale/registry/ModBlocks.java
Adds AlchemistsWorkbenchMenu, registers menu type, block entity type and block (alchemists_workbench) and links them into existing registries.
Recipe types & display
src/main/java/com/tcm/MineTale/registry/ModRecipes.java, src/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.java
Introduces ALCHEMISTS_TYPE recipe type and serializer, plus ALCHEMISTS_SEARCH recipe book category/display type.
Data generation — recipes
src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.java, .../BlacksmithRecipes.java, .../WorkbenchRecipes.java, .../ForgeRecipes.java
Activates and adds multiple workbench recipes (alchemist, blacksmith, workbench sets), renames/uses ALCHEMISTS serializer/type where applicable; minor non-functional comment tweaks in ForgeRecipes.
Data generation — support
src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java, .../ModLootTableProvider.java, .../ModLangProvider.java, .../ModModelProvider.java
Adds workbench block to mineable tag, loot table entry for block, translations for new block/items, and flat item models for new alchemy items.
Items registry
src/main/java/com/tcm/MineTale/registry/ModItems.java
Registers new alchemical items (Empty Potion Bottle, Antidote, Boom Powder, Popberry Bomb) and many crude/copper weapon/tool variants; adjusts COPPER_MACE stats.
Recipe datatypes & display wiring
src/client/java/com/tcm/MineTale/datagen/..., src/main/java/com/tcm/MineTale/registry/...
Various reference updates to use plural/specific ALCHEMISTS constants and ensure recipe-book/category mapping aligns with new type. (Spread across recipe datagen and registry files.)

Sequence Diagram(s)

sequenceDiagram
    participant Player
    participant Client as AlchemistsWorkbenchScreen
    participant RecipeBook as MineTaleRecipeBookComponent
    participant Inventory as PlayerInventory+Nearby
    participant Server as AlchemistsWorkbenchEntity

    Player->>Client: Open/workbench view
    Client->>RecipeBook: Query & render selected recipe
    RecipeBook-->>Client: SelectedRecipeID
    Client->>Inventory: Check ingredient availability (player + nearby)
    Inventory-->>Client: IngredientCounts
    alt Ingredients sufficient
        Client->>Client: Enable craft buttons
        Player->>Client: Click craft (1/10/all)
        Client->>Server: Send CraftRequestPayload(amount, recipeId)
        Server->>Server: Validate & consume ingredients
        Server-->>Player: Deliver crafted item(s)
    else Not sufficient
        Client->>Client: Disable craft buttons
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • AnyaPizza

Poem

In dim-lit labs where bubbly potions gleam, 🧪
A workbench spawns — the alchemist's big dream.
Recipes hum, buttons blink, packets zip with glee,
Inventory spies nearby stacks like sos no cap, gee —
Craft on, brave coder, ye blessed chaos of tea. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive Title vaguely references issue #66 but lacks specificity about what changes were made or what problems were addressed. Revise title to explicitly describe the primary changes, e.g., 'Add Alchemists Workbench and new weapon recipes' or similar, making the changeset clear without requiring issue context.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relatedness to the changeset. Add a meaningful pull request description explaining the objectives, changes made, and how issue #66 problems are addressed.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.24% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Blacksmith-an-stuff3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/client/java/com/tcm/MineTale/datagen/recipes/BlacksmithRecipes.java (1)

95-103: ⚠️ Potential issue | 🔴 Critical

Items.COPPER_SWORD does not exist in vanilla Minecraft nor in your mod registry—this shall not compile, I'm afraid.

By the Valar and the Infinite Improbability Drive combined, this reference to Items.COPPER_SWORD is more non-existent than the second restaurant at the end of the universe. Vanilla Minecraft 1.21 does NOT include a copper sword, and your ModItems.java registration contains no COPPER_SWORD entry either. This code will fail to compile faster than you can say "Don't Panic."

Your mod registers COPPER_LONGSWORD, COPPER_MACE, COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE, and COPPER_SHORTBOW, but no COPPER_SWORD. This is giving major skill issue energy, no cap.

You must either:

  1. Register COPPER_SWORD in ModItems.java and use ModItems.COPPER_SWORD here, or
  2. Use ModItems.COPPER_LONGSWORD if that's the intended output for this recipe
🤖 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/BlacksmithRecipes.java`
around lines 95 - 103, The recipe references a nonexistent Items.COPPER_SWORD
causing compilation failure; update the WorkbenchRecipeBuilder invocation to use
a valid registered item (e.g., replace Items.COPPER_SWORD with
ModItems.COPPER_LONGSWORD) or alternatively add and register a
ModItems.COPPER_SWORD in your item registry and then reference
ModItems.COPPER_SWORD in the .output(...) call; ensure the chosen symbol matches
your registration names so WorkbenchRecipeBuilder.save(exporter, "copper_sword")
compiles.
🧹 Nitpick comments (4)
src/main/java/com/tcm/MineTale/registry/ModItems.java (1)

12-12: Unused import detected, my dear hobbit of the infinite improbability drive.

By the beard of Gandalf and the towel of Ford Prefect, this ModTags import has materialised here like a Vogon at a poetry reading—utterly purposeless and lowkey sus. It's giving "I forgot to delete this" energy, no cap.

The import appears to serve no function within this file whatsoever. Frfr, consider yeeting it into the void.

🧹 Proposed fix
-import com.tcm.MineTale.util.ModTags;
🤖 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/ModItems.java` at line 12, Remove the
unused import of ModTags from ModItems.java; locate the import line "import
com.tcm.MineTale.util.ModTags;" in the ModItems class and delete it so the file
no longer contains an unused import. Ensure there are no remaining references to
ModTags in ModItems before committing.
src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.java (1)

24-43: Tiny DRY rune: unlock key string is duplicated in both recipes.

Not broken, just a little chaos goblin for future edits. A local constant would keep this cleaner.

🛠️ Suggested tidy-up diff
 public class AlchemistRecipes {
+    private static final String HAS_ALCHEMISTS_WORKBENCH = "has_alchemists_workbench";
+
     /**
      * Registers alchemist workbench recipes and writes them to the provided exporter.
@@
-            .unlockedBy("has_alchemists_workbench", provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()))
+            .unlockedBy(HAS_ALCHEMISTS_WORKBENCH, provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()))
@@
-            .unlockedBy("has_alchemists_workbench", provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()))
+            .unlockedBy(HAS_ALCHEMISTS_WORKBENCH, provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()))
🤖 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/AlchemistRecipes.java`
around lines 24 - 43, Introduce a single constant for the unlock key and use it
in both recipe definitions: add a private static final String (e.g.,
HAS_ALCHEMISTS_WORKBENCH = "has_alchemists_workbench") in the AlchemistRecipes
class and replace the duplicated literal in the unlockedBy(...) calls on the
WorkbenchRecipeBuilder instances so both calls use the constant together with
provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()).
src/main/java/com/tcm/MineTale/registry/ModBlocks.java (1)

245-245: That TODO is still lurking like a goblin side-quest.

If you want, I can draft a tight follow-up issue template for making these beds functional so it doesn’t get lost in the void.

🤖 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` at line 245, The TODO
in ModBlocks.java flags decorative bed blocks as non-functional; either replace
that stray TODO with a linked follow-up issue or implement full bed behavior
now: locate the bed block declarations in ModBlocks (the bed Block instances and
their registerBlock/registerBlockItem calls), and either remove the TODO and
create a concise issue ticket describing required work, or implement
functionality by replacing the decorative Block with a BedBlock-derived class,
register a corresponding BlockEntityType if needed, add onUse interaction to
call player.sleep/handle waking logic on the server, set correct
Block.Properties (collision, shape, respawn point handling) and update any
loot/recipes/registration so the bed behaves like a normal sleep-capable bed.
Ensure ModBlocks registration code and any helper methods used there are updated
to register the new bed class or link to the created issue.
src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java (1)

26-60: Ah, the ContainerData – a most curious hitchhiker through the cosmic void of UI synchronization, yet it serves no actual purpose here. No cap, this is lowkey sus.

The getCount() returns 4 whilst get() always yields 0 and set() is but an empty vessel. As foretold in the scrolls of AlchemistsWorkbenchMenu (Context snippet 3), the menu declares DATA_SIZE = 0, meaning this ContainerData is never registered for synchronisation anyway. 'Tis a vestigial appendage, much like the second head of Zaphod Beeblebrox – present, but not particularly useful.

Consider either:

  1. Setting getCount() to return 0 to match the menu's expectations (consistency is the hitchhiker's towel of programming)
  2. Removing this entire ContainerData if the workbench truly requires no synced data
♻️ Proposed 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;
     }
 };
🤖 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/AlchemistsWorkbenchEntity.java`
around lines 26 - 60, The ContainerData field named "data" on
AlchemistsWorkbenchEntity is unused/mismatched (getCount() returns 4 while
AlchemistsWorkbenchMenu declares DATA_SIZE = 0); either remove the entire
ContainerData "data" field and any registration/usages, or change its getCount()
to return 0 and keep get()/set() as no-ops so it matches
AlchemistsWorkbenchMenu.DATA_SIZE; ensure any constructor or menu registration
that references "data" (or the ContainerData instance) is updated accordingly to
maintain consistency.
🤖 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/AlchemistsWorkbenchScreen.java`:
- Around line 139-145: Remove the stray System.out.println debug statements in
AlchemistsWorkbenchScreen (the ones printing "Persistent Selection Success" and
"Request failed: No recipe was ever selected!" and the other occurrences around
lines handling results and CraftRequestPayload/amount); either delete them or
replace them with proper logger calls (e.g., use the class LOGGER at appropriate
levels) or silence them entirely if not needed, ensuring the control flow
(ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)) and
return) remains unchanged.
- Around line 79-87: The recipe filtering in selectMatchingRecipes incorrectly
hardcodes ModRecipeDisplay.WORKBENCH_TYPE causing ALCHEMISTS_TYPE recipes to be
dropped; update selectMatchingRecipes in MineTaleRecipeBookComponent to stop
checking for the literal ModRecipeDisplay.WORKBENCH_TYPE (or explicitly allow
ModRecipeDisplay.ALCHEMISTS_TYPE) and instead rely on the recipe-level
display/type check (e.g., use recipe.type() and this.filterType or accept any
WorkbenchRecipeDisplay) so alchemists recipes registered with
ModRecipeDisplay.ALCHEMISTS_TYPE are not filtered out.
- Around line 229-230: This code uses the deprecated Ingredient#items() (see the
local variable "key" in AlchemistsWorkbenchScreen.java) — remove the suppression
and refactor the grouping/inventory-checking logic to stop enumerating
ingredient items; instead use the Ingredient.test(ItemStack) predicate to check
whether a given ItemStack matches the Ingredient when scanning inventory slots
(replace any loops that build "key" from ing.items().toList() with per-slot
Ingredient.test(stack) checks), and apply the same refactor to the other
workbench screens (Armorers..., Blacksmiths..., Builders..., Farmers...,
Furniture..., Workbench...) or add a short maintainer note if an unavoidable
exception is required.

In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java`:
- Around line 126-132: The Javadoc for AlchemistsWorkbenchEntity incorrectly
documents the return as ArmorersWorkbenchMenu; update the method comment (the
`@return` line in the createMenu/Server-side container docblock inside class
AlchemistsWorkbenchEntity) to reference AlchemistsWorkbenchMenu and adjust the
descriptive text to say it returns an AlchemistsWorkbenchMenu bound to this
workbench's inventory and synced data so the doc matches the actual returned
type.
- Around line 62-68: The Javadoc for the factory/constructor currently
references ArmorersWorkbenchEntity but this class is AlchemistsWorkbenchEntity;
update the Javadoc comment above the relevant constructor/factory
(AlchemistsWorkbenchEntity) to correctly say "Creates an
AlchemistsWorkbenchEntity" and keep the rest of the description about setting
scanRadius to 0.0 and tier to 1 and the `@param` tags (blockPos, blockState)
accurate; ensure any other occurrences in that comment block that mention
ArmorersWorkbenchEntity are replaced with AlchemistsWorkbenchEntity so the
documentation matches the class name.
- Around line 43-45: The Javadoc for AlchemistsWorkbenchEntity has `@param` tags
wrapped in backticks which breaks Javadoc parsing; open the Javadoc block in
class AlchemistsWorkbenchEntity (the method/comment referencing "index" and
"value") and replace the backticked `@param` lines with proper Javadoc `@param`
tags and brief descriptions (e.g., "@param index the data index to set" and
"@param value the value to assign (ignored)"), ensuring the Javadoc block uses
/** ... */ so documentation generators can pick up the parameters.

In `@src/main/java/com/tcm/MineTale/registry/ModItems.java`:
- Around line 181-186: The blacksmith recipe for the Copper Mace in
BlacksmithRecipes still costs 6 copper ingots, 10 logs, and 2 plant fibre even
though the item properties in ModItems (Rarity UNCOMMON, durability 300,
enchantable 10 on the Copper Mace item) were nerfed; update the Copper Mace
recipe entry in the BlacksmithRecipes class (the recipe that references the
Copper Mace/its Item reference) to reduce ingredient counts to reflect the
downgraded stats (for example lower copper ingot count and fewer logs/plant
fibre) or adjust to whatever new balance target you choose so crafting cost
matches the item’s new rarity and durability. Ensure you modify the same recipe
registration method/block that constructs the Copper Mace recipe (the entry
currently using 6 copper ingots, 10 logs, 2 plant fibre) and keep the recipe
shape/keys consistent with other blacksmith recipes.

---

Outside diff comments:
In `@src/client/java/com/tcm/MineTale/datagen/recipes/BlacksmithRecipes.java`:
- Around line 95-103: The recipe references a nonexistent Items.COPPER_SWORD
causing compilation failure; update the WorkbenchRecipeBuilder invocation to use
a valid registered item (e.g., replace Items.COPPER_SWORD with
ModItems.COPPER_LONGSWORD) or alternatively add and register a
ModItems.COPPER_SWORD in your item registry and then reference
ModItems.COPPER_SWORD in the .output(...) call; ensure the chosen symbol matches
your registration names so WorkbenchRecipeBuilder.save(exporter, "copper_sword")
compiles.

---

Nitpick comments:
In `@src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.java`:
- Around line 24-43: Introduce a single constant for the unlock key and use it
in both recipe definitions: add a private static final String (e.g.,
HAS_ALCHEMISTS_WORKBENCH = "has_alchemists_workbench") in the AlchemistRecipes
class and replace the duplicated literal in the unlockedBy(...) calls on the
WorkbenchRecipeBuilder instances so both calls use the constant together with
provider.has(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem()).

In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java`:
- Around line 26-60: The ContainerData field named "data" on
AlchemistsWorkbenchEntity is unused/mismatched (getCount() returns 4 while
AlchemistsWorkbenchMenu declares DATA_SIZE = 0); either remove the entire
ContainerData "data" field and any registration/usages, or change its getCount()
to return 0 and keep get()/set() as no-ops so it matches
AlchemistsWorkbenchMenu.DATA_SIZE; ensure any constructor or menu registration
that references "data" (or the ContainerData instance) is updated accordingly to
maintain consistency.

In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Line 245: The TODO in ModBlocks.java flags decorative bed blocks as
non-functional; either replace that stray TODO with a linked follow-up issue or
implement full bed behavior now: locate the bed block declarations in ModBlocks
(the bed Block instances and their registerBlock/registerBlockItem calls), and
either remove the TODO and create a concise issue ticket describing required
work, or implement functionality by replacing the decorative Block with a
BedBlock-derived class, register a corresponding BlockEntityType if needed, add
onUse interaction to call player.sleep/handle waking logic on the server, set
correct Block.Properties (collision, shape, respawn point handling) and update
any loot/recipes/registration so the bed behaves like a normal sleep-capable
bed. Ensure ModBlocks registration code and any helper methods used there are
updated to register the new bed class or link to the created issue.

In `@src/main/java/com/tcm/MineTale/registry/ModItems.java`:
- Line 12: Remove the unused import of ModTags from ModItems.java; locate the
import line "import com.tcm.MineTale.util.ModTags;" in the ModItems class and
delete it so the file no longer contains an unused import. Ensure there are no
remaining references to ModTags in ModItems before committing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 908fc268-e788-4e1e-8b2d-397f4a8a31d5

📥 Commits

Reviewing files that changed from the base of the PR and between 97a1923 and 3ebe2bf.

📒 Files selected for processing (19)
  • src/client/java/com/tcm/MineTale/MineTaleClient.java
  • src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.java
  • src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java
  • src/client/java/com/tcm/MineTale/datagen/ModLangProvider.java
  • src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java
  • src/client/java/com/tcm/MineTale/datagen/ModModelProvider.java
  • src/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.java
  • src/client/java/com/tcm/MineTale/datagen/recipes/BlacksmithRecipes.java
  • src/client/java/com/tcm/MineTale/datagen/recipes/ForgeRecipes.java
  • src/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.java
  • src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java
  • src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
  • src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java
  • src/main/java/com/tcm/MineTale/registry/ModBlockEntities.java
  • src/main/java/com/tcm/MineTale/registry/ModBlocks.java
  • src/main/java/com/tcm/MineTale/registry/ModItems.java
  • src/main/java/com/tcm/MineTale/registry/ModMenuTypes.java
  • src/main/java/com/tcm/MineTale/registry/ModRecipeDisplay.java
  • src/main/java/com/tcm/MineTale/registry/ModRecipes.java

Comment on lines +79 to +87
private static MineTaleRecipeBookComponent createRecipeBookComponent(AlchemistsWorkbenchMenu menu) {
ItemStack tabIcon = new ItemStack(ModBlocks.ALCHEMISTS_WORKBENCH_BLOCK.asItem());

List<RecipeBookComponent.TabInfo> tabs = List.of(
new RecipeBookComponent.TabInfo(tabIcon.getItem(), ModRecipeDisplay.ALCHEMISTS_SEARCH)
);

return new MineTaleRecipeBookComponent(menu, tabs, ModRecipes.ALCHEMISTS_TYPE);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for ModRecipeDisplay definitions and ALCHEMISTS_TYPE
fd ModRecipeDisplay.java -t f --exec cat {} \;

Repository: CodeMonkeysMods/MineTale

Length of output: 4784


🏁 Script executed:

#!/bin/bash
# Search for WorkbenchRecipeDisplay and its getRecipeType implementation
fd -t f ".*RecipeDisplay.*\.java" | xargs rg -l "WorkbenchRecipeDisplay" | head -5

Repository: CodeMonkeysMods/MineTale

Length of output: 195


🏁 Script executed:

#!/bin/bash
# Find MineTaleRecipeBookComponent and check selectMatchingRecipes logic
fd MineTaleRecipeBookComponent.java -t f --exec cat {} \;

Repository: CodeMonkeysMods/MineTale

Length of output: 8788


The recipe book filtering is fundamentally broken—quite the cosmic catastrophe, honestly.

The selectMatchingRecipes() method has a hardcoded check that only accepts recipes with ModRecipeDisplay.WORKBENCH_TYPE, completely ignoring the ModRecipeDisplay.ALCHEMISTS_TYPE you've defined. While this might work if alchemists recipes are registered with WORKBENCH_TYPE as their display type (following the pattern from other workbenches), the logic is brittle and maintains a truly ghastly disconnect: you've bothered to define ALCHEMISTS_TYPE, but it's never actually used in the filtering.

The second condition does check this.filterType correctly at the recipe level, but the first condition creates a catastrophic bottleneck. If an alchemists recipe ever gets registered with ALCHEMISTS_TYPE as its display type instead of WORKBENCH_TYPE, it'll be silently filtered out—most unfortunate indeed, rather much like being left at the Restaurant at the End of the Universe without a decent soufflé.

You need to either:

  1. Replace the hardcoded ModRecipeDisplay.WORKBENCH_TYPE check with a dynamic check that accepts any WorkbenchRecipeDisplay type (remove the type() equality check entirely since the recipe type check handles differentiation), or
  2. Document clearly that all alchemists recipes must use WORKBENCH_TYPE as their display type, not ALCHEMISTS_TYPE—no cap, this inconsistency is sus af.
🤖 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/AlchemistsWorkbenchScreen.java`
around lines 79 - 87, The recipe filtering in selectMatchingRecipes incorrectly
hardcodes ModRecipeDisplay.WORKBENCH_TYPE causing ALCHEMISTS_TYPE recipes to be
dropped; update selectMatchingRecipes in MineTaleRecipeBookComponent to stop
checking for the literal ModRecipeDisplay.WORKBENCH_TYPE (or explicitly allow
ModRecipeDisplay.ALCHEMISTS_TYPE) and instead rely on the recipe-level
display/type check (e.g., use recipe.type() and this.filterType or accept any
WorkbenchRecipeDisplay) so alchemists recipes registered with
ModRecipeDisplay.ALCHEMISTS_TYPE are not filtered out.

Comment on lines +139 to +145
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!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

By the stars of Eärendil! Debug print statements litter this code like the debris of the Heart of Gold's improbability drive. This is NOT it, chief.

These System.out.println calls shall spam the console logs with unnecessary chatter during gameplay. Such debug artifacts are cringe and should be yeeted before merging to the main timeline.

🧹 Proposed cleanup - remove debug statements
             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!");
     }
     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;
                 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;
     }

Also applies to: 257-258, 276-277, 282-283, 286-287

🤖 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/AlchemistsWorkbenchScreen.java`
around lines 139 - 145, Remove the stray System.out.println debug statements in
AlchemistsWorkbenchScreen (the ones printing "Persistent Selection Success" and
"Request failed: No recipe was ever selected!" and the other occurrences around
lines handling results and CraftRequestPayload/amount); either delete them or
replace them with proper logger calls (e.g., use the class LOGGER at appropriate
levels) or silence them entirely if not needed, ensuring the control flow
(ClientPlayNetworking.send(new CraftRequestPayload(results.get(0), amount)) and
return) remains unchanged.

Comment on lines +229 to +230
@SuppressWarnings("deprecation")
List<Holder<Item>> key = ing.items().toList();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Minecraft 1.21 Ingredient items() method deprecated API

💡 Result:

In Minecraft/NeoForge 1.21.x, net.minecraft.world.item.crafting.Ingredient#items() (returns a Stream<Holder<Item>>) is deprecated in NeoForge (marked in its deprecated API list). [1][2]

What to use instead (depending on why you called it):

  • To test whether a stack matches an ingredient (gameplay logic): keep using ingredient.test(stack) (the intended API), not enumerating “all items”. [3]

  • To get “display / example stacks” for JEI/REI/tooltips/UI: prefer NeoForge’s custom-ingredient pathway:

    var custom = ingredient.getCustomIngredient();
    if (custom != null) {
        Stream<ItemStack> displayStacks = custom.getItems();
    } else {
        // vanilla fallback (no perfect exhaustive option)
    }

    NeoForge explicitly documents using Ingredient#getCustomIngredient() / ICustomIngredient and that ICustomIngredient#getItems() is for (mostly) display purposes. [3]

  • If you’re building recipe-book / slot rendering: use Ingredient#toDisplay() / display() style APIs rather than iterating item holders. [2]

Why the deprecation exists (practically): iterating “matching items” is increasingly unreliable/non-exhaustive with modern ingredients (especially custom/data-component-based ones), so NeoForge is pushing consumers toward test(...), toDisplay(), and ICustomIngredient#getItems() for display. [3][1]

Sources

[1] NeoForge 1.21.5 deprecated list (shows net.minecraft.world.item.crafting.Ingredient.items() deprecated) (aldak0.ru)
[2] Mappings view of Ingredient showing items() signature and related display methods (mappings.dev)
[3] NeoForge “Ingredients” documentation covering Ingredient#getCustomIngredient and ICustomIngredient#getItems() usage/purpose (docs.neoforged.net)

Citations:


🏁 Script executed:

# Find and examine the problematic code in context
fd "AlchemistsWorkbenchScreen.java" --type f -x cat -n {} | head -250

Repository: CodeMonkeysMods/MineTale

Length of output: 13852


🏁 Script executed:

# Search for the canCraft method and surrounding context
rg -A 20 "List<Holder<Item>> key = ing.items" --type java

Repository: CodeMonkeysMods/MineTale

Length of output: 20301


Eliminate the dodgy items() call across all workbench screens – it's giving deprecated-API energy and will absolutely yeet your code in future versions.

The Ingredient#items() method is formally deprecated in Minecraft 1.21 because it's unreliable with modern custom ingredients and data-component-based items. NeoForge actively discourages its use. Whilst there's no direct replacement for inventory-checking logic like yours, suppressing the warning and proceeding is mid-tier decision-making. This pattern appears in at least eight workbench screen files:

  • AlchemistsWorkbenchScreen.java
  • ArmorersWorkbenchScreen.java
  • BlacksmithsWorkbenchScreen.java
  • BuildersWorkbenchScreen.java
  • FarmersWorkbenchScreen.java
  • FurnitureWorkbenchScreen.java
  • WorkbenchWorkbenchScreen.java

Consider refactoring the ingredient-grouping logic to avoid enumerating all items entirely, or document explicitly why this suppression is necessary with a maintainer's note. As it stands, this is technical debt waiting to explode.

🤖 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/AlchemistsWorkbenchScreen.java`
around lines 229 - 230, This code uses the deprecated Ingredient#items() (see
the local variable "key" in AlchemistsWorkbenchScreen.java) — remove the
suppression and refactor the grouping/inventory-checking logic to stop
enumerating ingredient items; instead use the Ingredient.test(ItemStack)
predicate to check whether a given ItemStack matches the Ingredient when
scanning inventory slots (replace any loops that build "key" from
ing.items().toList() with per-slot Ingredient.test(stack) checks), and apply the
same refactor to the other workbench screens (Armorers..., Blacksmiths...,
Builders..., Farmers..., Furniture..., Workbench...) or add a short maintainer
note if an unavoidable exception is required.

Comment on lines +43 to +45
* `@param` index the data index to set
* `@param` value the value to assign (ignored)
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

The Javadoc has been corrupted by the forces of markdown escaping – a most grievous wound to documentation.

The @param tags are wrapped in backticks, rendering them invisible to documentation generators. This is giving "I didn't proofread" energy fr fr.

📝 Proposed fix
-         * `@param` index the data index to set
-         * `@param` value the value to assign (ignored)
+         * `@param` index the data index to set
+         * `@param` value the value to assign (ignored)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* `@param` index the data index to set
* `@param` value the value to assign (ignored)
*/
* `@param` index the data index to set
* `@param` value the value to assign (ignored)
🤖 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/AlchemistsWorkbenchEntity.java`
around lines 43 - 45, The Javadoc for AlchemistsWorkbenchEntity has `@param`
tags wrapped in backticks which breaks Javadoc parsing; open the Javadoc block
in class AlchemistsWorkbenchEntity (the method/comment referencing "index" and
"value") and replace the backticked `@param` lines with proper Javadoc `@param`
tags and brief descriptions (e.g., "@param index the data index to set" and
"@param value the value to assign (ignored)"), ensuring the Javadoc block uses
/** ... */ so documentation generators can pick up the parameters.

Comment on lines +62 to +68
/**
* Creates a ArmorersWorkbenchEntity for the specified world position and block state.
*
* Sets the entity's scanRadius to 0.0 and tier to 1.
*
* @param blockPos the world position of this block entity
* @param blockState the block state for this block entity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

By the beard of Gandalf and the infinite improbability of copy-paste errors! This Javadoc speaks of ArmorersWorkbenchEntity when we dwell within the realm of AlchemistsWorkbenchEntity.

This is not bussin', dear traveller. The documentation lies to those who seek knowledge.

📝 Proposed fix
     /**
-     * Creates a ArmorersWorkbenchEntity for the specified world position and block state.
+     * Creates an AlchemistsWorkbenchEntity for the specified world position and block state.
      *
      * Sets the entity's scanRadius to 0.0 and tier to 1.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Creates a ArmorersWorkbenchEntity for the specified world position and block state.
*
* Sets the entity's scanRadius to 0.0 and tier to 1.
*
* @param blockPos the world position of this block entity
* @param blockState the block state for this block entity
/**
* Creates an AlchemistsWorkbenchEntity for the specified world position and block state.
*
* Sets the entity's scanRadius to 0.0 and tier to 1.
*
* `@param` blockPos the world position of this block entity
* `@param` blockState the block state for this block entity
🤖 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/AlchemistsWorkbenchEntity.java`
around lines 62 - 68, The Javadoc for the factory/constructor currently
references ArmorersWorkbenchEntity but this class is AlchemistsWorkbenchEntity;
update the Javadoc comment above the relevant constructor/factory
(AlchemistsWorkbenchEntity) to correctly say "Creates an
AlchemistsWorkbenchEntity" and keep the rest of the description about setting
scanRadius to 0.0 and tier to 1 and the `@param` tags (blockPos, blockState)
accurate; ensure any other occurrences in that comment block that mention
ArmorersWorkbenchEntity are replaced with AlchemistsWorkbenchEntity so the
documentation matches the class name.

Comment on lines +126 to +132
/**
* Creates the server-side container menu for this workbench's UI.
*
* @param syncId the window id used to synchronize the menu with the client
* @param playerInventory the opening player's inventory
* @param player the player who opened the menu
* @return a ArmorersWorkbenchMenu bound to this workbench's inventory and synced data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Another dimensional rift in the documentation! The return type speaks of ArmorersWorkbenchMenu yet we return AlchemistsWorkbenchMenu.

The prophecy of copy-paste hath struck again. Most cringe, I must say.

📝 Proposed fix
     /**
      * Creates the server-side container menu for this workbench's UI.
      *
      * `@param` syncId the window id used to synchronize the menu with the client
      * `@param` playerInventory the opening player's inventory
      * `@param` player the player who opened the menu
-     * `@return` a ArmorersWorkbenchMenu bound to this workbench's inventory and synced data
+     * `@return` an AlchemistsWorkbenchMenu bound to this workbench's inventory and synced data
      */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Creates the server-side container menu for this workbench's UI.
*
* @param syncId the window id used to synchronize the menu with the client
* @param playerInventory the opening player's inventory
* @param player the player who opened the menu
* @return a ArmorersWorkbenchMenu bound to this workbench's inventory and synced data
/**
* Creates the server-side container menu for this workbench's UI.
*
* `@param` syncId the window id used to synchronize the menu with the client
* `@param` playerInventory the opening player's inventory
* `@param` player the player who opened the menu
* `@return` an AlchemistsWorkbenchMenu bound to this workbench's inventory and 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/AlchemistsWorkbenchEntity.java`
around lines 126 - 132, The Javadoc for AlchemistsWorkbenchEntity incorrectly
documents the return as ArmorersWorkbenchMenu; update the method comment (the
`@return` line in the createMenu/Server-side container docblock inside class
AlchemistsWorkbenchEntity) to reference AlchemistsWorkbenchMenu and adjust the
descriptive text to say it returns an AlchemistsWorkbenchMenu bound to this
workbench's inventory and synced data so the doc matches the actual returned
type.

Comment on lines +181 to +186
.rarity(Rarity.UNCOMMON)
.durability(300)
.component(DataComponents.TOOL, MaceItem.createToolProperties())
.repairable(Items.COPPER_INGOT)
.attributes(MaceItem.createAttributes())
.enchantable(15)
.enchantable(10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Verify recipe balance after the great COPPER_MACE nerf, you absolute legends.

By Eru Ilúvatar's infinite wisdom and Zaphod's two heads, this weapon has been nerfed harder than a Nazgûl discovering sunscreen. The stats went from "totally goated" to "mid at best":

  • Rarity: EPIC → UNCOMMON
  • Durability: 500 → 300
  • Enchantable: 15 → 10

However, the recipe in BlacksmithRecipes.java (lines 75-83) still demands 6 copper ingots, 10 logs, and 2 plant fibre—a cost befitting an EPIC item. This cost-to-benefit ratio is giving "not worth the grind" vibes. Consider adjusting the recipe inputs to match the downgraded item, or players will find this crafting choice lowkey L.

🤖 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/ModItems.java` around lines 181 -
186, The blacksmith recipe for the Copper Mace in BlacksmithRecipes still costs
6 copper ingots, 10 logs, and 2 plant fibre even though the item properties in
ModItems (Rarity UNCOMMON, durability 300, enchantable 10 on the Copper Mace
item) were nerfed; update the Copper Mace recipe entry in the BlacksmithRecipes
class (the recipe that references the Copper Mace/its Item reference) to reduce
ingredient counts to reflect the downgraded stats (for example lower copper
ingot count and fewer logs/plant fibre) or adjust to whatever new balance target
you choose so crafting cost matches the item’s new rarity and durability. Ensure
you modify the same recipe registration method/block that constructs the Copper
Mace recipe (the entry currently using 6 copper ingots, 10 logs, 2 plant fibre)
and keep the recipe shape/keys consistent with other blacksmith recipes.

@The-Code-Monkey The-Code-Monkey merged commit 357b705 into 1.21 Mar 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants