Correctly clone packet listeners.

ProtocolLib/src/main/java/com/comphenix/protocol/async/NullPacketListener.java

@@ -67,7 +67,7 @@ class NullPacketListener implements PacketListener {
 	
 	private ListeningWhitelist cloneWhitelist(ListenerPriority priority, ListeningWhitelist whitelist) {
 		if (whitelist != null) 
-			return new ListeningWhitelist(priority, whitelist.getWhitelist(), whitelist.getGamePhase());
+			return ListeningWhitelist.newBuilder(whitelist).priority(priority).build();
 		else 
 			return null;
 	}

ProtocolLib/src/main/java/com/comphenix/protocol/events/ListeningWhitelist.java

@@ -18,11 +18,13 @@
 package com.comphenix.protocol.events;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Set;
 
 import com.comphenix.protocol.injector.GamePhase;
+
 import com.google.common.base.Objects;
 import com.google.common.collect.Sets;
 
@@ -32,17 +34,23 @@ import com.google.common.collect.Sets;
  * @author Kristian 
  */
 public class ListeningWhitelist {
-	
 	/**
 	 * A whitelist with no packets - indicates that the listener shouldn't observe any packets.
 	 */
 	public static final ListeningWhitelist EMPTY_WHITELIST = new ListeningWhitelist(ListenerPriority.LOW);
-	
-	private ListenerPriority priority;
-	private Set<Integer> whitelist;
-	private GamePhase gamePhase;
-	private Set<ListenerOptions> options = EnumSet.noneOf(ListenerOptions.class);
 
+	private final ListenerPriority priority;
+	private final Set<Integer> whitelist;
+	private final GamePhase gamePhase;
+	private final Set<ListenerOptions> options;
+
+	private ListeningWhitelist(Builder builder) {
+		this.priority = builder.priority;
+		this.whitelist = builder.whitelist;
+		this.gamePhase = builder.gamePhase;
+		this.options = builder.options;
+	}
+	
 	/**
 	 * Creates a packet whitelist for a given priority with a set of packet IDs.
 	 * @param priority - the listener priority.
@@ -51,7 +59,7 @@ public class ListeningWhitelist {
 	public ListeningWhitelist(ListenerPriority priority, Set<Integer> whitelist) {
 		this(priority, whitelist, GamePhase.PLAYING);
 	}
-	
+
 	/**
 	 * Creates a packet whitelist for a given priority with a set of packet IDs.
 	 * @param priority - the listener priority.
@@ -60,10 +68,11 @@ public class ListeningWhitelist {
 	 */
 	public ListeningWhitelist(ListenerPriority priority, Set<Integer> whitelist, GamePhase gamePhase) {
 		this.priority = priority;
-		this.whitelist = whitelist;
+		this.whitelist = safeSet(whitelist);
 		this.gamePhase = gamePhase;
+		this.options = EnumSet.noneOf(ListenerOptions.class);
 	}
-	
+
 	/**
 	 * Creates a packet whitelist of a given priority for a list of packets.
 	 * @param priority - the listener priority.
@@ -73,8 +82,9 @@ public class ListeningWhitelist {
 		this.priority = priority;
 		this.whitelist = Sets.newHashSet(whitelist);
 		this.gamePhase = GamePhase.PLAYING;
+		this.options = EnumSet.noneOf(ListenerOptions.class);
 	}
-	
+
 	/**
 	 * Creates a packet whitelist for a given priority with a set of packet IDs.
 	 * @param priority - the listener priority.
@@ -85,24 +95,23 @@ public class ListeningWhitelist {
 		this.priority = priority;
 		this.whitelist = Sets.newHashSet(whitelist);
 		this.gamePhase = gamePhase;
+		this.options = EnumSet.noneOf(ListenerOptions.class);
 	}
-	
+
 	/**
 	 * Creates a packet whitelist for a given priority with a set of packet IDs and options.
 	 * @param priority - the listener priority.
 	 * @param whitelist - list of packet IDs to observe/enable.
 	 * @param gamePhase - which game phase to receieve notifications on.
+	 * @param options - every special option associated with this whitelist.
 	 */
 	public ListeningWhitelist(ListenerPriority priority, Integer[] whitelist, GamePhase gamePhase, ListenerOptions... options) {
 		this.priority = priority;
 		this.whitelist = Sets.newHashSet(whitelist);
 		this.gamePhase = gamePhase;
-		
+		this.options = safeEnumSet(Arrays.asList(options), ListenerOptions.class);
-		if (options != null) {
-			this.options.addAll(Arrays.asList(options));
-		}
 	}
-	
+
 	/**
 	 * Whether or not this whitelist has any enabled packets.
 	 * @return TRUE if there are any packets, FALSE otherwise.
@@ -110,7 +119,7 @@ public class ListeningWhitelist {
 	public boolean isEnabled() {
 		return whitelist != null && whitelist.size() > 0;
 	}
-	
+
 	/**
 	 * Retrieve the priority in the execution order of the packet listener. Highest priority will be executed last.
 	 * @return Execution order in terms of priority.
@@ -118,7 +127,7 @@ public class ListeningWhitelist {
 	public ListenerPriority getPriority() {
 		return priority;
 	}
-	
+
 	/**
 	 * Retrieves the list of packets that will be observed by the listeners.
 	 * @return Packet whitelist.
@@ -134,7 +143,7 @@ public class ListeningWhitelist {
 	public GamePhase getGamePhase() {
 		return gamePhase;
 	}
-	
+
 	/**
 	 * Retrieve every special option associated with this whitelist.
 	 * @return Every special option.
@@ -142,10 +151,10 @@ public class ListeningWhitelist {
 	public Set<ListenerOptions> getOptions() {
 		return Collections.unmodifiableSet(options);
 	}
-	
+
 	@Override
-	public int hashCode(){
+	public int hashCode() {
-	    return Objects.hashCode(priority, whitelist, gamePhase, options);
+		return Objects.hashCode(priority, whitelist, gamePhase, options);
 	}
 
 	/**
@@ -161,10 +170,10 @@ public class ListeningWhitelist {
 					return true;
 			}
 		}
-		
+
 		return false;
 	}
-	
+
 	/**
 	 * Determine if the given whitelist is empty or not.
 	 * @param whitelist - the whitelist to test.
@@ -178,30 +187,144 @@ public class ListeningWhitelist {
 		else
 			return whitelist.getWhitelist().isEmpty();
 	}
-	
+
 	@Override
-	public boolean equals(final Object obj){
+	public boolean equals(final Object obj) {
-	    if(obj instanceof ListeningWhitelist){
+		if (obj instanceof ListeningWhitelist) {
-	        final ListeningWhitelist other = (ListeningWhitelist) obj;
+			final ListeningWhitelist other = (ListeningWhitelist) obj;
-	        return Objects.equal(priority, other.priority)
+			return Objects.equal(priority, other.priority)
-	            && Objects.equal(whitelist, other.whitelist)
+					&& Objects.equal(whitelist, other.whitelist)
-		        && Objects.equal(gamePhase, other.gamePhase)
+					&& Objects.equal(gamePhase, other.gamePhase)
-		        && Objects.equal(options, other.options);
+					&& Objects.equal(options, other.options);
-	    } else{
+		} else {
-	        return false;
+			return false;
-	    }
+		}
 	}
-	
+
 	@Override
 	public String toString() {
 		if (this == EMPTY_WHITELIST)
 			return "EMPTY_WHITELIST";
 		else
-			return Objects.toStringHelper(this)
+			return Objects.toStringHelper(this).
-				.add("priority", priority)
+				add("priority", priority).
-				.add("packets", whitelist)
+				add("packets", whitelist).
-				.add("gamephase", gamePhase)
+				add("gamephase", gamePhase).
-				.add("options", options).
+				add("options", options).
 				toString();
 	}
+	
+	/**
+	 * Construct a new builder of whitelists.
+	 * @return New whitelist builder.
+	 */
+	public static Builder newBuilder() {
+		return new Builder(null);
+	}
+	
+	/**
+	 * Construct a new builder of whitelists initialized to the same values as the template.
+	 * @param template - the template object.
+	 * @return New whitelist builder.
+	 */
+	public static Builder newBuilder(ListeningWhitelist template) {
+		return new Builder(template);
+	}
+	
+	/**
+	 * Construct a copy of a given enum.
+	 * @param options - the options to copy, or NULL to indicate the empty set.
+	 * @return A copy of the enum set.
+	 */
+	private static <T extends Enum<T>> EnumSet<T> safeEnumSet(Collection<T> options, Class<T> enumClass) {
+		if (options != null) {
+			return EnumSet.copyOf(options);
+		} else {
+			return EnumSet.noneOf(enumClass);
+		}
+	}
+	
+	/**
+	 * Construct a copy of a given set.
+	 * @param list - the set to copy.
+	 * @return The copied set.
+	 */
+	private static <T> Set<T> safeSet(Collection<T> set) {
+		if (set != null)
+			return Sets.newHashSet(set);
+		else
+			return Collections.emptySet();
+	}
+
+	/**
+	 * Represents a builder of whitelists.
+	 * @author Kristian
+	 */
+	public static class Builder {
+		private ListenerPriority priority;
+		private Set<Integer> whitelist;
+		private GamePhase gamePhase;
+		private Set<ListenerOptions> options;
+		
+		/**
+		 * Construct a new listening whitelist template.
+		 * @param template - the template.
+		 */
+		private Builder(ListeningWhitelist template) {
+			if (template != null) {
+				priority(template.getPriority());
+				gamePhase(template.getGamePhase());
+				whitelist(template.getWhitelist());
+				options(template.getOptions());
+			}
+		}
+		
+		/**
+		 * Set the priority to use when constructing new whitelists.
+		 * @param priority - the priority.
+		 * @return This builder, for chaining.
+		 */
+		public Builder priority(ListenerPriority priority) {
+			this.priority = priority;
+			return this;
+		}
+		
+		/**
+		 * Set the whitelist of packet IDs to copy when constructing new whitelists.
+		 * @param whitelist - the whitelist of packets.
+		 * @return This builder, for chaining.
+		 */
+		public Builder whitelist(Collection<Integer> whitelist) {
+			this.whitelist = safeSet(whitelist);
+			return this;
+		}
+		
+		/**
+		 * Set the gamephase to use when constructing new whitelists.
+		 * @param gamePhase - the gamephase.
+		 * @return This builder, for chaining.
+		 */
+		public Builder gamePhase(GamePhase gamePhase) {
+			this.gamePhase = gamePhase;
+			return this;
+		}
+		
+		/**
+		 * Set the options to copy when constructing new whitelists.
+		 * @param options - the options.
+		 * @return This builder, for chaining.
+		 */
+		public Builder options(Set<ListenerOptions> options) {
+			this.options = safeSet(options);
+			return this;
+		}
+		
+		/**
+		 * Construct a new whitelist from the values in this builder.
+		 * @return The new whitelist.
+		 */
+		public ListeningWhitelist build() {
+			return new ListeningWhitelist(this);
+		}
+	}
 }