Fix inconsistency getting an item from a player slot

Previously when getting the item in hand, it would return an empty item stack instead of null. This was inconsistent with all other slot values, resulting in unexpected behavior and core errors in some item meta functions. To fix this inconsistent function behavior, I decided to return empty values where that would have already been a handled output, avoiding breaking any scripts where possible. This also matches the previous behavior of the most common use cases.

Author: PseudoKnight

src/main/java/com/laytonsmith/abstraction/bukkit/entities/BukkitMCPlayer.java

@@ -127,12 +127,15 @@ public MCPlayerInventory getInventory() {
 
 	@Override
 	public MCItemStack getItemAt(Integer slot) {
-		if(slot == null) {
-			return new BukkitMCItemStack(p.getInventory().getItemInMainHand());
-		}
 		ItemStack is = null;
-		//Special slots
-		if(slot == 100) {
+		if(slot == null) {
+			is = p.getInventory().getItemInMainHand();
+			// Empty slots should return null, but unlike other PlayerInventory methods,
+			// getItemInMainHand() never returns null.
+			if(is.getType() == Material.AIR) {
+				return null;
+			}
+		} else if(slot == 100) {
 			is = p.getInventory().getBoots();
 		} else if(slot == 101) {
 			is = p.getInventory().getLeggings();
@@ -142,15 +145,13 @@ public MCItemStack getItemAt(Integer slot) {
 			is = p.getInventory().getHelmet();
 		} else if(slot == -106) {
 			is = p.getInventory().getItemInOffHand();
-		}
-		if(slot >= 0 && slot <= 35) {
+		} else if(slot >= 0 && slot <= 35) {
 			is = p.getInventory().getItem(slot);
 		}
 		if(is == null) {
 			return null;
-		} else {
-			return new BukkitMCItemStack(is);
 		}
+		return new BukkitMCItemStack(is);
 	}
 
 	@Override

src/main/java/com/laytonsmith/core/functions/Enchantments.java

@@ -343,7 +343,7 @@ public Mixed exec(Target t, Environment environment, Mixed... args) throws Confi
 			}
 			MCItemStack is = p.getItemAt(slot instanceof CNull ? null : ArgumentValidation.getInt32(slot, t));
 			if(is == null) {
-				throw new CRECastException("There is no item at slot " + slot, t);
+				return new CArray(t);
 			}
 			return ObjectGenerator.GetGenerator().enchants(is.getEnchantments(), t);
 		}

src/main/java/com/laytonsmith/core/functions/ItemMeta.java

@@ -77,24 +77,18 @@ public Boolean runAsync() {
 
 		@Override
 		public Mixed exec(Target t, Environment environment, Mixed... args) throws ConfigRuntimeException {
-			MCPlayer p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
+			MCPlayer p;
 			MCItemStack is;
 			Mixed slot;
 			if(args.length == 2) {
 				p = Static.GetPlayer(args[0], t);
 				slot = args[1];
 			} else {
+				p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
+				Static.AssertPlayerNonNull(p, t);
 				slot = args[0];
 			}
-			Static.AssertPlayerNonNull(p, t);
-			if(slot instanceof CNull) {
-				is = p.getItemAt(null);
-			} else {
-				is = p.getItemAt(ArgumentValidation.getInt32(slot, t));
-			}
-			if(is == null) {
-				throw new CRECastException("There is no item at slot " + slot, t);
-			}
+			is = p.getItemAt(slot instanceof CNull ? null : ArgumentValidation.getInt32(slot, t));
 			return ObjectGenerator.GetGenerator().itemMeta(is, t);
 		}
 
@@ -186,7 +180,7 @@ public Boolean runAsync() {
 
 		@Override
 		public Mixed exec(Target t, Environment environment, Mixed... args) throws ConfigRuntimeException {
-			MCPlayer p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
+			MCPlayer p;
 			Mixed slot;
 			Mixed meta;
 			MCItemStack is;
@@ -195,15 +189,12 @@ public Mixed exec(Target t, Environment environment, Mixed... args) throws Confi
 				slot = args[1];
 				meta = args[2];
 			} else {
+				p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
+				Static.AssertPlayerNonNull(p, t);
 				slot = args[0];
 				meta = args[1];
 			}
-			Static.AssertPlayerNonNull(p, t);
-			if(slot instanceof CNull) {
-				is = p.getItemAt(null);
-			} else {
-				is = p.getItemAt(ArgumentValidation.getInt32(slot, t));
-			}
+			is = p.getItemAt(slot instanceof CNull ? null : ArgumentValidation.getInt32(slot, t));
 			if(is == null) {
 				throw new CRECastException("There is no item at slot " + slot, t);
 			}
@@ -441,16 +432,21 @@ public Boolean runAsync() {
 
 		@Override
 		public Mixed exec(Target t, Environment environment, Mixed... args) throws ConfigRuntimeException {
-			MCPlayer p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
-			int slot;
+			MCPlayer p;
+			Mixed slot;
 			if(args.length == 2) {
 				p = Static.GetPlayer(args[0], t);
-				slot = ArgumentValidation.getInt32(args[1], t);
+				slot = args[1];
 			} else {
-				slot = ArgumentValidation.getInt32(args[0], t);
+				p = environment.getEnv(CommandHelperEnvironment.class).GetPlayer();
+				Static.AssertPlayerNonNull(p, t);
+				slot = args[0];
 			}
-			Static.AssertPlayerNonNull(p, t);
-			return CBoolean.get(p.getItemAt(slot).getItemMeta() instanceof MCLeatherArmorMeta);
+			MCItemStack is = p.getItemAt(slot instanceof CNull ? null : ArgumentValidation.getInt32(slot, t));
+			if(is == null) {
+				return CBoolean.FALSE;
+			}
+			return CBoolean.get(is.getItemMeta() instanceof MCLeatherArmorMeta);
 		}
 
 		@Override