Skip to content
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

Allow 'continue' the continue outer loops #6024

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

UnderscoreTud
Copy link
Member

Description

This PR adds the ability to continue parent loops from within an inner loop. Loops are numbered from 1 to n and goes from outer to inner. For example:

loop all players: # Loop 1
    loop {values::%loop-value%::*}: # Loop 2
        set {_x} to foo(loop-value-2)
        if {_x} < 10:
            continue the 1st loop # Continues the 'loop all players' loop
    bar(loop-value)
    broadcast "Player has valid values!"

Target Minecraft Versions: any
Requirements: none
Related Issues: #6012

@UnderscoreTud UnderscoreTud added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Sep 17, 2023
Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Good

@Pikachu920
Copy link
Member

could we add some tests for this?

@Moderocky
Copy link
Member

Tests are a good idea!

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

When using continue loop without any specific number, the outer-most loop is continued instead of the inner-most.

command test:
    trigger:
        loop 3 times:
            broadcast "%loop-value%"
            loop 3 times:
                broadcast "- %loop-value-2%"
                loop 3 times:
                    broadcast "  - %loop-value-3%"
                    if loop-value-1 is 2, 5, or 8:
                        continue loop
[22:56:53 INFO]: 1
[22:56:53 INFO]: - 1
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3
[22:56:53 INFO]: - 2
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3
[22:56:53 INFO]: - 3
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3
[22:56:53 INFO]: 2
[22:56:53 INFO]: - 1
[22:56:53 INFO]:   - 1
[22:56:53 INFO]: 3
[22:56:53 INFO]: - 1
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3
[22:56:53 INFO]: - 2
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3
[22:56:53 INFO]: - 3
[22:56:53 INFO]:   - 1
[22:56:53 INFO]:   - 2
[22:56:53 INFO]:   - 3

Copy link
Contributor

@Fusezion Fusezion left a comment

Choose a reason for hiding this comment

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

Just a quick mild suggest to the pattern, I'd also like to mention the possibility in syntax like continue loop-3 I don't like the current method of forcing nd, rd, st or th and skript already has syntax similar to continue loop-3 in our loop-value-2

Comment on lines 62 to 63
Skript.registerEffect(EffContinue.class, "continue [[this|[the] (%*integer%(st|nd|rd|th)|current)] loop]");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think something like this might be a slightly better design, not only fixes the issue that was brought up about continue loop being a breaking change currently but seems like a little bit easier design to read this way anything using the number can be treated as it's respectful format

Suggested change
Skript.registerEffect(EffContinue.class, "continue [[this|[the] (%*integer%(st|nd|rd|th)|current)] loop]");
}
Skript.registerEffect(EffContinue.class,
"continue [[this|the [current]] loop]",
"continue [the] %*integer%(st|nd|rd|th) loop"
);
}

Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

I think some tests would be good for this as pikachu suggested :)

@UnderscoreTud
Copy link
Member Author

When using continue loop without any specific number, the outer-most loop is continued instead of the inner-most.

Ah, thanks for the catch!

Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

Well done ⚡

@AyhamAl-Ali AyhamAl-Ali added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Sep 18, 2023
@APickledWalrus APickledWalrus self-requested a review September 18, 2023 17:20
@Moderocky Moderocky merged commit 41349d1 into dev/feature Sep 18, 2023
@Moderocky Moderocky deleted the enhancement/continue-outer-loops branch September 18, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants