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

Fix StringName comparison #77197

Merged
merged 1 commit into from
May 18, 2023
Merged

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented May 18, 2023

Comparisons were not correct, probably due to being cast to String and compared there (due to the casting not being explicit) but not clear how it was broken, but now it isn't

Now:

a == b false
a != b true
a <  b false
a <= b false
a >  b true
a >= b true

Fixes: #76218

Documentation handled by #77083

The alternative to this would be to change how the comparison is made by changing how it works in variant ops, but I think this is more elegant and doesn't break compat (for the non-broken comparisons), alternatively we could just remove the other comparisons

I feel keeping the pointer based comparisons is useful, doesn't add a lot of hacky fixes to avoid using the built-in operators on StringName (and the lack of any such method of comparison indicates to me that the pointer-based comparison was intended), and if you want unicode comparisons you can just cast it to String

Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be merged with #77083.

@YuriSizov YuriSizov added this to the 4.1 milestone May 18, 2023
@YuriSizov YuriSizov merged commit 77991a0 into godotengine:master May 18, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@AThousandShips AThousandShips deleted the string_name_cmp branch May 18, 2023 16:43
@AThousandShips
Copy link
Member Author

Thank you!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.4.

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.

StringName comparisons give wrong and inconsistent results
3 participants