Skip to content

Conversation

@jkb0o
Copy link
Contributor

@jkb0o jkb0o commented Sep 19, 2017

What happens without this PR - in most cases an error is generated - Invalid operands 'int' and 'String' in operator '=='. This PR allows equality checks between any types of variants without errors:

var vars = [null, 0, 1, 1.1, "1", [], {},Vector2(), Vector3(), Node.new(), Reference.new(), Transform(), Transform2D(), NodePath("/")]
for i in range(vars.size()):
	for j in range(vars.size()):
		if i == j:
			assert(vars[i] == vars[j])
			assert(vars[j] == vars[i])
		else:
			assert(vars[i] != vars[j])
			assert(vars[j] != vars[i])

Some discussion was started here #11388. It also fixes #11411.

This PR is more like discussion over something should be merged right now. I'm not c++ pro and did it to provide my point of view first of all. I believe equality checks should not generate any errors, this is completely unexcepted behaviour, especially for such dynamicaly typed language as GDScript. I won't receive any errors just for asking is Monkey the same thing as Banana. I want false result in this case. Python (for example) is OK with equality checks of different types. In most cases this is matter of aesthetics, but if we think about how external data can be changed over time equality checks behaviour becomes incredibly important.

@akien-mga
Copy link
Member

CC @hpvb @karroffel @reduz

@hpvb
Copy link
Member

hpvb commented Sep 19, 2017

We were just discussing this on IRC and we decided to keep it this way. I don't know if @reduz and @karroffel want to go over it again here?

I'm really pretty much OK either way even though it will make type inference later much harder for the compiler we're planning. I'd like this to be consistent though and in master it is not.

I can see both sides of the argument in this case. For users this is a nice feature, for consistency and ease of optimization it's not great. I guess this will end up being a judgment call for @reduz.

@neikeq
Copy link
Contributor

neikeq commented Sep 19, 2017

What does this PR do? Would "1" == 1 return true or would it return false instead of erroring?

In both cases I am against this. If the latter is required though, it could be provided as a function.

@jkb0o
Copy link
Contributor Author

jkb0o commented Sep 19, 2017

"1" == 1 will return false. Hmm. I wonder why you are against this. I just can't imagine a single case when erroring is a choice.

@neikeq
Copy link
Contributor

neikeq commented Sep 19, 2017

I consider comparing a String to and int to be an error. I want it to fail. If such comparison is desired I want it to be explicitly specified, not the default.

@karroffel
Copy link
Contributor

I have to agree with @neikeq, comparing a string and an int in the first place is just begging for trouble, apart from that it doesn't make any sense.

GDScript is largely inspired by python, and python has the motto "explicit is better than implicit", so I'm in favour of only allowing comparison of same types (apart from numeric types).

Also @hpvb and I are planning on adding type inference and a more modern compiler architecture, weak typing makes type inference a lot harder while adding more weak typing adds no real value apart from being able to write more ambiguous code.

@hpvb
Copy link
Member

hpvb commented Sep 19, 2017

Yeah we run the risk of having to explain to people how their code being slow is due to it being ambiguous. We'd then have to add a feature to the editor that can point people to the places where type inference can't work.

By just not allowing this at all we're short circuiting it. It also has the additional benefit that once we've merged our new compiler and vm all GDScript will be fast(er).

@akien-mga
Copy link
Member

akien-mga commented Sep 19, 2017

The Godot philosophy is to only implement features when there is a real life use case for them.

Tests such as 1 == "1" have always been invalid in Godot, which this screenshot from Godot 2.1.4 confirms:
spectacle b16820

The fact that you never noticed it until @hpvb fixed some inconsistencies in Variant's typing shows that even as an (apparent) strong advocate of weaker type checking, you never had a use case for what you are trying to allow now.

Noone else every complained about it up to now. So I guess there's no reason to make things more lax just for the sake of it, if it doesn't solve a real life problem.

@jkb0o
Copy link
Contributor Author

jkb0o commented Sep 19, 2017

The fact that you never noticed it until @hpvb fixed some inconsistencies in Variant's typing shows that even as an (apparent) strong advocate of weaker type checking, you never had a use case for what you are trying to allow now. Noone else every complained about it up to now. So I guess there's no reason to max things more lax.

Fair enough. Ive released a pretty heavy game having no problems with equality checks. This is some old patterns won't get out from my head from python's times. Thank you for your time, guys.

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.

NodePath cant be compared with null

5 participants