From aa71764e4a7be7e990b74292f92647e9793293c2 Mon Sep 17 00:00:00 2001
From: _tud <98935832+UnderscoreTud@users.noreply.github.com>
Date: Sun, 15 Dec 2024 03:24:59 +0300
Subject: [PATCH 1/5] Fix the REMOVE changer of variables (#7163)
* Fix the REMOVE changer of variables
* Fix wording
* Fix tests
* Update Variable.java
---------
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
---
.../java/ch/njol/skript/lang/Variable.java | 47 +++++--------------
...removing from variable skips duplicates.sk | 6 +++
2 files changed, 18 insertions(+), 35 deletions(-)
create mode 100644 src/test/skript/tests/regressions/7162-removing from variable skips duplicates.sk
diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java
index 1b7a55f42b6..173ee1aeea3 100644
--- a/src/main/java/ch/njol/skript/lang/Variable.java
+++ b/src/main/java/ch/njol/skript/lang/Variable.java
@@ -1,23 +1,10 @@
-/**
- * This file is part of Skript.
- *
- * Skript is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Skript is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with Skript. If not, see .
- *
- * Copyright Peter Güttinger, SkriptLang team and contributors
- */
package ch.njol.skript.lang;
+import java.lang.reflect.Array;
+import java.util.*;
+import java.util.Map.Entry;
+import java.util.function.Function;
+
import ch.njol.skript.Skript;
import ch.njol.skript.SkriptAPIException;
import ch.njol.skript.SkriptConfig;
@@ -55,17 +42,6 @@
import org.skriptlang.skript.lang.script.Script;
import org.skriptlang.skript.lang.script.ScriptWarning;
-import java.lang.reflect.Array;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.NoSuchElementException;
-import java.util.TreeMap;
-import java.util.function.Function;
-
public class Variable implements Expression {
private final static String SINGLE_SEPARATOR_CHAR = ":";
@@ -579,14 +555,15 @@ public void change(Event event, @Nullable Object[] delta, ChangeMode mode) throw
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;
@@ -600,7 +577,7 @@ public void change(Event event, @Nullable Object[] delta, ChangeMode mode) throw
} 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/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"
From 48f931a9f92dcb3b6486513c862fd788a1ba8038 Mon Sep 17 00:00:00 2001
From: Patrick Miller
Date: Tue, 17 Dec 2024 11:46:12 -0500
Subject: [PATCH 2/5] Fix Improperly Typed Array in ExprXOf (#7268)
---
src/main/java/ch/njol/skript/expressions/ExprXOf.java | 4 +++-
.../skript/tests/regressions/6817-null of x incorrect type.sk | 3 +++
2 files changed, 6 insertions(+), 1 deletion(-)
create mode 100644 src/test/skript/tests/regressions/6817-null of x incorrect type.sk
diff --git a/src/main/java/ch/njol/skript/expressions/ExprXOf.java b/src/main/java/ch/njol/skript/expressions/ExprXOf.java
index 06393a0423a..04dc49c8971 100644
--- a/src/main/java/ch/njol/skript/expressions/ExprXOf.java
+++ b/src/main/java/ch/njol/skript/expressions/ExprXOf.java
@@ -36,6 +36,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")
@@ -62,7 +64,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/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}
From 219e358711e3db3172d836d5c86ec54f5b7d2af5 Mon Sep 17 00:00:00 2001
From: Moderocky
Date: Thu, 26 Dec 2024 20:44:35 +0000
Subject: [PATCH 3/5] Add boat data check to prevent error. (#7301)
---
.../java/ch/njol/skript/entity/BoatData.java | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/main/java/ch/njol/skript/entity/BoatData.java b/src/main/java/ch/njol/skript/entity/BoatData.java
index 22b8d58b251..47d647ee61f 100644
--- a/src/main/java/ch/njol/skript/entity/BoatData.java
+++ b/src/main/java/ch/njol/skript/entity/BoatData.java
@@ -55,22 +55,22 @@ public class BoatData extends EntityData {
}
-
+
public BoatData(){
this(0);
}
-
+
public BoatData(@Nullable TreeSpecies type){
this(type != null ? type.ordinal() + 2 : 1);
}
-
+
private BoatData(int type){
matchedPattern = type;
}
-
+
@Override
protected boolean init(Literal[] exprs, int matchedPattern, ParseResult parseResult) {
-
+
return true;
}
@@ -96,7 +96,7 @@ protected boolean match(Boat entity) {
@Override
public Class getType() {
- if (IS_RUNNING_1_21_3)
+ if (IS_RUNNING_1_21_3 && matchedPattern > 1)
return typeToClassMap.get(TreeSpecies.values()[matchedPattern - 2]);
return Boat.class;
}
@@ -124,7 +124,7 @@ public boolean isSupertypeOf(EntityData e) {
return matchedPattern <= 1 || matchedPattern == ((BoatData)e).matchedPattern;
return false;
}
-
+
public boolean isOfItemType(ItemType i){
int ordinal = -1;
@@ -142,7 +142,7 @@ else if (type == Material.ACACIA_BOAT)
else if (type == Material.DARK_OAK_BOAT)
ordinal = TreeSpecies.DARK_OAK.ordinal();
return hashCode_i() == ordinal + 2 || (matchedPattern + ordinal == 0) || ordinal == 0;
-
+
}
}
From 3c940632d60327cd019d8ab6d411c0aeb1750f97 Mon Sep 17 00:00:00 2001
From: kyoshuske <65446070+kyoshuske@users.noreply.github.com>
Date: Thu, 26 Dec 2024 21:55:49 +0100
Subject: [PATCH 4/5] Fix BlockIgniteEvent (#7252)
Update BukkitEventValues.java
Co-authored-by: Moderocky
---
.../java/ch/njol/skript/classes/data/BukkitEventValues.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 82fca450068..f40aa638d9e 100644
--- a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
+++ b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
@@ -423,7 +423,7 @@ public Player get(final BlockIgniteEvent e) {
@Override
@Nullable
public Block get(final BlockIgniteEvent e) {
- return e.getIgnitingBlock();
+ return e.getBlock();
}
}, 0);
// BlockDispenseEvent
From cca446dd040f5f60ac3cb1517d5aec9ce23bf37f Mon Sep 17 00:00:00 2001
From: _tud <98935832+UnderscoreTud@users.noreply.github.com>
Date: Mon, 30 Dec 2024 09:24:05 +0300
Subject: [PATCH 5/5] Fix expression conversion (#7165)
* Fix expression conversion
* Extract duplicate code into a separate helper method
* improve conversion strategy
* Add .sk to test file
* Simplify conversion usage
We need to use conversion whenever there are multiple return types. If the expression does not accept our supertype, then we can attempt to convert it, which will already handle safety checks for multiple return types
* SimpleExpression: fix possible return type conversion
This fixes SimpleExpression not converting possible return types that are not contained in the desired types array. For example, if an expression can return a Number or a String, and we want an Expression that is a Number or an World, it will now include converters for String->Number and String->World
* Use safety checks of ConvertedExpression
* Remove incorrect converter remake
* Move logic from SimpleExpression to ConvertedExpression
---------
Co-authored-by: APickledWalrus
Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>
Co-authored-by: Moderocky
---
.../ch/njol/skript/lang/SkriptParser.java | 10 ++---
.../skript/lang/util/ConvertedExpression.java | 40 +++++++++++--------
.../skript/lang/util/SimpleExpression.java | 30 --------------
...7164-expressions sometimes dont convert.sk | 13 ++++++
4 files changed, 42 insertions(+), 51 deletions(-)
create mode 100644 src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk
diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java
index cefa92b2549..6cffb2a30fd 100644
--- a/src/main/java/ch/njol/skript/lang/SkriptParser.java
+++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java
@@ -326,7 +326,6 @@ private static Variable parseVariable(String expr, Class[] r
}
}
-
@Nullable
@SuppressWarnings({"unchecked", "rawtypes"})
private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable LogEntry error, Class... types) {
@@ -368,9 +367,9 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral
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;
}
@@ -540,13 +539,14 @@ private Expression parseSingleExpr(boolean allowUnparsedLiteral, @Nullable Lo
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/util/ConvertedExpression.java b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java
index 4d6228112f0..e6150fda79c 100644
--- a/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java
+++ b/src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java
@@ -24,6 +24,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;
@@ -96,24 +97,31 @@ public ConvertedExpression(Expression source, Class to, Collecti
@Nullable
public static 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 c576cd8d774..eccb3c73ba5 100644
--- a/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java
+++ b/src/main/java/ch/njol/skript/lang/util/SimpleExpression.java
@@ -35,13 +35,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.
@@ -206,33 +203,6 @@ public Expression getConvertedExpression(Class... to) {
// 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/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"