EffOperations

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<?
 		}
 	}
 
-	private String getOperatorName() {
+	private String getOperatorVerb() {
 		return switch (operator) {
 			case MULTIPLICATION -> "multiplied";
 			case DIVISION -> "divided";

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);

src/main/java/org/skriptlang/skript/lang/arithmetic/Arithmetics.java

@@ -129,15 +129,27 @@ public static <L, R, T> OperationInfo<L, R, T> 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 <L, R> @Nullable OperationInfo<L, R, ?> lookupOperationInfo(
 		Operator operator,
 		Class<L> leftClass,
 		Class<R> rightClass,
-		Class<?> ... possibleReturnTypes
+		Class<?>... possibleReturnTypes
 	) {
 		OperationInfo<L, R, ?> 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<L, R, ?> convertedInfo = info.getConverted(leftClass, rightClass, returnType);
 			if (convertedInfo != null)

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