Fix handling of ItemStacks using getShareTag or Capabilites (#3171)

Temporary fix, ideally we would have a way to reference the original ItemStack so we don't need to send the full NBT data to the client.

src/main/java/appeng/container/AEBaseContainer.java

@@ -19,14 +19,11 @@
 package appeng.container;
 
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 
 import net.minecraft.entity.player.EntityPlayer;
@@ -37,8 +34,6 @@ import net.minecraft.inventory.IContainerListener;
 import net.minecraft.inventory.IInventory;
 import net.minecraft.inventory.Slot;
 import net.minecraft.item.ItemStack;
-import net.minecraft.nbt.CompressedStreamTools;
-import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.tileentity.TileEntity;
 import net.minecraftforge.items.IItemHandler;
 import net.minecraftforge.items.wrapper.PlayerInvWrapper;
@@ -73,7 +68,7 @@ import appeng.container.slot.SlotPlayerInv;
 import appeng.core.AELog;
 import appeng.core.sync.network.NetworkHandler;
 import appeng.core.sync.packets.PacketInventoryAction;
-import appeng.core.sync.packets.PacketPartialItem;
+import appeng.core.sync.packets.PacketTargetItemStack;
 import appeng.core.sync.packets.PacketValueConfig;
 import appeng.helpers.ICustomNameObject;
 import appeng.helpers.InventoryAction;
@@ -93,7 +88,6 @@ public abstract class AEBaseContainer extends Container
 	private final TileEntity tileEntity;
 	private final IPart part;
 	private final IGuiItemObject obj;
-	private final List<PacketPartialItem> dataChunks = new LinkedList<>();
 	private final HashMap<Integer, SyncData> syncData = new HashMap<>();
 	private boolean isContainerValid = true;
 	private String customName;
@@ -175,47 +169,6 @@ public abstract class AEBaseContainer extends Container
 		this.prepareSync();
 	}
 
-	public void postPartial( final PacketPartialItem packetPartialItem )
-	{
-		this.dataChunks.add( packetPartialItem );
-		if( packetPartialItem.getPageCount() == this.dataChunks.size() )
-		{
-			this.parsePartials();
-		}
-	}
-
-	private void parsePartials()
-	{
-		int total = 0;
-		for( final PacketPartialItem ppi : this.dataChunks )
-		{
-			total += ppi.getSize();
-		}
-
-		final byte[] buffer = new byte[total];
-		int cursor = 0;
-
-		for( final PacketPartialItem ppi : this.dataChunks )
-		{
-			cursor = ppi.write( buffer, cursor );
-		}
-
-		try
-		{
-			final NBTTagCompound data = CompressedStreamTools.readCompressed( new ByteArrayInputStream( buffer ) );
-			if( data != null )
-			{
-				this.setTargetStack( AEItemStack.fromNBT( data ) );
-			}
-		}
-		catch( final IOException e )
-		{
-			AELog.debug( e );
-		}
-
-		this.dataChunks.clear();
-	}
-
 	public IAEItemStack getTargetStack()
 	{
 		return this.clientRequestedTargetItem;
@@ -235,47 +188,7 @@ public abstract class AEBaseContainer extends Container
 				return;
 			}
 
-			final ByteArrayOutputStream stream = new ByteArrayOutputStream();
+			NetworkHandler.instance().sendToServer( new PacketTargetItemStack( (AEItemStack) stack ) );
-			final NBTTagCompound item = new NBTTagCompound();
-
-			if( stack != null )
-			{
-				stack.writeToNBT( item );
-			}
-
-			try
-			{
-				CompressedStreamTools.writeCompressed( item, stream );
-
-				final int maxChunkSize = 30000;
-				final List<byte[]> miniPackets = new LinkedList<>();
-
-				final byte[] data = stream.toByteArray();
-
-				final ByteArrayInputStream bis = new ByteArrayInputStream( data, 0, stream.size() );
-				while( bis.available() > 0 )
-				{
-					final int nextBLock = bis.available() > maxChunkSize ? maxChunkSize : bis.available();
-					final byte[] nextSegment = new byte[nextBLock];
-					bis.read( nextSegment );
-					miniPackets.add( nextSegment );
-				}
-				bis.close();
-				stream.close();
-
-				int page = 0;
-				for( final byte[] packet : miniPackets )
-				{
-					final PacketPartialItem ppi = new PacketPartialItem( page, miniPackets.size(), packet );
-					page++;
-					NetworkHandler.instance().sendToServer( ppi );
-				}
-			}
-			catch( final IOException e )
-			{
-				AELog.debug( e );
-				return;
-			}
 		}
 
 		this.clientRequestedTargetItem = stack == null ? null : stack.copy();

