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

Add PackedRealArray as an alias for PackedFloat(32/64)Array #1340

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

aaronfranke
Copy link
Member

This PR adds PackedRealArray as an alias for PackedFloat(32/64)Array to allow you to specify that you want an array of the float size defined for real_t. In the core engine, it is possible to use Vector<real_t>, but this is not possible in GDExtension because PackedFloat(32/64)Array are explicit types and not just Vector<T>.

I used using instead of #define to ensure that the namespace is preserved, so that if you don't have using namespace godot; then you need godot::PackedRealArray just like you would need godot::PackedFloat32Array. I also tried using typedef and it works fine too including with the namespace, but using is the newer preferred syntax.

@aaronfranke aaronfranke added the enhancement This is an enhancement on the current functionality label Dec 19, 2023
@aaronfranke aaronfranke requested a review from a team as a code owner December 19, 2023 10:46
@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

Thanks! This makes sense to me. I would hit "approve" but I don't want this to accidentally get merged before the equivalent change is merged to Godot. :-)

@Bromeon
Copy link
Contributor

Bromeon commented Dec 20, 2023

Incidentally, having this upstream as a GDExtension type would also solve one of the problems with godotengine/godot#86346. (There are still others remaining).

@AThousandShips
Copy link
Member

accidentally get merged before the equivalent change is merged to Godot. :-)

Is this not what the waiting for Godot label is for?

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

Is this not what the waiting for Godot label is for?

Oh! I was not aware of that label. Would production team folks hold off from merging an approved PR if that label is present?

@AThousandShips
Copy link
Member

I don't know, seems so from the description, like the one in docs, just add a note in the op for the Godot side PR I'd say to help indicate

@YuriSizov
Copy link
Contributor

Would production team folks hold off from merging an approved PR if that label is present?

Well, I don't think the production team does much merging here, so it's mostly a signal for yourself 🙃

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

Well, I don't think the production team does much merging here, so it's mostly a signal for yourself 🙃

I know he's on vacation right now, but lately Remi's been merging approved godot-cpp PRs before I have a chance to :-)

@dsnopek dsnopek merged commit 1c19d62 into godotengine:master Dec 20, 2023
12 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Dec 20, 2023

Thanks!

@aaronfranke aaronfranke deleted the really-packed branch December 20, 2023 15:05
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 22, 2024

Cherry-picking to 4.2 in #1372

@YuriSizov
Copy link
Contributor

FYI engine counterpart will be available in 4.2.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants