Skip to content

Commit f32ac2c

Browse files
[2.7.0 target] Fix issue where mixing different types of conditionals (#5932)
Fix issue where mixing different types of conditionals sometimes isn't allowed (#5872) (cherry picked from commit 639bcf7) Co-authored-by: Pikachu920 <28607612+Pikachu920@users.noreply.github.com>
1 parent d6cabe1 commit f32ac2c

File tree

2 files changed

+76
-20
lines changed

2 files changed

+76
-20
lines changed

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

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -118,21 +118,34 @@ public boolean init(Expression<?>[] exprs,
118118
multiline = parseResult.regexes.size() == 0 && type != ConditionalType.ELSE;
119119

120120
// ensure this conditional is chained correctly (e.g. an else must have an if)
121-
SecConditional lastIf;
122121
if (type != ConditionalType.IF) {
123-
lastIf = getClosestIf(triggerItems);
124-
if (lastIf == null) {
125-
if (type == ConditionalType.ELSE_IF) {
126-
Skript.error("'else if' has to be placed just after another 'if' or 'else if' section");
127-
} else if (type == ConditionalType.ELSE) {
128-
Skript.error("'else' has to be placed just after another 'if' or 'else if' section");
129-
} else if (type == ConditionalType.THEN) {
122+
if (type == ConditionalType.THEN) {
123+
/*
124+
* if this is a 'then' section, the preceding conditional has to be a multiline conditional section
125+
* otherwise, you could put a 'then' section after a non-multiline 'if'. for example:
126+
* if 1 is 1:
127+
* set {_example} to true
128+
* then: # this shouldn't be possible
129+
* set {_uh oh} to true
130+
*/
131+
SecConditional precedingConditional = getPrecedingConditional(triggerItems, null);
132+
if (precedingConditional == null || !precedingConditional.multiline) {
130133
Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
134+
return false;
135+
}
136+
} else {
137+
// find the latest 'if' section so that we can ensure this section is placed properly (e.g. ensure a 'if' occurs before an 'else')
138+
SecConditional precedingIf = getPrecedingConditional(triggerItems, ConditionalType.IF);
139+
if (precedingIf == null) {
140+
if (type == ConditionalType.ELSE_IF) {
141+
Skript.error("'else if' has to be placed just after another 'if' or 'else if' section");
142+
} else if (type == ConditionalType.ELSE) {
143+
Skript.error("'else' has to be placed just after another 'if' or 'else if' section");
144+
} else if (type == ConditionalType.THEN) {
145+
Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
146+
}
147+
return false;
131148
}
132-
return false;
133-
} else if (!lastIf.multiline && type == ConditionalType.THEN) {
134-
Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
135-
return false;
136149
}
137150
} else {
138151
// if this is a multiline if, we need to check if there is a "then" section after this
@@ -150,7 +163,6 @@ public boolean init(Expression<?>[] exprs,
150163
return false;
151164
}
152165
}
153-
lastIf = null;
154166
}
155167

156168
// if this an "if" or "else if", let's try to parse the conditions right away
@@ -231,9 +243,11 @@ public boolean init(Expression<?>[] exprs,
231243
return true;
232244

233245
if (type == ConditionalType.ELSE) {
246+
SecConditional precedingIf = getPrecedingConditional(triggerItems, ConditionalType.IF);
247+
assert precedingIf != null; // at this point, we've validated the section so this can't be null
234248
// In an else section, ...
235249
if (hasDelayAfter.isTrue()
236-
&& lastIf.hasDelayAfter.isTrue()
250+
&& precedingIf.hasDelayAfter.isTrue()
237251
&& getElseIfs(triggerItems).stream().map(SecConditional::getHasDelayAfter).allMatch(Kleenean::isTrue)) {
238252
// ... if the if section, all else-if sections and the else section have definite delays,
239253
// mark delayed as TRUE.
@@ -314,21 +328,28 @@ private Kleenean getHasDelayAfter() {
314328
return hasDelayAfter;
315329
}
316330

331+
/**
332+
* Gets the closest conditional section in the list of trigger items
333+
* @param triggerItems the list of items to search for the closest conditional section in
334+
* @param type the type of conditional section to find. if null is provided, any type is allowed.
335+
* @return the closest conditional section
336+
*/
317337
@Nullable
318-
private static SecConditional getClosestIf(List<TriggerItem> triggerItems) {
338+
private static SecConditional getPrecedingConditional(List<TriggerItem> triggerItems, @Nullable ConditionalType type) {
319339
// loop through the triggerItems in reverse order so that we find the most recent items first
320340
for (int i = triggerItems.size() - 1; i >= 0; i--) {
321341
TriggerItem triggerItem = triggerItems.get(i);
322342
if (triggerItem instanceof SecConditional) {
323-
SecConditional secConditional = (SecConditional) triggerItem;
343+
SecConditional conditionalSection = (SecConditional) triggerItem;
324344

325-
if (secConditional.type == ConditionalType.IF)
326-
// if the condition is an if, we found our most recent preceding "if"
327-
return secConditional;
328-
else if (secConditional.type == ConditionalType.ELSE)
345+
if (conditionalSection.type == ConditionalType.ELSE) {
329346
// if the conditional is an else, return null because it belongs to a different condition and ends
330347
// this one
331348
return null;
349+
} else if (type == null || conditionalSection.type == type) {
350+
// if the conditional matches the type argument, we found our most recent preceding conditional section
351+
return conditionalSection;
352+
}
332353
} else {
333354
return null;
334355
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,38 @@ test "SecConditional - else if all false":
143143
then:
144144
set {_a} to false
145145
assert {_a} is not set with "'else if all' ran even though all conditions were false"
146+
147+
test "SecConditional - starting with a non-multiline conditional":
148+
if 1 is 1:
149+
set {_a} to true
150+
else if:
151+
1 is 8
152+
2 is 7
153+
then:
154+
set {_b} to true
155+
assert {_a} is set with "non-multiline 'if' didn't run when before a multiline 'else if'"
156+
157+
test "SecConditional - non-multiline conditional in the middle":
158+
if:
159+
1 is 2
160+
3 is 4
161+
then:
162+
set {_b} to true
163+
else if 1 is 1:
164+
set {_a} to true
165+
else if:
166+
5 is 6
167+
7 is 8
168+
then:
169+
set {_c} to true
170+
assert {_a} is set with "non-multiline 'if' didn't run when used in the middle of a multiline 'else if'"
171+
172+
test "SecConditional - non-multiline conditional at the end":
173+
if:
174+
1 is 2
175+
3 is 4
176+
then:
177+
set {_b} to true
178+
else if 1 is 1:
179+
set {_a} to true
180+
assert {_a} is set with "non-multiline 'if' didn't run used at the end of a multiline 'if'"

0 commit comments

Comments
 (0)