From 3c3f5bc1506996b251111e0bee0311bb39ea5b8f Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Tue, 1 Apr 2025 17:11:41 -0400 Subject: [PATCH 01/12] Initial Commit --- .../ch/njol/skript/effects/EffOperations.java | 104 ++++++++++++++++++ .../tests/syntaxes/effects/EffOperations.sk | 30 +++++ 2 files changed, 134 insertions(+) create mode 100644 src/main/java/ch/njol/skript/effects/EffOperations.java create mode 100644 src/test/skript/tests/syntaxes/effects/EffOperations.sk diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java new file mode 100644 index 00000000000..9dc9b259d81 --- /dev/null +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -0,0 +1,104 @@ +package ch.njol.skript.effects; + +import ch.njol.skript.Skript; +import ch.njol.skript.doc.Description; +import ch.njol.skript.doc.Example; +import ch.njol.skript.doc.Name; +import ch.njol.skript.doc.Since; +import ch.njol.skript.lang.Effect; +import ch.njol.skript.lang.Expression; +import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.SyntaxStringBuilder; +import ch.njol.skript.util.LiteralUtils; +import ch.njol.skript.util.Patterns; +import ch.njol.util.Kleenean; +import org.bukkit.event.Event; +import org.jetbrains.annotations.Nullable; +import org.skriptlang.skript.lang.arithmetic.Arithmetics; +import org.skriptlang.skript.lang.arithmetic.Operation; +import org.skriptlang.skript.lang.arithmetic.Operator; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +@Name("Operations") +@Description("Perform multiplcation, division, or exponentiation operations on variable numbers. Cannot use literals.") +@Example(""" + set {_num} to 1 + multiply {_num} by 10 + """) +@Example(""" + set {_nums::*} to 15, 21 and 30 + divide {_nums::*} by 3 + """) +@Example(""" + set {_num} to 2 + raise {_num} to the power of 5 + """) +@Example(""" + # Will error due to literal + multiply 1 by 2 + """) +@Since("INSERT VERSION") +public class EffOperations extends Effect { + + private static final Patterns patterns = new Patterns<>(new Object[][]{ + {"multiply %~numbers% by %number%", Operator.MULTIPLICATION}, + {"divide %~numbers% by %number%", Operator.DIVISION}, + {"raise %~numbers% to [the] (power|exponent) [of] %number%", Operator.EXPONENTIATION} + }); + + private static final Map> operations = new HashMap<>(); + + static { + Skript.registerEffect(EffOperations.class, patterns.getPatterns()); + } + + private Operator operator; + private Expression baseExpr; + private Expression operativeExpr; + + @Override + public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { + operator = patterns.getInfo(matchedPattern); + //noinspection unchecked + baseExpr = (Expression) exprs[0]; + operativeExpr = LiteralUtils.defendExpression(exprs[1]); + return true; + } + + @Override + protected void execute(Event event) { + Number operativeNumber = operativeExpr.getSingle(event); + assert operativeNumber != null; + Operation operation = getOperation(operator); + assert operation != null; + Function function = number -> operation.calculate(number, operativeNumber); + baseExpr.changeInPlace(event, function); + } + + private static Operation getOperation(Operator operator) { + if (operations.containsKey(operator)) + return operations.get(operator); + Operation operation = Arithmetics.getOperation(operator, Number.class, Number.class, Number.class); + if (operation != null) { + operations.put(operator, operation); + return operation; + } + throw new IllegalStateException("No number operation for operator '" + operator + "'."); + } + + @Override + public String toString(@Nullable Event event, boolean debug) { + SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); + switch (operator) { + case MULTIPLICATION -> builder.append("multiply", baseExpr, "by"); + case DIVISION -> builder.append("divide", baseExpr, "by"); + case EXPONENTIATION -> builder.append("raise", baseExpr, "to the power of"); + } + builder.append(operativeExpr); + return builder.toString(); + } + +} diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk new file mode 100644 index 00000000000..a6d4dc56c69 --- /dev/null +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -0,0 +1,30 @@ +test "operations single variable": + set {_num} to 1 + multiply {_num} by 15 + assert {_num} is 15 with "Failed multiplication for single" + divide {_num} by 5 + assert {_num} is 3 with "Failed division for single" + raise {_num} to the power of 2 + assert {_num} is 9 with "Failed exponentiation for single" + +test "operations list variable": + set {_nums::*} to 2, 3, 4 and 5 + multiply {_nums::*} by 10 + assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for list" + divide {_nums::*} by 5 + assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for list" + raise {_nums::*} to the power of 2 + assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for list" + +test "operations error": + parse: + multiply 1 by 2 + assert last parse logs is set with "Operations should only work for variables - multiplication" + + parse: + divide 20 by 4 + assert last parse logs is set with "Operations should only work for variables - division" + + parse: + raise 3 to the powwer of 100 + assert last parse logs is set with "Operations should only work for variables - exponentiation" From 33829f3931a45ef51ec022d9c416f4ce8716169c Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Fri, 4 Apr 2025 01:14:17 -0400 Subject: [PATCH 02/12] Rewrite --- .../ch/njol/skript/effects/EffOperations.java | 123 +++++++++++++----- .../skript/lang/arithmetic/Arithmetics.java | 54 ++++++++ .../tests/syntaxes/effects/EffOperations.sk | 41 ++++-- 3 files changed, 177 insertions(+), 41 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 9dc9b259d81..7e0c2158de3 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -1,6 +1,8 @@ package ch.njol.skript.effects; import ch.njol.skript.Skript; +import ch.njol.skript.classes.ClassInfo; +import ch.njol.skript.config.Node; import ch.njol.skript.doc.Description; import ch.njol.skript.doc.Example; import ch.njol.skript.doc.Name; @@ -9,17 +11,19 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.lang.SyntaxStringBuilder; +import ch.njol.skript.lang.Variable; +import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.LiteralUtils; import ch.njol.skript.util.Patterns; +import ch.njol.skript.util.Utils; import ch.njol.util.Kleenean; import org.bukkit.event.Event; import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.arithmetic.Arithmetics; import org.skriptlang.skript.lang.arithmetic.Operation; import org.skriptlang.skript.lang.arithmetic.Operator; +import org.skriptlang.skript.log.runtime.SyntaxRuntimeErrorProducer; -import java.util.HashMap; -import java.util.Map; import java.util.function.Function; @Name("Operations") @@ -41,63 +45,116 @@ multiply 1 by 2 """) @Since("INSERT VERSION") -public class EffOperations extends Effect { +public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer { private static final Patterns patterns = new Patterns<>(new Object[][]{ - {"multiply %~numbers% by %number%", Operator.MULTIPLICATION}, - {"divide %~numbers% by %number%", Operator.DIVISION}, - {"raise %~numbers% to [the] (power|exponent) [of] %number%", Operator.EXPONENTIATION} + {"multiply %~objects% by %object%", Operator.MULTIPLICATION}, + {"divide %~objects% by %object%", Operator.DIVISION}, + {"raise %~objects% to [the] (power|exponent) [of] %object%", Operator.EXPONENTIATION} }); - private static final Map> operations = new HashMap<>(); - static { Skript.registerEffect(EffOperations.class, patterns.getPatterns()); } private Operator operator; - private Expression baseExpr; - private Expression operativeExpr; + private Expression base; + private Expression operative; + private Node node; @Override public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { operator = patterns.getInfo(matchedPattern); - //noinspection unchecked - baseExpr = (Expression) exprs[0]; - operativeExpr = LiteralUtils.defendExpression(exprs[1]); - return true; + base = exprs[0]; + if (!(base instanceof Variable)) { + Skript.error("Cannot operate on a non variable object."); + return false; + } + operative = LiteralUtils.defendExpression(exprs[1]); + node = getParser().getNode(); + return LiteralUtils.canInitSafely(operative); } @Override protected void execute(Event event) { - Number operativeNumber = operativeExpr.getSingle(event); - assert operativeNumber != null; - Operation operation = getOperation(operator); - assert operation != null; - Function function = number -> operation.calculate(number, operativeNumber); - baseExpr.changeInPlace(event, function); + Object operativeObject = operative.getSingle(event); + if (operativeObject == null) + return; + ClassInfo operativeClassInfo = Classes.getSuperClassInfo(operativeObject.getClass()); + Class operativeClass = operativeClassInfo.getC(); + Operation operation = null; + Function function = null; + if (base.isSingle()) { + // If the variable provided is single, then we can do some checks + // to ensure an operation is available; if not, we can produce a proper error. + Object baseObject = base.getSingle(event); + if (baseObject == null) + return; + ClassInfo baseClassInfo = Classes.getSuperClassInfo(baseObject.getClass()); + Class baseClass = baseClassInfo.getC(); + if (!baseClass.equals(operativeClass)) { + operation = getOperation(baseClass, operativeClass); + if (operation == null) { + error(Utils.A(baseClassInfo.getCodeName()) + "cannot be " + getOperatorString() + + " with " + Utils.a(operativeClassInfo.getCodeName())); + return; + } + } else { + operation = getOperation(operativeClass); + if (operation == null) { + error(Utils.A(operativeClassInfo.getCodeName()) + " cannot be " + getOperatorString() + "."); + return; + } + } + Operation finalOperation = operation; + function = object -> finalOperation.calculate(object, operativeObject); + } else { + // In the case of a list variable, we should probably ignore throwing multiple errors for each object + // that is not applicable for an operation + function = object -> { + Operation thisOperation = getOperation(object.getClass(), operativeClass); + if (thisOperation != null) + return thisOperation.calculate(object, operativeObject); + return object; + }; + } + //noinspection unchecked,rawtypes + base.changeInPlace(event, (Function) function); } - private static Operation getOperation(Operator operator) { - if (operations.containsKey(operator)) - return operations.get(operator); - Operation operation = Arithmetics.getOperation(operator, Number.class, Number.class, Number.class); - if (operation != null) { - operations.put(operator, operation); - return operation; - } - throw new IllegalStateException("No number operation for operator '" + operator + "'."); + private @Nullable Operation getOperation(Class type) { + //noinspection unchecked + return (Operation) Arithmetics.getConvertedOperation(operator, type, type, type); + } + + private @Nullable Operation getOperation(Class leftClass, Class rightClass) { + //noinspection unchecked + return (Operation) Arithmetics.getConvertedOperation(operator, leftClass, rightClass, leftClass); + } + + @Override + public Node getNode() { + return node; + } + + private String getOperatorString() { + return switch (operator) { + case MULTIPLICATION -> "multiplied"; + case DIVISION -> "divided"; + case EXPONENTIATION -> "exponentiated"; + default -> ""; + }; } @Override public String toString(@Nullable Event event, boolean debug) { SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); switch (operator) { - case MULTIPLICATION -> builder.append("multiply", baseExpr, "by"); - case DIVISION -> builder.append("divide", baseExpr, "by"); - case EXPONENTIATION -> builder.append("raise", baseExpr, "to the power of"); + case MULTIPLICATION -> builder.append("multiply", base, "by"); + case DIVISION -> builder.append("divide", base, "by"); + case EXPONENTIATION -> builder.append("raise", base, "to the power of"); } - builder.append(operativeExpr); + builder.append(operative); return builder.toString(); } diff --git a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java index a30cd499133..f248a082061 100644 --- a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java +++ b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java @@ -123,6 +123,60 @@ public static Operation getOperation(Operator operator, Class return info == null ? null : info.getOperation(); } + /** + * @see #getConvertedOperation(Operator, Class, Class, Class) + */ + public static @Nullable Operation getConvertedOperation(Operator operator, Class type) { + //noinspection unchecked + return (Operation) getConvertedOperation(operator, type, type, type); + } + + /** + * @see #getConvertedOperation(Operator, Class, Class, Class) + */ + public static @Nullable Operation getConvertedOperation(Operator operator, Class leftClass, Class rightClass) { + return getConvertedOperation(operator, leftClass, rightClass, leftClass); + } + + /** + *

+ * This method attempts to convert all parameters + * i.e. {@code leftClass}, {@code rightClass} and {@code returnType} + * into the expected operation. + *

+ */ + public static @Nullable Operation getConvertedOperation( + Operator operator, + Class leftClass, + Class rightClass, + Class returnType + ) { + OperationInfo exactInfo = getOperationInfo(operator, leftClass, rightClass, returnType); + if (exactInfo != null) + return exactInfo.getOperation(); + OperationInfo closestInfo = null; + for (OperationInfo info : getOperations(operator)) { + if (info.getLeft().isAssignableFrom(leftClass) + && info.getRight().isAssignableFrom(rightClass) + && info.getReturnType().isAssignableFrom(returnType) + ) { + if (closestInfo == null || ( + closestInfo.getLeft().isAssignableFrom(info.getLeft()) + && closestInfo.getRight().isAssignableFrom(info.getRight()) + && closestInfo.getReturnType().isAssignableFrom(info.getReturnType())) + ) { + closestInfo = info; + } + } + } + if (closestInfo == null) + return null; + //noinspection unchecked + Operation convertedOperation = (Operation) closestInfo.getOperation(); + getOperations_i(operator).add(new OperationInfo<>(leftClass, rightClass, returnType, convertedOperation)); + return convertedOperation; + } + @Nullable public static OperationInfo lookupOperationInfo(Operator operator, Class leftClass, Class rightClass, Class returnType) { OperationInfo info = lookupOperationInfo(operator, leftClass, rightClass); diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index a6d4dc56c69..57bee4e4c94 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -1,20 +1,45 @@ -test "operations single variable": +test "operations numbers": set {_num} to 1 multiply {_num} by 15 - assert {_num} is 15 with "Failed multiplication for single" + assert {_num} is 15 with "Failed multiplication for single number" divide {_num} by 5 - assert {_num} is 3 with "Failed division for single" + assert {_num} is 3 with "Failed division for single number" raise {_num} to the power of 2 - assert {_num} is 9 with "Failed exponentiation for single" + assert {_num} is 9 with "Failed exponentiation for single number" -test "operations list variable": set {_nums::*} to 2, 3, 4 and 5 multiply {_nums::*} by 10 - assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for list" + assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for number list" divide {_nums::*} by 5 - assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for list" + assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for number list" raise {_nums::*} to the power of 2 - assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for list" + assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for number list" + +test "operations vectors by vectors": + set {_vector} to vector(1,1,1) + multiply {_vector} by vector(4,6,8) + assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector" + divide {_vector} by vector(2,3,4) + assert {_vector} is vector(2,2,2) with "Failed division for single vector" + + set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) + multiply {_vectors::*} by vector(2,5,10) + assert {_vectors::*} is (vector(4,10,20), vector(6,15,30) and vector(8,20,40)) with "Failed multiplication for vector list" + divide {_vectors::*} by vector(1,5,5) + assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list" + +test "operations vectors by numbers": + set {_vector} to vector(1,1,1) + multiply {_vector} by 10 + assert {_vector} is vector(10,10,10) with "Failed multiplication for single vector by number" + divide {_vector} by 2 + assert {_vector} is vector(5,5,5) with "Failed division for single vector by number" + + set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) + multiply {_vectors::*} by 4 + assert {_vectors::*} is (vector(8,8,8), vector(12,12,12) and vector(16,16,16)) with "Failed multiplication for vector list by number" + divide {_vectors::*} by 2 + assert {_vectors::*} is (vector(4,4,4), vector(6,6,6) and vector(8,8,8)) with "Failed division for vector list by number" test "operations error": parse: From ac8dd10d8f3eeb1624e83c340d22964ec9e3fc1d Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Fri, 4 Apr 2025 01:33:19 -0400 Subject: [PATCH 03/12] Update Docs+Test --- .../ch/njol/skript/effects/EffOperations.java | 16 +++++++++++--- .../tests/syntaxes/effects/EffOperations.sk | 21 +++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 7e0c2158de3..3bb8fad43a2 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -27,18 +27,28 @@ import java.util.function.Function; @Name("Operations") -@Description("Perform multiplcation, division, or exponentiation operations on variable numbers. Cannot use literals.") +@Description("Perform multiplication, division, or exponentiation operations on variable objects " + + "(i.e. numbers, vectors, timespans, and other objects from addons). Cannot use literals.") @Example(""" set {_num} to 1 multiply {_num} by 10 + divide {_num} by 5 + raise {_num} to the power of 2 """) @Example(""" set {_nums::*} to 15, 21 and 30 divide {_nums::*} by 3 + multiply {_nums::*} by 5 + raise {_nums::*} to the power of 3 """) @Example(""" - set {_num} to 2 - raise {_num} to the power of 5 + set {_vector} to vector(1,1,1) + multiply {_vector} by vector(4,8,16) + divide {_vector} by 2 + """) +@Example(""" + set {_timespan} to 1 hour + multiply {_timespan} by 3 """) @Example(""" # Will error due to literal diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index 57bee4e4c94..7d8a7285c2d 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -18,15 +18,15 @@ test "operations numbers": test "operations vectors by vectors": set {_vector} to vector(1,1,1) multiply {_vector} by vector(4,6,8) - assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector" + assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector by vector" divide {_vector} by vector(2,3,4) - assert {_vector} is vector(2,2,2) with "Failed division for single vector" + assert {_vector} is vector(2,2,2) with "Failed division for single vector by vector" set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) multiply {_vectors::*} by vector(2,5,10) - assert {_vectors::*} is (vector(4,10,20), vector(6,15,30) and vector(8,20,40)) with "Failed multiplication for vector list" + assert {_vectors::*} is (vector(4,10,20), vector(6,15,30) and vector(8,20,40)) with "Failed multiplication for vector list by vector" divide {_vectors::*} by vector(1,5,5) - assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list" + assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list by vector" test "operations vectors by numbers": set {_vector} to vector(1,1,1) @@ -41,6 +41,19 @@ test "operations vectors by numbers": divide {_vectors::*} by 2 assert {_vectors::*} is (vector(4,4,4), vector(6,6,6) and vector(8,8,8)) with "Failed division for vector list by number" +test "operations timespans by numbers": + set {_timespan} to 1 hour + multiply {_timespan} by 4 + assert {_timespan} is 4 hours with "Failed multiplication for single timespan by number" + divide {_timespan} by 2 + assert {_timespan} is 2 hours with "Failed division for single timespan by number" + + set {_timespans::*} to (1 minute), (5 minutes) and (10 minutes) + multiply {_timespans::*} by 5 + assert {_timespans::*} is ((5 minutes), (25 minutes) and (50 minutes)) with "Failed multiplication for timespan list by number" + divide {_timespans::*} by 2 + assert {_timespans::*} is ((2 minutes and 30 seconds), (12 minutes and 30 seconds) and (25 minutes)) with "Failed division for timespan list by number" + test "operations error": parse: multiply 1 by 2 From 834996f7f2a366ff3574f673422adb08d6408784 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Fri, 4 Apr 2025 01:40:09 -0400 Subject: [PATCH 04/12] Update Arithmetics.java --- .../org/skriptlang/skript/lang/arithmetic/Arithmetics.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java index f248a082061..804f55c2615 100644 --- a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java +++ b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java @@ -126,9 +126,8 @@ public static Operation getOperation(Operator operator, Class /** * @see #getConvertedOperation(Operator, Class, Class, Class) */ - public static @Nullable Operation getConvertedOperation(Operator operator, Class type) { - //noinspection unchecked - return (Operation) getConvertedOperation(operator, type, type, type); + public static @Nullable Operation getConvertedOperation(Operator operator, Class type) { + return getConvertedOperation(operator, type, type, type); } /** From 1728c885a5b64f980b643f274e055e221319ac5a Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Sat, 5 Apr 2025 19:14:15 -0400 Subject: [PATCH 05/12] Partial Changes --- .../ch/njol/skript/effects/EffOperations.java | 60 ++++++++----------- .../skript/lang/arithmetic/Arithmetics.java | 55 +---------------- .../tests/syntaxes/effects/EffOperations.sk | 16 ++--- 3 files changed, 34 insertions(+), 97 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 3bb8fad43a2..4effeba1e9a 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -1,7 +1,7 @@ package ch.njol.skript.effects; import ch.njol.skript.Skript; -import ch.njol.skript.classes.ClassInfo; +import ch.njol.skript.classes.Changer.ChangeMode; import ch.njol.skript.config.Node; import ch.njol.skript.doc.Description; import ch.njol.skript.doc.Example; @@ -11,7 +11,6 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.lang.SyntaxStringBuilder; -import ch.njol.skript.lang.Variable; import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.LiteralUtils; import ch.njol.skript.util.Patterns; @@ -27,8 +26,9 @@ import java.util.function.Function; @Name("Operations") -@Description("Perform multiplication, division, or exponentiation operations on variable objects " + - "(i.e. numbers, vectors, timespans, and other objects from addons). Cannot use literals.") +@Description("Perform multiplication, division, or exponentiation operations on variable objects " + + "(i.e. numbers, vectors, timespans, and other objects from addons). " + + "Literals cannot be used on the left-hand side.") @Example(""" set {_num} to 1 multiply {_num} by 10 @@ -53,18 +53,19 @@ @Example(""" # Will error due to literal multiply 1 by 2 + divide 10 by {_num} """) @Since("INSERT VERSION") public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer { - private static final Patterns patterns = new Patterns<>(new Object[][]{ + private static final Patterns PATTERNS = new Patterns<>(new Object[][]{ {"multiply %~objects% by %object%", Operator.MULTIPLICATION}, {"divide %~objects% by %object%", Operator.DIVISION}, {"raise %~objects% to [the] (power|exponent) [of] %object%", Operator.EXPONENTIATION} }); static { - Skript.registerEffect(EffOperations.class, patterns.getPatterns()); + Skript.registerEffect(EffOperations.class, PATTERNS.getPatterns()); } private Operator operator; @@ -74,10 +75,10 @@ public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer @Override public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { - operator = patterns.getInfo(matchedPattern); + operator = PATTERNS.getInfo(matchedPattern); base = exprs[0]; - if (!(base instanceof Variable)) { - Skript.error("Cannot operate on a non variable object."); + if (base.acceptChange(ChangeMode.SET) == null) { + Skript.error("This expression cannot be operated on."); return false; } operative = LiteralUtils.defendExpression(exprs[1]); @@ -90,38 +91,28 @@ protected void execute(Event event) { Object operativeObject = operative.getSingle(event); if (operativeObject == null) return; - ClassInfo operativeClassInfo = Classes.getSuperClassInfo(operativeObject.getClass()); - Class operativeClass = operativeClassInfo.getC(); + Class operativeClass = operativeObject.getClass(); Operation operation = null; - Function function = null; + Function changerFunction = null; if (base.isSingle()) { // If the variable provided is single, then we can do some checks // to ensure an operation is available; if not, we can produce a proper error. Object baseObject = base.getSingle(event); if (baseObject == null) return; - ClassInfo baseClassInfo = Classes.getSuperClassInfo(baseObject.getClass()); - Class baseClass = baseClassInfo.getC(); - if (!baseClass.equals(operativeClass)) { - operation = getOperation(baseClass, operativeClass); - if (operation == null) { - error(Utils.A(baseClassInfo.getCodeName()) + "cannot be " + getOperatorString() - + " with " + Utils.a(operativeClassInfo.getCodeName())); - return; - } - } else { - operation = getOperation(operativeClass); - if (operation == null) { - error(Utils.A(operativeClassInfo.getCodeName()) + " cannot be " + getOperatorString() + "."); - return; - } + Class baseClass = baseObject.getClass(); + operation = getOperation(baseClass, operativeClass); + if (operation == null) { + error(Utils.A(getClassInfoCodeName(baseClass)) + " cannot be " + getOperatorString() + + " with " + Utils.a(getClassInfoCodeName(operativeClass))); + return; } Operation finalOperation = operation; - function = object -> finalOperation.calculate(object, operativeObject); + changerFunction = object -> finalOperation.calculate(object, operativeObject); } else { // In the case of a list variable, we should probably ignore throwing multiple errors for each object // that is not applicable for an operation - function = object -> { + changerFunction = object -> { Operation thisOperation = getOperation(object.getClass(), operativeClass); if (thisOperation != null) return thisOperation.calculate(object, operativeObject); @@ -129,17 +120,16 @@ protected void execute(Event event) { }; } //noinspection unchecked,rawtypes - base.changeInPlace(event, (Function) function); + base.changeInPlace(event, (Function) changerFunction); } - private @Nullable Operation getOperation(Class type) { + private @Nullable Operation getOperation(Class leftClass, Class rightClass) { //noinspection unchecked - return (Operation) Arithmetics.getConvertedOperation(operator, type, type, type); + return (Operation) Arithmetics.getOperation(operator, leftClass, rightClass, leftClass); } - private @Nullable Operation getOperation(Class leftClass, Class rightClass) { - //noinspection unchecked - return (Operation) Arithmetics.getConvertedOperation(operator, leftClass, rightClass, leftClass); + private String getClassInfoCodeName(Class clazz) { + return Classes.getSuperClassInfo(clazz).getCodeName(); } @Override diff --git a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java index 804f55c2615..860b682b064 100644 --- a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java +++ b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java @@ -87,7 +87,7 @@ public static boolean operationExists(Operator operator, Class leftClass, Cla @SuppressWarnings("unchecked") public static OperationInfo getOperationInfo(Operator operator, Class leftClass, Class rightClass, Class returnType) { OperationInfo info = getOperationInfo(operator, leftClass, rightClass); - if (info != null && returnType.isAssignableFrom(info.getReturnType())) + if (info != null && (returnType.isAssignableFrom(info.getReturnType()) || info.getReturnType().isAssignableFrom(returnType))) return (OperationInfo) info; return null; } @@ -123,59 +123,6 @@ public static Operation getOperation(Operator operator, Class return info == null ? null : info.getOperation(); } - /** - * @see #getConvertedOperation(Operator, Class, Class, Class) - */ - public static @Nullable Operation getConvertedOperation(Operator operator, Class type) { - return getConvertedOperation(operator, type, type, type); - } - - /** - * @see #getConvertedOperation(Operator, Class, Class, Class) - */ - public static @Nullable Operation getConvertedOperation(Operator operator, Class leftClass, Class rightClass) { - return getConvertedOperation(operator, leftClass, rightClass, leftClass); - } - - /** - *

- * This method attempts to convert all parameters - * i.e. {@code leftClass}, {@code rightClass} and {@code returnType} - * into the expected operation. - *

- */ - public static @Nullable Operation getConvertedOperation( - Operator operator, - Class leftClass, - Class rightClass, - Class returnType - ) { - OperationInfo exactInfo = getOperationInfo(operator, leftClass, rightClass, returnType); - if (exactInfo != null) - return exactInfo.getOperation(); - OperationInfo closestInfo = null; - for (OperationInfo info : getOperations(operator)) { - if (info.getLeft().isAssignableFrom(leftClass) - && info.getRight().isAssignableFrom(rightClass) - && info.getReturnType().isAssignableFrom(returnType) - ) { - if (closestInfo == null || ( - closestInfo.getLeft().isAssignableFrom(info.getLeft()) - && closestInfo.getRight().isAssignableFrom(info.getRight()) - && closestInfo.getReturnType().isAssignableFrom(info.getReturnType())) - ) { - closestInfo = info; - } - } - } - if (closestInfo == null) - return null; - //noinspection unchecked - Operation convertedOperation = (Operation) closestInfo.getOperation(); - getOperations_i(operator).add(new OperationInfo<>(leftClass, rightClass, returnType, convertedOperation)); - return convertedOperation; - } - @Nullable public static OperationInfo lookupOperationInfo(Operator operator, Class leftClass, Class rightClass, Class returnType) { OperationInfo info = lookupOperationInfo(operator, leftClass, rightClass); diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index 7d8a7285c2d..9c9f33c621b 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -1,4 +1,4 @@ -test "operations numbers": +test "operations effect numbers": set {_num} to 1 multiply {_num} by 15 assert {_num} is 15 with "Failed multiplication for single number" @@ -15,7 +15,7 @@ test "operations numbers": raise {_nums::*} to the power of 2 assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for number list" -test "operations vectors by vectors": +test "operations effect vectors by vectors": set {_vector} to vector(1,1,1) multiply {_vector} by vector(4,6,8) assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector by vector" @@ -28,7 +28,7 @@ test "operations vectors by vectors": divide {_vectors::*} by vector(1,5,5) assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list by vector" -test "operations vectors by numbers": +test "operations effect vectors by numbers": set {_vector} to vector(1,1,1) multiply {_vector} by 10 assert {_vector} is vector(10,10,10) with "Failed multiplication for single vector by number" @@ -41,7 +41,7 @@ test "operations vectors by numbers": divide {_vectors::*} by 2 assert {_vectors::*} is (vector(4,4,4), vector(6,6,6) and vector(8,8,8)) with "Failed division for vector list by number" -test "operations timespans by numbers": +test "operations effect timespans by numbers": set {_timespan} to 1 hour multiply {_timespan} by 4 assert {_timespan} is 4 hours with "Failed multiplication for single timespan by number" @@ -54,15 +54,15 @@ test "operations timespans by numbers": divide {_timespans::*} by 2 assert {_timespans::*} is ((2 minutes and 30 seconds), (12 minutes and 30 seconds) and (25 minutes)) with "Failed division for timespan list by number" -test "operations error": +test "operations effect literals error": parse: multiply 1 by 2 - assert last parse logs is set with "Operations should only work for variables - multiplication" + assert last parse logs is set with "Operations should not work for literals - multiplication" parse: divide 20 by 4 - assert last parse logs is set with "Operations should only work for variables - division" + assert last parse logs is set with "Operations should not work for literals - division" parse: raise 3 to the powwer of 100 - assert last parse logs is set with "Operations should only work for variables - exponentiation" + assert last parse logs is set with "Operations should not work for literals - exponentiation" From a18a491b0b518131453b49c37d2d3cc3abbeed16 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Sat, 5 Apr 2025 23:49:24 -0400 Subject: [PATCH 06/12] Additional Partial Changes --- .../ch/njol/skript/effects/EffOperations.java | 79 ++++++++--- .../tests/syntaxes/effects/EffOperations.sk | 126 ++++++++++++++---- 2 files changed, 159 insertions(+), 46 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 4effeba1e9a..63bccb34b66 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -7,10 +7,9 @@ import ch.njol.skript.doc.Example; import ch.njol.skript.doc.Name; import ch.njol.skript.doc.Since; -import ch.njol.skript.lang.Effect; -import ch.njol.skript.lang.Expression; +import ch.njol.skript.lang.*; import ch.njol.skript.lang.SkriptParser.ParseResult; -import ch.njol.skript.lang.SyntaxStringBuilder; +import ch.njol.skript.lang.util.SimpleExpression; import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.LiteralUtils; import ch.njol.skript.util.Patterns; @@ -72,16 +71,33 @@ public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer private Expression base; private Expression operative; private Node node; + private Operation operation = null; @Override public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { operator = PATTERNS.getInfo(matchedPattern); base = exprs[0]; - if (base.acceptChange(ChangeMode.SET) == null) { + if (!isOperable(null)) { Skript.error("This expression cannot be operated on."); return false; } operative = LiteralUtils.defendExpression(exprs[1]); + if (operative instanceof Literal || operative instanceof SimpleExpression) { + Class operativeType = operative.getReturnType(); + if (!isOperable(operativeType)) { + Skript.error("This expression cannot be " + getOperatorString() + " by " + + Utils.a(getClassInfoCodeName(operativeType)) + "."); + return false; + } + if (base instanceof SimpleExpression && !base.getReturnType().equals(Object.class)) { + operation = getOperation(base.getReturnType(), operative.getReturnType()); + if (operation == null) { + Skript.error("This expression cannot be " + getOperatorString() + " by " + + Utils.a(getClassInfoCodeName(operative.getReturnType())) + "."); + return false; + } + } + } node = getParser().getNode(); return LiteralUtils.canInitSafely(operative); } @@ -92,20 +108,22 @@ protected void execute(Event event) { if (operativeObject == null) return; Class operativeClass = operativeObject.getClass(); - Operation operation = null; + Operation operation = this.operation; Function changerFunction = null; - if (base.isSingle()) { - // If the variable provided is single, then we can do some checks - // to ensure an operation is available; if not, we can produce a proper error. - Object baseObject = base.getSingle(event); - if (baseObject == null) - return; - Class baseClass = baseObject.getClass(); - operation = getOperation(baseClass, operativeClass); + if (base.isSingle() || (base instanceof SimpleExpression && !base.getReturnType().equals(Object.class))) { if (operation == null) { - error(Utils.A(getClassInfoCodeName(baseClass)) + " cannot be " + getOperatorString() - + " with " + Utils.a(getClassInfoCodeName(operativeClass))); - return; + // If the variable provided is single, then we can do some checks + // to ensure an operation is available; if not, we can produce a proper error. + Object baseObject = base.getSingle(event); + if (baseObject == null) + return; + Class baseClass = baseObject.getClass(); + operation = getOperation(baseClass, operativeClass); + if (operation == null) { + error(Utils.A(getClassInfoCodeName(baseClass)) + " cannot be " + getOperatorString() + + " with " + Utils.a(getClassInfoCodeName(operativeClass)) + "."); + return; + } } Operation finalOperation = operation; changerFunction = object -> finalOperation.calculate(object, operativeObject); @@ -128,15 +146,15 @@ protected void execute(Event event) { return (Operation) Arithmetics.getOperation(operator, leftClass, rightClass, leftClass); } - private String getClassInfoCodeName(Class clazz) { - return Classes.getSuperClassInfo(clazz).getCodeName(); - } - @Override public Node getNode() { return node; } + private String getClassInfoCodeName(Class clazz) { + return Classes.getSuperClassInfo(clazz).getCodeName(); + } + private String getOperatorString() { return switch (operator) { case MULTIPLICATION -> "multiplied"; @@ -146,6 +164,27 @@ private String getOperatorString() { }; } + private boolean isOperable(@Nullable Class objectClass) { + Class[] classes = base.acceptChange(ChangeMode.SET); + if (classes == null) { + return false; + } else if (classes.length == 0) { + throw new IllegalStateException("A ChangeMode of 'SET' should never return an empty array."); + } + if (objectClass == null) + return true; + if (base.isSingle() || base instanceof SimpleExpression) { + for (Class clazz : classes) { + if (clazz.isAssignableFrom(objectClass)) { + return true; + } + } + } else if (base instanceof Variable) { + return true; + } + return false; + } + @Override public String toString(@Nullable Event event, boolean debug) { SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index 9c9f33c621b..02faea23e98 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -1,68 +1,142 @@ -test "operations effect numbers": +test "operations effect numbers by literal numbers": set {_num} to 1 multiply {_num} by 15 - assert {_num} is 15 with "Failed multiplication for single number" + assert {_num} is 15 with "Failed multiplication for single number by literal number" divide {_num} by 5 - assert {_num} is 3 with "Failed division for single number" + assert {_num} is 3 with "Failed division for single number by literal number" raise {_num} to the power of 2 - assert {_num} is 9 with "Failed exponentiation for single number" + assert {_num} is 9 with "Failed exponentiation for single number by literal number" set {_nums::*} to 2, 3, 4 and 5 multiply {_nums::*} by 10 - assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for number list" + assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for number list by literal number" divide {_nums::*} by 5 - assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for number list" + assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for number list by literal number" raise {_nums::*} to the power of 2 - assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for number list" + assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for number list by literal number" -test "operations effect vectors by vectors": +test "operations effect numbers by variable numbers": + set {_num} to 1 + set {_multiply} to 10 + set {_divide} to 5 + set {_raise} to 2 + multiply {_num} by {_multiply} + assert {_num} is 10 with "Failed multiplication for single number by variable number" + divide {_num} by {_divide} + assert {_num} is 2 with "Failed division for single number by variable number" + raise {_num} to the power of {_raise} + assert {_num} is 4 with "Failed exponentiation for single number by variable number" + + set {_nums::*} to 2, 3, 4 and 5 + multiply {_nums::*} by {_multiply} + assert {_nums::*} is (20, 30, 40 and 50) with "Failed multiplication for number list by variable number" + divide {_nums::*} by {_divide} + assert {_nums::*} is (4, 6, 8 and 10) with "Failed division for number list by variable number" + raise {_nums::*} to the power of {_raise} + assert {_nums::*} is (16, 36, 64 and 100) with "Failed exponentiation for number list by variable number" + +test "operations effect vectors by function vectors": set {_vector} to vector(1,1,1) multiply {_vector} by vector(4,6,8) - assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector by vector" + assert {_vector} is vector(4,6,8) with "Failed multiplication for single vector by function vector" divide {_vector} by vector(2,3,4) - assert {_vector} is vector(2,2,2) with "Failed division for single vector by vector" + assert {_vector} is vector(2,2,2) with "Failed division for single vector by function vector" set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) multiply {_vectors::*} by vector(2,5,10) - assert {_vectors::*} is (vector(4,10,20), vector(6,15,30) and vector(8,20,40)) with "Failed multiplication for vector list by vector" + assert {_vectors::*} is (vector(4,10,20), vector(6,15,30) and vector(8,20,40)) with "Failed multiplication for vector list by function vector" divide {_vectors::*} by vector(1,5,5) - assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list by vector" + assert {_vectors::*} is (vector(4,2,4), vector(6,3,6) and vector(8,4,8)) with "Failed division for vector list by function vector" + +test "operations effect vectors by variable vectors": + set {_vector} to vector(1,1,1) + set {_multiply} to vector(10,20,30) + set {_divide} to vector(5,5,5) + multiply {_vector} by {_multiply} + assert {_vector} is vector(10,20,30) with "Failed multiplication for single vector by vector" + divide {_vector} by {_divide} + assert {_vector} is vector(2,4,6) with "Failed division for single vector by vector" + + set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) + multiply {_vectors::*} by {_multiply} + assert {_vectors::*} is (vector(20,40,60), vector(30,60,90) and vector(40,80,120)) with "Failed multiplication for vector list by variable vector" + divide {_vectors::*} by {_divide} + assert {_vectors::*} is (vector(4,8,12), vector(6,12,18) and vector(8,16,24)) with "Failed division for vector list by variable vector" -test "operations effect vectors by numbers": +test "operations effect vectors by literal numbers": set {_vector} to vector(1,1,1) multiply {_vector} by 10 - assert {_vector} is vector(10,10,10) with "Failed multiplication for single vector by number" + assert {_vector} is vector(10,10,10) with "Failed multiplication for single vector by literal number" divide {_vector} by 2 - assert {_vector} is vector(5,5,5) with "Failed division for single vector by number" + assert {_vector} is vector(5,5,5) with "Failed division for single vector by literal number" set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) multiply {_vectors::*} by 4 - assert {_vectors::*} is (vector(8,8,8), vector(12,12,12) and vector(16,16,16)) with "Failed multiplication for vector list by number" + assert {_vectors::*} is (vector(8,8,8), vector(12,12,12) and vector(16,16,16)) with "Failed multiplication for vector list by literal number" divide {_vectors::*} by 2 - assert {_vectors::*} is (vector(4,4,4), vector(6,6,6) and vector(8,8,8)) with "Failed division for vector list by number" + assert {_vectors::*} is (vector(4,4,4), vector(6,6,6) and vector(8,8,8)) with "Failed division for vector list by literal number" + +test "operations effect vectors by variable numbers": + set {_vector} to vector(1,1,1) + set {_multiply} to 15 + set {_divide} to 3 + multiply {_vector} by {_multiply} + assert {_vector} is vector(15,15,15) with "Failed multiplication for single vector by variable number" + divide {_vector} by {_divide} + assert {_vector} is vector(5,5,5) with "Failed division for single vector by variable number" -test "operations effect timespans by numbers": + set {_vectors::*} to vector(2,2,2), vector(3,3,3) and vector(4,4,4) + multiply {_vectors::*} by {_multiply} + assert {_vectors::*} is (vector(30,30,30), vector(45,45,45) and vector(60,60,60)) with "Failed multiplication for vector list by variable number" + divide {_vectors::*} by {_divide} + assert {_vectors::*} is (vector(10,10,10), vector(15,15,15) and vector(20,20,20)) with "Failed division for vector list by variable number" + +test "operations effect timespans by literal numbers": set {_timespan} to 1 hour multiply {_timespan} by 4 - assert {_timespan} is 4 hours with "Failed multiplication for single timespan by number" + assert {_timespan} is 4 hours with "Failed multiplication for single timespan by literal number" divide {_timespan} by 2 - assert {_timespan} is 2 hours with "Failed division for single timespan by number" + assert {_timespan} is 2 hours with "Failed division for single timespan by literal number" set {_timespans::*} to (1 minute), (5 minutes) and (10 minutes) multiply {_timespans::*} by 5 - assert {_timespans::*} is ((5 minutes), (25 minutes) and (50 minutes)) with "Failed multiplication for timespan list by number" + assert {_timespans::*} is ((5 minutes), (25 minutes) and (50 minutes)) with "Failed multiplication for timespan list by literal number" divide {_timespans::*} by 2 - assert {_timespans::*} is ((2 minutes and 30 seconds), (12 minutes and 30 seconds) and (25 minutes)) with "Failed division for timespan list by number" + assert {_timespans::*} is ((2 minutes and 30 seconds), (12 minutes and 30 seconds) and (25 minutes)) with "Failed division for timespan list by literal number" + +test "operations effect timespans by variable numbers": + set {_timespan} to 1 hour + set {_multiply} to 6 + set {_divide} to 2 + multiply {_timespan} by {_multiply} + assert {_timespan} is 6 hours with "Failed multiplication for single timespan by variable number" + divide {_timespan} by {_divide} + assert {_timespan} is 3 hours with "Failed division for single timespan by variable number" + + set {_timespans::*} to (1 minute), (5 minutes) and (10 minutes) + multiply {_timespans::*} by {_multiply} + assert {_timespans::*} is ((6 minutes), (30 minutes) and (60 minutes)) with "Failed multiplication for timespan list by variable number" + divide {_timespans::*} by {_divide} + assert {_timespans::*} is ((3 minutes), (15 minutes) and (30 minutes)) with "Failed division for timespan list by variable number" test "operations effect literals error": parse: multiply 1 by 2 - assert last parse logs is set with "Operations should not work for literals - multiplication" + assert last parse logs is set with "Operations should not work for left side literals - multiplication" parse: divide 20 by 4 - assert last parse logs is set with "Operations should not work for literals - division" + assert last parse logs is set with "Operations should not work for left side literals - division" + + parse: + raise 3 to the power of 100 + assert last parse logs is set with "Operations should not work for left side literals - exponentiation" + +test "operations effect expression errors": + parse: + multiply (the hunger level of (random element out of all players)) by 1 hour + assert last parse logs contains "This expression cannot be multiplied by a timespan." with "Hunger level should not be able to be multiplied by timespan" parse: - raise 3 to the powwer of 100 - assert last parse logs is set with "Operations should not work for literals - exponentiation" + divide all worlds by 2 + assert last parse logs contains "This expression cannot be operated on." with "ExprWorlds should not have a 'SET' ChangeMode" From 858d6ea15a33110e35c6abe05d659f49f091c353 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Sun, 6 Apr 2025 19:02:26 -0400 Subject: [PATCH 07/12] Update EffOperations.java --- .../ch/njol/skript/effects/EffOperations.java | 103 ++++++++++-------- 1 file changed, 56 insertions(+), 47 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 63bccb34b66..28ea6bdcf87 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -7,9 +7,10 @@ import ch.njol.skript.doc.Example; import ch.njol.skript.doc.Name; import ch.njol.skript.doc.Since; -import ch.njol.skript.lang.*; +import ch.njol.skript.lang.Effect; +import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; -import ch.njol.skript.lang.util.SimpleExpression; +import ch.njol.skript.lang.SyntaxStringBuilder; import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.LiteralUtils; import ch.njol.skript.util.Patterns; @@ -19,6 +20,7 @@ import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.arithmetic.Arithmetics; import org.skriptlang.skript.lang.arithmetic.Operation; +import org.skriptlang.skript.lang.arithmetic.OperationInfo; import org.skriptlang.skript.lang.arithmetic.Operator; import org.skriptlang.skript.log.runtime.SyntaxRuntimeErrorProducer; @@ -77,28 +79,53 @@ public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { operator = PATTERNS.getInfo(matchedPattern); base = exprs[0]; - if (!isOperable(null)) { + Class[] baseAccepted = base.acceptChange(ChangeMode.SET); + if (baseAccepted == null) { Skript.error("This expression cannot be operated on."); return false; } operative = LiteralUtils.defendExpression(exprs[1]); - if (operative instanceof Literal || operative instanceof SimpleExpression) { - Class operativeType = operative.getReturnType(); - if (!isOperable(operativeType)) { + node = getParser().getNode(); + Class operativeType = operative.getReturnType(); + Class baseType = base.getReturnType(); + // Ensure 'baseType' is referencing a non-array class + if (baseType.isArray()) + baseType = baseType.getComponentType(); + if (operativeType.equals(Object.class) && baseType.equals(Object.class)) { + // Both are object, so we should do the checks within '#execute' + return LiteralUtils.canInitSafely(operative); + } else if (operativeType.equals(Object.class) || baseType.equals(Object.class)) { + // One is 'Object', so we can atleast see if the other has any operations registered + Class[] returnTypes = null; + if (baseType.equals(Object.class)) { + returnTypes = Arithmetics.getOperations(operator).stream() + .filter(info -> info.getRight().isAssignableFrom(operativeType)) + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + if (returnTypes.length == 0) { + Skript.error(operator.getName() + " cannot be performed with " + + Utils.a(getClassInfoCodeName(operativeType))); + return false; + } + } else { + returnTypes = Arithmetics.getOperations(operator, baseType).stream() + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + if (returnTypes.length == 0) { + Skript.error("This expression can not be " + getOperatorString() + "."); + return false; + } + } + } else { + // We've deduced that the return types from both 'base' and 'operative' are guaranteed not to be 'Object.class' + // We can check to see if an operation exists, if not, parse error + operation = getOperation(baseType, operativeType); + if (operation == null) { Skript.error("This expression cannot be " + getOperatorString() + " by " + Utils.a(getClassInfoCodeName(operativeType)) + "."); return false; } - if (base instanceof SimpleExpression && !base.getReturnType().equals(Object.class)) { - operation = getOperation(base.getReturnType(), operative.getReturnType()); - if (operation == null) { - Skript.error("This expression cannot be " + getOperatorString() + " by " - + Utils.a(getClassInfoCodeName(operative.getReturnType())) + "."); - return false; - } - } } - node = getParser().getNode(); return LiteralUtils.canInitSafely(operative); } @@ -107,31 +134,34 @@ protected void execute(Event event) { Object operativeObject = operative.getSingle(event); if (operativeObject == null) return; - Class operativeClass = operativeObject.getClass(); + // Ensure 'operativeType' is not 'Object.class' by using '#getReturnType' or the class of the object + Class operativeType = operative.getReturnType().equals(Object.class) ? operativeObject.getClass() : operative.getReturnType(); Operation operation = this.operation; Function changerFunction = null; - if (base.isSingle() || (base instanceof SimpleExpression && !base.getReturnType().equals(Object.class))) { + Class baseType = base.getReturnType(); + // Convert array classes to singular, allowing for easier checks + if (baseType.isArray()) + baseType = baseType.getComponentType(); + // If 'base' is single or the '#getReturnType' returns a non 'Object.class' we can use the same operation + if (base.isSingle() || !baseType.equals(Object.class)) { if (operation == null) { - // If the variable provided is single, then we can do some checks - // to ensure an operation is available; if not, we can produce a proper error. Object baseObject = base.getSingle(event); if (baseObject == null) return; - Class baseClass = baseObject.getClass(); - operation = getOperation(baseClass, operativeClass); + // If we reached here through 'base.isSingle()', need to ensure the class is not 'Object.class' + baseType = baseType.equals(Object.class) ? baseObject.getClass() : baseType; + operation = getOperation(baseType, operativeType); if (operation == null) { - error(Utils.A(getClassInfoCodeName(baseClass)) + " cannot be " + getOperatorString() - + " with " + Utils.a(getClassInfoCodeName(operativeClass)) + "."); + error(Utils.A(getClassInfoCodeName(baseType)) + " cannot be " + getOperatorString() + + " with " + Utils.a(getClassInfoCodeName(operativeType)) + "."); return; } } Operation finalOperation = operation; changerFunction = object -> finalOperation.calculate(object, operativeObject); } else { - // In the case of a list variable, we should probably ignore throwing multiple errors for each object - // that is not applicable for an operation changerFunction = object -> { - Operation thisOperation = getOperation(object.getClass(), operativeClass); + Operation thisOperation = getOperation(object.getClass(), operativeType); if (thisOperation != null) return thisOperation.calculate(object, operativeObject); return object; @@ -164,27 +194,6 @@ private String getOperatorString() { }; } - private boolean isOperable(@Nullable Class objectClass) { - Class[] classes = base.acceptChange(ChangeMode.SET); - if (classes == null) { - return false; - } else if (classes.length == 0) { - throw new IllegalStateException("A ChangeMode of 'SET' should never return an empty array."); - } - if (objectClass == null) - return true; - if (base.isSingle() || base instanceof SimpleExpression) { - for (Class clazz : classes) { - if (clazz.isAssignableFrom(objectClass)) { - return true; - } - } - } else if (base instanceof Variable) { - return true; - } - return false; - } - @Override public String toString(@Nullable Event event, boolean debug) { SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); From b4b7dea6adabb539dc1c880dca8613256c4538cc Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Mon, 7 Apr 2025 02:57:25 -0400 Subject: [PATCH 08/12] Minor Update --- .../ch/njol/skript/effects/EffOperations.java | 105 ++++++++++++++---- 1 file changed, 86 insertions(+), 19 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 28ea6bdcf87..fc0170eb4af 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -24,6 +24,7 @@ import org.skriptlang.skript.lang.arithmetic.Operator; import org.skriptlang.skript.log.runtime.SyntaxRuntimeErrorProducer; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; @Name("Operations") @@ -81,7 +82,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye base = exprs[0]; Class[] baseAccepted = base.acceptChange(ChangeMode.SET); if (baseAccepted == null) { - Skript.error("This expression cannot be operated on."); + printError(null, null); return false; } operative = LiteralUtils.defendExpression(exprs[1]); @@ -92,7 +93,13 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye if (baseType.isArray()) baseType = baseType.getComponentType(); if (operativeType.equals(Object.class) && baseType.equals(Object.class)) { - // Both are object, so we should do the checks within '#execute' + // Both are object, so we should check to see if '#getAllReturnTypes' contains a class + // that is accepted within 'baseAccepted' + Class[] allReturnTypes = Arithmetics.getAllReturnTypes(operator).toArray(Class[]::new); + if (!isOfType(baseAccepted, allReturnTypes)) { + printError(null, null); + return false; + } return LiteralUtils.canInitSafely(operative); } else if (operativeType.equals(Object.class) || baseType.equals(Object.class)) { // One is 'Object', so we can atleast see if the other has any operations registered @@ -103,8 +110,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye .map(OperationInfo::getReturnType) .toArray(Class[]::new); if (returnTypes.length == 0) { - Skript.error(operator.getName() + " cannot be performed with " - + Utils.a(getClassInfoCodeName(operativeType))); + printError(null, operativeType); return false; } } else { @@ -112,19 +118,27 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye .map(OperationInfo::getReturnType) .toArray(Class[]::new); if (returnTypes.length == 0) { - Skript.error("This expression can not be " + getOperatorString() + "."); + printError(null, null); return false; } } + if (!isOfType(baseAccepted, returnTypes)) { + Skript.error("Blah"); + return false; + } } else { // We've deduced that the return types from both 'base' and 'operative' are guaranteed not to be 'Object.class' // We can check to see if an operation exists, if not, parse error - operation = getOperation(baseType, operativeType); - if (operation == null) { - Skript.error("This expression cannot be " + getOperatorString() + " by " - + Utils.a(getClassInfoCodeName(operativeType)) + "."); + OperationInfo operationInfo = getOperationInfo(baseType, operativeType); + if (operationInfo == null) { + printError(null, operativeType); + return false; + } + if (!isOfType(baseAccepted, operationInfo.getReturnType())) { + printError(null, operativeType); return false; } + operation = operationInfo.getOperation(); } return LiteralUtils.canInitSafely(operative); } @@ -139,6 +153,8 @@ protected void execute(Event event) { Operation operation = this.operation; Function changerFunction = null; Class baseType = base.getReturnType(); + Class[] baseAccepted = base.acceptChange(ChangeMode.SET); + assert baseAccepted != null; // Convert array classes to singular, allowing for easier checks if (baseType.isArray()) baseType = baseType.getComponentType(); @@ -150,20 +166,27 @@ protected void execute(Event event) { return; // If we reached here through 'base.isSingle()', need to ensure the class is not 'Object.class' baseType = baseType.equals(Object.class) ? baseObject.getClass() : baseType; - operation = getOperation(baseType, operativeType); - if (operation == null) { - error(Utils.A(getClassInfoCodeName(baseType)) + " cannot be " + getOperatorString() - + " with " + Utils.a(getClassInfoCodeName(operativeType)) + "."); + OperationInfo operationInfo = getOperationInfo(baseType, operativeType); + if (operationInfo == null || !isOfType(baseAccepted, operationInfo.getReturnType())) { + printError(baseType, operativeType); return; } + operation = operationInfo.getOperation(); } Operation finalOperation = operation; changerFunction = object -> finalOperation.calculate(object, operativeObject); } else { + AtomicBoolean errorPrinted = new AtomicBoolean(false); changerFunction = object -> { - Operation thisOperation = getOperation(object.getClass(), operativeType); - if (thisOperation != null) - return thisOperation.calculate(object, operativeObject); + OperationInfo operationInfo = getOperationInfo(object.getClass(), operativeType); + if (operationInfo != null) { + if (isOfType(baseAccepted, operationInfo.getReturnType())) { + return operationInfo.getOperation().calculate(object, operativeObject); + } else if (!errorPrinted.get()) { + errorPrinted.set(true); + printError(null, operativeType); + } + } return object; }; } @@ -171,9 +194,29 @@ protected void execute(Event event) { base.changeInPlace(event, (Function) changerFunction); } - private @Nullable Operation getOperation(Class leftClass, Class rightClass) { + private @Nullable OperationInfo getOperationInfo(Class leftClass, Class rightClass) { //noinspection unchecked - return (Operation) Arithmetics.getOperation(operator, leftClass, rightClass, leftClass); + return (OperationInfo) Arithmetics.lookupOperationInfo(operator, leftClass, rightClass); + } + + private boolean isOfType(Class[] acceptedTypes, Class[] checkTypes) { + for (Class check : checkTypes) { + if (isOfType(acceptedTypes, check)) { + return true; + } + } + return false; + } + + private boolean isOfType(Class[] acceptedTypes, Class clazz) { + for (Class type : acceptedTypes) { + if (type.isArray()) + type = type.getComponentType(); + if (type.equals(Object.class) || type.isAssignableFrom(clazz)) { + return true; + } + } + return false; } @Override @@ -181,7 +224,7 @@ public Node getNode() { return node; } - private String getClassInfoCodeName(Class clazz) { + private String getCodeName(Class clazz) { return Classes.getSuperClassInfo(clazz).getCodeName(); } @@ -194,6 +237,30 @@ private String getOperatorString() { }; } + private void printError(boolean parseError, @Nullable Class baseClass, @Nullable Class operativeClass) { + String message = ""; + if (baseClass == null) { + if (operativeClass == null) { + message = "This expression cannot be operated on."; + } else { + message = "This expression cannot be " + getOperatorString() + " by " + + Utils.a(getCodeName(operativeClass)); + } + } else { + if (operativeClass == null) { + message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString(); + } else { + message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString() + " by " + + Utils.a(getCodeName(operativeClass)); + } + } + if (parseError) { + Skript.error(message); + } else { + error(message); + } + } + @Override public String toString(@Nullable Event event, boolean debug) { SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); From 2815399890a5f0e920785b522e4e4e4f781f4929 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Mon, 7 Apr 2025 03:14:49 -0400 Subject: [PATCH 09/12] Update EffOperations.java --- .../ch/njol/skript/effects/EffOperations.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index fc0170eb4af..6d5962ab166 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -82,8 +82,10 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye base = exprs[0]; Class[] baseAccepted = base.acceptChange(ChangeMode.SET); if (baseAccepted == null) { - printError(null, null); + printError(true,null, null); return false; + } else if (baseAccepted.length == 0) { + throw new IllegalStateException("An expression should never return an empty array for a ChangeMode of 'SET'."); } operative = LiteralUtils.defendExpression(exprs[1]); node = getParser().getNode(); @@ -92,17 +94,17 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye // Ensure 'baseType' is referencing a non-array class if (baseType.isArray()) baseType = baseType.getComponentType(); - if (operativeType.equals(Object.class) && baseType.equals(Object.class)) { + if (baseType.equals(Object.class) && operativeType.equals(Object.class)) { // Both are object, so we should check to see if '#getAllReturnTypes' contains a class // that is accepted within 'baseAccepted' Class[] allReturnTypes = Arithmetics.getAllReturnTypes(operator).toArray(Class[]::new); if (!isOfType(baseAccepted, allReturnTypes)) { - printError(null, null); + printError(true,null, null); return false; } return LiteralUtils.canInitSafely(operative); - } else if (operativeType.equals(Object.class) || baseType.equals(Object.class)) { - // One is 'Object', so we can atleast see if the other has any operations registered + } else if (baseType.equals(Object.class) || operativeType.equals(Object.class)) { + // One is 'Object', so we can at least see if the other has any operations registered Class[] returnTypes = null; if (baseType.equals(Object.class)) { returnTypes = Arithmetics.getOperations(operator).stream() @@ -110,7 +112,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye .map(OperationInfo::getReturnType) .toArray(Class[]::new); if (returnTypes.length == 0) { - printError(null, operativeType); + printError(true,null, operativeType); return false; } } else { @@ -118,12 +120,12 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye .map(OperationInfo::getReturnType) .toArray(Class[]::new); if (returnTypes.length == 0) { - printError(null, null); + printError(true,null, null); return false; } } if (!isOfType(baseAccepted, returnTypes)) { - Skript.error("Blah"); + printError(true, null, null); return false; } } else { @@ -131,11 +133,11 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye // We can check to see if an operation exists, if not, parse error OperationInfo operationInfo = getOperationInfo(baseType, operativeType); if (operationInfo == null) { - printError(null, operativeType); + printError(true,null, operativeType); return false; } if (!isOfType(baseAccepted, operationInfo.getReturnType())) { - printError(null, operativeType); + printError(true,null, operativeType); return false; } operation = operationInfo.getOperation(); @@ -168,7 +170,7 @@ protected void execute(Event event) { baseType = baseType.equals(Object.class) ? baseObject.getClass() : baseType; OperationInfo operationInfo = getOperationInfo(baseType, operativeType); if (operationInfo == null || !isOfType(baseAccepted, operationInfo.getReturnType())) { - printError(baseType, operativeType); + printError(false, baseType, operativeType); return; } operation = operationInfo.getOperation(); @@ -184,7 +186,7 @@ protected void execute(Event event) { return operationInfo.getOperation().calculate(object, operativeObject); } else if (!errorPrinted.get()) { errorPrinted.set(true); - printError(null, operativeType); + printError(false, null, operativeType); } } return object; @@ -244,14 +246,14 @@ private void printError(boolean parseError, @Nullable Class baseClass, @Nulla message = "This expression cannot be operated on."; } else { message = "This expression cannot be " + getOperatorString() + " by " - + Utils.a(getCodeName(operativeClass)); + + Utils.a(getCodeName(operativeClass)) + "."; } } else { if (operativeClass == null) { - message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString(); + message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString() + "."; } else { message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString() + " by " - + Utils.a(getCodeName(operativeClass)); + + Utils.a(getCodeName(operativeClass)) + "."; } } if (parseError) { From aa724b7f24c614491ef552832d837fa3d7192835 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Thu, 10 Apr 2025 00:33:56 -0400 Subject: [PATCH 10/12] Sovdes Version --- .../ch/njol/skript/effects/EffOperations.java | 247 ++++++++---------- .../arithmetic/ExprArithmetic.java | 16 +- .../skript/lang/arithmetic/Arithmetics.java | 19 +- 3 files changed, 133 insertions(+), 149 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 6d5962ab166..b050eb0735d 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -2,11 +2,13 @@ import ch.njol.skript.Skript; import ch.njol.skript.classes.Changer.ChangeMode; +import ch.njol.skript.classes.Changer.ChangerUtils; import ch.njol.skript.config.Node; import ch.njol.skript.doc.Description; import ch.njol.skript.doc.Example; import ch.njol.skript.doc.Name; import ch.njol.skript.doc.Since; +import ch.njol.skript.expressions.arithmetic.ExprArithmetic; import ch.njol.skript.lang.Effect; import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser.ParseResult; @@ -14,7 +16,6 @@ import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.LiteralUtils; import ch.njol.skript.util.Patterns; -import ch.njol.skript.util.Utils; import ch.njol.util.Kleenean; import org.bukkit.event.Event; import org.jetbrains.annotations.Nullable; @@ -24,7 +25,10 @@ import org.skriptlang.skript.lang.arithmetic.Operator; import org.skriptlang.skript.log.runtime.SyntaxRuntimeErrorProducer; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; import java.util.function.Function; @Name("Operations") @@ -71,166 +75,143 @@ public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer } private Operator operator; - private Expression base; - private Expression operative; + private Expression left; + private Class[] leftAccepts; + private Expression right; private Node node; private Operation operation = null; + private OperationInfo operationInfo; @Override public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { operator = PATTERNS.getInfo(matchedPattern); - base = exprs[0]; - Class[] baseAccepted = base.acceptChange(ChangeMode.SET); - if (baseAccepted == null) { - printError(true,null, null); + node = getParser().getNode(); + left = exprs[0]; + right = LiteralUtils.defendExpression(exprs[1]); + + leftAccepts = left.acceptChange(ChangeMode.SET); + // Ensure 'left' is changeable + if (leftAccepts == null) { + Skript.error("'" + left + "' cannot be set to anything and therefore cannot be " + getOperatorName() + "."); return false; - } else if (baseAccepted.length == 0) { - throw new IllegalStateException("An expression should never return an empty array for a ChangeMode of 'SET'."); + } else if (leftAccepts.length == 0) { + throw new IllegalStateException("An expression should never return an empty array for a ChangeMode of 'SET'"); } - operative = LiteralUtils.defendExpression(exprs[1]); - node = getParser().getNode(); - Class operativeType = operative.getReturnType(); - Class baseType = base.getReturnType(); - // Ensure 'baseType' is referencing a non-array class - if (baseType.isArray()) - baseType = baseType.getComponentType(); - if (baseType.equals(Object.class) && operativeType.equals(Object.class)) { - // Both are object, so we should check to see if '#getAllReturnTypes' contains a class - // that is accepted within 'baseAccepted' + // Ensure the accepted classes of 'left' are non-array classes + for (int i = 0; i < leftAccepts.length; i++) { + if (leftAccepts[i].isArray()) { + leftAccepts[i] = leftAccepts[i].getComponentType(); + } + } + + Class leftType = left.getReturnType(); + Class rightType = right.getReturnType(); + + if (leftType.isArray()) + leftType = leftType.getComponentType(); + + if (leftType.equals(Object.class) && rightType.equals(Object.class)) { + // 'left' and 'right' return 'Object.class' thus making operation checks non-applicable + // However, we can check to make sure any of the registered operations return types are applicable + // for 'left's acceptedClasses Class[] allReturnTypes = Arithmetics.getAllReturnTypes(operator).toArray(Class[]::new); - if (!isOfType(baseAccepted, allReturnTypes)) { - printError(true,null, null); + if (!ChangerUtils.acceptsChangeTypes(leftAccepts, allReturnTypes)) { + Skript.error(left + " cannot be " + getOperatorName() + "."); return false; } - return LiteralUtils.canInitSafely(operative); - } else if (baseType.equals(Object.class) || operativeType.equals(Object.class)) { - // One is 'Object', so we can at least see if the other has any operations registered - Class[] returnTypes = null; - if (baseType.equals(Object.class)) { + return LiteralUtils.canInitSafely(right); + } else if (leftType.equals(Object.class) || rightType.equals(Object.class)) { + Class[] returnTypes; + if (leftType.equals(Object.class)) { returnTypes = Arithmetics.getOperations(operator).stream() - .filter(info -> info.getRight().isAssignableFrom(operativeType)) + .filter(info -> info.getRight().isAssignableFrom(rightType)) .map(OperationInfo::getReturnType) .toArray(Class[]::new); - if (returnTypes.length == 0) { - printError(true,null, operativeType); - return false; - } } else { - returnTypes = Arithmetics.getOperations(operator, baseType).stream() + returnTypes = Arithmetics.getOperations(operator, leftType).stream() .map(OperationInfo::getReturnType) .toArray(Class[]::new); - if (returnTypes.length == 0) { - printError(true,null, null); - return false; - } } - if (!isOfType(baseAccepted, returnTypes)) { - printError(true, null, null); + + if (returnTypes.length == 0) { + noOperationError(left, leftType, rightType); return false; } - } else { - // We've deduced that the return types from both 'base' and 'operative' are guaranteed not to be 'Object.class' - // We can check to see if an operation exists, if not, parse error - OperationInfo operationInfo = getOperationInfo(baseType, operativeType); - if (operationInfo == null) { - printError(true,null, operativeType); + if (!ChangerUtils.acceptsChangeTypes(leftAccepts, returnTypes)) { + genericParseError(left, rightType); return false; } - if (!isOfType(baseAccepted, operationInfo.getReturnType())) { - printError(true,null, operativeType); + } else { + operationInfo = Arithmetics.lookupOperationInfo(operator, leftType, rightType, leftAccepts); + if (operationInfo == null || !ChangerUtils.acceptsChangeTypes(leftAccepts, operationInfo.getReturnType())) { + genericParseError(left, rightType); return false; } - operation = operationInfo.getOperation(); } - return LiteralUtils.canInitSafely(operative); + return LiteralUtils.canInitSafely(right); } @Override protected void execute(Event event) { - Object operativeObject = operative.getSingle(event); - if (operativeObject == null) + Object rightObject = right.getSingle(event); + if (rightObject == null) return; - // Ensure 'operativeType' is not 'Object.class' by using '#getReturnType' or the class of the object - Class operativeType = operative.getReturnType().equals(Object.class) ? operativeObject.getClass() : operative.getReturnType(); - Operation operation = this.operation; - Function changerFunction = null; - Class baseType = base.getReturnType(); - Class[] baseAccepted = base.acceptChange(ChangeMode.SET); - assert baseAccepted != null; - // Convert array classes to singular, allowing for easier checks - if (baseType.isArray()) - baseType = baseType.getComponentType(); - // If 'base' is single or the '#getReturnType' returns a non 'Object.class' we can use the same operation - if (base.isSingle() || !baseType.equals(Object.class)) { + + Class rightType = rightObject.getClass(); + + Map, Operation> cachedOperations = new HashMap<>(); + Set> invalidTypes = new HashSet<>(); + + Function changerFunction = (leftInput) -> { + Class leftType = leftInput.getClass(); + if (invalidTypes.contains(leftType)) { + printArithmeticError(leftType, rightType); + return leftInput; + } + Operation operation = cachedOperations.get(leftType); if (operation == null) { - Object baseObject = base.getSingle(event); - if (baseObject == null) - return; - // If we reached here through 'base.isSingle()', need to ensure the class is not 'Object.class' - baseType = baseType.equals(Object.class) ? baseObject.getClass() : baseType; - OperationInfo operationInfo = getOperationInfo(baseType, operativeType); - if (operationInfo == null || !isOfType(baseAccepted, operationInfo.getReturnType())) { - printError(false, baseType, operativeType); - return; + //noinspection unchecked + OperationInfo operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, leftType, rightType, leftAccepts); + if (operationInfo == null) { + printArithmeticError(leftType, rightType); + invalidTypes.add(leftType); + return leftInput; } operation = operationInfo.getOperation(); + cachedOperations.put(leftType, operation); } - Operation finalOperation = operation; - changerFunction = object -> finalOperation.calculate(object, operativeObject); - } else { - AtomicBoolean errorPrinted = new AtomicBoolean(false); - changerFunction = object -> { - OperationInfo operationInfo = getOperationInfo(object.getClass(), operativeType); - if (operationInfo != null) { - if (isOfType(baseAccepted, operationInfo.getReturnType())) { - return operationInfo.getOperation().calculate(object, operativeObject); - } else if (!errorPrinted.get()) { - errorPrinted.set(true); - printError(false, null, operativeType); - } - } - return object; - }; - } + return operation.calculate(leftInput, rightObject); + }; //noinspection unchecked,rawtypes - base.changeInPlace(event, (Function) changerFunction); - } - - private @Nullable OperationInfo getOperationInfo(Class leftClass, Class rightClass) { - //noinspection unchecked - return (OperationInfo) Arithmetics.lookupOperationInfo(operator, leftClass, rightClass); + left.changeInPlace(event, (Function) changerFunction); } - private boolean isOfType(Class[] acceptedTypes, Class[] checkTypes) { - for (Class check : checkTypes) { - if (isOfType(acceptedTypes, check)) { - return true; - } - } - return false; + @Override + public Node getNode() { + return node; } - private boolean isOfType(Class[] acceptedTypes, Class clazz) { - for (Class type : acceptedTypes) { - if (type.isArray()) - type = type.getComponentType(); - if (type.equals(Object.class) || type.isAssignableFrom(clazz)) { - return true; - } - } - return false; + private void printArithmeticError(Class left, Class right) { + String error = ExprArithmetic.getArithmeticErrorMessage(operator, left, right); + if (error != null) + error(error); } - @Override - public Node getNode() { - return node; + private void genericParseError(Expression leftExpr, Class rightType) { + Skript.error("'" + leftExpr + "' cannot be " + getOperatorName() + " by " + + Classes.getSuperClassInfo(rightType).getName().withIndefiniteArticle()); } - private String getCodeName(Class clazz) { - return Classes.getSuperClassInfo(clazz).getCodeName(); + private void noOperationError(Expression leftExpr, Class leftType, Class rightType) { + String error = ExprArithmetic.getArithmeticErrorMessage(operator, leftType, rightType); + if (error != null) { + Skript.error(error); + } else { + genericParseError(leftExpr, rightType); + } } - private String getOperatorString() { + private String getOperatorName() { return switch (operator) { case MULTIPLICATION -> "multiplied"; case DIVISION -> "divided"; @@ -239,39 +220,15 @@ private String getOperatorString() { }; } - private void printError(boolean parseError, @Nullable Class baseClass, @Nullable Class operativeClass) { - String message = ""; - if (baseClass == null) { - if (operativeClass == null) { - message = "This expression cannot be operated on."; - } else { - message = "This expression cannot be " + getOperatorString() + " by " - + Utils.a(getCodeName(operativeClass)) + "."; - } - } else { - if (operativeClass == null) { - message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString() + "."; - } else { - message = Utils.A(getCodeName(baseClass)) + " cannot be " + getOperatorString() + " by " - + Utils.a(getCodeName(operativeClass)) + "."; - } - } - if (parseError) { - Skript.error(message); - } else { - error(message); - } - } - @Override public String toString(@Nullable Event event, boolean debug) { SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug); switch (operator) { - case MULTIPLICATION -> builder.append("multiply", base, "by"); - case DIVISION -> builder.append("divide", base, "by"); - case EXPONENTIATION -> builder.append("raise", base, "to the power of"); + case MULTIPLICATION -> builder.append("multiply", left, "by"); + case DIVISION -> builder.append("divide", left, "by"); + case EXPONENTIATION -> builder.append("raise", left, "to the power of"); } - builder.append(operative); + builder.append(right); return builder.toString(); } diff --git a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java index 4916139842b..3fd4205d1d8 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -304,12 +304,22 @@ protected T[] get(Event event) { } private boolean error(Class firstClass, Class secondClass) { - ClassInfo first = Classes.getSuperClassInfo(firstClass), second = Classes.getSuperClassInfo(secondClass); - if (first.getC() != Object.class && second.getC() != Object.class) // errors with "object" are not very useful and often misleading - Skript.error(operator.getName() + " can't be performed on " + first.getName().withIndefiniteArticle() + " and " + second.getName().withIndefiniteArticle()); + String error = getArithmeticErrorMessage(operator, firstClass, secondClass); + if (error != null) + Skript.error(error); return false; } + public static @Nullable String getArithmeticErrorMessage(Operator operator, Class left, Class right) { + ClassInfo first = Classes.getSuperClassInfo(left); + ClassInfo second = Classes.getSuperClassInfo(right); + if (!first.getC().equals(Object.class) && !second.getC().equals(Object.class)) { + return operator.getName() + " can't be performed on " + first.getName().withIndefiniteArticle() + " and " + + second.getName().withIndefiniteArticle(); + } + return null; + } + @Override public Class getReturnType() { return returnType; diff --git a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java index 860b682b064..5c486c188b8 100644 --- a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java +++ b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java @@ -87,7 +87,7 @@ public static boolean operationExists(Operator operator, Class leftClass, Cla @SuppressWarnings("unchecked") public static OperationInfo getOperationInfo(Operator operator, Class leftClass, Class rightClass, Class returnType) { OperationInfo info = getOperationInfo(operator, leftClass, rightClass); - if (info != null && (returnType.isAssignableFrom(info.getReturnType()) || info.getReturnType().isAssignableFrom(returnType))) + if (info != null && returnType.isAssignableFrom(info.getReturnType())) return (OperationInfo) info; return null; } @@ -129,6 +129,23 @@ public static OperationInfo lookupOperationInfo(Operator oper return info != null ? info.getConverted(leftClass, rightClass, returnType) : null; } + public static @Nullable OperationInfo lookupOperationInfo( + Operator operator, + Class leftClass, + Class rightClass, + Class ... possibleReturnTypes + ) { + OperationInfo info = lookupOperationInfo(operator, leftClass, rightClass); + if (info == null) + return null; + for (Class returnType : possibleReturnTypes) { + OperationInfo convertedInfo = info.getConverted(leftClass, rightClass, returnType); + if (convertedInfo != null) + return convertedInfo; + } + return null; + } + @Nullable @SuppressWarnings("unchecked") public static OperationInfo lookupOperationInfo(Operator operator, Class leftClass, Class rightClass) { From fddda4b86b02f00287a6c3cac7ebdddb890e0655 Mon Sep 17 00:00:00 2001 From: SirSmurfy2 <82696841+TheAbsolutionism@users.noreply.github.com> Date: Thu, 10 Apr 2025 02:23:12 -0400 Subject: [PATCH 11/12] Update Test --- src/main/java/ch/njol/skript/effects/EffOperations.java | 2 +- src/test/skript/tests/syntaxes/effects/EffOperations.sk | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index b050eb0735d..8b9adc359d2 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -199,7 +199,7 @@ private void printArithmeticError(Class left, Class right) { private void genericParseError(Expression leftExpr, Class rightType) { Skript.error("'" + leftExpr + "' cannot be " + getOperatorName() + " by " - + Classes.getSuperClassInfo(rightType).getName().withIndefiniteArticle()); + + Classes.getSuperClassInfo(rightType).getName().withIndefiniteArticle() + "."); } private void noOperationError(Expression leftExpr, Class leftType, Class rightType) { diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index 02faea23e98..fdd5a0f250c 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -135,8 +135,8 @@ test "operations effect literals error": test "operations effect expression errors": parse: multiply (the hunger level of (random element out of all players)) by 1 hour - assert last parse logs contains "This expression cannot be multiplied by a timespan." with "Hunger level should not be able to be multiplied by timespan" + assert last parse logs contains "'the food level of a random element of all entities of type player' cannot be multiplied by a time span." with "Hunger level should not be able to be multiplied by timespan" parse: divide all worlds by 2 - assert last parse logs contains "This expression cannot be operated on." with "ExprWorlds should not have a 'SET' ChangeMode" + assert last parse logs contains "'worlds' cannot be set to anything and therefore cannot be divided." with "ExprWorlds should not have a 'SET' ChangeMode" From f502e5ccb2386e0697c3e0d58862da8b36d3197f Mon Sep 17 00:00:00 2001 From: SirSmurfy2 Date: Sat, 3 May 2025 15:27:43 -0400 Subject: [PATCH 12/12] Additional Changes --- .../ch/njol/skript/effects/EffOperations.java | 28 +++++++++++++------ .../arithmetic/ExprArithmetic.java | 7 +++++ .../skript/lang/arithmetic/Arithmetics.java | 14 +++++++++- .../tests/syntaxes/effects/EffOperations.sk | 14 ++++++++++ 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffOperations.java b/src/main/java/ch/njol/skript/effects/EffOperations.java index 8b9adc359d2..db5c1e9cc28 100644 --- a/src/main/java/ch/njol/skript/effects/EffOperations.java +++ b/src/main/java/ch/njol/skript/effects/EffOperations.java @@ -92,7 +92,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye leftAccepts = left.acceptChange(ChangeMode.SET); // Ensure 'left' is changeable if (leftAccepts == null) { - Skript.error("'" + left + "' cannot be set to anything and therefore cannot be " + getOperatorName() + "."); + Skript.error("'" + left + "' cannot be set to anything and therefore cannot be " + getOperatorVerb() + "."); return false; } else if (leftAccepts.length == 0) { throw new IllegalStateException("An expression should never return an empty array for a ChangeMode of 'SET'"); @@ -107,41 +107,47 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye Class leftType = left.getReturnType(); Class rightType = right.getReturnType(); - if (leftType.isArray()) - leftType = leftType.getComponentType(); - if (leftType.equals(Object.class) && rightType.equals(Object.class)) { // 'left' and 'right' return 'Object.class' thus making operation checks non-applicable // However, we can check to make sure any of the registered operations return types are applicable // for 'left's acceptedClasses Class[] allReturnTypes = Arithmetics.getAllReturnTypes(operator).toArray(Class[]::new); if (!ChangerUtils.acceptsChangeTypes(leftAccepts, allReturnTypes)) { - Skript.error(left + " cannot be " + getOperatorName() + "."); + Skript.error(left.toString(null, Skript.debug()) + " cannot be " + getOperatorVerb() + "."); return false; } return LiteralUtils.canInitSafely(right); } else if (leftType.equals(Object.class) || rightType.equals(Object.class)) { + // Only one returns 'Object.class' Class[] returnTypes; if (leftType.equals(Object.class)) { + // 'left' returns 'Object.class', so we get all operations where 'right' is assignable to the right side + // of the operations and store the return types returnTypes = Arithmetics.getOperations(operator).stream() .filter(info -> info.getRight().isAssignableFrom(rightType)) .map(OperationInfo::getReturnType) .toArray(Class[]::new); } else { + // 'right' returns 'Object.class', so we get all operations where 'left' is assignable to the left side + // of the operations and store the return types returnTypes = Arithmetics.getOperations(operator, leftType).stream() .map(OperationInfo::getReturnType) .toArray(Class[]::new); } + // No operations found, meaning nothing can be done if (returnTypes.length == 0) { noOperationError(left, leftType, rightType); return false; } + // Check if 'left' can be changed into at least one of the possible return types if (!ChangerUtils.acceptsChangeTypes(leftAccepts, returnTypes)) { genericParseError(left, rightType); return false; } } else { + // Both 'left' and 'right' return an exact class type, so we check if the operation exists + // Then if 'left' accepts the return type of the operation operationInfo = Arithmetics.lookupOperationInfo(operator, leftType, rightType, leftAccepts); if (operationInfo == null || !ChangerUtils.acceptsChangeTypes(leftAccepts, operationInfo.getReturnType())) { genericParseError(left, rightType); @@ -154,8 +160,14 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye @Override protected void execute(Event event) { Object rightObject = right.getSingle(event); - if (rightObject == null) + if (rightObject == null) { + error("Cannot operate with a null object."); + return; + } + if (left.isSingle() && left.getSingle(event) == null) { + error("Cannot operate on a null object."); return; + } Class rightType = rightObject.getClass(); @@ -198,7 +210,7 @@ private void printArithmeticError(Class left, Class right) { } private void genericParseError(Expression leftExpr, Class rightType) { - Skript.error("'" + leftExpr + "' cannot be " + getOperatorName() + " by " + Skript.error("'" + leftExpr + "' cannot be " + getOperatorVerb() + " by " + Classes.getSuperClassInfo(rightType).getName().withIndefiniteArticle() + "."); } @@ -211,7 +223,7 @@ private void noOperationError(Expression leftExpr, Class leftType, Class "multiplied"; case DIVISION -> "divided"; diff --git a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java index 3fd4205d1d8..9bab16507fa 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -310,6 +310,13 @@ private boolean error(Class firstClass, Class secondClass) { return false; } + /** + * Get an error message in format of "'operator' cant be performed on 'left' and 'right'". + * @param operator The {@link Operator} that was attempted operate + * @param left The left {@link Class} attempted to be operated on + * @param right The right {@link Class} attempted to be operated with + * @return The {@link String} error message + */ public static @Nullable String getArithmeticErrorMessage(Operator operator, Class left, Class right) { ClassInfo first = Classes.getSuperClassInfo(left); ClassInfo second = Classes.getSuperClassInfo(right); diff --git a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java index 5c486c188b8..eb687a5a749 100644 --- a/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java +++ b/src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java @@ -129,15 +129,27 @@ public static OperationInfo lookupOperationInfo(Operator oper return info != null ? info.getConverted(leftClass, rightClass, returnType) : null; } + /** + * Lookup an {@link OperationInfo} that uses the provided args {@code operator}, {@code leftClass} and {@code rightClass}, + * and is one of the {@code possibleReturnTypes} + * @param operator The {@link Operator} to be used + * @param leftClass The {@link Class} to be operated on + * @param rightClass The {@link Class} to be operated with + * @param possibleReturnTypes The accepted return type {@link Class}es + */ public static @Nullable OperationInfo lookupOperationInfo( Operator operator, Class leftClass, Class rightClass, - Class ... possibleReturnTypes + Class... possibleReturnTypes ) { OperationInfo info = lookupOperationInfo(operator, leftClass, rightClass); if (info == null) return null; + for (Class returnType : possibleReturnTypes) { + if (info.getReturnType().equals(returnType)) + return info; + } for (Class returnType : possibleReturnTypes) { OperationInfo convertedInfo = info.getConverted(leftClass, rightClass, returnType); if (convertedInfo != null) diff --git a/src/test/skript/tests/syntaxes/effects/EffOperations.sk b/src/test/skript/tests/syntaxes/effects/EffOperations.sk index fdd5a0f250c..912cae48e74 100644 --- a/src/test/skript/tests/syntaxes/effects/EffOperations.sk +++ b/src/test/skript/tests/syntaxes/effects/EffOperations.sk @@ -140,3 +140,17 @@ test "operations effect expression errors": parse: divide all worlds by 2 assert last parse logs contains "'worlds' cannot be set to anything and therefore cannot be divided." with "ExprWorlds should not have a 'SET' ChangeMode" + +test "operations effect invalid operation": # TODO: test for runtime errors + set {_date} to now + multiply {_date} by 2 + + set {_span} to 1 second + divide {_date} by {_span} + +test "operations effect on expression": + set {_player} to "Notch" parsed as offline player + set time played of {_player} to 1 second + multiply the time played of {_player} by 60 + assert the time played of {_player} is 1 minute with "ExprTimePlayed was not changed with EffOperations" + set the time played of {_player} to 0 seconds