Skript icon indicating copy to clipboard operation
Skript copied to clipboard

🚀 Add loop variable reference (`loop X as INDEX and VALUE`)

Open AyhamAl-Ali opened this issue 2 years ago • 13 comments

Description

Implement loop X as INDEX and VALUE, basically this doesn't handle any local var copying outside the loop because we don't do that in loops at all, so it's better to keep it this way

Currently:

  • Only local variables are accepted as reference
  • Can't use the same variable as both index and value reference
  • Can't use loop index reference when we're not looping a list variable
  • Local variables are shared outside loop, this was the behavior originally so I didn't touch it and I think it's good.

~I was thinking of supporting [as %-objects%] but couldn't find much use for that in Skript, if anyone has a good use case let me know.~

Preview

image

One of the main goals of this addition to simplify data of nested loops

loop {data::*} as {_i} and {_d}:
    add {someotherdata::%{_i}%} to {_d} # <----------- easier
    if difference between {_d} and loop-value > 5:
        loop {balances::*} as {_uuid} and {_balance}:
            # use {_uuid} instead of loop-index-2 <----------- cleaner
            # use {_balance} instead of loop-value-2 <----------- cleaner

More examples image


Target Minecraft Versions: Requirements: Related Issues:

  • #6052

AyhamAl-Ali avatar Sep 21 '23 17:09 AyhamAl-Ali

I'm not sure if this feature is necessary 😞. I feel like this is a really major change to fix a nonissue.

Pikachu920 avatar Sep 22 '23 02:09 Pikachu920

I do not think this feature is valid to Skript. I see no benefits over using loop-index and loop-value here. Using a loop section only variable just over complicates a loop. Also why is three broadcasted second and not in the third placement? Is it a Set?

TheLimeGlass avatar Sep 22 '23 03:09 TheLimeGlass

why is three broadcasted second and not in the third placement? Is it a Set?

skript sorts by indices. "three" comes before "two"

sovdeeth avatar Sep 22 '23 03:09 sovdeeth

I'm not sure if this feature is necessary 😞. I feel like this is a really major change to fix a nonissue.

What does make it major? it's a very simple addition as a shorthand for (loops, nested loops)

loop {data::*}:
    set {_d} to loop-value # <----------- instead of this
    # just some simple PoC
    add {someotherdata::%loop-index%} to {_d}
    if difference between {_d} and loop-value > 5:
        loop {balances::*}:
            # using loop-index-2 <----------- instead of this
            # using loop-value-2 <----------- instead of this

now you can simply do

loop {data::*} as {_i} and {_d}:
    add {someotherdata::%{_i}%} to {_d} # <----------- easier
    if difference between {_d} and loop-value > 5:
        loop {balances::*} as {_uuid} and {_balance}:
            # use {_uuid} instead of loop-index-2 <----------- cleaner
            # use {_balance} instead of loop-value-2 <----------- cleaner

It doesn't change any behavior, also just simplifies the code without any weird non-understandable syntax. It also doesn't fix anything, just an enhancement.

I do not think this feature is valid to Skript. I see no benefits over using loop-index and loop-value here. Using a loop section only variable just over complicates a loop. Also why is three broadcasted second and not in the third placement? Is it a Set?

What makes it that? it's similar to function params like instead of using function(string, player): then param-1 and param-2 you can use function(s: string, p: player) and use {_s} and {_p}

As for why three before two, as sovde said, because I didn't add them to the list instead I set the indicies, they get sorted alphabetically as this is the default Skript behavior which may seem weird IMO

AyhamAl-Ali avatar Sep 22 '23 07:09 AyhamAl-Ali

For the books:

I like this addition. I think its optional nature makes it easy to opt out of if you don't want to use it, but provides good tools for clarity and readability for users who are writing code with stacked loops. I'll leave a proper review once I have time to test it out.

sovdeeth avatar Sep 22 '23 07:09 sovdeeth

What does make it major? it's a very simple addition as a shorthand for (loops, nested loops)

loop {data::*}:
    set {_d} to loop-value # <----------- instead of this
    # just some simple PoC
    add {someotherdata::%loop-index%} to {_d}
    if difference between {_d} and loop-value > 5:
        loop {balances::*}:
            # using loop-index-2 <----------- instead of this
            # using loop-value-2 <----------- instead of this

now you can simply do

loop {data::*} as {_i} and {_d}:
    add {someotherdata::%{_i}%} to {_d} # <----------- easier
    if difference between {_d} and loop-value > 5:
        loop {balances::*} as {_uuid} and {_balance}:
            # use {_uuid} instead of loop-index-2 <----------- cleaner
            # use {_balance} instead of loop-value-2 <----------- cleaner

It doesn't change any behavior, also just simplifies the code without any weird non-understandable syntax. It also doesn't fix anything, just an enhancement.

This is not really a great argument for the use-case, since in reality changing loop-value is something to be fixed rather than adding an additional feature to work around it. That said, as a user, I am not against the feature however if you are going to go that route, may as well use a more standard and well-known syntax such as:

