From 86f04f53b5f2b338ecbe4ac601b2cfb994a32f0b Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 19 Nov 2013 00:50:55 +0100 Subject: [PATCH] Allow packet constructor to properly unwrap other Bukkit wrappers. --- .../protocol/injector/BukkitUnwrapper.java | 22 +++- .../protocol/injector/PacketConstructor.java | 51 ++++---- .../protocol/reflect/EquivalentConverter.java | 4 + .../protocol/utility/MinecraftReflection.java | 22 ++++ .../protocol/wrappers/BukkitConverters.java | 110 +++++++++++++++++- .../protocol/events/PacketContainerTest.java | 18 ++- 6 files changed, 199 insertions(+), 28 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/BukkitUnwrapper.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/BukkitUnwrapper.java index b7db2d7f..cc9ff538 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/BukkitUnwrapper.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/BukkitUnwrapper.java @@ -77,7 +77,11 @@ public class BukkitUnwrapper implements Unwrapper { // Special case if (wrappedObject == null) return null; - Class currentClass = wrappedObject.getClass(); + Class currentClass = PacketConstructor.getClass(wrappedObject); + + // No need to unwrap primitives + if (currentClass.isPrimitive() || currentClass.equals(String.class)) + return null; // Next, check for types that doesn't have a getHandle() if (wrappedObject instanceof Collection) { @@ -119,7 +123,7 @@ public class BukkitUnwrapper implements Unwrapper { * @param type - the type of the class. * @return An unwrapper for the given class. */ - private Unwrapper getSpecificUnwrapper(Class type) { + private Unwrapper getSpecificUnwrapper(final Class type) { // See if we're already determined this if (unwrapperCache.containsKey(type)) { // We will never remove from the cache, so this ought to be thread safe @@ -133,8 +137,9 @@ public class BukkitUnwrapper implements Unwrapper { Unwrapper methodUnwrapper = new Unwrapper() { @Override public Object unwrapItem(Object wrappedObject) { - try { + if (wrappedObject instanceof Class) + return checkClass((Class) wrappedObject, type, find.getReturnType()); return find.invoke(wrappedObject); } catch (IllegalArgumentException e) { @@ -180,7 +185,7 @@ public class BukkitUnwrapper implements Unwrapper { * @param type - a cached field unwrapper. * @return The cached field unwrapper. */ - private Unwrapper getFieldUnwrapper(Class type) { + private Unwrapper getFieldUnwrapper(final Class type) { final Field find = FieldUtils.getField(type, "handle", true); // See if we succeeded @@ -189,6 +194,8 @@ public class BukkitUnwrapper implements Unwrapper { @Override public Object unwrapItem(Object wrappedObject) { try { + if (wrappedObject instanceof Class) + return checkClass((Class) wrappedObject, type, find.getType()); return FieldUtils.readField(find, wrappedObject, true); } catch (IllegalAccessException e) { reporter.reportDetailed(this, @@ -210,4 +217,11 @@ public class BukkitUnwrapper implements Unwrapper { return null; } } + + private static Class checkClass(Class input, Class expected, Class result) { + if (expected.isAssignableFrom(input)) { + return result; + } + return null; + } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java index 7d7a3258..57105ffc 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketConstructor.java @@ -25,6 +25,7 @@ import com.comphenix.protocol.error.RethrowErrorReporter; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.injector.packet.PacketRegistry; import com.comphenix.protocol.reflect.FieldAccessException; +import com.comphenix.protocol.wrappers.BukkitConverters; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.primitives.Primitives; @@ -53,18 +54,19 @@ public class PacketConstructor { private List unwrappers; // Parameters that need to be unwrapped - private boolean[] unwrappable; + private Unwrapper[] paramUnwrapper; private PacketConstructor(Constructor constructorMethod) { this.constructorMethod = constructorMethod; this.unwrappers = Lists.newArrayList((Unwrapper) new BukkitUnwrapper(new RethrowErrorReporter() )); + this.unwrappers.addAll(BukkitConverters.getUnwrappers()); } - private PacketConstructor(int packetID, Constructor constructorMethod, List unwrappers, boolean[] unwrappable) { + private PacketConstructor(int packetID, Constructor constructorMethod, List unwrappers, Unwrapper[] paramUnwrapper) { this.packetID = packetID; this.constructorMethod = constructorMethod; this.unwrappers = unwrappers; - this.unwrappable = unwrappable; + this.paramUnwrapper = paramUnwrapper; } public ImmutableList getUnwrappers() { @@ -85,7 +87,7 @@ public class PacketConstructor { * @return A constructor with a different set of unwrappers. */ public PacketConstructor withUnwrappers(List unwrappers) { - return new PacketConstructor(packetID, constructorMethod, unwrappers, unwrappable); + return new PacketConstructor(packetID, constructorMethod, unwrappers, paramUnwrapper); } /** @@ -100,12 +102,12 @@ public class PacketConstructor { public PacketConstructor withPacket(int id, Object[] values) { Class[] types = new Class[values.length]; Throwable lastException = null; - boolean[] unwrappable = new boolean[values.length]; + Unwrapper[] paramUnwrapper = new Unwrapper[values.length]; for (int i = 0; i < types.length; i++) { // Default type if (values[i] != null) { - types[i] = (values[i] instanceof Class) ? (Class)values[i] : values[i].getClass(); + types[i] = PacketConstructor.getClass(values[i]); for (Unwrapper unwrapper : unwrappers) { Object result = null; @@ -118,8 +120,8 @@ public class PacketConstructor { // Update type we're searching for if (result != null) { - types[i] = result.getClass(); - unwrappable[i] = true; + types[i] = PacketConstructor.getClass(result); + paramUnwrapper[i] = unwrapper; break; } } @@ -141,7 +143,7 @@ public class PacketConstructor { if (isCompatible(types, params)) { // Right, we've found our type - return new PacketConstructor(id, constructor, unwrappers, unwrappable); + return new PacketConstructor(id, constructor, unwrappers, paramUnwrapper); } } @@ -160,15 +162,8 @@ public class PacketConstructor { try { // Convert types that needs to be converted for (int i = 0; i < values.length; i++) { - if (unwrappable[i]) { - for (Unwrapper unwrapper : unwrappers) { - Object converted = unwrapper.unwrapItem(values[i]); - - if (converted != null) { - values[i] = converted; - break; - } - } + if (paramUnwrapper[i] != null) { + values[i] = paramUnwrapper[i].unwrapItem(values[i]); } } @@ -196,7 +191,7 @@ public class PacketConstructor { Class paramType = params[i]; // The input type is always wrapped - if (paramType.isPrimitive()) { + if (!inputType.isPrimitive() && paramType.isPrimitive()) { // Wrap it paramType = Primitives.wrap(paramType); } @@ -213,6 +208,17 @@ public class PacketConstructor { // Parameter count must match return false; } + + /** + * Retrieve the class of an object, or just the class if it already is a class object. + * @param obj - the object. + * @return The class of an object. + */ + public static Class getClass(Object obj) { + if (obj instanceof Class) + return (Class) obj; + return obj.getClass(); + } /** * Represents a unwrapper for a constructor parameter. @@ -222,8 +228,11 @@ public class PacketConstructor { public static interface Unwrapper { /** * Convert the given wrapped object to the equivalent net.minecraft.server object. - * @param wrappedObject - wrapped object. - * @return The net.minecraft.server object. + *

+ * Note that we may pass in a class instead of object - in that case, the unwrapper should + * return the equivalent NMS class. + * @param wrappedObject - wrapped object or class. + * @return The equivalent net.minecraft.server object or class. */ public Object unwrapItem(Object wrappedObject); } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/EquivalentConverter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/EquivalentConverter.java index 63733abe..fef48f24 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/EquivalentConverter.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/EquivalentConverter.java @@ -26,6 +26,8 @@ package com.comphenix.protocol.reflect; public interface EquivalentConverter { /** * Retrieve a copy of the specific type using an instance of the generic type. + *

+ * This is usually a wrapper type in the Bukkit API. * @param generic - the generic type. * @return The new specific type. */ @@ -33,6 +35,8 @@ public interface EquivalentConverter { /** * Retrieve a copy of the generic type from a specific type. + *

+ * This is usually a native net.minecraft.server type in Minecraft. * @param genericType - class or super class of the generic type. * @param specific - the specific type we need to copy. * @return A copy of the specific type. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java index 3610b6a7..fec2cd4b 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java @@ -43,6 +43,7 @@ import org.bukkit.Bukkit; import org.bukkit.Server; import org.bukkit.inventory.ItemStack; +import com.comphenix.protocol.Packets; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; @@ -1120,6 +1121,27 @@ public class MinecraftReflection { } } + /** + * Retrieve the net.minecraft.server.MobEffect class. + * @return The mob effect class. + */ + public static Class getMobEffectClass() { + try { + return getMinecraftClass("MobEffect"); + } catch (RuntimeException e) { + // It is the second parameter in Packet41MobEffect + Class packet = PacketRegistry.getPacketClassFromID(Packets.Server.MOB_EFFECT); + Constructor constructor = FuzzyReflection.fromClass(packet).getConstructor( + FuzzyMethodContract.newBuilder(). + parameterCount(2). + parameterExactType(int.class, 0). + parameterMatches(getMinecraftObjectMatcher(), 1). + build() + ); + return setMinecraftClass("MobEffect", constructor.getParameterTypes()[1]); + } + } + /** * Determine if a given method retrieved by ASM is a constructor. * @param name - the name of the method. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java index 12d5c173..3cc9a4d6 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java @@ -18,6 +18,7 @@ package com.comphenix.protocol.wrappers; import java.lang.ref.WeakReference; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; @@ -28,18 +29,24 @@ import org.bukkit.World; import org.bukkit.WorldType; import org.bukkit.entity.Entity; import org.bukkit.inventory.ItemStack; +import org.bukkit.potion.PotionEffect; +import org.bukkit.potion.PotionEffectType; import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolManager; +import com.comphenix.protocol.injector.PacketConstructor; +import com.comphenix.protocol.injector.PacketConstructor.Unwrapper; import com.comphenix.protocol.reflect.EquivalentConverter; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.utility.MinecraftReflection; import com.comphenix.protocol.wrappers.nbt.NbtBase; import com.comphenix.protocol.wrappers.nbt.NbtCompound; import com.comphenix.protocol.wrappers.nbt.NbtFactory; import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; /** @@ -55,11 +62,16 @@ public class BukkitConverters { // The static maps private static Map, EquivalentConverter> specificConverters; private static Map, EquivalentConverter> genericConverters; + private static List unwrappers; // Used to access the world type private static Method worldTypeName; private static Method worldTypeGetType; + // Used for potion effect conversion + private static volatile Constructor mobEffectConstructor; + private static volatile StructureModifier mobEffectModifier; + static { try { MinecraftReflection.getWorldTypeClass(); @@ -449,6 +461,57 @@ public class BukkitConverters { } /** + * Retrieve the converter used to convert between a PotionEffect and the equivalent NMS Mobeffect. + * @return The potion effect converter. + */ + public static EquivalentConverter getPotionEffectConverter() { + return new IgnoreNullConverter() { + @Override + protected Object getGenericValue(Class genericType, PotionEffect specific) { + // Locate the constructor + if (mobEffectConstructor == null) { + try { + mobEffectConstructor = MinecraftReflection.getMobEffectClass(). + getConstructor(int.class, int.class, int.class, boolean.class); + } catch (Exception e) { + throw new RuntimeException("Cannot find mob effect constructor (int, int, int, boolean).", e); + } + } + + // Create the generic value + try { + return mobEffectConstructor.newInstance( + specific.getType().getId(), specific.getDuration(), + specific.getAmplifier(), specific.isAmbient()); + } catch (Exception e) { + throw new RuntimeException("Cannot construct MobEffect.", e); + } + } + + @Override + protected PotionEffect getSpecificValue(Object generic) { + if (mobEffectModifier == null) { + mobEffectModifier = new StructureModifier(MinecraftReflection.getMobEffectClass(), false); + } + StructureModifier ints = mobEffectModifier.withTarget(generic).withType(int.class); + StructureModifier bools = mobEffectModifier.withTarget(generic).withType(boolean.class); + + return new PotionEffect( + PotionEffectType.getById(ints.read(0)), /* effectId */ + ints.read(1), /* duration */ + ints.read(2), /* amplification */ + bools.read(1) /* ambient */ + ); + } + + @Override + public Class getSpecificType() { + return PotionEffect.class; + } + }; + } + + /** * Wraps a given equivalent converter in NULL checks, ensuring that such values are ignored. * @param delegate - the underlying equivalent converter. * @return A equivalent converter that ignores NULL values. @@ -473,6 +536,31 @@ public class BukkitConverters { }; } + /** + * Retrieve an equivalent unwrapper for the converter. + * @param nativeType - the native NMS type the converter produces. + * @param converter - the converter. + * @return The equivalent unwrapper. + */ + public static Unwrapper asUnwrapper(final Class nativeType, final EquivalentConverter converter) { + return new Unwrapper() { + @SuppressWarnings("rawtypes") + @Override + public Object unwrapItem(Object wrappedObject) { + Class type = PacketConstructor.getClass(wrappedObject); + + // Ensure the type is correct before we test + if (converter.getSpecificType().isAssignableFrom(type)) { + if (wrappedObject instanceof Class) + return nativeType; + else + return converter.getGeneric((Class) nativeType, wrappedObject); + } + return null; + } + }; + } + /** * Retrieve every converter that is associated with a specific class. * @return Every converter with a unique specific class. @@ -487,7 +575,8 @@ public class BukkitConverters { put(ItemStack.class, (EquivalentConverter) getItemStackConverter()). put(NbtBase.class, (EquivalentConverter) getNbtConverter()). put(NbtCompound.class, (EquivalentConverter) getNbtConverter()). - put(WrappedWatchableObject.class, (EquivalentConverter) getWatchableObjectConverter()); + put(WrappedWatchableObject.class, (EquivalentConverter) getWatchableObjectConverter()). + put(PotionEffect.class, (EquivalentConverter) getPotionEffectConverter()); if (hasWorldType) builder.put(WorldType.class, (EquivalentConverter) getWorldTypeConverter()); @@ -512,7 +601,8 @@ public class BukkitConverters { put(MinecraftReflection.getItemStackClass(), (EquivalentConverter) getItemStackConverter()). put(MinecraftReflection.getNBTBaseClass(), (EquivalentConverter) getNbtConverter()). put(MinecraftReflection.getNBTCompoundClass(), (EquivalentConverter) getNbtConverter()). - put(MinecraftReflection.getWatchableObjectClass(), (EquivalentConverter) getWatchableObjectConverter()); + put(MinecraftReflection.getWatchableObjectClass(), (EquivalentConverter) getWatchableObjectConverter()). + put(MinecraftReflection.getMobEffectClass(), (EquivalentConverter) getPotionEffectConverter()); if (hasWorldType) builder.put(MinecraftReflection.getWorldTypeClass(), (EquivalentConverter) getWorldTypeConverter()); @@ -522,4 +612,20 @@ public class BukkitConverters { } return genericConverters; } + + /** + * Retrieve every NMS <-> Bukkit converter as unwrappers. + * @return Every unwrapper. + */ + public static List getUnwrappers() { + if (unwrappers == null) { + ImmutableList.Builder builder = ImmutableList.builder(); + + for (Map.Entry, EquivalentConverter> entry : getConvertersForGeneric().entrySet()) { + builder.add(asUnwrapper(entry.getKey(), entry.getValue())); + } + unwrappers = builder.build(); + } + return unwrappers; + } } diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java index c6e67b0c..40f54610 100644 --- a/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java +++ b/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java @@ -35,6 +35,8 @@ import org.bukkit.craftbukkit.v1_6_R2.inventory.CraftItemFactory; import org.bukkit.Material; import org.bukkit.WorldType; import org.bukkit.inventory.ItemStack; +import org.bukkit.potion.PotionEffect; +import org.bukkit.potion.PotionEffectType; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -42,6 +44,7 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import com.comphenix.protocol.BukkitInitialization; import com.comphenix.protocol.Packets; +import com.comphenix.protocol.injector.PacketConstructor; import com.comphenix.protocol.reflect.EquivalentConverter; import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.utility.MinecraftReflection; @@ -345,7 +348,20 @@ public class PacketContainerTest { ToStringBuilder.reflectionToString(clonedSnapshot, ToStringStyle.SHORT_PREFIX_STYLE)); } - + @Test + public void testPotionEffect() { + PotionEffect effect = new PotionEffect(PotionEffectType.FIRE_RESISTANCE, 20 * 60, 1); + + // The constructor we want to call + PacketConstructor creator = PacketConstructor.DEFAULT.withPacket( + Packets.Server.MOB_EFFECT, new Class[] { int.class, PotionEffect.class }); + PacketContainer packet = creator.createPacket(1, effect); + + assertEquals(1, (int) packet.getIntegers().read(0)); + assertEquals(effect.getType().getId(), (byte) packet.getBytes().read(0)); + assertEquals(effect.getAmplifier(), (byte) packet.getBytes().read(1)); + assertEquals(effect.getDuration(), (short) packet.getShorts().read(0)); + } @Test public void testDeepClone() {