Plugin incompatibility: This plugin seems to modify ItemStacks of villager trades about eco HOT 8 CLOSED

Fixed in eco 5.3.0 + ecoenchants 7.2.8

from eco.

Fixed in latest version

from eco.

I assume this commit is supposed to fix it? bce1dd9

I am not sure how this is supposed to fix it for any items that actually have a max stack size of 1, such as books, tools, weapons, etc.

Edit: It may resolve the stacking issue, however, these items will still be modified and then be considered unequal in other situations. Be it other minecraft mechanics (eg. villager trades with more than one input item), or other plugins (such as for example shop plugins that compare items, or plugins that use Bukkit's API to setup crafting recipes that require the items to perfectly match, etc.).

Edit: I see you made another change to this (dbfef30). I haven't tested this yet, but this seems to be promising to solve this more generally. Thanks!

from eco.

A user of my plugin reported having this issue again. I suppose you reverted what I previously assumed might have fixed this in e9e3934

Is there any way you can avoid modifying the result item involved in villager trades? Maybe an option could be to not actually modify the item on the server, but only the copy that is being sent to clients.

from eco.

I only modify the object being sent to the client - however, I have to modify the NMS handle or the trade is cancelled altogether. It could be that the NMS handle is the server-side object? I could experiment with cancelling the OpenWindowMerchant packet and sending off a new one with modified recipes?

from eco.

It seems like the MerchantRecipes you get from ProtocolLib (

It could be that the NMS handle is the server-side object?

I think so, yes.

You then retrieve this underlying nms.MerchantRecipe via the handle and modify its result item ('sellingItem') (https://github.com/Auxilor/eco/blob/master/eco-core/core-nms/v1_16_R3/src/main/java/com/willfp/eco/proxy/v1_16_R3/VillagerTrade.java). This will modify the original nms.MerchantRecipes of the villager on the server.

(Offtopic: By the way, you also modify the result item of the bukkit MerchantRecipe portion of CraftMerchantRecipe ('result'). I don't think that this has any effect: That item is a copy, and changes to it are never applied to the underlying nms.MerchantRecipe or nms.ItemStack.)

Afterwards you then create a new MerchantRecipeList (

A first step might be to create copies of these MerchantRecipes, and then modify and send those. But this alone might indeed not resolve your trade cancelling issue.

however, I have to modify the NMS handle or the trade is cancelled altogether.

Trades are usually cancelled if there is some mismatch between items in the slots and of the merchant recipe. However, without having looked into this deeper, I don't know whether the cancelling of the trade in your case happens on the client or the server (the mismatch of these items could be checked for on either side).
Since the server updates the item of the merchant result slot when the player inserts items or selects a trading recipe in the list, you may also have to intercept that slot update packet, and copy and modify the sent item there.

The goal would be to not modify any recipes or items on the server, but only the copies that are transferred to the client. So on the client, the recipe result item and result slot item would match your modified version, and on the server they would still be the originals and match there as well.

I could experiment with cancelling the OpenWindowMerchant packet and sending off a new one with modified recipes

I don't think this will resolve your trade cancelling issue if this is caused by the server setting the result slot item to the non-modified server variant whenever something in the merchant inventory changes (input items, merchant recipe selection, etc.).

By the way, if you go this route of always only modifying the items sent to the client by copying them first, there should also be no need for your to keep track of this 'finalized' state.

from eco.

Thanks! I tested this briefly and was not able to reproduce the compatibility issue between Eco and Shopkeepers anymore. And the previously affected items also no longer needlessly gain the additional 'finalized' tag on the client side either.

from eco.

I see you partially reverted your changes again in d65c04a , modifying the original villager trades on the server (the handle) again.

May I ask what issue you ran into? If you ran into the "trade is cancelled altogether" issue you mentioned above: Your original change did not yet account for the slot items changing I mentioned above. So maybe try intercepting the relevant packet for that (https://wiki.vg/Protocol#Set_Slot) and modify the ItemStack being sent to clients to match the modified merchant recipe items.

Edit: And if the issue has been with copying the MerchantRecipe because the Bukkit MerchantRecipe is not able to represent all of the nms.MerchantRecipe data (eg. demand and special price): You can also create/copy a raw nms.MerchantRecipe (via the constructor, or by saving the data to NBT and loading it into a new nms.MerchantRecipe) and then create a CraftMerchantRecipe from that (via the 'asBukkit()' method).

from eco.