Skip to content

Capitalize PRINT_VERBOSE macro to match other macros #100224

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aaronfranke
Copy link
Member

This PR changes the print_verbose macro to PRINT_VERBOSE, for 3 reasons:

  1. Fixes a name conflict with GDExtension UtilityFunctions: Fix print_verbose() macro conflicting with UtilityFunctions::print_verbose() godot-cpp#1668
  2. Fixes a name conflict with the engine's internal VariantUtilityFunctions, avoiding this awkwardness:
Screenshot 2024-12-09 at 2 35 36 PM
  1. Capitalizing the macro is consistent with the style used for all other macros in Godot.

This PR may minorly affect compatibility with third-party modules, but does not change the exposed scripting API. However, it is trivial for third-party modules to either update their code or add print_verbose as an alias.

@aaronfranke aaronfranke added this to the 4.4 milestone Dec 9, 2024
@aaronfranke aaronfranke requested review from a team as code owners December 9, 2024 22:47
@aaronfranke aaronfranke removed the request for review from a team December 9, 2024 22:47
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from 4b2ca26 to c4b5b91 Compare December 13, 2024 05:04
@aaronfranke aaronfranke requested a review from a team as a code owner December 13, 2024 05:04
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from c4b5b91 to 3c31863 Compare December 18, 2024 07:56
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from 3c31863 to 8f62399 Compare January 3, 2025 15:59
@Repiteo Repiteo modified the milestones: 4.4, 4.x Jan 20, 2025
@akien-mga
Copy link
Member

The name conflict part of the issue has been solved by #102062.

Now, there's still merit in discussing whether we want to make the macro PRINT_VERBOSE for consistency with usual macro naming style, or keep it lower case for consistency with print_line and print_error functions.

I'm personally more for the latter, but I wouldn't strongly oppose the former if there's consensus that it's the way to go.

@dsnopek
Copy link
Contributor

dsnopek commented Jan 30, 2025

The name conflict part of the issue has been solved by #102062.

It fixes it on the Godot side. We still kind of have it on the godot-cpp side, although, we're working around it by making it a function, and maybe have a couple other options to work around it if we want print_verbose() to be a macro there too.

Now, there's still merit in discussing whether we want to make the macro PRINT_VERBOSE for consistency with usual macro naming style, or keep it lower case for consistency with print_line and print_error functions.

There are semantic differences between functions and macros, so I'd personally be for PRINT_VERBOSE so it's clear that it's a macro, avoiding any confusion. That said, this isn't a very big problem :-)

@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch 2 times, most recently from aa29d8c to 1183f70 Compare February 15, 2025 16:35
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch 2 times, most recently from fd356d5 to 33341d2 Compare March 15, 2025 09:23
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch 3 times, most recently from 2bce9c6 to d4087f4 Compare April 3, 2025 05:51
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch 3 times, most recently from 118a968 to 0985dea Compare April 10, 2025 08:16
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch 2 times, most recently from 53111e1 to f4de73e Compare May 7, 2025 09:58
@aaronfranke aaronfranke changed the title Capitalize PRINT_VERBOSE macro to avoid a conflict with UtilityFunctions Capitalize PRINT_VERBOSE macro to match other macros May 10, 2025
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from f4de73e to cd0fb47 Compare May 10, 2025 21:22
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from cd0fb47 to a1ba06f Compare May 21, 2025 12:23
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from a1ba06f to 8552ea9 Compare June 4, 2025 06:49
@aaronfranke aaronfranke force-pushed the print-verbose-macro-caps branch from 8552ea9 to 1500ff5 Compare June 9, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants