Fixed a bug causing the entity modifier to always lookup the same world

The entity modifier would try to cache each equivalent converter by its
specific type - i.e. whether or not it converts entities or watchable
collections - but this isn't enough to distinguish entity modifiers 
as they store a hidden reference to the world the entity is supposed 
to be occupying. 

Thus, after the first entity modifier is used for "world", it would 
be subsequently cached for every world ("world_nether", "world_end", 
etc.). The end result was that entity modifiers would only work for 
one world.

This patch fixes this problem by adding a proper equals() method to
the entity equivalent converter class, along with all the other 
equvialent converter classes, in case it should prove to become useful
in the future.
This commit is contained in:
Kristian S. Stangeland 2013-01-27 15:35:39 +01:00
parent 4f061e3464
commit eb996e73d8
2 changed files with 132 additions and 55 deletions

View File

@ -28,6 +28,7 @@ import java.util.concurrent.ConcurrentHashMap;
import com.comphenix.protocol.reflect.compiler.BackgroundCompiler; import com.comphenix.protocol.reflect.compiler.BackgroundCompiler;
import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.reflect.instances.DefaultInstances;
import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
/** /**
@ -394,23 +395,13 @@ public class StructureModifier<TField> {
result = result.withTarget(target); result = result.withTarget(target);
// And the converter, if it's needed // And the converter, if it's needed
if (!sameConverter(result.converter, converter)) { if (!Objects.equal(result.converter, converter)) {
result = result.withConverter(converter); result = result.withConverter(converter);
} }
return result; return result;
} }
private boolean sameConverter(EquivalentConverter<?> a, EquivalentConverter<?> b) {
// Compare the converter types
if (a == null)
return b == null;
else if (b == null)
return false;
else
return a.getSpecificType().equals(b.getSpecificType());
}
/** /**
* Retrieves the common type of each field. * Retrieves the common type of each field.
* @return Common type of each field. * @return Common type of each field.

View File

@ -36,6 +36,7 @@ import com.comphenix.protocol.reflect.instances.DefaultInstances;
import com.comphenix.protocol.utility.MinecraftReflection; import com.comphenix.protocol.utility.MinecraftReflection;
import com.comphenix.protocol.wrappers.nbt.NbtBase; import com.comphenix.protocol.wrappers.nbt.NbtBase;
import com.comphenix.protocol.wrappers.nbt.NbtFactory; import com.comphenix.protocol.wrappers.nbt.NbtFactory;
import com.google.common.base.Objects;
/** /**
* Contains several useful equivalent converters for normal Bukkit types. * Contains several useful equivalent converters for normal Bukkit types.
@ -58,12 +59,104 @@ public class BukkitConverters {
} }
} }
/**
* Represents a typical equivalence converter.
*
* @author Kristian
* @param <T> - type that can be converted.
*/
private static abstract class IgnoreNullConverter<TType> implements EquivalentConverter<TType> {
public final Object getGeneric(Class<?> genericType, TType specific) {
if (specific != null)
return getGenericValue(genericType, specific);
else
return null;
}
/**
* Retrieve a copy of the actual generic value.
* @param genericType - generic type.
* @param specific - the specific type-
* @return A copy of the specific type.
*/
protected abstract Object getGenericValue(Class<?> genericType, TType specific);
@Override
public final TType getSpecific(Object generic) {
if (generic != null)
return getSpecificValue(generic);
else
return null;
}
/**
* Retrieve a copy of the specific type using an instance of the generic type.
* @param generic - generic type.
* @return A copy of the specific type.
*/
protected abstract TType getSpecificValue(Object generic);
@Override
public boolean equals(Object obj) {
// Very short
if (this == obj)
return true;
if (obj == null)
return false;
// See if they're equivalent
if (obj instanceof EquivalentConverter) {
@SuppressWarnings("rawtypes")
EquivalentConverter other = (EquivalentConverter) obj;
return Objects.equal(this.getSpecificType(), other.getSpecificType());
}
return false;
}
}
/**
* Represents a converter that is only valid in a given world.
*
* @author Kristian
* @param <TType> - instance types it converts.
*/
private static abstract class WorldSpecificConverter<TType> extends IgnoreNullConverter<TType> {
protected World world;
/**
* Initialize a new world-specificn converter.
* @param world - the given world.
*/
public WorldSpecificConverter(World world) {
super();
this.world = world;
}
@Override
public boolean equals(Object obj) {
// More shortcuts
if (obj == this)
return true;
if (obj == null)
return false;
// Add another constraint
if (obj instanceof WorldSpecificConverter && super.equals(obj)) {
@SuppressWarnings("rawtypes")
WorldSpecificConverter other = (WorldSpecificConverter) obj;
return Objects.equal(world, other.world);
}
return false;
}
}
public static <T> EquivalentConverter<List<T>> getListConverter(final Class<?> genericItemType, final EquivalentConverter<T> itemConverter) { public static <T> EquivalentConverter<List<T>> getListConverter(final Class<?> genericItemType, final EquivalentConverter<T> itemConverter) {
// Convert to and from the wrapper // Convert to and from the wrapper
return getIgnoreNull(new EquivalentConverter<List<T>>() { return new IgnoreNullConverter<List<T>>() {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Override @Override
public List<T> getSpecific(Object generic) { protected List<T> getSpecificValue(Object generic) {
if (generic instanceof Collection) { if (generic instanceof Collection) {
List<T> items = new ArrayList<T>(); List<T> items = new ArrayList<T>();
@ -83,7 +176,7 @@ public class BukkitConverters {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Override @Override
public Object getGeneric(Class<?> genericType, List<T> specific) { protected Object getGenericValue(Class<?> genericType, List<T> specific) {
Collection<Object> newContainer = (Collection<Object>) DefaultInstances.DEFAULT.getDefault(genericType); Collection<Object> newContainer = (Collection<Object>) DefaultInstances.DEFAULT.getDefault(genericType);
// Convert each object // Convert each object
@ -105,8 +198,7 @@ public class BukkitConverters {
Class<?> dummy = List.class; Class<?> dummy = List.class;
return (Class<List<T>>) dummy; return (Class<List<T>>) dummy;
} }
} };
);
} }
/** /**
@ -114,13 +206,13 @@ public class BukkitConverters {
* @return A watchable object converter. * @return A watchable object converter.
*/ */
public static EquivalentConverter<WrappedWatchableObject> getWatchableObjectConverter() { public static EquivalentConverter<WrappedWatchableObject> getWatchableObjectConverter() {
return getIgnoreNull(new EquivalentConverter<WrappedWatchableObject>() { return new IgnoreNullConverter<WrappedWatchableObject>() {
@Override @Override
public Object getGeneric(Class<?> genericType, WrappedWatchableObject specific) { protected Object getGenericValue(Class<?> genericType, WrappedWatchableObject specific) {
return specific.getHandle(); return specific.getHandle();
} }
public WrappedWatchableObject getSpecific(Object generic) { protected WrappedWatchableObject getSpecificValue(Object generic) {
if (MinecraftReflection.isWatchableObject(generic)) if (MinecraftReflection.isWatchableObject(generic))
return new WrappedWatchableObject(generic); return new WrappedWatchableObject(generic);
else if (generic instanceof WrappedWatchableObject) else if (generic instanceof WrappedWatchableObject)
@ -133,7 +225,7 @@ public class BukkitConverters {
public Class<WrappedWatchableObject> getSpecificType() { public Class<WrappedWatchableObject> getSpecificType() {
return WrappedWatchableObject.class; return WrappedWatchableObject.class;
} }
}); };
} }
/** /**
@ -141,14 +233,14 @@ public class BukkitConverters {
* @return A DataWatcher converter. * @return A DataWatcher converter.
*/ */
public static EquivalentConverter<WrappedDataWatcher> getDataWatcherConverter() { public static EquivalentConverter<WrappedDataWatcher> getDataWatcherConverter() {
return getIgnoreNull(new EquivalentConverter<WrappedDataWatcher>() { return new IgnoreNullConverter<WrappedDataWatcher>() {
@Override @Override
public Object getGeneric(Class<?> genericType, WrappedDataWatcher specific) { protected Object getGenericValue(Class<?> genericType, WrappedDataWatcher specific) {
return specific.getHandle(); return specific.getHandle();
} }
@Override @Override
public WrappedDataWatcher getSpecific(Object generic) { protected WrappedDataWatcher getSpecificValue(Object generic) {
if (MinecraftReflection.isDataWatcher(generic)) if (MinecraftReflection.isDataWatcher(generic))
return new WrappedDataWatcher(generic); return new WrappedDataWatcher(generic);
else if (generic instanceof WrappedDataWatcher) else if (generic instanceof WrappedDataWatcher)
@ -161,7 +253,7 @@ public class BukkitConverters {
public Class<WrappedDataWatcher> getSpecificType() { public Class<WrappedDataWatcher> getSpecificType() {
return WrappedDataWatcher.class; return WrappedDataWatcher.class;
} }
}); };
} }
/** /**
@ -173,9 +265,9 @@ public class BukkitConverters {
if (!hasWorldType) if (!hasWorldType)
return null; return null;
return getIgnoreNull(new EquivalentConverter<WorldType>() { return new IgnoreNullConverter<WorldType>() {
@Override @Override
public Object getGeneric(Class<?> genericType, WorldType specific) { protected Object getGenericValue(Class<?> genericType, WorldType specific) {
try { try {
if (worldTypeGetType == null) if (worldTypeGetType == null)
worldTypeGetType = MinecraftReflection.getWorldTypeClass().getMethod("getType", String.class); worldTypeGetType = MinecraftReflection.getWorldTypeClass().getMethod("getType", String.class);
@ -189,7 +281,7 @@ public class BukkitConverters {
} }
@Override @Override
public WorldType getSpecific(Object generic) { protected WorldType getSpecificValue(Object generic) {
try { try {
if (worldTypeName == null) if (worldTypeName == null)
worldTypeName = MinecraftReflection.getWorldTypeClass().getMethod("name"); worldTypeName = MinecraftReflection.getWorldTypeClass().getMethod("name");
@ -207,7 +299,7 @@ public class BukkitConverters {
public Class<WorldType> getSpecificType() { public Class<WorldType> getSpecificType() {
return WorldType.class; return WorldType.class;
} }
}); };
} }
/** /**
@ -215,14 +307,14 @@ public class BukkitConverters {
* @return An equivalent converter for NBT. * @return An equivalent converter for NBT.
*/ */
public static EquivalentConverter<NbtBase<?>> getNbtConverter() { public static EquivalentConverter<NbtBase<?>> getNbtConverter() {
return getIgnoreNull(new EquivalentConverter<NbtBase<?>>() { return new IgnoreNullConverter<NbtBase<?>>() {
@Override @Override
public Object getGeneric(Class<?> genericType, NbtBase<?> specific) { protected Object getGenericValue(Class<?> genericType, NbtBase<?> specific) {
return NbtFactory.fromBase(specific).getHandle(); return NbtFactory.fromBase(specific).getHandle();
} }
@Override @Override
public NbtBase<?> getSpecific(Object generic) { protected NbtBase<?> getSpecificValue(Object generic) {
return NbtFactory.fromNMS(generic); return NbtFactory.fromNMS(generic);
} }
@ -233,7 +325,7 @@ public class BukkitConverters {
Class<?> dummy = NbtBase.class; Class<?> dummy = NbtBase.class;
return (Class<NbtBase<?>>) dummy; return (Class<NbtBase<?>>) dummy;
} }
}); };
} }
/** /**
@ -242,27 +334,25 @@ public class BukkitConverters {
* @return A converter between the underlying NMS entity and Bukkit's wrapper. * @return A converter between the underlying NMS entity and Bukkit's wrapper.
*/ */
public static EquivalentConverter<Entity> getEntityConverter(World world) { public static EquivalentConverter<Entity> getEntityConverter(World world) {
final World container = world;
final WeakReference<ProtocolManager> managerRef = final WeakReference<ProtocolManager> managerRef =
new WeakReference<ProtocolManager>(ProtocolLibrary.getProtocolManager()); new WeakReference<ProtocolManager>(ProtocolLibrary.getProtocolManager());
return getIgnoreNull(new EquivalentConverter<Entity>() { return new WorldSpecificConverter<Entity>(world) {
@Override @Override
public Object getGeneric(Class<?> genericType, Entity specific) { public Object getGenericValue(Class<?> genericType, Entity specific) {
// Simple enough // Simple enough
return specific.getEntityId(); return specific.getEntityId();
} }
@Override @Override
public Entity getSpecific(Object generic) { public Entity getSpecificValue(Object generic) {
try { try {
Integer id = (Integer) generic; Integer id = (Integer) generic;
ProtocolManager manager = managerRef.get(); ProtocolManager manager = managerRef.get();
// Use the // Use the entity ID to get a reference to the entity
if (id != null && manager != null) { if (id != null && manager != null) {
return manager.getEntityFromID(container, id); return manager.getEntityFromID(world, id);
} else { } else {
return null; return null;
} }
@ -276,7 +366,7 @@ public class BukkitConverters {
public Class<Entity> getSpecificType() { public Class<Entity> getSpecificType() {
return Entity.class; return Entity.class;
} }
}); };
} }
/** /**
@ -284,13 +374,14 @@ public class BukkitConverters {
* @return Item stack converter. * @return Item stack converter.
*/ */
public static EquivalentConverter<ItemStack> getItemStackConverter() { public static EquivalentConverter<ItemStack> getItemStackConverter() {
return getIgnoreNull(new EquivalentConverter<ItemStack>() { return new IgnoreNullConverter<ItemStack>() {
public Object getGeneric(Class<?> genericType, ItemStack specific) { @Override
protected Object getGenericValue(Class<?> genericType, ItemStack specific) {
return MinecraftReflection.getMinecraftItemStack(specific); return MinecraftReflection.getMinecraftItemStack(specific);
} }
@Override @Override
public ItemStack getSpecific(Object generic) { protected ItemStack getSpecificValue(Object generic) {
return MinecraftReflection.getBukkitItemStack(generic); return MinecraftReflection.getBukkitItemStack(generic);
} }
@ -298,7 +389,7 @@ public class BukkitConverters {
public Class<ItemStack> getSpecificType() { public Class<ItemStack> getSpecificType() {
return ItemStack.class; return ItemStack.class;
} }
}); };
} }
/** /**
@ -308,20 +399,15 @@ public class BukkitConverters {
*/ */
public static <TType> EquivalentConverter<TType> getIgnoreNull(final EquivalentConverter<TType> delegate) { public static <TType> EquivalentConverter<TType> getIgnoreNull(final EquivalentConverter<TType> delegate) {
// Automatically wrap all parameters to the delegate with a NULL check // Automatically wrap all parameters to the delegate with a NULL check
return new EquivalentConverter<TType>() { return new IgnoreNullConverter<TType>() {
public Object getGeneric(Class<?> genericType, TType specific) { @Override
if (specific != null) public Object getGenericValue(Class<?> genericType, TType specific) {
return delegate.getGeneric(genericType, specific); return delegate.getGeneric(genericType, specific);
else
return null;
} }
@Override @Override
public TType getSpecific(Object generic) { public TType getSpecificValue(Object generic) {
if (generic != null) return delegate.getSpecific(generic);
return delegate.getSpecific(generic);
else
return null;
} }
@Override @Override