DispenserItemBehavior for flint and brick.#5029
DispenserItemBehavior for flint and brick.#5029Waterpicker wants to merge 2 commits intoSlimeKnights:1.18.2from
Conversation
KnightMiner
added
Feature
|
General comments, will leave specific code comments later
|
| * @param stack Stack being dispensed | ||
| * @return True if dispenser action was taken | ||
| */ | ||
| public boolean onDispenserUse(IToolStackView tool, int level, BlockSource source, ItemStack stack) { |
There was a problem hiding this comment.
Do we need the item stack parameter? I generally leave it out of modifier hooks as people should be using the tool stack.
Looks like you use it to create a use on context below if I am understanding the code, double check if that is avoidable, if not modify the javadoc to be clear about the relationship between the tool and the stack and that the stack should not be used to create a ToolStack instance.
| Direction width, height; | ||
| // for Y, direction is based on facing | ||
| if (sideHit.getAxis() == Direction.Axis.Y) { | ||
| height = harvestDirection; |
There was a problem hiding this comment.
To be honest, the harvest direction parameter does not matter for circles. It matters for a rectangle as it determines the rectangle orientation and was conveient to reuse the rectangle logic for a circle, but if you are going to extract it I would just ditch the side/player argument entirely (could be accomplished by making player nullable in the above method signature then ignoring it here)
| return Item.getPlayerPOVHitResult(worldIn, player, fluidMode); | ||
| } | ||
|
|
||
| public static UseOnContext contextFromBlockSource(BlockSource source, ItemStack stack) { |
There was a problem hiding this comment.
Needs javadoc, and probably belongs in ModifierUtils or one of those similar helper classes. Above method is only here as getPlayerPOVHitResult is protected.
| return stack; | ||
| }; | ||
|
|
||
| DispenserBlock.registerBehavior(TinkerTools.flintAndBrick, behavior); |
There was a problem hiding this comment.
Flint and steel fires particles if its unable to do anything. Looks like they use a class called OptionalDispenseItemBehavior for that fallback particle, I suggest using that
|
|
||
| // AOE transforming, run even if we did not transform the center | ||
| // note we consider anything effective, as hoes are not effective on all tillable blocks | ||
| // if (player != null && !tool.isBroken()) { |
There was a problem hiding this comment.
Lets just hardcode to circle AOE here, using the same logic you were planning to use in fireprimer.
| } | ||
|
|
||
| /** Ignites the given block */ | ||
| private static boolean igniteDispenser(IToolStackView tool, Level world, BlockPos pos, BlockState state, Direction sideHit, Direction horizontalFacing) { |
There was a problem hiding this comment.
Why did you duplicate this whole method just to remove a nullable player argument? Just call the original method by passing null.
If there are any other differences, add the needed parameter to distinguish the two
KnightMiner
added
the
Needs Update
2 participants
No description provided.