From 2d992e7696f403477be411e38596cb445edd5461 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Mon, 22 Jan 2024 16:02:21 -0500 Subject: [PATCH 1/6] Implement continued searching for gibberish unparsed literals --- .../ch/njol/skript/log/ParseLogHandler.java | 13 +++++++ .../skript/patterns/TypePatternElement.java | 39 ++++++++++++++++--- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/main/java/ch/njol/skript/log/ParseLogHandler.java b/src/main/java/ch/njol/skript/log/ParseLogHandler.java index dea3cc3cc8e..6b2bffd1dee 100644 --- a/src/main/java/ch/njol/skript/log/ParseLogHandler.java +++ b/src/main/java/ch/njol/skript/log/ParseLogHandler.java @@ -31,6 +31,19 @@ public class ParseLogHandler extends LogHandler { private LogEntry error = null; private final List log = new ArrayList<>(); + + public ParseLogHandler backup() { + ParseLogHandler copy = new ParseLogHandler(); + copy.error = this.error; + copy.log.addAll(this.log); + return copy; + } + + public void paste(ParseLogHandler parseLogHandler) { + this.error = parseLogHandler.error; + this.log.clear(); + this.log.addAll(parseLogHandler.log); + } @Override public LogResult log(LogEntry entry) { diff --git a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java index f3cbdb5f666..bf91c5f8098 100644 --- a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java +++ b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java @@ -24,6 +24,7 @@ import ch.njol.skript.lang.Literal; import ch.njol.skript.lang.SkriptParser; import ch.njol.skript.lang.SkriptParser.ExprInfo; +import ch.njol.skript.lang.UnparsedLiteral; import ch.njol.skript.lang.parser.ParserInstance; import ch.njol.skript.log.ErrorQuality; import ch.njol.skript.log.ParseLogHandler; @@ -138,6 +139,10 @@ public MatchResult match(String expr, MatchResult matchResult) { ExprInfo exprInfo = getExprInfo(); + MatchResult matchBackup = null; + ParseLogHandler loopLogHandlerBackup = null; + ParseLogHandler expressionLogHandlerBackup = null; + ParseLogHandler loopLogHandler = SkriptLogger.startParseLogHandler(); try { while (newExprOffset != -1) { @@ -168,11 +173,27 @@ public MatchResult match(String expr, MatchResult matchResult) { } } - expressionLogHandler.printLog(); - loopLogHandler.printLog(); - newMatchResult.expressions[expressionIndex] = expression; - return newMatchResult; + + boolean hasUnparsedLiteral = false; + for (int i = expressionIndex + 1; i < newMatchResult.expressions.length; i++) { + if (newMatchResult.expressions[i] instanceof UnparsedLiteral) { + hasUnparsedLiteral = Classes.parse(((UnparsedLiteral) newMatchResult.expressions[i]).getData(), Object.class, newMatchResult.parseContext) == null; + if (hasUnparsedLiteral) { + break; + } + } + } + + if (!hasUnparsedLiteral) { + expressionLogHandler.printLog(); + loopLogHandler.printLog(); + return newMatchResult; + } else if (matchBackup == null) { + matchBackup = newMatchResult; + loopLogHandlerBackup = loopLogHandler.backup(); + expressionLogHandlerBackup = expressionLogHandler.backup(); + } } } finally { expressionLogHandler.printError(); @@ -193,11 +214,17 @@ public MatchResult match(String expr, MatchResult matchResult) { } } } finally { - if (!loopLogHandler.isStopped()) + if (loopLogHandlerBackup != null) { + loopLogHandler.paste(loopLogHandlerBackup); + assert expressionLogHandlerBackup != null; + expressionLogHandlerBackup.printLog(); + } + if (!loopLogHandler.isStopped()) { loopLogHandler.printError(); + } } - return null; + return matchBackup; } @Override From 1ff3333bc40c06e6d8868d24675815bb119c7aba Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Tue, 23 Jan 2024 12:48:36 -0500 Subject: [PATCH 2/6] Improve arithmetic type obtaining Significantly improves the conversion of UnparsedLiterals and return type resolution --- .../arithmetic/ExprArithmetic.java | 99 ++++++++++++++----- .../syntaxes/expressions/ExprArithmetic.sk | 7 ++ 2 files changed, 83 insertions(+), 23 deletions(-) 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 baa160890c8..ef33e26abe2 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -28,6 +28,7 @@ import ch.njol.skript.lang.ExpressionType; import ch.njol.skript.lang.Literal; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.UnparsedLiteral; import ch.njol.skript.lang.util.SimpleExpression; import ch.njol.skript.lang.util.SimpleLiteral; import ch.njol.skript.registrations.Classes; @@ -117,43 +118,96 @@ public PatternInfo(Operator operator, boolean leftGrouped, boolean rightGrouped) private boolean leftGrouped, rightGrouped; @Override - @SuppressWarnings({"ConstantConditions", "unchecked"}) + @SuppressWarnings({"ConstantConditions", "rawtypes", "unchecked"}) public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { - first = LiteralUtils.defendExpression(exprs[0]); - second = LiteralUtils.defendExpression(exprs[1]); - - if (!LiteralUtils.canInitSafely(first, second)) - return false; - - Class firstClass = first.getReturnType(); - Class secondClass = second.getReturnType(); + first = (Expression) exprs[0]; + second = (Expression) exprs[1]; PatternInfo patternInfo = patterns.getInfo(matchedPattern); leftGrouped = patternInfo.leftGrouped; rightGrouped = patternInfo.rightGrouped; operator = patternInfo.operator; - if (firstClass != Object.class && secondClass == Object.class && Arithmetics.getOperations(operator, firstClass).isEmpty()) { - // If the first class is known but doesn't have any operations, then we fail - return error(firstClass, secondClass); - } else if (firstClass == Object.class && secondClass != Object.class && Arithmetics.getOperations(operator).stream() - .noneMatch(info -> info.getRight().isAssignableFrom(secondClass))) { - // If the second class is known but doesn't have any operations, then we fail - return error(firstClass, secondClass); + // try to convert unparsed literals with known info + if (first instanceof UnparsedLiteral) { // first is not parsed + if (second instanceof UnparsedLiteral) { // first and second are not parsed + for (OperationInfo operation : Arithmetics.getOperations(operator)) { + Expression convertedFirst = first.getConvertedExpression(operation.getLeft()); + if (convertedFirst == null) + continue; + Expression convertedSecond = second.getConvertedExpression(operation.getRight()); + if (convertedSecond == null) + continue; + first = (Expression) convertedFirst; + second = (Expression) convertedSecond; + returnType = (Class) operation.getReturnType(); + } + } else { // second is parsed, first is not + // attempt to convert to types that make valid operations with + Class secondClass = second.getReturnType(); + Class[] leftTypes = Arithmetics.getOperations(operator).stream() + .filter(info -> info.getRight().isAssignableFrom(secondClass)) + .map(OperationInfo::getLeft) + .toArray(Class[]::new); + if (leftTypes.length == 0) { // no known operations with second's type + if (secondClass != Object.class) // there won't be any operations + return error(first.getReturnType(), secondClass); + first = (Expression) first.getConvertedExpression(Object.class); + } else { + first = (Expression) first.getConvertedExpression(leftTypes); + } + } + } else if (second instanceof UnparsedLiteral) { // first is parsed, second is not + // attempt to convert to types that make valid operations with + Class firstClass = first.getReturnType(); + List> operations = Arithmetics.getOperations(operator, firstClass); + if (operations.isEmpty()) { // no known operations with first's type + if (firstClass != Object.class) // there won't be any operations + return error(firstClass, second.getReturnType()); + second = (Expression) second.getConvertedExpression(Object.class); + } else { + second = (Expression) second.getConvertedExpression(operations.stream() + .map(OperationInfo::getRight) + .toArray(Class[]::new) + ); + } } - OperationInfo operationInfo; + if (!LiteralUtils.canInitSafely(first, second)) + return false; + + Class firstClass = first.getReturnType(); + Class secondClass = second.getReturnType(); + if (firstClass == Object.class || secondClass == Object.class) { // If either of the types is unknown, then we resolve the operation at runtime - operationInfo = null; - } else { - operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, firstClass, secondClass); + Class[] returnTypes = null; + if (!(firstClass == Object.class && secondClass == Object.class)) { // both aren't object + if (firstClass == Object.class) { + returnTypes = Arithmetics.getOperations(operator).stream() + .filter(info -> info.getRight().isAssignableFrom(secondClass)) + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + } else { // secondClass is Object + returnTypes = Arithmetics.getOperations(operator, firstClass).stream() + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + } + } + if (returnTypes == null) { + returnType = (Class) Object.class; + } else if (returnTypes.length == 0) { // One of the classes is known but doesn't have any operations + return error(firstClass, secondClass); + } else { + returnType = (Class) Classes.getSuperClassInfo(returnTypes).getC(); + } + } else if (returnType == null) { + OperationInfo operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, firstClass, secondClass); if (operationInfo == null) // We error if we couldn't find an operation between the two types return error(firstClass, secondClass); + returnType = operationInfo.getReturnType(); } - returnType = operationInfo == null ? (Class) Object.class : operationInfo.getReturnType(); - if (Number.class.isAssignableFrom(returnType)) { if (operator == Operator.DIVISION || operator == Operator.EXPONENTIATION) { returnType = (Class) Double.class; @@ -164,7 +218,6 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye firstIsInt |= i.isAssignableFrom(first.getReturnType()); secondIsInt |= i.isAssignableFrom(second.getReturnType()); } - returnType = (Class) (firstIsInt && secondIsInt ? Long.class : Double.class); } } diff --git a/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk b/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk index f035aecd41e..16a526d5f7e 100644 --- a/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk +++ b/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk @@ -255,3 +255,10 @@ local function component_equals(a: number, b: number) :: boolean: then: return true return true if {_a} is {_b}, else false + +test "arithmetic return types": + # the issue below is that is now returned for the function + # skript interprets this as "y-coordinate of ({_location} - 4)" which is valid due to Object.class return types + # however, we can get more specific return types by returning the superclass of the return types of all Object-Number operations + set {_location} to location(0,10,0,"world") + assert (y-coordinate of {_location} - 4) is 6 with "y-coordinate of {_location} - 4 is not 6 (got '%y-coordinate of {_location} - 4%')" From 0b08548f519765441e9eb2a39ea219934acc1fee Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Thu, 1 Feb 2024 09:54:28 -0500 Subject: [PATCH 3/6] avoid printing misleading errors --- .../ch/njol/skript/expressions/arithmetic/ExprArithmetic.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ef33e26abe2..b5d287ba95d 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -250,7 +250,8 @@ protected T[] get(Event event) { private boolean error(Class firstClass, Class secondClass) { ClassInfo first = Classes.getSuperClassInfo(firstClass), second = Classes.getSuperClassInfo(secondClass); - Skript.error(operator.getName() + " can't be performed on " + first.getName().withIndefiniteArticle() + " and " + second.getName().withIndefiniteArticle()); + 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()); return false; } From cdf5e741e32c347db0b748c9f14dfde6551a50ff Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Thu, 1 Feb 2024 13:02:29 -0500 Subject: [PATCH 4/6] more testing and documentation --- .../ch/njol/skript/log/ParseLogHandler.java | 15 +++++++++++++-- .../skript/patterns/TypePatternElement.java | 18 +++++++++++++++--- .../tests/misc/invalid unparsed literals.sk | 6 ++++++ 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 src/test/skript/tests/misc/invalid unparsed literals.sk diff --git a/src/main/java/ch/njol/skript/log/ParseLogHandler.java b/src/main/java/ch/njol/skript/log/ParseLogHandler.java index 6b2bffd1dee..716464da53f 100644 --- a/src/main/java/ch/njol/skript/log/ParseLogHandler.java +++ b/src/main/java/ch/njol/skript/log/ParseLogHandler.java @@ -18,8 +18,9 @@ */ package ch.njol.skript.log; -import ch.njol.skript.Skript; import org.eclipse.jdt.annotation.Nullable; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Contract; import java.util.ArrayList; import java.util.List; @@ -32,6 +33,12 @@ public class ParseLogHandler extends LogHandler { private final List log = new ArrayList<>(); + /** + * Internal method for creating a backup of this log. + * @return A new ParseLogHandler containing the contents of this ParseLogHandler. + */ + @ApiStatus.Internal + @Contract("-> new") public ParseLogHandler backup() { ParseLogHandler copy = new ParseLogHandler(); copy.error = this.error; @@ -39,7 +46,11 @@ public ParseLogHandler backup() { return copy; } - public void paste(ParseLogHandler parseLogHandler) { + /** + * Internal method for restoring a backup of this log. + */ + @ApiStatus.Internal + public void restore(ParseLogHandler parseLogHandler) { this.error = parseLogHandler.error; this.log.clear(); this.log.addAll(parseLogHandler.log); diff --git a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java index bf91c5f8098..1ecdfee11cc 100644 --- a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java +++ b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java @@ -175,6 +175,16 @@ public MatchResult match(String expr, MatchResult matchResult) { newMatchResult.expressions[expressionIndex] = expression; + /* + * the parser will return unparsed literals in cases where it cannot interpret an input and object is the desired return type. + * in those cases, it is up to the expression to interpret the input. + * however, this presents a problem for input that is not intended as being one of these object-accepting expressions. + * these object-accepting expressions will be matched instead but their parsing will fail as they cannot interpret the unparsed literals. + * even though it can't interpret them, this loop will have returned a match and thus parsing has ended (and the correct interpretation never attempted). + * to avoid this issue, while also permitting unparsed literals in cases where they are justified, + * the code below forces the loop to continue in hopes of finding a match without unparsed literals. + * if it is unsuccessful, a backup of the first successful match (with unparsed literals) is saved to be returned. + */ boolean hasUnparsedLiteral = false; for (int i = expressionIndex + 1; i < newMatchResult.expressions.length; i++) { if (newMatchResult.expressions[i] instanceof UnparsedLiteral) { @@ -189,7 +199,7 @@ public MatchResult match(String expr, MatchResult matchResult) { expressionLogHandler.printLog(); loopLogHandler.printLog(); return newMatchResult; - } else if (matchBackup == null) { + } else if (matchBackup == null) { // only backup the first occurrence of unparsed literals matchBackup = newMatchResult; loopLogHandlerBackup = loopLogHandler.backup(); expressionLogHandlerBackup = expressionLogHandler.backup(); @@ -214,8 +224,8 @@ public MatchResult match(String expr, MatchResult matchResult) { } } } finally { - if (loopLogHandlerBackup != null) { - loopLogHandler.paste(loopLogHandlerBackup); + if (loopLogHandlerBackup != null) { // print backup logs if applicable + loopLogHandler.restore(loopLogHandlerBackup); assert expressionLogHandlerBackup != null; expressionLogHandlerBackup.printLog(); } @@ -224,6 +234,8 @@ public MatchResult match(String expr, MatchResult matchResult) { } } + // if there were unparsed literals, we will return the backup now + // if there were not, this returns null return matchBackup; } diff --git a/src/test/skript/tests/misc/invalid unparsed literals.sk b/src/test/skript/tests/misc/invalid unparsed literals.sk new file mode 100644 index 00000000000..6d076a7e079 --- /dev/null +++ b/src/test/skript/tests/misc/invalid unparsed literals.sk @@ -0,0 +1,6 @@ +test "invalid unparsed literals": + + parse: + loop 9 times: + set {_i} to loop-value - 1 + assert last parse logs is not set with "skript should be able to understand 'loop-value - 1' but did not" From 6ae0b5406fb5185fd8bfe8f608949aaac4fb1cc2 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Thu, 1 Feb 2024 17:58:05 -0500 Subject: [PATCH 5/6] documentation galore --- .../arithmetic/ExprArithmetic.java | 96 ++++++++++++++++--- 1 file changed, 82 insertions(+), 14 deletions(-) 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 b5d287ba95d..531a6934419 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -128,21 +128,55 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye rightGrouped = patternInfo.rightGrouped; operator = patternInfo.operator; - // try to convert unparsed literals with known info - if (first instanceof UnparsedLiteral) { // first is not parsed - if (second instanceof UnparsedLiteral) { // first and second are not parsed + /* + * Step 1: UnparsedLiteral Resolving + * + * Since Arithmetic may be performed on a variety of types, it is possible that 'first' or 'second' + * will represent unparsed literals. That is, the parser could not determine what their literal contents represent. + * Thus, it is now up to this expression to determine what they mean. + * + * If there are no unparsed literals, nothing happens at this step. + * If there are unparsed literals, one of three possible execution flows will occur: + * + * Case 1. 'first' and 'second' are unparsed literals + * In this case, there is not a lot of information to work with. + * 'first' and 'second' are attempted to be converted to fit one of all operations using 'operator'. + * If they cannot be matched with the types of a known operation, init will fail. + * + * Case 2. 'first' is an unparsed literal, 'second' is not + * In this case, 'first' needs to be converted into the "left" type of + * any operation using 'operator' with the type of 'second' as the right type. + * If 'first' cannot be converted, init will fail. + * If no operations are found for converting 'first', init will fail, UNLESS the type of 'second' is object, + * where operations will be searched again later with the context of the type of first. + * TODO When 'first' can represent multiple literals, it might be worth checking which of those can work with 'operator' and 'second' + * + * Case 3. 'second' is an unparsed literal, 'first' is not + * In this case, 'second' needs to be converted into the "right" type of + * any operation using 'operator' with the type of 'first' as the "left" type. + * If 'second' cannot be converted, init will fail. + * If no operations are found for converting 'second', init will fail, UNLESS the type of 'first' is object, + * where operations will be searched again later with the context of the type of second. + * TODO When 'second' can represent multiple literals, it might be worth checking which of those can work with 'first' and 'operator' + */ + + if (first instanceof UnparsedLiteral) { + if (second instanceof UnparsedLiteral) { // first and second need converting for (OperationInfo operation : Arithmetics.getOperations(operator)) { + // match left type with 'first' Expression convertedFirst = first.getConvertedExpression(operation.getLeft()); if (convertedFirst == null) continue; + // match right type with 'second' Expression convertedSecond = second.getConvertedExpression(operation.getRight()); if (convertedSecond == null) continue; + // success, set the values first = (Expression) convertedFirst; second = (Expression) convertedSecond; returnType = (Class) operation.getReturnType(); } - } else { // second is parsed, first is not + } else { // first needs converting // attempt to convert to types that make valid operations with Class secondClass = second.getReturnType(); Class[] leftTypes = Arithmetics.getOperations(operator).stream() @@ -157,7 +191,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye first = (Expression) first.getConvertedExpression(leftTypes); } } - } else if (second instanceof UnparsedLiteral) { // first is parsed, second is not + } else if (second instanceof UnparsedLiteral) { // second needs converting // attempt to convert to types that make valid operations with Class firstClass = first.getReturnType(); List> operations = Arithmetics.getOperations(operator, firstClass); @@ -173,14 +207,34 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye } } - if (!LiteralUtils.canInitSafely(first, second)) + if (!LiteralUtils.canInitSafely(first, second)) // checks if there are still unparsed literals present return false; + /* + * Step 2: Return Type Calculation + * + * After the first step, everything that can be known about 'first' and 'second' during parsing is known. + * As a result, it is time to determine the return type of the operation. + * + * If the types of 'first' or 'second' are object, it is possible that multiple operations with different return types + * will be found. If that is the case, the supertype of these operations will be the return type (can be object). + * If the types of both are object (e.g. variables), the return type will be object (have to wait until runtime and hope it works). + * Of course, if no operations are found, init will fail. + * + * After these checks, it is same to assume returnType has a value, as init should have failed by now if not. + * One final check is performed specifically for numerical types. + * Any numerical operation involving division or exponents have a return type of Double. + * Other operations will also return Double, UNLESS 'first' and 'second' are of integer types, in which case the return type will be Long. + * + * If the types of both are something meaningful, the search for a registered operation commences. + * If no operation can be found, init will fail. + */ + Class firstClass = first.getReturnType(); Class secondClass = second.getReturnType(); if (firstClass == Object.class || secondClass == Object.class) { - // If either of the types is unknown, then we resolve the operation at runtime + // if either of the types is unknown, then we resolve the operation at runtime Class[] returnTypes = null; if (!(firstClass == Object.class && secondClass == Object.class)) { // both aren't object if (firstClass == Object.class) { @@ -194,20 +248,21 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye .toArray(Class[]::new); } } - if (returnTypes == null) { + if (returnTypes == null) { // both are object; can't determine anything returnType = (Class) Object.class; - } else if (returnTypes.length == 0) { // One of the classes is known but doesn't have any operations + } else if (returnTypes.length == 0) { // one of the classes is known but doesn't have any operations return error(firstClass, secondClass); } else { returnType = (Class) Classes.getSuperClassInfo(returnTypes).getC(); } - } else if (returnType == null) { + } else if (returnType == null) { // lookup OperationInfo operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, firstClass, secondClass); - if (operationInfo == null) // We error if we couldn't find an operation between the two types + if (operationInfo == null) // we error if we couldn't find an operation between the two types return error(firstClass, secondClass); returnType = operationInfo.getReturnType(); } + // ensure proper return types for numerical operations if (Number.class.isAssignableFrom(returnType)) { if (operator == Operator.DIVISION || operator == Operator.EXPONENTIATION) { returnType = (Class) Double.class; @@ -222,14 +277,27 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye } } - // Chaining - if (first instanceof ExprArithmetic && !leftGrouped) { + /* + * Step 3: Chaining and Parsing + * + * This step builds the arithmetic chain that will be parsed into an ordered operation to be executed at runtime. + * With larger operations, it is possible that 'first' or 'second' will be instances of ExprArithmetic. + * As a result, their chains need to be incorporated into this instance's chain. + * This is to ensure that, during parsing, a "gettable" that follows the order of operations is built. + * However, in the case of parentheses, the chains will not be combined as the + * order of operations dictates that the result of that chain be determined first. + * + * The chain (a list of values and operators) will then be parsed into a "gettable" that + * can be evaluated during runtime for a final result. + */ + + if (first instanceof ExprArithmetic && !leftGrouped) { // combine chain of 'first' if we do not have parentheses chain.addAll(((ExprArithmetic) first).chain); } else { chain.add(first); } chain.add(operator); - if (second instanceof ExprArithmetic && !rightGrouped) { + if (second instanceof ExprArithmetic && !rightGrouped) { // combine chain of 'second' if we do not have parentheses chain.addAll(((ExprArithmetic) second).chain); } else { chain.add(second); From f00a381893e05fab6b29462a25dbf1406fe5a0bf Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Thu, 1 Feb 2024 18:04:24 -0500 Subject: [PATCH 6/6] Correct typo Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> --- .../ch/njol/skript/expressions/arithmetic/ExprArithmetic.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 531a6934419..0f72b2cb87e 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -221,7 +221,7 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye * If the types of both are object (e.g. variables), the return type will be object (have to wait until runtime and hope it works). * Of course, if no operations are found, init will fail. * - * After these checks, it is same to assume returnType has a value, as init should have failed by now if not. + * After these checks, it is safe to assume returnType has a value, as init should have failed by now if not. * One final check is performed specifically for numerical types. * Any numerical operation involving division or exponents have a return type of Double. * Other operations will also return Double, UNLESS 'first' and 'second' are of integer types, in which case the return type will be Long.