diff --git a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java index ab91438aaf2..19ac863a49e 100644 --- a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java +++ b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java @@ -244,7 +244,7 @@ public Player get(final BlockIgniteEvent e) { @Override @Nullable public Block get(final BlockIgniteEvent e) { - return e.getIgnitingBlock(); + return e.getBlock(); } }, 0); // BlockDispenseEvent diff --git a/src/main/java/ch/njol/skript/entity/BoatData.java b/src/main/java/ch/njol/skript/entity/BoatData.java index f173cbec3fc..e6c9535f2be 100644 --- a/src/main/java/ch/njol/skript/entity/BoatData.java +++ b/src/main/java/ch/njol/skript/entity/BoatData.java @@ -50,7 +50,7 @@ public BoatData(@Nullable Boat.Type type){ private BoatData(int type) { matchedPattern = type; } - + @Override protected boolean init(Literal[] exprs, int matchedPattern, ParseResult parseResult) { return true; diff --git a/src/main/java/ch/njol/skript/expressions/ExprXOf.java b/src/main/java/ch/njol/skript/expressions/ExprXOf.java index 2e0ad4a6df2..00fd8fe63d3 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprXOf.java +++ b/src/main/java/ch/njol/skript/expressions/ExprXOf.java @@ -18,6 +18,8 @@ import org.bukkit.inventory.ItemStack; import org.jetbrains.annotations.Nullable; +import java.lang.reflect.Array; + @Name("X of Item") @Description("An expression to be able to use a certain amount of items where the amount can be any expression. Please note that this expression is not stable and might be replaced in the future.") @Examples("give level of player of pickaxes to the player") @@ -44,7 +46,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye protected Object[] get(Event e, Object[] source) { Number a = amount.getSingle(e); if (a == null) - return new Object[0]; + return (Object[]) Array.newInstance(getReturnType(), 0); return get(source, o -> { if (o instanceof ItemStack) { diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index 79d94bf6ebc..a6eb399dfc8 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -351,9 +351,9 @@ public boolean hasTag(String tag) { if ((flags & PARSE_EXPRESSIONS) != 0) { Expression parsedExpression = parseExpression(types, expr); if (parsedExpression != null) { // Expression/VariableString parsing success + Class parsedReturnType = parsedExpression.getReturnType(); for (Class type : types) { - // Check return type against everything that expression accepts - if (parsedExpression.canReturn(type)) { + if (type.isAssignableFrom(parsedReturnType)) { log.printLog(); return (Expression) parsedExpression; } @@ -519,13 +519,14 @@ public boolean hasTag(String tag) { if ((flags & PARSE_EXPRESSIONS) != 0) { Expression parsedExpression = parseExpression(types, expr); if (parsedExpression != null) { // Expression/VariableString parsing success + Class parsedReturnType = parsedExpression.getReturnType(); for (int i = 0; i < types.length; i++) { Class type = types[i]; if (type == null) // Ignore invalid (null) types continue; - // Check return type against everything that expression accepts - if (parsedExpression.canReturn(type)) { + // Check return type against the expression's return type + if (type.isAssignableFrom(parsedReturnType)) { if (!exprInfo.isPlural[i] && !parsedExpression.isSingle()) { // Wrong number of arguments if (context == ParseContext.COMMAND) { Skript.error(Commands.m_too_many_arguments.toString(exprInfo.classes[i].getName().getIndefiniteArticle(), exprInfo.classes[i].getName().toString()), ErrorQuality.SEMANTIC_ERROR); diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 72301c318b4..95e1331fd3c 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -555,14 +555,15 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro if (mode == ChangeMode.REMOVE) { if (map == null) return; - ArrayList toRemove = new ArrayList<>(); // prevents CMEs + Set toRemove = new HashSet<>(); // prevents CMEs for (Object value : delta) { for (Entry entry : map.entrySet()) { + String key = entry.getKey(); + if (key == null) + continue; // This is NOT a part of list variable + if (toRemove.contains(key)) + continue; // Skip if we've already marked this key to be removed if (Relation.EQUAL.isImpliedBy(Comparators.compare(entry.getValue(), value))) { - String key = entry.getKey(); - if (key == null) - continue; // This is NOT a part of list variable - // Otherwise, we'll mark that key to be set to null toRemove.add(key); break; @@ -576,7 +577,7 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro } else if (mode == ChangeMode.REMOVE_ALL) { if (map == null) return; - ArrayList toRemove = new ArrayList<>(); // prevents CMEs + Set toRemove = new HashSet<>(); // prevents CMEs for (Entry i : map.entrySet()) { for (Object value : delta) { if (Relation.EQUAL.isImpliedBy(Comparators.compare(i.getValue(), value))) diff --git a/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java index 70c41575ab8..9c17a103c67 100644 --- a/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java @@ -6,6 +6,7 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.registrations.Classes; +import ch.njol.skript.util.Utils; import ch.njol.util.Checker; import ch.njol.util.Kleenean; import ch.njol.util.coll.CollectionUtils; @@ -77,24 +78,31 @@ public ConvertedExpression(Expression source, Class to, Collecti @SafeVarargs public static @Nullable ConvertedExpression newInstance(Expression from, Class... to) { assert !CollectionUtils.containsSuperclass(to, from.getReturnType()); - // we track a list of converters that may work - List> converters = new ArrayList<>(); - for (Class type : to) { // REMIND try more converters? -> also change WrapperExpression (and maybe ExprLoopValue) - assert type != null; - // casting to is wrong, but since the converter is only used for values returned by the expression - // (which are instances of "") this won't result in any ClassCastExceptions. - for (Class checking : from.possibleReturnTypes()) { - //noinspection unchecked - ConverterInfo converter = (ConverterInfo) Converters.getConverterInfo(checking, type); - if (converter != null) - converters.add(converter); + + // we might be able to cast some (or all) of the possible return types to T + // for possible return types that can't be directly cast, regular converters will be used + List> infos = new ArrayList<>(); + for (Class type : from.possibleReturnTypes()) { + if (CollectionUtils.containsSuperclass(to, type)) { // this type is of T, build a converter simply casting + // noinspection unchecked - 'type' is a desired type in 'to' + Class toType = (Class) type; + infos.add(new ConverterInfo<>(type, toType, toType::cast, 0)); + } else { // this possible return type is not included in 'to' + // build all converters for converting the possible return type into any of the types of 'to' + for (Class toType : to) { + ConverterInfo converter = Converters.getConverterInfo(type, toType); + if (converter != null) + infos.add(converter); + } } - int size = converters.size(); - if (size == 1) // if there is only one info, there is no need to wrap it in a list - return new ConvertedExpression<>(from, type, converters.get(0)); - if (size > 1) - return new ConvertedExpression<>(from, type, converters, true); } + if (!infos.isEmpty()) { // there are converters for (at least some of) the return types + // a note: casting to is wrong, but since the converter is used only for values + // returned by the expression (which are instances of ), this won't result in any CCEs + // noinspection rawtypes, unchecked + return new ConvertedExpression(from, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true); + } + return null; } diff --git a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java index 909035cceb6..ba08a0f8c61 100644 --- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java +++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java @@ -19,13 +19,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.converter.Converter; -import org.skriptlang.skript.lang.converter.ConverterInfo; import java.lang.reflect.Array; -import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; -import java.util.List; /** * An implementation of the {@link Expression} interface. You should usually extend this class to make a new expression. @@ -189,33 +186,6 @@ public static boolean check(T @Nullable [] values, Checker checke // check whether this expression is already of type R if (CollectionUtils.containsSuperclass(to, getReturnType())) return (Expression) this; - - // we might be to cast some of the possible return types to R - List> infos = new ArrayList<>(); - for (Class type : this.possibleReturnTypes()) { - if (CollectionUtils.containsSuperclass(to, type)) { // this type is of R - // build a converter that for casting to R - // safety check is present in the event that we do not get this type at runtime - final Class toType = (Class) type; - infos.add(new ConverterInfo<>(getReturnType(), toType, fromObject -> { - if (toType.isInstance(fromObject)) - return (R) fromObject; - return null; - }, 0)); - } - } - int size = infos.size(); - if (size == 1) { // if there is only one info, there is no need to wrap it in a list - ConverterInfo info = infos.get(0); - //noinspection rawtypes - return new ConvertedExpression(this, info.getTo(), info); - } - if (size > 1) { - //noinspection rawtypes - return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, false); - } - - // attempt traditional conversion with proper converters return this.getConvertedExpr(to); } diff --git a/src/test/skript/tests/regressions/6817-null of x incorrect type.sk b/src/test/skript/tests/regressions/6817-null of x incorrect type.sk new file mode 100644 index 00000000000..ac93553d712 --- /dev/null +++ b/src/test/skript/tests/regressions/6817-null of x incorrect type.sk @@ -0,0 +1,3 @@ +test "null of x incorrect type": + # would cause an exception + spawn {_null} of pig above {_null} diff --git a/src/test/skript/tests/regressions/7162-removing from variable skips duplicates.sk b/src/test/skript/tests/regressions/7162-removing from variable skips duplicates.sk new file mode 100644 index 00000000000..c4fadc84163 --- /dev/null +++ b/src/test/skript/tests/regressions/7162-removing from variable skips duplicates.sk @@ -0,0 +1,6 @@ +test "removing from variables skips duplicates": + set {_list::*} to 1, 1, 2, 3, 4, 5, 6 and 6 + remove 1 and 1 from {_list::*} + assert {_list::*} is 2, 3, 4, 5, 6 and 6 with "didn't remove all instances of 1 from the list" + remove first 6 elements out of {_list::*} from {_list::*} + assert {_list::*} is not set with "didn't remove all elements" diff --git a/src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk b/src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk new file mode 100644 index 00000000000..f7954b3a642 --- /dev/null +++ b/src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk @@ -0,0 +1,13 @@ +test "expressions sometimes dont convert": + assert formatted ({_foo} + {_bar}) is not set with "formatting nothing shouldn't throw an error" + + set {_foo} to "test" + assert formatted ({_foo} + {_bar}) is not set with "formatting string + none shouldn't throw an error" + + set {_foo} to 1 + set {_bar} to 2 + assert formatted ({_foo} + {_bar}) is not set with "formatting number + number shouldn't throw an error" + + set {_foo} to "foo" + set {_bar} to "bar" + assert formatted ({_foo} + {_bar}) is "foobar" with "formatting strings doesn't work"