WhipDevelopment / CrashClaim

An advanced claiming plugin for Minecraft
https://www.spigotmc.org/resources/crashclaim-claiming-plugin.94037/
GNU General Public License v3.0
22 stars 27 forks source link

Claims + ChestShop #10

Open Pandas0 opened 2 years ago

Pandas0 commented 2 years ago

On my server, I use the plugin ChestShop [https://www.spigotmc.org/resources/chestshop.51856/]. Since switching to CrashClaims recently, players cannot interact with ChestShop signs in claims unless "interaction" is enabled.

The issue with this is Interaction gives players access to more than just these ChestShop signs which is not ideal.

Chasewhip8 commented 2 years ago

Ye that is not the most ideal, not really an easy solution to this though I will look into it.

Pandas0 commented 2 years ago

Update: "Interaction" also allows users to take items from item frames within the claim, so people can steal the display item for these shops.

DaJokni commented 1 year ago

changing the playerinteractevent eventpriority from lowest to low seems to fix the issue with quickshop atleast, i dont know if this is a proper fix though

griefprevention listens to the event in lowest priority and it works just fine there, but could be that quickshop has some special support for griefprevention, dont know.

SenPr commented 1 year ago

So AFAIK, ignoreCancelled when set to true, means that if the event is cancelled, then the EventHandler will not get called. This is the case for both ChestShop and Quickshop with how they handle their PlayerInteractEvent.

Since CrashClaims have LOWEST priority, it runs before the Shop plugins, which means when cancelled the Shop plugins doesn't do anything. I'm not sure why GriefPrevention works just fine still though, haven't tested it yet. Still looking into it

SenPr commented 1 year ago

My theory from looking at GriefPrevention is that it actually doesn't block players from interacting with signs unless they are holding a material that directly interact with the sign.

So in this case, the event doesn't get cancelled and the shop plugins could still do its EventHandlers. How CrashClaims handle it is to catch everything into the event, including signs.

I think good fix for now is to add an exception? Maybe check if the player is interacting with a sign, with an empty hand and return rather than cancelling the event.

DaJokni commented 1 year ago

there might be some cases where someone wants to sell a certain item and hold it in their hands & get confused because it gets blocked

SenPr commented 1 year ago

Right, didn't think about that. If so it might be necessary to hook into ChestShop/QuickShop to check if a sign is a shop to not cancel the event.

SenPr commented 1 year ago

Just had a thought, the issue might also occur with GriefPrevention if you're trying to sell dyes, bone meals, ink sacs. I haven't tested yet though.

DaJokni commented 1 year ago

right clicking the sign with dyes etc doesnt work on griefprevention, but left clicks work just fine with those items

DaJokni commented 1 year ago

ive tested many different things now, and all of them have some problems.

currently, setting the priority to low has been the most successful, is there a reason for it to be in lowest?

SenPr commented 1 year ago

I hooked ChestShop into CrashClaim to check whether the block is a shop sign and it works fine currently, not sure if there could be any problem with it.

I'm not entirely sure about the priority thing, but imo a Claim plugin should have priority over others for the sake of protection. Not having it on LOWEST might cause conflicts with other plugins that may utilize PlayerInteractEvent since that would break the order of how things are ran.

DaJokni commented 1 year ago

right, still wondering how to do it on quickshop, but im pretty close to figuring it out.

SenPr commented 1 year ago

for QuickShop I think you can access ShopManager, then use one of the getShop methods. Though I'm not entirely sure how QuickShop works, it seems like you can have a shop without a sign, or buy through holograms which is another headache.

DaJokni commented 1 year ago

it would appear that there is already a PR open to add support for quickshop #24