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

Remove "godot" namespace when binding global constants. #1351

Conversation

Daylily-Zeleen
Copy link
Contributor

Fix redundant prefix "godot." when binding method with godot's global constants.

@Daylily-Zeleen Daylily-Zeleen requested a review from a team as a code owner January 7, 2024 07:28
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 8, 2024

Thanks!

Is this fixing a problem? Or, just removing the godot:: because those macros internally wrap everything in namespace godot { ... }, making it unnecessary?

EDIT: Even though it might not be necessary, I think this change would have some knock on effects. The name that's passed to the macro ends up getting passed to enum_qualified_name_to_class_info_name(), which would change, for example, from "godot.MethodFlags" to just "MethodFlags". I don't know if that's desirable or undesirable, I haven't looked into what that affects, but it is a difference.

@Daylily-Zeleen
Copy link
Contributor Author

Even though it might not be necessary, I think this change would have some knock on effects. The name that's passed to the macro ends up getting passed to enum_qualified_name_to_class_info_name(), which would change, for example, from "godot.MethodFlags" to just "MethodFlags". I don't know if that's desirable or undesirable, I haven't looked into what that affects, but it is a difference.

This is the reason why I make this pr.

Currently, if a bind a method godot.Error do_something() in c++, it will be shown godot.Error do_something() in editor help document, and it will jump to a blank page instead of the Error in global scpoe by cliking "godot.Error".
This pr is fix this problem by getting rid of the "godot." prefix.

@dsnopek dsnopek added bug This has been identified as a bug cherrypick:4.1 cherrypick:4.2 labels Jan 10, 2024
@dsnopek dsnopek added this to the 4.x milestone Jan 10, 2024
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.

Ok, thanks, that makes sense!

It's always a good idea to include information about the motivation behind a PR. It's very helpful for reviewers :-)

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 11, 2024
@akien-mga akien-mga merged commit 6452936 into godotengine:master Jan 11, 2024
12 checks passed
@akien-mga
Copy link
Member

Thanks!

@Daylily-Zeleen Daylily-Zeleen deleted the daylily-zeleen/remove_namespace_in_global_constants_binding branch January 14, 2024 19:03
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 22, 2024

Cherry-picking to 4.2 in #1372

@dsnopek
Copy link
Collaborator

dsnopek commented Jan 22, 2024

Cherry-picking to 4.1 in #1373

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants