Further improve legacy material handling

EngineHub · Aug 9, 2018 · 12ee67a · 12ee67a
1 parent 709e98e
commit 12ee67a
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 49 deletions.
diff --git a/src/main/java/com/laytonsmith/abstraction/bukkit/BukkitConvertor.java b/src/main/java/com/laytonsmith/abstraction/bukkit/BukkitConvertor.java
@@ -245,6 +245,9 @@ public MCItemStack GetItemStack(int id, int qty) {
 	@Override
 	public MCItemStack GetItemStack(int id, int data, int qty) {
 		Material mat = BukkitMCLegacyMaterial.getMaterial(id, data);
+		if(mat == null) {
+			return new BukkitMCItemStack(new ItemStack(Material.AIR));
+		}
 		return new BukkitMCItemStack(new ItemStack(mat, qty, (short) data));
 	}
 

diff --git a/src/main/java/com/laytonsmith/abstraction/bukkit/blocks/BukkitMCBlock.java b/src/main/java/com/laytonsmith/abstraction/bukkit/blocks/BukkitMCBlock.java
@@ -82,7 +82,11 @@ public void setType(MCMaterial mat, boolean physics) {
 
 	@Override
 	public void setTypeId(int idata) {
-		b.setType(BukkitMCLegacyMaterial.getMaterial(idata));
+		Material mat = BukkitMCLegacyMaterial.getMaterial(idata);
+		if(mat == null) {
+			b.setType(Material.AIR);
+		}
+		b.setType(mat);
 	}
 
 	@Override

diff --git a/src/main/java/com/laytonsmith/abstraction/enums/bukkit/BukkitMCLegacyMaterial.java b/src/main/java/com/laytonsmith/abstraction/enums/bukkit/BukkitMCLegacyMaterial.java
@@ -493,38 +493,36 @@ public int getId() {
 	public static Material getMaterial(int id) {
 		BukkitMCLegacyMaterial legacy = BY_ID.get(id);
 		if(legacy == null) {
-			throw new IllegalArgumentException("Invalid material id: " + id);
+			return null;
 		}
 		return Material.getMaterial(legacy.name(), true);
 	}
 
 	public static Material getMaterial(int id, int data) {
 		BukkitMCLegacyMaterial legacy = BY_ID.get(id);
 		if(legacy == null) {
-			throw new IllegalArgumentException("Invalid material id: " + id);
+			return null;
 		}
 		return getMaterial(legacy.name(), data);
 	}
 
 	public static Material getMaterial(String type, int data) {
-		Material mat = null;
-		try {
-			mat = Material.getMaterial("LEGACY_" + type);
-			if(mat.getMaxDurability() == 0) {
-				mat = Bukkit.getUnsafe().fromLegacy(new MaterialData(mat, (byte) data));
-			} else {
-				mat = Bukkit.getUnsafe().fromLegacy(mat);
-			}
-		} catch (NullPointerException ex) {
-			// probably tests
+		return getMaterial(Material.getMaterial("LEGACY_" + type), data);
+	}
+
+	public static Material getMaterial(Material legacymat, int data) {
+		if(legacymat.getMaxDurability() == 0) {
+			return Bukkit.getUnsafe().fromLegacy(new MaterialData(legacymat, (byte) data));
+		} else {
+			// ignore data when it's actually durability
+			return Bukkit.getUnsafe().fromLegacy(legacymat);
 		}
-		return mat;
 	}
 
 	public static BlockData getBlockData(int id, int data) {
 		BukkitMCLegacyMaterial legacy = BY_ID.get(id);
 		if(legacy == null) {
-			throw new IllegalArgumentException("Invalid material id: " + id);
+			return Material.AIR.createBlockData();
 		}
 		Material mat = Material.getMaterial("LEGACY_" + legacy.name());
 		return Bukkit.getUnsafe().fromLegacy(mat, (byte) data);

diff --git a/src/main/java/com/laytonsmith/core/ObjectGenerator.java b/src/main/java/com/laytonsmith/core/ObjectGenerator.java
@@ -260,7 +260,7 @@ public MCItemStack item(Construct i, Target t, boolean legacy) {
 		}
 		CArray item = (CArray) i;
 		String mat;
-		MCItemStack ret;
+		MCItemStack ret = null;
 		int data = 0;
 		int qty = 1;
 
@@ -271,14 +271,15 @@ public MCItemStack item(Construct i, Target t, boolean legacy) {
 			}
 		}
 
-		if(item.containsKey("name")) {
-			mat = item.get("name", t).val();
-			if(item.containsKey("data")) {
-				data = Static.getInt32(item.get("data", t), t);
-			}
+		if(item.containsKey("data")) {
+			data = Static.getInt32(item.get("data", t), t);
+		}
+
+		if(legacy || item.containsKey("type")) {
+			// Do legacy item conversion
+			if(item.containsKey("name")) {
+				mat = item.get("name", t).val();
 
-			if(legacy) {
-				// ensure accurate conversion by assuming string is a legacy name
 				MCMaterial material;
 				if(mat.equals("MAP")) {
 					// special handling, ignore data until later
@@ -290,33 +291,43 @@ public MCItemStack item(Construct i, Target t, boolean legacy) {
 					throw new CREFormatException("Could not convert legacy item from " + mat + ":" + data, t);
 				}
 				ret = StaticLayer.GetItemStack(material, qty);
-
-			} else if(data > 0) {
-				ret = StaticLayer.GetItemStack(mat, data, qty);
+				CHLog.GetLogger().w(CHLog.Tags.DEPRECATION, "Converted \"" + mat + ":" + data + "\"" + " to "
+						+ ret.getType().getName(), t);
 			} else {
-				ret = StaticLayer.GetItemStack(mat, qty);
-			}
-		} else if(item.containsKey("type")) {
-			Construct type = item.get("type", t);
-			if(type instanceof CString) {
-				int seperatorIndex = type.val().indexOf(':');
-				if(seperatorIndex != -1) {
-					try {
-						data = Integer.parseInt(type.val().substring(seperatorIndex + 1));
-					} catch (NumberFormatException e) {
-						throw new CRERangeException("The item data \"" + type.val().substring(seperatorIndex + 1)
-								+ "\" is not a valid integer.", t);
+				Construct type = item.get("type", t);
+				if(type instanceof CString) {
+					int seperatorIndex = type.val().indexOf(':');
+					if(seperatorIndex != -1) {
+						try {
+							data = Integer.parseInt(type.val().substring(seperatorIndex + 1));
+						} catch (NumberFormatException e) {
+							throw new CRERangeException("The item data \"" + type.val().substring(seperatorIndex + 1)
+									+ "\" is not a valid integer.", t);
+						}
+						type = new CString(type.val().substring(0, seperatorIndex), t);
 					}
-					type = new CString(type.val().substring(0, seperatorIndex), t);
 				}
+				int id = Static.getInt32(type, t);
+				if(id == 358) {
+					// special map handling, ignore data until later
+					ret = StaticLayer.GetItemStack(id, 0, qty);
+				} else {
+					ret = StaticLayer.GetItemStack(id, data, qty);
+				}
+				CHLog.GetLogger().w(CHLog.Tags.DEPRECATION, "Converted \"" + type.val() + ":" + data + "\"" + " to "
+						+ ret.getType().getName(), t);
 			}
-			if(item.containsKey("data")) {
-				data = Static.getInt32(item.get("data", t), t);
+
+		} else if(item.containsKey("name")) {
+			mat = item.get("name", t).val();
+			if(data > 0) {
+				ret = StaticLayer.GetItemStack(mat, data, qty);
+			} else {
+				ret = StaticLayer.GetItemStack(mat, qty);
 			}
-			ret = StaticLayer.GetItemStack(Static.getInt32(type, t), data, qty);
-			CHLog.GetLogger().w(CHLog.Tags.DEPRECATION, "Converted \"" + type.val() + ":" + data + "\"" + " to "
-					+ ret.getType().getName(), t);
-		} else {
+		}
+
+		if(ret == null) {
 			throw new CREFormatException("Could not find item name!", t);
 		}
 

diff --git a/src/main/java/com/laytonsmith/core/functions/Minecraft.java b/src/main/java/com/laytonsmith/core/functions/Minecraft.java
@@ -265,11 +265,13 @@ public Integer[] numArgs() {
 		@Override
 		public String docs() {
 			return "array {itemArray} Converts old pre-1.13 item arrays to new item arrays."
-					+ " Almost all item arrays will be converted successfully just by passing them to a function that"
-					+ " accepts item arrays. However this function ensures best case accuracy."
-					+ " In addition, conversions may no longer be supported in future versions of Minecraft."
+					+ " Almost all item arrays will be converted successfully when passing them to a function that"
+					+ " accepts item arrays. However, if the array is missing the 'type' key, several item types"
+					+ " might not convert accurately due to name conflicts."
+					+ " This function offers convenience and ensures better conversion accuracy."
 					+ " Use this if you have item arrays stored in a database and want to convert them all at once."
-					+ " Passing new item arrays to this function is not supported.";
+					+ " Passing new item arrays to this function is not supported."
+					+ " Conversions may not be supported in far future versions of Minecraft.";
 		}
 
 		@Override