Skip to content

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
@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

3 participants