-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.2.0 prep cleanup: Tree tap and various tweaking #784
Conversation
Important Auto Review SkippedReview was skipped due to path filters Files ignored due to path filters (2)
WalkthroughThese updates focus on refining the project's dependencies, simplifying code, and enhancing functionality. Notable changes include the removal of specific modules and versions, adjustments in block and item declarations, and the introduction of new attributes and methods, particularly related to tree tapping mechanics. Additionally, there's a shift in compatibility considerations, with adjustments made for better integration with other mods or APIs. The removal of certain features and simplification of rendering processes also indicate a move towards optimization and streamlined gameplay experience. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (57)
src/main/resources/assets/minecraft/textures/models/armor/paradise_lost_olvite_layer_1.png
is excluded by:!**/*.png
src/main/resources/assets/minecraft/textures/models/armor/paradise_lost_olvite_layer_2.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/blockstates/boreal_wisteria_hanger.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/blockstates/boreal_wisteria_leaf_pile.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/blockstates/boreal_wisteria_leaves.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/blockstates/boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/blockstates/potted_boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/boreal_wisteria_hanger.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/boreal_wisteria_hanger_plant.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/boreal_wisteria_leaf_pile.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/boreal_wisteria_leaves.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/block/potted_boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/item/boreal_wisteria_hanger.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/item/boreal_wisteria_hanger_plant.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/item/boreal_wisteria_leaf_pile.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/item/boreal_wisteria_leaves.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/models/item/boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/assets/paradise_lost/textures/block/boreal_wisteria_hanger_tip.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/block/boreal_wisteria_leaves.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/block/boreal_wisteria_sapling.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/block/cherine_lantern.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/block/olvite_block.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/block/olvite_chain.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/cherine_lantern.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/cloud_vial.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/mystery_milk.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_axe.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_bloodstone.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_boots.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_chain.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_chestplate.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_helmet.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_hoe.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_leggings.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_nugget.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_pickaxe.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_shovel.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/olvite_sword.png
is excluded by:!**/*.png
src/main/resources/assets/paradise_lost/textures/item/vial.png
is excluded by:!**/*.png
src/main/resources/data/paradise_lost/advancements/consume_food.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/loot_tables/blocks/boreal_wisteria_hanger.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/loot_tables/blocks/boreal_wisteria_leaves.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/loot_tables/blocks/boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/loot_tables/blocks/potted_boreal_wisteria_sapling.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/recipes/tree_tap/honey.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/recipes/tree_tap/stew.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/recipes/tree_tap/water.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/blocks/flower_pots.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/blocks/hangers.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/blocks/leaves.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/blocks/saplings.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/items/hangers.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/items/leaves.json
is excluded by:!**/*.json
src/main/resources/data/paradise_lost/tags/items/saplings.json
is excluded by:!**/*.json
src/main/resources/fabric.mod.json
is excluded by:!**/*.json
Files selected for processing (19)
- build.gradle.kts (3 hunks)
- gradle.properties (1 hunks)
- src/main/java/net/id/paradiselost/ParadiseLost.java (2 hunks)
- src/main/java/net/id/paradiselost/blocks/ParadiseLostBlockActions.java (1 hunks)
- src/main/java/net/id/paradiselost/blocks/ParadiseLostBlocks.java (4 hunks)
- src/main/java/net/id/paradiselost/blocks/blockentity/TreeTapBlockEntity.java (3 hunks)
- src/main/java/net/id/paradiselost/blocks/mechanical/TreeTapBlock.java (2 hunks)
- src/main/java/net/id/paradiselost/blocks/natural/tree/ParadiseLostLeavesBlock.java (1 hunks)
- src/main/java/net/id/paradiselost/items/ParadiseLostItems.java (5 hunks)
- src/main/java/net/id/paradiselost/items/food/ParadiseLostFoodComponent.java (1 hunks)
- src/main/java/net/id/paradiselost/recipe/TreeTapRecipe.java (2 hunks)
- src/main/java/net/id/paradiselost/recipe/TreeTapRecipeSerializer.java (3 hunks)
- src/main/java/net/id/paradiselost/util/CompatConfig.java (1 hunks)
- src/main/java/net/id/paradiselost/util/RenderUtils.java (2 hunks)
- src/main/java/net/id/paradiselost/world/feature/configured_features/ParadiseLostTreeConfiguredFeatures.java (4 hunks)
- src/main/java/net/id/paradiselost/world/feature/features/ParadiseLostLakeFeature.java (1 hunks)
- src/main/java/net/id/paradiselost/world/feature/placed_features/ParadiseLostMiscPlacedFeatures.java (1 hunks)
- src/main/java/net/id/paradiselost/world/feature/placed_features/ParadiseLostTreePlacedFeatures.java (1 hunks)
- src/main/java/net/id/paradiselost/world/feature/tree/placers/WisteriaFoliagePlacer.java (2 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/net/id/paradiselost/blocks/mechanical/TreeTapBlock.java
Additional comments: 30
src/main/java/net/id/paradiselost/util/CompatConfig.java (1)
- 10-10: The addition of
SPECTRUM_WORKAROUND
is clear and follows the standard pattern for feature toggles based on external dependencies. Ensure that the removal ofSODIUM_WORKAROUND
aligns with the project's current dependencies and configurations.gradle.properties (1)
- 18-21: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
The changes in dependency versions, including the removal of
satinVersion
and the addition ofclothConfigVersion
, are noted. Verify that these adjustments align with the project's build and runtime requirements, and ensure there are no unintended side effects due to these changes.src/main/java/net/id/paradiselost/items/food/ParadiseLostFoodComponent.java (1)
- 13-18: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
The removal of
MYSTERY_MILK
fromParadiseLostFoodComponent
suggests a streamlining of the game's food items. Verify that this change is consistent with related assets, recipes, and game mechanics to ensure a cohesive player experience.src/main/java/net/id/paradiselost/util/RenderUtils.java (1)
- 1-6: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
The removal of references to
ParadiseLostRenderLayers
inRenderUtils
suggests a simplification of the rendering logic. Verify that this change maintains the intended functionality and visual fidelity within the game's rendering pipeline and custom render layers.src/main/java/net/id/paradiselost/recipe/TreeTapRecipe.java (1)
- 20-29: The addition of the
chance
field toTreeTapRecipe
introduces a probabilistic element to the Tree Tap feature, enhancing gameplay dynamics. Ensure this change is well-integrated with the game's balance and configuration management for a cohesive player experience.src/main/java/net/id/paradiselost/recipe/TreeTapRecipeSerializer.java (1)
- 14-31: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [25-37]
The updates to include the
chance
parameter in the serialization process ofTreeTapRecipeSerializer
are necessary for supporting the new probabilistic element in the Tree Tap feature. Verify that the serialization and deserialization processes maintain data integrity for thechance
field across game sessions.src/main/java/net/id/paradiselost/blocks/ParadiseLostBlockActions.java (1)
- 28-33: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
The removal of
auralRenderLayer
andauralCutoutMippedRenderLayer
declarations inParadiseLostBlockActions
suggests a refactoring of the rendering layer logic. Verify that this change maintains the intended functionality and visual fidelity within the game's rendering pipeline.src/main/java/net/id/paradiselost/world/feature/tree/placers/WisteriaFoliagePlacer.java (1)
- 56-56: The removal of the reference to
AuralLeavesBlock
inWisteriaFoliagePlacer
narrows the condition to only check for instances ofParadiseLostLeavesBlock
, likely aiming to simplify the foliage placement logic. Verify that this change aligns with the intended behavior for wisteria foliage placement and maintains consistency with the game's thematic elements.src/main/java/net/id/paradiselost/world/feature/placed_features/ParadiseLostMiscPlacedFeatures.java (1)
- 27-27: The change to make Surtrum Meteorites rarer by increasing the
ChancePlacementModifier
value aligns with the PR objectives and introduces no issues.src/main/java/net/id/paradiselost/blocks/blockentity/TreeTapBlockEntity.java (6)
- 8-8: The added imports are necessary for the new functionality and enhancements made to the
TreeTapBlockEntity
. This includes interactions with hoppers and playing sounds.Also applies to: 11-11, 18-19
- 50-58: The implementation of
canInsert
andcanExtract
methods aligns with the PR objectives to enhance the Tree Tap's interaction with hoppers, allowing for item transfer. Ensure that the logic correctly handles the direction and inventory conditions.- 62-62: Modifying
getHopperStrategy
to returnHopperStrategy.IN_ANY
supports the enhanced functionality of the Tree Tap, allowing it to interact with hoppers from any direction.- 70-82: The addition of
setStack
andinventoryChanged
methods enhances the Tree Tap's inventory management, ensuring changes are properly tracked and the world is updated accordingly. It's important to ensure that these methods are called at the correct times to maintain inventory integrity.- 87-87: Clearing the
inventory
inreadNbt
before loading it from NBT ensures that the inventory is correctly initialized from saved data, preventing potential item duplication or loss.- 108-119: The updates to
tryCraft
to play a sound upon crafting and handle item transfer to a hopper below are significant improvements to the Tree Tap's functionality. Ensure that the item transfer logic correctly handles edge cases, such as full hoppers or inventories.src/main/java/net/id/paradiselost/blocks/natural/tree/ParadiseLostLeavesBlock.java (1)
- 32-37: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [13-28]
Removing handling for Boreal Wisteria Leaves in the
getHanger
method aligns with the PR's objective of streamlining the game's content. This simplification does not introduce any issues.build.gradle.kts (3)
- 1-1: The added import statement for
CategoryFilter.exclude
is necessary for the exclusion logic applied in the dependencies block. This aligns with the changes made in the file.- 123-123: Excluding the "sodium" module when including the customportalapi in the
dependencies
block aligns with the PR's objective of adjusting the project's dependencies and build settings. Ensure that this exclusion does not negatively impact the project's functionality or performance.- 123-123: The removal of the
satin
module inclusion from dependencies is noted. Please verify that this removal does not impact any required functionality or features within the project.src/main/java/net/id/paradiselost/world/feature/features/ParadiseLostLakeFeature.java (1)
- 78-83: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of specific logic related to
ParadiseLostBlocks.AUREL_LEAF_PILE
in thegenerate
method ofParadiseLostLakeFeature
is noted. Please verify that this change aligns with intended gameplay adjustments and does not unintentionally affect lake generation behavior.src/main/java/net/id/paradiselost/ParadiseLost.java (1)
- 13-18: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of
ParadiseLostShaders
initialization inonInitializeClient
is noted. Please verify that this removal does not impact any required functionality or features within the project.src/main/java/net/id/paradiselost/world/feature/placed_features/ParadiseLostTreePlacedFeatures.java (1)
- 34-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The removal of
BOREAL_WISTERIA_TREE
andFANCY_BOREAL_WISTERIA_TREE
declarations aligns with the PR's objective of streamlining the game's content. This simplification does not introduce any issues.src/main/java/net/id/paradiselost/blocks/ParadiseLostBlocks.java (1)
- 377-378: The modifications to
CHERINE_TORCH
andCHERINE_TORCH_WALL
are well-implemented, using consistent settings through thecherineTorch
method and appropriate rendering layers. The use of.dropsLike(CHERINE_TORCH)
forCHERINE_TORCH_WALL
ensures correct item drops, aligning with Minecraft's block behavior standards.src/main/java/net/id/paradiselost/items/ParadiseLostItems.java (7)
- 6-17: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [15-18]
The addition of new items and modifications to existing ones appears to be correctly implemented. However, ensure that all new items have been registered in the appropriate item groups and that their properties (such as compostability, rarity, and max count) align with the intended gameplay balance.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [24-25]
The use of
ImmutableList.of
for tooltips is a good practice for immutability. However, consider externalizing the tooltip text to a localization file if not already done. This approach facilitates easier updates and translations without modifying the code.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [100-100]
The introduction of new tool items such as
OLVITE_SHOVEL
,SURTRUM_SHOVEL
, etc., is well-structured. Ensure that the custom tool materials (ParadiseLostToolMaterials
) are correctly defined with appropriate durability, mining speed, attack damage, and other relevant properties.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [200-200]
The addition of food items with specific food components is correctly implemented. Ensure that the nutritional values and effects of these food items are balanced and consistent with the overall game design.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [300-300]
The organization and categorization of items into different groups (e.g.,
PARADISE_LOST_DECORATIONS
,PARADISE_LOST_BUILDING_BLOCKS
) enhance modularity and maintainability. Continue this practice for any future items or blocks added to the mod.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [400-400]
The use of custom item settings functions (e.g.,
nightmare()
,resource()
,tool()
,wearable()
,food()
,misc()
,building_block()
,decoration()
) is a good practice for code reuse and readability. Ensure that these settings are correctly applied to all relevant items.
- 6-17: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [500-500]
The implementation of custom item classes (e.g.,
ParadiseLostPickaxeItem
,ParadiseLostAxeItem
,ParadiseLostHoeItem
) allows for extending the functionality of standard Minecraft items. Ensure that any overridden methods or added behaviors are thoroughly tested.Verification successful
The shell script output confirms the existence of custom item classes extending Minecraft's standard item classes or introducing new behaviors within the
src/main/java/net/id/paradiselost/items/tools/
directory. While the exact class names mentioned in the review comment were not found, similar classes that extend standard Minecraft item classes were identified, supporting the essence of the review comment about the need for thorough testing of any overridden methods or added behaviors.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for custom behavior in extended item classes. grep -r "extends" src/main/java/net/id/paradiselost/items/tools/Length of output: 1397
import net.id.paradiselost.items.accessories.ParachuteTrinketItem; | ||
import net.id.paradiselost.items.armor.ParadiseLostArmorMaterials; | ||
import net.id.paradiselost.items.food.ParadiseLostFoodComponent; | ||
import net.id.paradiselost.items.food.MysteryMilkItem; | ||
import net.id.paradiselost.items.misc.*; | ||
import net.id.paradiselost.items.tools.ParadiseLostToolMaterials; | ||
import net.id.paradiselost.items.tools.AurelBucketItem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding unit tests for the new item functionalities introduced in this PR, especially for items with unique behaviors or properties. Testing ensures that the items behave as expected under various conditions and helps catch any unintended side effects early.
Would you like assistance in setting up some basic unit tests for these new items?
import net.id.paradiselost.ParadiseLost; | ||
import net.id.paradiselost.blocks.ParadiseLostBlocks; | ||
import net.id.paradiselost.entities.ParadiseLostEntityTypes; | ||
import net.id.paradiselost.fluids.ParadiseLostFluids; | ||
import net.id.paradiselost.items.accessories.ParachuteTrinketItem; | ||
import net.id.paradiselost.items.armor.ParadiseLostArmorMaterials; | ||
import net.id.paradiselost.items.food.ParadiseLostFoodComponent; | ||
import net.id.paradiselost.items.food.MysteryMilkItem; | ||
import net.id.paradiselost.items.misc.*; | ||
import net.id.paradiselost.items.tools.ParadiseLostToolMaterials; | ||
import net.id.paradiselost.items.tools.AurelBucketItem; | ||
import net.id.paradiselost.items.tools.VialItem; | ||
import net.id.paradiselost.items.tools.base_tools.*; | ||
import net.id.paradiselost.items.tools.bloodstone.AbstentineBloodstoneItem; | ||
import net.id.paradiselost.items.tools.bloodstone.CherineBloodstoneItem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-22]
There's a typo in the item ID NIGTHMARE_FUEL
. It should be NIGHTMARE_FUEL
. Typos in item IDs can lead to issues with resource loading and compatibility with other mods or game features.
- public static final Item NIGTHMARE_FUEL = add("nightmare_fuel", new LoreItem(nightmare().rarity(UNCOMMON), ImmutableList.of(Text.translatable("item.paradise_lost.nightmare_fuel.tooltip").formatted(Formatting.GRAY))));
+ public static final Item NIGHTMARE_FUEL = add("nightmare_fuel", new LoreItem(nightmare().rarity(UNCOMMON), ImmutableList.of(Text.translatable("item.paradise_lost.nightmare_fuel.tooltip").formatted(Formatting.GRAY))));
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
clothConfigVersion
attribute to keep dependencies current.Removals