Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate integer and real division operators in GDScript #1866

Closed
dalexeev opened this issue Nov 20, 2020 · 110 comments
Closed

Separate integer and real division operators in GDScript #1866

dalexeev opened this issue Nov 20, 2020 · 110 comments

Comments

@dalexeev
Copy link
Member

dalexeev commented Nov 20, 2020

Describe the project you are working on:
A game

Describe the problem or limitation you are having in your project:

print(5 / 2)   # 2
print(5.0 / 2) # 2.5

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
I'm not a Python fanatic, but it seems to me that in this case GDScript should be consistent with it:

$ python3 -q
>>> 5 / 2
2.5
>>> 5 // 2
2

Arguments for:

  1. In many scripting languages 5 / 2 == 2.5 (JavaScript, PHP, etc.). GDScript is a scripting language.
  2. Those who come from Python will find it easier. GDScript is quite similar to Python. Anyway, many people perceive it that way.
  3. Do you enter 5.0/2 or 5/2 in your calculator?
  4. If 5 / 2 == 2, then 2 * 2 == 5.
  5. GDScript has no // comments.
  6. Since there are a lot of big changes coming in 4.0, I think this is a good chance to fix things.

Main argument: current behavior is bug prone.

If you have an x / y expression, you cannot tell which division will be performed: real or integer. To guarantee real you have to do float(x) / y. To guarantee integer, you have to do int(x / y). See comment.

Python once faced the same problem because is also a dynamically typed language. Version 3 has fixed this issue. See PEP 238.

See also these comments: one, two, three.

In addition, this change does not break compatibility as much as it might seem at first glance. See comment for details.


Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
See above.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
No.

Is there a reason why this should be core and not an add-on in the asset library?:
Yes.

@YuriSizov
Copy link
Contributor

Confusion can be taught out of a person as they get more familiar with the tools they use. On the other hand scripting languages oft-times have a very inconsistent type coercion to achieve that "newb-friendliness" with varying degrees of success, mostly the type of success that backfires a lot. GDScript is clear of that for the most part, so I'd prefer it stays this way.

Also, Python is not the only language that people come from, so I don't see why that would matter.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 20, 2020 via email

@YuriSizov
Copy link
Contributor

The problem is that one / is acting as two different division operators.

It is not though. It just doesn't coerce the type of the return value unless you give it a clear indication that you need to do that. It's as straightforward as it gets, but maybe not obvious to a person that doesn't understand data types yet.

In other (probably even most) other languages, the / operator behaves differently, regardless of whether it has // (or div) operator.

In dynamic languages, maybe. And granted, we have a somewhat dynamic language in GDScript here. But that doesn't mean we have to go into magical behavior to mirror classic maths. I'd prefer we stick to the rules of more strict languages so we have less chances to shoot ourselves in a foot.

Also, shaders would still require you to keep this nuance in mind as they have stricter types.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 21, 2020

But that doesn't mean we have to go into magical behavior to mirror classic maths.

<int|float> / <int|float> == <real division>
<int> // <int> == <int division>

If I need mathematical division, I always use / (regardless of the types of the operands). If I need integer division, I use //.

I don't see anything "magical". This works well in most dynamic languages and static languages too (for example, in Pascal).

Also, shaders would still require you to keep this nuance in mind as they have stricter types.

Shaders are irrelevant to this issue. They are written in a completely different language, not GDScript.

@mrjustaguy
Copy link

I don't see a point in adding // as you can very easily force a specific type onto them.. need result to be int? just do int(Arguments) and it'll spit an int

@dalexeev
Copy link
Member Author

dalexeev commented Nov 21, 2020

<float> / <float> == <real division>
<float> / <int> == <real division>
<int> / <float> == <real division>
<int> / <int> == <int division>         # Aaaaaaaaa!

# And you have to do:
float(<int>) / <int> == <real division>

# ----------------- vs -----------------

<float> / <float> == <real division>
<float> / <int> == <real division>
<int> / <float> == <real division>
<int> / <int> == <real division>

<int> // <int> == <int division>

Although I know about this trait in GDScript, I sometimes forget about it and when debugging I am surprised that the following code does not work:

ceil(INT_EXPR / 2)

# And this is correct:
ceil(INT_EXPR / 2.0)

