Fix PotionData NPE (#6757)

SkriptLang · Jun 6, 2024 · b9a7ab8 · b9a7ab8
1 parent 42f595b
commit b9a7ab8
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 54 deletions.
diff --git a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
@@ -18,9 +18,6 @@
  */
 package ch.njol.skript.classes.data;
 
-import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -662,34 +659,17 @@ public LivingEntity[] get(AreaEffectCloudApplyEvent event) {
 			}
 		}, EventValues.TIME_NOW);
 		EventValues.registerEventValue(AreaEffectCloudApplyEvent.class, PotionEffectType.class, new Getter<PotionEffectType, AreaEffectCloudApplyEvent>() {
-			@Nullable
-			private final MethodHandle BASE_POTION_DATA_HANDLE;
-
-			{
-				MethodHandle basePotionDataHandle = null;
-				if (Skript.methodExists(AreaEffectCloud.class, "getBasePotionData")) {
-					try {
-						basePotionDataHandle = MethodHandles.lookup().findVirtual(AreaEffectCloud.class, "getBasePotionData", MethodType.methodType(PotionData.class));
-					} catch (NoSuchMethodException | IllegalAccessException e) {
-						Skript.exception(e, "Failed to load legacy potion data support. Potions may not work as expected.");
-					}
-				}
-				BASE_POTION_DATA_HANDLE = basePotionDataHandle;
-			}
-
+			private final boolean HAS_POTION_TYPE_METHOD = Skript.methodExists(AreaEffectCloud.class, "getBasePotionType");
 			@Override
 			@Nullable
 			public PotionEffectType get(AreaEffectCloudApplyEvent e) {
-				if (BASE_POTION_DATA_HANDLE != null) {
-					try {
-						return ((PotionData) BASE_POTION_DATA_HANDLE.invoke(e.getEntity())).getType().getEffectType();
-					} catch (Throwable ex) {
-						throw Skript.exception(ex, "An error occurred while trying to invoke legacy area effect cloud potion effect support.");
-					}
-				} else {
+				// TODO needs to be reworked to support multiple values (there can be multiple potion effects)
+				if (HAS_POTION_TYPE_METHOD) {
 					PotionType base = e.getEntity().getBasePotionType();
-					if (base != null) // TODO this is deprecated... this should become a multi-value event value
+					if (base != null)
 						return base.getEffectType();
+				} else {
+					return e.getEntity().getBasePotionData().getType().getEffectType();
 				}
 				return null;
 			}

diff --git a/src/main/java/ch/njol/skript/util/PotionEffectUtils.java b/src/main/java/ch/njol/skript/util/PotionEffectUtils.java
@@ -18,9 +18,6 @@
  */
 package ch.njol.skript.util;
 
-import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -346,20 +343,7 @@ else if (HAS_SUSPICIOUS_META && meta instanceof SuspiciousStewMeta)
 		itemType.setItemMeta(meta);
 	}
 
-	@Nullable
-	private static final MethodHandle BASE_POTION_DATA_HANDLE;
-
-	static {
-		MethodHandle basePotionDataHandle = null;
-		if (Skript.methodExists(PotionMeta.class, "getBasePotionData")) {
-			try {
-				basePotionDataHandle = MethodHandles.lookup().findVirtual(PotionMeta.class, "getBasePotionData", MethodType.methodType(PotionData.class));
-			} catch (NoSuchMethodException | IllegalAccessException e) {
-				Skript.exception(e, "Failed to load legacy potion data support. Potions may not work as expected.");
-			}
-		}
-		BASE_POTION_DATA_HANDLE = basePotionDataHandle;
-	}
+	private static final boolean HAS_POTION_TYPE_METHOD = Skript.methodExists(PotionMeta.class, "hasBasePotionType");
 
 	/**
 	 * Get all the PotionEffects of an ItemType
@@ -374,21 +358,22 @@ public static List<PotionEffect> getEffects(ItemType itemType) {
 		ItemMeta meta = itemType.getItemMeta();
 		if (meta instanceof PotionMeta) {
 			PotionMeta potionMeta = ((PotionMeta) meta);
-			effects.addAll(potionMeta.getCustomEffects());
-			if (BASE_POTION_DATA_HANDLE != null) {
-				try {
-					effects.addAll(PotionDataUtils.getPotionEffects((PotionData) BASE_POTION_DATA_HANDLE.invoke(meta)));
-				} catch (Throwable e) {
-					throw Skript.exception(e, "An error occurred while trying to invoke legacy potion data support.");
+			if (potionMeta.hasCustomEffects())
+				effects.addAll(potionMeta.getCustomEffects());
+			if (HAS_POTION_TYPE_METHOD) {
+				if (potionMeta.hasBasePotionType()) {
+					//noinspection ConstantConditions - checked via hasBasePotionType
+					effects.addAll(potionMeta.getBasePotionType().getPotionEffects());
+				}
+			} else { // use deprecated method
+				PotionData data = potionMeta.getBasePotionData();
+				if (data != null) {
+					effects.addAll(PotionDataUtils.getPotionEffects(data));
 				}
-			} else if (potionMeta.hasBasePotionType()) {
-				//noinspection ConstantConditions - checked via hasBasePotionType
-				effects.addAll(potionMeta.getBasePotionType().getPotionEffects());
 			}
-
 		} else if (HAS_SUSPICIOUS_META && meta instanceof SuspiciousStewMeta)
 			effects.addAll(((SuspiciousStewMeta) meta).getCustomEffects());
 		return effects;
 	}
-	
+
 }
diff --git a/src/test/skript/tests/regressions/6756-potion missing base type causes npe.sk b/src/test/skript/tests/regressions/6756-potion missing base type causes npe.sk
@@ -0,0 +1,3 @@
+test "potion missing base type":
+
+	assert potion effects of (plain potion of mundane) is not set with "it should not have any effects"
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		test "potion missing base type":

		assert potion effects of (plain potion of mundane) is not set with "it should not have any effects"