Added a priorty to every listener. Improved performance.

ProtocolLib/src/com/comphenix/protocol/events/ListenerPriority.java

@@ -0,0 +1,51 @@
+package com.comphenix.protocol.events;
+
+/**
+ * Represents a packet event priority, similar to the Bukkit EventPriority.
+ * 
+ * @author Kristian
+ */
+public enum ListenerPriority {
+	/**
+	 * Event call is of very low importance and should be ran first, to allow
+	 * other plugins to further customise the outcome.
+	 */
+	LOWEST(0),
+	/**
+	 * Event call is of low importance.
+	 */
+	LOW(1),
+	/**
+	 * Event call is neither important or unimportant, and may be ran normally.
+	 */
+	NORMAL(2),
+	/**
+	 * Event call is of high importance.
+	 */
+	HIGH(3),
+	/**
+	 * Event call is critical and must have the final say in what happens to the
+	 * event.
+	 */
+	HIGHEST(4),
+	/**
+	 * Event is listened to purely for monitoring the outcome of an event.
+	 * <p/>
+	 * No modifications to the event should be made under this priority.
+	 */
+	MONITOR(5);
+
+	private final int slot;
+
+	private ListenerPriority(int slot) {
+		this.slot = slot;
+	}
+
+	/**
+	 * A low slot represents a low priority.
+	 * @return Integer representation of this priorty.
+	 */
+	public int getSlot() {
+		return slot;
+	}
+}

ProtocolLib/src/com/comphenix/protocol/events/PacketAdapter.java

