Shopkeepers

Shopkeepers

2M Downloads

Text formatting issues

detoxv opened this issue · 13 comments

commented

The itallic issue has been fixed as of the latest paper spigot update however there is still an issue preventing trades, this is a problem as item based currency is proving impossible to use with traders with all the text issues having to be so specifically formatted and they change when in 1.15.2 this wasn't an issue and it could be obtained and traded via plugin reward, commands and loot tables / give commands alike. Here is my issue

This is what i put into the shopkeeper to trade for:
2

This is what the item is after restart:
1

As the shop keeper only trades exact copies of the item the fact in appearance it is still the same item the way its formatted internally is different and i was curious if this is something i should wait to "fix" if it even is a problem to be fixed as it is holding back updating my server or to try figure out another currency system of sorts because of this changing text internal formatting

commented

I consolidate all issues related to items changing their internal data format during de/serialization in this ticket #578

The problem you see here is that in MC 1.16 Spigot has changed how they convert between the saved text §eBag of Silver to Minecraft's text format. So items that worked fine in MC 1.15 are now broken with this change and won't match the items loaded from the shopkeeper data anymore.

Ideally Spigot should not change the items' internal data representation when plugins (such as Shopkeepers) save and load them to/from config files (i.e. Shopkeepers' save.yml file).
I am currently looking through Spigot whether I can come up with a solution for this which they would be willing to pull. But in the meantime your possibilities are to either use a plugin to create the item or stick to the format Spigot uses. However, since Spigot has changed this format in MC 1.16 any previous items your players might still have are broken now..

I have created a new snapshot of Shopkeepers v2.10.1 which, among other things, includes a command with which you can convert a held item to Spigot's format. You can find that snapshot here: https://nexus.lichtspiele.org/repository/snapshots/com/nisovin/shopkeepers/Shopkeepers/2.10.1-SNAPSHOT/Shopkeepers-2.10.1-20200702.232947-4.jar
The command is /shopkeeper convertItems [player] ['all'] (The argument 'all' lets you convert all items inside your inventory at once).

commented

I understand its a difficult thing to resolve, the trade setting would be a life saver right now. The stacking can be an issue but what if this happens on a chest open event? Is this too impactful server side to have it run that often to enable it so whenever someone opens a chest of a loot table or to store said item it converts to spigots internal data format so naturally most inventory / trading interfaces convert. Honestly just that update alone meaning trades work would allow me to update my server and keep loot tables as if they get converted to enable trading at least economy will work. Do you have an estimated time when this update / setting will release so i can test it out and get it all updated? Trying to get a perfectly matching give command is very hard as i am not sure how to format the extra tags as there is no examples out there to learn from i tried adding in extra and it ended up giving me a default item with no NBT.

I think it would be a very useful because at least then trades will work and currency can be traded even if it doesn't have perfect formatting as long as it gets converted and traded at some point to allow for trade would be perfect. Is there anywhere i can check often for this update as i imagine it wont be official release right away but instead a snapshot? I am just in a rush to get these trades working with my currency because in its current state due to spigots changes trading wouldnt happen no matter where the item originated from (give command, loot table) and considering the general playerbase doesn't understand NBT and internal data if it appears on the surface level to match there will be confusion and endless explaining to do as to why things refuse to trade and the trading item based currency aspect is a core to some servers. Thanks again, looking forward to that update!

commented

Trying to get a perfectly matching give command is very hard as i am not sure how to format the extra tags as there is no examples out there to learn from

The format should look exactly like in your screenshots above. Not sure why you ended up with an empty item. A tip to more easily edit and verify the command could be to write the command inside a regular text editor and only copy it into minecraft afterwards to see if it works.

what if this happens on a chest open event? [..] Is this too impactful server side to have it run that often [..]