src/main/java/appeng/core/sync/AppEngPacketHandlerBase.java

@@ -41,11 +41,11 @@ import appeng.core.sync.packets.PacketMatterCannon;
 import appeng.core.sync.packets.PacketMockExplosion;
 import appeng.core.sync.packets.PacketPaintedEntity;
 import appeng.core.sync.packets.PacketPartPlacement;
-import appeng.core.sync.packets.PacketPartialItem;
 import appeng.core.sync.packets.PacketPatternSlot;
 import appeng.core.sync.packets.PacketProgressBar;
 import appeng.core.sync.packets.PacketSwapSlots;
 import appeng.core.sync.packets.PacketSwitchGuis;
+import appeng.core.sync.packets.PacketTargetItemStack;
 import appeng.core.sync.packets.PacketTransitionEffect;
 import appeng.core.sync.packets.PacketValueConfig;
 
@@ -90,7 +90,7 @@ public class AppEngPacketHandlerBase
 
 		PACKET_RECIPE_JEI( PacketJEIRecipe.class ),
 
-		PACKET_PARTIAL_ITEM( PacketPartialItem.class ),
+		PACKET_TARGET_ITEM( PacketTargetItemStack.class ),
 
 		PACKET_CRAFTING_REQUEST( PacketCraftRequest.class ),
 

src/main/java/appeng/core/sync/packets/PacketTargetItemStack.java

@@ -25,35 +25,56 @@ import io.netty.buffer.Unpooled;
 import net.minecraft.entity.player.EntityPlayer;
 
 import appeng.container.AEBaseContainer;
+import appeng.core.AELog;
 import appeng.core.sync.AppEngPacket;
 import appeng.core.sync.network.INetworkInfo;
+import appeng.util.item.AEItemStack;
 
 
-public class PacketPartialItem extends AppEngPacket
+public class PacketTargetItemStack extends AppEngPacket
 {
-
+	private AEItemStack stack;
-	private final short pageNum;
-	private final byte[] data;
 
 	// automatic.
-	public PacketPartialItem( final ByteBuf stream )
+	public PacketTargetItemStack( final ByteBuf stream )
 	{
-		this.pageNum = stream.readShort();
+		try
-		stream.readBytes( this.data = new byte[stream.readableBytes()] );
+		{
+			if( stream.readableBytes() > 0 )
+			{
+				this.stack = AEItemStack.fromPacket( stream );
+			}
+			else
+			{
+				this.stack = null;
+			}
+		}
+		catch( Exception ex )
+		{
+			AELog.debug( ex );
+			this.stack = null;
+		}
 	}
 
 	// api
-	public PacketPartialItem( final int page, final int maxPages, final byte[] buf )
+	public PacketTargetItemStack( AEItemStack stack )
 	{
 
+		this.stack = stack;
+
 		final ByteBuf data = Unpooled.buffer();
-
-		this.pageNum = (short) ( page | ( maxPages << 8 ) );
-		this.data = buf;
 		data.writeInt( this.getPacketID() );
-		data.writeShort( this.pageNum );
+		if( stack != null )
-		data.writeBytes( buf );
+		{
-
+			try
+			{
+				stack.writeToPacket( data );
+			}
+			catch( Exception ex )
+			{
+				AELog.debug( ex );
+			}
+		}
 		this.configureWrite( data );
 	}
 
@@ -62,23 +83,8 @@ public class PacketPartialItem extends AppEngPacket
 	{
 		if( player.openContainer instanceof AEBaseContainer )
 		{
-			( (AEBaseContainer) player.openContainer ).postPartial( this );
+			( (AEBaseContainer) player.openContainer ).setTargetStack( this.stack );
 		}
 	}
 
-	public int getPageCount()
-	{
-		return this.pageNum >> 8;
-	}
-
-	public int getSize()
-	{
-		return this.data.length;
-	}
-
-	public int write( final byte[] buffer, final int cursor )
-	{
-		System.arraycopy( this.data, 0, buffer, cursor, this.data.length );
-		return cursor + this.data.length;
-	}
 }

