From 286777b82428af64dcba3b8703fd80e30836628a Mon Sep 17 00:00:00 2001 From: mezz Date: Sun, 4 Jun 2017 18:38:16 -0700 Subject: [PATCH] Fix getShareTag replacing data on the server (#3776) Closes #3682 --- .../net/minecraft/item/Item.java.patch | 13 ++- .../net/minecraft/item/ItemStack.java.patch | 40 +++++++- .../network/NetHandlerPlayServer.java.patch | 9 ++ .../play/client/CPacketClickWindow.java.patch | 11 +++ .../CPacketCreativeInventoryAction.java.patch | 11 +++ .../common/util/PacketUtil.java | 58 +++++++++++ .../debug/NBTShareTagItemTest.java | 96 +++++++++++++++++++ .../models/item/nbt_share_tag_item.json | 6 ++ 8 files changed, 240 insertions(+), 4 deletions(-) create mode 100644 patches/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java.patch create mode 100644 patches/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java.patch create mode 100644 src/main/java/net/minecraftforge/common/util/PacketUtil.java create mode 100644 src/test/java/net/minecraftforge/debug/NBTShareTagItemTest.java create mode 100644 src/test/resources/assets/nbtsharetagitemtest/models/item/nbt_share_tag_item.json diff --git a/patches/minecraft/net/minecraft/item/Item.java.patch b/patches/minecraft/net/minecraft/item/Item.java.patch index b66413224..1d93e0b64 100644 --- a/patches/minecraft/net/minecraft/item/Item.java.patch +++ b/patches/minecraft/net/minecraft/item/Item.java.patch @@ -60,7 +60,7 @@ return p_77621_1_.func_147447_a(vec3d, vec3d1, p_77621_3_, !p_77621_3_, false); } -@@ -433,11 +440,625 @@ +@@ -433,11 +440,632 @@ return false; } @@ -145,6 +145,13 @@ + * Override this method to change the NBT data being sent to the client. + * You should ONLY override this when you have no other choice, as this might change behavior client side! + * ++ * Note that this will sometimes be applied multiple times, the following MUST be supported: ++ * Item item = stack.getItem(); ++ * NBTTagCompound nbtShare1 = item.getNBTShareTag(stack); ++ * stack.setTagCompound(nbtShare1); ++ * NBTTagCompound nbtShare2 = item.getNBTShareTag(stack); ++ * assert nbtShare1.equals(nbtShare2); ++ * + * @param stack The stack to send the NBT tag for + * @return The NBT tag + */ @@ -686,7 +693,7 @@ public static void func_150900_l() { func_179214_a(Blocks.field_150350_a, new ItemAir(Blocks.field_150350_a)); -@@ -972,6 +1593,8 @@ +@@ -972,6 +1600,8 @@ private final float field_78010_h; private final float field_78011_i; private final int field_78008_j; @@ -695,7 +702,7 @@ private ToolMaterial(int p_i1874_3_, int p_i1874_4_, float p_i1874_5_, float p_i1874_6_, int p_i1874_7_) { -@@ -1007,9 +1630,26 @@ +@@ -1007,9 +1637,26 @@ return this.field_78008_j; } diff --git a/patches/minecraft/net/minecraft/item/ItemStack.java.patch b/patches/minecraft/net/minecraft/item/ItemStack.java.patch index a85ee5346..02576c32a 100644 --- a/patches/minecraft/net/minecraft/item/ItemStack.java.patch +++ b/patches/minecraft/net/minecraft/item/ItemStack.java.patch @@ -283,7 +283,7 @@ @SideOnly(Side.CLIENT) public int func_190921_D() { -@@ -1013,4 +1085,28 @@ +@@ -1013,4 +1085,66 @@ { this.func_190917_f(-p_190918_1_); } @@ -310,5 +310,43 @@ + private Item getItemRaw() + { + return this.field_151002_e; ++ } ++ ++ /** ++ * Modeled after ItemStack.areItemStacksEqual ++ * Uses Item.getNBTShareTag for comparison instead of NBT and capabilities. ++ * Only used for comparing itemStacks that were transferred from server to client using Item.getNBTShareTag. ++ */ ++ public static boolean areItemStacksEqualUsingNBTShareTag(ItemStack stackA, ItemStack stackB) ++ { ++ if (stackA.func_190926_b()) ++ return stackB.func_190926_b(); ++ else ++ return !stackB.func_190926_b() && stackA.isItemStackEqualUsingNBTShareTag(stackB); ++ } ++ ++ /** ++ * Modeled after ItemStack.isItemStackEqual ++ * Uses Item.getNBTShareTag for comparison instead of NBT and capabilities. ++ * Only used for comparing itemStacks that were transferred from server to client using Item.getNBTShareTag. ++ */ ++ private boolean isItemStackEqualUsingNBTShareTag(ItemStack other) ++ { ++ return this.field_77994_a == other.field_77994_a && this.func_77973_b() == other.func_77973_b() && this.field_77991_e == other.field_77991_e && areItemStackShareTagsEqual(this, other); ++ } ++ ++ /** ++ * Modeled after ItemStack.areItemStackTagsEqual ++ * Uses Item.getNBTShareTag for comparison instead of NBT and capabilities. ++ * Only used for comparing itemStacks that were transferred from server to client using Item.getNBTShareTag. ++ */ ++ public static boolean areItemStackShareTagsEqual(ItemStack stackA, ItemStack stackB) ++ { ++ NBTTagCompound shareTagA = stackA.func_77973_b().getNBTShareTag(stackA); ++ NBTTagCompound shareTagB = stackB.func_77973_b().getNBTShareTag(stackB); ++ if (shareTagA == null) ++ return shareTagB == null; ++ else ++ return shareTagB != null && shareTagA.equals(shareTagB); + } } diff --git a/patches/minecraft/net/minecraft/network/NetHandlerPlayServer.java.patch b/patches/minecraft/net/minecraft/network/NetHandlerPlayServer.java.patch index 77d87e9b4..a26c099f0 100644 --- a/patches/minecraft/net/minecraft/network/NetHandlerPlayServer.java.patch +++ b/patches/minecraft/net/minecraft/network/NetHandlerPlayServer.java.patch @@ -51,3 +51,12 @@ if (this.field_147367_d.func_71199_h()) { +@@ -1097,7 +1105,7 @@ + { + ItemStack itemstack2 = this.field_147369_b.field_71070_bA.func_184996_a(p_147351_1_.func_149544_d(), p_147351_1_.func_149543_e(), p_147351_1_.func_186993_f(), this.field_147369_b); + +- if (ItemStack.func_77989_b(p_147351_1_.func_149546_g(), itemstack2)) ++ if (ItemStack.areItemStacksEqualUsingNBTShareTag(p_147351_1_.func_149546_g(), itemstack2)) + { + this.field_147369_b.field_71135_a.func_147359_a(new SPacketConfirmTransaction(p_147351_1_.func_149548_c(), p_147351_1_.func_149547_f(), true)); + this.field_147369_b.field_71137_h = true; diff --git a/patches/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java.patch b/patches/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java.patch new file mode 100644 index 000000000..b6843bb7f --- /dev/null +++ b/patches/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java.patch @@ -0,0 +1,11 @@ +--- ../src-base/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java ++++ ../src-work/minecraft/net/minecraft/network/play/client/CPacketClickWindow.java +@@ -55,7 +55,7 @@ + p_148840_1_.writeByte(this.field_149553_c); + p_148840_1_.writeShort(this.field_149550_d); + p_148840_1_.func_179249_a(this.field_149549_f); +- p_148840_1_.func_150788_a(this.field_149551_e); ++ net.minecraftforge.common.util.PacketUtil.writeItemStackFromClientToServer(p_148840_1_, this.field_149551_e); + } + + public int func_149548_c() diff --git a/patches/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java.patch b/patches/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java.patch new file mode 100644 index 000000000..390c68264 --- /dev/null +++ b/patches/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java.patch @@ -0,0 +1,11 @@ +--- ../src-base/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java ++++ ../src-work/minecraft/net/minecraft/network/play/client/CPacketCreativeInventoryAction.java +@@ -38,7 +38,7 @@ + public void func_148840_b(PacketBuffer p_148840_1_) throws IOException + { + p_148840_1_.writeShort(this.field_149629_a); +- p_148840_1_.func_150788_a(this.field_149628_b); ++ net.minecraftforge.common.util.PacketUtil.writeItemStackFromClientToServer(p_148840_1_, this.field_149628_b); + } + + public int func_149627_c() diff --git a/src/main/java/net/minecraftforge/common/util/PacketUtil.java b/src/main/java/net/minecraftforge/common/util/PacketUtil.java new file mode 100644 index 000000000..f40edd430 --- /dev/null +++ b/src/main/java/net/minecraftforge/common/util/PacketUtil.java @@ -0,0 +1,58 @@ +/* + * Minecraft Forge + * Copyright (c) 2016. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation version 2.1 + * of the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ +package net.minecraftforge.common.util; + +import net.minecraft.item.Item; +import net.minecraft.item.ItemStack; +import net.minecraft.nbt.NBTTagCompound; +import net.minecraft.network.PacketBuffer; + +public class PacketUtil +{ + private PacketUtil() {} + + /** + * Most ItemStack serialization is Server to Client, and must go through PacketBuffer.writeItemStack which uses Item.getNBTShareTag. + * One exception is items from the creative menu, which must be sent from Client to Server with their full NBT. + *
+ * This method matches PacketBuffer.writeItemStack but without the Item.getNBTShareTag patch. + * It is compatible with PacketBuffer.readItemStack. + */ + public static void writeItemStackFromClientToServer(PacketBuffer buffer, ItemStack stack) + { + if (stack.isEmpty()) + { + buffer.writeShort(-1); + } + else + { + buffer.writeShort(Item.getIdFromItem(stack.getItem())); + buffer.writeByte(stack.getCount()); + buffer.writeShort(stack.getMetadata()); + NBTTagCompound nbttagcompound = null; + + if (stack.getItem().isDamageable() || stack.getItem().getShareTag()) + { + nbttagcompound = stack.getTagCompound(); + } + + buffer.writeCompoundTag(nbttagcompound); + } + } +} diff --git a/src/test/java/net/minecraftforge/debug/NBTShareTagItemTest.java b/src/test/java/net/minecraftforge/debug/NBTShareTagItemTest.java new file mode 100644 index 000000000..2926f1b37 --- /dev/null +++ b/src/test/java/net/minecraftforge/debug/NBTShareTagItemTest.java @@ -0,0 +1,96 @@ +package net.minecraftforge.debug; + +import net.minecraft.client.renderer.block.model.ModelResourceLocation; +import net.minecraft.creativetab.CreativeTabs; +import net.minecraft.init.Items; +import net.minecraft.item.Item; +import net.minecraft.item.ItemStack; +import net.minecraft.item.crafting.CraftingManager; +import net.minecraft.nbt.NBTTagCompound; +import net.minecraft.util.NonNullList; +import net.minecraft.util.ResourceLocation; +import net.minecraftforge.client.model.ModelLoader; +import net.minecraftforge.fml.common.Mod; +import net.minecraftforge.fml.common.event.FMLPreInitializationEvent; +import net.minecraftforge.fml.common.registry.GameRegistry; +import net.minecraftforge.fml.relauncher.Side; + +import javax.annotation.Nullable; + +@Mod(modid = NBTShareTagItemTest.MOD_ID, name = "NBTShareTag Item Test", version = "1.0.0", acceptableRemoteVersions = "*") +public class NBTShareTagItemTest +{ + public static final String MOD_ID = "nbtsharetagitemtest"; + private static final ResourceLocation itemName = new ResourceLocation(MOD_ID, "nbt_share_tag_item"); + + @Mod.EventHandler + public void preInit(FMLPreInitializationEvent event) + { + ShareTagItem item = new ShareTagItem(); + item.setRegistryName(itemName); + GameRegistry.register(item); + + if (event.getSide() == Side.CLIENT) + { + ModelLoader.setCustomModelResourceLocation(item, 0, new ModelResourceLocation(itemName, "inventory")); + } + + ItemStack crafted = new ItemStack(item); + NBTTagCompound craftedNBT = new NBTTagCompound(); + craftedNBT.setBoolean("crafted", true); + crafted.setTagCompound(craftedNBT); + CraftingManager.getInstance().addShapelessRecipe(crafted, new ItemStack(Items.STICK)); + } + + public static class ShareTagItem extends Item + { + public ShareTagItem() + { + setCreativeTab(CreativeTabs.MISC); + } + + @Nullable + @Override + public NBTTagCompound getNBTShareTag(ItemStack stack) + { + NBTTagCompound networkNBT = stack.getTagCompound(); + if (networkNBT == null) + networkNBT = new NBTTagCompound(); + networkNBT.setBoolean("nbtShareTag", true); + return networkNBT; + } + + @Override + public void getSubItems(Item itemIn, CreativeTabs tab, NonNullList subItems) + { + ItemStack creativeMenuItem = new ItemStack(itemIn); + NBTTagCompound creativeMenuNBT = new NBTTagCompound(); + creativeMenuNBT.setBoolean("creativeMenu", true); + creativeMenuItem.setTagCompound(creativeMenuNBT); + subItems.add(creativeMenuItem); + } + + @Override + public String getItemStackDisplayName(ItemStack stack) + { + NBTTagCompound tagCompound = stack.getTagCompound(); + String name = "NBTShareTagItem: "; + if (tagCompound == null) + { + name += "From /give. "; + } + else + { + if (tagCompound.getBoolean("nbtShareTag")) + name += "Sent from server. "; + + if (tagCompound.getBoolean("creativeMenu")) + name += "From creative menu. "; + + if (tagCompound.getBoolean("crafted")) + name += "From crafting. "; + } + return name; + } + } +} diff --git a/src/test/resources/assets/nbtsharetagitemtest/models/item/nbt_share_tag_item.json b/src/test/resources/assets/nbtsharetagitemtest/models/item/nbt_share_tag_item.json new file mode 100644 index 000000000..927eae646 --- /dev/null +++ b/src/test/resources/assets/nbtsharetagitemtest/models/item/nbt_share_tag_item.json @@ -0,0 +1,6 @@ +{ + "parent": "item/generated", + "textures": { + "layer0": "items/stick" + } +} \ No newline at end of file