Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix entity casting issue and add regression test. #6614

Merged
merged 7 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/main/java/ch/njol/skript/expressions/ExprNearestEntity.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")
Expand Down Expand Up @@ -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);
Expand All @@ -97,9 +99,17 @@ public boolean isSingle() {
return entityDatas.length == 1;
}

private transient @Nullable Class<? extends Entity> knownReturnType;

@Override
public Class<? extends Entity> getReturnType() {
return entityDatas.length == 1 ? entityDatas[0].getType() : Entity.class;
if (knownReturnType != null)
return knownReturnType;
Class<? extends Entity>[] 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);
APickledWalrus marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
78 changes: 51 additions & 27 deletions src/main/java/ch/njol/skript/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
* <h3>Arbitrary Selection</h3>
* Classes may have <b>multiple</b> highest common denominators: interfaces that they share
* which do not extend each other.
* This method selects a <b>superclass</b> 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 <i>not</i> 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 <Found> The highest common denominator found
* @param <Type> The input type spread
*/
@SafeVarargs
@SuppressWarnings("unchecked")
public static <Found, Type extends Found> Class<Found> highestDenominator(Class<? super Found> bestGuess, @NotNull Class<? extends Type> @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<Found>) bestGuess;
}

if (!bestGuess.isAssignableFrom(chosen)) // we struck out on a type we don't want
return (Class<Found>) 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<Found>) (chosen == Cloneable.class ? bestGuess : chosen == Object.class ? bestGuess : chosen);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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