-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Array slicing for GDScript #4715
Comments
Note that this is easily done with var result = array[from:to:step]
# Is the same as:
var result = (array[i] for i in range(from,to,step)) |
Also, to throw in CoffeeScript syntax here as well: x = array[from...to] We might adapt this to support step size (since they don't): x = array[from..step..to]
# or
x = array[from...to:step] |
imho python syntax with : is more convenient - less characters and more compact |
+1. And less confusing. We are python-like, so let's not import behaviours from every other languages in the world or GDScript will become really inconsistent. |
D syntax ($ is the size of the array):
Just for reference. I agree Python's one fits better with GDScript :P |
I'm willing to work on this one. As a first step I'm preparing a PR that adds negative indexing. (If we allow |
What about assigning to array slices? Are there use cases? I'm confident that I can get the code to support it, but if there are no use cases, I might not bother with it. I think one restriction we have to make is that the replacement values must have the same size (which Python also enforces on array-like structures with a fixed size, like bytes) In other words, the following should work:
The following might:
But this most probably wont:
Opinions? |
Just for the record, I'm still working on it. I might have a PR ready in the next week, but it can take longer. |
@brakhane Nobody is in a great hurry -- take your time :) |
I think this should be implemented for 3.0, currently we just have the |
My implementation is 98% done (it works, but there are some memory leaks and random crashes which I didnt have the time to debug), but due to real life time constraints I won't be able to finish it. I'm more than willing to put my WIP into a branch so somebody can (help me) finish it. I can help with questions regarding the implementation. It would be a shame if my effort is going to waste (I spend probably around 40h so far on it) The implementation is pretty feature complete (and I'm actually quite proud of how complete it turned out), for example slicing works like in Python for most array types, including assignment ( There are a few caveats though:
Let me know If i should put the code somewhere. I think someone familiar with the godot script internals will get the kinks ironed out in 10h or less. |
@brakhane since you already have work done, it would be great if you put the code in your fork and open a Pull Request. This way other people can review and comment on your code. |
How's it going? |
@bojidar-bg is our new gdscript dev, care to give it a try? :P |
well, will push for 3.1 |
Just wanted to note that the subarray method uses inclusive ranges, whereas this will (hopefully) use python-style slicing ranges. I say hopefully because the subarray method is a bit painful to use, requiring you to write special casing to make sure you never try and create a zero width slice (e.g. if you're trying to consume part of a buffer by slicing it twice, the 'remaining' slice can be zero length), and to add Edit: in fact, I can't think of a situation off the top of my head where you wouldn't need to add a -1 to the end slice, unless you're slicing constant number bytes and so can do the -1 when writing it. |
Any progress with this? |
Moving to the next milestone as 3.1 is now feature frozen (and 3.2 has various GDScript improvements planned). |
Hi I just came across this item and wanted to throw in my two cents. If smart slicing in any way impacts the performance of individual array indexing then we should probably avoid it. Iterating over objects in GDScript is too common of a use-case to suffer performance penalties for the sake of a little syntactic sugar. While I'm all for syntactic sugar, I think the average game developer has reasonable expectations related to atomicity and speed when anything related to brackets, curly braces, parentheses etc. is implemented in GDScript. If that contract is broken then the question of when I should push something to GDNative becomes much more hazy. I think a good "sub_array()" or "slice()" function would be the best way to go because then (I assume) you could keep existing array indexing code as-is. |
Is there any progress on this? I could submit a PR that adds such methods. |
Still want this to be implemented. Any news? |
Same, I want to grab a subset from an array, and it seems like both list comprehensions and a function to slice a generic array don't exist. One of these should probably be added, as the alternative is a bit verbose. |
That's not currently correct, as comprehensions have yet to be included. The current syntax is: var result = []
for i in range(from, to, step):
result.append(array[i]) Bizarrely, the devs rejected a PR for this feature (#15222) for reasons of readability.
Personally, I find the one line no harder to read than the three. Anybody else think this conversation should be reopened? |
@SnailBones I think having a built-in |
I think list comprehensions would be nice, but having a separate |
I'm working on an |
Would it be more preferred to create a new Variant slice type like how python does it where that slice object is fed into the array as an operator, or for the call to the slice function in the array to be hard-coded in the compiler? I have the |
The slice object would be more "Pythonic" ;) but the hard-coded method
would almost certainly be faster. Either way, using the
start:end:delta syntax
would be very helpful.
…On Sat, Jul 20, 2019 at 6:05 PM Cameron Reikes ***@***.***> wrote:
Would it be more preferred to create a new Variant slice type like how
python does it where that slice object is fed into the array as an
operator, or for the call to the slice function in the array to be
hard-coded in the parser? I have the slice method done and I'm looking
into adding the start:end:delta syntax to gdscript indexing.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4715?email_source=notifications&email_token=ACJUE6P4A4CDQWUS4PSQQH3QAOK2JA5CNFSM4CEI4JPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NXQ4Q#issuecomment-513505394>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACJUE6LOTSF243FOYXXR2SDQAOK2JANCNFSM4CEI4JPA>
.
|
I only think to use a new slice kind of variant because then the slicing could be an operator in the parser and it would work out nicer, however I'm not super experienced in that area so it would be great to have one of the gdscript devs weigh in. |
You're right about that. It would also be much easier to re-use the syntax
structure for other things later. Sounds like the best option to me.
…On Sat, Jul 20, 2019 at 11:28 PM Cameron Reikes ***@***.***> wrote:
I only think to use a new slice kind of variant because then the slicing
could be an operator in the parser and it would work out nicer
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4715?email_source=notifications&email_token=ACJUE6IMHLNTYTVL7DITY2TQAPQX3A5CNFSM4CEI4JPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2N3D4Q#issuecomment-513520114>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACJUE6P5Q55EXAWD6IB4ARDQAPQX3ANCNFSM4CEI4JPA>
.
|
I will see if I can find my old implementation that's still lying around somewhere. It was 90% finished and worked kinda like Python does, in that there is a slice object that you can use to index into arrays. You could even do Stuff like arr[2:4] = [1,2,3]. I stopped working on it because of time constraints |
@brakhane That would be very helpful. Right now I'm in the process of adding a new slice object variant type so that the act of slicing can be considered an operation, making modifying the parser much easier. |
@creikey That sounds pretty similar to my approach. I've deleted my godot GitHub repo, but I might have the code still lying around elsewhere. I'll post an update |
@creikey I've found a not completely recent WIP (there might be some features missing in this version). You can find it here: https://github.com/brakhane/godot/compare/4c4ab14..8a258c0 There are still some bugs, for example, I just noticed here https://github.com/brakhane/godot/compare/4c4ab14..8a258c0#diff-7d521a4f767fb1ae3c908a20616084a4R1446 it should say ".end" instead of ".start" If you have questions, I'm happy to answer them. I will also try to find a more recent version. |
I am of the opinion now that adding pythonic array slicing rather than just a function with an inclusive upper bound is adding needless complexity, as it would entail creating a new variant, with little benefit in comparison to just adding a method. |
would be nice if we can use pythonish slice feature like
or even may be extended slicing with step
The text was updated successfully, but these errors were encountered: