Fix trigger terminating sections influencing delay behavior (#8518)

Author: APickledWalrus

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

@@ -10,7 +10,6 @@
 
 /**
  * Used to describe the intention of a {@link TriggerItem}.
- * As of now, it is only used to tell whether the item halts the execution or not and print the appropriate warnings.
  *
  * @see TriggerItem#executionIntent()
  */

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

@@ -86,9 +86,16 @@ protected void loadCode(SectionNode sectionNode) {
 		sections.add(this);
 		parser.setCurrentSections(sections);
 
+		Kleenean hadDelayBefore = parser.getHasDelayBefore();
 		try {
 			setTriggerItems(ScriptLoader.loadItems(sectionNode));
 		} finally {
+			if (hadDelayBefore != parser.getHasDelayBefore()) { // the loaded elements caused a delay
+				ExecutionIntent intent = triggerExecutionIntent();
+				if (intent instanceof ExecutionIntent.StopTrigger) { // execution is guaranteed to stop, delay has no effect
+					parser.setHasDelayBefore(hadDelayBefore);
+				}
+			}
 			parser.setCurrentSections(previousSections);
 		}
 	}
@@ -176,12 +183,12 @@ protected final Trigger loadCode(SectionNode sectionNode, String name,
 	 * if the loaded section has a possible or definite delay in it.
 	 */
 	protected void loadOptionalCode(SectionNode sectionNode) {
-		Kleenean hadDelayBefore = getParser().getHasDelayBefore();
+		ParserInstance parser = getParser();
+		Kleenean hadDelayBefore = parser.getHasDelayBefore();
 		loadCode(sectionNode);
-		if (hadDelayBefore.isTrue())
-			return;
-		if (!getParser().getHasDelayBefore().isFalse())
-			getParser().setHasDelayBefore(Kleenean.UNKNOWN);
+		if (!hadDelayBefore.isTrue() && !parser.getHasDelayBefore().isFalse()) {
+			parser.setHasDelayBefore(Kleenean.UNKNOWN);
+		}
 	}
 
 	@Nullable

src/main/java/ch/njol/skript/sections/SecConditional.java

@@ -111,7 +111,7 @@ public boolean init(Expression<?>[] exprs,
 				 *    set {_uh oh} to true
 				 */
 				SecConditional precedingConditional = getPrecedingConditional(triggerItems, null);
-				if (precedingConditional == null || !precedingConditional.multiline) {
+				if (precedingConditional == null || !precedingConditional.multiline || precedingConditional.type == ConditionalType.THEN) {
 					Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
 					return false;
 				}
@@ -123,8 +123,6 @@ public boolean init(Expression<?>[] exprs,
 						Skript.error("'else if' has to be placed just after another 'if' or 'else if' section");
 					} else if (type == ConditionalType.ELSE) {
 						Skript.error("'else' has to be placed just after another 'if' or 'else if' section");
-					} else if (type == ConditionalType.THEN) {
-						Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
 					}
 					return false;
 				}
@@ -386,27 +384,28 @@ private static List<SecConditional> getPrecedingConditionals(List<TriggerItem> t
 			TriggerItem triggerItem = triggerItems.get(i);
 			if (!(triggerItem instanceof SecConditional conditional))
 				break;
-			if (conditional.type == ConditionalType.ELSE)
-				// if the conditional is an else, break because it belongs to a different condition and ends
-				// this one
-				break;
 			conditionals.add(conditional);
+			if (conditional.type == ConditionalType.IF)
+				// if the conditional is an if, break because it is the root of our conditional
+				break;
 		}
 		return conditionals;
 	}
 
-	private static @Nullable Kleenean getPrecedingShouldDelayAfter(List<TriggerItem> triggerItems) {
+	private @Nullable Kleenean getPrecedingShouldDelayAfter(List<TriggerItem> triggerItems) {
+		if (this.type == ConditionalType.IF)
+			// this is the head, meaning there is no preceding conditional
+			return null;
 		// loop through the triggerItems in reverse order so that we find the most recent items first
 		for (int i = triggerItems.size() - 1; i >= 0; i--) {
 			TriggerItem triggerItem = triggerItems.get(i);
-			if (!(triggerItem instanceof SecConditional conditional))
+			if (!(triggerItem instanceof SecConditional precedingSecConditional))
 				break;
-			if (conditional.type == ConditionalType.ELSE)
-				// if the conditional is an else, break because it belongs to a different condition and ends
-				// this one
+			if (precedingSecConditional.shouldDelayAfter != null)
+				return precedingSecConditional.shouldDelayAfter;
+			if (precedingSecConditional.type == ConditionalType.IF)
+				// this is the head of the conditional, meaning the search failed
 				break;
-			if (conditional.shouldDelayAfter != null)
-				return conditional.shouldDelayAfter;
 		}
 		return null;
 	}

src/test/skript/tests/regressions/8203-current delay does not consider execution intent.sk

@@ -0,0 +1,7 @@
+test "section delays considering execution intent":
+	parse:
+		set {_var} to true
+		if {_var} is true:
+			wait 1 second
+			stop
+	assert has delay before is false with "delay carried beyond section"

src/test/skript/tests/syntaxes/sections/SecConditional.sk

@@ -179,6 +179,17 @@ test "SecConditional - non-multiline conditional at the end":
 		set {_a} to true
 	assert {_a} is set with "non-multiline 'if' didn't run used at the end of a multiline 'if'"
 
+test "SecConditional - repeating then sections":
+	parse:
+		if:
+			1 is 1
+			2 is 2
+		then:
+			stop
+		then:
+			stop
+	assert last parse logs is "'then' has to placed just after a multiline 'if' or 'else if' section"
+
 test "SecConditional - delay logic 1":
 	assert has delay before is false with "should not have delay at start of test"