Upstream/ProtocolLib
1
0
Fork 0
mirror of https://github.com/dmulloy2/ProtocolLib.git synced 2024-11-27 21:26:17 +01:00
Code Issues Projects Releases Wiki Activity

Fix some issues with item equality

Browse Source 
Seems to be an internal Bukkit bug, hopefully nothing more serious
This commit is contained in:
Dan Mulloy 2018-07-24 17:05:30 -04:00
parent 976e4b4217
commit 3ef537242f
8 changed files with 201 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
modules/API/src/main/java/com/comphenix/protocol/ProtocolLogger.java
View File

@ -31,11 +31,11 @@ public class ProtocolLogger {
ProtocolLogger.plugin = plugin;
}
private static boolean isDebugEnabled() {
public static boolean isDebugEnabled() {
try {
return plugin.getConfig().getBoolean("global.debug", false);
} catch (Throwable ex) { // Maybe we're testing or something
return false;
} catch (Throwable ex) { // Enable in testing environments
return true;
}
}

72
modules/API/src/main/java/com/comphenix/protocol/wrappers/EnumWrappers.java
View File

@ -3,17 +3,17 @@ package com.comphenix.protocol.wrappers;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;
import org.bukkit.GameMode;
import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.PacketType.Protocol;
import com.comphenix.protocol.ProtocolLogger;
import com.comphenix.protocol.reflect.EquivalentConverter;
import com.comphenix.protocol.reflect.FuzzyReflection;
import com.comphenix.protocol.utility.MinecraftReflection;
import com.google.common.collect.Maps;
import org.bukkit.GameMode;
/**
* Represents a generic enum converter.
* @author Kristian
@ -22,26 +22,26 @@ public abstract class EnumWrappers {
public enum ClientCommand {
PERFORM_RESPAWN,
REQUEST_STATS,
OPEN_INVENTORY_ACHIEVEMENT;
OPEN_INVENTORY_ACHIEVEMENT
}
public enum ChatVisibility {
FULL,
SYSTEM,
HIDDEN;
HIDDEN
}
public enum Difficulty {
PEACEFUL,
EASY,
NORMAL,
HARD;
HARD
}
public enum EntityUseAction {
INTERACT,
ATTACK,
INTERACT_AT;
INTERACT_AT
}
/**
@ -111,7 +111,7 @@ public abstract class EnumWrappers {
SUCCESSFULLY_LOADED,
DECLINED,
FAILED_DOWNLOAD,
ACCEPTED;
ACCEPTED
}
public enum PlayerInfoAction {
@ -119,15 +119,16 @@ public abstract class EnumWrappers {
UPDATE_GAME_MODE,
UPDATE_LATENCY,
UPDATE_DISPLAY_NAME,
REMOVE_PLAYER;
REMOVE_PLAYER
}
public enum TitleAction {
TITLE,
SUBTITLE,
ACTIONBAR,
TIMES,
CLEAR,
RESET;
RESET
}
public enum WorldBorderAction {
@ -136,13 +137,13 @@ public abstract class EnumWrappers {
SET_CENTER,
INITIALIZE,
SET_WARNING_TIME,
SET_WARNING_BLOCKS;
SET_WARNING_BLOCKS
}
public enum CombatEventType {
ENTER_COMBAT,
END_COMBAT,
ENTITY_DIED;
ENTITY_DIED
}
public enum PlayerDigType {
@ -152,7 +153,7 @@ public abstract class EnumWrappers {
DROP_ALL_ITEMS,
DROP_ITEM,
RELEASE_USE_ITEM,
SWAP_HELD_ITEMS;
SWAP_HELD_ITEMS
}
public enum PlayerAction {
@ -164,12 +165,12 @@ public abstract class EnumWrappers {
START_RIDING_JUMP,
STOP_RIDING_JUMP,
OPEN_INVENTORY,
START_FALL_FLYING;
START_FALL_FLYING
}
public enum ScoreboardAction {
CHANGE,
REMOVE;
REMOVE
}
public enum Particle {
@ -227,8 +228,8 @@ public abstract class EnumWrappers {
private static final Map<Integer, Particle> BY_ID;
static {
BY_ID = new HashMap<Integer, Particle>();
BY_NAME = new HashMap<String, Particle>();
BY_ID = new HashMap<>();
BY_NAME = new HashMap<>();
for (Particle particle : values()) {
BY_NAME.put(particle.getName().toLowerCase(Locale.ENGLISH), particle);
@ -241,11 +242,11 @@ public abstract class EnumWrappers {
private final boolean longDistance;
private final int dataLength;
private Particle(String name, int id, boolean longDistance) {
Particle(String name, int id, boolean longDistance) {
this(name, id, longDistance, 0);
}
private Particle(String name, int id, boolean longDistance, int dataLength) {
Particle(String name, int id, boolean longDistance, int dataLength) {
this.name = name;
this.id = id;
this.longDistance = longDistance;
@ -298,7 +299,8 @@ public abstract class EnumWrappers {
}
private final String key;
private SoundCategory(String key) {
SoundCategory(String key) {
this.key = key;
}
@ -317,12 +319,12 @@ public abstract class EnumWrappers {
FEET,
LEGS,
CHEST,
HEAD;
HEAD
}
public enum Hand {
MAIN_HAND,
OFF_HAND;
OFF_HAND
}
public enum Direction {
@ -331,22 +333,16 @@ public abstract class EnumWrappers {
NORTH,
SOUTH,
WEST,
EAST;
EAST
}
public enum ChatType {
CHAT(0),
SYSTEM(1),
GAME_INFO(2);
private byte id;
ChatType(int id) {
this.id = (byte) id;
}
CHAT,
SYSTEM,
GAME_INFO;
public byte getId() {
return id;
return (byte) ordinal();
}
}
@ -435,6 +431,8 @@ public abstract class EnumWrappers {
if (nativeClass != null) {
FROM_NATIVE.put(nativeClass, converter);
FROM_WRAPPER.put(wrapperClass, converter);
} else if (ProtocolLogger.isDebugEnabled()) {
new ClassNotFoundException(wrapperClass.getSimpleName()).printStackTrace();
}
}
@ -448,7 +446,11 @@ public abstract class EnumWrappers {
try {
return FuzzyReflection.fromClass(clazz, true).getFieldListByType(Enum.class).get(index).getType();
} catch (Throwable ex) {
return null; // Unsupported in this version
if (ProtocolLogger.isDebugEnabled()) {
ex.printStackTrace();
}
return null;
}
}

81
modules/API/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java
View File

@ -1,6 +1,6 @@
/**
* ProtocolLib - Bukkit server library that allows access to the Minecraft protocol.
* Copyright (C) 2016 dmulloy2
* Copyright (C) 2018 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
@ -39,8 +39,7 @@ import org.bukkit.entity.Entity;
import org.bukkit.inventory.ItemStack;
/**
* Represents a DataWatcher in 1.8 thru 1.10
*
* Represents a DataWatcher
* @author dmulloy2
*/
public class WrappedDataWatcher extends AbstractWrapper implements Iterable<WrappedWatchableObject> {
@ -74,7 +73,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
}
/**
* Constructs a new DataWatcher using a fake lightning entity. The
* Constructs a new DataWatcher using a fake egg entity. The
* resulting DataWatcher will not have any keys or values and new ones will
* have to be added using watcher objects.
*/
@ -94,7 +93,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
}
/**
* Constructs a new DataWatcher using a fake lightning entity and a given
* Constructs a new DataWatcher using a fake egg entity and a given
* list of watchable objects.
*
* @param objects The list of objects
@ -126,7 +125,8 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
return fakeEntity;
}
// We can create a fake lightning strike without it affecting anything
// We can create a fake egg without it affecting anything
// Mojang added difficulty to lightning strikes, so this'll have to do
if (eggConstructor == null) {
eggConstructor = Accessors.getConstructorAccessor(MinecraftReflection.getMinecraftClass("EntityEgg"),
MinecraftReflection.getNmsWorldClass(), double.class, double.class, double.class);
@ -140,15 +140,16 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
@SuppressWarnings("unchecked")
private Map<Integer, Object> getMap() {
if (MAP_FIELD == null) {
FuzzyReflection fuzzy = FuzzyReflection.fromClass(handleType, true);
MAP_FIELD = Accessors.getFieldAccessor(fuzzy.getField(FuzzyFieldContract.newBuilder()
.banModifier(Modifier.STATIC)
.typeDerivedOf(Map.class)
.build()));
}
if (MAP_FIELD == null) {
throw new FieldAccessException("Could not find index <-> Item map.");
try {
FuzzyReflection fuzzy = FuzzyReflection.fromClass(handleType, true);
MAP_FIELD = Accessors.getFieldAccessor(fuzzy.getField(FuzzyFieldContract
.newBuilder()
.banModifier(Modifier.STATIC)
.typeDerivedOf(Map.class)
.build()));
} catch (IllegalArgumentException ex) {
throw new FieldAccessException("Failed to find watchable object map");
}
}
return (Map<Integer, Object>) MAP_FIELD.get(handle);
@ -371,7 +372,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
try {
Object value = GETTER.invoke(handle, object.getHandle());
return WrappedWatchableObject.getWrapped(value);
} catch (RuntimeException ex) {
} catch (Exception ex) {
// Nothing exists at this index
return null;
}
@ -677,7 +678,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
*/
public WrappedDataWatcherObject(Object handle) {
this.handle = handle;
this.modifier = new StructureModifier<Object>(HANDLE_TYPE).withTarget(handle);
this.modifier = new StructureModifier<>(HANDLE_TYPE).withTarget(handle);
}
/**
@ -692,18 +693,31 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
static WrappedDataWatcherObject fromIndex(int index) {
if (MinecraftReflection.watcherObjectExists()) {
return new WrappedDataWatcherObject(index, null);
return new WrappedDataWatcherObject(newHandle(index));
} else {
return new DummyWatcherObject(index);
}
}
private static Object newHandle(int index, Serializer serializer) {
private static Object newHandle(int index) {
Validate.isTrue(index >= 0, "index cannot be negative!");
if (constructor == null) {
constructor = Accessors.getConstructorAccessor(HANDLE_TYPE.getConstructors()[0]);
}
Object handle = serializer != null ? serializer.getHandle() : null;
return constructor.invoke(index, null);
}
private static Object newHandle(int index, Serializer serializer) {
Validate.isTrue(index >= 0, "index cannot be negative!");
Validate.notNull(serializer, "serializer cannot be null!");
if (constructor == null) {
constructor = Accessors.getConstructorAccessor(HANDLE_TYPE.getConstructors()[0]);
}
Object handle = serializer.getHandle();
return constructor.invoke(index, handle);
}
@ -914,7 +928,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
* @return The serializer, or null if none exists
*/
public static Serializer get(Class<?> clazz) {
Validate.notNull("Class cannot be null!");
Validate.notNull(clazz,"Class cannot be null!");
initialize();
for (Serializer serializer : REGISTRY) {
@ -923,7 +937,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
}
}
return null;
throw new IllegalArgumentException("No serializer found for " + clazz);
}
/**
@ -940,6 +954,8 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
Validate.notNull(clazz, "Class cannot be null!");
initialize();
Validate.notEmpty(REGISTRY, "Registry has no elements!");
for (Serializer serializer : REGISTRY) {
if (serializer.getType().equals(clazz)
&& serializer.isOptional() == optional) {
@ -947,7 +963,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
}
}
return null;
throw new IllegalArgumentException("No serializer found for " + (optional ? "Optional<" + clazz + ">" : clazz));
}
/**
@ -956,7 +972,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
* @return The serializer, or null if none exists
*/
public static Serializer fromHandle(Object handle) {
Validate.notNull("handle cannot be null!");
Validate.notNull(handle, "handle cannot be null!");
initialize();
for (Serializer serializer : REGISTRY) {
@ -984,7 +1000,7 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
Type[] args = type.getActualTypeArguments();
Type arg = args[0];
Class<?> innerClass = null;
Class<?> innerClass;
boolean optional = false;
if (arg instanceof Class<?>) {
@ -1004,6 +1020,10 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
throw new IllegalStateException("Failed to read field " + candidate);
}
if (serializer == null) {
throw new RuntimeException("Failed to read serializer: " + candidate.getName());
}
REGISTRY.add(new Serializer(innerClass, serializer, optional));
}
}
@ -1016,7 +1036,16 @@ public class WrappedDataWatcher extends AbstractWrapper implements Iterable<Wrap
* @return The serializer
*/
public static Serializer getChatComponentSerializer() {
return get(MinecraftReflection.getIChatBaseComponentClass());
return getChatComponentSerializer(false);
}
/**
* Gets the serializer for IChatBaseComponents
* @param optional If true, objects <b>must</b> be wrapped in an {@link Optional}
* @return The serializer
*/
public static Serializer getChatComponentSerializer(boolean optional) {
return get(MinecraftReflection.getIChatBaseComponentClass(), optional);
}
/**

38
modules/ProtocolLib/src/test/java/com/comphenix/protocol/events/PacketContainerTest.java
View File

@ -18,10 +18,7 @@ package com.comphenix.protocol.events;
import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
import java.util.*;
import com.comphenix.protocol.BukkitInitialization;
import com.comphenix.protocol.PacketType;
@ -57,6 +54,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import static com.comphenix.protocol.utility.TestUtils.*;
import static org.junit.Assert.*;
// Ensure that the CraftItemFactory is mockable
@ -218,17 +216,7 @@ public class PacketContainerTest {
// Read back array
List<ItemStack> comparison = itemAccess.read(0);
assertEquals(items, comparison);
}
private boolean equivalentItem(ItemStack first, ItemStack second) {
if (first == null) {
return second == null;
} else if (second == null) {
return false;
} else {
return first.getType().equals(second.getType());
}
assertItemCollectionsEqual(items, comparison);
}
@Test
@ -270,14 +258,11 @@ public class PacketContainerTest {
PacketContainer mobSpawnPacket = new PacketContainer(PacketType.Play.Server.SPAWN_ENTITY_LIVING);
StructureModifier<WrappedDataWatcher> watcherAccessor = mobSpawnPacket.getDataWatcherModifier();
Entity entity = new EntityEgg(null, 0, 0, 0);
DataWatcher watcher = entity.getDataWatcher();
WrappedDataWatcher dataWatcher = new WrappedDataWatcher(watcher);
dataWatcher.setObject(1, (byte) 1);
dataWatcher.setObject(2, 301);
dataWatcher.setObject(3, true);
dataWatcher.setObject(4, "Lorem");
WrappedDataWatcher dataWatcher = new WrappedDataWatcher();
dataWatcher.setObject(new WrappedDataWatcherObject(1, Registry.get(Byte.class)), (byte) 1);
dataWatcher.setObject(new WrappedDataWatcherObject(2, Registry.get(String.class)), "Lorem");
dataWatcher.setObject(new WrappedDataWatcherObject(3, Registry.get(Boolean.class)), true);
dataWatcher.setObject(new WrappedDataWatcherObject(4, Registry.getUUIDSerializer(true)), Optional.of(UUID.randomUUID()));
assertNull(watcherAccessor.read(0));
@ -463,7 +448,7 @@ public class PacketContainerTest {
PacketContainer container = new PacketContainer(PacketType.Play.Server.NAMED_SOUND_EFFECT);
container.getSoundCategories().write(0, SoundCategory.PLAYERS);
assertEquals(container.getSoundCategories().read(0), SoundCategory.PLAYERS);
assertEquals(SoundCategory.PLAYERS, container.getSoundCategories().read(0));
}
@Test
@ -580,6 +565,11 @@ public class PacketContainerTest {
}
}
if (a instanceof ItemStack || b instanceof ItemStack) {
assertItemsEqual((ItemStack) a, (ItemStack) b);
return;
}
if (a == null || b == null) {
if (a == null && b == null) {
return;

3
modules/ProtocolLib/src/test/java/com/comphenix/protocol/utility/MinecraftReflectionTest.java
View File

@ -1,5 +1,6 @@
package com.comphenix.protocol.utility;
import static com.comphenix.protocol.utility.TestUtils.assertItemsEqual;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.mock;
@ -138,7 +139,7 @@ public class MinecraftReflectionTest {
public void testItemStacks() {
ItemStack stack = new ItemStack(Material.GOLDEN_SWORD);
Object nmsStack = MinecraftReflection.getMinecraftItemStack(stack);
assertEquals(stack, MinecraftReflection.getBukkitItemStack(nmsStack));
assertItemsEqual(stack, MinecraftReflection.getBukkitItemStack(nmsStack));
// The NMS handle for CraftItemStack is null with Material.AIR, make sure it is handled correctly
assertNotNull(MinecraftReflection.getMinecraftItemStack(CraftItemStack.asCraftCopy(new ItemStack(Material.AIR))));

5
modules/ProtocolLib/src/test/java/com/comphenix/protocol/utility/StreamSerializerTest.java
View File

@ -17,6 +17,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import static com.comphenix.protocol.utility.TestUtils.assertItemsEqual;
import static org.junit.Assert.assertEquals;
@RunWith(org.powermock.modules.junit4.PowerMockRunner.class)
@ -75,7 +76,7 @@ public class StreamSerializerTest {
String serialized = serializer.serializeItemStack(initial);
ItemStack deserialized = serializer.deserializeItemStack(serialized);
assertEquals(initial, deserialized);
assertItemsEqual(initial, deserialized);
}
@Test
@ -90,6 +91,6 @@ public class StreamSerializerTest {
String serialized = serializer.serializeItemStack(initial);
ItemStack deserialized = serializer.deserializeItemStack(serialized);
assertEquals(initial, deserialized);
assertItemsEqual(initial, deserialized);
}
}

45
modules/ProtocolLib/src/test/java/com/comphenix/protocol/utility/TestUtils.java Normal file
View File

@ -0,0 +1,45 @@
package com.comphenix.protocol.utility;
import java.util.List;
import org.bukkit.Bukkit;
import org.bukkit.inventory.ItemStack;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
public class TestUtils {
public static void assertItemCollectionsEqual(List<ItemStack> first, List<ItemStack> second) {
assertEquals(first.size(), second.size());
for (int i = 0; i < first.size(); i++) {
assertItemsEqual(first.get(i), second.get(i));
}
}
public static void assertItemsEqual(ItemStack first, ItemStack second) {
if (first == null) {
assertNull(second);
} else {
assertNotNull(first);
// The legacy check in ItemStack#isSimilar causes a null pointer
assertEquals(first.getType(), second.getType());
assertEquals(first.getDurability(), second.getDurability());
assertEquals(first.hasItemMeta(), second.hasItemMeta());
if (first.hasItemMeta()) {
assertTrue(Bukkit.getItemFactory().equals(first.getItemMeta(), second.getItemMeta()));
}
}
}
public static boolean equivalentItem(ItemStack first, ItemStack second) {
if (first == null) {
return second == null;
} else if (second == null) {
return false;
} else {
return first.getType().equals(second.getType());
}
}
}

42
modules/ProtocolLib/src/test/java/com/comphenix/protocol/wrappers/BukkitConvertersTest.java Normal file
View File

@ -0,0 +1,42 @@
package com.comphenix.protocol.wrappers;
import com.comphenix.protocol.BukkitInitialization;
import com.comphenix.protocol.reflect.EquivalentConverter;
import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.enchantments.Enchantment;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.meta.ItemMeta;
import org.junit.BeforeClass;
import org.junit.Test;
import static com.comphenix.protocol.utility.TestUtils.assertItemsEqual;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class BukkitConvertersTest {
@BeforeClass
public static void beforeClass() {
BukkitInitialization.initializeItemMeta();
}
@Test
public void testItemStacks() {
ItemStack item = new ItemStack(Material.DIAMOND_SWORD, 16);
item.addEnchantment(Enchantment.DAMAGE_ALL, 4);
ItemMeta meta = item.getItemMeta();
meta.setDisplayName(ChatColor.GREEN + "Diamond Sword");
item.setItemMeta(meta);
EquivalentConverter<ItemStack> converter = BukkitConverters.getItemStackConverter();
Object nmsStack = converter.getGeneric(item);
ItemStack back = converter.getSpecific(nmsStack);
assertEquals(item.getType(), back.getType());
assertEquals(item.getDurability(), back.getDurability());
assertEquals(item.hasItemMeta(), back.hasItemMeta());
assertTrue(Bukkit.getItemFactory().equals(item.getItemMeta(), back.getItemMeta()));
}
}