Rework ItemStack handling for 1.11

Note to developers: You should never supply a null ItemStack to any
method. ProtocolLib will never return a null ItemStack. Update your
plugins accordingly.

Author: dmulloy2

modules/API/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java

@@ -41,6 +41,7 @@
 import javax.annotation.Nonnull;
 
 import org.bukkit.Bukkit;
+import org.bukkit.Material;
 import org.bukkit.Server;
 import org.bukkit.inventory.ItemStack;
 
@@ -128,13 +129,13 @@ public class MinecraftReflection {
 	private static String packageVersion;
 
 	// Item stacks
-	private static Method craftNMSMethod;
+	/* private static Method craftNMSMethod;
 	private static Method craftBukkitNMS;
 	private static Method craftBukkitOBC;
 	private static boolean craftItemStackFailed;
 
 	private static Constructor<?> craftNMSConstructor;
-	private static Constructor<?> craftBukkitConstructor;
+	private static Constructor<?> craftBukkitConstructor; */
 
 	// net.minecraft.server
 	private static Class<?> itemStackArrayClass;
@@ -1840,7 +1841,7 @@ public static boolean signUpdateExists() {
 	 * @param bukkitItemStack - the Bukkit ItemStack to convert.
 	 * @return A CraftItemStack as an ItemStack.
 	 */
-	public static ItemStack getBukkitItemStack(ItemStack bukkitItemStack) {
+	/* public static ItemStack getBukkitItemStack(ItemStack bukkitItemStack) {
 		// Delegate this task to the method that can execute it
 		if (craftBukkitNMS != null)
 			return getBukkitItemByMethod(bukkitItemStack);
@@ -1890,7 +1891,7 @@ private static ItemStack getBukkitItemByMethod(ItemStack bukkitItemStack) {
 	 * @param minecraftItemStack - the NMS ItemStack to wrap.
 	 * @return The wrapped ItemStack.
 	 */
-	public static ItemStack getBukkitItemStack(Object minecraftItemStack) {
+	/* public static ItemStack getBukkitItemStack(Object minecraftItemStack) {
 		// Delegate this task to the method that can execute it
 		if (craftNMSMethod != null)
 			return getBukkitItemByMethod(minecraftItemStack);
@@ -1941,13 +1942,104 @@ private static ItemStack getBukkitItemByMethod(Object minecraftItemStack) {
 	 * @param stack - the Bukkit ItemStack to convert.
 	 * @return The NMS ItemStack, or NULL if the stack represents air.
 	 */
-	public static Object getMinecraftItemStack(ItemStack stack) {
+	/* public static Object getMinecraftItemStack(ItemStack stack) {
 		// Make sure this is a CraftItemStack
 		if (!isCraftItemStack(stack))
 			stack = getBukkitItemStack(stack);
 
 		BukkitUnwrapper unwrapper = new BukkitUnwrapper();
 		return unwrapper.unwrapItem(stack);
+	} */
+
+	// ---- ItemStack conversions
+
+	private static Method asNMSCopy = null;
+	private static Method asCraftMirror = null;
+
+	private static Boolean nullEnforced = null;
+	private static Method isEmpty = null;
+
+	/**
+	 * Retrieves the Bukkit equivalent of a NMS ItemStack. This method should
+	 * preserve NBT data and will never return null when supplied with a valid
+	 * ItemStack. Empty ItemStacks are returned as AIR.
+	 * 
+	 * @param generic NMS ItemStack
+	 * @return The Bukkit equivalent
+	 */
+	public static ItemStack getBukkitItemStack(Object generic) {
+		// Make sure it actually is an ItemStack
+		if (!is(getItemStackClass(), generic)) {
+			return null;
+		}
+
+		// Convert null to AIR
+		if (generic == null) {
+			return new ItemStack(Material.AIR);
+		}
+
+		// Check null enforcement
+		try {
+			if (nullEnforced == null) {
+				isEmpty = getItemStackClass().getMethod("isEmpty");
+				nullEnforced = true;
+			}
+
+			if (nullEnforced) {
+				if ((boolean) isEmpty.invoke(generic)) {
+					return new ItemStack(Material.AIR);
+				}
+			}
+		} catch (ReflectiveOperationException ex) {
+			nullEnforced = false;
+		}
+
+		// Find asCraftMirror
+		if (asCraftMirror == null) {
+			try {
+				asCraftMirror = getCraftItemStackClass().getMethod("asCraftMirror", getItemStackClass());
+			} catch (ReflectiveOperationException ex) {
+				throw new RuntimeException("Failed to obtain CraftItemStack.asCraftMirror", ex);
+			}
+		}
+
+		// Convert to a craft mirror to preserve NBT data
+		try {
+			return (ItemStack) asCraftMirror.invoke(nullEnforced, generic);
+		} catch (ReflectiveOperationException ex) {
+			throw new RuntimeException("Failed to obtain craft mirror of " + generic, ex);
+		}
+	}
+
+	/**
+	 * Retrieves the NMS equivalent of a Bukkit ItemStack. This method will
+	 * never return null and should preserve NBT data. Null inputs are treated
+	 * as empty (AIR) ItemStacks.
+	 * 
+	 * @param specific Bukkit ItemStack
+	 * @return The NMS equivalent
+	 */
+	public static Object getMinecraftItemStack(ItemStack specific) {
+		// Grab asNMSCopy first
+		if (asNMSCopy == null) {
+			try {
+				asNMSCopy = getCraftItemStackClass().getMethod("asNMSCopy", ItemStack.class);
+			} catch (ReflectiveOperationException ex) {
+				throw new RuntimeException("Failed to obtain CraftItemStack.asNMSCopy", ex);
+			}
+		}
+
+		// If it's already a CraftItemStack, use its handle
+		if (is(getCraftItemStackClass(), specific)) {
+			return new BukkitUnwrapper().unwrapItem(specific);
+		}
+
+		// If it's not, grab a NMS copy
+		try {
+			return asNMSCopy.invoke(null, specific);
+		} catch (ReflectiveOperationException ex) {
+			throw new RuntimeException("Failed to make NMS copy of " + specific, ex);
+		}
 	}
 
 	/**

modules/API/src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java

@@ -713,24 +713,24 @@ public Class<Entity> getSpecificType() {
 	 * @return Item stack converter.
 	 */
 	public static EquivalentConverter<ItemStack> getItemStackConverter() {
-		return new IgnoreNullConverter<ItemStack>() {
+		return new EquivalentConverter<ItemStack>() {
 			@Override
-			protected Object getGenericValue(Class<?> genericType, ItemStack specific) {
-				return MinecraftReflection.getMinecraftItemStack(specific);
+			public ItemStack getSpecific(Object generic) {
+				return MinecraftReflection.getBukkitItemStack(generic);
 			}
-			
+
 			@Override
-			protected ItemStack getSpecificValue(Object generic) {
-				return MinecraftReflection.getBukkitItemStack(generic);
+			public Object getGeneric(Class<?> genericType, ItemStack specific) {
+				return MinecraftReflection.getMinecraftItemStack(specific);
 			}
-			
+
 			@Override
 			public Class<ItemStack> getSpecificType() {
 				return ItemStack.class;
 			}
 		};
 	}
-	
+
 	/**
 	 * Retrieve the converter for the ServerPing packet in {@link PacketType.Status.Server#OUT_SERVER_INFO}.
 	 * @return Server ping converter.