added inline sorted by expression support with InputSource and as…#8436
Open
ethanrjs wants to merge 5 commits intoSkriptLang:dev/featurefrom
Open
added inline sorted by expression support with InputSource and as…#8436ethanrjs wants to merge 5 commits intoSkriptLang:dev/featurefrom
sorted by expression support with InputSource and as…#8436ethanrjs wants to merge 5 commits intoSkriptLang:dev/featurefrom
Conversation
…cending/descending order to ExprSortedList.
ethanrjs
requested review from
Absolutionism and
Burbulinis
and removed request for
a team
Efnilite
reviewed
Feb 9, 2026
…; use SyntaxStringBuilder
sovdeeth
requested changes
Feb 9, 2026
sovdeeth
requested changes
Feb 9, 2026
…ReturnType() instead of Object.class fixing a ClassCastException
sovdeeth
requested changes
Feb 10, 2026
Comment on lines
+193
to
+194
| catch runtime errors: | ||
| assert ({_data::*} sorted by ("%input%" parsed as time)) is not set with "keyed null mapping returned a value" |
Member
There was a problem hiding this comment.
should also assert the runtime error is what you expect it to be
Comment on lines
226
to
242
| # 'health of' requires %living entities%; 'all entities' returns %entities%, | ||
| # forcing getConvertedExpression(LivingEntity.class) on the sorted-by expression. | ||
| delete all entities in radius 50 around test-location | ||
| spawn a zombie at test-location: | ||
| set {_z1} to entity | ||
| set entity's name to "charlie" | ||
| spawn a zombie at test-location: | ||
| set {_z} to entity | ||
| spawn a pig at test-location: | ||
| set {_p} to entity | ||
| set {_entities::*} to {_z} and {_p} | ||
| set {_healths::*} to health of ({_entities::*} sorted by (health of input)) | ||
| assert size of {_healths::*} is 2 with "sorted-by entity type conversion: wrong count" | ||
| assert first element of {_healths::*} <= last element of {_healths::*} with "sorted-by entity type conversion: not ascending" | ||
| delete entity within {_z} | ||
| delete entity within {_p} | ||
| set {_z2} to entity | ||
| set entity's name to "alpha" | ||
| spawn a zombie at test-location: | ||
| set {_z3} to entity | ||
| set entity's name to "bravo" | ||
|
|
||
| set {_names::*} to names of ((all entities in radius 2 around test-location) sorted by (input's name)) | ||
| assert {_names::*} is "alpha", "bravo" and "charlie" with "sorted-by input name failed" | ||
| set {_healths::*} to health of ((all entities in radius 2 around test-location) sorted by (input's name)) | ||
| assert size of {_healths::*} is 3 with "sorted-by entity conversion failed" |
Member
There was a problem hiding this comment.
this is still a good test, but I would also add one that does inventories within ()
Bonus points for having a non-inventory holding entity like a minecart, and asserting that it's removed from the end list.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ExprSortedListonly supports basic natural-order sorting viasorted %objects%. To sort with a custom comparator or in descending order, you have to useEffSort, which mutates in-place, meaning you're forced to copy the list first:Solution
Added
sorted ... byexpression patterns toExprSortedListwithInputSourcesupport, matchingEffSort's behavior. The existingsorted %objects%pattern is unchanged.Testing Completed
Added
ExprSortedList.sktest file covering basic sorting, ascending/descending with sort-by, and edge cases.Also ran tests on-server.
Supporting Information
First contribution. Open to feedback :).
Completes: #8366
Related: #6158
AI assistance: Opus 4.6 Max reviewed my code + wrote tests in
ExprSortedList.sk