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

Add increment and decrement operators to GDScript (++/--) #2869

Closed
MaaaxiKing opened this issue Jun 15, 2021 · 26 comments
Closed

Add increment and decrement operators to GDScript (++/--) #2869

MaaaxiKing opened this issue Jun 15, 2021 · 26 comments

Comments

@MaaaxiKing
Copy link

Describe the project you are working on

Any

Describe the problem or limitation you are having in your project

I need more characters to increment a number by 1 than in other languages

Describe the feature / enhancement and how it helps to overcome the problem or limitation

You could in-/decrement a number (that means also floats, which is unfortunately not supported in Java or C# and I'm sure many others) more easily

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Just like in Java or C# or whatever language: ++/--

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, it's just a super tiny thing to implement

@YuriSizov
Copy link
Contributor

Yes, it's just a super tiny thing to implement

Not really. This syntax creates a lot of ambiguity for the parser, which was the stated reason by @vnen why it doesn't exist in GDScript. You can already do the same with x += 1, which is not that more complex.

@Calinou
Copy link
Member

Calinou commented Jun 15, 2021

This was discussed a long time ago in godotengine/godot#1107 and was rejected.

My argument is that i++ does not improve readability over using i += 1. In fact, some JavaScript linters go as far as forbidding the use of i++ to improve readability (despite the language having such a feature).

If you really want to use the lowest possible amount of keystrokes to increment a varible, use i+=1. However, this won't follow the GDScript style guide (but a code formatter could help you fix this automatically).

@MaaaxiKing
Copy link
Author

I'd neever use "i+=1" xD; a lack of whitespaces is a no-go! I'm a bit disappointed about the rejection but not really having a problem with it since I'm starting to switch to C# anyways—do you recommend this when having good experience in both languages?—just wanted to improve the language a bit. A difficulty with implementing is unforeseen for me, but OK, I can't change that.
I feel the linter to be grumping too much, they're about equally good.

@dalexeev
Copy link
Member

i++ is like goto in the expression world, only much worse.

  1. Unlike other operators, increment and decrement work not on values, but on variables.
  2. Increment and decrement violate the order in which the expression is evaluated.

We should not blindly copy such "convenient" shortcuts. It is better to type 2 clear lines than 1 confusing one.

i += 1 is

  1. short enough (no duplication of information as in case i = i + 1);
  2. clear enough (increase i by 1);
  3. versatile enough (you can write i += 2 or i += x).

@MaaaxiKing
Copy link
Author

Normally, it is not y = i++ + 1; but y = ++i + 1;!

@YuriSizov
Copy link
Contributor

Those two variants do different things and also demonstrate perfectly ambiguity for the parser.

What's the order of a + + + b?

@MaaaxiKing
Copy link
Author

MaaaxiKing commented Jun 16, 2021

I thought, + +—and therefore also + + +—doesn't even work, does it?

@YuriSizov
Copy link
Contributor

YuriSizov commented Jun 16, 2021

That was not the point, I added spaces for demonstration purposes. You can write it as a+++b if you want.

@MaaaxiKing
Copy link
Author

MaaaxiKing commented Jun 16, 2021

With a = 1, b = 6 and c = a+++b, c should be (a)+(++b) and that's (1) + (1 + 6), which is 8. a and also b should stay the same.
In C# and Java however, c is 7 (bad), a is 2 (bad), b stays the same (fine), which is in total absolutely terrible because in an assignment, not two things should happen! Moreover, in both languages with spaces added between the plusses, a stays the same (fine), b stays the same (fine), c is 7 (bad). That's so weird!
Try out, if you want:
C#:

using System;
					
public class Program
{
	public static void Main()
	{
		int a = 1;
		int b = 6;
		int c = a+++b;
		Console.WriteLine("a: " + a + " b: " + b + " c: " + c);
	}
}

Java:

public class MyClass {
    public static void main(String args[]) {
		int a = 1;
		int b = 6;
		int c = a+++b;
		System.out.println("a: " + a + " b: " + b + " c: " + c);
	}
}

@YuriSizov
Copy link
Contributor

And so you've seen why it is a terrible operator 🙃

The results are expected though, it is treated as a++ + b, which means "use a as it is with b, then increment a". So c = 7, a = 2 is definitely correct. But it's not obvious from the written code, is it?

@MaaaxiKing
Copy link
Author

I don't find it is a bad operator, it's just horribly implemented. I can now comprehend the rejection. I know that the result is expected but logically, it's the worst thing I've ever seen in programming because a two-in-one assignment is downright silly!
I'll close this now, but can I get an opinion about switching to C#? I'd never use int c = a+++b anyways.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

Well, I code in C++ as much as in GDScript, and I treat GDScript as a lightweight version of C++ (however weird that may sound). As a C++ developer, I'd find increment/decrement operators to be natural in GDScript. When I can't use increment/decrement operators in GDScript when switching from C++ (or pasting code from C++ and adapting the algorithm to make it work in GDScript), it makes me quite nervous. 😛

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

I'm reopening this proposal because #2894 popped up as well, and to avoid duplicate proposals in the future. @vnen is the only one in authority who can formally close this proposal to reflect his final decision, not moderators, sorry. Closing proposals decrease the chances of someone to chime into the discussion as well, and we'd like to avoid that.

To clarify, even if godotengine/godot#1107 was raised some time ago, it was a long time ago. We still don't see the current @vnen's opinion on the feature, and it would be better to express the final decision in the new GIP process (ideally with rationale), so people are instantly aware of this decision. It's the responsibility of the maintainer. Especially in the time when GDScript 2.0 is currently being worked on, where these kind of proposals could be reconsidered, or alternatives given.

If you ask me, it's not like the feature is going to be approved anyways I guess, but it's also important to be legitimate in the decision-making process (approval/disapproval), and not to make self-opinionated decisions (#2894 (comment)) as if they were made by maintainers.

@Xrayez Xrayez reopened this Jun 24, 2021
@Xrayez Xrayez removed the archived label Jun 24, 2021
@Joakker
Copy link

Joakker commented Jun 24, 2021

Why not just have it be a statement? Go does increment like that

var e = 0

func incE1():
    e++ # Valid

func incE2():
    print(e++) # Invalid, expect expression

You could make it a prefix so it's easier to distinguish

func incE3():
    ++e # Same as incE1

@YuriSizov
Copy link
Contributor

@Joakker That may solve the ambiguity for the reader, yes. Not sure if it's easier to implement or not.

That said, people coming from other languages would expect your second example to work as well, so we may still have unsatisfied users asking for this to be possible. This also limits this syntax a lot, making it even less useful when you can already do the +=1 thing.

@dalexeev
Copy link
Member

Seriously? Complicate the GDScript parser just to save 1-3 characters? In the next step there will be people who will ask to make the increment/decrement behavior complete, the same as in C, C++, JavaScript, PHP, etc.

@vnen
Copy link
Member

vnen commented Jun 24, 2021

Why not just have it be a statement? Go does increment like that

The main point of it is to use in an expression. If you only use in a statement, there's not much difference from x += 1.

Also, the main point of this operator in the C-family languages is to use in a for loop (and similar constructs). In GDScript you do a for loop without having to increment it manually so it's not as important (the same happens in Python). I know GDScript is very C-like because its dependency in Godot, but I don't see this as enough reason to add everything that C has.

I would like to hear other arguments besides "it saves me a few characters" and "I'm used to this in other languages", since I don't think those are enough reasons to add one more complication to the language (not to mention it open the precedent for more complex additions for the same reasons). I think it hurts readability and add confusion. When someone says "people already know this from C-family languages", I feel they forget this trips a lot of beginners in those languages. So for all this I'm still against adding this.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

Also, the main point of this operator in the C-family languages is to use in a for loop (and similar constructs). In GDScript you do a for loop without having to increment it manually so it's not as important (the same happens in Python).

This may be connected to use case in #2727 (see my comment below).

I would much appreciate the ability to use for loop just like in C-family languages, because it avoids the need to use while constructs and needlessly initializing temporary local variables for that which pollute the scope. I find it quite unintuitive to use range for inverse loops (where i-- would work better in my opinion), it's also quite costly because it would allocate an array of temporary values (the reason why for i in <number> construct was added), but it doesn't work inverse, nor allows to initialize variables to values other than 0, so it's not a complete solution.

The problem with while is that the initialization takes before loop, and I have to iterate dx, dy variables way below the loop body, and that lead me to errors in the past because of incorrectly indented statements. I also have to make sure to reset dy = 0 on each row.

I cannot use range() here because the step is not based on integers, but actual chunk size. I could compute the number of x/y chunks and iterate over that, but that really makes C++ and GDScript even more inconsistent, and performance is quite important in my case.

That said, how about restricting i++/i-- operators to for loops? Would that make it difficult to implement? Having i += 1 and i -= 1 work inside loops would also be sufficient if you insist on not adding i++/i-- operators:

(A)

for i = 10; i > 0; i -= 1:
	for j = 0; j < 10; j += 1:
		print(i, j)

As far as I know, GDScript allows to use ; to separate expressions like that. A combination of if/while keywords may also work to make this more readable.

That said, the proposed syntax is not that more readable than existing:
(B)

for i in 10:
	for j in 10:
		print(i, j)

But the (A) snippet would give much more flexibility in comparison

I think it hurts readability and add confusion.

but please don't remove ability as seen in (B), just because it's less readable than using range().

I understand that this won't change anything but it would be wrong from my part not to mention this...

@MaaaxiKing
Copy link
Author

Restrictions is not a good idea, in my view 😐.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

Restrictions is not a good idea, in my view 😐.

Restrictions are already part of GDScript btw. If you try to use if x = something (where == operator is usually expected), this will throw an error, unlike in C-family languages. I'm not saying that we should allow that, of course, but restriction are made specifically so that you can avoid those common human errors.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

I know GDScript is very C-like because its dependency in Godot, but I don't see this as enough reason to add everything that C has.

This could be a very strong reason if the initial design of GDScript was made to be compatible with the language Godot was written in (C++), and perhaps that was reduz philosophy as well. But obviously, the design decisions change with the maintainer (AFAIK, reduz no longer works on GDScript anymore), and the GDScript's original design may no longer reflect the needs of community. 🙂

@vnen
Copy link
Member

vnen commented Jun 24, 2021

it's also quite costly because it would allocate an array of temporary values (the reason why for i in <number> construct was added), but it doesn't work inverse, nor allows to initialize variables to values other than 0, so it's not a complete solution.

range() does not allocate an array when used in a for loop, as long as the values are constant. You can initialize to a value other than 0 since the function has three uses: range(N) for 0 to N-1, range(initial, final) for initial to final-1 or range(initial, final, increment). So you can set something other than 0 as the initial value.

But it seems this discussion has shifted from increment operator to a C-style for loop. The example code wouldn't benefit of the operators themselves since it's not incrementing by 1. This would need a different proposal. And I would argue the C-style loop wouldn't add a lot to readability, if anything.

For instance, this:

for i = 10; i > 0; i -= 1:
	for j = 0; j < 10; j += 1:
		print(i, j)

Looks to me like a lot of noise. Compare to the current way of doing this:

for i in range(10, 0, -1):
	for j in range(0, 10):
		print(i, j)

Which is much more straightforward to decode the loop information.

@MaaaxiKing
Copy link
Author

I also find the easyness of the current for loops super.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 24, 2021

range() does not allocate an array when used in a for loop, as long as the values are constant

In most cases that I had to deal with, for loop values are not constant, unless I misinterpret the meaning of a constant value in GDScript. If not, this should be surely documented, if not already.

But it seems this discussion has shifted from increment operator to a C-style for loop.

As far as I know, we treat problems as first-class citizen when considering implementing something in Godot, so to speak. You said that:

Also, the main point of this operator in the C-family languages is to use in a for loop (and similar constructs).

If the main point of increment/decrement operators is to solve those use cases in C-family languages, this could also be the case for GDScript. But since GDScript seems to solve this particular problem another way, and other syntaxes are not up for discussion, then yeah it does not make sense to have increment/decrement operators in GDScript, as you said. So what I've said is also on topic.

That said, my main argument against using range() is performance reasons (and even memory usage, if we deal with large arrays). If it still allocates an array for non-constant values, it's an issue to adoption for me, which brings us to the point of using while loops which are not so convenient to use compared to for loops, but at least using while loops are more performant, but as I said, using while may be error-prone in contrast.

@Teashrock

This comment has been minimized.

@Calinou Calinou changed the title Add increment/decrement operator to GDScript Add increment and decrement operators to GDScript (++/--) Sep 13, 2021
@akien-mga
Copy link
Member

akien-mga commented Jun 16, 2022

There seems to still be no consensus on this proposal and no compelling use case to sway the opinion towards implementing this.

As GDScript maintainer @vnen made it clear that it didn't support the feature (at least with the current arguments given), I'm personally also not in favor of this proposal which has been rejected several times in the past, and other maintainers who voiced their opinion don't seem to support it either. So closing, we do not plan to implement these operators.

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

9 participants