Change VariantUtility to prevent undef print_verbose

[lawnjelly]

Changes the VariantUtility function from print_verbose to _print_verbose, eliminating the need for undefining the print_verbose macro, which caused compilation problems.

Fixes #101966

Notes

• Although the problem was seen occurring in a SCU build, the problem was not directly related to SCU, but rather the fact that variant_utility.h undefined print_verbose.
• This meant that any code compiled after this include would fail to compile print_verbose.
• Feel free to suggest any variations on the macro name / func name, these were just a quick guess.

Discussion

VariantUtility contained a print_verbose function for binding (with gdscript etc) which conflicted with the global definition of print_verbose. This had been worked around by undefining print_verbose in these files, which was a little hacky, and had the side effect of breaking compilation in any later file that tried to use print_verbose.

The more sensible solution was to give the VariantUtility function a different name. @Calinou informed me that it was possible to bind a function with a different name exposed to gdscript than the c++ function, and this (hopefully) provides the simplest solution.

[Ivorforce]

Your implementation is perfectly fine.
As a proposed solution to the problem, it's acceptable, but I think the solution I'd prefer would be to change the print_verbose macro to a function:

template <typename... Args>
void print_verbose(const Variant &p_var, Args... p_args) {
	if (is_print_verbose_enabled()) {
		print_line(p_var, p_args...);
	}
}

I have no idea why the snippet was made a macro in the first place (premature optimization?), but we can see now that it being one has led to awkward workarounds.

[lawnjelly]

Yup a templated function was my first thought too, however I was suspecting the reason for using a define was to try and remove the work of the string concatenation:

For instance

print_verbose("blah blah" + "blah" + itos(a));

With a templated function, will it do the concatenation work, even though it will not be used further on? Or does the variable args remove this problem?

With the define, if the is_print_verbose_enabled() fails, it should avoid the string concatenation.

There is a note about this in the source:

// This version avoids processing the text to be printed until it actually has to be printed, saving some CPU usage.
#define print_verbose(m_text)             \
	{                                     \
		if (is_print_verbose_enabled()) { \
			print_line(m_text);           \
		}                                 \
	}

[Ivorforce]

Right. That makes sense, I suppose.
Then I would say the best solution would be a full project find and replace of print_verbose -> PRINT_VERBOSE. The reason macros are usually uppercase is precisely to avoid this kind of issue, where it interferes with normal function usage.

[lawnjelly]

I do agree on capitalization for macros, mind you that approach might be subject to more push back (as the change is less localized), and will require rebasing existing PRs that use print_verbose.

I wouldn't be surprised if this was historically a function and later changed to a macro for the string concat efficiency.

Probably better for a separate PR though, so preferred approach can be chosen in a meeting.

[Ivorforce]

I do agree on capitalization for macros, mind you that approach might be subject to more push back (as the change is less localized), and will require rebasing existing PRs that use print_verbose.

Agreed.
I see no reason to block this PR over there being another, possibly contentious solution. Your solution is self contained and unproblematic. The priority is fixing the bug, so I'm happy to approve.

I wouldn't be surprised if this was historically a function and later changed to a macro for the string concat efficiency.

I looked it up, that's correct!

Probably better for a separate PR though, so preferred approach can be chosen in a meeting.

Agreed!