It is better to explicitly use another operator when you need exactly integer division than to save on one character.

@mrjustaguy
Copy link

I still see no point in adding // sorry.
It would only add confusion for existing users, break some existing code further (possibly even hard to diagnose the issues that come with this change as most changes to GDScript will cause the parser to send errors rather then changing the output afaik), and it can be easily bypassed, as in it can be worked around with a handful of characters every time there is such a case.

@dalexeev
Copy link
Member Author

break some existing code

There is nothing you can do about it, as it often happens with major updates. If this is the only argument against it, then it doesn't sound convincing. Backward compatibility is not only good but also evil.

@mrjustaguy
Copy link

read up, it isn't the only argument against it. it is one of the arguments but it isn't the only one and not the main one.
arguments are as follow:

  1. Adding pointless confusion
  2. Break Compatibility
  3. Easy to workaround

@dalexeev
Copy link
Member Author

  1. On the contrary, the inconsistent behavior of the / operator is eliminated.
    Even knowing this, it is very easy to forget that 1 out of 4 cases the / operator behaves differently.
  2. Already answered.
  3. This is called syntax fighting.

The discussion is looping, let's wait until more than 3 people take part in it.

@Calinou
Copy link
Member

Calinou commented Nov 21, 2020

I think the current GDScript division behavior is fine as it is. I wouldn't break compatibility for something that is not guaranteed to be a win in all situations.

@bluenote10
Copy link

bluenote10 commented Nov 21, 2020

Python deemed this important enough to break it going from Python 2 to 3.

Having lived through exactly this transition I have to say it was absolutely worth it. Our code is clearer now in expressing the underlying intention and we face considerably less division based bugs.

Since GDScript is heavily Python looking, it is confusing that it departs from it in that regard. I wasn't aware of it and assumed Python semantics so far.

@YuriSizov
Copy link
Contributor

Since GDScript is heavily Python looking, it is confusing that it departs from it in that regard. I wasn't aware of it and assumed Python semantics so far.

GDScript has nothing to do with Python besides being indentation-based as well. It's a misconception to think otherwise. Please, don't base your arguments on this, it's unrelated.

@rainlizard
Copy link

Coming from GameMaker it briefly confused me too. Someone told me "most languages assume integers unless told otherwise" which I thought was a fine explanation. The argument being used in this thread should simply be that of standards.

If we changed it, then instead of us it'd just be the other side getting the unexpected behaviour. Someone has to suffer.

@dalexeev
Copy link
Member Author

Once again, I don't want GDScript to be more Python-like. The problem is that in most languages 5 / 2 == 2.5. Is there an important reason why we should go against the majority?

We can just copy this one piece from Python. In all other aspects, we can still think differently.

@YuriSizov
Copy link
Contributor

The problem is that in most languages 5 / 2 == 2.5. Is there an important reason why we should go against the majority?

Where do you get this idea? This is not true for the majority, just for a specific group of languages.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 21, 2020

Where do you get this idea?

This is true for most scripting languages (JavaScript, PHP, Perl, Python, Lua, Dart, etc.). And GDScript is definitely a scripting language.

GDScript has nothing to do with Python besides being indentation-based as well.

  1. Indentation-based, pass, most keywords
  2. and, or, in, % (formatting operator), etc
  3. print(), str(), len(), range(), etc

Yes, GDScript is not Python. But these languages are very similar. So it's no surprise that people expect these languages to work the same way with simple things like arithmetic operators.

@bluenote10
Copy link

bluenote10 commented Nov 22, 2020

GDScript has nothing to do with Python besides being indentation-based as well. It's a misconception to think otherwise. Please, don't base your arguments on this, it's unrelated.

It does not matter what GDScript wants to be. Python is the highest ranked scripting language on the TIOBE index and currently on place 2 overall. This means that there is a majority of developers familiar with (and naturally assuming) Python semantics. So it is not just a matter of syntactic resemblance. Departing from the semantics of the most widely used scripting language offers a bug potential for all developers with such a background. It would be unwise to ignore this argument entirely.

@mrjustaguy
Copy link

mrjustaguy commented Nov 22, 2020

Someone told me "most languages assume integers unless told otherwise"

