Merged
Conversation
Added A Whole lotta stuff includes the Alchemist Workbench
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughA Most Peculiar Journey Through the Annals of the Alchemists' Workshop — Revised EditionHail, traveller of code and craft; the workbench's runes have been tended, a few new portals opened, and the ember-checking rites simplified — the ledger below chronicles the changes. AlchemistsWorkbench.java — New Doorways, Ticking Charm, and the MapCodec Tome
Net effect: straightforward API additions for construction, ticking and codec wiring. AbstractWorkbenchEntity.java — The Central Tome (unchanged in behaviour)
AlchemistsWorkbenchEntity.java — Inventory Sorcery and a Decided Fuel Policy
Risk & Review Notes (in haltingly grand tone)
May your anvils ring true and your fuel checks be as intentional as the stars. WalkthroughAdds two public constructors and a protected Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 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 |
Collaborator
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
|
Note Docstrings generation - SUCCESS |
coderabbitai bot
added a commit
that referenced
this pull request
Mar 4, 2026
Docstrings generation was requested by @AnyaPizza. * #66 (comment) The following files were modified: * `src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.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/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/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: 10
🧹 Nitpick comments (4)
src/client/java/com/tcm/MineTale/datagen/recipes/ForgeRecipes.java (1)
8-8: Replace this lore-style note with an actionable TODO.Line 8 is giving “we’ll do it later, trust me bro” energy. By the stars and Gandalf’s beard, please make it traceable (e.g.,
TODO+ issue/PR reference + owner), so future folk know what “in development” actually means and where to continue.🛠️ Suggested tweak
- /// FORGE STILL IN DEVELOPMENT IN HYTALE + // TODO(#<issue-or-pr>): Implement Forge recipe generation once Forge feature parity is finalised.🤖 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/ForgeRecipes.java` at line 8, Replace the vague comment "/// FORGE STILL IN DEVELOPMENT IN HYTALE" inside the ForgeRecipes class with an actionable TODO that includes a clear owner and tracking reference; e.g., change it to a TODO with Issue/PR ID and assignee (for example "TODO: implement Forge recipe registration — see `#1234`, assign `@username`") so maintainers can find the work and continue it; update the comment located in the ForgeRecipes class to follow this pattern and optionally include expected deliverables (e.g., "register mod recipes, add tests").src/main/java/com/tcm/MineTale/registry/ModBlocks.java (1)
243-243: This TODO is useful, but don’t let it become ancient runes in the codebase.Please track the bed functionality gap in an issue with acceptance criteria, otherwise this debt will spawn side quests forever.
If you want, I can draft the issue text and a small implementation checklist.
🤖 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 243, Create a tracked issue for the bed functionality gap referenced by the TODO in ModBlocks (bed/decorative blocks) that includes clear acceptance criteria (e.g., beds must allow sleeping, set player spawn, handle dimension restrictions, play correct sounds/animations, prevent sleeping in hostile conditions, and include unit/integration tests and manual QA steps); then update the TODO comment in ModBlocks.java to include the new issue ID/URL and a short one-line summary so the code clearly links to the ticket.src/main/java/com/tcm/MineTale/registry/ModRecipes.java (1)
55-60: Tidy the JavaDoc list so the lore matches reality.The registration docs still omit the new alchemists pair, so future readers get fed stale map 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/registry/ModRecipes.java` around lines 55 - 60, Update the JavaDoc in ModRecipes to reflect the actual registered recipe pair by adding the alchemists entry to the list; specifically mention the ALCHEMISTS_TYPE (and its corresponding serializer, e.g., ALCHEMISTS_SERIALIZER or whatever symbol names you used) alongside FURNACE_T1_TYPE, CAMPFIRE_TYPE, WORKBENCH_TYPE, ARMORERS_TYPE, FARMERS_TYPE, BUILDERS_TYPE, BLACKSMITHS_TYPE and FURNITURE_TYPE so the documentation matches the implemented registrations.src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java (1)
33-35: Tiny clarity buff: useDATA_SIZEin the client constructor.Line 34 currently uses
EMPTY_SIZEforSimpleContainerData. It works (both are0), butDATA_SIZEbetter signals intent and avoids future drift.♻️ Proposed tidy-up
- this(syncId, playerInventory, new SimpleContainerData(EMPTY_SIZE), null); + this(syncId, playerInventory, new SimpleContainerData(DATA_SIZE), null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java` around lines 33 - 35, Constructor call in AlchemistsWorkbenchMenu uses EMPTY_SIZE; replace it with DATA_SIZE to convey intended container data length. In the AlchemistsWorkbenchMenu(int syncId, Inventory playerInventory) constructor, change new SimpleContainerData(EMPTY_SIZE) to new SimpleContainerData(DATA_SIZE) (keeping the rest of the delegation the same) and ensure the DATA_SIZE constant referenced is the intended constant used by this menu.
🤖 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 System.out.println calls in
AlchemistsWorkbenchScreen (the render-time craft check and the block handling
persistent selection results) and replace them with a proper logger usage
(declare a private static final Logger LOGGER in AlchemistsWorkbenchScreen),
then change prints to LOGGER.debug(...) or LOGGER.trace(...) and guard with
LOGGER.isDebugEnabled() so logs are not emitted every render; alternatively
remove logging entirely for normal success paths and only log exceptional cases
once (e.g., first failure) to avoid hot-path spam — update the print sites
around the persistent selection handling (the block that sends
CraftRequestPayload and the code referenced around lines 257-287) accordingly.
- Around line 193-199: The "All" button activation is off-by-one because it uses
canCraft(this.minecraft.player, selectedEntry, 2); change the check to call
canCraft with the "all" sentinel (e.g. -1) or with 1 depending on canCraft's
contract so that an available single craft still enables craftAllBtn;
specifically update the boolean canCraftMoreThanOne assignment (referencing
canCraft, selectedEntry, minecraft.player, and craftAllBtn) to use
canCraft(this.minecraft.player, selectedEntry, -1) (or canCraft(..., 1) if the
implementation expects a positive minimum) so the "All" button is active when at
least one craft can be performed.
In `@src/client/java/com/tcm/MineTale/datagen/ModLangProvider.java`:
- Line 275: The translation key used in translationBuilder.add is misspelled
("item.minetale.antitode") and doesn't match the actual item id "antidote", so
update the key to "item.minetale.antidote" in the call to translationBuilder.add
(the method in ModLangProvider) so the localisation maps to the correct item id
and the in-game name resolves properly.
In `@src/client/java/com/tcm/MineTale/datagen/ModModelProvider.java`:
- Around line 137-139: ModModelProvider is missing a generateFlatItem call for
the BOOM_POWDER item, causing it to show the missing-texture placeholder; add a
call to itemModelGenerators.generateFlatItem(ModItems.BOOM_POWDER,
ModelTemplates.FLAT_HANDHELD_ITEM) in the same item model generation block
(where generateFlatItem is used for EMPTY_POTION_BOTTLE, ANTIDOTE,
POPBERRY_BOMB) so BOOM_POWDER gets a proper flat handheld model; verify
ModItems.BOOM_POWDER is the correct identifier used elsewhere (e.g.,
AlchemistRecipes, ModLangProvider) before adding.
In `@src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java`:
- Around line 29-42: The JavaDoc for the constructor in AlchemistsWorkbench
incorrectly references "ArmorersWorkbench"; update the Javadoc text to the
correct class name "AlchemistsWorkbench" and correct any parameter descriptions
to match this class (e.g., the second constructor's supplier description).
Search for other occurrences in this file (notably the block around the second
constructor at the lines referenced) and replace "ArmorersWorkbench" with
"AlchemistsWorkbench" so all docs consistently name the class and its
parameters.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java`:
- Around line 119-123: The load logic in AlchemistsWorkbenchEntity that calls
valueInput.read(..., ItemStack.OPTIONAL_CODEC.listOf()) and then sets items via
inventory.setItem(...) can leave stale items if the saved list is shorter than
inventory.getContainerSize(); before applying loaded stacks, clear the inventory
(e.g., set each slot to ItemStack.EMPTY or call the container's
clearContent/clear method) or after copying saved stacks explicitly clear any
remaining slots up to inventory.getContainerSize() so no ghost items remain.
Ensure you modify the same block that calls valueInput.read(...) and uses
inventory.setItem and inventory.getContainerSize.
- Around line 26-59: The workbench never progresses because
AlchemistsWorkbenchMenu constructs a SimpleContainer with EMPTY_SIZE = 0 so
hasFuel() (which calls getItem(Constants.FUEL_SLOT)) always reads a non-existent
slot and returns EMPTY; fix by ensuring the menu/inventory passed to the entity
contains a real fuel slot: update AlchemistsWorkbenchMenu to construct the
container with the correct non-zero size (include the fuel slot), or adjust the
EMPTY_SIZE constant to the actual inventory size used by
AlchemistsWorkbenchEntity, and verify Constants.FUEL_SLOT index falls within
that size; alternatively, if the workbench should not require fuel, override
hasFuel() in AlchemistsWorkbenchEntity to return true or to check a valid
existing slot instead.
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java`:
- Around line 97-99: Replace the System.out.println debug line in
AlchemistsWorkbenchMenu (the one printing nearbyItems.size() in the
constructor/handler where this.playerInventory.player.level().isClientSide() is
checked) by using the project's logger: call MineTale.LOGGER.debug with the same
message (include context like nearbyItems.size()), or remove the statement
entirely if no longer needed; also scan for other System.out.println usages
across workbench menu classes, entity handlers, screen classes, and registries
and either delete them or convert them to MineTale.LOGGER at an appropriate
level to eliminate stdout spam and centralize logging.
In `@src/main/java/com/tcm/MineTale/registry/ModItems.java`:
- Around line 203-214: Several weapon Item registrations (CRUDE_BUILDERS_HAMMER,
CRUDE_HATCHET, CRUDE_BATTLEAXE, CRUDE_DAGGERS, CRUDE_SHORTBOW, CRUDE_SWORD,
CRUDE_LONGSWORD, COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE,
COPPER_LONGSWORD, COPPER_SHORTBOW) are created with Item::new and lack the
DataComponents.TOOL and DataComponents.WEAPON components used by
CRUDE_MACE/COPPER_MACE; update each register(...) call to attach the same
.component(DataComponents.TOOL, ...) and .component(DataComponents.WEAPON, ...)
configuration (matching the component parameters used by CRUDE_MACE/COPPER_MACE)
so these items provide tool/weapon behavior and stats instead of being inert
items.
In `@src/main/java/com/tcm/MineTale/registry/ModMenuTypes.java`:
- Around line 55-58: The Alchemists workbench MenuType is only registered
server-side; add a client-side registration by calling MenuScreens.register for
ModMenuTypes.ALCHEMISTS_WORKBENCH_MENU mapped to AlchemistsWorkbenchScreen in
the MineTaleClient class alongside the other MenuScreens.register(...) calls so
the client knows how to render the menu.
---
Nitpick comments:
In `@src/client/java/com/tcm/MineTale/datagen/recipes/ForgeRecipes.java`:
- Line 8: Replace the vague comment "/// FORGE STILL IN DEVELOPMENT IN HYTALE"
inside the ForgeRecipes class with an actionable TODO that includes a clear
owner and tracking reference; e.g., change it to a TODO with Issue/PR ID and
assignee (for example "TODO: implement Forge recipe registration — see `#1234`,
assign `@username`") so maintainers can find the work and continue it; update the
comment located in the ForgeRecipes class to follow this pattern and optionally
include expected deliverables (e.g., "register mod recipes, add tests").
In
`@src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java`:
- Around line 33-35: Constructor call in AlchemistsWorkbenchMenu uses
EMPTY_SIZE; replace it with DATA_SIZE to convey intended container data length.
In the AlchemistsWorkbenchMenu(int syncId, Inventory playerInventory)
constructor, change new SimpleContainerData(EMPTY_SIZE) to new
SimpleContainerData(DATA_SIZE) (keeping the rest of the delegation the same) and
ensure the DATA_SIZE constant referenced is the intended constant used by this
menu.
In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Line 243: Create a tracked issue for the bed functionality gap referenced by
the TODO in ModBlocks (bed/decorative blocks) that includes clear acceptance
criteria (e.g., beds must allow sleeping, set player spawn, handle dimension
restrictions, play correct sounds/animations, prevent sleeping in hostile
conditions, and include unit/integration tests and manual QA steps); then update
the TODO comment in ModBlocks.java to include the new issue ID/URL and a short
one-line summary so the code clearly links to the ticket.
In `@src/main/java/com/tcm/MineTale/registry/ModRecipes.java`:
- Around line 55-60: Update the JavaDoc in ModRecipes to reflect the actual
registered recipe pair by adding the alchemists entry to the list; specifically
mention the ALCHEMISTS_TYPE (and its corresponding serializer, e.g.,
ALCHEMISTS_SERIALIZER or whatever symbol names you used) alongside
FURNACE_T1_TYPE, CAMPFIRE_TYPE, WORKBENCH_TYPE, ARMORERS_TYPE, FARMERS_TYPE,
BUILDERS_TYPE, BLACKSMITHS_TYPE and FURNITURE_TYPE so the documentation matches
the implemented registrations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98a4aec1-fb2f-4da0-98b4-dd88efe3b8af
📒 Files selected for processing (18)
src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.javasrc/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.javasrc/client/java/com/tcm/MineTale/datagen/ModLangProvider.javasrc/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.javasrc/client/java/com/tcm/MineTale/datagen/ModModelProvider.javasrc/client/java/com/tcm/MineTale/datagen/recipes/AlchemistRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/BlacksmithRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/ForgeRecipes.javasrc/client/java/com/tcm/MineTale/datagen/recipes/WorkbenchRecipes.javasrc/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.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
src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.java
Show resolved
Hide resolved
src/client/java/com/tcm/MineTale/block/workbenches/screen/AlchemistsWorkbenchScreen.java
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java
Outdated
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
Show resolved
Hide resolved
src/main/java/com/tcm/MineTale/block/workbenches/menu/AlchemistsWorkbenchMenu.java
Show resolved
Hide resolved
Comment on lines
+203
to
+214
| public static final Item CRUDE_BUILDERS_HAMMER = register("crude_builders_hammer", Item::new, new Item.Properties()); | ||
| public static final Item CRUDE_HATCHET = register("crude_hatchet", Item::new, new Item.Properties()); | ||
| public static final Item CRUDE_BATTLEAXE = register("crude_battleaxe", Item::new, new Item.Properties()); | ||
| public static final Item CRUDE_DAGGERS = register("crude_daggers", Item::new, new Item.Properties()); | ||
| public static final Item CRUDE_SHORTBOW = register("crude_shortbow", BowItem::new, new Item.Properties()); | ||
| public static final Item CRUDE_SWORD = register("crude_sword", Item::new, new Item.Properties()); | ||
| public static final Item CRUDE_LONGSWORD = register("crude_longsword", Item::new, new Item.Properties()); | ||
| public static final Item COPPER_HATCHET = register("copper_hatchet", Item::new, new Item.Properties()); | ||
| public static final Item COPPER_DAGGERS = register("copper_daggers", Item::new, new Item.Properties()); | ||
| public static final Item COPPER_BATTLEAXE = register("copper_battleaxe", Item::new, new Item.Properties()); | ||
| public static final Item COPPER_LONGSWORD = register("copper_longsword", Item::new, new Item.Properties()); | ||
| public static final Item COPPER_SHORTBOW = register("copper_shortbow", BowItem::new, new Item.Properties()); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for references to the new weapon/tool items and component assignments
echo "=== Searching for item references and component assignments ==="
rg -nP --type=java --type=json \
'DataComponents\.(WEAPON|TOOL|DAMAGE|ARMOR)|attribute_modifiers|attack_damage|attack_speed|crude_builders_hammer|crude_hatchet|crude_battleaxe|crude_daggers|copper_hatchet|copper_daggers|copper_battleaxe|copper_longsword' \
-A 2 -B 2
echo -e "\n=== Searching for JSON data files that might define item components ==="
find . -type f \( -name "*.json" \) | grep -E '(item|component|data)' | head -20
echo -e "\n=== Looking for item model or component definitions in JSON ==="
rg -l 'crude_builders_hammer|crude_hatchet|copper_hatchet' --type=json 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking ModItems.java for any component setup patterns ==="
rg -n 'setDefaultModelData|withDamage|withMaxStackSize|component|Component' src/main/java/com/tcm/MineTale/registry/ModItems.java || echo "No component patterns found"Repository: CodeMonkeysMods/MineTale
Length of output: 13019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if there's a pattern for weapon item registration in the file ==="
rg -n 'public static final Item' src/main/java/com/tcm/MineTale/registry/ModItems.java | head -30
echo -e "\n=== Checking for any data-driven component setup or JSON resource packs ==="
find ./src -type f -name '*.json' -path '*/data/*' | grep -iE 'component|item' | head -20
echo -e "\n=== Looking for item component definitions in any JSON resources ==="
rg -l 'components.*weapon|components.*tool' --type=json || echo "No component JSON found"Repository: CodeMonkeysMods/MineTale
Length of output: 3459
Ye cursed and utterly non-functional gear—these weapon-named items lack all combat components, which is deeply grievous and most uncouth.
Behold: COPPER_MACE and CRUDE_MACE (lines 181–201) are festooned with .component(DataComponents.TOOL, ...) and .component(DataComponents.WEAPON, ...), as befits any respectable implement of battle. Yet these twelve entries at lines 203–214—thy CRUDE_BUILDERS_HAMMER, CRUDE_HATCHET, CRUDE_BATTLEAXE, CRUDE_DAGGERS, CRUDE_SWORD, CRUDE_LONGSWORD, COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE, COPPER_LONGSWORD, plus both shortbows—are registered as bare Item::new with no components whatsoever. The recipes exist (verified in thy WorkbenchRecipes and BlacksmithRecipes), but the items themselves? Utterly no cap, they're just decorative sticks. Players craft them and get... nothing. This is giving broken quest item energy, and it's absolutely mid.
Wire up .component(DataComponents.TOOL, ...) and .component(DataComponents.WEAPON, ...) as the maces do, or these alleged weapons shall forever be non-functional dreck. Not giving what they're supposed to give, fr fr.
🤖 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 203 -
214, Several weapon Item registrations (CRUDE_BUILDERS_HAMMER, CRUDE_HATCHET,
CRUDE_BATTLEAXE, CRUDE_DAGGERS, CRUDE_SHORTBOW, CRUDE_SWORD, CRUDE_LONGSWORD,
COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE, COPPER_LONGSWORD,
COPPER_SHORTBOW) are created with Item::new and lack the DataComponents.TOOL and
DataComponents.WEAPON components used by CRUDE_MACE/COPPER_MACE; update each
register(...) call to attach the same .component(DataComponents.TOOL, ...) and
.component(DataComponents.WEAPON, ...) configuration (matching the component
parameters used by CRUDE_MACE/COPPER_MACE) so these items provide tool/weapon
behavior and stats instead of being inert items.
Collaborator
There was a problem hiding this comment.
These are placeholders for now until we make classes for these weapons/tools
Contributor
There was a problem hiding this comment.
@AnyaPizza Understood, bestie — no cap, that's a totally valid approach. Placeholders it is, until the custom weapon/tool classes are forged in the fires of Mount Doom (or, y'know, your IDE). We shall trouble thee no further on this matter. 🧙♂️
✏️ Learnings added
Learnt from: AnyaPizza
Repo: CodeMonkeysMods/MineTale PR: 66
File: src/main/java/com/tcm/MineTale/registry/ModItems.java:203-214
Timestamp: 2026-03-04T21:31:06.885Z
Learning: In `src/main/java/com/tcm/MineTale/registry/ModItems.java`, the following items are intentional placeholders registered as bare `Item::new` with no `DataComponents.TOOL` or `DataComponents.WEAPON` components: `CRUDE_BUILDERS_HAMMER`, `CRUDE_HATCHET`, `CRUDE_BATTLEAXE`, `CRUDE_DAGGERS`, `CRUDE_SHORTBOW`, `CRUDE_SWORD`, `CRUDE_LONGSWORD`, `COPPER_HATCHET`, `COPPER_DAGGERS`, `COPPER_BATTLEAXE`, `COPPER_LONGSWORD`, `COPPER_SHORTBOW`. They are awaiting dedicated custom item classes in a future PR. Do not flag these as missing combat components.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: The-Code-Monkey
Repo: CodeMonkeysMods/MineTale PR: 30
File: src/client/java/com/tcm/MineTale/datagen/builders/WorkbenchRecipeBuilder.java:120-131
Timestamp: 2026-02-14T08:07:02.386Z
Learning: In Minecraft 1.21, create an Ingredient from a TagKey<Item> by calling Ingredient.of(TagKey<Item>). There is no Ingredient.ofTag(...) or Ingredient.of(HolderSet<Item>) overload in this version. This guideline applies to Java code across the project; replace any usages of deprecated or non-existent overloads with Ingredient.of(tagKey) in all relevant files (not just this one).
Contributor
Author
There was a problem hiding this comment.
Add an issue to track this
Contributor
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Contributor
Author
There was a problem hiding this comment.
@coderabbitai make an issue to track this
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue create \
--repo CodeMonkeysMods/MineTale \
--title "Implement dedicated custom item classes for crude/copper weapon & tool placeholders" \
--body "## Overview
Several weapon and tool items in \`src/main/java/com/tcm/MineTale/registry/ModItems.java\` are currently registered as bare \`Item::new\` placeholders with no \`DataComponents.TOOL\` or \`DataComponents.WEAPON\` components. These were intentionally left as stubs pending dedicated custom item classes.
## Items to implement
- \`CRUDE_BUILDERS_HAMMER\`
- \`CRUDE_HATCHET\`
- \`CRUDE_BATTLEAXE\`
- \`CRUDE_DAGGERS\`
- \`CRUDE_SHORTBOW\`
- \`CRUDE_SWORD\`
- \`CRUDE_LONGSWORD\`
- \`COPPER_HATCHET\`
- \`COPPER_DAGGERS\`
- \`COPPER_BATTLEAXE\`
- \`COPPER_LONGSWORD\`
- \`COPPER_SHORTBOW\`
## Acceptance Criteria
- Each item above should have a dedicated custom item class (similar to how \`MaceItem\` is used for \`COPPER_MACE\` / \`CRUDE_MACE\`).
- Each class should attach the appropriate \`DataComponents.TOOL\` and/or \`DataComponents.WEAPON\` components, attributes, durability, rarity, and repair material.
- Recipes in \`WorkbenchRecipes\` and \`BlacksmithRecipes\` already exist for these items.
## References
- PR: https://github.com/CodeMonkeysMods/MineTale/pull/66
- Comment: https://github.com/CodeMonkeysMods/MineTale/pull/66#discussion_r2886145819
Requested by `@The-Code-Monkey`." \
--assignee The-Code-Monkey
Length of output: 53
@The-Code-Monkey, issue created and assigned to thee, as the One True Code Monkey of the Fellowship! 🧙♂️
#69 — Implement dedicated custom item classes for crude/copper weapon & tool placeholders
It tracks all twelve placeholder items (CRUDE_BUILDERS_HAMMER, CRUDE_HATCHET, CRUDE_BATTLEAXE, CRUDE_DAGGERS, CRUDE_SHORTBOW, CRUDE_SWORD, CRUDE_LONGSWORD, COPPER_HATCHET, COPPER_DAGGERS, COPPER_BATTLEAXE, COPPER_LONGSWORD, COPPER_SHORTBOW) with acceptance criteria and backlinks to this PR. No cap, it's giving organised dev energy. ✨
🧠 Learnings used
Learnt from: AnyaPizza
Repo: CodeMonkeysMods/MineTale PR: 66
File: src/main/java/com/tcm/MineTale/registry/ModItems.java:203-214
Timestamp: 2026-03-04T21:31:06.885Z
Learning: In `src/main/java/com/tcm/MineTale/registry/ModItems.java`, the following items are intentional placeholders registered as bare `Item::new` with no `DataComponents.TOOL` or `DataComponents.WEAPON` components: `CRUDE_BUILDERS_HAMMER`, `CRUDE_HATCHET`, `CRUDE_BATTLEAXE`, `CRUDE_DAGGERS`, `CRUDE_SHORTBOW`, `CRUDE_SWORD`, `CRUDE_LONGSWORD`, `COPPER_HATCHET`, `COPPER_DAGGERS`, `COPPER_BATTLEAXE`, `COPPER_LONGSWORD`, `COPPER_SHORTBOW`. They are awaiting dedicated custom item classes in a future PR. Do not flag these as missing combat components.
Learnt from: The-Code-Monkey
Repo: CodeMonkeysMods/MineTale PR: 30
File: src/client/java/com/tcm/MineTale/datagen/builders/WorkbenchRecipeBuilder.java:120-131
Timestamp: 2026-02-14T08:07:02.386Z
Learning: In Minecraft 1.21, create an Ingredient from a TagKey<Item> by calling Ingredient.of(TagKey<Item>). There is no Ingredient.ofTag(...) or Ingredient.of(HolderSet<Item>) overload in this version. This guideline applies to Java code across the project; replace any usages of deprecated or non-existent overloads with Ingredient.of(tagKey) in all relevant files (not just this one).
AnyaPizza
added a commit
that referenced
this pull request
Mar 4, 2026
The-Code-Monkey
added a commit
that referenced
this pull request
Mar 5, 2026
Fixed some of #66's problems
Docstrings generation was requested by @The-Code-Monkey. The following files were modified: * `src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java`
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java (2)
116-123:⚠️ Potential issue | 🟡 MinorClear inventory unconditionally before reading, not only when the tag exists.
Right now, if
"Inventory"is absent, old contents can linger like a cursed side-quest. Moveinventory.clearContent()beforeread(...)so deserialisation always resets state.🧼 Suggested fix
- // Read the inventory list back - valueInput.read("Inventory", ItemStack.OPTIONAL_CODEC.listOf()).ifPresent(stacks -> { - // Fix: Clear existing items to prevent stale data if the saved list is smaller - inventory.clearContent(); - + // Reset first, then read the inventory list back + inventory.clearContent(); + valueInput.read("Inventory", ItemStack.OPTIONAL_CODEC.listOf()).ifPresent(stacks -> { for (int i = 0; i < stacks.size() && i < inventory.getContainerSize(); i++) { inventory.setItem(i, stacks.get(i)); } });🤖 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 116 - 123, The inventory is only cleared when the "Inventory" tag is present, allowing stale items to persist if the tag is absent; move the clear so it always runs before deserialisation: in AlchemistsWorkbenchEntity, call inventory.clearContent() immediately before invoking valueInput.read("Inventory", ItemStack.OPTIONAL_CODEC.listOf()) (and then iterate/set items from the optional list as you currently do) so the inventory is reset regardless of whether the tag exists.
155-165:⚠️ Potential issue | 🟡 MinorJavadoc and implementation are now in different universes.
The method docs still claim fuel-slot checks, but Line 164 always returns
true. That mismatch is giving Vogon-manual energy and will mislead future maintainers.📝 Suggested doc update
/** - * Determines whether the workbench currently has fuel available. - * - * Checks that the entity is in a loaded level and that the configured fuel slot contains an item. - * - * `@return` `true` if the entity is in a loaded level and the fuel slot contains an item, `false` otherwise. + * Alchemist workbenches do not consume fuel. + * + * `@return` always {`@code` 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/block/workbenches/entity/AlchemistsWorkbenchEntity.java` around lines 155 - 165, The Javadoc and the hasFuel() implementation diverge: instead of always returning true, update AlchemistsWorkbenchEntity.hasFuel() to actually check the configured fuel slot; e.g., return whether the entity is in a loaded level and the fuel slot contains an item by checking the inventory/fuel access (use the class's inventory accessor such as getItem(FUEL_SLOT) or inventory.getStack(FUEL_SLOT) and ensure level != null/loaded), or alternatively change the Javadoc to match the current behavior—prefer implementing the real check in hasFuel() to use the entity's fuel slot (reference: AlchemistsWorkbenchEntity.hasFuel(), FUEL_SLOT or fuelSlot, and the inventory getter method).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java`:
- Around line 116-123: The inventory is only cleared when the "Inventory" tag is
present, allowing stale items to persist if the tag is absent; move the clear so
it always runs before deserialisation: in AlchemistsWorkbenchEntity, call
inventory.clearContent() immediately before invoking
valueInput.read("Inventory", ItemStack.OPTIONAL_CODEC.listOf()) (and then
iterate/set items from the optional list as you currently do) so the inventory
is reset regardless of whether the tag exists.
- Around line 155-165: The Javadoc and the hasFuel() implementation diverge:
instead of always returning true, update AlchemistsWorkbenchEntity.hasFuel() to
actually check the configured fuel slot; e.g., return whether the entity is in a
loaded level and the fuel slot contains an item by checking the inventory/fuel
access (use the class's inventory accessor such as getItem(FUEL_SLOT) or
inventory.getStack(FUEL_SLOT) and ensure level != null/loaded), or alternatively
change the Javadoc to match the current behavior—prefer implementing the real
check in hasFuel() to use the entity's fuel slot (reference:
AlchemistsWorkbenchEntity.hasFuel(), FUEL_SLOT or fuelSlot, and the inventory
getter method).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96fe137d-4f2a-4895-9dad-a3fd4be8751c
📒 Files selected for processing (2)
src/main/java/com/tcm/MineTale/block/workbenches/entity/AbstractWorkbenchEntity.javasrc/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/tcm/MineTale/block/workbenches/entity/AbstractWorkbenchEntity.java
Docstrings generation was requested by @The-Code-Monkey. The following files were modified: * `src/main/java/com/tcm/MineTale/block/workbenches/AlchemistsWorkbench.java` * `src/main/java/com/tcm/MineTale/block/workbenches/entity/AbstractWorkbenchEntity.java` * `src/main/java/com/tcm/MineTale/block/workbenches/entity/AlchemistsWorkbenchEntity.java`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.