Godot: The phrase "q0" is highlighted as a keyword in the editor

[umfk]

Operating system or device:
Windows 10

Issue description (what happened, and what was expected):
Typing "q0" in your script gets it highlighted in yellow even though it's not a keyword.

Steps to reproduce:
Type "q0" into a script and bask in the yellow light.

[bojidar-bg]

Can confirm

[akien-mga]

@bojidar-bg and I had a look and it doesn't seem obvious to find the cause of this bug, so removing the junior job tag.

[bojidar-bg]

Here is a snippet from IRC, showing our finds (not from official godot engine channels):

BTW, searching for "q0" ( https://github.com/godotengine/godot/issues/3843 ) in the repo didn't lead to any finds...
bojidar_bg: Yeah I've tried to understand #3843 too but couldn't
Akien: maybe some wierd hashing stuff?
Akien: only place outside drivers is scene/resources/animation.cpp
Well it probably wouldn't be "q0" directly, but more likely some parsing of a 'q' token and then '0'
Akien: where were highlighted words defined? :D
Akien: I grepped for 'q too, no luck
bojidar_bg: Check tools/editor/plugins/script_editor_plugin.cpp
The part after "set_syntax_coloring(true)"
It then refers to functions in modules/gdscript, etc.
I've looked at it but couldn't find the culprit hehe
I guess with some prints of the actual keywords it could be narrowed down
Akien: they come from gdscriptLanguage::get_reserved_words :P
so, https://github.com/godotengine/godot/blob/master/modules/gdscript/gd_editor.cpp
so, https://github.com/godotengine/godot/blob/6ea0863ed3e836127cfd38587a50679d40ca40bc/modules/gdscript/gd_script.cpp#L2678 :D
Yeah I've checked that, but did not find any q :p
Akien: https://github.com/godotengine/godot/blob/6ea0863ed3e836127cfd38587a50679d40ca40bc/modules/gdscript/gd_script.cpp#L2722
s/q/clue/ ;
maybe that one ? :D
I wondered about that one too :p
But that's still weird hehe
Akien: who shall try removing it? :D
Why would it make "q0" a keyword?
Akien: no reason :P
I have no clue what it does, but I'm not good with static const char* stuff :)
Akien: maybe it would get the memory from another location
so the q actually is some texture or something..
You can try )
:)
Akien: also, if this makes some error... https://github.com/godotengine/godot/blob/master/modules/gdscript/gd_functions.cpp#L110
but it has a ERR_FAIL_INDEX_V(p_func,FUNC_MAX,""); guard
so, no
Akien: nope, not from that zero
nothing has changed
Akien: actaully it seems like a condition to terminate the loop on 2728
Akien: seems like no luck for today :P
Akien: so, not a junoir job
better if they don't try :P

[nunodonato]

The problem is in String::hash() in ustring.cpp.
It seems the hash generated for "q0" is the same as for the keyword "or" :)

[nunodonato]

here's an improved algorithm for the hashing function (less collisions and better performance)

    static const unsigned int FNV_PRIME = 16777619u;
    static const unsigned int OFFSET_BASIS = 2166136261u;

    unsigned int hash2(char* str)
    {
        unsigned int hash = OFFSET_BASIS;
        while ((*str))
        {
            hash ^= *str++;
            hash *=FNV_PRIME;
        }

        return hash;
    }

[nunodonato]

PS - I tested and compared the performance of the two algs in a 10000000 for cycle. My suggested solution is faster, but only when the strings are relatively small. When I switched to longer strings (14chars), the performance was only slightly worse than the current one.

[bojidar-bg]

@nunodonato ~ BTW, your solution might (must) still have collisions. That's a general problem with hashing... Maybe a better solution would be to compare strings directly, even after the same-hash test passes :smile:

Still, if it is better, we might include your hash function too.

[reduz]

Feel free to make a PR with your hash function, if performance is about the
same there shouldn't be a problem.
That said, it may be a mistake to rely on hash functions for this feature
(or maybe longer/64 bit hashes should be used)

On Sun, Feb 28, 2016 at 6:43 AM, Bojidar Marinov [email protected]
wrote:

@nunodonato https://github.com/nunodonato ~ BTW, your solution might
(must) still have collisions. That's a general problem with hashing...
Maybe a better solution would be to compare strings directly, even after
the same-hash test passes [image: :smile:]

Still, if it is better, we might include your hash function too.

—
Reply to this email directly or view it on GitHub
https://github.com/godotengine/godot/issues/3843#issuecomment-189832161.

[magpie514]

I found another random false positive, "acR":

[bojidar-bg]

I guess we should both use hashes, and string comparison... or maybe the Dictionary class already does it?

[reduz]

I'm checking the code again, I can't find anywhere a hash alone is being used..

[reduz]

Something must be wrong and I am being unable to see it:

here the hash is computed
https://github.com/godotengine/godot/blob/master/scene/gui/text_edit.cpp#L836
and here the value is obtained from the hash and the string
https://github.com/godotengine/godot/blob/master/core/hash_map.h#L401

as you can see above, there is a string comparison happening too, so unless I'm missing something, it makes no sense that "q0" or "acR" are reported as keywords.

We should add some kind of tag "paranormal investigator job".

[SuperUserNameMan]

We should add some kind of tag "paranormal investigator job".

Yeah, because that was actually a very StrRange bug.