Skip to content

Add support for initializer_list to Array and TypedArray #1716

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

Merged
merged 1 commit into from
Apr 5, 2025

Conversation

tomfull123
Copy link
Contributor

This PR adds support for initializer_list syntax, allowing TypedArrays to be assigned more easily, with syntax such as: TypedArray<int> x = { 1, 2, 3 };

Its a nice to have feature that I've found missing while developing gdextensions, I'm also writing a gdscript to gdextension transpiler and this syntax would help to make that possible.

@tomfull123 tomfull123 requested a review from a team as a code owner February 22, 2025 00:04
@Ivorforce
Copy link
Member

Ivorforce commented Feb 22, 2025

Hi, and welcome!
This is a welcome contribution - initializer list construction is useful indeed.
In this case, it's relevant to know that godot-cpp has a design goal to stay close to Godot code as closely as possible. In some circumstances, this means not (yet?) adding features that are not in Godot upstream.

initializer_list construction for TypedArray is a bit of an outlier though, perhaps. While it's not in Godot yet, there are three (!) open pull requests that attempt to add it ([1], [2], [3]). It's probably only a matter of time till the Godot codebase merges one that adds them... But arguably we might wait to merge this PR until then.

@tomfull123
Copy link
Contributor Author

Yeah I saw those PRs a while back, I wasn't sure if it was a requirement to have it in Godot first, either way, I don't mind waiting, at least this PR will be ready to go when we finally get the feature in the Godot codebase.

@dsnopek dsnopek added enhancement This is an enhancement on the current functionality waiting for Godot This issue needs a Godot Engine improvement to be solved labels Feb 22, 2025
@dsnopek dsnopek added this to the 4.x milestone Feb 22, 2025
@dsnopek
Copy link
Collaborator

dsnopek commented Feb 22, 2025

Thanks!

However, I think we should wait for Godot to merge one of the upstream PRs before merging this one

@tomfull123
Copy link
Contributor Author

godotengine/godot#86015 Has been merged into Godot, can this PR be merged now?

@tomfull123
Copy link
Contributor Author

tomfull123 commented Mar 28, 2025

godotengine/godot#89782 has also been merged, can this PR be merged? 😊

@dsnopek
Copy link
Collaborator

dsnopek commented Apr 1, 2025

In the change to Godot, it added support for initializer list to Array and TypedArray - I think we should do the same here.

And, ideally, the implementation should look as similar to Godot's as possible, since typed_array.hpp is one of the files that we occasionally sync with Godot (although, it was skipped in the last sync)

@dsnopek dsnopek added cherrypick:4.4 and removed waiting for Godot This issue needs a Godot Engine improvement to be solved labels Apr 1, 2025
@tomfull123 tomfull123 force-pushed the master branch 3 times, most recently from 9ccd6d6 to 944627b Compare April 1, 2025 20:29
@tomfull123 tomfull123 marked this pull request as draft April 1, 2025 20:34
@tomfull123 tomfull123 force-pushed the master branch 2 times, most recently from ae81275 to 69c53a3 Compare April 1, 2025 20:45
@tomfull123 tomfull123 marked this pull request as ready for review April 1, 2025 20:45
@tomfull123 tomfull123 changed the title Add support for initializer_list to TypedArray Add support for initializer_list to Array and TypedArray Apr 1, 2025
@tomfull123 tomfull123 changed the title Add support for initializer_list to Array and TypedArray Add support for initializer_list to Array and TypedArray Apr 1, 2025
@tomfull123
Copy link
Contributor Author

In the change to Godot, it added support for initializer list to Array and TypedArray - I think we should do the same here.

And, ideally, the implementation should look as similar to Godot's as possible, since typed_array.hpp is one of the files that we occasionally sync with Godot (although, it was skipped in the last sync)

Added initializer list support to Array and made TypedArray call the Array constructor, how does it look?

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks great :-)

@dsnopek dsnopek merged commit 478e263 into godotengine:master Apr 5, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.4 enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants