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

GDScript typed arrays #46830

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Conversation

vnen
Copy link
Member

@vnen vnen commented Mar 9, 2021

This add typed arrays as valid GDScript.

Syntax

var my_array: Array[int] = [1, 2, 3]

Borrowing from Python since @pycbouh convinced me. It makes it easier to change from a typed array to a generic one and gets more consistent. It open margins for other collection types as well.

Inference is also possible:

var my_array := [1, 2, 3]

There's not an exact way to create a typed array literal. This is inferred by the context: if assigned to a typed variable or passed as a function argument to a typed parameter, then the array is typed (assuming all elements if of the same type). If this is deemed not enough we may consider adding one later, but for now I don't see a need.

Caveats

This relies on the typed array implementation from #38063. This shows an error in the console but it doesn't break the execution of the script. I've also had some issues with the API like Node.get_children() which in theory returns a typed array but it doesn't have any validation if you try to change it (this is separate from this PR though and should be fixed on the typed array implementation).

Edit: I've included here the needed changes in Array. You still don't get breakpoints when some errors happen though, as they are deep into the Array implementation and GDScript can't know about those.


Incidentally, this fixes issues with uninitialized values:
Fix #41631
Fix #44442
Fix #44793

@vnen vnen added this to the 4.0 milestone Mar 9, 2021
@vnen vnen requested review from a team as code owners March 9, 2021 15:47
@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 9, 2021

Python has the Array[int] syntax, but since we don't have full generics, you can assume the Array type is implied.

I would argue that Array[int] as a hint would look better next to a hint for a regular, untyped array. It would not be a question of the implication, but a question of familiarity and relation between a typed array and an untyped array. Plus adding a type would require to add a bit of text, not to replace it completely.

var var_list : Array = []
var int_list : Array[int] = []
# versus
var var_list : Array = []
var int_list : [int] = []

@vnen
Copy link
Member Author

vnen commented Mar 10, 2021

@pycbouh that's actually a good point

@vnen vnen marked this pull request as draft March 10, 2021 14:25
@vnen
Copy link
Member Author

vnen commented Mar 10, 2021

I'll make the change and also found another issue, so converting to draft for now.

@vnen vnen force-pushed the gdscript-typed-arrays branch from d942079 to 8d679b0 Compare March 10, 2021 16:01
@vnen vnen marked this pull request as ready for review March 10, 2021 16:02
@vnen
Copy link
Member Author

vnen commented Mar 10, 2021

Should be okay to merge. There are still some issues with some typed arrays, like I mentioned but I'm discussing with reduz a solution and could be done in another PR.

@vonflyhighace2
Copy link

@vnen, I was doing some testing using 4.0 branch and found that accessing a Dictionary has issues.

var my_dict : Dictionary = {"fun_stuff" : 0}
func _ready():
 print(my_dict.get('fun_stuff') #this works
 print(my_dict.fun_stuff)# this doesn't work
 print(my_dict["fun_stuff"]#also doesn't work
#an Invalid index error is given when trying to access the value

@Calinou
Copy link
Member

Calinou commented Mar 12, 2021

@vonflyhighace2 This was already reported here: #44241

@hilfazer
Copy link
Contributor

Typed array can accept anything in its definition. This will work:
var my_array: Array[int] = [1, "6", 3.6, null, "s2", AABB()]

Screenshot:
typedArrayAcceptsAnything

@YuriSizov
Copy link
Contributor

@hilfazer I think that issues vnen mentioned are precisely about that. Values are not properly validated, and it's the same internally (i.e. it's not a bug introduced in this PR)

@hilfazer
Copy link
Contributor

@hilfazer I think that issues vnen mentioned are precisely about that. Values are not properly validated, and it's the same internally (i.e. it's not a bug introduced in this PR)

Possibly, but i'd rather mention known problems than not mention problems people are not aware of.

There's also this:

	var mm :Array= [3.3]
	mm.append(2)
	

It doesn't allow me to append an int. Type got inferred as Array[float] even though i used explicit typing.

@barbaros83
Copy link

whats the diffrence between a typed array and a packed array ?

@vnen
Copy link
Member Author

vnen commented Mar 14, 2021

There are still some issues in the core Typed Array, I'm working on fixing those.

@vnen
Copy link
Member Author

vnen commented Mar 14, 2021

whats the diffrence between a typed array and a packed array ?

A packed array is a contiguous memory location with every element of the same type. A typed array is still an array of Variants, but there are extra guarantees that every element will have the same type. So packed arrays are a discrete list (only those shown are available) and typed arrays can have any valid type.

@vnen vnen force-pushed the gdscript-typed-arrays branch 2 times, most recently from 6d8a083 to 0e0bf4c Compare March 15, 2021 13:39
@vnen
Copy link
Member Author

vnen commented Mar 15, 2021

@hilfazer those issues should be fixed now.

@MaaaxiKing
Copy link

Isn't this the same as Pool<Variant>Array?

@hilfazer
Copy link
Contributor

The second issue is fixed indeed but the first one remains (both for local and script variables).

I've also did some testing for 2d typed arrays:

	var arrint2d : Array[Array[int]] = [[]]

	arrint2d[0].append(0)	# works, good
	arrint2d[0].append('re')	# works, bad
	arrint2d.append(2)	# runtime error, good
	arrint2d.append([5.5])	# works, bad
	arrint2d[0] = [null] # parsing error and it's expecting a value of type [int], good!

Inner array is a regular array during runtime, not Array[int].

@YuriSizov
Copy link
Contributor

@MaaaxiKing There are no PoolArrays in master, they are somewhat replaced by PackedArrays. As to the difference, look at the comment above:
#46830 (comment)

@vnen
Copy link
Member Author

vnen commented Mar 15, 2021

@hilfazer nested typed arrays are not supported.

Edit: I'll add an error message for this case.

@vonflyhighace2
Copy link

@vnen does this also apply to dictionary and how keys are validated?

@vnen
Copy link
Member Author

vnen commented Mar 15, 2021

@vnen does this also apply to dictionary and how keys are validated?

@vonflyhighace2 this is only for arrays.

@vnen vnen force-pushed the gdscript-typed-arrays branch from 0e0bf4c to 961c385 Compare March 16, 2021 13:19
@vnen
Copy link
Member Author

vnen commented Mar 16, 2021

Updated to add an error message in nested arrays and when adding the element type to anything that isn't array (since those aren't supported for now). Should have fixed the reported issues too.

@vnen vnen force-pushed the gdscript-typed-arrays branch from 961c385 to 3015fd5 Compare March 16, 2021 13:30
@hilfazer
Copy link
Contributor

