From 50c2931484ee07b566eab5abbc167ce4bbccfbce Mon Sep 17 00:00:00 2001
From: "Kristian S. Stangeland" <kr_stang@hotmail.com>
Date: Tue, 6 Aug 2013 19:27:58 +0200
Subject: [PATCH] ItemStacks that represents AIR cannot store NMS tags. Use
 exceptions.

---
 .../protocol/utility/MinecraftReflection.java |  4 +++-
 .../protocol/wrappers/nbt/NbtFactory.java     | 24 ++++++++++++++-----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java
index cdff70f3..f0a1eae5 100644
--- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java
+++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java
@@ -1175,8 +1175,10 @@ public class MinecraftReflection {
 	
 	/**
 	 * Retrieve the net.minecraft.server ItemStack from a Bukkit ItemStack.
+	 * <p>
+	 * By convention, item stacks that contain air are usually represented as NULL.
 	 * @param stack - the Bukkit ItemStack to convert.
-	 * @return The NMS ItemStack.
+	 * @return The NMS ItemStack, or NULL if the stack represents air.
 	 */
 	public static Object getMinecraftItemStack(ItemStack stack) {
 		// Make sure this is a CraftItemStack
diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/nbt/NbtFactory.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/nbt/NbtFactory.java
index 97339f97..d2810b55 100644
--- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/nbt/NbtFactory.java
+++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/nbt/NbtFactory.java
@@ -118,12 +118,12 @@ public class NbtFactory {
 	 * <p>
 	 * The item stack must be a wrapper for a CraftItemStack. Use 
 	 * {@link MinecraftReflection#getBukkitItemStack(ItemStack)} if not.
-	 * @param stack - the item stack.
-	 * @param compound - the new NBT compound.
+	 * @param stack - the item stack, cannot be air.
+	 * @param compound - the new NBT compound, or NULL to remove it.
+	 * @throws IllegalArgumentException If the stack is not a CraftItemStack, or it represents air.
 	 */
 	public static void setItemTag(ItemStack stack, NbtCompound compound) {
-		if (!MinecraftReflection.isCraftItemStack(stack))
-			throw new IllegalArgumentException("Stack must be a CraftItemStack.");
+		checkItemStack(stack);
 		
 		StructureModifier<NbtBase<?>> modifier = getStackModifier(stack);
 		modifier.write(0, compound);
@@ -140,8 +140,7 @@ public class NbtFactory {
 	 * @return A wrapper for its NBT tag.
 	 */
 	public static NbtWrapper<?> fromItemTag(ItemStack stack) {
-		if (!MinecraftReflection.isCraftItemStack(stack))
-			throw new IllegalArgumentException("Stack must be a CraftItemStack.");
+		checkItemStack(stack);
 		
 		StructureModifier<NbtBase<?>> modifier = getStackModifier(stack);
 		NbtBase<?> result = modifier.read(0);
@@ -154,6 +153,19 @@ public class NbtFactory {
 		return fromBase(result);
 	}
 	
+	/**
+	 * Ensure that the given stack can store arbitrary NBT information.
+	 * @param stack - the stack to check.
+	 */
+	private static void checkItemStack(ItemStack stack) {
+		if (stack == null)
+			throw new IllegalArgumentException("Stack cannot be NULL.");
+		if (!MinecraftReflection.isCraftItemStack(stack))
+			throw new IllegalArgumentException("Stack must be a CraftItemStack.");
+		if (stack.getTypeId() == 0)
+			throw new IllegalArgumentException("ItemStacks representing air cannot store NMS information.");
+	}
+	
 	/**
 	 * Retrieve a structure modifier that automatically marshalls between NBT wrappers and their NMS counterpart.
 	 * @param stack - the stack that will store the NBT compound.