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

Array.slice has an off-by-one error #72027

Closed
tavurth opened this issue Jan 25, 2023 · 9 comments
Closed

Array.slice has an off-by-one error #72027

tavurth opened this issue Jan 25, 2023 · 9 comments
Labels

Comments

@tavurth
Copy link
Contributor

tavurth commented Jan 25, 2023

Godot version

v4.0.beta14.official [28a2463]

System information

Mac

Issue description

print([1,2,3,4,5,6,7,8,9].slice(0, -1))

The above should include nine, but instead prints:

[1, 2, 3, 4, 5, 6, 7, 8]

Steps to reproduce

print([1,2,3,4,5,6,7,8,9].slice(0, -1))

Minimal reproduction project

N/a

@tavurth
Copy link
Contributor Author

tavurth commented Jan 25, 2023

var a = [1,2,3,4,5,6,7,8,9]
print(a.slice(0, len(a)-1))

Also prints [1, 2, 3, 4, 5, 6, 7, 8]

@tavurth
Copy link
Contributor Author

tavurth commented Jan 25, 2023

To get the correct result you have to now do:

var a = [1,2,3,4,5,6,7,8,9]
print(a.slice(0, len(a)))

However, negative slices are not working as expected.

@aXu-AP
Copy link
Contributor

aXu-AP commented Jan 25, 2023

I think this is how it should work? As noted by class reference:

Returns the slice of the Array, from begin (inclusive) to end (exclusive), as a new Array.

So the ending index is not included in resulting array. As you noted, these are the same:

print(a.slice(0, -1))
print(a.slice(0, len(a)-1))

Which return whole array except for the last element (index of which is len(a)-1).

@EMBYRDEV
Copy link
Contributor

This is the intended behaviour I believe.

@akien-mga
Copy link
Member

akien-mga commented Jan 25, 2023

Yes this is intentional. See #56661 and #56668.

@tavurth
Copy link
Contributor Author

tavurth commented Jan 25, 2023

Perhaps we should note in the docs somewhere that this has been updated. It took a while to track down this change when upgrading to 4.x

@aXu-AP
Copy link
Contributor

aXu-AP commented Jan 25, 2023

I too have pondered on how to know all the changes in Godot 4.
With endless amount of small changes a full upgrade guide would be quite big and hard to make.
If class reference would have a note on each method that has been changed, that would too have it's own cons. Adding extra information, which would become unnecessary in a few years (after most have moved on from 3.x). In this case especially, since old behaviour was the stranger one (see issues akien linked).

@tavurth
Copy link
Contributor Author

tavurth commented Jan 25, 2023

I too have pondered on how to know all the changes in Godot 4.
With endless amount of small changes a full upgrade guide would be quite big and hard to make.

This is the fourth (fifth?) time I've tried to upgrade my main project to Godot 4.x and I can see the amount of hard work that has gone into it.

There are a few niggling issues but in general I was excited and impressed to see that it took less than two hours to get it to render again.

Perhaps however a migration guide, which contains such things in an easy to read format could be helpful for users who will upgrade.

@tavurth
Copy link
Contributor Author

tavurth commented Jan 25, 2023

Perhaps the migration tool could add a TODO to each array.slice call.

I usually calculate indexes and so I doubt the tool could resolve it correctly unless using ChatGPT 😂

However a small TODO note would be very much appreciated and it would only apply to older projects as the upgrade tool would be working on them anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants