-
-
Notifications
You must be signed in to change notification settings - Fork 23.7k
Add capitalized PRINT_VERBOSE macro alongside the lowercase one
#106147
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
base: master
Are you sure you want to change the base?
Conversation
Ivorforce
left a comment
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.
For me this is an acceptable middle ground.
It's of course not super nice to have two things doing the same, but as long as we clearly communicate we prefer one over the other, this solves the godot-cpp problem we've been having.
Let's hear from more people though before we do anything as merge :)
|
Instead of adding a redundant macro for what's mostly a technicality, I would prefer to evaluate first what options @dsnopek referred to here:
If godot-cpp is the main motivation for making this change, we should see if the problem can be solved in godot-cpp. |
d7c1910 to
f58f5a2
Compare
I'm actually not sure that The reason I'm unsure is that crossing the GDExtension <-> Godot boundary has a cost, and both checking if verbose output is enabled and doing the print itself involve crossing that boundary. So, it may actually be faster to just cross the boundary once (calling print regardless of if verbose output is enabled or not), rather than checking first. Although, thinking about this some more as I'm writing this, maybe we'd just need to cache if verbose output is enabled on the godot-cpp side, and then checking first would indeed be faster. Hm.. |
f58f5a2 to
57fc1b9
Compare
57fc1b9 to
384c62f
Compare
384c62f to
768bf9b
Compare
768bf9b to
8ac701c
Compare
8ac701c to
5456cce
Compare
5456cce to
42b4760
Compare
42b4760 to
61e15a9
Compare
61e15a9 to
56b6b50
Compare
56b6b50 to
df5e2b2
Compare
In the Godot core team meeting on 2025-05-07, it was discussed that PR #100224 is a massive codebase-wide change that will conflict with many PRs and break compatibility with third-party modules. Therefore, there is a lot of hesitation with merging it for a minor improvement, which is itself not unanimously agreed on.
However, there is still the problem of the name conflict in godot-cpp GDExtension, which currently uses a work-around of using a function instead of a macro. This means that
print_verbosein godot-cpp has different behavior compared to the macro in the engine, and is objectively slower for no good reason.This PR adds the capitalized
PRINT_VERBOSEmacro without changing all existing uses throughout the codebase. This allows us to gradually migrate without breaking every PR, and more importantly, allows us to add the capitalized macro to godot-cpp, which lets users get the behavior and performance benefits of the macro, while still having their code be compatible between in-engine C++ and godot-cpp GDExtension C++.