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

Add explicit sort order to EffSort #6997

Merged
merged 8 commits into from
Aug 24, 2024
Merged

Add explicit sort order to EffSort #6997

merged 8 commits into from
Aug 24, 2024

Conversation

TenFont
Copy link
Contributor

@TenFont TenFont commented Aug 23, 2024

Description

Simply adds an explicit sort order to the sort effect, defaulting to ascending order.


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

@UnderscoreTud UnderscoreTud added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Aug 23, 2024
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.

Since you've changed the pattern you'll need to change the toString to reflect this.

@TenFont TenFont requested a review from Moderocky August 23, 2024 11:55
@UnderscoreTud UnderscoreTud requested a review from sovdeeth August 23, 2024 12:17
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.

Besides kenzie's requested change and this one looks fine to me

src/main/java/ch/njol/skript/effects/EffSort.java Outdated Show resolved Hide resolved
@Moderocky Moderocky merged commit 6b40d2e into SkriptLang:dev/feature Aug 24, 2024
3 checks passed
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants