Skip to content
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

Add print_line for compatibility with engine modules #1653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
Contributor

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).

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

Successfully merging this pull request may close these issues.

print_line is not available
3 participants