From 672de73f6fc5723acd281fc94c10da38a12da438 Mon Sep 17 00:00:00 2001 From: Moderocky Date: Wed, 1 May 2024 21:01:02 +0100 Subject: [PATCH] Fix entity casting issue and add regression test. (#6614) --- .../skript/expressions/ExprNearestEntity.java | 16 +++- src/main/java/ch/njol/skript/util/Utils.java | 78 ++++++++++++------- .../6612-nearest entity cast error.sk | 18 +++++ 3 files changed, 82 insertions(+), 30 deletions(-) create mode 100644 src/test/skript/tests/regressions/6612-nearest entity cast error.sk diff --git a/src/main/java/ch/njol/skript/expressions/ExprNearestEntity.java b/src/main/java/ch/njol/skript/expressions/ExprNearestEntity.java index a2c37a1ab6b..59978951fde 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprNearestEntity.java +++ b/src/main/java/ch/njol/skript/expressions/ExprNearestEntity.java @@ -29,6 +29,7 @@ import ch.njol.skript.lang.ExpressionType; import ch.njol.skript.lang.Literal; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.util.Utils; import ch.njol.util.Kleenean; import ch.njol.util.StringUtils; import org.bukkit.Location; @@ -37,6 +38,7 @@ import org.bukkit.event.Event; import org.eclipse.jdt.annotation.Nullable; +import java.lang.reflect.Array; import java.util.Arrays; @Name("Nearest Entity") @@ -80,8 +82,8 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye protected Entity[] get(Event event) { Object relativeTo = this.relativeTo.getSingle(event); if (relativeTo == null || (relativeTo instanceof Location && ((Location) relativeTo).getWorld() == null)) - return new Entity[0]; - Entity[] nearestEntities = new Entity[entityDatas.length]; + return (Entity[]) Array.newInstance(this.getReturnType(), 0);; + Entity[] nearestEntities = (Entity[]) Array.newInstance(this.getReturnType(), entityDatas.length); for (int i = 0; i < nearestEntities.length; i++) { if (relativeTo instanceof Entity) { nearestEntities[i] = getNearestEntity(entityDatas[i], ((Entity) relativeTo).getLocation(), (Entity) relativeTo); @@ -97,9 +99,17 @@ public boolean isSingle() { return entityDatas.length == 1; } + private transient @Nullable Class knownReturnType; + @Override public Class getReturnType() { - return entityDatas.length == 1 ? entityDatas[0].getType() : Entity.class; + if (knownReturnType != null) + return knownReturnType; + Class[] types = new Class[entityDatas.length]; + for (int i = 0; i < types.length; i++) { + types[i] = entityDatas[i].getType(); + } + return knownReturnType = Utils.highestDenominator(Entity.class, types); } @Override diff --git a/src/main/java/ch/njol/skript/util/Utils.java b/src/main/java/ch/njol/skript/util/Utils.java index 8897d6ef1ec..862e440ceb8 100644 --- a/src/main/java/ch/njol/skript/util/Utils.java +++ b/src/main/java/ch/njol/skript/util/Utils.java @@ -62,6 +62,7 @@ import ch.njol.util.coll.CollectionUtils; import ch.njol.util.coll.iterator.EnumerationIterable; import net.md_5.bungee.api.ChatColor; +import org.jetbrains.annotations.NotNull; /** * Utility class. @@ -673,41 +674,64 @@ public static int random(final int start, final int end) { throw new IllegalArgumentException("end (" + end + ") must be > start (" + start + ")"); return start + random.nextInt(end - start); } - - // TODO improve - public static Class getSuperType(final Class... cs) { - assert cs.length > 0; - Class r = cs[0]; - assert r != null; - outer: for (final Class c : cs) { - assert c != null && !c.isArray() && !c.isPrimitive() : c; - if (c.isAssignableFrom(r)) { - r = c; + + /** + * @see #highestDenominator(Class, Class[]) + */ + public static Class getSuperType(final Class... classes) { + return highestDenominator(Object.class, classes); + } + + /** + * Searches for the highest common denominator of the given types; + * in other words, the first supertype they all share. + * + *

Arbitrary Selection

+ * Classes may have multiple highest common denominators: interfaces that they share + * which do not extend each other. + * This method selects a superclass first (where possible) + * but its selection of interfaces is quite random. + * For this reason, it is advised to specify a "best guess" class as the first parameter, which will be selected if + * it's appropriate. + * Note that if the "best guess" is not a real supertype, it can never be selected. + * + * @param bestGuess The fallback class to guess + * @param classes The types to check + * @return The most appropriate common class of all provided + * @param The highest common denominator found + * @param The input type spread + */ + @SafeVarargs + @SuppressWarnings("unchecked") + public static Class highestDenominator(Class bestGuess, @NotNull Class @NotNull ... classes) { + assert classes.length > 0; + Class chosen = classes[0]; + outer: + for (final Class checking : classes) { + assert checking != null && !checking.isArray() && !checking.isPrimitive() : checking; + if (chosen.isAssignableFrom(checking)) continue; + Class superType = checking; + do if (superType != Object.class && superType.isAssignableFrom(chosen)) { + chosen = superType; + continue outer; } - if (!r.isAssignableFrom(c)) { - Class s = c; - while ((s = s.getSuperclass()) != null) { - if (s != Object.class && s.isAssignableFrom(r)) { - r = s; - continue outer; - } + while ((superType = superType.getSuperclass()) != null); + for (final Class anInterface : checking.getInterfaces()) { + superType = highestDenominator(Object.class, anInterface, chosen); + if (superType != Object.class) { + chosen = superType; + continue outer; } - for (final Class i : c.getInterfaces()) { - s = getSuperType(i, r); - if (s != Object.class) { - r = s; - continue outer; - } - } - return Object.class; } + return (Class) bestGuess; } - + if (!bestGuess.isAssignableFrom(chosen)) // we struck out on a type we don't want + return (Class) bestGuess; // Cloneable is about as useful as object as super type // However, it lacks special handling used for Object supertype // See #1747 to learn how it broke returning items from functions - return r.equals(Cloneable.class) ? Object.class : r; + return (Class) (chosen == Cloneable.class ? bestGuess : chosen == Object.class ? bestGuess : chosen); } /** diff --git a/src/test/skript/tests/regressions/6612-nearest entity cast error.sk b/src/test/skript/tests/regressions/6612-nearest entity cast error.sk new file mode 100644 index 00000000000..c5b503e7cbd --- /dev/null +++ b/src/test/skript/tests/regressions/6612-nearest entity cast error.sk @@ -0,0 +1,18 @@ +test "charge creeper nearest entity cast": + set {_location} to spawn of world "world" + spawn a creeper at {_location} + set {_creeper} to last spawned creeper + assert {_creeper} is not charged with "spawning a normal creeper shouldn't spawn a charged one" + make (nearest creeper to {_location}) charged # this used to throw an exception + assert {_creeper} is charged with "a creeper should be charged after it is set as charged" + uncharge the nearest creeper to {_location} # this would also throw an exception + assert {_creeper} is not charged with "uncharging a charged creeper should uncharge it" + delete the last spawned creeper + +test "poison nearest entity cast": + set {_location} to spawn of world "world" + spawn a creeper at {_location} + set {_creeper} to last spawned creeper + poison (nearest entity to {_location}) # this never threw an exception + poison (nearest creeper to {_location}) # this used to throw an exception + delete the last spawned creeper