Paper: CraftBukkit bug: Villagers unable to sell books with NBT

[Cat7373]

https://hub.spigotmc.org/jira/browse/SPIGOT-515
https://hub.spigotmc.org/jira/browse/SPIGOT-686
https://hub.spigotmc.org/jira/browse/SPIGOT-818

Test command

/summon Villager ~ ~1 ~ {Offers:{Recipes:[{buy:{id:"minecraft:stone",Count:1b,Damage:0s},sell:{id:"minecraft:written_book",Count:1b,tag:{pages:["test_test"],author:"Test_Player",title:"Test_Book",resolved:1},Damage:0s}},{buy:{id:"minecraft:written_book",Count:1b,tag:{pages:["test_test"],author:"Test_Player",title:"Test_Book",resolved:1},Damage:0s},sell:{id:"minecraft:stone",Count:1b,Damage:0s}}]}}

• The first transaction completed successfully
• The second result of the transaction can not be removed

[Black-Hole]

That seems to be invalid JSON if pasted here: http://json.parser.online.fr/

[Cat7373]

Minecraft commands actually used is not the standard JSON
At least until 1.9
Also Paper and the above command successfully executed

[willies952002]

I can confirm that this is an issue. You are able to purchase the Written Book from the Villager but are unable to Sell the book back to them.

Edit: I just spawned another villager and it seems to work now. It might be an intermittent issue.

Edit: Confirmed as intermittent. I spawned in 10 more villagers and some accepted the book while others didn't.

Edit: It seems that once a villager acquires their profession, you can no longer trade them the book back.

[aikar]

While the other related bug appears fixed, this one is still valid as of today.

[BillyGalbreath]

I wanted to test this one, but I don't understand what the issue is. Haven't found an SSCCE that fully explains the issue vs what is expected.

[aikar]

after crafting the first and 2nd option, the 2nd one result slot is buggy. you will see a stone result, but you cant reliably pick it up.

[ISSchuster]

I can confirm the problem in the latest paper-build.

But the problem is NOT a Enchantment NBT Tag - the problem is the StoredEnchantment Tag...

Please test it with this command - I can reproduce it on a clean server with the latest paper version.

summon villager -1100 70 -150 {Silent:1b,PersistenceRequired:1b,NoAI:1b,Profession:1,CareerLevel:1,Offers:{Recipes:[{maxUses:2147483647,uses:0,buy:{id:"minecraft:enchanted_book",tag:{StoredEnchantments:[
{id:"minecraft:mending",lvl:1}

]},Count:1b},buyB:{id:"minecraft:bow",Count:1b},sell:{id:"minecraft:bow",tag:{Enchantments:[
{id:"minecraft:mending",lvl:1}

]},Count:1b}}]}}

[Combustible]

I suspect this bug is related to the explanation I posted in #420. Basically - server-side minecraft checks that the NBT data is exactly the same for villager trades. Client-side seems to check only that the NBT data has the same contents (order insensitive). Many operations cause the game to automatically reorder the NBT data - so when you buy a book (or other item), it may sometimes reorder the NBT data on that book so that it no longer matches the villager.

I believe a fix for this would involve changing the equality test on NBT objects to be order-insensitive.

[FendtXerion3800]

so i played around with a Villager and got the trades work all the time with enchantments. The order is not causing issue, but changing NBT tag values. For example if the id is just vanishing_curse, the game will change the id value most of the time to "minecraft:vanishing_curse" which will cause the server to think its a different item. Same applies to the lvl value. if you just put lvl:1 the game changes it to lvl:1s and its a different value again. However the order of enchantments matter to the game.

To see what to change in this scenario i used
you get a list with your inventory intems and all the matching NBT tags

/data get entity @p

when i set the values to id:vanishng_curse and lvl:1 i could sometimes trade those items back in like 1 out of 10. In the nbt tags i could see that these items still got my set values and the rest had the changed ones to lvl:1s and id:"minecraft:vanishing_curse"

the working Villager i was testing with:

summon villager ~ ~0.5 ~ 
    {Rotation: [270f,0f,0f],
    CustomName: "\"Mojo\"",
    NoAI: 1,
    Silent: 1,
    Invulnerable: 1,
    HandItems: [{id: emerald , Count:1}],
    VillagerData:
        {level: 40,
        profession: librarian,
        type: savanna},
    Offers: { Recipes: [ 
        {maxUses: 1000000000, rewardExp: false, buy: {id: paper, Count: 1}, sell: { id: milk_bucket, Count: 1, tag: {Enchantments:[{lvl:1s, id:"minecraft:vanishing_curse"}]} }},
        {maxUses: 1000000000, rewardExp: false, sell: {id: paper, Count: 1}, buy: { id: milk_bucket, Count: 1, tag: {Enchantments:[{id:"minecraft:vanishing_curse", lvl:1s}]} }}
        ]}
    }

[sandtechnology]

so i played around with a Villager and got the trades work all the time with enchantments. The order is not causing issue, but changing NBT tag values. For example if the id is just vanishing_curse, the game will change the id value most of the time to "minecraft:vanishing_curse" which will cause the server to think its a different item. Same applies to the lvl value. if you just put lvl:1 the game changes it to lvl:1s and its a different value again. However the order of enchantments matter to the game.

To see what to change in this scenario i used
you get a list with your inventory intems and all the matching NBT tags

/data get entity @p

when i set the values to id:vanishng_curse and lvl:1 i could sometimes trade those items back in like 1 out of 10. In the nbt tags i could see that these items still got my set values and the rest had the changed ones to lvl:1s and id:"minecraft:vanishing_curse"

the working Villager i was testing with:

summon villager ~ ~0.5 ~ 
  {Rotation: [270f,0f,0f],
  CustomName: "\"Mojo\"",
  NoAI: 1,
  Silent: 1,
  Invulnerable: 1,
  HandItems: [{id: emerald , Count:1}],
  VillagerData:
      {level: 40,
      profession: librarian,
      type: savanna},
  Offers: { Recipes: [ 
      {maxUses: 1000000000, rewardExp: false, buy: {id: paper, Count: 1}, sell: { id: milk_bucket, Count: 1, tag: {Enchantments:[{lvl:1s, id:"minecraft:vanishing_curse"}]} }},
      {maxUses: 1000000000, rewardExp: false, sell: {id: paper, Count: 1}, buy: { id: milk_bucket, Count: 1, tag: {Enchantments:[{id:"minecraft:vanishing_curse", lvl:1s}]} }}
      ]}
  }

Can confirm, after deep seaching and debuging, it look like CraftMetaBookSigned class affected tags by reapply it, so NBT tag type and amount changed, but it also change the text due to craftbukkit deserialize and serialize it by ChatSerializer, so it was hard to fix, unless just let it always is orginal one, or do the same thing on the villager mechant item.

[sandtechnology]

There are some screenshots to show the difference between the mechant item and the item got by trade on 1.8.8, hopefully it will get fixed soon (it was valid for 4 years):

[aikar]

Those issues aren't going to be resolved. Mojang likely didn't convert something in villagers trades and I'm surely not comfortable with trying to add DFU's for that (nor would it help already converted)

But notably, the pages change is just another unfortunate change of text encoding.

You will have to rebuild the villagers trades with 1.16 data.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Proximyst]

Stale who? Stale you! Zim-zalabim, unstale! ✨✨