From fb0c8ec9347839affa9f5b7f9cac5e0326d3953c Mon Sep 17 00:00:00 2001 From: nossr50 Date: Sat, 11 May 2024 15:05:27 -0700 Subject: [PATCH] Fix potions not brewing as the correct type --- Changelog.txt | 6 +++ pom.xml | 2 +- .../config/skills/alchemy/PotionConfig.java | 39 +++++++++----- .../skills/alchemy/AlchemyPotion.java | 30 ++++++++--- .../datatypes/skills/alchemy/PotionStage.java | 9 ++-- .../nossr50/listeners/InventoryListener.java | 13 +++++ .../skills/AlchemyBrewCheckTask.java | 5 +- .../skills/alchemy/AlchemyManager.java | 5 ++ .../skills/alchemy/AlchemyPotionBrewer.java | 3 +- .../com/gmail/nossr50/util/PotionUtil.java | 54 ++++++++++++++++--- .../gmail/nossr50/util/PotionUtilTest.java | 2 +- .../util/random/ProbabilityTestUtils.java | 2 +- 12 files changed, 135 insertions(+), 35 deletions(-) diff --git a/Changelog.txt b/Changelog.txt index fe6f1b950..2bbd5e969 100644 --- a/Changelog.txt +++ b/Changelog.txt @@ -1,3 +1,9 @@ +Version 2.2.008 + Fixed alchemy potions not upgrading correctly (See notes) + + NOTES: + Alchemy potions will now be brewed as type "Mundane" behind the scenes, this used to be Uncraftable/Water. This led to some issues. So I've changed it to be Mundane. + Version 2.2.007 Compatibility with the 1.20.5 / 1.20.6 MC Update Fixed bug where Alchemy was not brewing certain potions (haste, etc) diff --git a/pom.xml b/pom.xml index 8693a420f..99c18b60d 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.gmail.nossr50.mcMMO mcMMO - 2.2.007 + 2.2.008-SNAPSHOT mcMMO https://github.com/mcMMO-Dev/mcMMO diff --git a/src/main/java/com/gmail/nossr50/config/skills/alchemy/PotionConfig.java b/src/main/java/com/gmail/nossr50/config/skills/alchemy/PotionConfig.java index 9df3115b8..50466ced4 100644 --- a/src/main/java/com/gmail/nossr50/config/skills/alchemy/PotionConfig.java +++ b/src/main/java/com/gmail/nossr50/config/skills/alchemy/PotionConfig.java @@ -127,9 +127,6 @@ public class PotionConfig extends LegacyConfigLoader { private AlchemyPotion loadPotion(ConfigurationSection potion_section) { try { final String key = potion_section.getName(); - final String displayName = potion_section.getString("Name") != null - ? LocaleLoader.addColors(potion_section.getString("Name")) - : convertKeyToName(key); final ConfigurationSection potionData = potion_section.getConfigurationSection("PotionData"); boolean extended = false; @@ -159,14 +156,11 @@ public class PotionConfig extends LegacyConfigLoader { final PotionMeta potionMeta = (PotionMeta) itemStack.getItemMeta(); if (potionMeta == null) { - mcMMO.p.getLogger().severe("PotionConfig: Failed to get PotionMeta for " + displayName + ", from configuration section:" + + mcMMO.p.getLogger().severe("PotionConfig: Failed to get PotionMeta for " + key + ", from configuration section:" + " " + potion_section); return null; } - // Set the name of the potion - potionMeta.setDisplayName(displayName); - // extended and upgraded seem to be mutually exclusive if (extended && upgraded) { mcMMO.p.getLogger().warning("Potion " + key + " has both Extended and Upgraded set to true," + @@ -176,7 +170,7 @@ public class PotionConfig extends LegacyConfigLoader { String potionTypeStr = potionData.getString("PotionType", null); if (potionTypeStr == null) { - mcMMO.p.getLogger().severe("PotionConfig: Missing PotionType for " + displayName + ", from configuration section:" + + mcMMO.p.getLogger().severe("PotionConfig: Missing PotionType for " + key + ", from configuration section:" + " " + potion_section); return null; } @@ -190,7 +184,7 @@ public class PotionConfig extends LegacyConfigLoader { if (potionType == null) { mcMMO.p.getLogger().severe("PotionConfig: Failed to parse potion type for: " + potionTypeStr - + ", upgraded: " + upgraded + ", extended: " + extended + " for potion " + displayName + + ", upgraded: " + upgraded + ", extended: " + extended + " for potion " + key + ", from configuration section: " + potion_section); return null; } @@ -225,7 +219,7 @@ public class PotionConfig extends LegacyConfigLoader { if (type != null) { potionMeta.addCustomEffect(new PotionEffect(type, duration, amplifier), true); } else { - mcMMO.p.getLogger().severe("PotionConfig: Failed to parse effect for potion " + displayName + ": " + effect); + mcMMO.p.getLogger().severe("PotionConfig: Failed to parse effect for potion " + key + ": " + effect); } } } @@ -238,17 +232,21 @@ public class PotionConfig extends LegacyConfigLoader { } potionMeta.setColor(color); - Map children = new HashMap<>(); + final Map children = new HashMap<>(); if (potion_section.contains("Children")) { for (String child : potion_section.getConfigurationSection("Children").getKeys(false)) { ItemStack ingredient = loadIngredient(child); if (ingredient != null) { children.put(ingredient, potion_section.getConfigurationSection("Children").getString(child)); } else { - mcMMO.p.getLogger().severe("PotionConfig: Failed to parse child for potion " + displayName + ": " + child); + mcMMO.p.getLogger().severe("PotionConfig: Failed to parse child for potion " + key + ": " + child); } } } + + // Set the name of the potion + setPotionDisplayName(potion_section, potionMeta); + // TODO: Might not need to .setItemMeta itemStack.setItemMeta(potionMeta); return new AlchemyPotion(itemStack, children); @@ -258,6 +256,23 @@ public class PotionConfig extends LegacyConfigLoader { } } + private void setPotionDisplayName(ConfigurationSection section, PotionMeta potionMeta) { + String configuredName = section.getString("Name", null); + if (configuredName != null) { + potionMeta.setDisplayName(configuredName); + return; + } + + // Potion is water, but has effects + if (isPotionTypeWater(potionMeta) + && (PotionUtil.hasBasePotionEffects(potionMeta) || !potionMeta.getCustomEffects().isEmpty())) { + // If we don't set a name for these potions, they will simply be called "Water Potion" + final String name = section.getName().toUpperCase().replace("_", " "); + potionMeta.setDisplayName(name); + System.out.println("DEBUG: Renaming potion to " + name); + } + } + /** * Parse a string representation of an ingredient. * Format: '<MATERIAL>[:data]' diff --git a/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/AlchemyPotion.java b/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/AlchemyPotion.java index c7b7fd442..fb5ea7899 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/AlchemyPotion.java +++ b/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/AlchemyPotion.java @@ -53,12 +53,11 @@ public class AlchemyPotion { } final PotionMeta otherPotionMeta = (PotionMeta) otherPotion.getItemMeta(); - - // all custom effects must be present - for (var effect : getAlchemyPotionMeta().getCustomEffects()) { - if (!otherPotionMeta.hasCustomEffect(effect.getType())) { - return false; - } + // compare custom effects on both potions, this has to be done in two traversals + // comparing thisPotionMeta -> otherPotionMeta and otherPotionMeta -> thisPotionMeta + if (hasDifferingCustomEffects(getAlchemyPotionMeta(), otherPotionMeta) + || hasDifferingCustomEffects(otherPotionMeta, getAlchemyPotionMeta())) { + return false; } if (!samePotionType(getAlchemyPotionMeta(), otherPotionMeta)) { @@ -78,6 +77,25 @@ public class AlchemyPotion { return true; } + private boolean hasDifferingCustomEffects(PotionMeta potionMeta, PotionMeta otherPotionMeta) { + for (int i = 0; i < potionMeta.getCustomEffects().size(); i++) { + var effect = potionMeta.getCustomEffects().get(i); + + // One has an effect the other does not, they are not the same potion + if (!otherPotionMeta.hasCustomEffect(effect.getType())) { + return true; + } + + var otherEffect = otherPotionMeta.getCustomEffects().get(i); + // Amplifier or duration are not equal, they are not the same potion + if (effect.getAmplifier() != otherEffect.getAmplifier() + || effect.getDuration() != otherEffect.getDuration()) { + return true; + } + } + return false; + } + public PotionMeta getAlchemyPotionMeta() { return (PotionMeta) potionItemstack.getItemMeta(); } diff --git a/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/PotionStage.java b/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/PotionStage.java index 4515835d1..303cf3b19 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/PotionStage.java +++ b/src/main/java/com/gmail/nossr50/datatypes/skills/alchemy/PotionStage.java @@ -34,12 +34,13 @@ public enum PotionStage { } public static PotionStage getPotionStage(AlchemyPotion input, AlchemyPotion output) { - PotionStage potionStage = getPotionStage(output); - if (!isWaterBottle(input) && getPotionStage(input) == potionStage) { - potionStage = PotionStage.FIVE; + PotionStage outputPotionStage = getPotionStage(output); + PotionStage inputPotionStage = getPotionStage(input); + if (!isWaterBottle(input) && inputPotionStage == outputPotionStage) { + outputPotionStage = PotionStage.FIVE; } - return potionStage; + return outputPotionStage; } private static boolean isWaterBottle(AlchemyPotion alchemyPotion) { diff --git a/src/main/java/com/gmail/nossr50/listeners/InventoryListener.java b/src/main/java/com/gmail/nossr50/listeners/InventoryListener.java index 4467ff723..879fbb820 100644 --- a/src/main/java/com/gmail/nossr50/listeners/InventoryListener.java +++ b/src/main/java/com/gmail/nossr50/listeners/InventoryListener.java @@ -5,6 +5,7 @@ import com.gmail.nossr50.datatypes.player.McMMOPlayer; import com.gmail.nossr50.datatypes.skills.PrimarySkillType; import com.gmail.nossr50.datatypes.skills.SubSkillType; import com.gmail.nossr50.events.fake.FakeBrewEvent; +import com.gmail.nossr50.events.fake.FakeEvent; import com.gmail.nossr50.mcMMO; import com.gmail.nossr50.runnables.player.PlayerUpdateInventoryTask; import com.gmail.nossr50.skills.alchemy.Alchemy; @@ -28,6 +29,7 @@ import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; +import org.bukkit.event.block.BrewingStartEvent; import org.bukkit.event.inventory.*; import org.bukkit.inventory.*; @@ -343,6 +345,7 @@ public class InventoryListener implements Listener { if (event instanceof FakeBrewEvent) return; + Location location = event.getBlock().getLocation(); if (Alchemy.brewingStandMap.containsKey(location)) { Alchemy.brewingStandMap.get(location).finishImmediately(); @@ -350,6 +353,16 @@ public class InventoryListener implements Listener { } } +// @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) +// public void onBrewStart(BrewingStartEvent event) { +// /* WORLD BLACKLIST CHECK */ +// if(WorldBlacklist.isWorldBlacklisted(event.getBlock().getWorld())) +// return; +// +// if (event instanceof FakeEvent) +// return; +// } + @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onInventoryMoveItemEvent(InventoryMoveItemEvent event) { /* WORLD BLACKLIST CHECK */ diff --git a/src/main/java/com/gmail/nossr50/runnables/skills/AlchemyBrewCheckTask.java b/src/main/java/com/gmail/nossr50/runnables/skills/AlchemyBrewCheckTask.java index f0121fa8e..224538244 100644 --- a/src/main/java/com/gmail/nossr50/runnables/skills/AlchemyBrewCheckTask.java +++ b/src/main/java/com/gmail/nossr50/runnables/skills/AlchemyBrewCheckTask.java @@ -29,7 +29,10 @@ public class AlchemyBrewCheckTask extends CancellableRunnable { boolean validBrew = brewingStand.getFuelLevel() > 0 && AlchemyPotionBrewer.isValidBrew(player, newInventory); if (Alchemy.brewingStandMap.containsKey(location)) { - if (oldInventory[Alchemy.INGREDIENT_SLOT] == null || newInventory[Alchemy.INGREDIENT_SLOT] == null || !oldInventory[Alchemy.INGREDIENT_SLOT].isSimilar(newInventory[Alchemy.INGREDIENT_SLOT]) || !validBrew) { + if (oldInventory[Alchemy.INGREDIENT_SLOT] == null + || newInventory[Alchemy.INGREDIENT_SLOT] == null + || !oldInventory[Alchemy.INGREDIENT_SLOT].isSimilar(newInventory[Alchemy.INGREDIENT_SLOT]) + || !validBrew) { Alchemy.brewingStandMap.get(location).cancelBrew(); } } else if (validBrew) { diff --git a/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyManager.java b/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyManager.java index 4224ad123..8cc53bc5b 100644 --- a/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyManager.java +++ b/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyManager.java @@ -52,6 +52,11 @@ public class AlchemyManager extends SkillManager { return Math.min(Alchemy.catalysisMaxSpeed, Alchemy.catalysisMinSpeed + (Alchemy.catalysisMaxSpeed - Alchemy.catalysisMinSpeed) * (skillLevel - RankUtils.getUnlockLevel(SubSkillType.ALCHEMY_CATALYSIS)) / (Alchemy.catalysisMaxBonusLevel - RankUtils.getUnlockLevel(SubSkillType.ALCHEMY_CATALYSIS))) * (isLucky ? LUCKY_MODIFIER : 1.0); } + /** + * Handle the XP gain for a successful potion brew. + * @param potionStage The potion stage, this is used to determine the XP gain. + * @param amount The amount of potions brewed. + */ public void handlePotionBrewSuccesses(PotionStage potionStage, int amount) { applyXpGain((float) (ExperienceConfig.getInstance().getPotionXP(potionStage) * amount), XPGainReason.PVE, XPGainSource.PASSIVE); } diff --git a/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyPotionBrewer.java b/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyPotionBrewer.java index 82b2e0e85..1060e3f5c 100644 --- a/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyPotionBrewer.java +++ b/src/main/java/com/gmail/nossr50/skills/alchemy/AlchemyPotionBrewer.java @@ -38,7 +38,8 @@ public final class AlchemyPotionBrewer { continue; } - if (getChildPotion(mcMMO.p.getPotionConfig().getPotion(contents[i]), contents[Alchemy.INGREDIENT_SLOT]) != null) { + final AlchemyPotion potion = mcMMO.p.getPotionConfig().getPotion(contents[i]); + if (getChildPotion(potion, contents[Alchemy.INGREDIENT_SLOT]) != null) { return true; } } diff --git a/src/main/java/com/gmail/nossr50/util/PotionUtil.java b/src/main/java/com/gmail/nossr50/util/PotionUtil.java index a83897a5f..0661cd487 100644 --- a/src/main/java/com/gmail/nossr50/util/PotionUtil.java +++ b/src/main/java/com/gmail/nossr50/util/PotionUtil.java @@ -1,7 +1,6 @@ package com.gmail.nossr50.util; import com.gmail.nossr50.mcMMO; -import org.bukkit.Bukkit; import org.bukkit.NamespacedKey; import org.bukkit.inventory.meta.PotionMeta; import org.bukkit.potion.PotionEffectType; @@ -33,15 +32,16 @@ public class PotionUtil { public static final String STRONG = "STRONG"; public static final String LONG = "LONG"; - public static final String LEGACY_WATER_POTION_TYPE = "WATER"; + public static final String WATER_POTION_TYPE_STR = "WATER"; private static final PotionCompatibilityType COMPATIBILITY_MODE; static { - // We used to use uncraftable as the potion type - // this type isn't available anymore, so water will do potionDataClass = getPotionDataClass(); - legacyPotionTypes.put("UNCRAFTABLE", "WATER"); + // Uncraftable doesn't exist in modern versions + // It served as a potion that didn't craft into anything else so it didn't conflict with vanilla systems + // Instead we will use Mundane, which doesn't make anything in vanilla systems + legacyPotionTypes.put("UNCRAFTABLE", "MUNDANE"); legacyPotionTypes.put("JUMP", "LEAPING"); legacyPotionTypes.put("SPEED", "SWIFTNESS"); legacyPotionTypes.put("INSTANT_HEAL", "HEALING"); @@ -314,6 +314,12 @@ public class PotionUtil { return (NamespacedKey) methodPotionTypeGetKey.invoke(potionType); } + public static boolean isPotionJustWater(PotionMeta potionMeta) { + return isPotionTypeWater(potionMeta) + && !hasBasePotionEffects(potionMeta) + && potionMeta.getCustomEffects().isEmpty(); + } + public static boolean isPotionTypeWater(@NotNull PotionMeta potionMeta) { if (COMPATIBILITY_MODE == PotionCompatibilityType.PRE_1_20_5) { return isPotionTypeWaterLegacy(potionMeta); @@ -322,12 +328,44 @@ public class PotionUtil { } } + public static boolean isPotionType(@NotNull PotionMeta potionMeta, String potionType) { + if (COMPATIBILITY_MODE == PotionCompatibilityType.PRE_1_20_5) { + return isPotionTypeLegacy(potionMeta, potionType); + } else { + return isPotionTypeModern(potionMeta, potionType); + } + } + + public static boolean isPotionTypeWithoutEffects(@NotNull PotionMeta potionMeta, String potionType) { + return isPotionType(potionMeta, potionType) + && !hasBasePotionEffects(potionMeta) + && potionMeta.getCustomEffects().isEmpty(); + } + + private static boolean isPotionTypeModern(@NotNull PotionMeta potionMeta, String potionType) { + try { + return getModernPotionTypeKey(potionMeta).getKey().equalsIgnoreCase(potionType); + } catch (IllegalAccessException | InvocationTargetException ex) { + throw new RuntimeException(ex); + } + } + + private static boolean isPotionTypeLegacy(@NotNull PotionMeta potionMeta, String potionType) { + try { + Object potionData = methodPotionMetaGetBasePotionData.invoke(potionMeta); + PotionType potionTypeObj = (PotionType) methodPotionDataGetType.invoke(potionData); + return potionTypeObj.name().equalsIgnoreCase(potionType); + } catch (IllegalAccessException | InvocationTargetException ex) { + throw new RuntimeException(ex); + } + } + private static boolean isPotionTypeWaterLegacy(@NotNull PotionMeta potionMeta) { try { Object potionData = methodPotionMetaGetBasePotionData.invoke(potionMeta); PotionType potionType = (PotionType) methodPotionDataGetType.invoke(potionData); - return potionType.name().equalsIgnoreCase(LEGACY_WATER_POTION_TYPE) - || PotionType.valueOf(LEGACY_WATER_POTION_TYPE) == potionType; + return potionType.name().equalsIgnoreCase(WATER_POTION_TYPE_STR) + || PotionType.valueOf(WATER_POTION_TYPE_STR) == potionType; } catch (InvocationTargetException | IllegalAccessException ex) { throw new RuntimeException(ex); } @@ -335,7 +373,7 @@ public class PotionUtil { private static boolean isPotionTypeWaterModern(@NotNull PotionMeta potionMeta) { try { - return getModernPotionTypeKey(potionMeta).getKey().equalsIgnoreCase(LEGACY_WATER_POTION_TYPE); + return getModernPotionTypeKey(potionMeta).getKey().equalsIgnoreCase(WATER_POTION_TYPE_STR); } catch (IllegalAccessException | InvocationTargetException ex) { throw new RuntimeException(ex); } diff --git a/src/test/java/com/gmail/nossr50/util/PotionUtilTest.java b/src/test/java/com/gmail/nossr50/util/PotionUtilTest.java index caa3a1218..d60c0f7cb 100644 --- a/src/test/java/com/gmail/nossr50/util/PotionUtilTest.java +++ b/src/test/java/com/gmail/nossr50/util/PotionUtilTest.java @@ -30,6 +30,6 @@ class PotionUtilTest { void testConvertLegacyNames() { final String potionTypeStr = "UNCRAFTABLE"; final String converted = convertLegacyNames(potionTypeStr); - assertEquals("WATER", converted); + assertEquals("MUNDANE", converted); } } \ No newline at end of file diff --git a/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java index e97119be6..0e41b4649 100644 --- a/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java +++ b/src/test/java/com/gmail/nossr50/util/random/ProbabilityTestUtils.java @@ -16,7 +16,7 @@ public class ProbabilityTestUtils { System.out.println("Wins: " + winCount); System.out.println("Fails: " + (iterations - winCount)); System.out.println("Percentage succeeded: " + successPercent + ", Expected: " + expectedWinPercent); - assertEquals(expectedWinPercent, successPercent, 0.025D); + assertEquals(expectedWinPercent, successPercent, 0.035D); System.out.println("Variance is within tolerance levels!"); } }