Merged
Conversation
7ae5767 to
e51030f
Compare
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1415 |
e51030f to
874a6de
Compare
ac7053d to
6f97e3c
Compare
Easier than converting to String/GString, and requires no allocation.
6f97e3c to
7ae901f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of:
you can now write:
And the best part, it's not only shorter but also faster.
Performance
The
Eqimpl doesn't convert or allocate, as it can reuse theGString::chars()method and combine it with iterators.The implementation is very simple:
We might add a similar
EqforStringNameandNodePathin the future, but those currently need a conversion since there's no way to get internal raw pointers in GDExtension (which isn't such a big deal for rarely-usedNodePath).Compatibility
This change nicely highlights yet another reason why
Fromtraits are a bad idea.By adding
Eq<&str>, I now break existing code"str".into()that has just become ambiguous. This all wouldn't happen forGString::from("str")or an explicit"str".to_gstring().This might be one of the last nails in the
Fromcoffin for string types in godot-rust. It might be time to call it a failed experiment and rely on explicit constructors and/or namedto_gstring()converters instead. I despise breaking such code, butFromreally stands in the way of progress, and this not for the first time.