From d39a750709e95fd38d39422b5274598731847024 Mon Sep 17 00:00:00 2001 From: t00thpick1 Date: Sat, 9 Apr 2016 11:49:04 -0400 Subject: [PATCH] SPIGOT-2085 / SPIGOT-2087 / SPIGOT-2156: Rework internal PotionMeta state to be correct and less complex. --- .../inventory/CraftMetaPotion.java | 32 +++++++------------ .../craftbukkit/potion/CraftPotionBrewer.java | 2 +- .../craftbukkit/potion/CraftPotionUtil.java | 9 ++++-- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaPotion.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaPotion.java index 569a92d319..f512dc011a 100644 --- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaPotion.java +++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaPotion.java @@ -32,7 +32,9 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { static final ItemMetaKey ID = new ItemMetaKey("Id", "potion-id"); static final ItemMetaKey DEFAULT_POTION = new ItemMetaKey("Potion", "potion-type"); - private String type; + // Having an initial "state" in ItemMeta seems bit dirty but the UNCRAFTABLE potion type + // is treated as the empty form of the meta because it represents an empty potion with no effect + private PotionData type = new PotionData(PotionType.UNCRAFTABLE, false, false); private List customEffects; CraftMetaPotion(CraftMetaItem meta) { @@ -50,7 +52,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { CraftMetaPotion(NBTTagCompound tag) { super(tag); if (tag.hasKey(DEFAULT_POTION.NBT)) { - type = tag.getString(DEFAULT_POTION.NBT); + type = CraftPotionUtil.toBukkit(tag.getString(DEFAULT_POTION.NBT)); } if (tag.hasKey(POTION_EFFECTS.NBT)) { NBTTagList list = tag.getList(POTION_EFFECTS.NBT, 10); @@ -71,7 +73,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { CraftMetaPotion(Map map) { super(map); - type = SerializableMeta.getString(map, DEFAULT_POTION.BUKKIT, true); + type = CraftPotionUtil.toBukkit(SerializableMeta.getString(map, DEFAULT_POTION.BUKKIT, true)); Iterable rawEffectList = SerializableMeta.getObject(Iterable.class, map, POTION_EFFECTS.BUKKIT, true); if (rawEffectList == null) { @@ -89,9 +91,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { @Override void applyToItem(NBTTagCompound tag) { super.applyToItem(tag); - // NBT expects a PotionType under all circumstances, but ItemMeta API contract expects that a fresh stack have blank meta - // As such we will equate the NMS default type of "Empty"/UNCRAFTABLE to be null - tag.setString(DEFAULT_POTION.NBT, type != null ? type : CraftPotionUtil.fromBukkit(new PotionData(PotionType.UNCRAFTABLE, false, false))); + tag.setString(DEFAULT_POTION.NBT, CraftPotionUtil.fromBukkit(type)); if (customEffects != null) { NBTTagList effectList = new NBTTagList(); tag.set(POTION_EFFECTS.NBT, effectList); @@ -114,7 +114,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { } boolean isPotionEmpty() { - return (type == null) && !(hasCustomEffects()); + return (type.getType() == PotionType.UNCRAFTABLE) && !(hasCustomEffects()); } @Override @@ -143,20 +143,12 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { @Override public void setBasePotionData(PotionData data) { Validate.notNull(data, "PotionData cannot be null"); - PotionType type = data.getType(); - if (type == PotionType.UNCRAFTABLE) { - this.type = null; - return; - } - this.type = CraftPotionUtil.fromBukkit(data); + this.type = data; } @Override public PotionData getBasePotionData() { - if (type == null) { - return new PotionData(PotionType.UNCRAFTABLE, false, false); - } - return CraftPotionUtil.toBukkit(type); + return type; } public boolean hasCustomEffects() { @@ -257,7 +249,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { int applyHash() { final int original; int hash = original = super.applyHash(); - if (type != null) { + if (type.getType() != PotionType.UNCRAFTABLE) { hash = 73 * hash + type.hashCode(); } if (hasCustomEffects()) { @@ -274,7 +266,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { if (meta instanceof CraftMetaPotion) { CraftMetaPotion that = (CraftMetaPotion) meta; - return ((type == null && that.type == null) || type.equals(that.type)) && (this.hasCustomEffects() ? that.hasCustomEffects() && this.customEffects.equals(that.customEffects) : !that.hasCustomEffects()); + return type.equals(that.type) && (this.hasCustomEffects() ? that.hasCustomEffects() && this.customEffects.equals(that.customEffects) : !that.hasCustomEffects()); } return true; } @@ -287,7 +279,7 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta { @Override Builder serialize(Builder builder) { super.serialize(builder); - if (type != null) { + if (type.getType() != PotionType.UNCRAFTABLE) { builder.put(DEFAULT_POTION.BUKKIT, type); } diff --git a/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionBrewer.java b/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionBrewer.java index 897b7a7ac6..14b79c13a4 100644 --- a/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionBrewer.java +++ b/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionBrewer.java @@ -24,7 +24,7 @@ public class CraftPotionBrewer implements PotionBrewer { if (cache.containsKey(damage)) return cache.get(damage); - List mcEffects = PotionRegistry.a(CraftPotionUtil.fromBukkit(new PotionData(damage, upgraded, extended))).a(); + List mcEffects = PotionRegistry.a(CraftPotionUtil.fromBukkit(new PotionData(damage, extended, upgraded))).a(); ImmutableList.Builder builder = new ImmutableList.Builder(); for (MobEffect effect : mcEffects) { diff --git a/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionUtil.java b/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionUtil.java index f85650f9e6..5a8b77a01b 100644 --- a/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionUtil.java +++ b/src/main/java/org/bukkit/craftbukkit/potion/CraftPotionUtil.java @@ -68,9 +68,12 @@ public class CraftPotionUtil { } public static PotionData toBukkit(String type) { - if (type.startsWith("minecraft:")) { - type = type.substring(10); - } + if (type == null) { + return new PotionData(PotionType.UNCRAFTABLE, false, false); + } + if (type.startsWith("minecraft:")) { + type = type.substring(10); + } PotionType potionType = null; potionType = extendable.inverse().get(type); if (potionType != null) {