Add an Array.pop_at() method to pop an element at an arbitrary index

[Calinou]

master version of #52143.

Negative indices are supported to pop an element relative from the end.

Example

func _ready():
	var array = [2, 4, 6, 8, 10]
	print("Original array: ", array)
	print(array.pop_at(2), ". Array is now ", array)
	print(array.pop_at(2), ". Array is now ", array)
	# Pop the last element.
	print(array.pop_at(-1), ". Array is now ", array)	
	# This is invalid since the array only has 3 elements at this point.
	# This will print an error and return `null`.
	print(array.pop_at(-15), ". Array is now ", array)
	print(array.pop_at(0), ". Array is now ", array)
	print(array.pop_at(0), ". Array is now ", array)
	# This will return `null` without printing an error.
	print("Trying to pop empty array. ", array.pop_at(24), ". Array is now ", array)

Original array: [2, 4, 6, 8, 10]
6. Array is now [2, 4, 8, 10]
8. Array is now [2, 4, 10]
10. Array is now [2, 4]
Null. Array is now [2, 4]
2. Array is now [4]
4. Array is now []
Trying to pop empty array. Null. Array is now []

This closes godotengine/godot-proposals#562.

PS: Should we print an error when trying to pop an empty array, or is the current behavior fine?

Edit: Not displaying an error is fine, for consistency with pop_back() and pop_front()'s behavior.

[realkotob]

I don't think we need an error when popping an empty array, it's not dangerous and makes a lot of sense to get a null element.

Imo errors should be for consequences that are either unsafe, suboptimal (performance wise), broken, or dangerous. This scenario is probably a Warning in the worse case :b

I like the idea of having a helper method instead of using splice() every time, but pop() has a well known and specific meaning already (removes and returns the last element) so not sure if it'll cause confusion.

[vnen]

I understand the appeal but I find the name confusing. array.pop(2) sounds like you are popping 2 elements.

Since we have pop_back and pop_front this could be pop_middle or pop_index.

Alternatively, we could make Array.remove() return the value.

[realkotob]

I prefer pop_index over Array.remove() but both are great ideas.

[Calinou]

I don't think we need an error when popping an empty array, it's not dangerous and makes a lot of sense to get a null element.

Indeed, it makes sense to follow pop_back()/pop_front()'s behavior here.

[katoneko]

How about pop_at()?

[Calinou]

I updated the pull request to rename the new method from pop() to pop_at(). I also improved the documentation in the process.

[mhilbrunner]

Needs to be rebased, otherwise looks good to go.

[groud]

Thanks!

[kleonc]

Take in mind that allowing negative indices makes things kind of inconsistent: they're allowed for pop_at() but they're not allowed for remove() or insert().

[Calinou]

Take in mind that allowing negative indices makes things kind of inconsistent: they're allowed for pop_at() but they're not allowed for remove() or insert().

I guess we should allow negative indices in remove() and insert() as well. Feel free to open a pull request for this 🙂