@@ -34,6 +34,7 @@ public abstract class PacketAdapter implements PacketListener {
 	protected Plugin plugin;
 	protected Set<Integer> packetsID;
 	protected ConnectionSide connectionSide;
+	protected ListenerPriority listenerPriority;
 	
 	/**
 	 * Initialize a packet listener.
@@ -42,9 +43,21 @@ public abstract class PacketAdapter implements PacketListener {
 	 * @param packets - the packet IDs the listener is looking for.
 	 */
 	public PacketAdapter(Plugin plugin, ConnectionSide connectionSide, Integer... packets) {
+		this(plugin, connectionSide, ListenerPriority.NORMAL, packets);
+	}
+	
+	/**
+	 * Initialize a packet listener.
+	 * @param plugin - the plugin that spawned this listener.
+	 * @param connectionSide - the packet type the listener is looking for.
+	 * @param listenerPriority - the event priority.
+	 * @param packets - the packet IDs the listener is looking for.
+	 */
+	public PacketAdapter(Plugin plugin, ConnectionSide connectionSide, ListenerPriority listenerPriority, Integer... packets) {
 		this.plugin = plugin;
 		this.connectionSide = connectionSide;
 		this.packetsID = Sets.newHashSet(packets);
+		this.listenerPriority = listenerPriority;
 	}
 	
 	@Override
@@ -72,6 +85,11 @@ public abstract class PacketAdapter implements PacketListener {
 		return plugin;
 	}
 	
+	@Override
+	public ListenerPriority getListenerPriority() {
+		return listenerPriority;
+	}
+	
 	/**
 	 * Retrieves the name of the plugin that has been associated with the listener.
 	 * @return Name of the associated plugin.

ProtocolLib/src/com/comphenix/protocol/events/PacketListener.java

@@ -48,6 +48,12 @@ public interface PacketListener {
 	 */
 	public ConnectionSide getConnectionSide();
 	
+	/**
+	 * Retrieve the priority in the execution order of this packet listener. Highest priority will be executed last.
+	 * @return Execution order in terms of priority.
+	 */
+	public ListenerPriority getListenerPriority();
+	
 	/**
 	 * Set of packet ids we expect to recieve.
 	 * @return Packets IDs.

ProtocolLib/src/com/comphenix/protocol/injector/ConcurrentListenerMultimap.java

@@ -0,0 +1,209 @@
+package com.comphenix.protocol.injector;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import com.comphenix.protocol.events.PacketAdapter;
+import com.comphenix.protocol.events.PacketEvent;
+import com.comphenix.protocol.events.PacketListener;
+import com.google.common.base.Objects;
+import com.google.common.primitives.Ints;
+
+/**
+ * A thread-safe implementation of a listener multimap.
+ * 
+ * @author Kristian
+ */
+public class ConcurrentListenerMultimap {
+
+	// The core of our map
+	protected ConcurrentMap<Integer, SortedArrayList<PacketListener>> listeners = 
+		  new ConcurrentHashMap<Integer, SortedArrayList<PacketListener>>();
+	
+	/**
+	 * Adds a listener to its requested list of packet recievers.
+	 * @param listener - listener with a list of packets to recieve notifcations for.
+	 */
+	public void addListener(PacketListener listener) {
+		for (Integer packetID : listener.getPacketsID()) {
+			addListener(packetID, listener);
+		}
+	}
+	
+	// Add the listener to a specific packet notifcation list
+	private void addListener(Integer packetID, PacketListener listener) {
+		
+		SortedArrayList<PacketListener> list = listeners.get(packetID);
+		
+		// We don't want to create this for every lookup
+		if (list == null) {
+			// It would be nice if we could use a PriorityBlockingQueue, but it doesn't preseve iterator order,
+			// which is a essential feature for our purposes.
+			final SortedArrayList<PacketListener> value = new SortedArrayList<PacketListener>(new Comparator<PacketListener>() {
+				@Override
+				public int compare(PacketListener o1, PacketListener o2) {
+					// This ensures that lower priority listeners are executed first
+					return Ints.compare(o1.getListenerPriority().getSlot(),
+							            o2.getListenerPriority().getSlot());
+				}
+			});
+			
+			list = listeners.putIfAbsent(packetID, value);
+			
+			// We may end up creating multiple multisets, but we'll agree 
+			// on the one to use.
+			if (list == null) {
+				list = value;
+			}
+		}
+		
+		// Careful when modifying the set
+		synchronized(list) {
+			list.insertSorted(listener);
+		}
+	}
+	
+	/**
+	 * Removes the given listener from the packet event list.
+	 * @param listener - listener to remove.
+	 * @return Every packet ID that was removed due to no listeners.
+	 */
+	public List<Integer> removeListener(PacketListener listener) {
+	
+		List<Integer> removedPackets = new ArrayList<Integer>();
+		
+		// Again, not terribly efficient. But adding or removing listeners should be a rare event.
+		for (Integer packetID : listener.getPacketsID()) {
+			
+			SortedArrayList<PacketListener> list = listeners.get(packetID);
+			
+			// Remove any listeners
+			if (list != null) {
+				synchronized(list) {
+					// Don't remove from newly created lists
+					if (list.size() > 0) {
+						list.removeAll(listener);
+						
+						if (list.size() == 0) {
+							listeners.remove(packetID);
+							removedPackets.add(packetID);
+						}
+					}
+				}
+			}
+			
+			// Move on to the next
+		}
+		
+		return removedPackets;
+	}
+	
+	/**
+	 * Invokes the given packet event for every registered listener.
+	 * @param logger - the logger that will be used to inform about listener exceptions.
+	 * @param event - the packet event to invoke.
+	 */
+	public void invokePacketRecieving(Logger logger, PacketEvent event) {
+		SortedArrayList<PacketListener> list = listeners.get(event.getPacketID());
+		
+		if (list == null)
+			return;
+		
+		// We have to be careful. Cannot modify the underlying list when sending notifications.
+		synchronized (list) {
+			for (PacketListener listener : list) {
+				try {
+					listener.onPacketReceiving(event);
+				} catch (Exception e) {
+					// Minecraft doesn't want your Exception.
+					logger.log(Level.SEVERE, 
+							"Exception occured in onPacketReceiving() for " + PacketAdapter.getPluginName(listener), e);
+				}
+			}
+		}
+	}
+	
+	/**
+	 * Invokes the given packet event for every registered listener.
+	 * @param logger - the logger that will be used to inform about listener exceptions.
+	 * @param event - the packet event to invoke.
+	 */
+	public void invokePacketSending(Logger logger, PacketEvent event) {
+		SortedArrayList<PacketListener> list = listeners.get(event.getPacketID());
+		
+		if (list == null)
+			return;
+		
+		synchronized (list) {
+			for (PacketListener listener : list) {
+				try {
+					listener.onPacketSending(event);
+				} catch (Exception e) {
+					// Minecraft doesn't want your Exception.
+					logger.log(Level.SEVERE, 
+							"Exception occured in onPacketReceiving() for " + PacketAdapter.getPluginName(listener), e);
+				}
+			}
+		}	
+	}
+	
+	/**
+	 * An implicitly sorted array list that preserves insertion order and maintains duplicates.
+	 * 
+	 * Note that only the {@link insertSorted} method will update the list correctly,
+	 * @param <T> - type of the sorted list.
+	 */
+	private class SortedArrayList<T> implements Iterable<T> {
+
+		private Comparator<T> comparator;
+		private List<T> list = new ArrayList<T>();
+		
+		public SortedArrayList(Comparator<T> comparator) {
+			this.comparator = comparator;
+		}
+		
+		/**
+		 * Inserts the given element in the proper location.
+		 * @param value - element to insert.
+		 */
+	    public void insertSorted(T value) {
+	    	list.add(value);
+	        for (int i = list.size() - 1; i > 0 && comparator.compare(value, list.get(i-1)) < 0; i--) {
+	            T tmp = list.get(i);
+	            list.set(i, list.get(i-1));
+	            list.set(i-1, tmp);
+	        }
+	    }
+	    
+	    /**
+	     * Removes every instance of the given element.
+	     * @param element - element to remove.
+	     */
+	    public void removeAll(T element) {
+			for (Iterator<T> it = list.iterator(); it.hasNext(); ) {
+				if (Objects.equal(it.next(), element)) {
+					it.remove();
+				}
+			}
+	    }
+	    
+	    /**
+	     * Retrieve the size of the list.
+	     * @return Size of the list.
+	     */
+	    public int size() {
+	    	return list.size();
+	    }
+
+		@Override
+		public Iterator<T> iterator() {
+			return list.iterator();
+		}
+	}
+}

ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java

@@ -20,10 +20,12 @@ package com.comphenix.protocol.injector;
 import java.io.DataInputStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -43,10 +45,8 @@ import org.bukkit.plugin.Plugin;
 import org.bukkit.plugin.PluginManager;
 
 import com.comphenix.protocol.ProtocolManager;
-import com.comphenix.protocol.events.ConnectionSide;
-import com.comphenix.protocol.events.PacketContainer;
-import com.comphenix.protocol.events.PacketEvent;
-import com.comphenix.protocol.events.PacketListener;
+import com.comphenix.protocol.events.*;
+import com.comphenix.protocol.reflect.FieldAccessException;
 import com.comphenix.protocol.reflect.FuzzyReflection;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
@@ -57,14 +57,18 @@ public final class PacketFilterManager implements ProtocolManager {
 	private Set<PacketListener> packetListeners = new CopyOnWriteArraySet<PacketListener>();
 	
 	// Player injection
-	private Map<DataInputStream, Player> connectionLookup = new HashMap<DataInputStream, Player>();
+	private Map<DataInputStream, Player> connectionLookup = new ConcurrentHashMap<DataInputStream, Player>();
 	private Map<Player, PlayerInjector> playerInjection = new HashMap<Player, PlayerInjector>();
 	
 	// Packet injection
 	private PacketInjector packetInjector;
 	
 	// Enabled packet filters
-	private Set<Integer> packetFilters = new HashSet<Integer>();
+	private Set<Integer> sendingFilters = Collections.newSetFromMap(new ConcurrentHashMap<Integer, Boolean>());
+	
+	// The two listener containers
+	private ConcurrentListenerMultimap recievedListeners = new ConcurrentListenerMultimap();
+	private ConcurrentListenerMultimap sendingListeners = new ConcurrentListenerMultimap();
 	
 	// Whether or not this class has been closed
 	private boolean hasClosed;
@@ -107,10 +111,18 @@ public final class PacketFilterManager implements ProtocolManager {
 	public void addPacketListener(PacketListener listener) {
 		if (listener == null)
 			throw new IllegalArgumentException("listener cannot be NULL.");
-		
+
+		ConnectionSide side = listener.getConnectionSide();
 		packetListeners.add(listener);
-		enablePacketFilters(listener.getConnectionSide(), 
-							listener.getPacketsID());
+		
+		// Add listeners
+		if (side.isForServer())
+			sendingListeners.addListener(listener);
+		if (side.isForClient())
+			recievedListeners.addListener(listener);
+		
+		// Inform our injected hooks
+		enablePacketFilters(side, listener.getPacketsID());
 	}
 	
 	@Override
@@ -118,9 +130,24 @@ public final class PacketFilterManager implements ProtocolManager {
 		if (listener == null)
 			throw new IllegalArgumentException("listener cannot be NULL");
 		
+		ConnectionSide side = listener.getConnectionSide();
+		List<Integer> sendingRemoved = null;
+		List<Integer> receivingRemoved = null;
+		
+		// Remove from the overal list of listeners
 		packetListeners.remove(listener);
-		disablePacketFilters(listener.getConnectionSide(),
-						 	 listener.getPacketsID());
+		
+		// Add listeners
+		if (side.isForServer())
+			sendingRemoved = sendingListeners.removeListener(listener);
+		if (side.isForClient())
+			receivingRemoved = recievedListeners.removeListener(listener);
+		
+		// Remove hooks, if needed
+		if (sendingRemoved != null && sendingRemoved.size() > 0)
+			disablePacketFilters(ConnectionSide.SERVER_SIDE, sendingRemoved);
+		if (receivingRemoved != null && receivingRemoved.size() > 0)
+			disablePacketFilters(ConnectionSide.CLIENT_SIDE, receivingRemoved);
 	}
 	
 	@Override
@@ -132,7 +159,7 @@ public final class PacketFilterManager implements ProtocolManager {
 
 			// Remove the listener
 			if (Objects.equal(listener.getPlugin(), plugin)) {
-				packetListeners.remove(listener);
+				removePacketListener(listener);
 			}
 		}
 	}
@@ -142,15 +169,7 @@ public final class PacketFilterManager implements ProtocolManager {
 	 * @param event - the packet event to invoke.
 	 */
 	public void invokePacketRecieving(PacketEvent event) {
-		for (PacketListener listener : packetListeners) {
-			try {
-				if (canHandlePacket(listener, event))
-					listener.onPacketReceiving(event);
-			} catch (Exception e) {
-				// Minecraft doesn't want your Exception.
-				logger.log(Level.SEVERE, "Exception occured in onPacketReceiving() for " + listener.toString(), e);
-			}
-		}	
+		recievedListeners.invokePacketRecieving(logger, event);
 	}
 	
 	/**
@@ -158,26 +177,7 @@ public final class PacketFilterManager implements ProtocolManager {
 	 * @param event - the packet event to invoke.
 	 */
 	public void invokePacketSending(PacketEvent event) {
-		for (PacketListener listener : packetListeners) {
-			try {
-				if (canHandlePacket(listener, event))
-					listener.onPacketSending(event);
-			} catch (Exception e) {
-				logger.log(Level.SEVERE, "Exception occured in onPacketReceiving() for " + listener.toString(), e);
-			}
-		}	
-	}
-	
-	private boolean canHandlePacket(PacketListener listener, PacketEvent event) {
-		// Make sure the listener is looking for this packet
-		if (!listener.getPacketsID().contains(event.getPacket().getID()))
-			return false;
-		
-		// And this type of packet
-		if (event.isServerPacket())
-			return listener.getConnectionSide().isForServer();
-		else
-			return listener.getConnectionSide().isForClient();
+		sendingListeners.invokePacketSending(logger, event);
 	}
 	
 	/**
@@ -188,13 +188,13 @@ public final class PacketFilterManager implements ProtocolManager {
 	 * @param side - which side the event will arrive from.
 	 * @param packets - the packet id(s).
 	 */
-	private void enablePacketFilters(ConnectionSide side, Set<Integer> packets) {
+	private void enablePacketFilters(ConnectionSide side, Iterable<Integer> packets) {
 		if (side == null)
 			throw new IllegalArgumentException("side cannot be NULL.");
 		
 		for (int packetID : packets) {
-			if (side.isForServer())
-				packetFilters.add(packetID);
+			if (side.isForServer()) 
+				sendingFilters.add(packetID);
 			if (side.isForClient() && packetInjector != null)
 				packetInjector.addPacketHandler(packetID);
 		}
@@ -205,13 +205,13 @@ public final class PacketFilterManager implements ProtocolManager {
 	 * @param packets - the packet id(s).
 	 * @param side - which side the event no longer should arrive from.
 	 */
-	private void disablePacketFilters(ConnectionSide side, Set<Integer> packets) {
+	private void disablePacketFilters(ConnectionSide side, Iterable<Integer> packets) {
 		if (side == null)
 			throw new IllegalArgumentException("side cannot be NULL.");
 		
 		for (int packetID : packets) {
 			if (side.isForServer())
-				packetFilters.remove(packetID);
+				sendingFilters.remove(packetID);
 			if (side.isForClient() && packetInjector != null) 
 				packetInjector.removePacketHandler(packetID);
 		}
@@ -268,7 +268,7 @@ public final class PacketFilterManager implements ProtocolManager {
 		if (!skipDefaults) {
 			try {
 				packet.getModifier().writeDefaults();
-			} catch (IllegalAccessException e) {
+			} catch (FieldAccessException e) {
 				throw new RuntimeException("Security exception.", e);
 			}
 		}
@@ -279,9 +279,9 @@ public final class PacketFilterManager implements ProtocolManager {
 	@Override
 	public Set<Integer> getPacketFilters() {
 		if (packetInjector != null)
-			return Sets.union(packetFilters, packetInjector.getPacketHandlers());
+			return Sets.union(sendingFilters, packetInjector.getPacketHandlers());
 		else
-			return packetFilters;
+			return sendingFilters;
 	}
 	
 	/**
@@ -297,7 +297,7 @@ public final class PacketFilterManager implements ProtocolManager {
 		// Don't inject if the class has closed
 		if (!hasClosed && player != null && !playerInjection.containsKey(player)) {
 			try {
-				PlayerInjector injector = new PlayerInjector(player, this, packetFilters);
+				PlayerInjector injector = new PlayerInjector(player, this, sendingFilters);
 				
 				injector.injectManager();
 				playerInjection.put(player, injector);

ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java

@@ -84,15 +84,15 @@ class PlayerInjector {
 	
 	// The packet manager and filters
 	private PacketFilterManager manager;
-	private Set<Integer> packetFilters;
+	private Set<Integer> sendingFilters;
 	
 	// Previous data input
 	private DataInputStream cachedInput;
 
-	public PlayerInjector(Player player, PacketFilterManager manager, Set<Integer> packetFilters) throws IllegalAccessException {
+	public PlayerInjector(Player player, PacketFilterManager manager, Set<Integer> sendingFilters) throws IllegalAccessException {
 		this.player = player;
 		this.manager = manager;
-		this.packetFilters = packetFilters;
+		this.sendingFilters = sendingFilters;
 		initialize();
 	}
 
@@ -342,7 +342,7 @@ class PlayerInjector {
 		Integer id = MinecraftRegistry.getPacketToID().get(packet.getClass());
 
 		// Make sure we're listening
-		if (packetFilters.contains(id)) {	
+		if (sendingFilters.contains(id)) {	
 			// A packet has been sent guys!
 			PacketContainer container = new PacketContainer(id, packet);
 			PacketEvent event = PacketEvent.fromServer(manager, container, player);

ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java

@@ -28,7 +28,7 @@ import com.comphenix.protocol.reflect.StructureModifier;
  * Caches structure modifiers.
  * @author Kristian
  */
-class StructureCache {
+public class StructureCache {
 	// Structure modifiers
 	private static Map<Integer, StructureModifier<Object>> structureModifiers = new HashMap<Integer, StructureModifier<Object>>();