Bandaid fix for Effect (#9548)

Effect or LevelEvent needs to be replaced
but ideally after the enum PR has been merged
upstream. Until then, this test and these fixes
should address all the known issues with them
This commit is contained in:
Jake Potrebic 2023-08-12 15:19:34 -07:00 committed by GitHub
parent 29aaf7bc24
commit 2df309bd49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 221 additions and 3 deletions

View File

@ -5,10 +5,26 @@ Subject: [PATCH] Add missing effects
diff --git a/src/main/java/org/bukkit/Effect.java b/src/main/java/org/bukkit/Effect.java
index 879d637691683ca862045402f74b751a892bf3ff..3ae64e7e42338a2a550917ccd01d755a1ba0bf03 100644
index 879d637691683ca862045402f74b751a892bf3ff..63034a66bcc10db939c497552b73ba96b0aa4b9d 100644
--- a/src/main/java/org/bukkit/Effect.java
+++ b/src/main/java/org/bukkit/Effect.java
@@ -337,7 +337,100 @@ public enum Effect {
@@ -132,12 +132,12 @@ public enum Effect {
/**
* Sound of a block breaking. Needs block ID as additional info.
*/
- STEP_SOUND(2001, Type.SOUND, Material.class),
+ STEP_SOUND(2001, Type.SOUND, org.bukkit.block.data.BlockData.class, Material.class), // Paper - block data is more correct, but the impl of the mtehods will still work with Material
/**
- * Visual effect of a splash potion breaking. Needs potion data value as
+ * Visual effect of a splash potion breaking. Needs color data value as
* additional info.
*/
- POTION_BREAK(2002, Type.VISUAL, Potion.class),
+ POTION_BREAK(2002, Type.VISUAL, Color.class, Potion.class), // Paper - color is correct
/**
* Visual effect of an instant splash potion breaking. Needs color data
* value as additional info.
@@ -337,21 +337,124 @@ public enum Effect {
* block.
*/
OXIDISED_COPPER_SCRAPE(3005, Type.VISUAL),
@ -103,13 +119,69 @@ index 879d637691683ca862045402f74b751a892bf3ff..3ae64e7e42338a2a550917ccd01d755a
+ */
+ @Deprecated(forRemoval = true) @org.jetbrains.annotations.ApiStatus.ScheduledForRemoval(inVersion = "1.21")
+ WET_SPONGE_VAPORIZES_IN_NETHER(2009, Type.VISUAL),
+
+ SOUND_STOP_JUKEBOX_SONG(1011, Type.SOUND),
+
+ PARTICLES_SCULK_CHARGE(3006, Type.VISUAL, Integer.class),
+
+ PARTICLES_SCULK_SHRIEK(3007, Type.SOUND),
+
+ PARTICLES_AND_SOUND_BRUSH_BLOCK_COMPLETE(3008, Type.VISUAL, org.bukkit.block.data.BlockData.class),
+
+ PARTICLES_EGG_CRACK(3009, Type.VISUAL)
;
+ private static final org.apache.logging.log4j.Logger LOGGER = org.apache.logging.log4j.LogManager.getLogger();
+ // Paper end
private final int id;
private final Type type;
@@ -397,10 +490,22 @@ public enum Effect {
- private final Class<?> data;
+ private final java.util.List<Class<?>> data; // Paper - support multiple data types
private static final Map<Integer, Effect> BY_ID = Maps.newHashMap();
Effect(int id, /*@NotNull*/ Type type) {
- this(id, type, null);
+ this(id, type, (Class<?>[]) null); // Paper - support multiple data types
}
- Effect(int id, /*@NotNull*/ Type type, /*@Nullable*/ Class<?> data) {
+ Effect(int id, /*@NotNull*/ Type type, /*@Nullable*/ Class<?>...data) { // Paper - support multiple data types
this.id = id;
this.type = type;
- this.data = data;
+ this.data = data != null ? java.util.List.of(data) : null; // Paper - support multiple data types
}
/**
@@ -367,8 +470,10 @@ public enum Effect {
/**
* @return The type of the effect.
+ * @deprecated some effects can be both or neither
*/
@NotNull
+ @Deprecated // Paper - both
public Type getType() {
return this.type;
}
@@ -379,8 +484,15 @@ public enum Effect {
*/
@Nullable
public Class<?> getData() {
- return this.data;
+ return this.data == null ? null : this.data.get(0); // Paper
+ }
+
+ // Paper start - support deprecated data types
+ @org.jetbrains.annotations.ApiStatus.Internal
+ public boolean isApplicable(Object obj) {
+ return this.data != null && com.google.common.collect.Iterables.any(this.data, aClass -> aClass.isAssignableFrom(obj.getClass()));
}
+ // Paper end - support deprecated data types
/**
* Gets the Effect associated with the given ID.
@@ -397,12 +509,26 @@ public enum Effect {
static {
for (Effect effect : values()) {
@ -131,7 +203,11 @@ index 879d637691683ca862045402f74b751a892bf3ff..3ae64e7e42338a2a550917ccd01d755a
+
/**
* Represents the type of an effect.
+ * @deprecated not representative of what Effect does
*/
+ @Deprecated // Paper
public enum Type { SOUND, VISUAL }
}
diff --git a/src/test/java/org/bukkit/EffectTest.java b/src/test/java/org/bukkit/EffectTest.java
index 54e621e86e8fe3414099494d419929b282b33489..759081f15992e07271567d65250f27f14f6c99c3 100644
--- a/src/test/java/org/bukkit/EffectTest.java

View File

@ -0,0 +1,142 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jake Potrebic <jake.m.potrebic@gmail.com>
Date: Fri, 28 Jul 2023 15:02:44 -0700
Subject: [PATCH] Bandaid fix for Effect
Effect or LevelEvent needs to be replaced
but ideally after the enum PR has been merged
upstream. Until then, this test and these fixes
should address all the known issues with them
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftEffect.java b/src/main/java/org/bukkit/craftbukkit/CraftEffect.java
index 5a5a8945c786e16ff0df62494ddd1ac85c42b53f..63f9735d356dafd579cee4423d3037eb4ed9b2c6 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftEffect.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftEffect.java
@@ -15,12 +15,15 @@ public class CraftEffect {
public static <T> int getDataValue(Effect effect, T data) {
int datavalue;
switch (effect) {
+ case PARTICLES_SCULK_CHARGE: // Paper - add missing effects
case VILLAGER_PLANT_GROW:
datavalue = (Integer) data;
break;
case POTION_BREAK:
+ if (data instanceof Potion) { // Paper - use Color for POTION_BREAK
datavalue = ((Potion) data).toDamageValue() & 0x3F;
break;
+ } // Paper - Color will fall through to cast below
case INSTANT_POTION_BREAK:
datavalue = ((Color) data).asRGB();
break;
@@ -59,8 +62,15 @@ public class CraftEffect {
}
break;
case STEP_SOUND:
+ if (data instanceof Material) { // Paper - support BlockData
Preconditions.checkArgument(((Material) data).isBlock(), "Material %s is not a block!", data);
datavalue = Block.getId(CraftMagicNumbers.getBlock((Material) data).defaultBlockState());
+ // Paper start - support BlockData
+ break;
+ }
+ case PARTICLES_AND_SOUND_BRUSH_BLOCK_COMPLETE:
+ datavalue = Block.getId(((org.bukkit.craftbukkit.block.data.CraftBlockData) data).getState());
+ // Paper end
break;
case COMPOSTER_FILL_ATTEMPT:
datavalue = ((Boolean) data) ? 1 : 0;
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
index 550dcb7d595221b221e4710890d8a3cad789fc07..f857f490ffba2f25f7c06c5fb1a1905f0b51fbe2 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -1374,7 +1374,7 @@ public class CraftWorld extends CraftRegionAccessor implements World {
public <T> void playEffect(Location loc, Effect effect, T data, int radius) {
if (data != null) {
Preconditions.checkArgument(effect.getData() != null, "Effect.%s does not have a valid Data", effect);
- Preconditions.checkArgument(effect.getData().isAssignableFrom(data.getClass()), "%s data cannot be used for the %s effect", data.getClass().getName(), effect);
+ Preconditions.checkArgument(effect.isApplicable(data), "%s data cannot be used for the %s effect", data.getClass().getName(), effect); // Paper
} else {
// Special case: the axis is optional for ELECTRIC_SPARK
Preconditions.checkArgument(effect.getData() == null || effect == Effect.ELECTRIC_SPARK, "Wrong kind of data for the %s effect", effect);
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
index 94dd03ccd95b497987117244eb9f008fe4b01a09..9273d21d59bfd93d6480e57b83ebc4b8df2cc758 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
@@ -860,7 +860,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
Preconditions.checkArgument(effect != null, "Effect cannot be null");
if (data != null) {
Preconditions.checkArgument(effect.getData() != null, "Effect.%s does not have a valid Data", effect);
- Preconditions.checkArgument(effect.getData().isAssignableFrom(data.getClass()), "%s data cannot be used for the %s effect", data.getClass().getName(), effect);
+ Preconditions.checkArgument(effect.isApplicable(data), "%s data cannot be used for the %s effect", data.getClass().getName(), effect); // Paper
} else {
// Special case: the axis is optional for ELECTRIC_SPARK
Preconditions.checkArgument(effect.getData() == null || effect == Effect.ELECTRIC_SPARK, "Wrong kind of data for the %s effect", effect);
diff --git a/src/test/java/org/bukkit/EffectTest.java b/src/test/java/org/bukkit/EffectTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..35e46e3f3747193ddc2f5b8bb7bec1bc955228f3
--- /dev/null
+++ b/src/test/java/org/bukkit/EffectTest.java
@@ -0,0 +1,64 @@
+package org.bukkit;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import net.minecraft.world.level.block.LevelEvent;
+import org.junit.Test;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class EffectTest {
+
+ private static List<Integer> collectNmsLevelEvents() throws ReflectiveOperationException {
+ final List<Integer> events = new ArrayList<>();
+ for (final Field field : LevelEvent.class.getFields()) {
+ if (Modifier.isStatic(field.getModifiers()) && Modifier.isFinal(field.getModifiers()) && field.getType() == int.class) {
+ events.add((int) field.get(null));
+ }
+ }
+ return events;
+ }
+
+ private static boolean isNotDeprecated(Effect effect) throws ReflectiveOperationException {
+ return !Effect.class.getDeclaredField(effect.name()).isAnnotationPresent(Deprecated.class);
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void checkAllApiExists() throws ReflectiveOperationException {
+ Map<Integer, Effect> toId = new HashMap<>();
+ for (final Effect effect : Effect.values()) {
+ if (isNotDeprecated(effect)) {
+ final Effect put = toId.put(effect.getId(), effect);
+ assertNull("duplicate API effect: " + put, put);
+ }
+ }
+
+ for (final Integer event : collectNmsLevelEvents()) {
+ assertNotNull("missing API Effect: " + event, toId.get(event));
+ }
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void checkNoExtraApi() throws ReflectiveOperationException {
+ Map<Integer, Effect> toId = new HashMap<>();
+ for (final Effect effect : Effect.values()) {
+ if (isNotDeprecated(effect)) {
+ final Effect put = toId.put(effect.getId(), effect);
+ assertNull("duplicate API effect: " + put, put);
+ }
+ }
+
+ final List<Integer> nmsEvents = collectNmsLevelEvents();
+ for (final Map.Entry<Integer, Effect> entry : toId.entrySet()) {
+ assertTrue("Extra API Effect: " + entry.getValue(), nmsEvents.contains(entry.getKey()));
+ }
+ }
+}