Shopkeepers version: `Shopkeepers-2.6.0
Spigot version: Paper 1.13.2 566

About much of the time when setting up a trade the item I place in the editing menu is removed from my inventory. Basically to set up to trade diamond blocks for a bow costs me my diamond block. These are player trade type shopkeepers.

Example: https://gyazo.com/34e3f5b506f778852a3ba2d093fa9196

I am not able to reproduce this. Are you able to reproduce this on a fresh server without other plugins?

What other plugins are you running? Are there any plugins among those that might interfer with packet sending or inventory interactions (eg. ViaVersion, etc.)?

Are those items actually lost, or only visually (i.e. if you reconnect, are the items still missing)?

Are you running with any client mods?

It is also odd that the item amount in those clicked slots ends up at 2. The intended behavior (and what I see when testing this) is that the item is copied in that slot with amount of 1. And the original item then remains on the cursor.

You called it right. It was interference from an inventory management plugin. Thank you for your time.

The conflict is with StackableItems. I opened a ticket there and the dev of that plugin asked that you check the EventPriority to be sure it's not set to highest.

Reference: haveric/StackableItems#135

I don't remember the details anymore, but it seems like these priorities were purposely set to highest at some point to prevent incompatibilities with other plugin(s): 5c31d00

I will have to check if this can be changed.

In the past there have been cases in which Shopkeepers makes assumption about the outcome of the inventory click event after having processed it (and not cancelled). Basically relying on minecraft to process the inventory click as normal. So if some other plugin (like StackableItems) cancels the event or modifies the involved items afterwards, these assumption might no longer hold leading to inconsistent results. I will have to check whether this is still the case (there have been some changes to inventory handling since then).

Also, I haven't checked yet how StackableItems works in details, but I am not sure yet if Shopkeepers can even deal correctly with custom item stack sizes. For example, Shopkeepers will make use of the max stack sizes defined by Bukkit's Material. If StackableItems doesn't somehow (this is probably tricky and dangerously.. if even possible at all) overwrite this, then Shopkeepers might produce incompatible results (items of oversized stacks getting lost, or items getting stacked higher than anticipated by StackableItems).

Just dropping in to note that I made a lot of assumptions in my response over on StackableItems. My main assumption is that Shopkeeper's handling of inventories should always take priority over/before StackableItems at least with respect to shop inventories. (Basically, StackableItems shouldn't stack shop items as then we'll both just be fighting over what is there in the slot)

"... I am not sure yet if Shopkeepers can even deal correctly with custom item stack sizes."
It wouldn't be difficult to tell players to be sure the item they use while editing is a single item and not stacked. This should prevent shopkeepers from having to deal with superstacked items.

My main assumption is that Shopkeeper's handling of inventories should always take priority over/before StackableItems at least with respect to shop inventories.

If this incompatibility is limited to Shopkeepers 'editor menu' (a regular inventory), then this might work. I will however have to fundamentally (sounds more drastically than it probably is) change how Shopkeepers handles inventory clicks, because currently it handles inventory clicks generically the same (single inventory listener at highest priority that forwards the click event to the currently open 'UI') for all types of custom inventories (editor (regular chest inventory), trading (villager merchant inventory), etc.) and for some of these (the trading inventory) the highest priority is required for the same reason as to why StackableItems requires it: Making sure that no trades are processed (=applying exchanged items to the shop's stock in an associated chest) if some plugin has cancelled the trading by cancelling the corresponding inventory click event.

I did some experimenting with changing the event priority for the editor inventory interaction events (4e04946) and I also reduced the event priority for handling the trading from HIGHEST to HIGH (c985a65).
A compiled build of this is available here for experimenting: https://nexus.lichtspiele.org/repository/snapshots/com/nisovin/shopkeepers/Shopkeepers/2.6.1-SI-compat-SNAPSHOT/Shopkeepers-2.6.1-SI-compat-20190626.215228-2.jar

This seems to work quite well already in my basic testing. Unfortunely, it seems like StackableItems is currently not ignoring cancelled InventoryDragEvents (https://github.com/haveric/StackableItems/blob/master/mods/bukkit/src/main/java/haveric/stackableItems/listeners/SIPlayerListener.java#L459), so this still doesn't work.

Edit: I only tested this very briefly, but regarding super-sized stacks:

• I think this might not be an issue for player shops, since you cannot insert a full-stack item into the editor directly. The trading player shop for instance copies the item from the cursor and reduces its size to 1 initially. The player then has to increase the stack size manually again by clicking the item in the editor. The max stack size used there will then be limited by the minecraft defaults.
• For admin shops, the super-sized stacks seems to work fine as well. In the editor the items are simply used as-is to create the trade (without any validation).
• Inside the trading interface I noticed no issue with the super-sized items either. The logging seems to properly work as well. Just to be sure, I slightly reduced the event priority anyways, so StackableItems should be able to ignore the event after Shopkeepers as processed and cancelled it. While at the same time other plugins are still able to cancel the trading if they are using normal or below priorities (any plugin which uses HIGH priority for some reason for this might break with this change though).
  The only 'issue' I noticed (isn't a real issue either) was that when shift clicking to trade multiple times, Shopkeepers will use the default max stack sizes to determine whether the player has enough storage space inside his inventory for the next trade. This will not take the extended stack sizes into account and will therefore block the trades in some circumstances even though the player theoretically has enough storage space available for these super-sized stacks.

What mc version is this build compatible with? I'm currently running 1.13.2.

Yes, I have been testing with 1.13.2 as well. StackableItems doesn't work on 1.14.x

@haveric
Seems to work fine in my very basic testing. Thanks!

Just released an update to StackableItems (v1.0.6) that now cancels events properly.