Skip to content

Add missing Projection constructor with 16 real_t values #104113

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
Mar 14, 2025

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Mar 14, 2025

When GDExtension generates bindings, it creates C++ code based on the API JSON. Values are serialized into literals constructed with numbers matching the value, for example, Vector3(0, 1, 0). This includes default values for function parameters. Projection default values are serialized like Projection(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1). However, Godot is missing a constructor for Projection, so the generated C++ code doesn't compile:

Screenshot 2025-03-14 at 2 41 03 AM

I encountered this problem when the CI checks for godot-cpp failed to compile with my 4D module, because one of the bound methods includes a Projection default value, which cannot be constructed, as seen in the above image. This wasn't a problem in the engine before because the engine doesn't have any Projection default values in parameters.

Note: I believe to fix the exact problem, it only needs to be added to godot-cpp (see godotengine/godot-cpp#1742). However, we should make the same change in the engine for consistency. I also added this to C# but not GDScript since we do the same for other similar types (Transform2D/Transform3D/Basis).

Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

I could've sworn we already had a bulk constructor like this for Projection, but it really is only for the types you mentioned. We probably should have GDScript binds for this type of constructor, but that should be handled in a dedicated PR.

Either way, everything checks out!

@Repiteo Repiteo merged commit 4292f24 into godotengine:master Mar 14, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Mar 14, 2025

Thanks!

@aaronfranke aaronfranke deleted the projection-construct branch March 14, 2025 19:28
@aaronfranke aaronfranke added cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release labels Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release topic:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants