From dea725e1e30ba7d4f281a90e71f6e0b34a7c7b97 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 27 Dec 2012 10:32:26 +0100 Subject: [PATCH] Move the field cloning reponsibility to FieldCloner. --- .../player/InjectedServerConnection.java | 5 ++- .../player/NetworkServerInjector.java | 5 ++- .../protocol/reflect/ObjectWriter.java | 38 +++++++------------ .../protocol/reflect/cloning/FieldCloner.java | 18 ++++++++- .../protocol/events/PacketContainerTest.java | 20 +++++----- 5 files changed, 49 insertions(+), 37 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java index 307e8b24..6123ec0f 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java @@ -212,6 +212,9 @@ class InjectedServerConnection { * Shut up Eclipse! */ 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) { @@ -219,7 +222,7 @@ class InjectedServerConnection { 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); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java index f13b261c..74a0a3e6 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java +++ b/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 { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/ObjectWriter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/ObjectWriter.java index 82224912..5ab3eb2d 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/ObjectWriter.java +++ b/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> cache = new ConcurrentHashMap>(); - /** - * The default value cloner to use. - */ - private static final Cloner DEFAULT_CLONER = new IdentityCloner(); - /** * Retrieve a usable structure modifier for the given object type. *

@@ -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 getModifier(Class type) { + private StructureModifier 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. - *

- * 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 modifierSource, StructureModifier 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) { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/cloning/FieldCloner.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/cloning/FieldCloner.java index 18d6554b..5eb7f514 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/cloning/FieldCloner.java +++ b/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 modifierSource, + StructureModifier 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; } diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java index 1782518b..bbee0a76 100644 --- a/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java +++ b/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; - - 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; + if (val instanceof Object[]) + return (Object[]) val; + if (val == null) + return null; + + 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; } }