mirror of
https://github.com/PaperMC/Paper.git
synced 2024-12-27 15:00:13 +01:00
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:
parent
d96d1cb7dd
commit
2e3118d02a
2 changed files with 218 additions and 0 deletions
|
@ -8,6 +8,22 @@ diff --git a/src/main/java/org/bukkit/Effect.java b/src/main/java/org/bukkit/Eff
|
|||
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
|
||||
--- a/src/main/java/org/bukkit/Effect.java
|
||||
+++ b/src/main/java/org/bukkit/Effect.java
|
||||
@@ -0,0 +0,0 @@ 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.
|
||||
@@ -0,0 +0,0 @@ public enum Effect {
|
||||
* block.
|
||||
*/
|
||||
|
@ -103,12 +119,68 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
|
|||
+ */
|
||||
+ @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;
|
||||
- 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
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -0,0 +0,0 @@ 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;
|
||||
}
|
||||
@@ -0,0 +0,0 @@ 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.
|
||||
@@ -0,0 +0,0 @@ public enum Effect {
|
||||
|
||||
static {
|
||||
|
@ -131,7 +203,11 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
|
|||
+
|
||||
/**
|
||||
* 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
|
||||
--- a/src/test/java/org/bukkit/EffectTest.java
|
||||
|
|
142
patches/server/Bandaid-fix-for-Effect.patch
Normal file
142
patches/server/Bandaid-fix-for-Effect.patch
Normal 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
|
||||
--- a/src/main/java/org/bukkit/craftbukkit/CraftEffect.java
|
||||
+++ b/src/main/java/org/bukkit/craftbukkit/CraftEffect.java
|
||||
@@ -0,0 +0,0 @@ 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;
|
||||
@@ -0,0 +0,0 @@ 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
|
||||
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
|
||||
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
|
||||
@@ -0,0 +0,0 @@ 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
|
||||
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
|
||||
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
|
||||
@@ -0,0 +0,0 @@ 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..0000000000000000000000000000000000000000
|
||||
--- /dev/null
|
||||
+++ b/src/test/java/org/bukkit/EffectTest.java
|
||||
@@ -0,0 +0,0 @@
|
||||
+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()));
|
||||
+ }
|
||||
+ }
|
||||
+}
|
Loading…
Reference in a new issue