src/main/java/appeng/util/item/AEFluidStack.java

@@ -101,7 +101,6 @@ public final class AEFluidStack extends AEStack<IAEFluidStack> implements IAEFlu
 		}
 
 		final AEFluidStack fluid = AEFluidStack.fromFluidStack( fluidStack );
-		// fluid.priority = i.getInteger( "Priority" );
 		fluid.setStackSize( i.getLong( "Cnt" ) );
 		fluid.setCountRequestable( i.getLong( "Req" ) );
 		fluid.setCraftable( i.getBoolean( "Craft" ) );
@@ -111,12 +110,10 @@ public final class AEFluidStack extends AEStack<IAEFluidStack> implements IAEFlu
 	public static IAEFluidStack fromPacket( final ByteBuf data ) throws IOException
 	{
 		final byte mask = data.readByte();
-		// byte PriorityType = (byte) (mask & 0x03);
 		final byte stackType = (byte) ( ( mask & 0x0C ) >> 2 );
 		final byte countReqType = (byte) ( ( mask & 0x30 ) >> 4 );
 		final boolean isCraftable = ( mask & 0x40 ) > 0;
 		final boolean hasTagCompound = ( mask & 0x80 ) > 0;
-		boolean showCraftingLabel = data.readBoolean();
 
 		// don't send this...
 		final NBTTagCompound d = new NBTTagCompound();
@@ -139,17 +136,11 @@ public final class AEFluidStack extends AEStack<IAEFluidStack> implements IAEFlu
 			d.setTag( "tag", CompressedStreamTools.read( di ) );
 		}
 
-		// long priority = getPacketValue( PriorityType, data );
 		final long stackSize = getPacketValue( stackType, data );
 		final long countRequestable = getPacketValue( countReqType, data );
 
 		final FluidStack fluidStack = FluidStack.loadFluidStackFromNBT( d );
 
-		if( !showCraftingLabel )
-		{
-			showCraftingLabel = stackSize == 0;
-		}
-
 		if( fluidStack == null )
 		{
 			return null;
@@ -170,10 +161,6 @@ public final class AEFluidStack extends AEStack<IAEFluidStack> implements IAEFlu
 		{
 			return;
 		}
-
-		// if ( priority < ((AEFluidStack) option).priority )
-		// priority = ((AEFluidStack) option).priority;
-
 		this.incStackSize( option.getStackSize() );
 		this.setCountRequestable( this.getCountRequestable() + option.getCountRequestable() );
 		this.setCraftable( this.isCraftable() || option.isCraftable() );
@@ -364,7 +351,20 @@ public final class AEFluidStack extends AEStack<IAEFluidStack> implements IAEFlu
 	}
 
 	@Override
-	protected void writeToStream( final ByteBuf i ) throws IOException
+	public void writeToPacket( final ByteBuf i ) throws IOException
+	{
+		final byte mask = (byte) ( ( this.getType( this.getStackSize() ) << 2 ) | ( this
+				.getType( this.getCountRequestable() ) << 4 ) | ( (byte) ( this.isCraftable() ? 1 : 0 ) << 6 ) | ( this.hasTagCompound() ? 1 : 0 ) << 7 );
+
+		i.writeByte( mask );
+
+		writeToStream( i );
+
+		this.putPacketValue( i, this.getStackSize() );
+		this.putPacketValue( i, this.getCountRequestable() );
+	}
+
+	private void writeToStream( final ByteBuf i ) throws IOException
 	{
 		final byte[] name = this.fluid.getName().getBytes( "UTF-8" );
 		i.writeByte( (byte) name.length );

src/main/java/appeng/util/item/AEItemStack.java

@@ -19,8 +19,8 @@
 package appeng.util.item;
 
 
-import java.io.IOException;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 
 import javax.annotation.Nonnull;
@@ -106,15 +106,23 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 		return item;
 	}
 
-	public static IAEItemStack fromPacket( final ByteBuf data ) throws IOException
+	@Override
+	public void writeToNBT( final NBTTagCompound i )
+	{
+		this.getDefinition().writeToNBT( i );
+		i.setLong( "Cnt", this.getStackSize() );
+		i.setLong( "Req", this.getCountRequestable() );
+		i.setBoolean( "Craft", this.isCraftable() );
+	}
+
+	public static AEItemStack fromPacket( final ByteBuf data )
 	{
 		final byte mask = data.readByte();
-		// byte PriorityType = (byte) (mask & 0x03);
 		final byte stackType = (byte) ( ( mask & 0x0C ) >> 2 );
 		final byte countReqType = (byte) ( ( mask & 0x30 ) >> 4 );
 		final boolean isCraftable = ( mask & 0x40 ) > 0;
 
-		final ItemStack itemstack = ByteBufUtils.readItemStack( data );
+		final ItemStack itemstack = new ItemStack( ByteBufUtils.readTag( data ) );
 		final long stackSize = getPacketValue( stackType, data );
 		final long countRequestable = getPacketValue( countReqType, data );
 
@@ -123,13 +131,24 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 			return null;
 		}
 
-		final AEItemStack item = AEItemStack.fromItemStack( itemstack );
+		final AEItemStack item = new AEItemStack( AEItemStackRegistry.getRegisteredStack( itemstack ), stackSize );
-		item.setStackSize( stackSize );
 		item.setCountRequestable( countRequestable );
 		item.setCraftable( isCraftable );
 		return item;
 	}
 
+	@Override
+	public void writeToPacket( final ByteBuf i )
+	{
+		final byte mask = (byte) ( ( this.getType( this.getStackSize() ) << 2 ) | ( this
+				.getType( this.getCountRequestable() ) << 4 ) | ( (byte) ( this.isCraftable() ? 1 : 0 ) << 6 ) | ( this.hasTagCompound() ? 1 : 0 ) << 7 );
+
+		i.writeByte( mask );
+		ByteBufUtils.writeTag( i, this.getDefinition().serializeNBT() );
+		this.putPacketValue( i, this.getStackSize() );
+		this.putPacketValue( i, this.getCountRequestable() );
+	}
+
 	@Override
 	public void add( final IAEItemStack option )
 	{
@@ -143,15 +162,6 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 		this.setCraftable( this.isCraftable() || option.isCraftable() );
 	}
 
-	@Override
-	public void writeToNBT( final NBTTagCompound i )
-	{
-		this.getDefinition().writeToNBT( i );
-		i.setLong( "Cnt", this.getStackSize() );
-		i.setLong( "Req", this.getCountRequestable() );
-		i.setBoolean( "Craft", this.isCraftable() );
-	}
-
 	@Override
 	public boolean fuzzyComparison( final Object st, final FuzzyMode mode )
 	{
@@ -338,7 +348,7 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 			return false;
 		}
 
-		return this.sharedStack == ( (AEItemStack) otherStack ).sharedStack;
+		return Objects.equals( this.sharedStack, ( (AEItemStack) otherStack ).sharedStack );
 	}
 
 	@Override
@@ -424,12 +434,6 @@ public final class AEItemStack extends AEStack<IAEItemStack> implements IAEItemS
 		return this.oreReference;
 	}
 
