godot: GDScript: logical and/or between non boolean value cause a parser error

Godot version: 7d972b8c6

OS/device including version: windows10

Issue description: if both operands of the and/or operator are compile time known non boolean values, the parser throws error (where it shouldn’t)

func _ready():	
	if "" or []:
		pass
## Parse Error: Invalid operands "String" and "Array" for "or" operator.

Minimal reproduction project: N/A

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 9
  • Comments: 17 (13 by maintainers)

Most upvoted comments

I think a language should either be strict about if/and/or/not only taking boolean, or it should fully support truthy/falsey validation.

For a high level scripting language like GDScript I’d personally prefer the latter. Users who want to be safe can always use == to ensure explicit boolean comparison. Users / projects / cases where loose validation is fine should be able to use if/and/or/not with the same types.

When if "": is supported, then so should be if true and "": When if []: is supported, then so should be if true and []: and if not []:

Got curious and made a breakdown:

if in 3.x not in 3.x and/or in 3.x if in 4.0.alpha not in 4.0.alpha and/or in 4.0.alpha
“” false true supported false ⚠️ false, and only works with literal ⚠️ not supported
“a” true false supported true false, ⚠️ only works with literal ⚠️ not supported
0 false true supported false true supported
1 true false supported true false supported
0.0 false true supported false true supported
1.0 true false supported true false supported
[] false true supported false ⚠️ not supported ⚠️ not supported
[1] true false supported true ⚠️ not supported ⚠️ not supported
{} false true supported false ⚠️ not supported ⚠️ not supported
{“a”: 1} true false supported true ⚠️ not supported ⚠️ not supported
^“” false true supported false ⚠️ false, and only works with literal ⚠️ not supported
^“a” true false supported true false, ⚠️ only works with literal ⚠️ not supported
&“” n/a n/a n/a true false, ⚠️ only works with literal ⚠️ not supported
&“a” n/a n/a n/a ⚠️ false false, ⚠️ only works with literal ⚠️ not supported
Vector2.ZERO false true supported false ⚠️ false, and only works with literal ⚠️ not supported
Vector2.ONE true false supported true false, ⚠️ only works with literal ⚠️ not supported
null false true supported false true supported
[Object] true false supported true false supported
[Freed Object] true false supported true ⚠️ true supported

String and Vector only working with not as literals as well as not "" and not Vector2.ZERO validating to false seem to be separate bugs. Also [Freed Object] behaving weirdly, that’s reported here: #59816

Types like Vector3, AABB, Quaternion, Rect2, etc. seem to be suffering from the same issues as Vector2.

No idea what’s going on with NodePath and StringName. They are probably not really relevant in this table but still weird how they behave.

A few examples of such conditions in my code:

if last_action and Input.is_action_just_released(last_action): #last_action is String
if not menu.last_action or action != menu.last_action:
if in_water and randi() % 100 == 0: #in_water is array
if prev_crouch and not crouch and (check_raycast(Vector2(-15, -45), Vector2(-15, 0)) or check_raycast(Vector2(0, -45)) or check_raycast(Vector2(15, -45), Vector2(15, 0))): #check_raycast() is helper for intersect_ray(), which returns Dictionary
if animator.assigned_animation and  (target_animation.begins_with(animator.assigned_animation) or animator.assigned_animation.begins_with(target_animation)):
if is_inside_tree() and map_rect and not map_rect.has_point(global_position): #map_rect is Rect2
if not projectile_data: #Dictionary

Another common thing to do is if some_object and some_object.some_variable, where some_object might be null.

The main rationale behind keeping the old behavior is that these different objects do evaluate to boolean. If it’s possible to do if "string", forbidding if not "string" doesn’t make sense.

Another problem, this is more personal, is that long time ago I suggested that GDScript should treat non-booleans like Ruby or LUA, i.e. anything is “true” if exists and “false” if it’s null. I didn’t like that in Godot an empty Dictionary or Array or Vector2 is “false”. But eventually I got used to it, found it convenient and adapted into my code. And now, if suddenly we can’t use these values for boolean operations, it would turn out to be a big mistake.

That said, is there any reason not to keep the old behavior? Does it impact performance or something? If there is a strong reason against supporting objects in boolean operations then I guess I could maybe live with that (in pain, but still ;_😉. This mostly affects legacy projects. I’d have to add != "" or .is_empty() or == null in lots of places, but for new projects it’s just minor inconvenience (it adds unnecessary comparisons and increases verbosity, but the code is more explicit I guess).

Closing as it’s not a bug.

I don’t think it’s any less readable. Compare:

if some_object and some_bool:
if some_object != null and some_bool == true:

You wouldn’t normally use some_bool == true, no? It doesn’t matter whether it’s boolean or not if it’s inside a condition.

The thing is that booleans are usually named using a convention that makes them obvious in this context, so something like if is_in_floor is quite easy to understand. But something like if last_action can be ambiguous (is it trying to check if this is the last action in a potential list?). Also, if used in a condition I first assume it’s a boolean, so something like if in_water makes me think it’s a boolean, not an array. Same thing for object IMO, if some_object != null is usually better to read than just if some_object, even though for some reason objects were added to the new logical operators.

In any case, feel free to open a PR adding the missing combinations in the logical operators.

I don’t think this is a bug, the logical operators should only accept booleans.

It goes beyond being easy to read. This issue is about not having a arbitrary language.

var arr:Array = []
if not arr:#error because you cannot use something that is not a bool with "not"
  print("array empty")
if arr:#no error
  print("not empty")
var other_condition:bool = true
if arr and other_condition:#error because you cannot use something that is not a bool with "and"
  print("not empty")

But it gets better ! GDScript is not supposed to force typing on you, is it ? So if my variable is not type then I need to also check for non-nullity before checking for emptiness, right ? Nope, since it is not typed then I can just use the simpler check because … reason ? (I mean if its not a bug then there must be a reason right ?)

var arr = []
if arr and arr[0] == 0:
  print("arr[0] is 0")

Typing your data you want to be more sure of the data it contains to not have to be as explicit when trying to use that data.

Now for the addressing the “less readable” argument. Do you really think that :

var arr:Array = []
if arr and arr[0] == 0:
  print("arr[0] is 0")

… is less readable than :

var arr:Array = []
if arr.is_empty() and arr[0] == 0:
  print("arr[0] is 0")

I take that example because that is an extremely common occurrence for me, checking if something exists or is null before actually using it. How can it not be readable ? Why would you force the use of a more explicit when it is already more than obvious in that common case. I’m not against being more explicit at time when it is warranted, but here it seems to me like a case of Enterprise Edition coding. If you cannot understand a simple check of a object right before calling one of its function then I thing there is no amount of “!= null” that will help you and you should better get some sleep because you won’t write any good code until then.

Me: I want python
Mom: No, we have python at home 
Python at home: ...

The previous version of GDScript was already an ersatz of python, but ok, it was only a bit rougher and mostly due to the way classes and built-in types were different. But now it’s really going backward on a very common syntax like that doesn’t even work.

So yeah, that may not be a “bug” but that’s clearly an oversight and the discrepancy in behavior makes it an issue and not just a matter of opinion about readability.