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 2.0: Error when trying to call a method with a ternary operator #71065

Closed
DarkMessiah opened this issue Jan 8, 2023 · 8 comments · Fixed by #71120
Closed

GDScript 2.0: Error when trying to call a method with a ternary operator #71065

DarkMessiah opened this issue Jan 8, 2023 · 8 comments · Fixed by #71120

Comments

@DarkMessiah
Copy link
Contributor

Godot version

v4.0custom_build[fcba87e]

System information

Windows 10

Issue description

Error when trying to call a method with a ternary operator.

Steps to reproduce

extends Node

func _ready() -> void:
	test() if true else test()

func test() -> void:
	pass

fcba87e - Line 4:Cannot get return value of call to "test()" because it returns "void".

v4.0beta10 - works fine

Minimal reproduction project

ternary_bug.zip

@Cykyrios
Copy link
Contributor

Cykyrios commented Jan 8, 2023

Is this actually a bug or is it intended to work this way? As I understand it, the ternary operator is used to get a value, and as such is not suited for the way you are trying to use it.
It should however work if the test() function returns something, e.g.:

func _ready() -> void:
    var x := test() if true else test()

func test() -> float:
    return 4.0

@jordi-star
Copy link
Contributor

@Cykyrios I assume its a regression since they listed that it worked in beta 10.

@Cykyrios
Copy link
Contributor

Cykyrios commented Jan 8, 2023

My bad, missed the part about this working in beta 10. For reference, this change comes from #70702.

I updated the code snippet as follows:

func _ready() -> void:
	test_one() if true else print("test one")
	print(test_two() if true else 2.0)

func test_one() -> void:
	print("test_one function")

func test_two() -> float:
	return 4.0

Before #70702, this prints "test_one function" and "4". Note that test_one() if true else print("test one") raises a warning: Standalone expression (the line has no effect)..
After #70702, this line additionally raises two errors: Cannot get return value of call to "test_one()" because it returns "void". and Cannot get return value of call to "print()" because it returns "void".

I still stand by my previous comment, but whether it is an actual bug or by design should be clarified, what's your take on this @vnen?

@DarkMessiah
Copy link
Contributor Author

This also works in 3.x versions and has always worked.

@vnen
Copy link
Member

vnen commented Jan 9, 2023

This is actually debatable. The ternary operator is meant to be an expression: you get the return value of either branch when the code runs. If it returns void, there's no value to return.

The only thing is that it is being shoehorned as a shorthand if statement here, which is not really the intended case. It was only working before by the same way you could use the return value of void functions. Since that is now fixed, the ternary operator cannot use the non-existing value anymore.

The question is whether or not we want to allow this usage. I did notice that you cannot use if-else in a single line, like: if true: test_one(); else print("test one") gives an error, but for me allowing this would be better than using the ternary operator. Though I don't have anything particularly against the ternary operator for this case.

@vnen
Copy link
Member

vnen commented Jan 9, 2023

BTW, using ternary standalone already raises the STANDALONE_EXPRESSION warning. If we want to allow this usage then we should remove this warning for the ternary operator.

@anvilfolk
Copy link
Contributor

anvilfolk commented Jan 9, 2023

As a guideline for what ought to work/shouldn't work, it was very useful for me when @akien-mga suggested thinking of GDScript as a scripting language that behaves similarly to C++. In that case, and in many (most?) programming languages, ternary operators are expressions, and expressions return values.

I think this would be unexpected for people who already know ternary operators, and might be confusing for newbies who might end up using it with a function that doesn't return a value (when it should), and they get no warning.

I'd personally prefer doing what @vnen said and allowing single-line if-else statements. I don't feel strongly about that though, as I understand there are concerns around backwards compatibility :)

@vonagam
Copy link
Contributor

vonagam commented Jan 12, 2023

Hm. Turns out I used that construction in one of my code bases:

func set_prop(key: Variant, value: Variant) -> void:
    self.node.set_indexed(key, value) if key is NodePath else self.node.set(key, value)

For C++ and other languages: In C++ you can have standalone ternary operator with void branches. And as I remember almost all languages that do have ternary do allow it to be standalone without need for saving return value. I say almost because maybe I don't know some.

For "expressions should return value": CallNode is ExpressionNode and it can lack result value just as well.

vnen added a commit that referenced this issue Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants