Skip to content

Allow florist to use shears on tall grass and ferns #10930

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

Open
wants to merge 6 commits into
base: version/main
Choose a base branch
from

Conversation

pop1040
Copy link

@pop1040 pop1040 commented Jun 8, 2025

Closes #10019

Added as setting rather than hard requirement, in case the player doesn't need tall grass or ferns and would rather not waste iron on shears (or would explicitly like the byproducts of breaking grass without shearing)

Changes proposed in this pull request

  • Adds setting to flower shop to use shears
  • Modifies Florist AI to request shears only if use shears is enabled
  • Adds check to automatically dismiss requests for shears if the setting has been turned back off

Testing

  • Yes I tested this before submitting it.
  • I also did a multiplayer test.

Review please

@CLAassistant
Copy link

CLAassistant commented Jun 8, 2025

CLA assistant check
All committers have signed the CLA.

return IDLE;
}
}

worker.setItemInHand(InteractionHand.MAIN_HAND, ItemStack.EMPTY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this have to switch to shears in hand if available then?

Copy link
Author

@pop1040 pop1040 Jun 8, 2025

Choose a reason for hiding this comment

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

I was just debugging this and found that while in development they did, on a new world they didn't, so I'm gonna have to debug this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's also fine to make it a fixed requirement. It makes sense thematically to use shears on flowers too and it's a mid-late game worker anyway.

Copy link
Author

Choose a reason for hiding this comment

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

So after further testing the invocation of

        if (!mineBlock(harvestPosition))

is responsible for automatically selecting the correct tool as well as breaking the block, as used in the EntityAIWorkLumberjack.java and lots of other things, however the reason it wasn't working in further testing, is because it will correctly select shears only for 1 block tall tall grass and ferns, but not 2 block tall ferns and tall grass. I will work on fixing the tool selection for double tall grass and ferns.

Copy link
Author

@pop1040 pop1040 Jun 9, 2025

Choose a reason for hiding this comment

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

Interestingly the mineBlock call always requests the right tool, but doesn't mark it as required for grass and ferns, so shears aren't requested. It also determines if shears are the right tool by checking if the block implements IForgeShearable (but notably it doesn't call isShearable, just checks if it's an instance of IForgeShearable) in WorkerUtil in the getBestToolForBlock method (line 143), and while ferns and grass implement IForgeShearable, double tall ferns and grass do not implement it, they are merely instances of DoublePlantBlock which doesn't implement the interface. Instead they handle shearing with a custom loot table forge/data/minecraft/loot_tables/blocks/large_fern.json

Copy link
Author

Choose a reason for hiding this comment

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

Not sure if it's a good idea/ok to add a manual check there like (state.getBlock() instanceof IForgeShearable || state.getBlock() == Blocks.LARGE_FERN || state.getBlock() == Blocks.TALL_GRASS) or if I need to delve into the loot table mechanic and check for custom rules on shearing (or if I should change the florist to not use mineBlock and instead do something custom) but I found that even when using shears, they still don't trigger the custom loot table for mining with shears, will have to debug further.

@Raycoms
Copy link
Contributor

Raycoms commented Jun 9, 2025 via email

@someaddons
Copy link
Contributor

I actually want the florist to use shears for everything, its his working tool and as such contributes to his working cost/requirements. Its a small cost to pay for flowers which you normally cannot grow at all in vanilla

@Shadizar
Copy link

Several flowers (not all, true) can be bonemealed in vanilla fyi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Florist not harvesting ferns and grass
5 participants