diff --git a/src/main/java/ch/njol/skript/effects/EffEnchant.java b/src/main/java/ch/njol/skript/effects/EffEnchant.java index 016b8c66be8..0e83dfcfefb 100644 --- a/src/main/java/ch/njol/skript/effects/EffEnchant.java +++ b/src/main/java/ch/njol/skript/effects/EffEnchant.java @@ -18,11 +18,6 @@ */ package ch.njol.skript.effects; -import org.bukkit.enchantments.Enchantment; -import org.bukkit.event.Event; -import org.bukkit.inventory.ItemStack; -import org.jetbrains.annotations.Nullable; - import ch.njol.skript.Skript; import ch.njol.skript.aliases.ItemType; import ch.njol.skript.classes.Changer.ChangeMode; @@ -36,6 +31,11 @@ import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.util.EnchantmentType; import ch.njol.util.Kleenean; +import org.bukkit.event.Event; +import org.bukkit.inventory.ItemStack; +import org.jetbrains.annotations.Nullable; + +import java.util.function.Function; /** * @author Peter Güttinger @@ -72,29 +72,26 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye @Override protected void execute(Event event) { - ItemType[] items = this.items.getArray(event); - if (items.length == 0) // short circuit - return; + Function changeFunction; if (enchantments != null) { EnchantmentType[] types = enchantments.getArray(event); if (types.length == 0) return; - for (ItemType item : items) { - for (EnchantmentType type : types) { - Enchantment enchantment = type.getType(); - assert enchantment != null; - item.addEnchantments(new EnchantmentType(enchantment, type.getLevel())); - } - } + changeFunction = item -> { + item.addEnchantments(types); + return item; + }; } else { - for (ItemType item : items) { + changeFunction = item -> { item.clearEnchantments(); - } + return item; + }; } - this.items.change(event, items.clone(), ChangeMode.SET); + + this.items.changeInPlace(event, changeFunction); } - + @Override public String toString(@Nullable Event event, boolean debug) { return enchantments == null ? "disenchant " + items.toString(event, debug) : "enchant " + items.toString(event, debug) + " with " + enchantments; diff --git a/src/main/java/ch/njol/skript/effects/EffReplace.java b/src/main/java/ch/njol/skript/effects/EffReplace.java index 0bb52d4eb26..3274bc6dbf9 100644 --- a/src/main/java/ch/njol/skript/effects/EffReplace.java +++ b/src/main/java/ch/njol/skript/effects/EffReplace.java @@ -36,9 +36,11 @@ import org.bukkit.event.Event; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Map; +import java.util.function.Function; import java.util.regex.Matcher; @Name("Replace") @@ -106,20 +108,9 @@ private void replace(Event event, Object[] needles, Expression haystackExpr) if (replacement == null || haystack == null || haystack.length == 0 || needles == null || needles.length == 0) return; if (replaceString) { - if (replaceFirst) { - for (int x = 0; x < haystack.length; x++) - for (Object n : needles) { - assert n != null; - haystack[x] = StringUtils.replaceFirst((String)haystack[x], (String)n, Matcher.quoteReplacement((String)replacement), caseSensitive); - } - } else { - for (int x = 0; x < haystack.length; x++) - for (Object n : needles) { - assert n != null; - haystack[x] = StringUtils.replace((String) haystack[x], (String) n, (String) replacement, caseSensitive); - } - } - haystackExpr.change(event, haystack, ChangeMode.SET); + Function replaceFunction = getReplaceFunction(needles, (String) replacement); + //noinspection unchecked + ((Expression) haystackExpr).changeInPlace(event, replaceFunction); } else { for (Inventory inv : (Inventory[]) haystack) for (ItemType needle : (ItemType[]) needles) @@ -137,14 +128,36 @@ private void replace(Event event, Object[] needles, Expression haystackExpr) } } } - + + private @NotNull Function getReplaceFunction(Object[] needles, String replacement) { + Function replaceFunction; + if (replaceFirst) { + replaceFunction = haystackString -> { + for (Object needle : needles) { + assert needle != null; + haystackString = StringUtils.replaceFirst(haystackString, (String) needle, Matcher.quoteReplacement(replacement), caseSensitive); + } + return haystackString; + }; + } else { + replaceFunction = haystackString -> { + for (Object needle : needles) { + assert needle != null; + haystackString = StringUtils.replace(haystackString, (String) needle, replacement, caseSensitive); + } + return haystackString; + }; + } + return replaceFunction; + } + @Override public String toString(@Nullable Event event, boolean debug) { if (replaceFirst) - return "replace first " + needles.toString(event, debug) + " in " + haystack.toString(event, debug) + " with " + replacement.toString(event, debug) - + "(case sensitive: " + caseSensitive + ")"; - return "replace " + needles.toString(event, debug) + " in " + haystack.toString(event, debug) + " with " + replacement.toString(event, debug) - + "(case sensitive: " + caseSensitive + ")"; + return "replace first " + needles.toString(event, debug) + " in " + haystack.toString(event, debug) + + " with " + replacement.toString(event, debug) + "(case sensitive: " + caseSensitive + ")"; + return "replace " + needles.toString(event, debug) + " in " + haystack.toString(event, debug) + + " with " + replacement.toString(event, debug) + "(case sensitive: " + caseSensitive + ")"; } } diff --git a/src/main/java/ch/njol/skript/expressions/ExprVectorLength.java b/src/main/java/ch/njol/skript/expressions/ExprVectorLength.java index 6e0af7d63cc..62e5696e098 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprVectorLength.java +++ b/src/main/java/ch/njol/skript/expressions/ExprVectorLength.java @@ -18,18 +18,20 @@ */ package ch.njol.skript.expressions; -import ch.njol.util.VectorMath; -import org.bukkit.event.Event; -import org.bukkit.util.Vector; -import org.jetbrains.annotations.Nullable; - import ch.njol.skript.classes.Changer.ChangeMode; import ch.njol.skript.doc.Description; import ch.njol.skript.doc.Examples; import ch.njol.skript.doc.Name; import ch.njol.skript.doc.Since; import ch.njol.skript.expressions.base.SimplePropertyExpression; +import ch.njol.skript.lang.Expression; +import ch.njol.util.VectorMath; import ch.njol.util.coll.CollectionUtils; +import org.bukkit.event.Event; +import org.bukkit.util.Vector; +import org.jetbrains.annotations.Nullable; + +import java.util.function.Function; @Name("Vectors - Length") @Description("Gets or sets the length of a vector.") @@ -61,39 +63,49 @@ public Class[] acceptChange(ChangeMode mode) { } @Override - public void change(Event event, @Nullable Object[] delta, ChangeMode mode) { + public void change(Event event, Object @Nullable [] delta, ChangeMode mode) { assert delta != null; - final Vector[] vectors = getExpr().getArray(event); double deltaLength = ((Number) delta[0]).doubleValue(); + + Function changeFunction; switch (mode) { case REMOVE: deltaLength = -deltaLength; //$FALL-THROUGH$ case ADD: - for (Vector vector : vectors) { - if (VectorMath.isZero(vector) || (deltaLength < 0 && vector.lengthSquared() < deltaLength * deltaLength)) { + final double finalDeltaLength = deltaLength; + final double finalDeltaLengthSquared = deltaLength * deltaLength; + changeFunction = vector -> { + if (VectorMath.isZero(vector) || (finalDeltaLength < 0 && vector.lengthSquared() < finalDeltaLengthSquared)) { vector.zero(); } else { - double newLength = deltaLength + vector.length(); + double newLength = finalDeltaLength + vector.length(); if (!vector.isNormalized()) vector.normalize(); vector.multiply(newLength); } - } + return vector; + }; break; case SET: - for (Vector vector : vectors) { - if (deltaLength < 0 || VectorMath.isZero(vector)) { + final double finalDeltaLength1 = deltaLength; + changeFunction = vector -> { + if (finalDeltaLength1 < 0 || VectorMath.isZero(vector)) { vector.zero(); } else { if (!vector.isNormalized()) vector.normalize(); - vector.multiply(deltaLength); + vector.multiply(finalDeltaLength1); } - } + return vector; + }; break; + default: + return; } - getExpr().change(event, vectors, ChangeMode.SET); + + //noinspection unchecked,DataFlowIssue + ((Expression) getExpr()).changeInPlace(event, changeFunction); } @Override diff --git a/src/main/java/ch/njol/skript/expressions/ExprVectorXYZ.java b/src/main/java/ch/njol/skript/expressions/ExprVectorXYZ.java index 82a0c03d790..cc2b5deb9cb 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprVectorXYZ.java +++ b/src/main/java/ch/njol/skript/expressions/ExprVectorXYZ.java @@ -18,10 +18,6 @@ */ package ch.njol.skript.expressions; -import org.bukkit.event.Event; -import org.bukkit.util.Vector; -import org.jetbrains.annotations.Nullable; - import ch.njol.skript.classes.Changer; import ch.njol.skript.classes.Changer.ChangeMode; import ch.njol.skript.doc.Description; @@ -33,6 +29,11 @@ import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.util.Kleenean; import ch.njol.util.coll.CollectionUtils; +import org.bukkit.event.Event; +import org.bukkit.util.Vector; +import org.jetbrains.annotations.Nullable; + +import java.util.function.Function; @Name("Vectors - XYZ Component") @Description("Gets or changes the x, y or z component of a vector.") @@ -84,39 +85,45 @@ public Class[] acceptChange(ChangeMode mode) { @Override public void change(Event event, @Nullable Object[] delta, ChangeMode mode) { assert delta != null; - Vector[] vectors = getExpr().getArray(event); - if (vectors.length == 0) - return; double deltaValue = ((Number) delta[0]).doubleValue(); + Function changeFunction; switch (mode) { case REMOVE: deltaValue = -deltaValue; //$FALL-THROUGH$ case ADD: - for (Vector vector : vectors) { - if (axis == 0) - vector.setX(vector.getX() + deltaValue); - else if (axis == 1) - vector.setY(vector.getY() + deltaValue); - else - vector.setZ(vector.getZ() + deltaValue); - } + final double finalDeltaValue1 = deltaValue; + changeFunction = vector -> { + if (axis == 0) { + vector.setX(vector.getX() + finalDeltaValue1); + } else if (axis == 1) { + vector.setY(vector.getY() + finalDeltaValue1); + } else { + vector.setZ(vector.getZ() + finalDeltaValue1); + } + return vector; + }; break; case SET: - for (Vector vector : vectors) { - if (axis == 0) - vector.setX(deltaValue); - else if (axis == 1) - vector.setY(deltaValue); - else - vector.setZ(deltaValue); - } + final double finalDeltaValue = deltaValue; + changeFunction = vector -> { + if (axis == 0) { + vector.setX(finalDeltaValue); + } else if (axis == 1) { + vector.setY(finalDeltaValue); + } else { + vector.setZ(finalDeltaValue); + } + return vector; + }; break; default: assert false; return; } - getExpr().change(event, vectors, ChangeMode.SET); + + //noinspection unchecked,DataFlowIssue + ((Expression) getExpr()).changeInPlace(event, changeFunction); } @Override diff --git a/src/main/java/ch/njol/skript/lang/Expression.java b/src/main/java/ch/njol/skript/lang/Expression.java index da39fcf669a..1f68484e045 100644 --- a/src/main/java/ch/njol/skript/lang/Expression.java +++ b/src/main/java/ch/njol/skript/lang/Expression.java @@ -35,11 +35,14 @@ import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.converter.Converter; +import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Spliterators; +import java.util.function.Function; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -345,6 +348,54 @@ default Map[]> getAcceptedChangeModes() { */ void change(Event event, @Nullable Object[] delta, ChangeMode mode); + /** + * Changes the contents of an expression using the given {@link Function}. + * Intended for changes that change properties of the values of the expression, rather than completely + * changing the expression. For example, {@code set vector length of {_v} to 1}, rather than + * {@code set {_v} to vector(0,1,0)}. + *
+ * This is a 1 to 1 transformation and should not add or remove elements. + * For {@link Variable}s, this will retain indices. For non-{@link Variable}s, it will + * evaluate {@link #getArray(Event)}, apply the change function on each, and call + * {@link #change(Event, Object[], ChangeMode)} with the modified values and {@link ChangeMode#SET}. + * + * @param event The event to use for local variables and evaluation + * @param changeFunction A 1-to-1 function that transforms a single input to a single output. + * @param The output type of the change function. Must be a type returned + * by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}. + */ + default void changeInPlace(Event event, Function changeFunction) { + changeInPlace(event, changeFunction, false); + } + + /** + * Changes the contents of an expression using the given {@link Function}. + * Intended for changes that change properties of the values of the expression, rather than completely + * changing the expression. For example, {@code set vector length of {_v} to 1}, rather than + * {@code set {_v} to vector(0,1,0)}. + *
+ * This is a 1 to 1 transformation and should not add or remove elements. + * For {@link Variable}s, this will retain indices. For non-{@link Variable}s, it will + * evaluate the expression, apply the change function on each value, and call + * {@link #change(Event, Object[], ChangeMode)} with the modified values and {@link ChangeMode#SET}. + * + * @param event The event to use for local variables and evaluation + * @param changeFunction A 1-to-1 function that transforms a single input to a single output. + * @param getAll Whether to evaluate with {@link #getAll(Event)} or {@link #getArray(Event)}. + * @param The output type of the change function. Must be a type returned + * by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}. + */ + default void changeInPlace(Event event, Function changeFunction, boolean getAll) { + T[] values = getAll ? getAll(event) : getArray(event); + if (values.length == 0) + return; + List newValues = new ArrayList<>(); + for (T value : values) { + newValues.add(changeFunction.apply(value)); + } + change(event, newValues.toArray(), ChangeMode.SET); + } + /** * This method is called before this expression is set to another one. * The return value is what will be used for change. You can use modified diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index d85d6140aeb..1b7a55f42b6 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -18,16 +18,6 @@ */ package ch.njol.skript.lang; -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 ch.njol.skript.Skript; import ch.njol.skript.SkriptAPIException; import ch.njol.skript.SkriptConfig; @@ -35,9 +25,6 @@ import ch.njol.skript.classes.Changer.ChangeMode; import ch.njol.skript.classes.Changer.ChangerUtils; import ch.njol.skript.classes.ClassInfo; -import org.skriptlang.skript.lang.arithmetic.Arithmetics; -import org.skriptlang.skript.lang.arithmetic.OperationInfo; -import org.skriptlang.skript.lang.arithmetic.Operator; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.lang.parser.ParserInstance; import ch.njol.skript.lang.util.SimpleExpression; @@ -59,12 +46,26 @@ import org.bukkit.entity.Player; import org.bukkit.event.Event; import org.jetbrains.annotations.Nullable; +import org.skriptlang.skript.lang.arithmetic.Arithmetics; +import org.skriptlang.skript.lang.arithmetic.OperationInfo; +import org.skriptlang.skript.lang.arithmetic.Operator; import org.skriptlang.skript.lang.comparator.Comparators; import org.skriptlang.skript.lang.comparator.Relation; import org.skriptlang.skript.lang.converter.Converters; 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 = ":"; @@ -666,6 +667,25 @@ public void change(Event event, @Nullable Object[] delta, ChangeMode mode) throw } } + @Override + public void changeInPlace(Event event, Function changeFunction) { + if (!list) { + T value = getSingle(event); + if (value == null) + return; + set(event, changeFunction.apply(value)); + return; + } + variablesIterator(event).forEachRemaining(pair -> { + String index = pair.getKey(); + T value = Converters.convert(pair.getValue(), types); + if (value == null) + return; + Object newValue = changeFunction.apply(value); + setIndex(event, index, newValue); + }); + } + @Override @Nullable public T getSingle(Event event) { diff --git a/src/test/skript/tests/regressions/pull-7120-changes overwriting indices.sk b/src/test/skript/tests/regressions/pull-7120-changes overwriting indices.sk new file mode 100644 index 00000000000..47b02990e7a --- /dev/null +++ b/src/test/skript/tests/regressions/pull-7120-changes overwriting indices.sk @@ -0,0 +1,31 @@ +test "EffEnchant overwriting var indices": + set {_test::1} to diamond sword + set {_test::a} to gold sword + set {_test::hello world} to block at spawn of world "world" + enchant {_test::*} with sharpness 1 + assert {_test::*} is diamond sword of sharpness 1, gold sword of sharpness 1, and block at spawn of world "world" with "failed to enchant items" + assert indices of {_test::*} is "1", "a", and "hello world" with "enchanting modified indices" + +test "EffReplace overwriting var indices": + set {_test::1} to "abc" + set {_test::a} to "cde" + set {_test::hello world} to block at spawn of world "world" + replace all "c" in {_test::*} with "z" + assert {_test::*} is "abz", "zde", and block at spawn of world "world" with "failed to replace strings" + assert indices of {_test::*} is "1", "a", and "hello world" with "replacing modified indices" + +test "ExprVectorLength overwriting var indices": + set {_test::1} to vector(0,3,0) + set {_test::a} to vector(0,0,4) + set {_test::hello world} to "abc" + set vector length of {_test::*} to 1 + assert {_test::*} is vector(0,1,0), vector(0,0,1), and "abc" with "failed to change vector length" + assert indices of {_test::*} is "1", "a", and "hello world" with "changing vector length modified indices" + +test "ExprVectorXYZ overwriting var indices": + set {_test::1} to vector(0,3,0) + set {_test::a} to vector(0,0,4) + set {_test::hello world} to "abc" + set x component of {_test::*} to 1 + assert {_test::*} is vector(1,3,0), vector(1,0,4), and "abc" with "failed to change vector x" + assert indices of {_test::*} is "1", "a", and "hello world" with "changing vector x modified indices"