Harvest with ease

Harvest with ease

15M Downloads

[Bug]May not compat with TFC in 1.18.2

Tollainmear opened this issue · 10 comments

commented

Describe the bug
right click on a full grown wheat plant with hoe, the wheat indeed be harvest, but the plant block was not be fully reseted, so you can see the info 'Mature' tag was still in HUD, and in the next random tick, the wheat just fully grown again.

To Reproduce
Make sure the test env is TerraFirmaCraft 1.18.2-2.2.32
plant a seed and wait it be grown
right click it with hoe
you will see the info 'Mature' in HUD was not reset
and just wait for the next random tick to make it update, the wheat was fully grown immediately

Expected behavior
see my screen shot below.

Screenshots
image

Stacktrace and logs
If applicable, copy and paste relevant logs.

System information:

  • Forge
  • harvest with easy 8.0.0.0
  • TFC 2.2.32
  • 1.18.2

Additional context
Terra firma craft

commented

@alcatrazEscapee I'm coming back to this since I'm refractoring the mod to employ a multiloader environment.
What do you suggest to make our mods compatible?
I am adding a configuration option to blacklist crops, so at least that's something that the end user can do.

What about ditching the events and instead exposing an interface (automatically applied to the base CropBlock class) with methods one can optionally override to tweak the harvest behavior?
This way it could be very easy to both override default behavior without duplicating logic, and also disable harvest for certain crops (for example overriding a method canHarvest and always return false).
It would still require an optional explicit dependency, but maybe it's better than subscribing to a few different events.

Let me know if you have any suggestion, thank you.

commented

Exactly like I said above,

  • Any way for the user to blacklist certain crops (seriously this becomes a "implement the API in the target mod" issue?) No config?
  • Any soft dependency way, i.e. via a tag, to blacklist certain crops.

Add a config to blacklist crops; and a block tag to blacklist crops. Both are trivially implemented.

commented

Yeah, I am going to do both.

I was wondering, however, if there was a way that could make it easier for you, and thus other modders too, to add "proper" compatibility rather than just disabling the feature.

commented

Since v9.0.0.3-beta it's possible to add either a configuration option or a datapack to blacklist certain crops from interacting with this mod.
This can "solve" any incompatibility by blacklisting the crops that do not work.
Still, to add proper compatibility, the other mod would need to implement HWE events.

Closing this for now.

commented

Yep, here's additional info about this problem:
I've removed every mod and just left these three mod:

  • TFC
  • Patchouli(the lib for TFC)
  • Harvest with ease
    The problem still exists.

And I found that you don't need to wait for a next random tick to prove this problem, just quit the world after harvesting then rejoin it, the plant which you just harvested was fully grown and the wheat you just obtained is also in your inventory.

commented

Most likely you need to open an issue with TFC and ask of them to add compatibility with HWE by using the provided API (I think they need to be using the AfterHarvest event and reset other properties other than the age).

Anyway, as soon as I can I will let you know more, thank you in the meantime!

commented

I confirm that to fix the bug it's needed for TFC to use HWE API.
If you open an issue with TFC, please link it here.

commented

I confirm that to fix the bug it's needed for TFC to use HWE API. If you open an issue with TFC, please link it here.

okay, thanks for your work, I'll make a report to tfc and paste link here later~

commented

Frankly, given the description "works with all modded crops", the solution "the mod has to implement an API to handle checks notes a single right click" is not a solution. This mod runs into all sorts of issues trying to harvest TFC crops, including:

  1. When breaking a tall crop from the bottom, with a stick, it duplicates the stick, leaves it in place, and breaks the crop

2023-12-10_10 35 40

  1. When breaking a tall crop from the top with a stick, it drops nothing and just nukes the block:

2023-12-10_10 38 49

  1. It breaks the behavior of our crops that already have right click functionality (namely, picking without completely resetting the growth).
  2. All crops broken by this mod instantly regrow, because it just assumes that "set any property named age" is a sufficient replacement for "break the block and replant it". Which is is NOT.

Given all these issues, there's no way in hell I am introducing a soft dependency on this mod, subscribing to three separate bouncer events, preventing all manner of broken behavior. At best, I would like to disable this mod from working at all on TFC crops because it would be drastically easier to just implement right-click harvest within TFC. But this mod also seems to lack:

  • Any way for the user to blacklist certain crops (seriously this becomes a "implement the API in the target mod" issue?) No config?
  • Any soft dependency way, i.e. via a tag, to blacklist certain crops.

If harvest with ease is duplicating crops, sticks, in TFC, harvest with ease should be fixing it, or providing a way for the user to fix it. "Fix it yourself in your mod with our API" is not that.

commented

@alcatrazEscapee thank you for sharing your point of view.
First of all you're right, so I will change "all modded crops" in "almost all modded crops".

  • For 1. & 2.: For tall crops it's assumed that all blocks are of the same kind (i.e. a tomato block is always a tomato block, regardless of whether it's at the bottom, in the middle, or at the top). Correct me if I'm mistaken, but TFC tall crops do not work like that. Unfortunately, I can't think of a better assumption that works with many modded crops (like the current one does), but that improves the situation (recognizes also tall crops that are not made of the same block and either does nothing or harvests them correctly), but if you have one let me know. The drops are calculated using the right-clicked crop or, in case of tall crops, the base block. Unfortunately, since your tall crops are not recognized as such, the right-clicked crop is used to get the drops, which results in wrong ones.
  • For 3.: Again, it's because of an assumption that works fine most of the times, which is that crops reset their age to 0 when harvested (because it's the same thing that would happen if you break and replant the crop).
  • For 4.: Once again, assumption that works in all other cases, but if you have a better way to reset a crop I'm all ears.

Generally all these points can be easily solved by implementing HWE API:
1 & 2 can be solved by either preventing the harvest of your tall crops (RightClickHarvestCheck event) (and optionally implement your logic to handle right click that correctly harvests the crop) or changing the drops (HarvestDrops event) and the aftermath of the harvest (AfterHarvest event).
3 can be solved by preventing the harvest (RightClickHarvestCheck event).
4 can be solved by resetting the other properties added by TFC (AfterHarvest event).

As I see it, there should be no need for a blacklist option as no crop should have a reason not to work. If a mod needs to add/tweak specific behaviors, the API exists for this purpose. For a better user experience and consistency, I also think it's best if other mods are required to add a proper right-click harvest behavior rather than just saying "ignore these crops".