Fix a few issues with data watchers

ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java

@@ -1,6 +1,6 @@
 /**
  *  ProtocolLib - Bukkit server library that allows access to the Minecraft protocol.
- *  Copyright (C) 2012 Kristian S. Stangeland
+ *  Copyright (C) 2016 dmulloy2
  *
  *  This program is free software; you can redistribute it and/or modify it under the terms of the
  *  GNU General Public License as published by the Free Software Foundation; either version 2 of
@@ -16,7 +16,6 @@
  */
 package com.comphenix.protocol.wrappers;
 
-import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
@@ -37,47 +36,76 @@ import com.comphenix.protocol.reflect.FieldUtils;
 import com.comphenix.protocol.reflect.FuzzyReflection;
 import com.comphenix.protocol.reflect.StructureModifier;
 import com.comphenix.protocol.reflect.accessors.Accessors;
+import com.comphenix.protocol.reflect.accessors.ConstructorAccessor;
+import com.comphenix.protocol.reflect.accessors.FieldAccessor;
 import com.comphenix.protocol.reflect.accessors.MethodAccessor;
 import com.comphenix.protocol.utility.MinecraftReflection;
+import com.google.common.base.Optional;
 
+/**
+ * Represents a DataWatcher in 1.9
+ * @author dmulloy2
+ */
 public class WrappedDataWatcher extends AbstractWrapper implements Iterable<WrappedWatchableObject> {
-	private static MethodAccessor getter = null;
+	private static final Class<?> HANDLE_TYPE = MinecraftReflection.getDataWatcherClass();
-	private static MethodAccessor setter = null;
 
-	private static Field MAP_FIELD = null;
+	private static MethodAccessor GETTER = null;
+	private static MethodAccessor SETTER = null;
+
+	private static FieldAccessor MAP_FIELD = null;
 	private static Field ENTITY_DATA_FIELD = null;
 	private static Field ENTITY_FIELD = null;
 
-	/**
+	private static ConstructorAccessor constructor = null;
-	 * Constructs a new Data Watcher. This is currently unsupported in 1.9 and up due to changes in data watcher structure.
+	private static ConstructorAccessor lightningConstructor = null;
-	 * Essentially, Mojang further tied data watchers to their entities.
+
-	 * @deprecated
+	private static Object fakeEntity = null;
-	 */
+
-	@Deprecated
+	// ---- Construction
-	public WrappedDataWatcher() {
-		super(MinecraftReflection.getDataWatcherClass());
-	}
 
 	/**
 	 * Constructs a wrapped data watcher around an existing NMS data watcher.
 	 * @param handle NMS data watcher
 	 */
 	public WrappedDataWatcher(Object handle) {
-		this();
+		super(HANDLE_TYPE);
 		setHandle(handle);
 	}
 
-	@Override
+	/**
-	public Iterator<WrappedWatchableObject> iterator() {
+	 * Constructs a new DataWatcher using a fake entity.
-		return getWatchableObjects().iterator();
+	 */
+	public WrappedDataWatcher() {
+		this(newHandle(fakeEntity()));
+		clear();
 	}
 
-	public List<WrappedWatchableObject> getWatchableObjects() {
+	private static Object newHandle(Object entity) {
-		return new ArrayList<>(asMap().values());
+		if (constructor == null) {
+			constructor = Accessors.getConstructorAccessor(HANDLE_TYPE, MinecraftReflection.getEntityClass());
+		}
+
+		return constructor.invoke(entity);
 	}
 
+	private static Object fakeEntity() {
+		if (fakeEntity != null) {
+			return fakeEntity;
+		}
+
+		// We can create a fake lightning strike without it affecting anything
+		if (lightningConstructor == null) {
+			lightningConstructor = Accessors.getConstructorAccessor(MinecraftReflection.getMinecraftClass("EntityLightning"),
+					MinecraftReflection.getNmsWorldClass(), double.class, double.class, double.class, boolean.class);
+		}
+
+		return fakeEntity = lightningConstructor.invoke(null, 0, 0, 0, true);
+	}
+
+	// ---- Collection Methods
+
 	@SuppressWarnings("unchecked")
-	public Map<Integer, WrappedWatchableObject> asMap() {
+	private Map<Integer, ?> getMap() {
 		FuzzyReflection fuzzy = FuzzyReflection.fromClass(handleType, true);
 		List<Field> candidates = fuzzy.getFieldListByType(Map.class);
 
@@ -86,8 +114,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 				// This is the entity class to current index map, which we really don't have a use for
 			} else {
 				// This is the map we're looking for
-				MAP_FIELD = candidate;
+				MAP_FIELD = Accessors.getFieldAccessor(candidate);
-				MAP_FIELD.setAccessible(true);
 			}
 		}
 
@@ -95,28 +122,69 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 			throw new FieldAccessException("Could not find index -> object map.");
 		}
 
-		Map<Integer, ?> map = null;
+		return (Map<Integer, ?>) MAP_FIELD.get(handle);
-
+	}
-		try {
-			map = (Map<Integer, ?>) MAP_FIELD.get(handle);
-		} catch (ReflectiveOperationException e) {
-			throw new FieldAccessException("Failed to access index -> object map", e);
-		}
 
