Always add decimal when printing float

[KoBeWi]

So that

var a: float = 1
print(a)

prints

1.0

Makes working with numbers less confusing. I remember someone mentioned it around godotengine/godot-proposals#1866

Closes godotengine/godot-proposals#7894

[Calinou]

Does this change apply automatically to floats within Vectors/Colors/Transforms/Matrices as well?

[KoBeWi]

Hmm, it doesn't. Changing it might be not as easy :I

[KoBeWi]

Ok, so I extended this to these types (in addition to floats): Vector2, Rect2, Vector3.
The original aim was to differentiate integer types from floats. As we don't have AABBi or Basisi I left these types untouched.

[dalexeev]

Also need to solve this problem:

print(5.0)                    # 5
print(5.0000001234)           # 5 <--
print("%.16f" % 5.0)          # 5.0000000000000000
print("%.16f" % 5.0000001234) # 5.0000001234000004

$ python3 -q
>>> print(5.0000001234)
5.0000001234

[KoBeWi]

The precision problem doesn't look related.

[dalexeev]

The precision problem doesn't look related.

The problem is not with the precision of the numbers themselves, but that print doesn't display the number as precisely as it could. This can be seen by the fact that everything works with the formatting operator.

[KoBeWi]

Ah, right. I don't know how to fix this tbh. I can't use is_equal_approx() in FLOAT_STRING, because it produces stuff like 1.00001.0.

Well maybe I could search for . inside the resulting String 🤔

[KoBeWi]

Ok changed it. Here's the FLOAT_STRING as it was originally:

#define FLOAT_STRING(f) (rtos(f) + (Math::floor(f) == f ? ".0" : ""))

The new one is slightly less efficient, but maybe it doesn't matter that much.

[akien-mga]

We approved the change in a PR review meeting today, though we were concerned about the performance cost of the FLOAT_STRING define (doing a costly calculation twice + a find()).

A better option might be to use String::num_real() instead of String::num(), which already takes care of adding a .0 where relevant. I'm not sure how num() and num_real() differ aside from that and whether there would be some stuff handled by the one and not the other.

[KoBeWi]

Rebased and reworked. I removed the macro and just changed usages of String::num_real() to use true as second argument (which makes the .0 always appear). I did it in the operators instead of stringify() method to avoid duplicating formats. As a result, some more classes are affected, e.g. Basis, which uses vectors.

print(1.0)
print(Vector2.ONE)
print(Rect2(Vector2.DOWN, Vector2.ONE))
print(Basis())

->

1.0
(1.0, 1.0)
[P: (0.0, 1.0), S: (1.0, 1.0)]
[X: (1.0, 0.0, 0.0), Y: (0.0, 1.0, 0.0), Z: (0.0, 0.0, 1.0)]

[KoBeWi]

So I just noticed that changing Variant::stringify() will affect generated docs (as seen in failed CI, PI and TAU lose their precision). Not sure if that's ok 🤔

[akien-mga]

This seems to break the generated Mono glue:

/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/Generated/GodotObjects/VisualScript.cs(125,84): error CS1503: Argument 1: cannot convert from 'double' to 'float' [/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj]
/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/Generated/GodotObjects/VisualScript.cs(125,89): error CS1503: Argument 2: cannot convert from 'double' to 'float' [/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj]
/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/Generated/GodotObjects/TextParagraph.cs(351,110): error CS1503: Argument 1: cannot convert from 'double' to 'float' [/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj]
/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/Generated/GodotObjects/TextParagraph.cs(351,115): error CS1503: Argument 2: cannot convert from 'double' to 'float' [/home/runner/work/godot/godot/modules/mono/glue/GodotSharp/GodotSharp/GodotSharp.csproj]

CC @godotengine/mono

[raulsntos]

@akien-mga It's likely because the bindings generator now generates code using literal 0.0 (which is a double in C#) instead of 0 (which is an int). An int can be implicitly converted to a float but a double cannot be implicitly converted to a float because of precision-loss so in methods that take a float it fails.

The generator uses operator String() to generate the constructor for the default value of variants:

godot/modules/mono/editor/bindings_generator.cpp

