Move the field cloning reponsibility to FieldCloner.

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java

@@ -213,13 +213,16 @@ class InjectedServerConnection {
 			 */
 			private static final long serialVersionUID = 2070481080950500367L;
 			
+			// Object writer we'll use
+			private final ObjectWriter writer = new ObjectWriter();
+
 			@Override
 			protected void onReplacing(Object inserting, Object replacement) {
 				// Is this a normal Minecraft object?
 				if (!(inserting instanceof Factory)) {
 					// If so, copy the content of the old element to the new
 					try {
-						ObjectWriter.copyTo(inserting, replacement, inserting.getClass());
+						writer.copyTo(inserting, replacement, inserting.getClass());
 					} catch (Throwable e) {
 						reporter.reportDetailed(InjectedServerConnection.this, "Cannot copy old " + inserting + 
 								 				" to new.", e, inserting, replacement);

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java

@@ -66,6 +66,9 @@ public class NetworkServerInjector extends PlayerInjector {
 	// Whether or not the player has disconnected
 	private boolean hasDisconnected;
 	
+	// Used to copy fields
+	private final ObjectWriter writer = new ObjectWriter();
+	
 	public NetworkServerInjector(
 			ClassLoader classLoader, ErrorReporter reporter, Player player, 
 			ListenerInvoker invoker, IntegerSet sendingFilters, 
@@ -248,7 +251,7 @@ public class NetworkServerInjector extends PlayerInjector {
 	@Override
 	protected void cleanHook() {
 		if (serverHandlerRef != null && serverHandlerRef.isCurrentSet()) {
-			ObjectWriter.copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue(), serverHandler.getClass());
+			writer.copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue(), serverHandler.getClass());
 			serverHandlerRef.revertValue();
 			
 			try {

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/ObjectWriter.java

@@ -23,8 +23,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
 import com.comphenix.protocol.injector.StructureCache;
-import com.comphenix.protocol.reflect.cloning.Cloner;
-import com.comphenix.protocol.reflect.cloning.IdentityCloner;
 import com.comphenix.protocol.utility.MinecraftReflection;
 
 /**
@@ -38,11 +36,6 @@ public class ObjectWriter {
 	private static ConcurrentMap<Class, StructureModifier<Object>> cache =
 			new ConcurrentHashMap<Class, StructureModifier<Object>>();
 	
-	/**
-	 * The default value cloner to use.
-	 */
-	private static final Cloner DEFAULT_CLONER = new IdentityCloner();
-	
 	/**
 	 * Retrieve a usable structure modifier for the given object type.
 	 * <p>
@@ -50,7 +43,7 @@ public class ObjectWriter {
 	 * @param type - the type of the object we are modifying.
 	 * @return A structure modifier for the given type.
 	 */
-	private static StructureModifier<Object> getModifier(Class<?> type) {
+	private StructureModifier<Object> getModifier(Class<?> type) {
 		Class<?> packetClass = MinecraftReflection.getPacketClass();
 		
 		// Handle subclasses of the packet class with our custom structure cache
@@ -82,26 +75,24 @@ public class ObjectWriter {
 	 * @param destination - fields to copy to.
 	 * @param commonType - type containing each field to copy.
 	 */
-	public static void copyTo(Object source, Object destination, Class<?> commonType) {
+	public void copyTo(Object source, Object destination, Class<?> commonType) {
 		// Note that we indicate that public fields will be copied the first time around
-		copyToInternal(source, destination, commonType, DEFAULT_CLONER, true);
+		copyToInternal(source, destination, commonType, true);
 	}
 
 	/**
-	 * Copy every field in object A to object B. Each value is copied using the supplied cloner.
-	 * <p>
-	 * The two objects must have the same number of fields of the same type.
-	 * @param source - fields to copy.
-	 * @param destination - fields to copy to.
-	 * @param commonType - type containing each field to copy.
-	 * @param valueCloner - a object responsible for copying the content of each field.
+	 * Called for every non-static field that will be copied.
+	 * @param modifierSource - modifier for the original object.
+	 * @param modifierDest - modifier for the new cloned object.
+	 * @param fieldIndex - the current field index.
 	 */
-	public static void copyTo(Object source, Object destination, Class<?> commonType, Cloner valueCloner) {
-		copyToInternal(source, destination, commonType, valueCloner, true);
+	protected void transformField(StructureModifier<Object> modifierSource, StructureModifier<Object> modifierDest, int fieldIndex) {
+		Object value = modifierSource.read(fieldIndex);
+		modifierDest.write(fieldIndex, value);
 	}
 	
 	// Internal method that will actually implement the recursion
-	private static void copyToInternal(Object source, Object destination, Class<?> commonType, Cloner valueCloner, boolean copyPublic) {
+	private void copyToInternal(Object source, Object destination, Class<?> commonType, boolean copyPublic) {
 		if (source == null)
 			throw new IllegalArgumentException("Source cannot be NULL");
 		if (destination == null)
@@ -119,10 +110,9 @@ public class ObjectWriter {
 				Field field = modifierSource.getField(i);
 				int mod = field.getModifiers();
 				
-				// Skip static fields. We also get the "public" field fairly often, so we'll skip that.
+				// Skip static fields. We also get the "public" fields fairly often, so we'll skip that.
 				if (!Modifier.isStatic(mod) && (!Modifier.isPublic(mod) || copyPublic)) {
-					Object value = modifierSource.read(i);
-					modifierDest.write(i, valueCloner.clone(value));
+					transformField(modifierSource, modifierDest, i);
 				}
 			}
 			
@@ -130,7 +120,7 @@ public class ObjectWriter {
 			Class<?> superclass = commonType.getSuperclass();
 			
 			if (!superclass.equals(Object.class)) {
-				copyToInternal(source, destination, superclass, valueCloner, false);
+				copyToInternal(source, destination, superclass, false);
 			}
 			
 		} catch (FieldAccessException e) {

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/cloning/FieldCloner.java

@@ -1,6 +1,7 @@
 package com.comphenix.protocol.reflect.cloning;
 
 import com.comphenix.protocol.reflect.ObjectWriter;
+import com.comphenix.protocol.reflect.StructureModifier;
 import com.comphenix.protocol.reflect.instances.InstanceProvider;
 
 /**
@@ -12,6 +13,9 @@ public class FieldCloner implements Cloner {
 	private final Cloner defaultCloner;
 	private final InstanceProvider instanceProvider;
 	
+	// Used to clone objects
+	private final ObjectWriter writer;
+	
 	/**
 	 * Constructs a field cloner that copies objects by reading and writing the internal fields directly.
 	 * @param defaultCloner - the default cloner used while copying fields.
@@ -20,6 +24,16 @@ public class FieldCloner implements Cloner {
 	public FieldCloner(Cloner defaultCloner, InstanceProvider instanceProvider) {
 		this.defaultCloner = defaultCloner;
 		this.instanceProvider = instanceProvider;
+		
+		// Remember to clone the value too
+		this.writer = new ObjectWriter() {
+			@Override
+			protected void transformField(StructureModifier<Object> modifierSource,
+					StructureModifier<Object> modifierDest, int fieldIndex) {
+				Object value = modifierSource.read(fieldIndex);
+				modifierDest.write(fieldIndex, getDefaultCloner().clone(value));
+			}
+		};
 	}
 
 	@Override
@@ -38,8 +52,8 @@ public class FieldCloner implements Cloner {
 		
 		Object copy = instanceProvider.create(source.getClass());
 		
-		// Copy public and private fields alike. Skip static and transient fields.
-		ObjectWriter.copyTo(source, copy, source.getClass(), defaultCloner);
+		// Copy public and private fields alike. Skip static fields.
+		writer.copyTo(source, copy, source.getClass());
 		return copy;
 	}
 	

ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java

@@ -367,14 +367,16 @@ public class PacketContainerTest {
 	 * @return An object array.
 	 */
 	private Object[] getArray(Object val) {
-	    if (val instanceof Object[])
-	        return (Object[]) val;
+		if (val instanceof Object[])
+			return (Object[]) val;
+		if (val == null)
+			return null;
 
-	     int arrlength = Array.getLength(val);
-	     Object[] outputArray = new Object[arrlength];
+		int arrlength = Array.getLength(val);
+		Object[] outputArray = new Object[arrlength];
  
-	     for (int i = 0; i < arrlength; ++i)
-	        outputArray[i] = Array.get(val, i);
-	     return outputArray;
+		for (int i = 0; i < arrlength; ++i)
+			outputArray[i] = Array.get(val, i);
+		return outputArray;
 	}
 }