Int/Int also naturally assumes the output is desired to be Int
Godot assumes same type as both of the operands (if both are the same type) are unless told otherwise, and that makes a ton of sense. It's as simple as that.
Thing is, that is a design decision, and having division force between real and int division instead of it deciding by the situation based on how it's programmed..
also here's an example of something that'd be broken with the force inbetween and would be way harder to work around:

4 vars: a,b,c,d
a is int 5
b is float 3
c is int 4

some function f1 does this and prints result
d=a/b
Output -> 1.66 (float)

somewhere later in the code in some function f2
b=c --> b is now int 4
call function f1
Output -> 1 (int)

This wouldn't be possible if / would force real division and // int division as the output would be float or int in both scenarios and this type of change where some variables aren't always the same type and expect to be treated differently is quite useful in quite a few scenarios.. True it's harder to keep track of constantly changing types, but working that around would make it much worse and would probably lead to plenty of Copied code left and right for every such case... meanwhile the workaround for your issues are simpler.. just turn one or both into a float if you want real division.

@mrjustaguy
Copy link

mrjustaguy commented Nov 22, 2020

Also if you don't want to do float() you can always go var1 as float/var2 instead of float(var1)/var2 if you want more readable code that is..

@dalexeev
Copy link
Member Author

dalexeev commented Nov 22, 2020

Int/Int also naturally assumes the output is desired to be Int

naturally

This is not the case in mathematics:

3 - 5 = -2       -:(ℕ,ℕ) → ℤ
3 / 2 = 1.5      /:(ℤ,ℤ) → ℚ
2 ** 0.5 = √2   **:(ℚ,ℚ) → ℝ

The situation is a bit similar with +, which combines the operators of addition and string concatenation. However, unlike /, it at least does not allow mixing a number with a string, or a string with a number. If, according to your logic, <int> / <int> == <int> and <float> / <float> == <float>, what should be <int> / <float> and <float> / <int>? So this argument doesn't count.

And I still do not understand what inconvenience (except for breaking compatibility) creates the separation of / into / and //. The behavior becomes consistent and now only depends on intent of programmer, and not on a subtle difference in the type of operands.

For example, Pascal is a statically typed language. However, it has <integer> / <integer> = <real>.
And <integer> div <integer> = <integer>. What is the problem here?

No need to blindly copy the behavior of C/C++, Java, etc., especially since they are not scripting languages.

@mrjustaguy
Copy link

mrjustaguy commented Nov 22, 2020

Just read my example above. seperating would cause that use case to require quite a bit more code which isn't worth the small benefit of a seperation when that can be changed with a simple float() or with var as float.

on the int/float and float/int you misunderstood me, and i've clarified it up in an edit.. I ment if both operands were same type not different types.. in mixed types it depends and goes like vector>float, float>int and the like.

@dalexeev
Copy link
Member Author

Nothing is clear from your example, sorry.
But I'm pretty sure the change never increases the number of lines of code. Only their length, sometimes. For one character. 😃

@mrjustaguy
Copy link

It does. quite a bit. it isn't as simple as you think. the issue is you can't change between real division and int division at runtime with / and // the way you can with the current system, meaning you'd have to add lines of code to check if it's real division or int division that you are expecting, and that is in a SIMPLE setup... a more complex setup would require many more lines of code to deal with it.. magnitudes more in fact..

@dalexeev
Copy link
Member Author

you can't change between real division and int division at runtime

Explain what you want. This is a very strange formulation of the question. Give an example of a real-world problem that would require you to use the following function:

func old_div(a, b):
    if a is int and b is int:
        return a // b
    else:
        return a / b

@YuriSizov
Copy link
Contributor

It does not matter what GDScript wants to be. Python is the highest ranked scripting language on the TIOBE index and currently on place 2 overall. This means that there is a majority of developers familiar with (and naturally assuming) Python semantics. So it is not just a matter of syntactic resemblance. Departing from the semantics of the most widely used scripting language offers a bug potential for all developers with such a background. It would be unwise to ignore this argument entirely.

This is still a pointless comparison. Python is a general purpose language and pretty irrelevant to the gamedev domain. GDScript is purpose built language that is bound by that domain. Int math is more relevant in gamedev than in general programming. And it is faster than any proposed alternative when you actually rely on it.

And as I've mentioned before, people will still face that same behavior in our shader language. It doesn't matter where they will learn the importance of it, they inevitably will.

This is the case where familiarity is not that important. If anything, familiarity within the domain will be quite different.

@mrjustaguy
Copy link

you can't change between real division and int division at runtime

Explain what you want. This is a very strange formulation of the question. Give an example of a real-world problem that would require you to use the following function:

func old_div(a, b):
    if a is int and b is int:
        return a // b
    else:
        return a / b

This proves my point.. also you cherrypicked the quote, as the continuation explains and doesn't say this is impossible.

This is 5 lines of code, + turning every / into a function, adding another 4-5 chars min per / not to mention the fact that the proposal can be worked around with only as few as 7 total chars per needed case (2 if it's a hard coded constant like in your examples above where you turn a 2 into 2.0 in a line of script instead of it being a var that is 2)

I totally agree with #1866 (comment)
There is no point in breaking compatibility for something that isn't a win for most situations.
It would be worth Breaking compatibility if it was something impossible/hard to work around even if it isn't a win in all situations (but only if it doesn't make the losing situations end up being as hard or harder to work around in the aftermath) however this change isn't difficult to workaround and just complicates stuff needlessly for fixing compatibility, adds confusion to the existing userbase and for what? to make it more in line with some existing scripting languages? When changing Languages naturally you should look up the basics of the new language.. this is what can be considered a basic of this language.

@dalexeev
Copy link
Member Author

his is 5 lines of code, + turning every / into a function, adding another 4-5 chars min per / not to mention the fact that the proposal can be worked around with only as few as 7 total chars per needed case (2 if it's a hard coded constant like in your examples above where you turn a 2 into 2.0 in a line of script instead of it being a var that is 2)

Great calculations... You never gave an example of a case where you need this exotic behavior. In 99% of cases, either normal division or integer division is required, and not something incomprehensible, depending on the exact types of the operands.

No matter how you think about it, division and integer division are two different operations and should not be combined. Those who do not agree with this can combine the sewer with the water supply.

@mrjustaguy
Copy link

mrjustaguy commented Nov 22, 2020

No matter how you think about it, division and integer division are two different operations and should not be combined. Those who do not agree with this can combine the sewer with the water supply.

Division and Integer Division aren't 2 different operations, and thus don't need to be seperated.
It is like saying that there should be 2 different operations to handle 5+5, one as strings and one as Ints/Floats when you can do "5"+"5" to get string and just have 5+5 return Ints/Floats

Note: using Ints/Floats as if a type of var is forced to float, 5 is still a valid float value and thus 5+5 can be Floats if the Vars are of the Float Type.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 25, 2020

@Wykleph It is impossible to keep everything in your head. I, a few people here and a few in the titter sometimes forget about this behavior and get bugs. No wonder. Tell me, do you enter 5.0/2 or 5/2 in your smartphone's calculator?

(I won't fix the typo about twitter.)

@Feniks-Gaming
Copy link

if you need to adapt then the language isn't intuitive.

Imagine you come across this code sniped.

var hours_passed = min_passed/60

Can you seriously put a hand on a heart and swear to me that you are 100% certain that the intention of a person is to get int and can you be 100% certain that the intention of this code is for

var min_passed = 75
var hours_passed = min_passed/60

to return 1 not 1.25?

Under new system code is more easy to ready

var hours_passed = min_passed/60 # I know fellow coder wants only full hours passed.
var hours_passed = min_passed//60 # I know fellow coder wants hours and some left over passed. 

There is no ambiguity. Code is more intentional that way.

Any argument against this is mostly "we are used to it that way".

@akien-mga
Copy link
Member

I think many here are way too emotionally invested in this discussion, so I'll lock it for a few days so we can all take time to chill.

@godotengine godotengine locked as too heated and limited conversation to collaborators Nov 25, 2020
@reduz
Copy link
Member

reduz commented Nov 25, 2020

Sorry guys, I explained it as best as possible, so please try to understand it.

The fact we are doing things this way already (with little to no complaining), and that there is strong opposition to changing it from a significant share of the users and core contributors (lack of consensus) makes this a no-go for the time being. Discussion on technical merit or whether users would adapt is irrelevant at this point.

@Calinou
Copy link
Member

Calinou commented Dec 12, 2020

Closing due to lack of support – see the above discussion and reactions on OP.

@Calinou Calinou closed this as completed Dec 12, 2020
@godotengine godotengine unlocked this conversation Mar 7, 2021
@Calinou Calinou closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2023
@AlfishSoftware
Copy link

Sorry if this was suggested before (I didn't see it in this thread), but...

Why not just introducing 2 new division operators and keeping the current behavior on /?
Just an example:
a // b always integer (or floor) division (or, alternatively syntax could be something like a div b)
a ./ b always floating-point division, i.e. result is Float64 even if a and b are int
a / b look at types of a and b and decide (no changes to current behavior)

If you don't like this, just ignore the new syntax. Your code will still work.
If you don't like the current behavior, use only the new syntax.
If you have no preference, you get more tools so you can use whatever fits the intent better and make your code clearer.
Problem solved, everybody wins.

I suggested ./ because:

  • it's easy to type (no shift)
  • shouldn't clash with any existing syntax (I think)
    there's syntax like 1. for float, but that would just have same behavior regardless of token precedence: 1./d
    i.e: in 1. / d == 1 ./ d both operators will end up doing a FP division
  • the dot can be mentally associated with float/decimal point numbers
    or you can pretend that / is a method name and you access it with ./
  • you don't need to repeat the dot because if you use numbers like 5.0 then you can just use / instead, like 5.0/2
    this is more for cases like x./2
  • if you're in favor of using it exclusively, it's much cleaner than putting float(a)/b everywhere

@strutcode
Copy link

I just want to chime in with another voice toward this being a design flaw in GDScript that should be fixed. Let's take a moment to compare some real data using a smattering of languages:

C: 1 / 4 = 0
C#: 1 / 4 = 0
Go: 1 / 4 = 0
Java: 1 / 4 = 0
Javascript: 1 / 4 = 0.25
Perl: 1 / 4 = 0.25
PHP: 1 / 4 = 0.25
Python: 1 / 4 = 0.25
Lua: 1 / 4 = 0.25
Ruby: 1 / 4 = 0
Rust: 1 / 4 = 0
Scratch (yes, really): 1 / 4 = 0.25

My take away -- which matches my intuition regarding GDScript -- is that the common ground in handling of these expressions appears to be related to whether the target language has rigid typing or not. With the notable exception of Ruby, the more dynamic "scripting" type languages all seem to handle this the intuitive way because without a strong type system to tell you that you've done something wrong this behavior can be quite baffling.

Based on the intentions of GDScript conveyed through its syntax and structure it seems clear to me that it has more in common with the languages that return floats than the ones that return ints, and it seems obvious that this should be the default behavior in a future version of Godot.

It seems to me that in the rare case that the user actually wants integer division int(8 / 2) is a very simple workaround compared to the constant confusion and frustration caused by having to convert ints to floats in order to do simple math.

Consider one of the most basic practical cases that might be used in any game:

var hp = 50
var max_hp = 100

func get_hp_percent():
  return (hp / max_hp) * 100

Please explain to me why this should return 0.

@dalexeev
Copy link
Member Author

Due to lack of consensus, the decision was made to keep the 3.x behavior. This cannot be revised for backwards compatibility reasons, at least in 4.x. Godot 5.0 is not expected in the next few years, so resuming the discussion will not be productive.

This discussion is left unlocked only to allow users to add reactions on the comments. Please refrain from adding new comments here. Thanks for understanding!

@strutcode
Copy link

@dalexeev Apologies, my intention wasn't to revive a dead issue but I arrived here via a comment you yourself made on #5344 -- am I and future contributors to understand that discussion on this topic is simply not allowed at all? Is there a better place to gather support for this change? Either way thank you for responding and I will refrain from further discussion unless advised.

@scripturial
Copy link

Apologies for extending the discussion further. But can someone please explain to me why dividing an integer with an integer and assigning the result of an integer division into an integer should return a warning at all?

Consider this example:

var pairs:int = 0
pairs = int(len(q.fields) / 2)

Perhaps Godot 4 could be updated to not output the warning in the case where the result is explicitly declared to be a integer?

@AThousandShips
Copy link
Member

Repeating the above, please respect this:

This discussion is left unlocked only to allow users to add reactions on the comments. Please refrain from adding new comments here. Thanks for understanding!

Do not comment further here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests