PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.99k stars 2.32k forks source link

Armor Stand items lose NBT after opening inventory in gm1 #559

Closed jayhopkins219 closed 6 years ago

jayhopkins219 commented 7 years ago

Description of issue:

On a fresh, clean install of Paper (no plugins), Armor Stand items lose their NBT data after opening your own inventory while in creative mode. This issue is not present in vanilla minecraft.

Paper build number:

1002

Steps/models to reproduce:

/gamemode 1 /give @p minecraft:armor_stand 1 0 {EntityTag:{ShowArms:1,NoBasePlate:1}} Place armor stand, observe it has arms and no base plate as expected Open & close inventory Place armor stand, nbt data has been lost

Plugin list:

None

zachbr commented 7 years ago

Confirmed with latest Paper (b1002) and latest CB (git-Bukkit-70bc70b)

Black-Hole commented 7 years ago

Caused by this patch: https://github.com/PaperMC/Paper/blob/master/Spigot-Server-Patches/0172-Filter-bad-data-from-ArmorStand-and-SpawnEgg-items.patch

And that patch is configurable. So you could disable filtering by setting "filter-nbt-data-from-spawn-eggs-and-related" to false in paper.yml

aikar commented 7 years ago

Seems that the data isnt getting sent to the client it seems. Client doesnt show extra NBT Tags, which creative has full control over NBT so its getting actually erased.

zachbr commented 7 years ago

@Black-Hole no it isn't.

That is called when the armor stand item is used, so if it was responsible you would never be able to place them at all. You can place them. The issue appears to be that we're losing data somewhere when CB makes wrappers because of inventory handling. I tested this yesterday and confirmed the same issue occurs all the way up to CraftBukkit itself.

Edit: It looks like something is stripping the entire EntityTag off of it.

jayhopkins219 commented 7 years ago

Don't know if it helps, but it just seems to be the creative player inventory. Other inventories (echest, chests, hoppers, furnaces, minecart variants, shulker boxes, etc) do not wipe the EntityTag.

Weird stuff I found through testing:

*An armor stand w/ nbt can be temporarily stacked on top of an armor stand without nbt in a chest inventory. They either separate after a few ticks (if stacked by shift-clicking), or remain stacked until you close the inventory (if manually stacked). If you close the inventory while they are stacked, they are dropped onto the ground as separate item entities. I have a feeling this is because the client does not show that the armor stand has nbt (in vanilla it does, and the two are not stackable)

*If you give yourself an armor stand with nbt, then drop it on the ground and pick it up, the nbt data will stay after opening a creative inventory as long as it remains in your first slot (very left of hotbar) and is not moved. This does not work with other slots it seems EDIT: This seems to be a bit buggy, only working with certain sections of the gm1 inventory, and not always reproducible...

*If you give yourself an armor stand with nbt, then switch it to your offhand (default 'f' key), the nbt data is lost. I have a feeling this is a separate bug on its own

zachbr commented 6 years ago

Confirmed again; git-Paper-1317 & git-Bukkit-c765646

zachbr commented 6 years ago

As the process occurs:

PacketDataSerializer.k() ~L270:

// CraftBukkit start
if (itemstack.getTag() != null) {
    CraftItemStack.setItemMeta(itemstack, CraftItemStack.getItemMeta(itemstack));
}
// CraftBukkit end

Bukkit creates its item meta based on the NBT attached to the tag. It then sets the NBT associated with its item meta back onto the item.

ItemMeta.getItemMeta(nms.Item):

Bukkit gets the material API type of the item, then runs it through a switch statement checking for specific subclasses of CraftMetaItem to run the item through.

        switch (getType(item)) {
            case WRITTEN_BOOK:
                return new CraftMetaBookSigned(item.getTag());
            case BOOK_AND_QUILL:
                return new CraftMetaBook(item.getTag());
            case SKULL_ITEM:
                return new CraftMetaSkull(item.getTag());
            case LEATHER_HELMET:
            case LEATHER_CHESTPLATE:
            case LEATHER_LEGGINGS:
            case LEATHER_BOOTS:
                return new CraftMetaLeatherArmor(item.getTag());
            case POTION:
            case SPLASH_POTION:
            case LINGERING_POTION:
            case TIPPED_ARROW:
                return new CraftMetaPotion(item.getTag());
            case MAP:
                return new CraftMetaMap(item.getTag());
            case FIREWORK:
                return new CraftMetaFirework(item.getTag());
            case FIREWORK_CHARGE:
                return new CraftMetaCharge(item.getTag());
            case ENCHANTED_BOOK:
                return new CraftMetaEnchantedBook(item.getTag());
            case BANNER:
                return new CraftMetaBanner(item.getTag());
            case MONSTER_EGG:
                return new CraftMetaSpawnEgg(item.getTag());
            case KNOWLEDGE_BOOK:
                return new CraftMetaKnowledgeBook(item.getTag());
            case ARMOR_STAND:
                return new CraftMetaArmorStand(item.getTag());
            case FURNACE:
            case CHEST:
            case TRAPPED_CHEST:
            case JUKEBOX:
            case DISPENSER:
            case DROPPER:
            case SIGN:
            case MOB_SPAWNER:
            case NOTE_BLOCK:
            case BREWING_STAND_ITEM:
            case ENCHANTMENT_TABLE:
            case COMMAND:
            case COMMAND_REPEATING:
            case COMMAND_CHAIN:
            case BEACON:
            case DAYLIGHT_DETECTOR:
            case DAYLIGHT_DETECTOR_INVERTED:
            case HOPPER:
            case REDSTONE_COMPARATOR:
            case FLOWER_POT_ITEM:
            case SHIELD:
            case STRUCTURE_BLOCK:
            case WHITE_SHULKER_BOX:
            case ORANGE_SHULKER_BOX:
            case MAGENTA_SHULKER_BOX:
            case LIGHT_BLUE_SHULKER_BOX:
            case YELLOW_SHULKER_BOX:
            case LIME_SHULKER_BOX:
            case PINK_SHULKER_BOX:
            case GRAY_SHULKER_BOX:
            case SILVER_SHULKER_BOX:
            case CYAN_SHULKER_BOX:
            case PURPLE_SHULKER_BOX:
            case BLUE_SHULKER_BOX:
            case BROWN_SHULKER_BOX:
            case GREEN_SHULKER_BOX:
            case RED_SHULKER_BOX:
            case BLACK_SHULKER_BOX:
            case ENDER_CHEST:
                return new CraftMetaBlockState(item.getTag(), CraftMagicNumbers.getMaterial(item.getItem()));
            default:
                return new CraftMetaItem(item.getTag());
        }

There is no such case for ArmorStands, nor any subclass that would support them. It falls back to the generic case, without support for the entity NBT. ~L401

It returns that generic ItemMeta back up to PacketDataSerializer.k() where it is set as NBT, stripped of the original tags.

ProblemsSender commented 3 years ago

Can someone tell me the specific item that has to be related to this bug? filter-nbt-data-from-spawn-eggs-and-related related what? I really don't understand this. I want more information than just "spawn-eggs, falling-blocks, and other often abused items in creative mode."