haveric / StackableItems

Minecraft Bukkit plugin that allows you to change the default stack size of items.
22 stars 17 forks source link

Conflict with Shopkeepers trade editor menu #135

Closed FatherWh0 closed 5 years ago

FatherWh0 commented 5 years ago

An issue has arisen with 1.13 (shocker, I know ;P). While placing items in the Shopkeepers trade editor menu the items often disappear. I tried it with and without StackableItems and was about to determine it's a conflict between those two plugins.

To reproduce it create a trade type shopkeeper, place items in the chest, then open the editor menu and start placing items to set up trades. The items will be 'stolen' from your inventory.

I saw your post that you are just getting back into plugin developing. I really do appreciate your effort. This has been a really nice plugin, adding an nice feature to my server. It might seem like this issue is kind of specific but shopkeepers is a very popular plugin so I imagine others would benefit if the two were compatible again.

Thank you for your time.

haveric commented 5 years ago

I haven't looked into that plugin, but every other time this has come up for , it has to do with the shop plugin having too high of an EventPriority set (both plugins are likely set to highest) and StackableItems relies on other plugins running at a lower (earlier) priority and cancelling the event. I would ask those devs to dig into their priorities and consider lowering their priority (Ideally, it should be as low as possible to effectively set actions to be ignored by any plugins properly checking for cancellations).

If somehow the EventPriority is not the issue, I apologize in advance, but in case clarification is needed: If a plugin needs to always take control and prevent other plugins from taking actions. EventPriority should be set to LOWEST and the event should be cancelled so that it runs first and tells other plugins to not take action. With a shop plugin, this should be easy because you're only cancelling events that you know only happen for a certain inventory that only the shop plugin should interact with.

My plugin runs at EventPriority.HIGHEST because I don't want my actions to run if any plugin at a lower priority has decided the event needs to be cancelled. The naming of EventPriorities is unfortunately completely backwards as to how they are implied to work.

FatherWh0 commented 5 years ago

I've reopened the ticket with Shopkeepers asking the dev to check the eventpriority.

Reference: https://github.com/Shopkeepers/Shopkeepers/issues/554

blablubbabc commented 5 years ago

Seems like Stackable isn't ignoring cancelled drag events: https://github.com/Shopkeepers/Shopkeepers/issues/554#issuecomment-506049159

haveric commented 5 years ago

Just released v1.0.6 which is cancelling events properly.