Skip to content

EffOperations #7764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: dev/feature
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 247 additions & 0 deletions src/main/java/ch/njol/skript/effects/EffOperations.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
package ch.njol.skript.effects;

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;
import ch.njol.skript.lang.SyntaxStringBuilder;
import ch.njol.skript.registrations.Classes;
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.OperationInfo;
import org.skriptlang.skript.lang.arithmetic.Operator;
import org.skriptlang.skript.log.runtime.SyntaxRuntimeErrorProducer;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
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). "
+ "Literals cannot be used on the left-hand side.")
@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 {_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
multiply 1 by 2
divide 10 by {_num}
""")
@Since("INSERT VERSION")
public class EffOperations extends Effect implements SyntaxRuntimeErrorProducer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not EffArithmetic?


private static final Patterns<Operator> 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());
}

private Operator operator;
private Expression<?> left;
private Class<?>[] leftAccepts;
private Expression<?> right;
private Node node;
private Operation<Object, Object, Object> operation = null;
private OperationInfo<?, ?, ?> operationInfo;

@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
operator = PATTERNS.getInfo(matchedPattern);
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 " + getOperatorVerb() + ".");
return false;
} else if (leftAccepts.length == 0) {
throw new IllegalStateException("An expression should never return an empty array for a ChangeMode of 'SET'");
}
// 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.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.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);
return false;
}
}
return LiteralUtils.canInitSafely(right);
}

@Override
protected void execute(Event event) {
Object rightObject = right.getSingle(event);
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;
}
Comment on lines +162 to +170
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we'd want runtime errors or not for null values (would like to hear what others think). Some operations/types have default values... do we need to be checking those here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since no one has replied, should I take it that it's fine to have?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should act identically to how set {_x} to {_x} * {_y} works
whatever that behavior is should be mirrored.


Class<?> rightType = rightObject.getClass();

Map<Class<?>, Operation<Object, Object, ?>> cachedOperations = new HashMap<>();
Set<Class<?>> invalidTypes = new HashSet<>();

Function<?, ?> changerFunction = (leftInput) -> {
Class<?> leftType = leftInput.getClass();
if (invalidTypes.contains(leftType)) {
printArithmeticError(leftType, rightType);
return leftInput;
}
Operation<Object, Object, ?> operation = cachedOperations.get(leftType);
if (operation == null) {
//noinspection unchecked
OperationInfo<Object, Object, ?> operationInfo = (OperationInfo<Object, Object, ?>) Arithmetics.lookupOperationInfo(operator, leftType, rightType, leftAccepts);
if (operationInfo == null) {
printArithmeticError(leftType, rightType);
invalidTypes.add(leftType);
return leftInput;
}
operation = operationInfo.getOperation();
cachedOperations.put(leftType, operation);
}
return operation.calculate(leftInput, rightObject);
};
//noinspection unchecked,rawtypes
left.changeInPlace(event, (Function) changerFunction);
}

@Override
public Node getNode() {
return node;
}

private void printArithmeticError(Class<?> left, Class<?> right) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would mark these methods as static if possible

Copy link
Contributor Author

@Absolutionism Absolutionism Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could mark genericParseError and noOperationError as static, but won't be able to with printArithmeticError since it uses runtime error.

String error = ExprArithmetic.getArithmeticErrorMessage(operator, left, right);
if (error != null)
error(error);
}

private void genericParseError(Expression<?> leftExpr, Class<?> rightType) {
Skript.error("'" + leftExpr + "' cannot be " + getOperatorVerb() + " by "
+ Classes.getSuperClassInfo(rightType).getName().withIndefiniteArticle() + ".");
}

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 getOperatorVerb() {
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", left, "by");
case DIVISION -> builder.append("divide", left, "by");
case EXPONENTIATION -> builder.append("raise", left, "to the power of");
}
builder.append(right);
return builder.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,29 @@ 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;
}

/**
* 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);
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<? extends T> getReturnType() {
return returnType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,35 @@ 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
) {
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)
return convertedInfo;
}
return null;
}

@Nullable
@SuppressWarnings("unchecked")
public static <L, R> OperationInfo<L, R, ?> lookupOperationInfo(Operator operator, Class<L> leftClass, Class<R> rightClass) {
Expand Down
Loading