The issues i mentioned are solved! But i found a new one (:

func makeFloats() -> Array[float]:
	return [.4, .5, 's']	# typed array is not enforced for the return type

It returns a normal array.

vnen added 3 commits March 18, 2021 10:18
The array should just assimilate the type of the other one since
assignment in this case means a change in the reference.

This also adds a `typed_assign` function for the cases where type
validation is wanted.
This ensure that typed arrays are properly checked when setting an
element.

Moved the macro to a straight declaration since the macro was only used
for Array and it now is quite specific to the Array class.
@vnen vnen force-pushed the gdscript-typed-arrays branch from 3015fd5 to 293be0f Compare March 18, 2021 13:26
@vnen
Copy link
Member Author

vnen commented Mar 18, 2021

@hilfazer ah, I forgot about this case, should be fixed now. Thanks for testing!

@hilfazer
Copy link
Contributor

That last issue i posted is now dealt with. But i decided to give it some serious testing and found some more:

func _ready():
	var floatArray1 = makeFloats1()
	var floatArray2 = makeFloats2()
	var floatArray3 = makeFloats3()
	var floatArray4 = makeFloats4()
	var floatArray5 = makeFloats5()
	pass


#func makeFloats0() -> Array[float]:
#	return [.3,AABB()]	# not allowed, good

func makeFloats1() -> Array[float]:
	var ar : = [.03]	# inferred as Array[float]
	ar.append(Plane() ) # not allowed, good
	return ar

func makeFloats2() -> Array[float]:
	var ar := []	# regular array
	ar.append(Plane() ) # gets appended
	return ar	# returns a normal array with Plane

func makeFloats3() -> Array[float]:
	var ar = [.03] # untyped variable
	ar.append(Plane() )
	return ar # it'll return because 'ar' currently holds an array

func makeFloats4() -> Array[float]:
	var ar :Array # regular array
	ar.append(Plane() ) # gets appended
	return ar	# returns a normal array with Plane
	
func makeFloats5() -> Array[float]:
	return [2] + ['orange']	# returns a normal array

@qarmin
Copy link
Contributor

qarmin commented Mar 23, 2021

I have this crash when running https://github.com/qarmin/Qarminer/archive/refs/heads/master.zip with address sanitizer

core/variant/array.cpp:143:16: runtime error: member access within misaligned address 0xbebebebebebebebe for type 'struct ArrayPrivate', which requires 8 byte alignment
0xbebebebebebebebe: note: pointer points here
<memory cannot be printed>
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s() [0x1e22550] (/mnt/Miecz/mojgodot/platform/linuxbsd/crash_handler_linuxbsd.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f4d342c4210] (??:0)
[3] Array::_assign(Array const&) (/mnt/Miecz/mojgodot/core/variant/array.cpp:143)
[4] Array::typed_assign(Array const&) (/mnt/Miecz/mojgodot/core/variant/array.cpp:534)
[5] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/mnt/Miecz/mojgodot/modules/gdscript/gdscript_vm.cpp:1119 (discriminator 1))
[6] GDScript::_super_implicit_constructor(GDScript*, GDScriptInstance*, Callable::CallError&) (/mnt/Miecz/mojgodot/modules/gdscript/gdscript.cpp:94)
[7] GDScript::_create_instance(Variant const**, int, Object*, bool, Callable::CallError&) (/mnt/Miecz/mojgodot/modules/gdscript/gdscript.cpp:121)
[8] GDScript::instance_create(Object*) (/mnt/Miecz/mojgodot/modules/gdscript/gdscript.cpp:352)
[9] Object::set_script(Variant const&) (/mnt/Miecz/mojgodot/core/object/object.cpp:837)
[10] Main::start() (/mnt/Miecz/mojgodot/main/main.cpp:2120 (discriminator 3))
[11] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s(main+0x33e) [0x1e1ff54] (/mnt/Miecz/mojgodot/platform/linuxbsd/godot_linuxbsd.cpp:57)
[12] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f4d342a50b3] (??:0)
[13] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s(_start+0x2e) [0x1e1fb5e] (??:?)

- Use `Array[type]` for type-hints. e.g.:
  `var array: Array[int] = [1, 2, 3]`
- Array literals are typed if their storage is typed (variable
  asssignment of as argument in function all). Otherwise they are
  untyped.
@vnen vnen force-pushed the gdscript-typed-arrays branch from 293be0f to 85e316a Compare March 29, 2021 13:45
@vnen
Copy link
Member Author

vnen commented Mar 29, 2021

The crash should be solved now. I believe this is good to merge. The error with the return value is actually deeper than this so I would prefer to fix it in a different PR.

@akien-mga akien-mga merged commit aba0311 into godotengine:master Mar 29, 2021
@akien-mga
Copy link
Member

Thanks!

@qarmin
Copy link
Contributor

qarmin commented Mar 29, 2021

Probably since this commit I have this message when running editor with undefined sanitizer:

modules/gdscript/gdscript_analyzer.cpp:647:100: runtime error: member access within address 0x612001103150 which does not point to an object of type 'VariableNode'
0x612001103150: note: object is of type 'GDScriptParser::ConstantNode'
 be be be be  90 3f 3a 1f 00 00 00 00  0c 00 00 00 10 00 00 00  10 00 00 00 07 00 00 00  12 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'GDScriptParser::ConstantNode'

@vnen
Copy link
Member Author

vnen commented Mar 30, 2021

@qarmin ah, nice catch, I've made a copy paste mistake there.

@Gastronok
Copy link
Contributor

Are these supposed to support Object types?

All the examples I've seen have been with int/float/String/NodePath and those seem to work fine, but if I try to create say an Array[Node3D] I get the error @implicit_new: Class names can only be set for type OBJECT.

@dongohu
Copy link

dongohu commented Sep 30, 2023

There's a bit of a snag with the typed arrays implementation in Godot 4.1.2rc (haven't tried earlier version of Godot 4)
Certain typed arrays will generate new ID-ed empty members upon serialization without copying the original ID members.
This can results in some lost links between exported array variables and what's actually generated in a scene.

From my experience, this is an example:

I have set up a scene for my settings menu.
All the toggles in the settings are member of specific defined Buttons Groups.
(One group for language, one group for each graphics settings, etc.)

To allow myself to set the Toggle buttons state properly when the menu is open after the settings were loaded from a saved file, I access the Button Groups and, in each case, tell Godot which button should be pressed.

To access each Button Group, if I @export every Button Groups separately (not in an array) as variables of type ButtonGroup, then set their references in the inspector I was able to connect and access those Button Groups just fine even if those Button Group were "Local to Scene" toggled ON.

This would look like this:

@export var BtnG_Settings_Language: ButtonGroup
@export var BtnG_Settings_GraphicsQuality: ButtonGroup 
@export var BtnG_Settings_ShadowsQuality: ButtonGroup

...and so on. It works and I was able to use something like this:

BtnG_Settings_Language.get_buttons()[GameData.LanguageID].set_pressed(true)

to set, for example, which of the buttons in the Language Button Group would be pressed.
(GameData is a global script in my project that manage files access such as Profile save and settings files.)

Now, if I was to use an array instead for getting all the button groups like this:

@export var SceneButtonGroups: Array[ButtonGroup]

Then I setup the array in the editor similarly to when I set up the individual variables previously.
and then uses this to access the buttons in the group:

SceneButtonGroups[0].get_buttons()[GameData.LanguageID].set_pressed(true)

If the check box "Local to Scene" of the button group is toggled ON, the SceneButtonGroups[0] of my line above will refer, once the scene is loaded, to an new Object ID that is different from the original Object ID which is the Button Group located in the file (that is a normal behavior). But when I look at the content of that newly generated (by the scene) Button Group, it has 0 members.

I went further into thing and looked at the Button Group from which the buttons are members of (in remote view).
For example, as my game has 2 languages: English and French, I got 2 Toggle Buttons as member of the same Button group for that language selection. If I look at the Object ID pointed out by each button as their Button Group source ID, I get:
Button Group = Object ID: 9223372079083029868
Then if I look at the script/exposed value which manage the setting menu (which is where I try to access the exposed variable), for that particular button group, I get:
Remote ButtonGroup: -9223371997663197950

If I look up the members of the Button Group Object ID 9223372079083029868, I do get a reference to the right 2 buttons as members.
If I look up the members of the Button Group Object ID -9223371997663197950, I get a Button Array that has 0 members.

At the moment, the only way of making such this problem doesn't appear is to make it so that any references in an array of certain types (if applicable) is set to Global and not Scene Local within their resources parameters.

@vnen
Copy link
Member Author

vnen commented Oct 19, 2023

@dongohu you should open an issue about your problem. It is hard to keep track of comments in closed PRs.

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