Godot: Should floor return Int?

Created on 30 May 2018  路  27Comments  路  Source: godotengine/godot

I had a problem that perplexed me for a while. The code below never prints anything. Finally last night upon searching the docs I figured out my problem.

func Test(whichOne):
    match whichOne:
        0:
            print("zero")
        1:
            print("one")
        2:
            print("two")

func Main():
    Test(floor(3/1.5))


floor is returning a float disguised as an integer with no decimals.
In the Godot documentation it describes "floor" lIke this:
"Rounds 's' to the closest smaller integer and returns it."
If you search "floor in math" in Google, all descriptions say that floor returns the smallest Integer.

Anyway, do what you will. Just my thoughts. It may help someone else not have the same problem. I know in the docs it says "float floor(float s)", but it slipped through unnoticed for almost a month working on a game. :-)

discussion enhancement gdscript

Most helpful comment

I think most languages return floats to avoid overflowing stuff like floor(10^50). That's a moot point for precision, but still. That said, your particular issue may be a problem with match itself. The equivalent if-elif "switch" statement correctly prints two:

var a = floor(3/1.5)
if a == 0:
    print("zero")
elif a == 1:
    print("one")
elif a == 2:
    print("two")

I don't think match should care about the type, so I'd flag this as a bug.

All 27 comments

Well, in the same doc you can see that the return type of floor() is float, so it's not disguised. The description is correct, but it's "integer" in mathematical terms, not the type of the returned value.

Now, the return type could be an int (same for round() and ceil()), that's up for discussion.

You are right, I saw that the signature of the function was "float floor(float s)".

I said "disguised" because it returns a number with no decimals, like "1.0" Instead it return just "1". Kind of throws me off. Yes, it would be nice if round(), ceil(), floor() and those that return numbers without decimals be returned as an INT. Others like stepify() make sense to be returned as floats.
Thanks for considering it.

I think most languages return floats to avoid overflowing stuff like floor(10^50). That's a moot point for precision, but still. That said, your particular issue may be a problem with match itself. The equivalent if-elif "switch" statement correctly prints two:

var a = floor(3/1.5)
if a == 0:
    print("zero")
elif a == 1:
    print("one")
elif a == 2:
    print("two")

I don't think match should care about the type, so I'd flag this as a bug.

CC @karroffel

I don't agree that it's a bug, switching on integer keys and switching on float keys is a whole different story.

The pattern matcher has to do type checks before actually comparing anything because of GDScript's strong typing.

I could add in that every typeof(__PATTERN__) == typeof(__KEY__) actually checks for both int and float if it's one of those, but I think that adds complexity that's not really needed IMO.

To fix your exmple, you can do

func Test(whichOne):
    match whichOne:
        0, 0.0:
            print("zero")
        1, 1.0:
            print("one")
        2, 2.0:
            print("two")

or do match int(whichOne): instead. I think having floats separate from ints is useful, as floats are a bit trickier when it comes to equality.

Adding a note about difference of using float and int as keys in match clause to documentation will be useful I think.

I think it's good as is too, as long as we're sure that int(floor(3/1.5)) will end up as int(floor(2.0)) or int(floor(2.00000231)) and not int(floor(1.99999231)) which would give bad surprises.

floor shouldn't return int, because the range of integer values storeable in a float is larger than the range storeable in an int (though the precision of floats is bad at such ranges).

Also, I would just prevent people from using floats in a match statement.

floats could be good in a match statement if you need do check 2.50 or something like that. If it is not possible it would be nice if the debugger picked it up and threw an error

Honestly floats should never be directly equality testable unless very set predefined unambiguous values, such as NaN or so. Even testing against 0 is questionable as it might not be 0 in the bit form. If it were possible I'd say that equality comparisons between floats in gdscript should throw an error, only less/greater comparisons should be allowed (perhaps with a helper function to allow an 'equality' test via a defined epsilon). Consequently match's should throw an error if a float is passed in. This would be the safest course of action and would help to force that people treat floats properly.

Not using floats for equality is yet another argument for integer vectors btw :)

Also, I've seen Unity added FloorToInt(x). I wonder if it does anything more than (int)Floor(x), if it doesn't, there wouldn't even be a point for that another function.

Also, I've seen Unity added FloorToInt(x). I wonder if it does anything more than (int)Floor(x), if it doesn't, there wouldn't even be a point for that another function.

There's some assembly bit twiddling magic you can do that floor's to an int faster than a generic cast back in the old days, but I think some sseN or something like that has an instruction or so that can do it faster now, so if it does it via something like that then sure, but then again if you need that kind of performance then why are you in gdscript? ^.^

The discussion died down, but I think the conclusion is that we can't make floor return int due to potentially overflowing the size of an int as mentioned by @isaacremnant and @bojidar-bg, so closing.

I know this conversation is over but I'd ask that you'd reconsider. After the number 2^24, floats can no longer represent integers reliably. And using floats past 2^32 is very rarely done that I don't think that should be a concern (a.k.a. corner case). If anyone is actually using floats at that range I think they should just roll their own functions. That corner case can also be documented to let people know. For the rest of us, floor, round, ceil, etc. should return an int.

I checked Python 2 and 3 and it seems like Python 2's floor() returns float, while it was changed in Python 3 to return int. So I guess there might be something to consider.

Python 2.7.15 (default, May  1 2018, 17:08:05) 
[GCC 5.4.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import math
>>> type(3.05)
<type 'float'>
>>> math.floor(3.05)
3.0
>>> type(math.floor(3.05))
<type 'float'>
>>> math.floor(308912387912379897213.2)
3.089123879123799e+20
Python 3.5.3 (default, May 23 2018, 14:20:56) 
[GCC 5.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import math
>>> type(3.05)
<class 'float'>
>>> math.floor(3.05)
3
>>> type(math.floor(3.05))
<class 'int'>
>>> math.floor(308912387912379897213.2)
308912387912379924480

Python uses big integers instead of longs, so it can fit arbitrary-size ints.

Personally, I would expect a floor() function that accepts a float to return a float. Perhaps a floor() function that returns an int could be called floori()? Same with round() (roundi()) and ceil() (ceili()).

I agree with @LikeLakers2, functions shouldn't perform casts themselves. It's true that floor and ceil are more likely to be used in integer expressions, but what about sign? For consistency, it too should return an int, but you almost always use it in floating expressions.

I don't particularly mind either way, but I would much rather see casts in MY code than in the blackbox that is a math function.

When it comes to the example use case of a switch statement, I think GDScript should show a warning when trying to use floats as keys. Floating-point numbers can have precision issues and often aren't exactly equal, comparison usually requires using an epsilon value for approximate equality. I can't think of a situation where I ever wanted exact equality float matching with a switch statement.

It would make more sense to require the programmer to explicitly cast it to an int for the switch statement if that's what's desired, or specify approximate equality using functions like is_equal_approx.

Another idea, it may be useful to add a trunc method which returns an integer. Numerically, it behaves the same as floor for positive inputs, but not for negative ones. Its intended purpose is also specifically to get rid of the fractional part of the number.

In C++, trunc can be implemented as return (int)number;. There is also std::trunc which returns floats, but I don't think that I've ever wanted to have the result of a truncate operation be a float. Also, in Python, math.trunc() returns an integer while math.floor() returns a float (source).

@aaronfranke You can already make C-style casts to int, such as int(5.3) = 5 and int(-1.2) = -1.

I'm not sure if this is useful to note, but I found this incredibly strange that the documentation for floor and ceil show return type as float but say "return integral value" - I'm not sure if "integral" has some mathematical connotation that I am not aware of, but when I see "integral" I expect integer (could totally be my ignorance).

I think most developers will expect floor, ceil and round to return integers, but if you prefer to keep as float, I would recommend changing the text description to avoid the word "integral" and maybe showing an example with typecasting to int in the docs.

I'm not sure if "integral" has some mathematical connotation that I am not aware of, but when I see "integral" I expect integer (could totally be my ignorance).

While I understand the confusion, it's the correct term. A decimal number (e.g. 4.12) is composed of an integral part (4) and a fractional part (0.12). See e.g. https://www.tutorialspoint.com/c_standard_library/c_function_floor.htm
But yes, this could be clarified further if it's still unclear to users.

The description could also mention that if one wants an integer, int(x) should be used instead of floor(x).

Looking back over this issue, I don't really see a strong case for keeping these functions returning floats other than it's common to do in other programming languages.

we can't make floor return int due to potentially overflowing the size of an int

  1. Godot uses 64-bit ints and floats inside Variant, so the possibility of overflowing is very low (it would only occur in cases where the value is extremely large). If this does occur, then it would be easy to detect and throw a warning.

  2. 64-bit floats (doubles) can only reliably represent integers up to 2^53. If users know in advance that they are going to be using values larger than 2^53, then rounding, flooring, and ceiling become meaningless operations anyway.

We could always implement the opposite of @LikeLakers2's suggestion, and have roundf etc instead of roundi etc in case we need to keep float-returning versions.

My only worry about floor returning an int is that it goes against the common way this function is exposed (not everyone is going to look up the doc about its signature, since it's so common). And due to this, it can cause errors down the line due to being an integer. For example, if a division happens, it will do an integer division silently, and now you have a bug. (that may also break existing scripts in hard-to-find ways).

Also correct me if I'm wrong but in Python, division / always does decimal division, while // is used for integer division.

@Zylann Also correct me if I'm wrong but in Python, division / always does decimal division, while // is used for integer division.

// in Python does floor division, so for example, -2 / 3 is -1. Would be nice to have in GDScript for my own uses, though I understand if it's not common enough :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

blurymind picture blurymind  路  3Comments

RebelliousX picture RebelliousX  路  3Comments

RayKoopa picture RayKoopa  路  3Comments

testman42 picture testman42  路  3Comments

SleepProgger picture SleepProgger  路  3Comments