Skip to content

Commit

Permalink
Fix some syntaxes overwriting variable list indices and add new helpe…
Browse files Browse the repository at this point in the history 
…r method for changing expressions. (#7120)

* changeInPlace, fix indices being overwritten, regr tests

* can't count

* requested changes

* Apply suggestions from code review

Co-authored-by: Patrick Miller <[email protected]>

---------

Co-authored-by: Patrick Miller <[email protected]>
  • Loading branch information
@sovdeeth @APickledWalrus
sovdeeth and APickledWalrus authored Nov 23, 2024
1 parent e2ddd35 commit 49b2a37
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 91 deletions.
35 changes: 16 additions & 19 deletions src/main/java/ch/njol/skript/effects/EffEnchant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<ItemType, ItemType> 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;
Expand Down
51 changes: 32 additions & 19 deletions src/main/java/ch/njol/skript/effects/EffReplace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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<String, String> replaceFunction = getReplaceFunction(needles, (String) replacement);
//noinspection unchecked
((Expression<String>) haystackExpr).changeInPlace(event, replaceFunction);
} else {
for (Inventory inv : (Inventory[]) haystack)
for (ItemType needle : (ItemType[]) needles)
Expand All @@ -137,14 +128,36 @@ private void replace(Event event, Object[] needles, Expression<?> haystackExpr)
}
}
}


private @NotNull Function<String, String> getReplaceFunction(Object[] needles, String replacement) {
Function<String, String> 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 + ")";
}

}
44 changes: 28 additions & 16 deletions src/main/java/ch/njol/skript/expressions/ExprVectorLength.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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<Vector, Vector> 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<Vector>) getExpr()).changeInPlace(event, changeFunction);
}

@Override
Expand Down
55 changes: 31 additions & 24 deletions src/main/java/ch/njol/skript/expressions/ExprVectorXYZ.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.")
Expand Down Expand Up @@ -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<Vector, Vector> 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<Vector>) getExpr()).changeInPlace(event, changeFunction);
}

@Override
Expand Down
51 changes: 51 additions & 0 deletions src/main/java/ch/njol/skript/lang/Expression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -345,6 +348,54 @@ default Map<ChangeMode, Class<?>[]> 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)}.
* <br>
* 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 <R> The output type of the change function. Must be a type returned
* by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}.
*/
default <R> void changeInPlace(Event event, Function<T, R> 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)}.
* <br>
* 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 <R> The output type of the change function. Must be a type returned
* by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}.
*/
default <R> void changeInPlace(Event event, Function<T, R> changeFunction, boolean getAll) {
T[] values = getAll ? getAll(event) : getArray(event);
if (values.length == 0)
return;
List<R> 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
Expand Down
Loading

0 comments on commit 49b2a37

Please sign in to comment.