-	@Override
-	protected void writeToStream( final ByteBuf data ) throws IOException
-	{
-		ByteBufUtils.writeItemStack( data, this.getDefinition() );
-	}
-
 	@Override
 	public boolean hasTagCompound()
 	{

src/main/java/appeng/util/item/AEItemStackRegistry.java

@@ -31,15 +31,30 @@ import javax.annotation.Nonnull;
 
 import net.minecraft.item.ItemStack;
 
+import appeng.util.Platform;
+
 
 public final class AEItemStackRegistry
 {
-	private static final WeakHashMap<AESharedItemStack, WeakReference<AESharedItemStack>> REGISTRY = new WeakHashMap<>();
+	private static final WeakHashMap<AESharedItemStack, WeakReference<AESharedItemStack>> SERVER_REGISTRY = new WeakHashMap<>();
+	private static final WeakHashMap<AESharedItemStack, WeakReference<AESharedItemStack>> CLIENT_REGISTRY = new WeakHashMap<>();
 
 	private AEItemStackRegistry()
 	{
 	}
 
+	private static WeakHashMap<AESharedItemStack, WeakReference<AESharedItemStack>> registry()
+	{
+		if( Platform.isClient() )
+		{
+			return CLIENT_REGISTRY;
+		}
+		else
+		{
+			return SERVER_REGISTRY;
+		}
+	}
+
 	static synchronized AESharedItemStack getRegisteredStack( final @Nonnull ItemStack itemStack )
 	{
 		if( itemStack.isEmpty() )
@@ -51,7 +66,7 @@ public final class AEItemStackRegistry
 		itemStack.setCount( 1 );
 
 		AESharedItemStack search = new AESharedItemStack( itemStack );
-		WeakReference<AESharedItemStack> weak = REGISTRY.get( search );
+		WeakReference<AESharedItemStack> weak = registry().get( search );
 		AESharedItemStack ret = null;
 
 		if( weak != null )
@@ -62,7 +77,7 @@ public final class AEItemStackRegistry
 		if( ret == null )
 		{
 			ret = new AESharedItemStack( itemStack.copy() );
-			REGISTRY.put( ret, new WeakReference<>( ret ) );
+			registry().put( ret, new WeakReference<>( ret ) );
 		}
 		itemStack.setCount( oldStackSize );
 

src/main/java/appeng/util/item/AESharedItemStack.java

@@ -56,6 +56,11 @@ final class AESharedItemStack implements Comparable<AESharedItemStack>
 		return this.itemDamage;
 	}
 
+	int getItemID()
+	{
+		return this.itemId;
+	}
+
 	@Override
 	public int hashCode()
 	{
@@ -104,7 +109,17 @@ final class AESharedItemStack implements Comparable<AESharedItemStack>
 			return damageValue;
 		}
 
-		return this.compareNBT( b.getDefinition() );
+		final int nbt = this.compareNBT( b.getDefinition() );
+		if( nbt != 0 )
+		{
+			return nbt;
+		}
+
+		if( !this.itemStack.areCapsCompatible( b.getDefinition() ) )
+		{
+			return System.identityHashCode( this.itemStack ) - System.identityHashCode( b.getDefinition() );
+		}
+		return 0;
 	}
 
 	private int compareNBT( final ItemStack b )

src/main/java/appeng/util/item/AEStack.java

@@ -19,8 +19,6 @@
 package appeng.util.item;
 
 
-import java.io.IOException;
-
 import io.netty.buffer.ByteBuf;
 
 import appeng.api.storage.data.IAEStack;
@@ -144,23 +142,7 @@ public abstract class AEStack<StackType extends IAEStack<StackType>> implements
 		this.countRequestable -= i;
 	}
 
-	@Override
+	protected byte getType( final long num )
-	public void writeToPacket( final ByteBuf i ) throws IOException
-	{
-		final byte mask = (byte) ( this.getType( 0 ) | ( this.getType( this.stackSize ) << 2 ) | ( this
-				.getType( this.countRequestable ) << 4 ) | ( (byte) ( this.isCraftable ? 1 : 0 ) << 6 ) | ( this.hasTagCompound() ? 1 : 0 ) << 7 );
-
-		i.writeByte( mask );
-
-		this.writeToStream( i );
-
-		this.putPacketValue( i, this.stackSize );
-		this.putPacketValue( i, this.countRequestable );
-	}
-
-	protected abstract void writeToStream( final ByteBuf data ) throws IOException;
-
-	private byte getType( final long num )
 	{
 		if( num <= 255 )
 		{
@@ -182,7 +164,7 @@ public abstract class AEStack<StackType extends IAEStack<StackType>> implements
 
 	abstract boolean hasTagCompound();
 
-	private void putPacketValue( final ByteBuf tag, final long num )
+	protected void putPacketValue( final ByteBuf tag, final long num )
 	{
 		if( num <= 255 )
 		{