-
-
Notifications
You must be signed in to change notification settings - Fork 22.8k
Convert PtrToArg
macros to regular C++ structs.
#105231
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
Conversation
44fcd89
to
1151d9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than globally-defining structs with a leading underscore, could we contain them in an Internal
namespace instead?
1151d9d
to
42dd13f
Compare
Good idea, done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I went through and compared the new templates line-by-line to the original macros, and double checked that the right ones were used in the conversion. It all looked good to me!
I also built this and run it with the godot-cpp unit tests, and it all worked, but they don't comprehensively test all possible conversions
In any case, this looks great to me!
42dd13f
to
78ae591
Compare
Rebased and removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Thanks! |
I'd like to reduce the number of both macro and template magic throughout Godot. Some of it is pretty convoluted and can be simplified.
As a first step, macros should be converted to regular C++ structs as much as possible.
C style macros are difficult to edit, because they are (until called) not 'regular' code. For example, IDEs struggle to syntax highlight them, or give insights. Therefore, they are undesirable.
For this PR, I simply copy-pasted macro code to named structs. Then, I replaced the macro contents to simply instantiate the requested type with a specialization. Finally, I used my IDE to replace the macros with their contents, to avoid any mistakes with type arguments if I was manually converting.
As a second step, some of these can probably be simplified with C++ template magic in the future. But that's out of scope for this PR.