Skip to content

Conversation

@aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Nov 26, 2024

Fixes #1596. This PR adds most of the function signatures from the engine's print_string.h and places them in a file called print_string.hpp. The implementation from the engine is not copied, instead these wrap around the functions in UtilityFunctions. Personally I only need the non-vararg print_line but I've included the rest for parity.

print_string.hpp is included in class_db.hpp just like in the engine's class_db.h, which means these functions are available for anywhere that includes class_db.hpp. For the verbose function, I had to put the OS call into a separate file since os.hpp depends on class_db.hpp so there was a circular dependency if I did not. Also, the templates need to exist in the header, so I kept them and most of the functions in the header for simplicity.

Reference: https://github.com/godotengine/godot/blob/master/core/string/print_string.h

@aaronfranke aaronfranke added enhancement This is an enhancement on the current functionality cherrypick:4.3 labels Nov 26, 2024
@aaronfranke aaronfranke added this to the 4.4 milestone Nov 26, 2024
@aaronfranke aaronfranke requested a review from a team as a code owner November 26, 2024 11:03
@dsnopek
Copy link
Collaborator

dsnopek commented Nov 26, 2024

Overall, I think this is the right approach to getting module compatibility for these functions.

My one concern is if this could lead to more things being #included everywhere, because it's adding the #include to class_db.hpp, which any GDExtension class would need to include. But it's possible that something is already pulling in utility_functions.hpp everywhere and so it would make no difference? This should be looked into

@aaronfranke
Copy link
Member Author

I had that concern too, but I don't see a way around it, so long as we want these to be lightweight wrappers around the functions in UtilityFunctions. Anyway, it's not really even a bad thing to have utility_functions.hpp included. These functions are very general and are available in engine modules anyway.

The dependencies of utility_functions.hpp are variant.hpp which is already included in class_db.hpp through object.hpp, builtin_types.hpp which is already included from variant.hpp, and <array> which is already included from variant.hpp, so including utility_functions.hpp in class_db.hpp only adds what's in utility_functions.hpp, no additional transitive dependencies.

@Ivorforce
Copy link
Member

Ivorforce commented Nov 26, 2024

I can confirm utility_functions isn't part of any common include yet - any time i need to debug print i currently have to include it explicitly. Including it "by default" would actually reduce friction (in my workflow anyway).

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!

@dsnopek dsnopek merged commit 5255034 into godotengine:master Nov 28, 2024
11 checks passed
@aaronfranke aaronfranke deleted the print branch November 28, 2024 15:22
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 27, 2025

Cherry-picked for 4.3 in PR #1695

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

3 participants