Lines 3188 to 3207 in 44516d1

	case Variant::PLANE: {
	Plane plane = p_val.operator Plane();
	r_iarg.default_argument = "new Plane(new Vector3" + plane.normal.operator String() + ", " + rtos(plane.d) + ")";
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::AABB: {
	AABB aabb = p_val.operator ::AABB();
	r_iarg.default_argument = "new AABB(new Vector3" + aabb.position.operator String() + ", new Vector3" + aabb.size.operator String() + ")";
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::RECT2: {
	Rect2 rect = p_val.operator Rect2();
	r_iarg.default_argument = "new Rect2(new Vector2" + rect.position.operator String() + ", new Vector2" + rect.size.operator String() + ")";
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::RECT2I: {
	Rect2i rect = p_val.operator Rect2i();
	r_iarg.default_argument = "new Rect2i(new Vector2i" + rect.position.operator String() + ", new Vector2i" + rect.size.operator String() + ")";
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;

godot/modules/mono/editor/bindings_generator.cpp

Lines 3251 to 3287 in 44516d1

	case Variant::TRANSFORM2D: {
	Transform2D transform = p_val.operator Transform2D();
	if (transform == Transform2D()) {
	r_iarg.default_argument = "Transform2D.Identity";
	} else {
	r_iarg.default_argument = "new Transform2D(new Vector2" + transform.columns[0].operator String() + ", new Vector2" + transform.columns[1].operator String() + ", new Vector2" + transform.columns[2].operator String() + ")";
	}
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::TRANSFORM3D: {
	Transform3D transform = p_val.operator Transform3D();
	if (transform == Transform3D()) {
	r_iarg.default_argument = "Transform3D.Identity";
	} else {
	Basis basis = transform.basis;
	r_iarg.default_argument = "new Transform3D(new Vector3" + basis.get_column(0).operator String() + ", new Vector3" + basis.get_column(1).operator String() + ", new Vector3" + basis.get_column(2).operator String() + ", new Vector3" + transform.origin.operator String() + ")";
	}
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::BASIS: {
	Basis basis = p_val.operator Basis();
	if (basis == Basis()) {
	r_iarg.default_argument = "Basis.Identity";
	} else {
	r_iarg.default_argument = "new Basis(new Vector3" + basis.get_column(0).operator String() + ", new Vector3" + basis.get_column(1).operator String() + ", new Vector3" + basis.get_column(2).operator String() + ")";
	}
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;
	case Variant::QUATERNION: {
	Quaternion quaternion = p_val.operator Quaternion();
	if (quaternion == Quaternion()) {
	r_iarg.default_argument = "Quaternion.Identity";
	} else {
	r_iarg.default_argument = "new Quaternion" + quaternion.operator String();
	}
	r_iarg.def_param_mode = ArgumentInterface::NULLABLE_VAL;
	} break;

For example, we use it to generate new Vector2(0, 0) but now, because of this PR, it generates new Vector2(0.0, 0.0) which won't work because the Vector2 constructor takes floats not doubles.

[aaronfranke]

For the Mono bindings generator, we'll probably need to manually generate each piece of the constructor.

[KoBeWi]

Thanks JSON 😬

[Calinou]

Tested locally, it works as expected with float but not Vector2 (tested on single-precision build).

func _ready() -> void:
	print(-0.0 * Vector2.ONE)
	print(-0.0000000001 * Vector2.ONE)
	print(-1.0 * Vector2.ONE)
	print(-1.0000000001 * Vector2.ONE)
	print(0.0 * Vector2.ONE)
	print(0.0000000001 * Vector2.ONE)
	print(1.0 * Vector2.ONE)
	print(1.0000000001 * Vector2.ONE)

Results in:

(0.0, 0.0)
(-0, -0)
(-1.0, -1.0)
(-1.0, -1.0)
(0.0, 0.0)
(0, 0)
(1.0, 1.0)
(1.0, 1.0)

In master, this is the output:

(0, 0)
(-0, -0)
(-1, -1)
(-1, -1)
(0, 0)
(0, 0)
(1, 1)
(1, 1)

Notice the 0 and -0 which shouldn't be here, as Vector2 is always floating-point.

The same occurs with Vector3.

While this issue is not introduced by this PR, I think people will have more confidence in the displayed type if we merge this PR. This means that we need to get the displayed type right, as showing a misleading type will be a lot more problematic once this is merged.

[KoBeWi]

Fixed. String::num() was removing all trailing zeroes.

[Calinou]

Tested locally, it works as expected with vector types now:

(0.0, 0.0)
(-0.0, -0.0)
(-1.0, -1.0)
(-1.0, -1.0)
(0.0, 0.0)
(0.0, 0.0)
(1.0, 1.0)
(1.0, 1.0)

It still says -0.0 on the second line, but this is technically more correct given the number is being rounded from a very low negative value. (This rounding should be addressed in a separate PR.)

The PR also affects the inspector correctly as well now:

[m4rr5]

Since this PR ended up improving the output of String::num() I noticed that this same code also reports back if a float/double is "nan" or "inf". For "inf" it also checks, using signbit(), if it's "+inf" or "-inf". I was wondering if it makes sense to also do this for zero values and nan values. I don't think this is just a theoretical improvement either. Let me give an example I bumped into recently. When you calculate the "arc tangent" of a number, an often used way to do that is using some implementation (see for example https://en.cppreference.com/w/cpp/numeric/math/atan2) of atan2(y, x). There are edge cases where this function will return a different value based on if one of the inputs was +0 or -0, therefore I think it makes sense if printing such a value would show the sign bit correctly. I'm not saying this should be part of this PR since it addresses a different issue, it's just that me reading this triggered the thought.

[aaronfranke]

This PR breaks line numbers in the script editor. Line numbers are integers, they should not have .0 at the end.

I tested this on GodSVG, a project that heavily relies on float stringification, and here are the results:

The resulting SVG text looks quite different, but it is actually completely valid, just not the most minimal. This is a good sign that even in UI-heavy applications, this PR will not break things. GodSVG already has code to minify floats, which is why 0.0 becomes .0, because it expects any number with . is not a whole number. The float minifier will only require small tweaks to account for the trailing .0. But anyway, the project still works correctly exactly as-is.

[KoBeWi]

This PR breaks line numbers in the script editor. Line numbers are integers, they should not have .0 at the end.

I changed String::num() to add trailing zero, because it takes double as argument. Seems like CodeEdit uses this method.

EDIT:
Pushed a fix.

[aaronfranke]

I tested this on The Mirror and the app still works fine, and in fact it highlighted a pre-existing bug which I've fixed here the-mirror-gdp/the-mirror#115

I am clicking approve because I support this change, the code looks good from a quick review, and I have tested it on 2 large projects. However, I am approving it on the condition that this is merged immediately after the release of Godot 4.3, so that it can get lots of testing in master before being included in Godot 4.4, since this PR changes a fundamental part of the engine and is inherently risky it needs testing. I also removed the cherrypick:3.x label for the same reason.