for {_value}, {_index} in {_objects::*}:
  # do stuff

moraedu avatar Sep 22 '23 13:09 moraedu

This is not really a great argument for the use-case, since in reality changing loop-value is something to be fixed rather than adding an additional feature to work around it. That said, as a user, I am not against the feature however if you are going to go that route, may as well use a more standard and well-known syntax such as:

for {_value}, {_index} in {_objects::*}:
  # do stuff

That is a breaking change. That's indeed how some other languages do it but we don't see other languages as main reference, Skript has its own Enlgish-friendly syntax.

AyhamAl-Ali avatar Sep 22 '23 13:09 AyhamAl-Ali

I'm not sure if this feature is necessary 😞. I feel like this is a really major change to fix a nonissue.

What does make it major? it's a very simple addition as a shorthand for (loops, nested loops)

It's major because loops are so core to the language that implementing a change like this is a big deal. I think the existing way of setting a variable manually is totally fine. there's also the issue of these variables polluting the scope after the loop finishes (e.g. {_balance} is still set after the loop is over)

Pikachu920 avatar Sep 22 '23 16:09 Pikachu920

It's major because loops are so core to the language that implementing a change like this is a big deal. I think the existing way of setting a variable manually is totally fine. there's also the issue of these variables polluting the scope after the loop finishes (e.g. {_balance} is still set after the loop is over)

But we are not changing them Pikachu, we are only adding an optional feature that's why I asked why is it a major.

Also, RE scoped variable please see the description, this is how it's currently already which is why I haven't changed it.

AyhamAl-Ali avatar Sep 22 '23 16:09 AyhamAl-Ali

Maybe I'm a heretic, but I feel like this could be done better as a second, extra loop syntax. We could borrow inspiration from other languages and have something like

for each (1:key|2:value) %variable% in {_list::*}:

allowing a user to choose if they're interested in looping the indices or values specifically, or we could have a syntax that provides both,

for [key] %var% and [value] %var% in {_list::*}:

there are a lot more possibilities here, please be encouraged to think outside the box :)

Moderocky avatar Sep 22 '23 17:09 Moderocky

I'm not sure if this feature is necessary 😞. I feel like this is a really major change to fix a nonissue.

What does make it major? it's a very simple addition as a shorthand for (loops, nested loops)

loop {data::*}:
    set {_d} to loop-value # <----------- instead of this
    # just some simple PoC
    add {someotherdata::%loop-index%} to {_d}
    if difference between {_d} and loop-value > 5:
        loop {balances::*}:
            # using loop-index-2 <----------- instead of this
            # using loop-value-2 <----------- instead of this

now you can simply do

loop {data::*} as {_i} and {_d}:
    add {someotherdata::%{_i}%} to {_d} # <----------- easier
    if difference between {_d} and loop-value > 5:
        loop {balances::*} as {_uuid} and {_balance}:
            # use {_uuid} instead of loop-index-2 <----------- cleaner
            # use {_balance} instead of loop-value-2 <----------- cleaner

It doesn't change any behavior, also just simplifies the code without any weird non-understandable syntax. It also doesn't fix anything, just an enhancement.

I do not think this feature is valid to Skript. I see no benefits over using loop-index and loop-value here. Using a loop section only variable just over complicates a loop. Also why is three broadcasted second and not in the third placement? Is it a Set?

What makes it that? it's similar to function params like instead of using function(string, player): then param-1 and param-2 you can use function(s: string, p: player) and use {_s} and {_p}

As for why three before two, as sovde said, because I didn't add them to the list instead I set the indicies, they get sorted alphabetically as this is the default Skript behavior which may seem weird IMO

That's not short hand at all, when you just replace {_d} with loop-value and don't even need to be setting the variable there.

This is bad behaviour as variables act as objects and will ultimately allow for placement in many syntaxes, but will only silently error at runtime when Skript finds out it's true runtime value.

People will be like "why dis no work" and we'll be like, well you put it in a variable for no reason, and now Skript sees it as an object.

TheLimeGlass avatar Sep 27 '23 09:09 TheLimeGlass

That's not short hand at all, when you just replace {_d} with loop-value and don't even need to be setting the variable there.

This is bad behaviour as variables act as objects and will ultimately allow for placement in many syntaxes, but will only silently error at runtime when Skript finds out it's true runtime value.

I disagree with you on that Lime. It's a good short-hand for nested loops, easier and cleaner with less code and nothing is broken with this feature, literally nothing because it's a known feature in other languages and also we've added it as optional with no new weird syntax with good limitation on weird user inputs.

I don't see where this could error at runtime? it's just setting vars just like any user would've done that. I would appreciate it if you give some examples on [runtime] errors.

AyhamAl-Ali avatar Sep 27 '23 09:09 AyhamAl-Ali

I still really don't like modifying the loop syntax like this because it's such an established thing.

Moderocky avatar Apr 13 '24 08:04 Moderocky

Closing in favour of #6562

sovdeeth avatar Sep 15 '24 20:09 sovdeeth