Skip to content

Conversation

aaronfranke
Copy link
Member

In the Godot core team meeting on 2025-05-07, it was discussed that PR godotengine/godot#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_verbose in godot-cpp has different behavior compared to the macro in the engine, and is objectively slower for no good reason.

Because of this, I opened PR godotengine/godot#106147 to add PRINT_VERBOSE, but @akien-mga suggested that if godot-cpp is the main motivation for making this change, we should see if the problem can be solved in godot-cpp.

This PR adds a capitalized PRINT_VERBOSE macro in parallel with the print_verbose function. This allows godot-cpp users to get the performance benefits of the macro. If users want to write C++ code that is compatible with both in-engine C++ and godot-cpp GDExtension C++, it is trivial to use PRINT_VERBOSE in an engine module like this:

#if GODOT_MODULE
#define PRINT_VERBOSE print_verbose
#endif // GODOT_MODULE

Alternate design to consider: GODOT_PRINT_VERBOSE as the name.

@aaronfranke aaronfranke added this to the 4.x milestone May 7, 2025
@aaronfranke aaronfranke requested a review from a team as a code owner May 7, 2025 17:54
@aaronfranke aaronfranke added the enhancement This is an enhancement on the current functionality label May 7, 2025
@dsnopek
Copy link
Collaborator

dsnopek commented Aug 26, 2025

For this to really be worth it peformance-wise, I think we should also be statically caching the verbose flag in godot::is_print_verbose_enabled().

So, changing it to look like:

bool is_print_verbose_enabled() {
	static bool verbose_enabled = OS::get_singleton()->is_stdout_verbose();
	return verbose_enabled;
}

Otherwise, we're making two calls across the GDExtension <-> Godot boundary instead of just one: the first to check OS::get_singleton()->is_stdout_verbose() and the 2nd to do print_line(), and that could actually be slower than the non-macro version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants