Vector2 division breaks the editor

[jeff-s-zhou]

Operating system or device - Godot version:
2.2.1

Issue description (what happened, and what was expected):
Typing out Vector2 division immediately crashes the editor, and opening the project afterwards causes the editor to immediately crash again until you remove the line using some other method.

Steps to reproduce:
I wrote this line in the _ready call of an otherwise empty script in an empty scene.

Vector2(5, 0)/Vector2(1, 0)

[RebelliousX]

It is supposed to crash! In fact, I crashed trying to think about it, lol. There is no such thing as vector division. Vectors have sum, subtraction and 2 types of multiplication scalar (result is not a vector, just a number, a.k.a dot product) and cross product (result is a vector, which has many useful applications, determining if a vector is perpendicular to another is one of them).

This is not a bug, although, it should give a big fat error, not crashing the whole editor. Adding some exception for such expression would do it.

[jeff-s-zhou]

I'm not sure I was clear here. I didn't RUN the script, and in fact the specific line was actually just an in between step as I was rewriting the code.

Just writing the line at all causes the editor to crash and to crash every time the project is opened.

[Zylann]

Vector division is fine, it's just the same logic as member-wise operation as with add, multiply and subtract.
And as such, it also implies handling divide-by-zero cases.

I debugged the crash, and here is the call stack:

Weird thing that it tries to reload the script as if it was a tool. Of course, if it was, the editor itself would give errors too because the code is executed in the same context.

In gd_compiler.cpp:

			gdfunc->constants[idx]=*K;

idx was out of bounds, printed an error and returned null, causing a Variant assign error. Probably because evaluating the vector operation failed?

[Zylann]

I found that if you have a constant that is NaN, the GDScript parser will silently put it as key of constants_map. However, NaN == NaN returns false, which probably makes dictionary lookup to fail all the times (because the key is compared for equality once found through the hash) and return null anyways. It probably messes up earlier because of this :s

		while((K=codegen.constant_map.next(K))) {
			idx = codegen.constant_map[*K];
			gdfunc->constants[idx]=*K;
		}

Iterating on the keys will work, however access will fail if K has NaN inside...

As a side note, access to the hash map was done with the non-const operator, so a new entry is created (because key comparing failed) but its value was left uninitialized to -1163005939, which is obviously out of bounds.

Several possible fixes, not exclusive:

1. Prevent users from putting NaNs in hash-sensitive containers?
2. Prevent NaN from appearing in this case by throwing an error if the compiled constant is wrong?
3. Improve hashing so NaNs don't matter?

This particular case can be detected at parsing time by putting this in gd_parser.cpp in the _REDUCE_BINARY macro:

	if(res != res) {\
		printf("DDDDD Nan!!!! (3)\n");\
	}\

However it solves only one case and there are plenty of times a ConstantNode is created and assigned a value, and doesn't solves other cases where NaNs are used in hashes...

[Zylann]

Hmmm that fix isn't what I expected, and it makes comparisons slower (and I'm sure you can still inject NaNs :p)... but if @reduz thinks it's better... :)

[reduz]

So far it's just for gdscript, so should be hopefully ok

…

On Feb 14, 2017 12:15 PM, "Marc" ***@***.***> wrote: Hmmm that fix isn't what I expected, and it makes comparisons slower (and I'm sure you can still inject NaNs :p)... but if @reduz <https://github.com/reduz> thinks it's better... :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7354 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF-Z23W1ikftMwMepD8Ir9HNFRgNuq5vks5rccUXgaJpZM4LUaLr> .

[akien-mga]

Reopening as #7807 was reverted (better fix incoming, @hpvb and @tagcup are on it :D).