From f5cb7ddc7b2384ab7efa2a844c18dc809db234fb Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 23 Nov 2012 08:48:08 +0100 Subject: [PATCH 01/13] Adding a blocking hash map to use while looking up data connections. --- .../protocol/concurrency/BlockingHashMap.java | 165 ++++++++++++++++++ .../protocol/injector/PacketInjector.java | 18 +- .../player/PlayerInjectionHandler.java | 62 ++++--- ProtocolLib/src/main/resources/plugin.yml | 2 +- .../concurrency/BlockingHashMapTest.java | 42 +++++ 5 files changed, 257 insertions(+), 32 deletions(-) create mode 100644 ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/BlockingHashMap.java create mode 100644 ProtocolLib/src/test/java/com/comphenix/protocol/concurrency/BlockingHashMapTest.java diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/BlockingHashMap.java b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/BlockingHashMap.java new file mode 100644 index 00000000..793f3c11 --- /dev/null +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/BlockingHashMap.java @@ -0,0 +1,165 @@ +package com.comphenix.protocol.concurrency; + +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; + +import com.google.common.collect.MapMaker; + +/** + * A map that supports blocking on read operations. Null keys are not supported. + *

+ * Keys are stored as weak references, and will be automatically removed once they've all been dereferenced. + *

+ * @author Kristian + * + * @param - type of the key. + * @param - type of the value. + */ +public class BlockingHashMap { + + // Map of values + private final ConcurrentMap backingMap; + + // Map of locked objects + private final ConcurrentMap locks; + + /** + * Initialize a new map. + */ + public BlockingHashMap() { + backingMap = new MapMaker().weakKeys().makeMap(); + locks = new MapMaker().weakKeys().makeMap(); + } + + /** + * Initialize a new map. + * @return The created map. + */ + public static BlockingHashMap create() { + return new BlockingHashMap(); + } + + /** + * Waits until a value has been associated with the given key, and then retrieves that value. + * @param key - the key whose associated value is to be returned + * @return The value to which the specified key is mapped. + * @throws InterruptedException If the current thread got interrupted while waiting. + */ + public TValue get(TKey key) throws InterruptedException { + if (key == null) + throw new IllegalArgumentException("key cannot be NULL."); + + TValue value = backingMap.get(key); + + // Only lock if no value is available + if (value == null) { + final Object lock = getLock(key); + + synchronized (lock) { + while (value == null) { + lock.wait(); + value = backingMap.get(key); + } + } + } + + return value; + } + + /** + * Waits until a value has been associated with the given key, and then retrieves that value. + * @param key - the key whose associated value is to be returned + * @param timeout - the amount of time to wait until an association has been made. + * @param unit - unit of timeout. + * @return The value to which the specified key is mapped, or NULL if the timeout elapsed. + * @throws InterruptedException If the current thread got interrupted while waiting. + */ + public TValue get(TKey key, long timeout, TimeUnit unit) throws InterruptedException { + if (key == null) + throw new IllegalArgumentException("key cannot be NULL."); + if (unit == null) + throw new IllegalArgumentException("Unit cannot be NULL."); + + TValue value = backingMap.get(key); + + // Only lock if no value is available + if (value == null) { + final Object lock = getLock(key); + final long stopTimeNS = System.nanoTime() + unit.toNanos(timeout); + + // Don't exceed the timeout + synchronized (lock) { + while (value == null) { + long remainingTime = stopTimeNS - System.nanoTime(); + + if (remainingTime > 0) { + TimeUnit.NANOSECONDS.timedWait(lock, remainingTime); + value = backingMap.get(key); + } + } + } + } + + return value; + } + + /** + * Associate a given key with the given value. + *

+ * Wakes up any blocking getters on this specific key. + * + * @param key - the key to associate. + * @param value - the value. + * @return The previously associated value. + */ + public TValue put(TKey key, TValue value) { + if (value == null) + throw new IllegalArgumentException("This map doesn't support NULL values."); + + final TValue previous = backingMap.put(key, value); + final Object lock = getLock(key); + + // Inform our readers about this change + synchronized (lock) { + lock.notifyAll(); + return previous; + } + } + + public int size() { + return backingMap.size(); + } + + public Collection values() { + return backingMap.values(); + } + + public Set keys() { + return backingMap.keySet(); + } + + /** + * Atomically retrieve the lock associated with a given key. + * @param key - the current key. + * @return An asssociated lock. + */ + private Object getLock(TKey key) { + Object lock = locks.get(key); + + if (lock == null) { + Object created = new Object(); + + // Do this atomically + lock = locks.putIfAbsent(key, created); + + // If we succeeded, use the latch we created - otherwise, use the already inserted latch + if (lock == null) { + lock = created; + } + } + + return lock; + } +} diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java index 7630c360..637ec025 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java @@ -202,13 +202,19 @@ class PacketInjector { // Called from the ReadPacketModified monitor PacketEvent packetRecieved(PacketContainer packet, DataInputStream input) { - Player client = playerInjection.getPlayerByConnection(input); - - // Never invoke a event if we don't know where it's from - if (client != null) - return packetRecieved(packet, client); - else + try { + Player client = playerInjection.getPlayerByConnection(input); + + // Never invoke a event if we don't know where it's from + if (client != null) + return packetRecieved(packet, client); + else + return null; + + } catch (InterruptedException e) { + reporter.reportDetailed(this, "Thread was interrupted.", e, packet, input); return null; + } } /** diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java index fb2136b3..7d4a6a4a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -24,12 +24,14 @@ import java.net.Socket; import java.net.SocketAddress; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import net.minecraft.server.Packet; import org.bukkit.Server; import org.bukkit.entity.Player; +import com.comphenix.protocol.concurrency.BlockingHashMap; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.events.PacketAdapter; import com.comphenix.protocol.events.PacketContainer; @@ -47,6 +49,10 @@ import com.google.common.collect.Maps; * @author Kristian */ public class PlayerInjectionHandler { + /** + * The maximum number of milliseconds to wait until a player can be looked up by connection. + */ + private static final long TIMEOUT_PLAYER_LOOKUP = 1000; // ms /** * The highest possible packet ID. It's unlikely that this value will ever change. @@ -64,9 +70,11 @@ public class PlayerInjectionHandler { // Player injection private Map addressLookup = Maps.newConcurrentMap(); - private Map dataInputLookup = Maps.newConcurrentMap(); private Map playerInjection = Maps.newConcurrentMap(); + // Lookup player by connection + private BlockingHashMap dataInputLookup = BlockingHashMap.create(); + // Player injection types private volatile PlayerInjectHooks loginPlayerHook = PlayerInjectHooks.NETWORK_SERVER_OBJECT; private volatile PlayerInjectHooks playingPlayerHook = PlayerInjectHooks.NETWORK_SERVER_OBJECT; @@ -193,26 +201,37 @@ public class PlayerInjectionHandler { * Retrieve a player by its DataInput connection. * @param inputStream - the associated DataInput connection. * @return The player. + * @throws InterruptedException If the thread was interrupted during the wait. */ - public Player getPlayerByConnection(DataInputStream inputStream) { - try { - // Concurrency issue! - netLoginInjector.getReadLock().lock(); - - PlayerInjector injector = dataInputLookup.get(inputStream); - - if (injector != null) { - return injector.getPlayer(); - } else { - reporter.reportWarning(this, "Unable to find stream: " + inputStream); - return null; - } - - } finally { - netLoginInjector.getReadLock().unlock(); + public Player getPlayerByConnection(DataInputStream inputStream) throws InterruptedException { + return getPlayerByConnection(inputStream, TIMEOUT_PLAYER_LOOKUP, TimeUnit.MILLISECONDS); + } + + /** + * Retrieve a player by its DataInput connection. + * @param inputStream - the associated DataInput connection. + * @param playerTimeout - the amount of time to wait for a result. + * @param unit - unit of playerTimeout. + * @return The player. + * @throws InterruptedException If the thread was interrupted during the wait. + */ + public Player getPlayerByConnection(DataInputStream inputStream, long playerTimeout, TimeUnit unit) throws InterruptedException { + + PlayerInjector injector = dataInputLookup.get(inputStream, playerTimeout, unit); + + if (injector != null) { + return injector.getPlayer(); + } else { + reporter.reportWarning(this, "Unable to find stream: " + inputStream); + return null; } } + /** + * Helper function that retrieves the injector type of a given player injector. + * @param injector - injector type. + * @return The injector type. + */ private PlayerInjectHooks getInjectorType(PlayerInjector injector) { return injector != null ? injector.getHookType() : PlayerInjectHooks.NONE; } @@ -404,7 +423,6 @@ public class PlayerInjectionHandler { PlayerInjector injector = playerInjection.remove(player); if (injector != null) { - DataInputStream input = injector.getInputStream(true); InetSocketAddress address = player.getAddress(); injector.cleanupAll(); @@ -423,8 +441,7 @@ public class PlayerInjectionHandler { // Clean up if (removeAuxiliary) { - if (input != null) - dataInputLookup.remove(input); + // Note that the dataInputLookup will clean itself if (address != null) addressLookup.remove(address); } @@ -571,7 +588,6 @@ public class PlayerInjectionHandler { } public void close() { - // Guard if (hasClosed || playerInjection == null) return; @@ -593,7 +609,6 @@ public class PlayerInjectionHandler { hasClosed = true; playerInjection.clear(); - dataInputLookup.clear(); addressLookup.clear(); invoker = null; } @@ -607,12 +622,9 @@ public class PlayerInjectionHandler { // Update the DataInputStream if (injector != null) { - final DataInputStream old = injector.getInputStream(true); - injector.scheduleAction(new Runnable() { @Override public void run() { - dataInputLookup.remove(old); dataInputLookup.put(injector.getInputStream(false), injector); } }); diff --git a/ProtocolLib/src/main/resources/plugin.yml b/ProtocolLib/src/main/resources/plugin.yml index d632e814..d96bf284 100644 --- a/ProtocolLib/src/main/resources/plugin.yml +++ b/ProtocolLib/src/main/resources/plugin.yml @@ -1,5 +1,5 @@ name: ProtocolLib -version: 1.7.0 +version: 1.7.1-SNAPSHOT description: Provides read/write access to the Minecraft protocol. author: Comphenix website: http://www.comphenix.net/ProtocolLib diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/concurrency/BlockingHashMapTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/concurrency/BlockingHashMapTest.java new file mode 100644 index 00000000..7be7e705 --- /dev/null +++ b/ProtocolLib/src/test/java/com/comphenix/protocol/concurrency/BlockingHashMapTest.java @@ -0,0 +1,42 @@ +package com.comphenix.protocol.concurrency; + +import static org.junit.Assert.*; + +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import org.junit.Test; + +public class BlockingHashMapTest { + + @Test + public void test() throws InterruptedException, ExecutionException { + + final BlockingHashMap map = BlockingHashMap.create(); + + ExecutorService service = Executors.newSingleThreadExecutor(); + + // Create a reader + Future future = service.submit(new Callable() { + @Override + public String call() throws Exception { + // Combine for easy reading + return map.get(0) + map.get(1); + } + }); + + // Wait a bit + Thread.sleep(50); + + // Insert values + map.put(0, "hello "); + map.put(1, "world"); + + // Wait for the other thread to complete + assertEquals(future.get(), "hello world"); + } + +} From c49b1ddc0278efa2978bd7c23e8c63b28dd0ec65 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 23 Nov 2012 08:48:32 +0100 Subject: [PATCH 02/13] Incremented to 1.7.1-SNAPSHOT for development to the next version. --- ProtocolLib/dependency-reduced-pom.xml | 2 +- ProtocolLib/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ProtocolLib/dependency-reduced-pom.xml b/ProtocolLib/dependency-reduced-pom.xml index a83cd055..e2e722a0 100644 --- a/ProtocolLib/dependency-reduced-pom.xml +++ b/ProtocolLib/dependency-reduced-pom.xml @@ -4,7 +4,7 @@ com.comphenix.protocol ProtocolLib ProtocolLib - 1.7.0 + 1.7.1-SNAPSHOT Provides read/write access to the Minecraft protocol. http://dev.bukkit.org/server-mods/protocollib/ diff --git a/ProtocolLib/pom.xml b/ProtocolLib/pom.xml index 5897f7c6..0c3e3732 100644 --- a/ProtocolLib/pom.xml +++ b/ProtocolLib/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.comphenix.protocol ProtocolLib - 1.7.0 + 1.7.1-SNAPSHOT jar Provides read/write access to the Minecraft protocol. From d53b6dbee189f45674ea020f05587cab9b23e493 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 23 Nov 2012 08:52:02 +0100 Subject: [PATCH 03/13] Removed the ReadWriteLock as it's superseeded by the BlockingHashMap. --- .../injector/player/NetLoginInjector.java | 18 ------------------ .../player/PlayerInjectionHandler.java | 2 +- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetLoginInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetLoginInjector.java index ac1c16d6..64e2f005 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetLoginInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetLoginInjector.java @@ -1,9 +1,6 @@ package com.comphenix.protocol.injector.player; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; import org.bukkit.Server; import org.bukkit.entity.Player; @@ -29,8 +26,6 @@ class NetLoginInjector { // The current error rerporter private ErrorReporter reporter; - private ReadWriteLock injectionLock = new ReentrantReadWriteLock(); - // Used to create fake players private TemporaryPlayerFactory tempPlayerFactory = new TemporaryPlayerFactory(); @@ -46,9 +41,6 @@ class NetLoginInjector { * @return An injected NetLoginHandler, or the original object. */ public Object onNetLoginCreated(Object inserting) { - - injectionLock.writeLock().lock(); - try { // Make sure we actually need to inject during this phase if (!injectionHandler.isInjectionNecessary(GamePhase.LOGIN)) @@ -73,19 +65,9 @@ class NetLoginInjector { reporter.reportDetailed(this, "Unable to hook NetLoginHandler.", e, inserting); return inserting; - } finally { - injectionLock.writeLock().unlock(); } } - /** - * Retrieve the lock used for reading. - * @return Reading lock. - */ - public Lock getReadLock() { - return injectionLock.readLock(); - } - /** * Invoked when a NetLoginHandler should be reverted. * @param inserting - the original NetLoginHandler. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java index 7d4a6a4a..c5f832b9 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -216,7 +216,7 @@ public class PlayerInjectionHandler { * @throws InterruptedException If the thread was interrupted during the wait. */ public Player getPlayerByConnection(DataInputStream inputStream, long playerTimeout, TimeUnit unit) throws InterruptedException { - + // Wait until the connection owner has been established PlayerInjector injector = dataInputLookup.get(inputStream, playerTimeout, unit); if (injector != null) { From 03ad9be07887f1d1919576c7c2caa36adf6fb5e6 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 23 Nov 2012 11:42:17 +0100 Subject: [PATCH 04/13] Be careful when registering commands. It shouldn't break the plugin. Found an error report in the wild suggesting that getCommand() might occationally fail (if plugin.yml isn't loaded properly, maybe) and return NULL instead. Since the commands are only for administrators and plugin authors, we'll simply ignore it if this occurs. --- .../comphenix/protocol/ProtocolLibrary.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java index ffe6b34d..e3f8e09d 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java @@ -23,6 +23,8 @@ import java.util.logging.LogRecord; import java.util.logging.Logger; import org.bukkit.Server; +import org.bukkit.command.CommandExecutor; +import org.bukkit.command.PluginCommand; import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; @@ -187,8 +189,8 @@ public class ProtocolLibrary extends JavaPlugin { } // Set up command handlers - getCommand(CommandProtocol.NAME).setExecutor(commandProtocol); - getCommand(CommandPacket.NAME).setExecutor(commandPacket); + registerCommand(CommandProtocol.NAME, commandProtocol); + registerCommand(CommandPacket.NAME, commandPacket); // Notify server managers of incompatible plugins checkForIncompatibility(manager); @@ -217,6 +219,24 @@ public class ProtocolLibrary extends JavaPlugin { reporter.reportDetailed(this, "Metrics cannot be enabled. Incompatible Bukkit version.", e, statistisc); } } + + private void registerCommand(String name, CommandExecutor executor) { + try { + if (executor == null) + throw new RuntimeException("Executor was NULL."); + + PluginCommand command = getCommand(name); + + // Try to load the command + if (command != null) + command.setExecutor(executor); + else + throw new RuntimeException("plugin.yml might be corrupt."); + + } catch (RuntimeException e) { + reporter.reportWarning(this, "Cannot register command " + name + ": " + e.getMessage()); + } + } /** * Disable the current plugin. From beb4bba7fabb414c7024a24d4d419f4db8976860 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 27 Nov 2012 08:00:52 +0100 Subject: [PATCH 05/13] Switching away from checked exceptions. --- .../com/comphenix/protocol/reflect/FieldAccessException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/FieldAccessException.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/FieldAccessException.java index 1d1c13c2..3abec315 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/FieldAccessException.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/FieldAccessException.java @@ -22,7 +22,7 @@ package com.comphenix.protocol.reflect; * * @author Kristian */ -public class FieldAccessException extends Exception { +public class FieldAccessException extends RuntimeException { /** * Generated by Eclipse. From f81e3262d0147126f0a4a9c71667023b5e974f06 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sat, 1 Dec 2012 17:24:46 +0100 Subject: [PATCH 06/13] Don't spam the console when a player logs out quickly. --- .../java/com/comphenix/protocol/injector/PacketInjector.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java index 637ec025..8d3374dc 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketInjector.java @@ -212,7 +212,8 @@ class PacketInjector { return null; } catch (InterruptedException e) { - reporter.reportDetailed(this, "Thread was interrupted.", e, packet, input); + // We will ignore this - it occurs when a player disconnects + //reporter.reportDetailed(this, "Thread was interrupted.", e, packet, input); return null; } } From 2bd06922e055b3b11ec2af6797cadfd04589e809 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sat, 1 Dec 2012 22:34:46 +0100 Subject: [PATCH 07/13] Ensure that the wrapped watchable object returns wrapped objects. In addition, made it possible to construct watchable objects directly. Also fixed a bug preventing data watchers from creating new watchable objects. --- .../protocol/wrappers/WrappedDataWatcher.java | 69 ++++-------- .../wrappers/WrappedWatchableObject.java | 102 +++++++++++++++++- 2 files changed, 120 insertions(+), 51 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java index 5f39621e..fd2087ba 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java @@ -1,6 +1,7 @@ package com.comphenix.protocol.wrappers; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; @@ -116,8 +117,7 @@ public class WrappedDataWatcher { */ public static Integer getTypeID(Class clazz) throws FieldAccessException { initialize(); - - return typeMap.get(clazz); + return typeMap.get(WrappedWatchableObject.getUnwrappedType(clazz)); } /** @@ -145,7 +145,7 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public Byte getByte(int index) throws FieldAccessException { - return (Byte) getObjectRaw(index); + return (Byte) getObject(index); } /** @@ -155,7 +155,7 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public Short getShort(int index) throws FieldAccessException { - return (Short) getObjectRaw(index); + return (Short) getObject(index); } /** @@ -165,7 +165,7 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public Integer getInteger(int index) throws FieldAccessException { - return (Integer) getObjectRaw(index); + return (Integer) getObject(index); } /** @@ -175,7 +175,7 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public Float getFloat(int index) throws FieldAccessException { - return (Float) getObjectRaw(index); + return (Float) getObject(index); } /** @@ -185,7 +185,7 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public String getString(int index) throws FieldAccessException { - return (String) getObjectRaw(index); + return (String) getObject(index); } /** @@ -215,25 +215,6 @@ public class WrappedDataWatcher { * @throws FieldAccessException Cannot read underlying field. */ public Object getObject(int index) throws FieldAccessException { - Object result = getObjectRaw(index); - - // Handle the special cases too - if (result instanceof net.minecraft.server.ItemStack) { - return BukkitConverters.getItemStackConverter().getSpecific(result); - } else if (result instanceof ChunkCoordinates) { - return new WrappedChunkCoordinate((ChunkCoordinates) result); - } else { - return result; - } - } - - /** - * Retrieve a watchable object by index. - * @param index - index of the object to retrieve. - * @return The watched object. - * @throws FieldAccessException Cannot read underlying field. - */ - private Object getObjectRaw(int index) throws FieldAccessException { // The get method will take care of concurrency WatchableObject watchable = getWatchedObject(index); @@ -319,27 +300,7 @@ public class WrappedDataWatcher { * @param update - whether or not to refresh every listening clients. * @throws FieldAccessException Cannot read underlying field. */ - public void setObject(int index, Object newValue, boolean update) throws FieldAccessException { - // Convert special cases - if (newValue instanceof WrappedChunkCoordinate) - newValue = ((WrappedChunkCoordinate) newValue).getHandle(); - if (newValue instanceof ItemStack) - newValue = BukkitConverters.getItemStackConverter().getGeneric( - net.minecraft.server.ItemStack.class, (ItemStack) newValue); - - // Next, set the object - setObjectRaw(index, newValue, update); - } - - /** - * Set a watchable object by index. - * @param index - index of the object to retrieve. - * @param newValue - the new watched value. - * @param update - whether or not to refresh every listening clients. - * @return The watched object. - * @throws FieldAccessException Cannot read underlying field. - */ - private void setObjectRaw(int index, Object newValue, boolean update) throws FieldAccessException { + public void setObject(int index, Object newValue, boolean update) throws FieldAccessException { // Aquire write lock Lock writeLock = getReadWriteLock().writeLock(); writeLock.lock(); @@ -349,12 +310,22 @@ public class WrappedDataWatcher { if (watchable != null) { new WrappedWatchableObject(watchable).setValue(newValue, update); + } else { + createKeyValueMethod.invoke(handle, index, WrappedWatchableObject.getUnwrapped(newValue)); } - } finally { + + // Handle invoking the method + } catch (IllegalArgumentException e) { + throw new FieldAccessException("Cannot convert arguments.", e); + } catch (IllegalAccessException e) { + throw new FieldAccessException("Illegal access.", e); + } catch (InvocationTargetException e) { + throw new FieldAccessException("Checked exception in Minecraft.", e); + } finally { writeLock.unlock(); } } - + private WatchableObject getWatchedObject(int index) throws FieldAccessException { // We use the get-method first and foremost if (getKeyValueMethod != null) { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedWatchableObject.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedWatchableObject.java index d9862029..2124d2a3 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedWatchableObject.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedWatchableObject.java @@ -5,6 +5,7 @@ import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.instances.DefaultInstances; +import net.minecraft.server.ChunkCoordinates; import net.minecraft.server.ItemStack; import net.minecraft.server.WatchableObject; @@ -27,7 +28,35 @@ public class WrappedWatchableObject { // Type of the stored value private Class typeClass; + /** + * Wrap a given raw Minecraft watchable object. + * @param handle - the raw watchable object to wrap. + */ public WrappedWatchableObject(WatchableObject handle) { + load(handle); + } + + /** + * Construct a watchable object from an index and a given value. + * @param index - the index. + * @param value - non-null value of specific types. + */ + public WrappedWatchableObject(int index, Object value) { + if (value == null) + throw new IllegalArgumentException("Value cannot be NULL."); + + // Get the correct type ID + Integer typeID = WrappedDataWatcher.getTypeID(value.getClass()); + + if (typeID != null) { + load(new WatchableObject(typeID, index, getUnwrapped(value))); + } else { + throw new IllegalArgumentException("Cannot watch the type " + value.getClass()); + } + } + + // Wrap a NMS object + private void load(WatchableObject handle) { initialize(); this.handle = handle; this.modifier = baseModifier.withTarget(handle); @@ -57,6 +86,15 @@ public class WrappedWatchableObject { * @throws FieldAccessException Unable to read values. */ public Class getType() throws FieldAccessException { + return getWrappedType(getTypeRaw()); + } + + /** + * Retrieve the correct super type of the current value, given the raw NMS object. + * @return Super type. + * @throws FieldAccessException Unable to read values. + */ + private Class getTypeRaw() throws FieldAccessException { if (typeClass == null) { typeClass = WrappedDataWatcher.getTypeClass(getTypeID()); @@ -131,7 +169,7 @@ public class WrappedWatchableObject { setDirtyState(true); // Use the modifier to set the value - modifier.withType(Object.class).write(0, newValue); + modifier.withType(Object.class).write(0, getUnwrapped(newValue)); } /** @@ -140,7 +178,7 @@ public class WrappedWatchableObject { * @throws FieldAccessException Unable to use reflection. */ public Object getValue() throws FieldAccessException { - return modifier.withType(Object.class).read(0); + return getWrapped(modifier.withType(Object.class).read(0)); } /** @@ -161,6 +199,66 @@ public class WrappedWatchableObject { return modifier.withType(boolean.class).read(0); } + /** + * Retrieve the wrapped object value, if needed. + * @param value - the raw NMS object to wrap. + * @return The wrapped object. + */ + static Object getWrapped(Object value) { + // Handle the special cases + if (value instanceof net.minecraft.server.ItemStack) { + return BukkitConverters.getItemStackConverter().getSpecific(value); + } else if (value instanceof ChunkCoordinates) { + return new WrappedChunkCoordinate((ChunkCoordinates) value); + } else { + return value; + } + } + + /** + * Retrieve the wrapped type, if needed. + * @param wrapped - the wrapped class type. + * @return The wrapped class type. + */ + static Class getWrappedType(Class unwrapped) { + if (unwrapped.equals(net.minecraft.server.ChunkPosition.class)) + return ChunkPosition.class; + else if (unwrapped.equals(ChunkCoordinates.class)) + return WrappedChunkCoordinate.class; + else + return unwrapped; + } + + /** + * Retrieve the raw NMS value. + * @param wrapped - the wrapped position. + * @return The raw NMS object. + */ + static Object getUnwrapped(Object wrapped) { + // Convert special cases + if (wrapped instanceof WrappedChunkCoordinate) + return ((WrappedChunkCoordinate) wrapped).getHandle(); + else if (wrapped instanceof ItemStack) + return BukkitConverters.getItemStackConverter().getGeneric( + net.minecraft.server.ItemStack.class, (org.bukkit.inventory.ItemStack) wrapped); + else + return wrapped; + } + + /** + * Retrieve the unwrapped type, if needed. + * @param wrapped - the unwrapped class type. + * @return The unwrapped class type. + */ + static Class getUnwrappedType(Class wrapped) { + if (wrapped.equals(ChunkPosition.class)) + return net.minecraft.server.ChunkPosition.class; + else if (wrapped.equals(WrappedChunkCoordinate.class)) + return ChunkCoordinates.class; + else + return wrapped; + } + /** * Clone the current wrapped watchable object, along with any contained objects. * @return A deep clone of the current watchable object. From f4accbfe2d1a38cb9c6b64a0a81ad694d54cb8f5 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sun, 2 Dec 2012 02:49:11 +0100 Subject: [PATCH 08/13] Make it possible to get every player that is tracking a given entity. --- .../comphenix/protocol/ProtocolManager.java | 10 +++++- .../protocol/injector/EntityUtilities.java | 36 ++++++++++++++++++- .../injector/PacketFilterManager.java | 6 ++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolManager.java index 3e319249..741c54bc 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolManager.java @@ -130,7 +130,7 @@ public interface ProtocolManager extends PacketStream { public PacketConstructor createPacketConstructor(int id, Object... arguments); /** - * Completely refresh all clients about an entity. + * Completely resend an entity to a list of clients. *

* Note that this method is NOT thread safe. If you call this method from anything * but the main thread, it will throw an exception. @@ -148,6 +148,14 @@ public interface ProtocolManager extends PacketStream { */ public Entity getEntityFromID(World container, int id) throws FieldAccessException; + /** + * Retrieve every client that is receiving information about a given entity. + * @param entity - the entity that is being tracked. + * @return Every client/player that is tracking the given entity. + * @throws FieldAccessException If reflection failed. + */ + public List getEntityTrackers(Entity entity) throws FieldAccessException; + /** * Retrieves a immutable set containing the ID of the sent server packets that will be observed by listeners. * @return Every filtered server packet. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java index 9671c6ce..ef9c9e8c 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java @@ -21,12 +21,14 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; +import net.minecraft.server.EntityPlayer; import net.minecraft.server.EntityTrackerEntry; import org.bukkit.World; @@ -118,7 +120,39 @@ class EntityUtilities { } catch (SecurityException e) { throw new FieldAccessException("Security limitation prevents access to 'scanPlayers' method in trackerEntry.", e); } catch (NoSuchMethodException e) { - throw new FieldAccessException("Canot find 'scanPlayers' method. Is ProtocolLib up to date?", e); + throw new FieldAccessException("Cannot find 'scanPlayers' method. Is ProtocolLib up to date?", e); + } + } + + /** + * Retrieve every client that is receiving information about a given entity. + * @param entity - the entity that is being tracked. + * @return Every client/player that is tracking the given entity. + * @throws FieldAccessException If reflection failed. + */ + public static List getEntityTrackers(Entity entity) { + try { + List result = new ArrayList(); + Object trackerEntry = getEntityTrackerEntry(entity.getWorld(), entity.getEntityId()); + + if (trackedPlayersField == null) + trackedPlayersField = FuzzyReflection.fromObject(trackerEntry).getFieldByType("java\\.util\\..*"); + + Collection trackedPlayers = (Collection) FieldUtils.readField(trackedPlayersField, trackerEntry, false); + + // Wrap every player - we also ensure that the underlying tracker list is immutable + for (Object tracker : trackedPlayers) { + if (tracker instanceof EntityPlayer) { + EntityPlayer nmsPlayer = (EntityPlayer) tracker; + result.add(nmsPlayer.getBukkitEntity()); + } + } + return result; + + } catch (IllegalAccessException e) { + throw new FieldAccessException("Security limitation prevented access to the list of tracked players.", e); + } catch (InvocationTargetException e) { + throw new FieldAccessException("Exception occurred in Minecraft.", e); } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index a8706e90..663b4c09 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -55,6 +55,7 @@ import com.comphenix.protocol.events.*; import com.comphenix.protocol.injector.player.PlayerInjectionHandler; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.wrappers.BukkitConverters; import com.google.common.base.Objects; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; @@ -571,6 +572,11 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok return EntityUtilities.getEntityFromID(container, id); } + @Override + public List getEntityTrackers(Entity entity) throws FieldAccessException { + return EntityUtilities.getEntityTrackers(entity); + } + /** * Initialize the packet injection for every player. * @param players - list of players to inject. From 300111b8503d3eb253923247e938eef02b2c0212 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Mon, 3 Dec 2012 17:51:18 +0100 Subject: [PATCH 09/13] Make it easy to clone packets. --- .../protocol/events/PacketContainer.java | 39 +++++++++++++++++++ .../injector/PacketFilterManager.java | 1 - 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java index 4150acba..fc52b2bd 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java @@ -17,6 +17,8 @@ package com.comphenix.protocol.events; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; @@ -41,6 +43,7 @@ import com.comphenix.protocol.wrappers.BukkitConverters; import com.comphenix.protocol.wrappers.ChunkPosition; import com.comphenix.protocol.wrappers.WrappedDataWatcher; import com.comphenix.protocol.wrappers.WrappedWatchableObject; +import com.google.common.io.Closeables; import net.minecraft.server.Packet; @@ -347,6 +350,42 @@ public class PacketContainer implements Serializable { return id; } + /** + * Create a deep copy of the current packet. + * @return A deep copy of the current packet. + */ + public PacketContainer deepClone() { + ObjectOutputStream output = null; + ObjectInputStream input = null; + + try { + // Use a small buffer of 32 bytes initially. + ByteArrayOutputStream bufferOut = new ByteArrayOutputStream(); + output = new ObjectOutputStream(bufferOut); + output.writeObject(this); + + ByteArrayInputStream bufferIn = new ByteArrayInputStream(bufferOut.toByteArray()); + input = new ObjectInputStream(bufferIn); + return (PacketContainer) input.readObject(); + + } catch (IOException e) { + throw new IllegalStateException("Unexpected error occured during object cloning.", e); + } catch (ClassNotFoundException e) { + // Cannot happen + throw new IllegalStateException("Unexpected failure with serialization.", e); + } finally { + try { + if (output != null) + output.close(); + if (input != null) + input.close(); + + } catch (IOException e) { + // STOP IT + } + } + } + private void writeObject(ObjectOutputStream output) throws IOException { // Default serialization output.defaultWriteObject(); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index 663b4c09..27f7c7c0 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -55,7 +55,6 @@ import com.comphenix.protocol.events.*; import com.comphenix.protocol.injector.player.PlayerInjectionHandler; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; -import com.comphenix.protocol.wrappers.BukkitConverters; import com.google.common.base.Objects; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; From 98a937738422a8decf8b757f16a58668617fb6d0 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 4 Dec 2012 00:18:33 +0100 Subject: [PATCH 10/13] Add a convenience retriever for integer array fields. --- .../com/comphenix/protocol/events/PacketContainer.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java index fc52b2bd..2ff67143 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketContainer.java @@ -43,7 +43,6 @@ import com.comphenix.protocol.wrappers.BukkitConverters; import com.comphenix.protocol.wrappers.ChunkPosition; import com.comphenix.protocol.wrappers.WrappedDataWatcher; import com.comphenix.protocol.wrappers.WrappedWatchableObject; -import com.google.common.io.Closeables; import net.minecraft.server.Packet; @@ -196,6 +195,14 @@ public class PacketContainer implements Serializable { public StructureModifier getByteArrays() { return structureModifier.withType(byte[].class); } + + /** + * Retrieves a read/write structure for every int array field. + * @return A modifier for every int array field. + */ + public StructureModifier getIntegerArrays() { + return structureModifier.withType(int[].class); + } /** * Retrieves a read/write structure for ItemStack. From b9abf0b683606c2e6c932c851edecbf435ba68b2 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 4 Dec 2012 00:19:55 +0100 Subject: [PATCH 11/13] Incrementing to 1.7.1 --- ProtocolLib/pom.xml | 2 +- .../com/comphenix/protocol/wrappers/WrappedDataWatcher.java | 1 - ProtocolLib/src/main/resources/plugin.yml | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ProtocolLib/pom.xml b/ProtocolLib/pom.xml index 0c3e3732..1b769aac 100644 --- a/ProtocolLib/pom.xml +++ b/ProtocolLib/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.comphenix.protocol ProtocolLib - 1.7.1-SNAPSHOT + 1.7.1 jar Provides read/write access to the Minecraft protocol. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java index fd2087ba..0342e47c 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java @@ -22,7 +22,6 @@ import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.google.common.base.Objects; -import net.minecraft.server.ChunkCoordinates; import net.minecraft.server.DataWatcher; import net.minecraft.server.WatchableObject; diff --git a/ProtocolLib/src/main/resources/plugin.yml b/ProtocolLib/src/main/resources/plugin.yml index d96bf284..69f13146 100644 --- a/ProtocolLib/src/main/resources/plugin.yml +++ b/ProtocolLib/src/main/resources/plugin.yml @@ -1,5 +1,5 @@ name: ProtocolLib -version: 1.7.1-SNAPSHOT +version: 1.7.1 description: Provides read/write access to the Minecraft protocol. author: Comphenix website: http://www.comphenix.net/ProtocolLib From 7062b9327cea485e510f8ff8efe25e19bc75890d Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 4 Dec 2012 00:21:14 +0100 Subject: [PATCH 12/13] Fix documentation. --- .../comphenix/protocol/concurrency/AbstractIntervalTree.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/AbstractIntervalTree.java b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/AbstractIntervalTree.java index 571083d5..9b241e90 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/AbstractIntervalTree.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/AbstractIntervalTree.java @@ -132,7 +132,7 @@ public abstract class AbstractIntervalTree, TValue * Removes every interval that intersects with the given range. * @param lowerBound - lowest value to remove. * @param upperBound - highest value to remove. - * @param preserveOutside - whether or not to preserve the intervals that are partially outside. + * @param preserveDifference - whether or not to preserve the intervals that are partially outside. */ public Set remove(TKey lowerBound, TKey upperBound, boolean preserveDifference) { checkBounds(lowerBound, upperBound); From 98edcb177293cf299010fc8b2d133a396f3cf9bb Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 4 Dec 2012 00:54:25 +0100 Subject: [PATCH 13/13] Maven crap again. --- ProtocolLib/dependency-reduced-pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/dependency-reduced-pom.xml b/ProtocolLib/dependency-reduced-pom.xml index e2e722a0..8bf4ad99 100644 --- a/ProtocolLib/dependency-reduced-pom.xml +++ b/ProtocolLib/dependency-reduced-pom.xml @@ -4,7 +4,7 @@ com.comphenix.protocol ProtocolLib ProtocolLib - 1.7.1-SNAPSHOT + 1.7.1 Provides read/write access to the Minecraft protocol. http://dev.bukkit.org/server-mods/protocollib/