From 2fceaa803e2b99c4426db24da83a7b0aa50bf24e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 03:54:50 +0200 Subject: [PATCH] Fixed the "moved too quickly" error that would cause players to fall out into the void on 1.3.2. --- .../injector/InjectedServerConnection.java | 19 +++++- .../injector/NetworkServerInjector.java | 32 ++-------- .../protocol/injector/ObjectCloner.java | 64 +++++++++++++++++++ .../protocol/injector/ReplacedArrayList.java | 25 ++++++-- 4 files changed, 107 insertions(+), 33 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java index ea2576db..2b92a150 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java @@ -7,6 +7,8 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import net.sf.cglib.proxy.Factory; + import org.bukkit.Server; import com.comphenix.protocol.reflect.FieldUtils; @@ -134,18 +136,33 @@ class InjectedServerConnection { if (list instanceof ReplacedArrayList) { replacedLists.add((ReplacedArrayList) list); } else { - replacedLists.add(new ReplacedArrayList(list)); + replacedLists.add(createReplacement(list)); listFieldRef.setValue(replacedLists.get(0)); listFields.add(listFieldRef); } } + // Hack to avoid the "moved to quickly" error + private ReplacedArrayList createReplacement(List list) { + return new ReplacedArrayList(list) { + @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 + ObjectCloner.copyTo(inserting, replacement, inserting.getClass()); + } + } + }; + } + /** * Replace the server handler instance kept by the "keep alive" object. * @param oldHandler - old server handler. * @param newHandler - new, proxied server handler. */ public void replaceServerHandler(Object oldHandler, Object newHandler) { + if (!hasAttempted) { injectList(); } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index fb901a1a..5ea5bf0b 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java @@ -13,10 +13,8 @@ import net.sf.cglib.proxy.MethodProxy; import org.bukkit.entity.Player; import com.comphenix.protocol.events.PacketListener; -import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; -import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.instances.CollectionGenerator; import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.reflect.instances.ExistingGenerator; @@ -30,8 +28,6 @@ import com.comphenix.protocol.reflect.instances.PrimitiveGenerator; public class NetworkServerInjector extends PlayerInjector { private static Method sendPacketMethod; - - private StructureModifier serverHandlerModifier; private InjectedServerConnection serverInjection; public NetworkServerInjector(Player player, PacketFilterManager manager, @@ -48,8 +44,6 @@ public class NetworkServerInjector extends PlayerInjector { if (hasInitialized) { if (sendPacketMethod == null) sendPacketMethod = FuzzyReflection.fromObject(serverHandler).getMethodByName("sendPacket.*"); - if (serverHandlerModifier == null) - serverHandlerModifier = new StructureModifier(serverHandler.getClass(), null, false); } } @@ -119,41 +113,23 @@ public class NetworkServerInjector extends PlayerInjector { CollectionGenerator.INSTANCE); Object proxyObject = serverInstances.forEnhancer(ex).getDefault(serverClass); - serverInjection.replaceServerHandler(serverHandler, proxyObject); // Inject it now if (proxyObject != null) { - copyTo(serverHandler, proxyObject); + // This will be done by InjectedServerConnection instead + //copyTo(serverHandler, proxyObject); + serverInjection.replaceServerHandler(serverHandler, proxyObject); serverHandlerRef.setValue(proxyObject); } else { throw new RuntimeException( "Cannot hook player: Unable to find a valid constructor for the NetServerHandler object."); } } - - /** - * Copy every field in server handler A to server handler B. - * @param source - fields to copy. - * @param destination - fields to copy to. - */ - private void copyTo(Object source, Object destination) { - StructureModifier modifierSource = serverHandlerModifier.withTarget(source); - StructureModifier modifierDest = serverHandlerModifier.withTarget(destination); - // Copy every field - try { - for (int i = 0; i < modifierSource.size(); i++) { - modifierDest.write(i, modifierSource.read(i)); - } - } catch (FieldAccessException e) { - throw new RuntimeException("Unable to copy fields from NetServerHandler.", e); - } - } - @Override public void cleanupAll() { if (serverHandlerRef != null && serverHandlerRef.isCurrentSet()) { - copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue()); + ObjectCloner.copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue(), serverHandler.getClass()); serverHandlerRef.revertValue(); } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java b/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java new file mode 100644 index 00000000..c3cec1a3 --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java @@ -0,0 +1,64 @@ +package com.comphenix.protocol.injector; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import com.comphenix.protocol.reflect.FieldAccessException; +import com.comphenix.protocol.reflect.StructureModifier; + +/** + * Can copy an object field by field. + * + * @author Kristian + */ +class ObjectCloner { + + // Cache structure modifiers + @SuppressWarnings("rawtypes") + private static ConcurrentMap> cache = + new ConcurrentHashMap>(); + + /** + * Copy every field in object A to object B. + *

+ * 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. + */ + public static void copyTo(Object source, Object destination, Class commonType) { + + if (source == null) + throw new IllegalArgumentException("Source cannot be NULL"); + if (destination == null) + throw new IllegalArgumentException("Destination cannot be NULL"); + + StructureModifier modifier = cache.get(commonType); + + // Create the structure modifier if we haven't already + if (modifier == null) { + StructureModifier value = new StructureModifier(commonType, null, false); + modifier = cache.putIfAbsent(commonType, value); + + if (modifier == null) + modifier = value; + } + + // Add target + StructureModifier modifierSource = modifier.withTarget(source); + StructureModifier modifierDest = modifier.withTarget(destination); + + // Copy every field + try { + for (int i = 0; i < modifierSource.size(); i++) { + Object value = modifierSource.read(i); + modifierDest.write(i, value); + + // System.out.println(String.format("Writing value %s to %s", + // value, modifier.getFields().get(i).getName())); + } + } catch (FieldAccessException e) { + throw new RuntimeException("Unable to copy fields from " + commonType.getName(), e); + } + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java b/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java index 64e3b0de..2923785c 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java @@ -24,10 +24,21 @@ class ReplacedArrayList extends ForwardingList { this.underlyingList = underlyingList; } + /** + * Invoked when a element inserted is replaced. + * @param inserting - the element inserted. + * @param replacement - the element that it should replace. + */ + protected void onReplacing(TKey inserting, TKey replacement) { + // Default is to do nothing. + } + @Override public boolean add(TKey element) { if (replaceMap.containsKey(element)) { - return super.add(replaceMap.get(element)); + TKey replacement = replaceMap.get(element); + onReplacing(element, replacement); + return super.add(replacement); } else { return super.add(element); } @@ -36,7 +47,9 @@ class ReplacedArrayList extends ForwardingList { @Override public void add(int index, TKey element) { if (replaceMap.containsKey(element)) { - super.add(index, replaceMap.get(element)); + TKey replacement = replaceMap.get(element); + onReplacing(element, replacement); + super.add(index, replacement); } else { super.add(index, element); } @@ -101,8 +114,10 @@ class ReplacedArrayList extends ForwardingList { */ public synchronized void replaceAll(TKey find, TKey replace) { for (int i = 0; i < underlyingList.size(); i++) { - if (Objects.equal(underlyingList.get(i), find)) + if (Objects.equal(underlyingList.get(i), find)) { + onReplacing(find, replace); underlyingList.set(i, replace); + } } } @@ -121,7 +136,9 @@ class ReplacedArrayList extends ForwardingList { TKey replaced = underlyingList.get(i); if (inverse.containsKey(replaced)) { - underlyingList.set(i, inverse.get(replaced)); + TKey original = inverse.get(replaced); + onReplacing(replaced, original); + underlyingList.set(i, original); } }