Update to 1.13.1, rework cloning, fix a particle NPE

modules/API/src/main/java/com/comphenix/protocol/reflect/cloning/BukkitCloner.java

@@ -21,20 +21,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 import com.comphenix.protocol.reflect.EquivalentConverter;
 import com.comphenix.protocol.reflect.StructureModifier;
 import com.comphenix.protocol.utility.MinecraftReflection;
-import com.comphenix.protocol.utility.MinecraftVersion;
-import com.comphenix.protocol.wrappers.BlockPosition;
-import com.comphenix.protocol.wrappers.BukkitConverters;
-import com.comphenix.protocol.wrappers.ChunkPosition;
-import com.comphenix.protocol.wrappers.MinecraftKey;
-import com.comphenix.protocol.wrappers.WrappedBlockData;
-import com.comphenix.protocol.wrappers.WrappedDataWatcher;
-import com.comphenix.protocol.wrappers.WrappedServerPing;
+import com.comphenix.protocol.wrappers.*;
 import com.comphenix.protocol.wrappers.nbt.NbtFactory;
-import com.comphenix.protocol.wrappers.nbt.NbtWrapper;
 import com.google.common.collect.Maps;
 
 /**
@@ -43,114 +37,77 @@
  * @author Kristian
  */
 public class BukkitCloner implements Cloner {
-	// List of classes we support
-	private final Map<Integer, Class<?>> clonableClasses = Maps.newConcurrentMap();
+	private static final Map<Class<?>, Function<Object, Object>> CLONERS = Maps.newConcurrentMap();
 
-	public BukkitCloner() {
-		addClass(0, MinecraftReflection.getItemStackClass());
-		addClass(1, MinecraftReflection.getDataWatcherClass());
-
-		// Try to add position classes
+	private static void fromWrapper(Supplier<Class<?>> getClass, Function<Object, ClonableWrapper> fromHandle) {
 		try {
-			addClass(2, MinecraftReflection.getBlockPositionClass());
-		} catch (Throwable ex) {
-		}
-
-		try {
-			addClass(3, MinecraftReflection.getChunkPositionClass());
-		} catch (Throwable ex) {
-		}
-
-		if (MinecraftReflection.isUsingNetty()) {
-			addClass(4, MinecraftReflection.getServerPingClass());
-		}
-
-		if (MinecraftReflection.watcherObjectExists()) {
-			addClass(5, MinecraftReflection.getDataWatcherSerializerClass());
-			addClass(6, MinecraftReflection.getMinecraftKeyClass());
-		}
-
-		try {
-			addClass(7, MinecraftReflection.getIBlockDataClass());
-		} catch (Throwable ex) {
-		}
+			Class<?> nmsClass = getClass.get();
+			if (nmsClass != null) {
+				CLONERS.put(nmsClass, nmsObject -> fromHandle.apply(nmsObject).deepClone().getHandle());
+			}
+		} catch (RuntimeException ignored) { }
+	}
 
+	private static void fromConverter(Supplier<Class<?>> getClass, EquivalentConverter converter) {
 		try {
-			addClass(8, MinecraftReflection.getNonNullListClass());
-		} catch (Throwable ex) {
-		}
+			Class<?> nmsClass = getClass.get();
+			if (nmsClass != null) {
+				CLONERS.put(nmsClass, nmsObject -> converter.getGeneric(converter.getSpecific(nmsObject)));
+			}
+		} catch (RuntimeException ignored) { }
+	}
 
+	private static void fromManual(Supplier<Class<?>> getClass, Function<Object, Object> cloner) {
 		try {
-			addClass(9, MinecraftReflection.getNBTBaseClass());
-		} catch (Throwable ex) { }
+			Class<?> nmsClass = getClass.get();
+			if (nmsClass != null) {
+				CLONERS.put(nmsClass, cloner);
+			}
+		} catch (RuntimeException ignored) { }
 	}
 
