Runtime error for when EffSort aborts due to null value (#8206)

* Swap reset and delete errors in EffChange (#8177)

swap reset and delete errors

* Fix improper grammar in update block syntax (#8072)

Co-authored-by: sovdee <[email protected]>

* Fix Incorrect Variable Change Queue Processing (#8182)

Fix incorrect change processing order

* Change registerExpression() parameter name (#8180)

change parameter name

* Move EvtRealTime to main thread (#8185)

* Fix type-aware function parsing for functions with only optional arguments (#8189)

Account for functions with all optional parameters

* Properly parse exprsecs in function calls (#8199)

modify section context when parsing functions

* add runtime error when EffSort aborts due to null values.

* catch runtime errors

Author: sovdeeth

src/main/java/ch/njol/skript/Skript.java

@@ -1588,19 +1588,19 @@ public static <E extends Section> void registerSection(Class<E> sectionClass, St
 	/**
 	 * Registers an expression.
 	 *
-	 * @param expressionType The expression's class
+	 * @param expressionClass The expression's class
 	 * @param returnType The superclass of all values returned by the expression
 	 * @param type The expression's {@link ExpressionType type}. This is used to determine in which order to try to parse expressions.
 	 * @param patterns Skript patterns that match this expression
 	 * @throws IllegalArgumentException if returnType is not a normal class
 	 */
 	public static <E extends Expression<T>, T> void registerExpression(
-		Class<E> expressionType, Class<T> returnType, ExpressionType type, String... patterns
+		Class<E> expressionClass, Class<T> returnType, ExpressionType type, String... patterns
 	) throws IllegalArgumentException {
 		checkAcceptRegistrations();
-		skript.syntaxRegistry().register(SyntaxRegistry.EXPRESSION, SyntaxInfo.Expression.builder(expressionType, returnType)
+		skript.syntaxRegistry().register(SyntaxRegistry.EXPRESSION, SyntaxInfo.Expression.builder(expressionClass, returnType)
 				.priority(type.priority())
-				.origin(getSyntaxOrigin(expressionType))
+				.origin(getSyntaxOrigin(expressionClass))
 				.addPatterns(patterns)
 				.build()
 		);

src/main/java/ch/njol/skript/effects/EffBlockUpdate.java

@@ -32,7 +32,7 @@ public class EffBlockUpdate extends Effect {
 
 	static {
 		Skript.registerEffect(EffBlockUpdate.class,
-			"update %blocks% (as|to be) %blockdata% [physics:without [neighbo[u]r[ing]|adjacent] [physic[s]] update[s]]");
+			"update %blocks% (as|to be) %blockdata% [physics:without [neighbo[u]r[ing]|adjacent] [physics] update[s]]");
 	}
 
 	private boolean physics;

src/main/java/ch/njol/skript/effects/EffChange.java

@@ -145,14 +145,14 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 					}
 					yield what + " can't have anything removed from it";
 				}
-				case DELETE -> {
+				case RESET -> {
 					String error = what + " can't be reset";
 					if (changed.acceptChange(ChangeMode.DELETE) != null) {
 						error += ". However, it can be deleted which might result in the desired effect";
 					}
 					yield error;
 				}
-				case RESET -> {
+				case DELETE -> {
 					String error = what + " can't be deleted";
 					if (changed.acceptChange(ChangeMode.RESET) != null) {
 						error += ". However, it can be reset which might result in the desired effect";

src/main/java/ch/njol/skript/effects/EffSort.java

@@ -95,8 +95,12 @@ protected void execute(Event event) {
 				currentIndex = keyedValue.key();
 				currentValue = keyedValue.value();
 				Object mappedValue = mappingExpr.getSingle(event);
-				if (mappedValue == null)
+				if (mappedValue == null) {
+					error("Sorting failed because Skript cannot sort null values. "
+						+ "The mapping expression '" + mappingExpr.toString(event, false)
+						+ "' returned a null value when given the input '"+currentValue+"'.");
 					return;
+				}
 				mappedValues.add(new MappedValue(currentValue, mappedValue));
 			}
 			try {

src/main/java/ch/njol/skript/events/EvtRealTime.java

@@ -6,6 +6,7 @@
 import ch.njol.skript.lang.SkriptEvent;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.util.Time;
+import org.bukkit.Bukkit;
 import org.bukkit.event.Event;
 import org.bukkit.event.HandlerList;
 import org.jetbrains.annotations.NotNull;
@@ -82,8 +83,9 @@ public void run() {
 	@Override
 	public void unload() {
 		unloaded = true;
-		for (TimerTask task : timerTasks)
+		for (TimerTask task : timerTasks) {
 			task.cancel();
+		}
 		TIMER.purge();
 	}
 
@@ -94,14 +96,18 @@ public boolean check(Event event) {
 
 	private void execute() {
 		// Ensure this element wasn't unloaded
-		if (unloaded)
+		if (unloaded) {
 			return;
-		RealTimeEvent event = new RealTimeEvent();
-		SkriptEventHandler.logEventStart(event);
-		SkriptEventHandler.logTriggerStart(trigger);
-		trigger.execute(event);
-		SkriptEventHandler.logTriggerEnd(trigger);
-		SkriptEventHandler.logEventEnd();
+		}
+
+		Bukkit.getScheduler().scheduleSyncDelayedTask(Skript.getInstance(), () -> {
+			RealTimeEvent event = new RealTimeEvent();
+			SkriptEventHandler.logEventStart(event);
+			SkriptEventHandler.logTriggerStart(trigger);
+			trigger.execute(event);
+			SkriptEventHandler.logTriggerEnd(trigger);
+			SkriptEventHandler.logEventEnd();
+		});
 	}
 
 	@Override

src/main/java/ch/njol/skript/lang/SkriptParser.java

@@ -1234,7 +1234,7 @@ record SignatureData(ClassInfo<?> classInfo, boolean plural) { }
 				boolean trySingle = false;
 				boolean trySinglePlural = false;
 				for (var signature : signatures) {
-					trySingle |= signature.getMinParameters() == 1 || signature.getMaxParameters() == 1;
+					trySingle |= signature.getMinParameters() <= 1 || signature.getMaxParameters() == 1;
 					trySinglePlural |= trySingle && !signature.getParameter(0).isSingleValue();
 					for (int i = 0; i < signature.getMaxParameters(); i++) {
 						if (signatureDatas.size() <= i) {

src/main/java/ch/njol/skript/lang/Statement.java

@@ -30,7 +30,13 @@ public abstract class Statement extends TriggerItem implements SyntaxElement {
 
 	public static @Nullable Statement parse(String input, @Nullable String defaultError, @Nullable SectionNode node, @Nullable List<TriggerItem> items) {
 		try (ParseLogHandler log = SkriptLogger.startParseLogHandler()) {
-			EffFunctionCall functionCall = EffFunctionCall.parse(input);
+			Section.SectionContext sectionContext = ParserInstance.get().getData(Section.SectionContext.class);
+			EffFunctionCall functionCall;
+			if (node != null) {
+				functionCall = sectionContext.modify(node, items, () -> EffFunctionCall.parse(input));
+			} else {
+				functionCall = EffFunctionCall.parse(input);
+			}
 			if (functionCall != null) {
 				log.printLog();
 				return functionCall;
@@ -49,7 +55,6 @@ public abstract class Statement extends TriggerItem implements SyntaxElement {
 
 			Statement statement;
 			var iterator = Skript.instance().syntaxRegistry().syntaxes(org.skriptlang.skript.registration.SyntaxRegistry.STATEMENT).iterator();
-			Section.SectionContext sectionContext = ParserInstance.get().getData(Section.SectionContext.class);
 			if (node != null) {
 				var wrappedIterator = new Iterator<>() {
 					@Override

src/main/java/ch/njol/skript/variables/Variables.java

@@ -602,15 +602,14 @@ public static void setVariable(String name, @Nullable Object value, @Nullable Ev
 	 * @param value the value, or {@code null} to delete the variable.
 	 */
 	static void setVariable(String name, @Nullable Object value) {
-		boolean gotLock = variablesLock.writeLock().tryLock();
-		if (gotLock) {
+		if (variablesLock.writeLock().tryLock()) {
 			try {
-				// Set the variable
+				if (!changeQueue.isEmpty()) { // Process older, queued changes if available
+					processChangeQueue();
+				}
+				// Process and save requested change
 				variables.setVariable(name, value);
-				// ..., save the variable change
 				saveVariableChange(name, value);
-				// ..., and process all previously queued changes
-				processChangeQueue();
 			} finally {
 				variablesLock.writeLock().unlock();
 			}

src/test/skript/tests/regressions/8187-functions behave wrong with all default args.sk

@@ -0,0 +1,7 @@
+local function test(x: number = 3, y: number = 5) returns number:
+	return {_x} + {_y}
+
+test "functions behave wrong with all default args":
+	assert test() is 8 with "no args did not use both default values"
+	assert test(1) is 6 with "only one arg did not use the second default value"
+	assert test(1, 2) is 3 with "both args still used a default value"

src/test/skript/tests/regressions/8199-parse exprsecs in function args.sk

@@ -0,0 +1,6 @@
+local function f(x: worldborder):
+	assert worldborder warning time of {_x} is 1 second with "Failed to modify worldborder before calling function. Check Statement.parse() for sectioncontext handling."
+
+test "load expr secs in functions":
+    f(a worldborder):
+        set worldborder warning time to 20 ticks