This could be an option. However, there are so many ways in how players can end up with items (including other plugins adding items directly to the player's inventory). Items not stacking is not only a problem when players try to manually stack them, but also when something tries to add items into their inventory. I will always only be able to detect a small subset of those.
Edit: Actually, there is no event triggered for when a player opens his own inventory, only when the player opens an external inventory. So I think I will limit it for now to whenever a player opens some type of shopkeeper UI.

Also, like you noted, this conversion is likely not cheap. There is also no easy way to check if an item is affected by this conversion without actually running it. So I always need to run the conversion for all items, and repeat it whenever it gets triggered again by some event. So this really is not meant as a permanent solution but only to help migrating to this new Minecraft/Spigot update.

Ideally you should consider disabling this option again at some point once you believe that most of your players' items got converted by now. This also means that you should try to get your give command working with Spigot's new format, since otherwise you will always depend on this setting to be enabled for things to keep working.

Is there anywhere i can check often for this update as i imagine it wont be official release right away but instead a snapshot?

You can join the discord server (there is a link inside the wiki). I will likely upload snapshot releases there.

commented

Thank you i appreciate the information, is there any way you can help me achieve using spigots new format? I can't even get a simple /minecraft:give command to give me anything that works after a restart. I'm trying to give myself an item with name and lore and all these extra fields pop up after on the shopkeepers version of the item.

commented

With the above linked snapshot you can create the item like before, hold the item inside your hand, and then run /shopkeeper convertItems. This will convert it to Spigot's format. Alternatively, you can also put the item inside an admin shopkeeper, reload the plugin, and then extract the item from the shopkeeper again. This should produce the same result.

If you want to setup a give command to give you the item in Spigot's format right away, you could analyze the internal data of the just converted item and copy it from there into the give command. You can use Minecraft's /data get .. command (like you probably used for the above screenshots) to view the internal data of the item. I am not aware of any shortcut for copying the data into the command..

commented

Thank you for the response its a massive help and its appreciated, i have been using the snapshot to convert for testing but when it comes to generating currency from outside of that in the eyes of a player such as an essentials /kit to use a minecrafts give command for the currency to match the shopkeeper for say a daily piece of currency it will not work as it has extra fields that i am struggling to replicate manually in the give command. I have been using a colored firework star as the player heads are proving difficult but i still face the same formatting issues with the new item.

This is my current give command - /minecraft:give {player} minecraft:firework_star{display:{Name:"[{"text":"Ruby Ore","italic":false,"bold":false,"strikethrough":false,"underlined":false,"obfuscated":false,"color":"yellow"}]"},Explosion:{Type:0b,Colors:[I;11743532]},HideFlags:32} 1

Which produces
give

This is the shopkeepers data
generated

It is very close to being a match but it has the extra fields such as "extra" and i just cannot seem to get the formatting right in the give command to make an exact match, this is frustrating from the point of view of someone using item based economy. Do you think the use of minecrafts loot tables are now impossible to be compatible with shopkeepers? I had players find currency in the past and i cannot see this working again on 1.16. It is a shame there isn't a sensitivity option for shopkeepers i am aware this is impossible but if an item in appearance is identicle it would be nice for it to be tradable, the fact it has to be an exact match with formatting is making this update very challenging to know how to proceed to get everything set up like it once was. I know of some servers that are still using shopkeepers with item based currency, Minewind being one of them i have seen on 1.16.1 (credited with the use of this plugin) and i have no idea how they have managed to do this. They also use loot tables similarly like i did. Thanks again for your time and assistance

commented

It is very close to being a match but it has the extra fields such as "extra" and i just cannot seem to get the formatting right in the give command to make an exact match

Yeah, you have to exactly replicate that extra portion of the text format as well.

It is a shame there isn't a sensitivity option for shopkeepers i am aware this is impossible but if an item in appearance is identicle it would be nice for it to be tradable, the fact it has to be an exact match with formatting is making this update very challenging to know how to proceed to get everything set up like it once was.

Yeah, I agree it's annoying. However, it's actually Minecraft which requires the item data to perfectly match.. This is nothing I can change, because part of these item comparisons happen deep inside the Minecraft server's implementation of the trading interface, and partly on the client side as well. The minecraft client will actively clear the item from the result slot if the items used inside the trade don't match (so I can't simple replace this part with an own implementation; at least not without sending fake packets to the client.. Which opens the door for a whole lot of other problems and difficulties which I hesitate to go into..).

Even worse, this issue will come up again whenever Spigot changes something on how they convert legacy text to Minecraft's Json text.

I am currently adding a setting which allows you to automatically convert all items in players' inventories and shop chests to Spigot's internal data format whenever a player opens the trading or editor interface. This might resolve issues with trades not working, however, you might then run into issues with items not stacking if your players end up with a few items in Spigot's format and a few items in the format you originally created the items with.. So I am not sure yet how helpful this would actually be..
The proper solution would be for Spigot to persist the original item data during de/serialization.

commented

Okay, I added that setting and just tried it. It works, however, it is actually performing so badly (much worse than I expected) that I will probably not include it in any official update..
If you want I can give you the snapshot to test for yourself. But with a full inventory of items that need to be converted (or checked whether they need to be converted) this takes something around 10-33 ms on my testing server whenever someone interacts with a shopkeeper .. which is far too worse for me to be able to release this.

commented

I would like to try it out for sure but if it is very bad i'll refrain from using it, do you think you can assist me finding whats wrong with this command? I am trying like you said to analyse the data and make a give command (this would fix all my issues if i can get a consistent /give command to work with shopkeepers) however i am struggling to form a command that works. I figure you have far more experience with this kind of thing than i do and there is seemingly not many answers online

trader
/minecraft:give @p minecraft:firework_star{display:{Name:"{"extra":[{"bold":false,"italic":false,"underlined":false,"strikethrough":false,"obfuscated":false,"color":"yellow","text":"Ruby Ore"}],"text":""},Explosion:{Type:0b,Colors:[I;11743532]},HideFlags:32} 1

This above command is my attempt to recreate it however it just gives a firework star without the name NBT so it clearly is wrong. Once i have one working command i can use common sense to then re-create anything its just getting that initial working template which ive tried many times to do and it never works. Thank you for getting back to me!

commented

Don't have much time right now, but from a quick look this might be an issue with missing to escape inner quites, i.e. using \" instead of ". Or you can change the outer quotes to single quites ', like in the screenshots.

commented

Okay, I was able to drastically improve the performance of these item conversions by now. It's still not cheap but a lot better compared to before (around 3.5 ms in the absolute worst case during my testing; per player interacting with a shopkeeper).

Regarding your give command, this seems to work for me:
/minecraft:give @p minecraft:firework_star{display:{Name:'{"extra":[{"bold":false,"italic":false,"underlined":false,"strikethrough":false,"obfuscated":false,"color":"yellow","text":"Ruby Ore"}],"text":""}'},Explosion:{Type:0b,Colors:[I;11743532]},HideFlags:32} 1
I added the single quotes as mentioned, and one closing bracket } was missing after the text.

commented

Thank you very much for taking the time to look into that and send the version of shopkeepers, thankfully i managed to figure out the give command and even how to format the loot table so the are all persistently the same item from give and loot table and all working perfectly with shopkeepers, took a lot of trial and error at first. I'll check this verison out regardless, thanks a lot