-	private void addClass(int id, Class<?> clazz) {
-		if (clazz != null)
-			clonableClasses.put(id, clazz);
+	static {
+		fromManual(MinecraftReflection::getItemStackClass, source ->
+				MinecraftReflection.getMinecraftItemStack(MinecraftReflection.getBukkitItemStack(source).clone()));
+		fromWrapper(MinecraftReflection::getDataWatcherClass, WrappedDataWatcher::new);
+		fromConverter(MinecraftReflection::getBlockPositionClass, BlockPosition.getConverter());
+		fromConverter(MinecraftReflection::getChunkPositionClass, ChunkPosition.getConverter());
+		fromWrapper(MinecraftReflection::getServerPingClass, WrappedServerPing::fromHandle);
+		fromConverter(MinecraftReflection::getMinecraftKeyClass, MinecraftKey.getConverter());
+		fromWrapper(MinecraftReflection::getIBlockDataClass, WrappedBlockData::fromHandle);
+		fromManual(MinecraftReflection::getNonNullListClass, source -> nonNullListCloner().clone(source));
+		fromWrapper(MinecraftReflection::getNBTBaseClass, NbtFactory::fromNMS);
 	}
 
-	private int findMatchingClass(Class<?> type) {
-		// See if is a subclass of any of our supported superclasses
-		for (Entry<Integer, Class<?>> entry : clonableClasses.entrySet()) {
-			if (entry.getValue().isAssignableFrom(type)) {
-				return entry.getKey();
+	private Function<Object, Object> findCloner(Class<?> type) {
+		for (Entry<Class<?>, Function<Object, Object>> entry : CLONERS.entrySet()) {
+			if (entry.getKey().isAssignableFrom(type)) {
+				return entry.getValue();
 			}
 		}
 
-		return -1;
+		return null;
 	}
 
 	@Override
 	public boolean canClone(Object source) {
 		if (source == null)
 			return false;
 
-		return findMatchingClass(source.getClass()) >= 0;
+		return findCloner(source.getClass()) != null;
 	}
 
 	@Override
 	public Object clone(Object source) {
 		if (source == null)
 			throw new IllegalArgumentException("source cannot be NULL.");
 
-		// Convert to a wrapper
-		switch (findMatchingClass(source.getClass())) {
-			case 0:
-				return MinecraftReflection.getMinecraftItemStack(MinecraftReflection.getBukkitItemStack(source).clone());
-			case 1:
-				EquivalentConverter<WrappedDataWatcher> dataConverter = BukkitConverters.getDataWatcherConverter();
-				return dataConverter.getGeneric(dataConverter.getSpecific(source).deepClone());
-			case 2:
-				EquivalentConverter<BlockPosition> blockConverter = BlockPosition.getConverter();
-				return blockConverter.getGeneric(blockConverter.getSpecific(source));
-			case 3:
-				EquivalentConverter<ChunkPosition> chunkConverter = ChunkPosition.getConverter();
-				return chunkConverter.getGeneric(chunkConverter.getSpecific(source));
-			case 4:
-				EquivalentConverter<WrappedServerPing> serverConverter = BukkitConverters.getWrappedServerPingConverter();
-				return serverConverter.getGeneric(serverConverter.getSpecific(source).deepClone());
-			case 5:
-				return source;
-			case 6:
-				EquivalentConverter<MinecraftKey> keyConverter = MinecraftKey.getConverter();
-				return keyConverter.getGeneric(keyConverter.getSpecific(source));
-			case 7:
-				EquivalentConverter<WrappedBlockData> blockDataConverter = BukkitConverters.getWrappedBlockDataConverter();
-				return blockDataConverter.getGeneric(blockDataConverter.getSpecific(source).deepClone());
-			case 8:
-				return nonNullListCloner().clone(source);
-			case 9:
-				NbtWrapper<?> clone = (NbtWrapper<?>) NbtFactory.fromNMS(source).deepClone();
-				return clone.getHandle();
-			default:
-				throw new IllegalArgumentException("Cannot clone objects of type " + source.getClass());
-		}
+		return findCloner(source.getClass()).apply(source);
 	}
 
 	private static Constructor<?> nonNullList = null;
 
-	private static final Cloner nonNullListCloner() {
+	private static Cloner nonNullListCloner() {
 		return new Cloner() {
 			@Override
 			public boolean canClone(Object source) {

modules/API/src/main/java/com/comphenix/protocol/reflect/cloning/ImmutableDetector.java

@@ -26,11 +26,15 @@
 import java.net.URL;
 import java.security.PublicKey;
 import java.util.Locale;
+import java.util.Set;
 import java.util.UUID;
+import java.util.function.Supplier;
 
 import javax.crypto.SecretKey;
 
 import com.comphenix.protocol.utility.MinecraftReflection;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.google.common.primitives.Primitives;
 
 /**
@@ -42,19 +46,41 @@
  */
 public class ImmutableDetector implements Cloner {
 	// Notable immutable classes we might encounter
-	private static final Class<?>[] immutableClasses = {
+	private static final Set<Class<?>> immutableClasses = ImmutableSet.of(
 			StackTraceElement.class, BigDecimal.class,
 			BigInteger.class, Locale.class, UUID.class,
 			URL.class, URI.class, Inet4Address.class,
 			Inet6Address.class, InetSocketAddress.class,
 			SecretKey.class, PublicKey.class
-	};
+	);
+
+	private static final Set<Class<?>> immutableNMS = Sets.newConcurrentHashSet();
+
+	static {
+		add(MinecraftReflection::getGameProfileClass);
+		add(MinecraftReflection::getDataWatcherSerializerClass);
+		add(() -> MinecraftReflection.getMinecraftClass("SoundEffect"));
+		add(MinecraftReflection::getBlockClass);
+		add(MinecraftReflection::getItemClass);
+		add(MinecraftReflection::getFluidTypeClass);
+		add(MinecraftReflection::getParticleTypeClass);
+	}
+
+	private static void add(Supplier<Class<?>> getClass) {
+		try {
+			Class<?> clazz = getClass.get();
+			if (clazz != null) {
+				immutableNMS.add(clazz);
+			}
+		} catch (RuntimeException ignored) { }
+	}
 
 	@Override
 	public boolean canClone(Object source) {
 		// Don't accept NULL
-		if (source == null)
+		if (source == null) {
 			return false;
+		}
 
 		return isImmutable(source.getClass());
 	}
@@ -66,44 +92,33 @@ public boolean canClone(Object source) {
 	 */
 	public static boolean isImmutable(Class<?> type) {
 		// Cases that are definitely not true
-		if (type.isArray())
+		if (type.isArray()) {
 			return false;
+		}
 
 		// All primitive types
-		if (Primitives.isWrapperType(type) || String.class.equals(type))
+		if (Primitives.isWrapperType(type) || String.class.equals(type)) {
 			return true;
+		}
 
 		// May not be true, but if so, that kind of code is broken anyways
-		if (isEnumWorkaround(type))
+		if (isEnumWorkaround(type)) {
 			return true;
-
-		for (Class<?> clazz : immutableClasses)
-			if (clazz.equals(type))
-				return true;
-
-		// Check for known immutable classes in 1.7.2
-		if (MinecraftReflection.isUsingNetty()) {
-			if (type.equals(MinecraftReflection.getGameProfileClass())) {
-				return true;
-			}
 		}
 
-		// Check for known immutable classes in 1.9
-		if (MinecraftReflection.watcherObjectExists()) {
-			if (type.equals(MinecraftReflection.getDataWatcherSerializerClass())
-					|| type.equals(MinecraftReflection.getMinecraftClass("SoundEffect"))) {
-				return true;
-			}
+		// No good way to clone lambdas
+		if (type.getName().contains("$$Lambda$")) {
+			return true;
 		}
 
-		if (MinecraftReflection.is(MinecraftReflection.getBlockClass(), type)
-				|| MinecraftReflection.is(MinecraftReflection.getItemClass(), type)
-				|| MinecraftReflection.is(MinecraftReflection.getFluidTypeClass(), type)) {
+		if (immutableClasses.contains(type)) {
 			return true;
 		}
 
-		if (type.getName().contains("$$Lambda$")) {
-			return true;
+		for (Class<?> clazz : immutableNMS) {
+			if (MinecraftReflection.is(clazz, type)) {
+				return true;
+			}
 		}
 
 		// Probably not
@@ -113,10 +128,13 @@ public static boolean isImmutable(Class<?> type) {
 	// This is just great. Just great.
 	private static boolean isEnumWorkaround(Class<?> enumClass) {
 		while (enumClass != null) {
-			if (enumClass.isEnum())
+			if (enumClass.isEnum()) {
 				return true;
+			}
+
 			enumClass = enumClass.getSuperclass();
 		}
+
 		return false;
 	}
 

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

@@ -48,6 +48,7 @@ private static NavigableMap<MinecraftVersion, Integer> createLookup() {
 		map.put(new MinecraftVersion(1, 12, 1), 338);
 		map.put(new MinecraftVersion(1, 12, 2), 340);
 		map.put(new MinecraftVersion(1, 13, 0), 393);
+		map.put(new MinecraftVersion(1, 13, 1), 401);
 		return map;
 	}
 

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

@@ -1156,6 +1156,10 @@ public static Class<?> getFluidTypeClass() {
 		return getNullableNMS("FluidType");
 	}
 
+	public static Class<?> getParticleTypeClass() {
+		return getNullableNMS("ParticleType");
+	}
+
 	/**
 	 * Retrieve the WorldType class.
 	 * @return The WorldType class.
@@ -1360,7 +1364,7 @@ public static boolean watcherObjectExists() {
 
 	public static Class<?> getDataWatcherSerializerClass() {
 		// TODO Implement a fallback
-		return getMinecraftClass("DataWatcherSerializer");
+		return getNullableNMS("DataWatcherSerializer");
 	}
 
 	public static Class<?> getDataWatcherRegistryClass() {

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

@@ -0,0 +1,7 @@
+package com.comphenix.protocol.wrappers;
+
+public interface ClonableWrapper {
+	Object getHandle();
+	ClonableWrapper deepClone();
+
+}

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

@@ -33,7 +33,7 @@
  * @author dmulloy2
  */
 
-public abstract class WrappedBlockData extends AbstractWrapper {
+public abstract class WrappedBlockData extends AbstractWrapper implements ClonableWrapper {
 	private static final Class<?> MAGIC_NUMBERS = MinecraftReflection.getCraftBukkitClass("util.CraftMagicNumbers");
 	private static final Class<?> IBLOCK_DATA = MinecraftReflection.getIBlockDataClass();
 	private static final Class<?> BLOCK = MinecraftReflection.getBlockClass();

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

@@ -42,7 +42,7 @@
  * Represents a DataWatcher
  * @author dmulloy2
  */
-public class WrappedDataWatcher extends AbstractWrapper implements Iterable<WrappedWatchableObject> {
+public class WrappedDataWatcher extends AbstractWrapper implements Iterable<WrappedWatchableObject>, ClonableWrapper {
 	private static final Class<?> HANDLE_TYPE = MinecraftReflection.getDataWatcherClass();
 
 	private static MethodAccessor GETTER = null;

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

@@ -91,6 +91,8 @@ public Object getHandle() {
 	}
 
 	public static WrappedParticle fromHandle(Object handle) {
+		ensureMethods();
+
 		Particle bukkit = (Particle) toBukkit.invoke(null, handle);
 		Object data = null;
 

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

@@ -40,7 +40,7 @@
  * Represents a server ping packet data.
  * @author Kristian
  */
-public class WrappedServerPing extends AbstractWrapper {
+public class WrappedServerPing extends AbstractWrapper implements ClonableWrapper {
 	private static Class<?> GAME_PROFILE = MinecraftReflection.getGameProfileClass();
 	private static Class<?> GAME_PROFILE_ARRAY = MinecraftReflection.getArrayClass(GAME_PROFILE);