Added Loot Table datagen. Added sticks to the short/tall grass loot table.#49
Merged
The-Code-Monkey merged 6 commits into1.21from Feb 20, 2026
Merged
Added Loot Table datagen. Added sticks to the short/tall grass loot table.#49The-Code-Monkey merged 6 commits into1.21from
The-Code-Monkey merged 6 commits into1.21from
Conversation
AnyaPizza
added
bug
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a loot-table data-gen provider and runtime loot modifiers, expands block tag assignments, tightens furnace workbench block properties, and implements multi-block destruction/drop suppression logic for workbenches; also registers loot-table generation at startup. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant AbstractWorkbench
participant World
participant LootSystem
Player->>AbstractWorkbench: playerWillDestroy(level,pos,state,player)
AbstractWorkbench->>World: determine master vs slave and footprint
alt Destroying Master
AbstractWorkbench->>World: silently remove slave parts (no drops)
AbstractWorkbench->>World: remove master (produce drops unless creative)
else Destroying Slave
AbstractWorkbench->>World: remove/suppress slave drop
AbstractWorkbench->>World: trigger master destruction (produce drops unless creative)
end
AbstractWorkbench->>World: emit GameEvent.BLOCK_DESTROY for affected positions
World->>LootSystem: request drops per loot tables and modifiers
LootSystem->>Player: deliver drops (or none if suppressed / creative)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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: 5
🧹 Nitpick comments (3)
src/main/java/com/tcm/MineTale/registry/ModBlocks.java (1)
63-67: Indentation inconsistency ontrueargumentThe closing
trueargument on line 66 has an extra level of indentation compared to the same argument inFURNACE_WORKBENCH_BLOCK_T2(line 75) and all otherregister(...)call sites in the file.✏️ Proposed fix
BlockBehaviour.Properties.of().sound(SoundType.STONE).noOcclusion() .requiresCorrectToolForDrops().strength(3.5F) .lightLevel(litBlockEmission(13)), - true + true );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java` around lines 63 - 67, The trailing boolean argument in the register(...) call inside ModBlocks is indented one level too deep; locate the register invocation that builds BlockBehaviour.Properties.of().sound(SoundType.STONE).noOcclusion().requiresCorrectToolForDrops().strength(3.5F).lightLevel(litBlockEmission(13)) and align the final `true` argument to match the other registrations (e.g., FURNACE_WORKBENCH_BLOCK_T2) so it sits under the opening parenthesis of the register call rather than indented further; no logic changes required, just adjust whitespace to be consistent with other register(...) call sites.src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java (1)
23-27: Stale Javadoc —addTagsnow also populates mining tags.The existing doc block says "Populates the
BlockTags.LOGStag"; it no longer accurately describes the method.📝 Suggested update
- /** - * Populates the BlockTags.LOGS tag with this mod's log blocks. - * - * Registers each mod-defined log block so they are included in the game's LOGS tag mapping. - * - * `@param` provider a registry lookup provider used to resolve holders during tag population - */ + /** + * Populates block tags for this mod's blocks. + * + * <p>Registers mod workbench blocks under {`@link` BlockTags#MINEABLE_WITH_PICKAXE} or + * {`@link` BlockTags#MINEABLE_WITH_AXE} and registers all mod log blocks under {`@link` BlockTags#LOGS}.</p> + * + * `@param` provider a registry lookup provider used to resolve holders during tag population + */🤖 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/ModBlockTagProvider.java` around lines 23 - 27, The Javadoc for ModBlockTagProvider::addTags is stale — it only mentions populating BlockTags.LOGS while addTags now also registers mining-related tags; update the method comment to accurately describe all responsibilities (e.g., registering mod log blocks and populating mining tags used by the mod), referencing addTags and the specific tags it handles (such as BlockTags.LOGS and any mining tag constants) and include a short summary of behavior and purpose so the documentation matches the implementation.src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java (1)
45-45: Method namesAverageOreDrops,SingleOreDrops,LightOreDropsuse PascalCase — Java convention is camelCase.♻️ Proposed rename
- public LootTable.Builder AverageOreDrops(Block drop, Item item) { + public LootTable.Builder averageOreDrops(Block drop, Item item) { - public LootTable.Builder SingleOreDrops(Block drop, Item item) { + public LootTable.Builder singleOreDrops(Block drop, Item item) { - public LootTable.Builder LightOreDrops(Block drop, Item item) { + public LootTable.Builder lightOreDrops(Block drop, Item item) {Also applies to: 52-52, 59-59
🤖 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/ModLootTableProvider.java` at line 45, Rename the PascalCase methods to Java camelCase: change AverageOreDrops, SingleOreDrops, and LightOreDrops to averageOreDrops, singleOreDrops, and lightOreDrops respectively in ModLootTableProvider.java; update all internal references/call sites to these methods (any builders or registrations that call AverageOreDrops/SingleOreDrops/LightOreDrops) so compilation succeeds and adjust any Javadoc/comments mentioning the old names to the new names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java`:
- Line 3: Remove the incorrect and unused import com.jcraft.jorbis.Block from
the ModBlockTagProvider file (the import at the top referencing Block) — it is
unrelated to Minecraft; delete that line and then re-run import
organizing/formatting to ensure only relevant imports remain in the
ModBlockTagProvider class.
- Line 35: ARMORERS_WORKBENCH_BLOCK is incorrectly added to the pickaxe tag;
update ModBlockTagProvider so ARMORERS_WORKBENCH_BLOCK is added to the
MINEABLE_WITH_AXE tag instead of MINEABLE_WITH_PICKAXE: locate the
.add(ModBlocks.ARMORERS_WORKBENCH_BLOCK) call in the block tag registration (the
same area where CAMPFIRE_WORKBENCH_BLOCK and WORKBENCH_WORKBENCH_BLOCK are
placed), remove it from the MINEABLE_WITH_PICKAXE group and add it to the
MINEABLE_WITH_AXE group to match its wood SoundType properties.
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 33-34: The loot-table registration mistakenly calls dropSelf for
ModBlocks.FURNACE_WORKBENCH_BLOCK_T1 twice in ModLootTableProvider; replace the
second call with dropSelf(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2) so the T2 block
gets a loot table and will drop itself—locate the duplicate dropSelf(...) calls
in the method that registers block loot tables and change the second identifier
from FURNACE_WORKBENCH_BLOCK_T1 to FURNACE_WORKBENCH_BLOCK_T2.
In `@src/main/java/com/tcm/MineTale/registry/ModItems.java`:
- Around line 21-25: The file imports unused and inaccessible symbols (notably
the static import Items.registerBlock, plus UnaryOperator, DataComponents,
BlockItem, Items, ItemContainerContents, Block) which will cause a compilation
error and are semantically incorrect for this class; remove the static import of
Items.registerBlock and drop all other listed unused imports from ModItems.java,
keeping only imports actually referenced by the class, and only re-add these
specific imports when code that uses Items.registerBlock or the other types is
introduced (the register() method is the place that currently registers
standalone items and should not rely on Items.registerBlock).
In `@src/main/java/com/tcm/MineTale/util/ModLootTableModifiers.java`:
- Line 17: The loot modifier registered in LootTableEvents.MODIFY currently adds
Items.AIR as a sibling entry in the SHORT_GRASS (and tall grass) pool and lacks
a sources.isBuiltin() guard; replace the AIR placeholder by removing the AIR
entry and instead add a LootItemRandomChanceCondition (import
net.minecraft.world.level.storage.loot.predicates.LootItemRandomChanceCondition)
to the Items.STICK LootPoolEntry so it has a 50% drop chance, and wrap the
handler body with if (!sources.isBuiltin()) return; (or if (sources.isBuiltin())
{ ... }) to avoid overwriting datapack tables; update the listener lambda
registered at LootTableEvents.MODIFY accordingly.
---
Nitpick comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java`:
- Around line 23-27: The Javadoc for ModBlockTagProvider::addTags is stale — it
only mentions populating BlockTags.LOGS while addTags now also registers
mining-related tags; update the method comment to accurately describe all
responsibilities (e.g., registering mod log blocks and populating mining tags
used by the mod), referencing addTags and the specific tags it handles (such as
BlockTags.LOGS and any mining tag constants) and include a short summary of
behavior and purpose so the documentation matches the implementation.
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Line 45: Rename the PascalCase methods to Java camelCase: change
AverageOreDrops, SingleOreDrops, and LightOreDrops to averageOreDrops,
singleOreDrops, and lightOreDrops respectively in ModLootTableProvider.java;
update all internal references/call sites to these methods (any builders or
registrations that call AverageOreDrops/SingleOreDrops/LightOreDrops) so
compilation succeeds and adjust any Javadoc/comments mentioning the old names to
the new names.
In `@src/main/java/com/tcm/MineTale/registry/ModBlocks.java`:
- Around line 63-67: The trailing boolean argument in the register(...) call
inside ModBlocks is indented one level too deep; locate the register invocation
that builds
BlockBehaviour.Properties.of().sound(SoundType.STONE).noOcclusion().requiresCorrectToolForDrops().strength(3.5F).lightLevel(litBlockEmission(13))
and align the final `true` argument to match the other registrations (e.g.,
FURNACE_WORKBENCH_BLOCK_T2) so it sits under the opening parenthesis of the
register call rather than indented further; no logic changes required, just
adjust whitespace to be consistent with other register(...) call sites.
| @@ -1,5 +1,6 @@ | |||
| package com.tcm.MineTale.datagen; | |||
|
|
|||
| import com.jcraft.jorbis.Block; | |||
Contributor
There was a problem hiding this comment.
Remove wrong com.jcraft.jorbis.Block import.
com.jcraft.jorbis is an Ogg Vorbis audio decoder library; its Block class is a DSP processing block and has nothing to do with Minecraft blocks. This import is unused in this file and should be removed.
🐛 Proposed fix
-import com.jcraft.jorbis.Block;
import com.tcm.MineTale.registry.ModBlocks;📝 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
| import com.jcraft.jorbis.Block; | |
| import com.tcm.MineTale.registry.ModBlocks; |
🤖 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/ModBlockTagProvider.java` at line 3,
Remove the incorrect and unused import com.jcraft.jorbis.Block from the
ModBlockTagProvider file (the import at the top referencing Block) — it is
unrelated to Minecraft; delete that line and then re-run import
organizing/formatting to ensure only relevant imports remain in the
ModBlockTagProvider class.
| valueLookupBuilder(BlockTags.MINEABLE_WITH_PICKAXE) | ||
| .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T1) | ||
| .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2) | ||
| .add(ModBlocks.ARMORERS_WORKBENCH_BLOCK); |
Contributor
There was a problem hiding this comment.
ARMORERS_WORKBENCH_BLOCK should be in MINEABLE_WITH_AXE, not MINEABLE_WITH_PICKAXE.
Per the ModBlocks snippet, ARMORERS_WORKBENCH_BLOCK is registered with BlockBehaviour.Properties.of().sound(SoundType.WOOD) — the same wood properties as CAMPFIRE_WORKBENCH_BLOCK and WORKBENCH_WORKBENCH_BLOCK, both of which are correctly placed in MINEABLE_WITH_AXE. Placing a wood-type block under the pickaxe tag is inconsistent and will give the wrong mining-speed bonus.
🐛 Proposed fix
valueLookupBuilder(BlockTags.MINEABLE_WITH_PICKAXE)
.add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T1)
- .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2)
- .add(ModBlocks.ARMORERS_WORKBENCH_BLOCK);
+ .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2);
valueLookupBuilder(BlockTags.MINEABLE_WITH_AXE)
.add(ModBlocks.CAMPFIRE_WORKBENCH_BLOCK)
- .add(ModBlocks.WORKBENCH_WORKBENCH_BLOCK);
+ .add(ModBlocks.WORKBENCH_WORKBENCH_BLOCK)
+ .add(ModBlocks.ARMORERS_WORKBENCH_BLOCK);📝 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
| .add(ModBlocks.ARMORERS_WORKBENCH_BLOCK); | |
| valueLookupBuilder(BlockTags.MINEABLE_WITH_PICKAXE) | |
| .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T1) | |
| .add(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2); | |
| valueLookupBuilder(BlockTags.MINEABLE_WITH_AXE) | |
| .add(ModBlocks.CAMPFIRE_WORKBENCH_BLOCK) | |
| .add(ModBlocks.WORKBENCH_WORKBENCH_BLOCK) | |
| .add(ModBlocks.ARMORERS_WORKBENCH_BLOCK); |
🤖 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/ModBlockTagProvider.java` at line
35, ARMORERS_WORKBENCH_BLOCK is incorrectly added to the pickaxe tag; update
ModBlockTagProvider so ARMORERS_WORKBENCH_BLOCK is added to the
MINEABLE_WITH_AXE tag instead of MINEABLE_WITH_PICKAXE: locate the
.add(ModBlocks.ARMORERS_WORKBENCH_BLOCK) call in the block tag registration (the
same area where CAMPFIRE_WORKBENCH_BLOCK and WORKBENCH_WORKBENCH_BLOCK are
placed), remove it from the MINEABLE_WITH_PICKAXE group and add it to the
MINEABLE_WITH_AXE group to match its wood SoundType properties.
src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java
Outdated
Show resolved
Hide resolved
Comment on lines
+21
to
+25
| import net.minecraft.world.item.Items; | ||
| import net.minecraft.world.item.component.ItemContainerContents; | ||
| import net.minecraft.world.level.block.Block; | ||
|
|
||
| import static net.minecraft.world.item.Items.registerBlock; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check for any access widener file that widens Items.registerBlock
echo "=== Searching for access widener files ==="
fd -e accesswidener 2>/dev/null | head -20
# 2. Print the full content of any access widener(s) found
echo -e "\n=== Content of access widener files ==="
fd -e accesswidener -x cat {} 2>/dev/null
# 3. Confirm no usages of registerBlock or the other imported symbols in this file
echo -e "\n=== Searching for usage of imported symbols in ModItems.java ==="
rg "registerBlock|UnaryOperator|DataComponents|BlockItem|ItemContainerContents" src/main/java/com/tcm/MineTale/registry/ModItems.java -n 2>/dev/null || echo "No matches found"
# 4. Show the actual imports in ModItems.java to confirm they exist
echo -e "\n=== Current imports in ModItems.java (lines 1-35) ==="
head -35 src/main/java/com/tcm/MineTale/registry/ModItems.java 2>/dev/null | cat -nRepository: CodeMonkeysMods/MineTale
Length of output: 2106
🏁 Script executed:
#!/bin/bash
# Check if Items, Block, or BlockItem are actually used in the file body (after line 35)
echo "=== Checking for usage of Items class methods or BlockItem/Block in file body ==="
tail -n +27 src/main/java/com/tcm/MineTale/registry/ModItems.java | grep -n "Items\|BlockItem\|Block" | grep -v "^[0-9]*:.*import" || echo "No usages found in file body"
# Show the entire ModItems.java file to be thorough
echo -e "\n=== Full ModItems.java file ==="
wc -l src/main/java/com/tcm/MineTale/registry/ModItems.java
cat -n src/main/java/com/tcm/MineTale/registry/ModItems.javaRepository: CodeMonkeysMods/MineTale
Length of output: 9797
import static Items.registerBlock will fail to compile — method is inaccessible and unused.
Items.registerBlock is a private static helper in the vanilla Items class used exclusively within its own static initializer. No access widener exists in this project to expose it, so this static import will cause a compilation error. Additionally:
- Semantically incorrect —
Items.registerBlockassociates vanillaBlock→BlockItemmappings; this file'sregister()method handles standalone item registration (lines 117–126), which is a different operation. - Completely unused in the file.
All other newly added imports on lines 6, 13, 19, 21–23 (UnaryOperator, DataComponents, BlockItem, Items, ItemContainerContents, Block) are also unreferenced anywhere in the file body. Remove all of them and add them only when the code that actually uses them is introduced.
🗑️ Proposed fix — remove the dead imports
-import java.util.function.UnaryOperator;
import com.tcm.MineTale.MineTale;
import com.tcm.MineTale.item.ModCreativeTab;
import net.fabricmc.fabric.api.itemgroup.v1.ItemGroupEvents;
import net.minecraft.core.Registry;
-import net.minecraft.core.component.DataComponents;
import net.minecraft.core.registries.BuiltInRegistries;
import net.minecraft.core.registries.Registries;
import net.minecraft.resources.Identifier;
import net.minecraft.resources.ResourceKey;
import net.minecraft.world.food.FoodProperties;
-import net.minecraft.world.item.BlockItem;
import net.minecraft.world.item.Item;
-import net.minecraft.world.item.Items;
-import net.minecraft.world.item.component.ItemContainerContents;
-import net.minecraft.world.level.block.Block;
-
-import static net.minecraft.world.item.Items.registerBlock;🤖 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 21 - 25,
The file imports unused and inaccessible symbols (notably the static import
Items.registerBlock, plus UnaryOperator, DataComponents, BlockItem, Items,
ItemContainerContents, Block) which will cause a compilation error and are
semantically incorrect for this class; remove the static import of
Items.registerBlock and drop all other listed unused imports from ModItems.java,
keeping only imports actually referenced by the class, and only re-add these
specific imports when code that uses Items.registerBlock or the other types is
introduced (the register() method is the place that currently registers
standalone items and should not rely on Items.registerBlock).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/main/java/com/tcm/MineTale/block/workbenches/AbstractWorkbench.java (3)
395-404:state.setValue(BlockStateProperties.LIT, false)is dead code — remove it.The comment describes this as "change state to desync loot", but drop suppression is not achieved this way. What actually prevents the slave from dropping is
level.setBlock(pos, Blocks.AIR.defaultBlockState(), 35)on line 399: by the time the vanilla destroy chain callsremoveBlock(pos, false)afterplayerWillDestroyreturns, it finds the position already AIR, returnsfalse, andafterBreak/dropResourcesis never invoked. Thestate.setValuecall is a no-op in this flow and the misleading comment should also be removed.🧹 Proposed cleanup
if (!isMaster && !player.isCreative()) { - // This prevents the current block from dropping its loot table - // because we already triggered the Master's drop above. - state = state.setValue(BlockStateProperties.LIT, false); // Optional: change state to desync loot + // Pre-remove the slave so removeBlock() finds AIR and skips afterBreak/dropResources. level.setBlock(pos, Blocks.AIR.defaultBlockState(), 35); }🤖 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/AbstractWorkbench.java` around lines 395 - 404, Remove the dead state mutation and its misleading comment in playerWillDestroy: the call to state.setValue(BlockStateProperties.LIT, false) (and its comment about desyncing loot) is a no-op because level.setBlock(pos, Blocks.AIR.defaultBlockState(), 35) already prevents drops; delete the state.setValue(...) invocation and the associated comment inside AbstractWorkbench.playerWillDestroy so the method only calls level.setBlock(...) before returning super.playerWillDestroy(...).
37-38: Remove leftover development comment.
// DoorBlockappears to be a note referencingDoorBlock's analogous multi-block destruction logic and serves no purpose in the committed code.🧹 Proposed cleanup
-// DoorBlock - public abstract class AbstractWorkbench<E extends AbstractWorkbenchEntity> extends BaseEntityBlock {🤖 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/AbstractWorkbench.java` around lines 37 - 38, Remove the leftover development comment "// DoorBlock" in AbstractWorkbench (the class AbstractWorkbench) — delete that stray inline comment so the code has no vestigial notes referencing DoorBlock's multi-block logic.
364-368: HoistDirection facingoutside the nested loops.
state.getValue(FACING)returns the same value on every iteration; computing it inside the inner loop is redundant.♻️ Proposed refactor
+ Direction facing = state.getValue(FACING); for (int y = 0; y <= (isTall ? 1 : 0); y++) { for (int x = 0; x <= (isWide ? 1 : 0); x++) { - Direction facing = state.getValue(FACING); BlockPos targetPos = masterPos.above(y).relative(facing.getClockWise(), x);🤖 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/AbstractWorkbench.java` around lines 364 - 368, Move the repeated call to state.getValue(FACING) out of the nested for-loops in AbstractWorkbench.java: fetch Direction facing once before the y/x loops and reuse it inside, so you no longer call state.getValue(FACING) each iteration; keep use of masterPos.above(y).relative(facing.getClockWise(), x) unchanged and ensure variable scope allows both inner loops to access the precomputed facing.src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java (2)
67-86: Triplicated ore-drop logic — extract a shared private helper.All three methods are structurally identical; only the
NumberProviderfor the item count differs. Extract a private method accepting the count provider to eliminate the duplication.♻️ Proposed refactor
+ private LootTable.Builder oreDrops(Block drop, Item item, net.minecraft.world.level.storage.loot.providers.number.NumberProvider count) { + HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); + return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, + ((LootPoolSingletonContainer.Builder<?>) + LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(count))) + .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); + } + - public LootTable.Builder AverageOreDrops(Block drop, Item item) { - HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(UniformGenerator.between(2, 5)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); - } + public LootTable.Builder averageOreDrops(Block drop, Item item) { + return oreDrops(drop, item, UniformGenerator.between(2.0F, 5.0F)); + } - public LootTable.Builder SingleOreDrops(Block drop, Item item) { - HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(ConstantValue.exactly(1)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); - } + public LootTable.Builder singleOreDrops(Block drop, Item item) { + return oreDrops(drop, item, ConstantValue.exactly(1.0F)); + } - public LootTable.Builder LightOreDrops(Block drop, Item item) { - HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(UniformGenerator.between(1, 2)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); - } + public LootTable.Builder lightOreDrops(Block drop, Item item) { + return oreDrops(drop, item, UniformGenerator.between(1.0F, 2.0F)); + }🤖 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/ModLootTableProvider.java` around lines 67 - 86, AverageOreDrops, SingleOreDrops and LightOreDrops duplicate the same loot-table construction logic differing only by the NumberProvider for item count; extract a private helper (e.g., buildOreDrops(Block drop, Item item, NumberProvider countProvider)) that performs the registries.lookupOrThrow(Registries.ENCHANTMENT) lookup once and calls createSilkTouchDispatchTable(drop, applyExplosionDecay(drop, (LootPoolSingletonContainer.Builder<?>)LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(countProvider)).apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE)))) and then have AverageOreDrops, SingleOreDrops and LightOreDrops call this helper passing UniformGenerator.between(2,5), ConstantValue.exactly(1), and UniformGenerator.between(1,2) respectively.
67-86: Method names use PascalCase — should be camelCase per Java conventions.
AverageOreDrops,SingleOreDrops, andLightOreDropsfollow type-naming conventions rather than method-naming ones. The example Javadoc on line 65 would also need updating.♻️ Proposed rename
- public LootTable.Builder AverageOreDrops(Block drop, Item item) { + public LootTable.Builder averageOreDrops(Block drop, Item item) { ... - public LootTable.Builder SingleOreDrops(Block drop, Item item) { + public LootTable.Builder singleOreDrops(Block drop, Item item) { ... - public LootTable.Builder LightOreDrops(Block drop, Item item) { + public LootTable.Builder lightOreDrops(Block drop, Item item) {🤖 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/ModLootTableProvider.java` around lines 67 - 86, The three public methods AverageOreDrops, SingleOreDrops, and LightOreDrops violate Java naming conventions; rename them to camelCase (e.g., averageOreDrops, singleOreDrops, lightOreDrops), update their declarations and all call sites, and adjust the example Javadoc comment above these methods to reflect the new names; ensure method signatures (parameters Block drop, Item item) and usages of registries.lookupOrThrow(Registries.ENCHANTMENT) / impl variable remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 42-44: The three dropSelf calls for
ModBlocks.ARMORERS_WORKBENCH_BLOCK, ModBlocks.CAMPFIRE_WORKBENCH_BLOCK, and
ModBlocks.WORKBENCH_WORKBENCH_BLOCK produce duplicate drops for multi-block
workbenches; replace each dropSelf invocation in ModLootTableProvider with a
custom loot table that mirrors the fix used for FURNACE_WORKBENCH_BLOCK_T1: emit
the workbench item only when the block-state predicate marks the master/left
half (e.g., TYPE == ChestType.LEFT or the equivalent master property), so only
the primary block drops the item; update the loot table registrations for those
three symbols to use that state-gated loot table instead of dropSelf.
---
Duplicate comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 40-59: FURNACE_WORKBENCH_BLOCK_T2 is missing a loot table entry in
ModLootTableProvider.generate(), so add an entry mirroring the T1 logic: either
call dropSelf(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2) if default behavior
suffices, or register a custom LootTable for
ModBlocks.FURNACE_WORKBENCH_BLOCK_T2 using the same pattern as the T1 block
(LootTable.lootTable() with a LootPool that sets rolls to 1, adds
LootItem.lootTableItem(ModBlocks.FURNACE_WORKBENCH_BLOCK_T2), applies the same
LootItemBlockStatePropertyCondition with AbstractWorkbench.HALF = LOWER and
AbstractWorkbench.TYPE = LEFT, and ExplosionCondition.survivesExplosion());
update the generate() method in ModLootTableProvider accordingly.
---
Nitpick comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 67-86: AverageOreDrops, SingleOreDrops and LightOreDrops duplicate
the same loot-table construction logic differing only by the NumberProvider for
item count; extract a private helper (e.g., buildOreDrops(Block drop, Item item,
NumberProvider countProvider)) that performs the
registries.lookupOrThrow(Registries.ENCHANTMENT) lookup once and calls
createSilkTouchDispatchTable(drop, applyExplosionDecay(drop,
(LootPoolSingletonContainer.Builder<?>)LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(countProvider)).apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))
and then have AverageOreDrops, SingleOreDrops and LightOreDrops call this helper
passing UniformGenerator.between(2,5), ConstantValue.exactly(1), and
UniformGenerator.between(1,2) respectively.
- Around line 67-86: The three public methods AverageOreDrops, SingleOreDrops,
and LightOreDrops violate Java naming conventions; rename them to camelCase
(e.g., averageOreDrops, singleOreDrops, lightOreDrops), update their
declarations and all call sites, and adjust the example Javadoc comment above
these methods to reflect the new names; ensure method signatures (parameters
Block drop, Item item) and usages of
registries.lookupOrThrow(Registries.ENCHANTMENT) / impl variable remain
unchanged.
In `@src/main/java/com/tcm/MineTale/block/workbenches/AbstractWorkbench.java`:
- Around line 395-404: Remove the dead state mutation and its misleading comment
in playerWillDestroy: the call to state.setValue(BlockStateProperties.LIT,
false) (and its comment about desyncing loot) is a no-op because
level.setBlock(pos, Blocks.AIR.defaultBlockState(), 35) already prevents drops;
delete the state.setValue(...) invocation and the associated comment inside
AbstractWorkbench.playerWillDestroy so the method only calls level.setBlock(...)
before returning super.playerWillDestroy(...).
- Around line 37-38: Remove the leftover development comment "// DoorBlock" in
AbstractWorkbench (the class AbstractWorkbench) — delete that stray inline
comment so the code has no vestigial notes referencing DoorBlock's multi-block
logic.
- Around line 364-368: Move the repeated call to state.getValue(FACING) out of
the nested for-loops in AbstractWorkbench.java: fetch Direction facing once
before the y/x loops and reuse it inside, so you no longer call
state.getValue(FACING) each iteration; keep use of
masterPos.above(y).relative(facing.getClockWise(), x) unchanged and ensure
variable scope allows both inner loops to access the precomputed facing.
src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java (1)
119-138: Method names violate Java naming conventions — use camelCase.
AverageOreDrops,SingleOreDrops, andLightOreDropsuse PascalCase, which is reserved for types in Java. Methods must use camelCase.Additionally,
SetItemCountFunction.setCount(ConstantValue.exactly(1))inSingleOreDropsis redundant (the default item count is already 1), and the explicit cast toLootPoolSingletonContainer.Builder<?>is unnecessary sinceLootItem.lootTableItem(item)already returns that type.♻️ Proposed refactor
- public LootTable.Builder AverageOreDrops(Block drop, Item item) { + public LootTable.Builder averageOreDrops(Block drop, Item item) { HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(UniformGenerator.between(2, 5)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); + return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, + LootItem.lootTableItem(item) + .apply(SetItemCountFunction.setCount(UniformGenerator.between(2, 5))) + .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); } - public LootTable.Builder SingleOreDrops(Block drop, Item item) { + public LootTable.Builder singleOreDrops(Block drop, Item item) { HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(ConstantValue.exactly(1)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); + return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, + LootItem.lootTableItem(item) + .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); } - public LootTable.Builder LightOreDrops(Block drop, Item item) { + public LootTable.Builder lightOreDrops(Block drop, Item item) { HolderLookup.RegistryLookup<Enchantment> impl = this.registries.lookupOrThrow(Registries.ENCHANTMENT); - return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, ((LootPoolSingletonContainer.Builder<?>) - LootItem.lootTableItem(item).apply(SetItemCountFunction.setCount(UniformGenerator.between(1, 2)))) - .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); + return this.createSilkTouchDispatchTable(drop, this.applyExplosionDecay(drop, + LootItem.lootTableItem(item) + .apply(SetItemCountFunction.setCount(UniformGenerator.between(1, 2))) + .apply(ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE))))); }🤖 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/ModLootTableProvider.java` around lines 119 - 138, The three methods AverageOreDrops, SingleOreDrops, and LightOreDrops violate Java naming conventions and contain small redundancies; rename them to camelCase (averageOreDrops, singleOreDrops, lightOreDrops) throughout the class and any callers, and in singleOreDrops remove the redundant SetItemCountFunction.setCount(ConstantValue.exactly(1)) call (default count is 1) and drop the unnecessary cast to LootPoolSingletonContainer.Builder<?> around LootItem.lootTableItem(item); keep the existing logic that uses registries.lookupOrThrow(Registries.ENCHANTMENT) and ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE)) and ensure createSilkTouchDispatchTable and applyExplosionDecay usages remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 73-77: In ModLootTableProvider, remove the redundant HALF property
predicate from the LootItemBlockStatePropertyCondition for
ModBlocks.WORKBENCH_WORKBENCH_BLOCK: delete the
.hasProperty(AbstractWorkbench.HALF, DoubleBlockHalf.LOWER) call in the
StatePropertiesPredicate.Builder so only the varying property
.hasProperty(AbstractWorkbench.TYPE, ChestType.LEFT) remains; this keeps the
loot condition accurate and removes unnecessary noise while leaving
StatePropertiesPredicate.Builder and the WORKBENCH_WORKBENCH_BLOCK condition
intact.
---
Nitpick comments:
In `@src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java`:
- Around line 119-138: The three methods AverageOreDrops, SingleOreDrops, and
LightOreDrops violate Java naming conventions and contain small redundancies;
rename them to camelCase (averageOreDrops, singleOreDrops, lightOreDrops)
throughout the class and any callers, and in singleOreDrops remove the redundant
SetItemCountFunction.setCount(ConstantValue.exactly(1)) call (default count is
1) and drop the unnecessary cast to LootPoolSingletonContainer.Builder<?> around
LootItem.lootTableItem(item); keep the existing logic that uses
registries.lookupOrThrow(Registries.ENCHANTMENT) and
ApplyBonusCount.addOreBonusCount(impl.getOrThrow(Enchantments.FORTUNE)) and
ensure createSilkTouchDispatchTable and applyExplosionDecay usages remain
unchanged.
Comment on lines
+73
to
+77
| .when(LootItemBlockStatePropertyCondition.hasBlockStateProperties(ModBlocks.WORKBENCH_WORKBENCH_BLOCK) | ||
| .setProperties(StatePropertiesPredicate.Builder.properties() | ||
| .hasProperty(AbstractWorkbench.HALF, DoubleBlockHalf.LOWER) | ||
| .hasProperty(AbstractWorkbench.TYPE, ChestType.LEFT) | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine AbstractWorkbench to understand HALF property registration
fd -e java "AbstractWorkbench" --type f -exec cat -n {} +Repository: CodeMonkeysMods/MineTale
Length of output: 298
🏁 Script executed:
# Find WORKBENCH_WORKBENCH_BLOCK definition
fd -e java . | xargs rg -l "WORKBENCH_WORKBENCH_BLOCK" | head -20Repository: CodeMonkeysMods/MineTale
Length of output: 535
🏁 Script executed:
# Check ModBlocks to see WORKBENCH_WORKBENCH_BLOCK registration
fd -e java "ModBlocks" --type f -exec cat -n {} +Repository: CodeMonkeysMods/MineTale
Length of output: 298
🏁 Script executed:
# Find AbstractWorkbench file
fd "AbstractWorkbench" --type fRepository: CodeMonkeysMods/MineTale
Length of output: 314
🏁 Script executed:
# Read ModBlocks.java to see WORKBENCH_WORKBENCH_BLOCK definition
cat src/main/java/com/tcm/MineTale/registry/ModBlocks.javaRepository: CodeMonkeysMods/MineTale
Length of output: 30000
🏁 Script executed:
# Check the size of ModLootTableProvider to read it properly
wc -l src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.javaRepository: CodeMonkeysMods/MineTale
Length of output: 138
🏁 Script executed:
# Read AbstractWorkbench to understand HALF property registration
cat src/main/java/com/tcm/MineTale/block/workbenches/AbstractWorkbench.javaRepository: CodeMonkeysMods/MineTale
Length of output: 18822
🏁 Script executed:
# Read WorkbenchWorkbench to verify IS_TALL/IS_WIDE configuration
cat src/main/java/com/tcm/MineTale/block/workbenches/WorkbenchWorkbench.javaRepository: CodeMonkeysMods/MineTale
Length of output: 4014
🏁 Script executed:
# Read ModLootTableProvider fully
cat src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.javaRepository: CodeMonkeysMods/MineTale
Length of output: 7984
Remove spurious HALF predicate from WORKBENCH_WORKBENCH_BLOCK — the condition is redundant but harmless.
While the HALF property is indeed registered on WORKBENCH_WORKBENCH_BLOCK (AbstractWorkbench always adds it regardless of IS_TALL), the condition checking for HALF = LOWER is redundant. Since IS_TALL = false, the block can only ever have HALF = LOWER and will never have an upper half. The predicate will work, but it adds unnecessary noise to the loot table. Simplify by keeping only the TYPE = LEFT condition, which is the only property that varies for a wide-only multi-block.
Suggested cleanup
this.add(ModBlocks.WORKBENCH_WORKBENCH_BLOCK,
LootTable.lootTable()
.withPool(LootPool.lootPool()
.setRolls(ConstantValue.exactly(1.0F))
.add(LootItem.lootTableItem(ModBlocks.WORKBENCH_WORKBENCH_BLOCK))
.when(LootItemBlockStatePropertyCondition.hasBlockStateProperties(ModBlocks.WORKBENCH_WORKBENCH_BLOCK)
.setProperties(StatePropertiesPredicate.Builder.properties()
- .hasProperty(AbstractWorkbench.HALF, DoubleBlockHalf.LOWER)
.hasProperty(AbstractWorkbench.TYPE, ChestType.LEFT)
)
)
.when(ExplosionCondition.survivesExplosion())
)
);🤖 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/ModLootTableProvider.java` around
lines 73 - 77, In ModLootTableProvider, remove the redundant HALF property
predicate from the LootItemBlockStatePropertyCondition for
ModBlocks.WORKBENCH_WORKBENCH_BLOCK: delete the
.hasProperty(AbstractWorkbench.HALF, DoubleBlockHalf.LOWER) call in the
StatePropertiesPredicate.Builder so only the varying property
.hasProperty(AbstractWorkbench.TYPE, ChestType.LEFT) remains; this keeps the
loot condition accurate and removes unnecessary noise while leaving
StatePropertiesPredicate.Builder and the WORKBENCH_WORKBENCH_BLOCK condition
intact.
Contributor
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @The-Code-Monkey. * #49 (comment) The following files were modified: * `src/client/java/com/tcm/MineTale/MineTaleDataGen.java` * `src/client/java/com/tcm/MineTale/datagen/ModBlockTagProvider.java` * `src/client/java/com/tcm/MineTale/datagen/ModLootTableProvider.java` * `src/main/java/com/tcm/MineTale/MineTale.java` * `src/main/java/com/tcm/MineTale/block/workbenches/AbstractWorkbench.java` * `src/main/java/com/tcm/MineTale/util/ModLootTableModifiers.java`
…1072ef 📝 Add docstrings to `Loot-Table-Update`
The-Code-Monkey
approved these changes
Feb 20, 2026
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.
Workbenchs drops are a bit broken
Summary by CodeRabbit
New Features
Bug Fixes