-
-
Notifications
You must be signed in to change notification settings - Fork 97
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 guards to match patterns in GDScript #4775
Comments
I suppose this means no more matching multiple patterns in one |
No, multiple patterns would still work. Check the sample code right above the phrase you quoted. |
I was reffering to matching several cases in the var damage_data := {amount=50, fire_duration=1, fire_amount=10}
match damage_data:
{"amount": var amount, ..}:
# take raw damage
continue
{"fire_amount": var fire, "fire_duration": var duration, ..}:
# setup fire damage
continue |
Related to #160 (which is about the use of |
Ah, yes, this wouldn't work anymore. But is there any reason to use var damage_data := {amount=50, fire_duration=1, fire_amount=10}
match damage_data:
{"amount": var amount, ..}:
# take raw damage
match damage_data:
{"fire_amount": var fire, "fire_duration": var duration, ..}:
# setup fire damage Unless you have very complex logic to trigger the |
Wouldn't be easier to make match use another keyword instead of continue, like cascade, skip, fall, etc.? |
It seems using |
The point here is that using Using a different keyword is difficult because it would be harder for users to discover it and many of the suggestion I see either doesn't convey what it actually does (like |
I like it, but I'm not sure about the syntax, it looks like a ternary operator without match x:
1 if y > 5:
bar()
2 if y < 5:
baz()
1, 2: # Will match if the guards fail
qux() Perhaps some separator or other keyword is needed instead of |
Do I understand correctly that now only one match x:
1, 2:
print("a")
continue
1:
print("b1")
2:
print("b2") must now be: match x:
1:
print("a")
print("b1")
2:
print("a")
print("b2") The part that is common to several branches should now be duplicated or be moved to a separate |
Yes, that's the one drawback of this. However, I don't believe |
Overall, I like this change, although I must point out that I find the ability to use match data:
{"pos": var pos, ..}:
position = pos
continue
{"size": var size, ..}:
if shape is RectangleShape2D:
shape.extents = size / 2
continue
{"enabled": true, ..}:
enable()
continue
# ...and more... Would now have to each be a separate if data.has("pos"):
position = data.pos
if data.has("size") and shape is RectangleShape2D:
shape.extents = data.size / 2
if data.has("enabled") and data.enabled is bool and data.enabled:
enable()
# ...and more... ...which requires checking to see if a key exists on the Dictionary, then getting the value. Although, looking at it now, perhaps that's exactly how I should be doing it. Overall, I think this is a good change, I just worry that the very convenient ability to easily check many Dictionary values will be gone. |
Added the |
I was thinking about match x:
0 -> if y == 0:
print("y is zero")
0 -> if y < 5:
print("y is less than 5") match x:
0 :: if y == 0:
print("y is zero")
0 :: if y < 5:
print("y is less than 5") match x:
0 when y == 0:
print("y is zero")
0 when y < 5:
print("y is less than 5") |
Same tokens are used in different contexts, it's not exclusive to this one. The context makes it very obvious this isn't a return type. I prefer using something already in the language than introducing yet more tokens and keywords.
That's not correct, the right condition is checked only after the pattern matches. Not that it matters much because this an
That's very subjective. Also, I see the separator as match x:
0 -> y == 0:
print("y is zero")
0 -> y < 5:
print("y is less than 5")
I think introducing a new keyword just makes things harder. It's one more name to remember, while |
I didn't get why |
I asked for opinions in a local Godot group, and got confirmation that some people also associate Also, I'm concerned that the guard separator consists of two tokens. In my opinion, when it is a single token, its role is more clear (separate the guard expression from the pattern list). func _ready() -> void:
var x := 1
var y := 2
var a := true
var b := false
match x:
1 if true else 2 -> if a if y == 2 else b:
# ^^^^^^^^^^^^^^^^ ^^^^^ ^^^^^^^^^^^^^^^^^^
# const expression guard guard expression
# pattern sep.
print("Branch 0")
_:
print("Branch 1") The problem with the two-token separator is that it is not solid. There is a space, many people will read it like this: 1 if true else 2 -> if a if y == 2 else b:
# ^^^^^^^^^^^^^^^^ ^^ ^^^^^^^^^^^^^^^^^^^^^
# something left something right Yes, this example seems far-fetched, but it demonstrates that both pattern and guard can include a ternary operator. Now, the pattern expression must be constant (unless it's an identifier or an attribute chain). [code 1] [code 2] But that's reason enough, in my opinion. Plus, we may allow arbitrary non-constant expressions (like function calls) in the future, as far as I remember there is an issue about this. Compare: 1 if true else 2 when a if y == 2 else b:
# ^^^^^^^^^^^^^^^^ ^^^^ ^^^^^^^^^^^^^^^^^^
# const expression g. guard expression
# pattern sep. Alternatively, we could use 1 if true else 2 while a if y == 2 else b:
# ^^^^^^^^^^^^^^^^ ^^^^^ ^^^^^^^^^^^^^^^^^^
# const expression guard guard expression
# pattern sep. |
Personally I find python style ternary atrocious, so yeah, if you pile those nasty things it does not look good. Even your example with Even a simple ternary expression as a pattern without a guard seems strange to me: match value:
1 if true else 2:
print("hm") But, again, python has that ternary syntax and they were ok with marking guard with |
Patterns can be constant expressions, which can include the ternary In Python patterns are strictly literals, never expressions, and they introduced guards right away with
It is meant to be read as two sides (pattern and guard) so I don't understand what is the actual problem. I feel like that space is actually better for reading. I can see the Perhaps not the I do think @vonagam gave a good idea: force using parenthesis if you really need the ternary inside the pattern: match x:
1 if true: # This is a guard
pass
1 if true else 2: # This is not allowed
pass
(1 if true else 2): # This is fine
pass
(1 if true else 2) if true: # Ternary pattern with guard
pass This way the parser can disambiguate the ternary from the guard, while still allowing some way of making a ternary pattern. Now, this can still allow some atrocious writing: match x:
(true if true else false) if true if true else false:
pass But we shouldn't measure by the worst possible case (though the parenthesis actually help in this one). This is already a "problem" with the regular if x if true else y: That does break compatibility, but at least provide an alternative and one that also works on the current version so there's no need for a version switch. |
Ah, so those things are ok: print(1 if true if false else false else 2)
print(1 if false else 2 if false else 3) Yeah, I see how this will complicate parsing those cases, even if there is only one valid interpretation of a case it still can be problematic to write parser logic for that. (I mean that I think it is possible to do without breaking change, but it might be complicated, maybe too much...) An alternative to |
What about using match x:
0 | if y < 0:
print(n) |
Well, If you go for shorter - just do match value:
[var x] and x > 0:
print(x) There should be no problem with constants, since the point here is that right side is non-constant expression. Is there any downside that I do not see? Edit: I think I found some downside, so can pass on simple |
To me it looks better than an arrow. It looks like: https://en.wikipedia.org/wiki/Vertical_bar:
But I'm not sure if our parser can handle it as easily as with |
I for one prefer the |
I've mentioned |
I don't think it's a "hard" conflict. Another keyword could be used for compile-time |
I think it's important that whatever syntax we choose also reads well when used in conjunction with the proposals for a type narrowing syntax. For example. match x:
var y: int: y > 0:
# code treating x as a positive int named y or match x:
var y: int if y > 0:
# code treating x as a positive int named y or match x:
var y: int when y > 0:
# code treating x as a positive int named y |
To summarize the suggestions for syntax we have until now:
I think I got them all? It's very difficult and somewhat unproductive to discuss such things from a rational point of view, because all of it is subjective. I'm not going to attempt having very objective reasons. I will however provide my 2c:
For the same reasons of consistency, I would remove anything requiring a sequence of tokens, like That leaves
All three are valid, for my money, I don't feel either is a big departure from the language, or too difficult to teach. Out of these, I do understand the issues with ternaries, and I agree there's a problem there. I already see myself explaining 3 times a day on Discord that "the if used in match is not the same as the if somewhere else). I still think it is the best tradeoff overall. Would it be possible to treat an Second favorite is |
I also suggested
I think that we could reuse existing keywords in this case. For example, |
Good summary. Can remove
Yes, it should be. There whole thing about finding alternative syntax to simple if happens because "it is possibly confusing", not because "it is breaking". I do not agree with that. Using simple I would understand the search for alternatives if there was an ambiguous (for parser) statement with ternary. But there is no such example, right? |
I don't think the comparison with Python is correct. Python doesn't support ternary patterns, but we don't have to copy this constraint, our Other languages mostly have C-style ternary operator, so there can be no conflict with
See GDScript progress report: Writing a new parser - Less lookahead: Quote
Either we 1. restrict expressions in the context of
|
I mean, yes, we don't have to copy that constraint. That's what I am saying, is there anything that blocks supporting combination of ternaries and guards? If yes, an example would be beneficial.
At this position in parser you would just add |
Well, it's probably not as difficult to implement as I thought (although it still requires context and should only work for the top-level "ternary", not for nested ternaries). With another token, such hacks are not needed. My main concern is not technical, but semantic. It seems wrong to me to use the same token in different meanings:
|
Python uses it. A lot of people come from there and expect similar syntax. Other languages use So unless there is something that blocks using |
We don't have to repeat Python, it's okay that there are differences. For example, we don't have a case. I expect users to read the documentation or the changelog and not try to copy code from another language. All languages are slightly different. I emphasize that the goal is not to do "different from Python". It's just that Python initially chose a more limited option, while we can (and already partially do) support pattern expressions more universally. And in this case, the Plus maybe even with the limitation it's not a good idea in Python. We shouldn't blindly copy it as "this is how it's done in Python". Python can have bad solutions too, and we should discuss any solution, whether it's good specifically in GDScript, before copying it from Python.
|
There are two different arguments here that I've already addressed:
|
I don't see an issue for forbidding ternaries in |
I think operator precedence and parsing order for equal precedence may prevent us from implementing Option 2 (allow ternary operators). The Also, if you are proposing to allow both ternary patterns and ternary pattern guards, then we must consider all 4 cases: p1 if g1
(p1 if p2 else p3) if g1
p1 if (g1 if g2 else g3)
(p1 if p2 else p3) if (g1 if g2 else g3) Without parentheses: p1 if g1
p1 if p2 else p3 if g1
p1 if g1 if g2 else g3
p1 if p2 else p3 if g1 if g2 else g3 Since you said that parentheses are only for readability and should not affect the result, I tried to check how the parser will consume tokens. To do this, I added fake operands. print(pc if gc else fk)
print(pa if pc else pb if gc else fk)
print(pc if ga if gc else gb else fk)
print(pa if pc else pb if ga if gc else gb else fk)
It looks like the parser is grouping the For comparison, there is no ambiguity with p1 when g1
p1 if p2 else p3 when g1
p1 when g1 if g2 else g3
p1 if p2 else p3 when g1 if g2 else g3
I'll try to find an example later. Yes, this is rare case, but I see no reason to forbid it. |
Hm... that's a valid thing. That order will complicate things. Might be possible still, but more complex than worth it. Ok, I switch to |
I like |
It would be ideal to have something that uses an existing keyword, can't be ambiguous, and makes sense. I thought of reusing the keyword match x:
var y match y > 0:
print("x is greater than 0") It's just an idea, but it seems to fit the requirements better than other keywords. |
Describe the project you are working on
The GDScript implementation.
Describe the problem or limitation you are having in your project
The GDScript implementation.When using a
match
statement, there's no natural way to implement guards as it's done in other languages with this functionality. The close we can get is using anif
statement inside a block together with thecontinue
keyword, like this:This is awkward because the "guards" in this case have the opposite conditions that you want to capture. It needs more mental effort to understand what is happening.
Another case is using ranges instead of absolute values when matching numbers:
Which is better expressed as series of
if
blocks:But loses the more obvious approach with
match
that makes it all about the samenumber
, since anelif
can have any arbitrary expression.There's also the issue that the
continue
keyword is grabbed by thematch
statement, so you can't use it to go to the next iteration of a loop inside it.Describe the feature / enhancement and how it helps to overcome the problem or limitation
Using match guards we are able to take advantage of the
match
syntax while still being able to use arbitrary conditional expressions to further restrict each pattern arm. This without the need of having awkwardif
statements with negations inside the block.Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
Edit: Added
->
token for disambiguation.Following a syntax similar to what Python and other languages do, we could add an
if
keyword after the match pattern, which could have any expression as the condition. This would come after the->
token to avoid mixing up with the ternary expression in the pattern (while arbitrary expression aren't valid patterns, they still work if evaluated at compile time, including the ternary):Which makes the blocks much more straightforward to understand.
For the example of ranges:
Note that condition for the guard can use any arbitrary expression and use identifiers in the scope.
Guards are only evaluated after the pattern matches. If there are multiple patterns, the condition applies to all of them. That is, if any of pattern matches, then the guard expression will be evaluated.
This proposal also includes removing the use of
continue
insidematch
. This will allow user to use the keyword to go to the next iteration of the loop, even when inside match.If this enhancement will not be used often, can it be worked around with a few lines of script?
As mentioned before, it can be worked around but it's cumbersome and makes for a confusing use of the
continue
keyword. Many users don't know about this usage and trip when trying to use the keyword for the loop inside amatch
statement.Is there a reason why this should be core and not an add-on in the asset library?
This is an integral part of GDScript, it cannot be implemented as an add-on.
The text was updated successfully, but these errors were encountered: