Crystal-Nest / harvest-with-ease

Don't break crops, right click them!
https://modrinth.com/mod/harvest-with-ease
GNU General Public License v3.0
5 stars 1 forks source link

Flan claims incompat (Forge) #14

Closed Flemmli97 closed 1 year ago

Flemmli97 commented 1 year ago

Describe the bug Claimed areas get ignored and you can still harvest crops there despite it being protected

To Reproduce Create a claim with the flan mod. Transfer claim to another player (so you dont have perms anymore). You can still harvest crops there

Expected behavior Harvesting crops are prevented

System information:

Additional context Because i dont cancel the RightClickBlock but instead disable item use and block use you would also need to check for RightClickBlock#getUseBlock and RightClickBlock#getUseItem

Crystal-Spider commented 1 year ago

Working on it

Crystal-Spider commented 1 year ago

@Flemmli97 would you mind telling me what I should check in the Fabric version? In Forge I can check the use block, use item or result, but in Fabric what should I check to prevent harvesting in a claimed area?

Crystal-Spider commented 1 year ago

It seems, from what I could understand from your code, that in the Fabric version you have a Mixin to return InteractionResult.PASS if the action should not be accomplished. However, if this is really the only change that happens, I have no idea how to check in Fabric whether I should proceed with handling the event or returning PASS too.
What do you suggest @Flemmli97 ?

Flemmli97 commented 1 year ago

ye sadly the fabric event is not as extensive as the forge one and as such i had to mimic the forge behaviour. this is used to decide what is allowed or not. i changed it in ver 1.8.4 to make it more accessible https://github.com/Flemmli97/Flan/blob/1.19/fabric/src/main/java/io/github/flemmli97/flan/api/fabric/ItemUseBlockFlags.java

Crystal-Spider commented 1 year ago

So, without me doing anything on Fabric, now when trying to harvest I get the message "Sorry, you can't do that here!" but despite this the right-click harvest still happens.
My guess is that the order in which the events are fired messes things up, likely calling first my callback (resulting in harvesting) and then yours (resulting in the message). However this seems weird because if indeed my callback were to be called first then yours shouldn't be called at all since I return SUCCESS.
So my second guess is that maybe it's your callback getting called first and then mine, but yours doesn't return FAIL as it should for some reason. Maybe instead of doing if (!flags.allowUseBlocks() && !flags.allowUseItems()) you should do if (!flags.allowUseBlocks() || !flags.allowUseItems()) ? I'm not sure. Also this shouldn't matter much in my test setup because I'm transferring all claimed areas to another player, removing all of my permissions. Any insight?
I was testing using a local world open to LAN, trying to right-click harvest a crop in another player's claimed area. Minecraft 1.19.2, Fabric loader 14.14, HWE 4.0.0.3 and Flan 1.8.4

Flemmli97 commented 1 year ago

there is 2 things that happen when player right clicks on a block. if they hold an item BlockState#use is called and if that does nothing ItemStack#useOn. The first one is more like if the block should do something if interacted while the second is if the item should do something with the block. Fabrics callback is just a prevent both if the callback is cancelled while forge allows you to both or either one of them.

in this case since its more along the line of block should do things i would for fabric just check if #allowUseBlocks is true. and on forge side just check #getUseBlock() != Result.DENY(you dont need to technically check #getUseItem then)

i do

if (!flags.allowUseBlocks() && !flags.allowUseItems())
                return InteractionResult.FAIL;

since the callback should fail if both are disallowed and otherwise i handle it via mixin to cancel one of them if needed. i just added this bit since while its technically not needed it might save performance or something since it then stops other callbacks from running

Crystal-Spider commented 1 year ago

So would I have to add Flan as a dependency to my build and call its API? To be honest I'm not really fond of this idea because I'd really like to keep this mod dependency-free. Isn't there anything else we could do?

Flemmli97 commented 1 year ago

ye. the only other way i see is for fabric to expand that callback or something

Crystal-Spider commented 1 year ago

I will probably add my own API then, so that it's possible for other mods (which means Flan too) to tweak some behaviors.

Crystal-Spider commented 1 year ago

Waiting for #18

Crystal-Spider commented 1 year ago

Version 6.0.0.0 is now out along with the newly added API.
In Forge you shouldn't need to do anything because I added those checks mentioned before.
In Fabric you can subscribe to the HARVEST_CHECK event and return false or true depending on your logic. I also suggest in your case to cancel the event so that other mods cannot change the return value.
Let me know if you face any issue!

Crystal-Spider commented 1 year ago

@Flemmli97 sorry for the ping, I will have soon to release a bugfix version, did you face any issue with using the new API? Did it work? Please let me know, thank you!

Flemmli97 commented 1 year ago

worked. fixed on my end