+	/**
+	 * Gets the contents of this DataWatcher as a map.
+	 * @return The contents
+	 */
+	public Map<Integer, WrappedWatchableObject> asMap() {
 		Map<Integer, WrappedWatchableObject> ret = new HashMap<>();
-		for (Entry<Integer, ?> entry : map.entrySet()) {
+
+		for (Entry<Integer, ?> entry : getMap().entrySet()) {
 			ret.put(entry.getKey(), new WrappedWatchableObject(entry.getValue()));
 		}
 
 		return ret;
 	}
 
-	public WrappedWatchableObject getWatchableObject(int index) {
+	/**
-		return asMap().get(index);
+	 * Gets a list of the contents of this DataWatcher.
+	 * @return The contents
+	 */
+	public List<WrappedWatchableObject> getWatchableObjects() {
+		return new ArrayList<>(asMap().values());
 	}
 
+	@Override
+	public Iterator<WrappedWatchableObject> iterator() {
+		return getWatchableObjects().iterator();
+	}
+
+	/**
+	 * Gets the size of this DataWatcher's contents.
+	 * @return The size
+	 */
 	public int size() {
-		return asMap().size();
+		return getMap().size();
+	}
+
+	private void clear() {
+		getMap().clear();
+	}
+
+	/**
+	 * Gets the watchable object at a given index.
+	 * @param index Index
+	 * @return The watchable object, or null if none exists
+	 */
+	public WrappedWatchableObject getWatchableObject(int index) {
+		Object handle = getMap().get(index);
+		if (handle != null) {
+			return new WrappedWatchableObject(handle);
+		} else {
+			return null;
+		}
+	}
+
+	/**
+	 * Whether or not this DataWatcher has an object at a given index.
+	 * @param index Index
+	 * @return True if it does, false if not
+	 */
+	public boolean hasIndex(int index) {
+		return getMap().containsKey(index);
 	}
 
 	// ---- Object Getters
@@ -206,27 +274,39 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 	}
    
     private Object getWatcherObject(WrappedDataWatcherObject watcherObject) {
-		if (getter == null) {
+		if (GETTER == null) {
-			getter = Accessors.getMethodAccessor(handleType, "get", watcherObject.getHandleType());
+			GETTER = Accessors.getMethodAccessor(handleType, "get", watcherObject.getHandleType());
 		}
 
-		return getter.invoke(handle, watcherObject.getHandle());
+		return GETTER.invoke(handle, watcherObject.getHandle());
     }
 
 	// ---- Object Setters
 
+    /**
+     * Sets the DataWatcher Item at a given index to a new value.
+     * @param index Index
+     * @param value New value
+     */
 	public void setObject(int index, Object value) {
 		setObject(new WrappedDataWatcherObject(index, null), value);
 	}
 
+	/**
+	 * Sets the DataWatcher Item associated with a given watcher object to a new value.
+	 * @param watcherObject Associated watcher object
+	 * @param value New value
+	 */
 	public void setObject(WrappedDataWatcherObject watcherObject, Object value) {
-		if (setter == null) {
+		if (SETTER == null) {
-			setter = Accessors.getMethodAccessor(handleType, "set", watcherObject.getHandleType(), Object.class);
+			SETTER = Accessors.getMethodAccessor(handleType, "set", watcherObject.getHandleType(), Object.class);
 		}
 
-		setter.invoke(handle, watcherObject.getHandle(), WrappedWatchableObject.getUnwrapped(value));
+		SETTER.invoke(handle, watcherObject.getHandle(), WrappedWatchableObject.getUnwrapped(value));
 	}
 
+	// TODO Add support for setting the dirty state
+
 	/**
 	 * Clone the content of the current DataWatcher.
 	 * 
@@ -299,14 +379,22 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 		}
 	}
 
+	/**
+	 * No longer supported in 1.9 due to the removal of a consistent type <-> ID map.
+	 * @param clazz
+	 * @return Null
+	 */
 	@Deprecated
-	@SuppressWarnings("unused")
 	public static Integer getTypeID(Class<?> clazz) {
 		return null;
 	}
 
+	/**
+	 * No longer supported in 1.9 due to the removal of a consistent type <-> ID map.
+	 * @param typeID
+	 * @return Null
+	 */
 	@Deprecated
-	@SuppressWarnings("unused")
 	public static Class<?> getTypeClass(int typeID) {
 		return null;
 	}
@@ -341,52 +429,75 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 		return getWatchableObjects().hashCode();
 	}
 
+	/**
+	 * Represents a DataWatcherObject in 1.9.
+	 * @author dmulloy2
+	 */
 	public static class WrappedDataWatcherObject extends AbstractWrapper {
 		private static final Class<?> HANDLE_TYPE = MinecraftReflection.getDataWatcherObjectClass();
-		private static Constructor<?> constructor = null;
+		private static ConstructorAccessor constructor = null;
 
-		private StructureModifier<Object> modifier = new StructureModifier<Object>(HANDLE_TYPE);
+		private final StructureModifier<Object> modifier;
 
+		/**
+		 * Creates a new watcher object from a NMS handle
+		 * @param handle NMS handle
+		 */
 		public WrappedDataWatcherObject(Object handle) {
 			super(HANDLE_TYPE);
+
 			setHandle(handle);
-			modifier.withTarget(handle);
+			this.modifier = new StructureModifier<Object>(HANDLE_TYPE).withTarget(handle);
 		}
 
+		/**
+		 * Creates a new watcher object from an index and serializer
+		 * @param index Index
+		 * @param serializer Serializer, see {@link Registry}
+		 */
 		public WrappedDataWatcherObject(int index, Serializer serializer) {
 			this(newHandle(index, serializer));
 		}
 
 		private static Object newHandle(int index, Serializer serializer) {
 			if (constructor == null) {
-				constructor = HANDLE_TYPE.getConstructors()[0];
+				constructor = Accessors.getConstructorAccessor(HANDLE_TYPE.getConstructors()[0]);
 			}
 
 			Object handle = serializer != null ? serializer.getHandle() : null;
-
+			return constructor.invoke(index, handle);
-			try {
-				return constructor.newInstance(index, handle);
-			} catch (ReflectiveOperationException e) {
-				throw new RuntimeException("Failed to create new DataWatcherObject", e);
-			}
 		}
 
+		/**
+		 * Gets this watcher object's index
+		 * @return The index
+		 */
 		public int getIndex() {
 			return (int) modifier.read(0);
 		}
 
-		public Serializer getSerializer() {
+		// TODO this
-			// TODO this
+		/* public Serializer getSerializer() {
 			return null;
-		}
+		} */
 	}
 
+	/**
+	 * Represents a DataWatcherSerializer in 1.9.
+	 * @author dmulloy2
+	 */
 	public static class Serializer extends AbstractWrapper {
 		private static final Class<?> HANDLE_TYPE = MinecraftReflection.getDataWatcherSerializerClass();
 
 		private final Class<?> type;
 		private final boolean optional;
 
+		/**
+		 * Constructs a new Serializer
+		 * @param type Type it serializes
+		 * @param handle NMS handle
+		 * @param optional Whether or not it's {@link Optional}
+		 */
 		public Serializer(Class<?> type, Object handle, boolean optional) {
 			super(HANDLE_TYPE);
 			this.type = type;
@@ -395,19 +506,39 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
 			setHandle(handle);
 		}
 
+		/**
+		 * Gets the type this serializer serializes.
+		 * @return The type
+		 */
 		public Class<?> getType() {
 			return type;
 		}
 
+		/**
+		 * Whether or not this serializer is optional, that is whether or not
+		 * the return type is wrapped in a {@link Optional}.
+		 * @return True if it is, false if not
+		 */
 		public boolean isOptional() {
 			return optional;
 		}
 	}
 
+	/**
+	 * Represents a DataWatcherRegistry containing the supported {@link Serializer}s in 1.9.
+	 * @author dmulloy2
+	 */
 	public static class Registry {
 		private static boolean INITIALIZED = false;
 		private static Map<Class<?>, Serializer> REGISTRY = new HashMap<>();
 
+		/**
+		 * Gets the serializer associated with a given class. </br>
+		 * <b>Note</b>: If {@link Serializer#isOptional()}, the values must be wrapped in {@link Optional}
+		 * 
+		 * @param clazz Class to find serializer for
+		 * @return The serializer, or null if none exists
+		 */
 		public static Serializer get(Class<?> clazz) {
 			if (! INITIALIZED) {
 				initialize();

ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedWatchableObject.java

@@ -1,5 +1,18 @@
 /**
- * (c) 2016 dmulloy2
+ *  ProtocolLib - Bukkit server library that allows access to the Minecraft protocol.
+ *  Copyright (C) 2016 dmulloy2
+ *
+ *  This program is free software; you can redistribute it and/or modify it under the terms of the
+ *  GNU General Public License as published by the Free Software Foundation; either version 2 of
+ *  the License, or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ *  without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *  See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with this program;
+ *  if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ *  02111-1307 USA
  */
 package com.comphenix.protocol.wrappers;
 
@@ -10,33 +23,38 @@ import com.comphenix.protocol.utility.MinecraftReflection;
 import com.comphenix.protocol.wrappers.WrappedDataWatcher.WrappedDataWatcherObject;
 
 /**
+ * Represents a DataWatcher Item in 1.9.
  * @author dmulloy2
  */
 
 public class WrappedWatchableObject extends AbstractWrapper {
-	private StructureModifier<Object> modifier;
+	private final StructureModifier<Object> modifier;
-
-	private WrappedWatchableObject() {
-		super(MinecraftReflection.getDataWatcherItemClass());
-	}
 
 	/**
 	 * Constructs a wrapped watchable object around an existing NMS data watcher item.
 	 * @param handle Data watcher item
 	 */
 	public WrappedWatchableObject(Object handle) {
-		this();
+		super(MinecraftReflection.getDataWatcherItemClass());
-		setHandle(handle);
 
-		modifier = new StructureModifier<Object>(handleType).withTarget(handle);
+		setHandle(handle);
+		this.modifier = new StructureModifier<Object>(handleType).withTarget(handle);
 	}
 
 	// ---- Getter methods
 
+	/**
+	 * Gets this Item's watcher object, which contains the index and serializer.
+	 * @return The watcher object
+	 */
 	public WrappedDataWatcherObject getWatcherObject() {
 		return new WrappedDataWatcherObject(modifier.read(0));
 	}
 
+	/**
+	 * Gets this Item's index from the watcher object
+	 * @return The index
+	 */
 	public int getIndex() {
 		return getWatcherObject().getIndex();
 	}
@@ -94,6 +112,11 @@ public class WrappedWatchableObject extends AbstractWrapper {
 			return unwrapped;
 	}
 
+	/**
+	 * Sets the value of this item.
+	 * @param value New value
+	 * @param updateClient Whether or not to update the client
+	 */
 	public void setValue(Object value, boolean updateClient) {
 		modifier.write(1, getUnwrapped(value));
 
@@ -102,6 +125,10 @@ public class WrappedWatchableObject extends AbstractWrapper {
 		}
 	}
 
+	/**
+	 * Sets the value of this item.
+	 * @param value New value
+	 */
 	public void setValue(Object value) {
 		setValue(value, false);
 	}
@@ -145,10 +172,18 @@ public class WrappedWatchableObject extends AbstractWrapper {
 			return wrapped;
 	}
 
+	/**
+	 * Whether or not the value must be synchronized with the client.
+	 * @return True if it must, false if not
+	 */
 	public boolean getDirtyState() {
 		return (boolean) modifier.read(2);
 	}
 
+	/**
+	 * Sets this item's dirty state
+	 * @param dirty New state
+	 */
 	public void setDirtyState(boolean dirty) {
 		modifier.write(2, dirty);
 	}

ProtocolLib/src/test/java/com/comphenix/protocol/wrappers/WrappedDataWatcherTest.java

@@ -12,6 +12,9 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import com.comphenix.protocol.BukkitInitialization;
+import com.comphenix.protocol.wrappers.WrappedDataWatcher.Registry;
+import com.comphenix.protocol.wrappers.WrappedDataWatcher.Serializer;
+import com.comphenix.protocol.wrappers.WrappedDataWatcher.WrappedDataWatcherObject;
 
 /**
  * @author dmulloy2
@@ -33,5 +36,11 @@ public class WrappedDataWatcherTest {
 
 		wrapper.setObject(0, (byte) 1);
 		assertTrue(wrapper.getByte(0) == 1);
+
+		Serializer serializer = Registry.get(String.class);
+		WrappedDataWatcherObject watcherObject = new WrappedDataWatcherObject(3, serializer);
+		wrapper.setObject(watcherObject, "Hiya");
+
+		assertTrue(wrapper.getString(3).equals("Hiya"));
 	}
 }