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

GDScript: support mixing multiple variable definitions and expressions in if-statement #98335

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zjin123
Copy link

@zjin123 zjin123 commented Oct 19, 2024

UPDATE: As @KoBeWi suggested, the original PR is split into two commits. The first one implements support of variable definition in if-statement (godotengine/godot-proposals#2727). The second one introduces a new syntax for multiple variables and conditions in if-statement. For the rationale behind the second commit, please see comment and comment below.

Another attempt to implement godotengine/godot-proposals#2727 for if-statement. This patch implements two functions: allow variable definition as condition (if var n := foo():) and allow multiple conditions separated by comma in a single if-statement(if foo(), var n := bar(), n > 1:).

About implementation of the second commit: the ExpressionNode *condition field of IfNode is replaced with a list of either ExpressionNode or VariableNode. When parsing a single if-statement, a temporary block(SuiteNode) is created for parsing multiple conditions within and then the true block within. For an expression condition, one ExpressionNode is created. For a variable definition, one VariableNode and one ExpressionNode are created. The former one is used for generating code for initial assignment while the later one is used for testing as usual.

Compilation is done like usual except that, all of the not-true-jump-address for each condition will be patched to the same starting address else/elif if any, otherwise the end address of the if statement. Thus, the write_else() interface is changed to write_else(int count).

@zjin123 zjin123 requested review from a team as code owners October 19, 2024 12:50
@KoBeWi KoBeWi added this to the 4.x milestone Oct 19, 2024
@zjin123 zjin123 force-pushed the ifvar2 branch 3 times, most recently from bfde7e1 to c035f99 Compare October 19, 2024 15:32
@zjin123 zjin123 force-pushed the ifvar2 branch 2 times, most recently from f55af4d to 590a8cb Compare October 21, 2024 14:44
@KoBeWi
Copy link
Member

KoBeWi commented Oct 21, 2024

I'm not sure about the comma syntax, it's basically a weird and that allows multiple definitions. I think single definition is enough in most cases.

@zjin123
Copy link
Author

zjin123 commented Oct 22, 2024

I'm not sure about the comma syntax, it's basically a weird and that allows multiple definitions. I think single definition is enough in most cases.

Both the comma syntax and multiple definitions / conditions are borrowed from Swift (optional binding). As a Swift user, the comma syntax appears more readable than and keyword to me personally, but of course it can be changed to other syntax which may be more suitable for GDScript.

As for multiple definitions / conditions, it is mainly for static typing. While GDScript is a dynamic typing language, some people like me actually use it with static typing as much as possible. Static typing will introduce extra code. For example, consider dragging:

Without static typing:

func _input(event: InputEvent) -> void:
	if event is InputEventMouseButton && event.button_index == MOUSE_BUTTON_LEFT:
		if event.is_pressed():
			start_dragging(event)
		else:
			stop_dragging(event)
	elif dragging && event is InputEventMouseMotion:
		do_dragging(event)

Is very readable. But if one write it with static typing (maybe for performance and static checking):

func _input(event: InputEvent) -> void:
	if event is InputEventMouseButton:
		var mouseButtonEvent := event as InputEventMouseButton
		if mouseButtonEvent.button_index == MOUSE_BUTTON_LEFT:
			if mouseButtonEvent.is_pressed():
				start_dragging(mouseButtonEvent)
			else:
				stop_dragging(mouseButtonEvent)
	elif dragging && event is InputEventMouseMotion:
		var mouseMotionEvent := event as InputEventMouseMotion
		do_dragging(mouseMotionEvent)

If only one definition / condition is allowed, the first if (in the dynamic typing one) has to be split into two if as one can only call button_index on the casted result with static typing.

With static typing and multiple conditions:

func _input(event: InputEvent) -> void:
	if var mouseButtonEvent := event as InputEventMouseButton, mouseButtonEvent.button_index == MOUSE_BUTTON_LEFT:
		if mouseButtonEvent.is_pressed():
			start_dragging(mouseButtonEvent)
		else:
			stop_dragging(mouseButtonEvent)
	elif dragging, var mouseMotionEvent := event as InputEventMouseMotion:
		do_dragging(mouseMotionEvent)

This one requires less code lines and is as readable as the first dynamic typing one. Multiple conditions is useful for code with static typing. Code with dynamic typing perhaps do not need it at all.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 22, 2024

Still, the comma syntax should be a separate proposal and separate PR IMO. Can you split it?

@zjin123
Copy link
Author

zjin123 commented Oct 24, 2024

Sure. I close this PR for now and will reopen it when split is done.

@zjin123 zjin123 closed this Oct 24, 2024
@adamscott
Copy link
Member

@zjin123 You created two separate commits, but @KoBeWi suggested to create two separate PRs.

@zjin123
Copy link
Author

zjin123 commented Oct 26, 2024

@zjin123 You created two separate commits, but @KoBeWi suggested to create two separate PRs.

Thanks for the reminder. I just opened another PR #98538 for the first commit.

@zjin123 zjin123 changed the title GDScript: support variable definitions in if-statement GDScript: support mixing multiple variable definitions and expressions in if-statement Oct 26, 2024
@zjin123
Copy link
Author

zjin123 commented Oct 26, 2024

The reason of using comma here is that and or && is not suitable with variable definition. Because when either and or && is used directly after variable definition, they will become part of the variable initializer. For example,

if var x = 1 and y == 2:
	print(x) # x is true

if var x = 1 and foo(x): # analyzer error: x is not defined
	print(x)

To fix it:

if (var x = 1) and y == 2:
	print(x) # x is int

if (var x = 1) and foo(x):
	print(x) # x is int

The one works but requires additional parentheses.

With comma syntax, it is simple:

if var x = 1, y == 2:
	print(x) # x is int

if var x = 1, foo(x):
	print(x) # x is int

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

Successfully merging this pull request may close these issues.

4 participants