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 warnings and errors are inconsistent, fail case for remove is not documented #3834

Closed
goatchurchprime opened this issue Jul 24, 2020 · 1 comment · Fixed by godotengine/godot#41812
Labels

Comments

@goatchurchprime
Copy link

Your Godot version:
3.2.2.rc1

Issue description:

When I run the following GDscript:

	var a = [10,11]
	print("a1: ",  a)
	a.remove(0)
	print("a2: ",  a)
	a.remove(10)
	print("a3: ", a)
	a.remove(0)
	print("a4: ", a)
	print("a5: ", a.front())
	print("a6: ", a.back())
	print("a7: ", a.pop_back())
	print("a8: ", a[5])
	print("Does not get here")

The output is:

a1: [10, 11]
a2: [11]
* ./core/cowdata.h:161 - Index p_index = 10 is out of bounds (size() = 1).
a3: [11]
a4: []
* Can't take value from empty array.
a5: Null
* Can't take value from empty array.
a6: Null
a7: Null
* res://immediate_code.gd:30 - Invalid get index '5' (on base: 'Array').

The documentation for the functions front(), back(), pop_front() and pop_back() all say: Returns null if the array is empty. However, a red warning message is printed for front() but not pop_front(). Why the warning if this is documented behavior?

The biggest issue is Array.remove() with an index outside the range, which sometimes (not always), prints out a cryptic message from core/cowdata.h.

The documentation does not say what will happen (warning? crash?) if the index in remove() is outside of the range.

Finally, when you access an element by an index outside the range, like a[0] on an empty array, you get an immediate crash. This means that a.front() and a[0] do different things.

I've looked into the code and there are uses of the macros ERR_FAIL_INDEX that prints a warning and CRASH_BAD_INDEX that generates a trap in some low-level code which is used in a lot of other places, not just the GDscript Array class.

It is not clear that the fail behaviors for these six Array functions are the most obvious, but what they do should be documented precisely.

URL to the documentation page:
https://docs.godotengine.org/en/stable/classes/class_array.html

@omicron321
Copy link
Contributor

although not consistent, aren't warnings, debug informations?
values returned by respective methods seem correctly document, reading your console output.

remove() behavior isn't specified/documented for invalid argument, indeed

Calinou added a commit to Calinou/godot that referenced this issue Sep 9, 2020
akien-mga pushed a commit to godotengine/godot that referenced this issue Sep 10, 2020
MarcusElg pushed a commit to MarcusElg/godot that referenced this issue Oct 19, 2020
huhund pushed a commit to huhund/godot that referenced this issue Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants