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 SecFilter, refactor Variable and Variables slightly to accomodate. #6912

Merged
merged 12 commits into from
Sep 1, 2024

Conversation

sovdeeth
Copy link
Member

@sovdeeth sovdeeth commented Jul 16, 2024

Description

Adds a filter section to allow filtering on many conditions at once, while maintaining indices and adding significant performance optimization when only removing a few elements of a large list.

Also support any and all, like multiline ifs.

set {_a::*} to integers between -10 and 10

filter {_a::*} to match:
	input is a number
	mod(input, 2) = 0
	input > 0

I would like feedback on the abuses of the Variable api I have committed, as I don't really think the code is that great. I couldn't think of any better approaches, however.


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

@sovdeeth sovdeeth added feature Pull request adding a new feature. up for debate When the decision is yet to be debated on the issue in question labels Jul 16, 2024
@Mwexim
Copy link
Contributor

Mwexim commented Jul 21, 2024

Wouldn't it be smart to also allow a lambda-like structure where one can return a boolean?

filter {_players::*}:
    if health of player > 10:
        return true
    return false

Not sure if Skript's current section API facilitates this feature, but it allows for more flexibility and is more intuitive in my opinion.

Either way, I like both options actually since your proposition is a really clean way to filter expressions without the cluttering '[]' syntax that is used currently.

@Fusezion
Copy link
Contributor

Sections have access to a return api iirc in 2.9+ so this should be possible

@sovdeeth
Copy link
Member Author

Wouldn't it be smart to also allow a lambda-like structure where one can return a boolean?

filter {_players::*}:
    if health of player > 10:
        return true
    return false

Not sure if Skript's current section API facilitates this feature, but it allows for more flexibility and is more intuitive in my opinion.

Either way, I like both options actually since your proposition is a really clean way to filter expressions without the cluttering '[]' syntax that is used currently.

I considered this, but I thought it could get messy, and at that point there's no benefit to having a filter section over just doing

loop {list::*}:
    if loop-value is x:
        delete {list::%loop-index%}

@TheAbsolutionism
Copy link
Contributor

TheAbsolutionism commented Aug 20, 2024

I have a suggestion question for this,

  1. What about an "alias" for input called value
  2. Matching the value if the list that is being looped, is a list variable, to also be able to use index of sorts. That way you can filter out the values and indices within one section, if a user wanted to

Example

filter {_a::*}:
    value is an integer
    index parsed as number is set

I know for 2 you could just simply do two filters, one that processes the values, then once that is done, to do the indices.
Was just curious though

@sovdeeth
Copy link
Member Author

I have a suggestion question for this,

1. What about an "alias" for `input` called `value`

2. Matching the `value` if the list that is being looped, is a list variable, to also be able to use `index` of sorts. That way you can filter out the values and indices within one section, if a user wanted to

Example

filter {_a::*}:
    value is an integer
    index parsed as number is set

I know for 2 you could just simply do two filters, one that processes the values, then once that is done, to do the indices. Was just curious though

You can already use input index! I'm not sure if using value would cause any issues but it seems reasonable.

@TheAbsolutionism
Copy link
Contributor

You can already use input index! I'm not sure if using value would cause any issues but it seems reasonable.

Ahhh ok, I was unaware of the input index, my apologies

@sovdeeth sovdeeth added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. and removed up for debate When the decision is yet to be debated on the issue in question labels Aug 20, 2024
while (keys.hasNext()) {
key = keys.next();
if (key != null) {
next = Variable.convertIfOldPlayer(subName + key, local, event, Variables.getVariable(subName + key, event, local));
Copy link
Member

Choose a reason for hiding this comment

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

unfortunate... we should aim to add an api for variable conversion (or find a way to move this) (not in this pr of course)

@sovdeeth sovdeeth merged commit 29c6764 into SkriptLang:dev/feature Sep 1, 2024
3 checks passed
@TheAbsolutionism TheAbsolutionism mentioned this pull request Jan 26, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature. 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.

7 participants