decimals() always return 4 when there are decimals

[ghost]

The aim of this function is well to count the number of decimals after the point ?

print(decimals(5)) # 0 ok
print(decimals(5.1)) # 4 ?
print(decimals(5.12)) # 4 ?
print(decimals(5.123)) # 4 maybe
print(decimals(5.1234)) # 4 ok
print(decimals(5.12345)) # 4 ?

[shackra]

and what's the use case?

[ghost]

For convert a Rect2 with a negative size in a equivalent Rect2 with a positive size. The pos/end of the second is shifted of the size's number of decimals (pow(10, -number)).
At least i have not found another way...

[Fiona]

This is a hacky work around but does work.

num_decimals = str(number).split(".")[1].length()

or

func decimal_length(number):
    number = str(number).split(".")
    if number.size() == 1:
        return 0
    else:
        return number[1].length()

[shackra]

I'm a joke with mathematics too, but you can achieve that easily multiplying the negative number by -1.0, the result will give you exactly the same number but positive.

El 22 de mayo de 2015 01:20:45 CST, BBric [email protected] escribió:

For convert a Rect2 with a negative size in a equivalent Rect2 with a
positive size. The pos/end of the second is shifted of the size's
number of decimals.
At least i have not found another way...

---

Reply to this email directly or view it on GitHub:
#1955 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[ghost]

This works only with the size, but not with the position.
I have also used a String:

max(0, str(abs(number - int(number))).length() - 2)

It is not excessively complex of course but if this function exists it should work.

[akien-mga]

Please try to be a bit more explicit in your issue titles and in the description, I think none of the commenters understood that you were reporting a bug in the built-in decimals() function, we likely all assumed you were asking to add such a function (and the "4 ?" are therefore hard to understand, but well).

In the end I tried myself and could understand that indeed, the decimals() built-in function always returns 4. I'll fix that.

[akien-mga]

This is a typical case of precision issue with floating point numbers. The problematic code is https://github.com/godotengine/godot/blob/897a1aa/core/math/math_funcs.cpp#L207-L220

When trying to analyse 5.133 for example, p_step will successively take the following values:

0.133 
1.33                                   
13.3                                  
133.000001

So it returns 4 (4 steps) instead of 3.

I've googled about this a bit, and it's a typical issue, the main solution given being to parse a string instead of trying to do math with floating points. I've tried to improve the algorithm but did not manage to reach something working so far.

[akien-mga]

The best I could come up with seems to work fine for numbers with less than 4 digits (the "max" in the function), but returns 0 for numbers with more than 4 digits:

diff --git a/core/math/math_funcs.cpp b/core/math/math_funcs.cpp
index 3c94ac5..927fadb 100644
--- a/core/math/math_funcs.cpp
+++ b/core/math/math_funcs.cpp
@@ -208,9 +208,10 @@ int Math::decimals(double p_step) {

        int max=4;
        int i=0;
-       while( (p_step - Math::floor(p_step)) != 0.0 && max) {
-               
+       double threshold=0.00005;
+       while (abs(p_step - Math::round(p_step)) > threshold && max) {
                p_step*=10.0;
+               threshold*=10.0;
                max--;
                i++;
        }

I've tried to make some test before the while to check if the p_step has more than 4 digits, but I did not manage to find something that works. @reduz Any clue?

[bojidar-bg]

Quick side question, what should decimals(1/3) return? Infinity? "4"? -1?
Until this question is answered by consensus, I guess the function should be removed, as this is undefined behaviour (plus, I don't see the point in it either).

[akien-mga]

I made a quick search on the repo and it looks like Math::decimals() is used in two places in the engine:

• godot/scene/gui/tree.cpp

  Line 1137 in 8420c24

  String valtext = String::num( p_item->cells[i].val, Math::decimals( p_item->cells[i].step ) );

  (also further down)

• godot/scene/gui/spin_box.cpp

  Line 42 in 2c45ee5

  String value = String::num(get_val(),Math::decimals(get_step()));

And the algorithm is somewhat reimplemented here:

godot/tools/editor/animation_editor.cpp

Lines 1191 to 1209 in 7d8f187

	while(!step_found) {
	static const int _multp[3]={1,2,5};
	for(int i=0;i<3;i++) {
	step = (_multp[i] * dec);
	if (step*scale/SC_ADJ> min) {
	step_found=true;
	break;
	}
	}
	if (step_found)
	break;
	dec*=10;
	decimals--;
	if (decimals<0)
	decimals=0;
	}

I don't know about the re-implementation, but we should check the two usages of Math::decimals() to see if they are broken currently, and if we could replace case in the case of dropping Math::decimals() completely.

[akien-mga]

So by looking at the above two usages of Math::decimals(), I guess it could be worth implementing using string manipulation, and then removed from the GDScript API as it's too confusing to be presented as a reliable method IMO. For the engine, as well as the input floats are well controlled, it should be ok (here it's mostly about knowing the number of decimals of a range's step for example in the spin_box.cpp example).