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 a syntax to get the recursive size of a list #1869

Closed
bloggy opened this issue Feb 2, 2019 · 5 comments
Closed

Add a syntax to get the recursive size of a list #1869

bloggy opened this issue Feb 2, 2019 · 5 comments
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).

Comments

@bloggy
Copy link

bloggy commented Feb 2, 2019

Issue:
The "size of {_list::*}" variable seems to be broken. It returns wrong number.

Version:
[15:49:42] [Server thread/INFO]: [Skript] Enabling Skript v2.3.2
[15:49:42] [Server thread/INFO]: [Skript] Timings support enabled!
[15:49:43] [ForkJoinPool.commonPool-worker-1/INFO]: [Skript] You're currently running the latest stable version of Skript.

Example command:

command /listtest:
    trigger:
        add "test" to {_testlist::*}
        add "name1" to {_testlist::test::members::*}
        add "name2" to {_testlist::test::members::*}
        broadcast "%size of {_testlist::*}%"
        broadcast "%{_testlist::*}%"
        message "-----"
        loop {_testlist::*}:
            message "%loop-index%: %loop-value%"

Result:

2
test and <none>
----
1: test
@BaeFell
Copy link
Member

BaeFell commented Feb 4, 2019

In theory, it's right. You've got to picture it as testlist having the value "test" and then another value. That other value is a list but still regarded as 1.

@Blueyescat
Copy link
Contributor

Blueyescat commented Feb 4, 2019

Might be hard to fix but i can confirm. But it is not related to the size expression (assuming it shouldn't count nulls in the list). A better test code would be

command /listtest:
	trigger:
		add "test" to {_list::*}
		add "test2" to {_list::sublist::*}
		add "test3" to {_list::sublist2::*}
		add "test4" to {_list::sublist3::*}
		set {_list::sublist3} to true
		broadcast "%{_list::*}%"
		broadcast "---------"
		loop {_list::*}:
			broadcast "%loop-index%: %loop-value%"

broadcasts test, <none>, true and <none>

Somehow it includes sublists and returns null values for them, because they don't have actual values (like the true in the code above).

About the loop, it is a known issue you can't loop variable indexes that doesn't have a value, now we see reason of this ^^ The loop will broadcast 1: test and sublist3: true
Would you except it to loop only indexes but loop-value will be null? Then looping list variables would require a special behaviour.

@TheBentoBox
Copy link
Member

Yeah, I agree with nfell that this isn't exactly incorrect; looping a list doesn't iterate over sublists (and never has), but the sublist is still part of the list so it returns when checking its size.

That being said, this is obviously somewhat confusing and Skript could really use some improved list expressions -- namely the ability to deep loop over lists, via a new syntax so it doesn't break old loops. Something like MundoSK's loop tree of %objects% where branch is the entire deep key of the current iteration. A list created via something like set {_list::sublist::anotherSublist::exampleIndex} to 2 the could "deep loop"/"tree loop" should provide branch of loop-key which would be sublist::anotherSublist while loop-index would be exampleIndex. Or just have loop-index be the entire "deep" index/branch (sublist::anotherSublist::exampleIndex).

I'll leave it to devs to decide if they want to take any action on this.

@TheBentoBox TheBentoBox added enhancement Feature request, an issue about something that could be improved, or a PR improving something. dev-needed labels Feb 4, 2019
@BaeFell
Copy link
Member

BaeFell commented Feb 4, 2019

@TheBentoBox - without breaking current looping systems as scripts would of been designed to accommodate the fact that Skript doesn't loop sub-lists. Potentially it could be an easy thing such as:
loop {_list::*} including sub-lists:
That'd mean no current scripts are broken but it's easy to extend upon. But the behaviour of that would need to be figured out. In Java or PHP for example when looping an array, you need to check if the looped value is an array and handle that yourself. It doesn't loop into that array.

$arr = ["single" => "value", "array" =>  ["inside array" => "array value"]];
function loop_array($array) {
    foreach($array as $value) {
        if(is_array($value) {
            loop_array($value);
        } else {
            echo $value. "<br>";
        }
    }
}

What I'm trying to say is, would Skript loop every sub-list or just the first level of sub-lists? Would a condition like %object% is list be needed? Or does the current looping system be changed but have the is list condition and let people loop lists while looping? It's quite a tricky one to figure out.

@Nicofisi Nicofisi changed the title Size of list variable Bug Add a syntax to get the recursive size of a list Feb 5, 2019
@bensku
Copy link
Member

bensku commented Feb 8, 2019

This is not exactly easy to implement, but might be handy.

@bensku bensku added priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements). and removed dev-needed labels Feb 8, 2019
@Wealthyturtle Wealthyturtle added the completed The issue has been fully resolved and the change will be in the next Skript update. label Oct 21, 2019
@Wealthyturtle Wealthyturtle reopened this Dec 22, 2019
@bensku bensku closed this as completed Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something. priority: lowest "Nice to have" updates that are not required (tiny low impact bug fixes or QoL enhancements).
Projects
None yet
Development

No branches or pull requests

6 participants