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

C#: Adding = new Array<int>() when declaring an int array Export, makes 0 be considered a null value instead of 0 #98309

Closed
cjohnson57 opened this issue Oct 18, 2024 · 4 comments · Fixed by #98545

Comments

@cjohnson57
Copy link

Tested versions

4.3 stable, as well as 4.3.dev5

System information

Godot v4.3.stable.mono - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2070 SUPER (NVIDIA; 32.0.15.6081) - AMD Ryzen 9 3900X 12-Core Processor (24 Threads)

Issue description

In C#, using Godot.Collections.Array<int>, I ran into the following obscure issue. My habit when declaring class variables is usually to initialize them, so I did so like this:

[Export]
public Array<int> includedPanels = new Array<int>();

After doing so, I initialized the array in the editor and added a value of 0 to it. However, upon running includedPanels.Contains(0) on it, I was surprised that it returned false.

Eventually, I realized that the 0 was actually being stored as null. Bizarrely, I figured out that, if I remove the = new Array<int>(), then rebuild the project and re-initialize the array, then the 0 is actually stored as 0. I changed my declaration like so, before building, resetting the array in the editor, and initializing it again:

[Export]
public Array<int> includedPanels;

And this produced the following change (screenshot from my Git client):
image

Steps to reproduce

Create a class and attach its script to a node.

Add the following properties to the class:

[Export]
public Array<int> noInitialValue;

[Export]
public Array<int> initialValue = new Array<int>();

Build the project, and then, in the editor, click on the node the script is attached to. Initialize both arrays with a value of 0. Open the scene file in a text editor, and you will see that the first contains 0 and the other contains null.

Minimal reproduction project (MRP)

N/A

@raulsntos
Copy link
Member

This is probably because Godot.Collections.Array<T> does not set the underlying array type, so the editor stores an untyped array, and when you add an element to it, it adds a default Variant which is null.

@cjohnson57
Copy link
Author

This is probably because Godot.Collections.Array<T> does not set the underlying array type, so the editor stores an untyped array, and when you add an element to it, it adds a default Variant which is null.

That's interesting, thanks for the insight!

@a-johnston
Copy link
Contributor

This is probably because Godot.Collections.Array<T> does not set the underlying array type, so the editor stores an untyped array, and when you add an element to it, it adds a default Variant which is null.

@raulsntos would it be reasonable to add an alternative godotsharp_array_new which accepts type information, so that when some Array<T> is created from the c# side the native value can have set_typed called as appropriate? Or potentially some new godotsharp_array_set_typed for the Array<T> constructor to invoke? I probably would bias towards the former to reduce native calls but the latter feels a bit cleaner.

I guess it's a bit weird in either case because in every scenario besides changing c# arrays from the editor it's c# handling type validation anyways.

@raulsntos
Copy link
Member

We likely want to match what godot-cpp is doing and call Array::set_typed in the constructor1. There's already a PR that I think does this but I haven't had a chance to review it yet:

Footnotes

  1. https://github.com/godotengine/godot-cpp/blob/a98d41f62bdb8b7aa903e8e37c1faa48fe8fdae8/include/godot_cpp/variant/typed_array.hpp#L58

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.

4 participants