Capitalize INF, -INF, and NAN in serializing

[aaronfranke]

• Implements Change inf, inf_neg, nan serialization to uppercase, and add INF_NEG GDScript constant godot-proposals#3398
• alternative to Recognize inf, nan, and inf_neg constants (note: lower case) godot-proposals#3390
• undoes Only allow finite numbers in Range.value. #81076
• fixed previously Float values NAN or INF are shown as 0 in the debugger and inspector #88006
• fixes Inspector doesn't allow entering values of NAN, INF, etc. for float fields #87664
• used to fix part of Make floating-point values in the inspector more permissive godot-proposals#10068
• supersedes String format inf, nan values to uppercase #53698
• related to Add toggle to Range for allowing non finite numbers #88354
• would be nice to merge after Allow using any-case constants in Expression #100395

As explained by bug report #88006, the editor inspector is unable to display non-finite values such as infinity and NaN. This is due to this check in Range (range.cpp) added by #81076:

If we remove this check, then non-finite values become allowed. They have to be typed as INF, -INF, or NAN like GDScript, and then get displayed as inf, -inf, or nan:

Inside of the .tscn file the values get serialized as inf, inf_neg, or nan:

So, it's clear that simply removing this check is not enough. We need to fix these problems:

• The text the user can type is not the same as the displayed text (vital for good UX).
• The constants in GDScript are not the same as the displayed text (ideal for good UX).
• The value serialized in the file is not the same as the displayed text in the case of -inf and inf_neg (less important, but we should still be consistent).

Allowing the user to type both inf and INF is just generally a good idea, and stands on its own, so I put this into a separate PR here: #100395. This fixes the first point, since users can type inf like they see inf.

However, I think there is value in fixing the other two points as well. Capitalizing the displayed and serialized text is pretty straightforward, it's just an arbitrary choice we make as engine developers, although we need to keep the code being able to read inf and such to keep reading old files (and if this is merged, we should backport a fix to older branches to allow INF etc to be read by older Godot versions).

As for inf_neg, the current reason for this is that VariantParser is not able to parse -INF as a single thing. It has this code in master which checks for - or a number as a start of a numeric literal:

if (cchar == '-' || (cchar >= '0' && cchar <= '9')) {
	StringBuffer<> num;
	if (cchar == '-') {
		num += '-';
		cchar = p_stream->get_char();
	}
	// and so on

However, if you look closely at the code, we can see that it's needlessly checking for '-' twice. A simple fix would be to rearrange this code slightly, so that it only needs to check for a leading '-' once:

StringBuffer<> num;
if (cchar == '-') {
	num += '-';
	cchar = p_stream->get_char();
}
if (cchar >= '0' && cchar <= '9') {
	// and so on

This way, we can handle reading -INF or really any negated identifier (-INF would be the only one at the moment, but I could imagine adding -PI or something, in any case making this code work for all cases has no downsides).

---

If desired, I can undo the change in range.cpp, un-undoing #81076, leaving #88354 to be the PR changing the exposed behavior of Range. However, as for the rest of this PR, I think it's a good improvement to have the serialization and display of INF, -INF, and NAN values match what users already expect from GDScript. Aside from capitalization, it's important that this PR fixes reading negated identifiers such as -inf and -INF, so we can be consistent and avoid inf_neg / INF_NEG.

[akien-mga]

There's really a lot to unpack here, and it's hard to have a good overview. I'm not against changing the serialization to uppercase identifiers and specifically -INF instead of inf_neg if that solves actual issues. But I'm not in favor of allowing all kinds of spellings just for the sake of it.

We should pick one convention and enforce it (and if we're changing it, then we do still need to support the previous convention as compatibility code - but that's only for compatibility's sake, not a design decision that we should support any possible variant the user may come up with).

I would suggest focusing on fixing bugs in priority, and then assess the more "cosmetic" changes. If we can fix parsing -inf and allow it in range, then it's probably a better change IMO than breaking compat by renaming them all to upper case for some seemingly arbitrary reason.

[aaronfranke]

I feel like this is a bit of a paradox, then. As you stated in #100395, the constants are supposed to be capitalized to match GDScript. But capitalizing the constants is a "cosmetic" change. So... it's non-cosmetic, yet cosmetic.

If we can fix parsing -inf and allow it in range

PR #100395 allows Expression to read -inf. Range uses Expression to read user input and turn it into a number.

I would suggest focusing on fixing bugs in priority, and then assess the more "cosmetic" changes.

I did both at once since this way we only need to handle negative infinity as -INF or inf_neg. If I separated the changes to make it -inf first, then we would have to handle 3 different names, 2 for compatibility.

[Mickeon]

In the meeting, it was brought up that it is critical for this PR to stay fully backwards compatible, because users would really be unhappy for us breaking their tscn files over this change.

In this PR, the relevant changes are done in core/variant/variant_parser.cpp. So this PR should in theory maintain compatibility already, not just for PackedScenes, but for other methods that convert strings into variants.

[Mickeon]

I won't hold you at gunpoint for this but there's actually a few more places in the @GlobalScope.xml that directly refer to these values, and they would need to be changed as well.

[Zehir]

Tested the PR and works as expected.

The value in JSON.from_native are now uppercase.

The value in the .tscn and in var_to_str are like before aka inf_neg for -INF
The tscn files does decode for both lowercase and upper case and -INF.

Looked at the code, seems OK for me.