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

Impossible to get the last element of an array using negative indices in ARRAY.slice() #61530

Closed
NationalityNZ opened this issue May 29, 2022 · 3 comments · Fixed by #79103
Closed

Comments

@NationalityNZ
Copy link

Godot version

Godot 4 alpha 8

System information

Windows 11, Surface pro

Issue description

It is currently impossible to return the last element of an array using Array.slice, when using negative indices. When you try, it silently fails and returns an empty array. Consequently, using Array.slice on an array with a single element is impossible.
Presumably, using arr.slice(-1,0) would return the final value of an array, but it does not.

func test3():
	
	var tiny_array := ["tinyarray"]
	var large_array := ["a","rather","bit","larger","array"]
	print(large_array)
	print(tiny_array)
	print(tiny_array.slice(-1,0))
	print(tiny_array.slice(0,1))
	print(large_array.slice(-1,0))
	print(large_array.slice(0,1))
	print(large_array.slice(-5,0))
	print(large_array.slice(-5,-1))
	print(large_array.slice(-2,-1))

Expected output:

["a", "rather", "bit", "larger", "array"]
["tinyarray"]
["tinyarray"]
["tinyarray"]
["array"]
["a"]
["a","rather","bit","larger","array"]
["a", "rather", "bit", "larger"]
["larger"]

Actual output:

["a", "rather", "bit", "larger", "array"]
["tinyarray"]
[]
["tinyarray"]
[]
["a"]
[]
["a", "rather", "bit", "larger"]
["larger"]

Steps to reproduce

See above.

Minimal reproduction project

No response

@kleonc
Copy link
Member

kleonc commented May 29, 2022

0 is a valid index so it won't be treated in any special way.
array.slice(-1, 0) is equivalent to array.slice(array.size() - 1, 0, 1) so it will return an empty array as end parameter is exclusive and step is positive.

To obtain the last element you just omit the second parameter so it will default to INT_MAX which will be correctly clamped to the array.size():

# These are equivalent:
array.slice(-1)
array.slice(array.size() - 1, array.size(), 1)

Current description in the docs correctly explains this behavior:

godot/doc/classes/Array.xml

Lines 445 to 458 in 823f1d3

<method name="slice" qualifiers="const">
<return type="Array" />
<argument index="0" name="begin" type="int" />
<argument index="1" name="end" type="int" default="2147483647" />
<argument index="2" name="step" type="int" default="1" />
<argument index="3" name="deep" type="bool" default="false" />
<description>
Returns the slice of the [Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [Array].
The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]).
If specified, [code]step[/code] is the relative index between source elements. It can be negative, then [code]begin[/code] must be higher than [code]end[/code]. For example, [code][0, 1, 2, 3, 4, 5].slice(5, 1, -2)[/code] returns [code][5, 3][/code]).
If [code]deep[/code] is true, each element will be copied by value rather than by reference.
</description>
</method>

@NationalityNZ
Copy link
Author

NationalityNZ commented May 29, 2022

Current description in the docs correctly explains this behavior:

Granted, the docs do explain it, but in a fairly opaque way that isn't obvious to amateurs like myself, relying on some understanding of how slice() works. It's really not easy to understand that a negative index will return the right side of an array.

Here is my proposed revision of the docs:


 <description> 
 		Returns the slice of the [Array], from [code]begin[/code] (inclusive) to [code]end[/code] (exclusive), as a new [Array]. 
 		The absolute value of [code]begin[/code] and [code]end[/code] will be clamped to the array size, so the default value for [code]end[/code] makes it slice to the size of the array by default (i.e. [code]arr.slice(1)[/code] is a shorthand for [code]arr.slice(1, arr.size())[/code]).
**This means that leaving the end parameter empty will return every element to the right of index. For example, [code][0, 1, 2, 3, 4, 5].slice(3) returns [code][3, 4, 5][/code]**
 		If either [code]begin[/code] or [code]end[/code] are negative, they will be relative to the end of the array (i.e. [code]arr.slice(0, -2)[/code] is a shorthand for [code]arr.slice(0, arr.size() - 2)[/code]). **Negative and positive indices can be combined, as long as the [code]begin[/code] has a smaller index on the array than [code]end[/code], otherwise an empty array will be returned. For example, [code][0, 1, 2, 3, 4, 5].slice(-4,5) returns [code][1, 2, 3, 4, 5][/code]
 		If specified, [code]step[/code] is the relative index between source elements. **In the case of step being negative, then the index of [code]begin[/code] must be higher than [code]end[/code]. For example, [code][0, 1, 2, 3, 4, 5].slice(5, 1, -2)[/code] returns [code][5, 3][/code]).**
 		If [code]deep[/code] is true, each element will be copied by value rather than by reference. 
 	</description>
 	

New text is encapsulated by **

@kleonc
Copy link
Member

kleonc commented Jul 7, 2023

There's indeed a flaw in the design but for the opposite case: currently when slicing with a negative step it's impossible to make the slice include the first element of the array (see #79103 (comment)). Marking this as a bug.

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

Successfully merging a pull request may close this issue.

2 participants