Skip to content

Conversation

@Bromeon
Copy link
Member

@Bromeon Bromeon commented Nov 24, 2025

Instead of:

if gstring == "text".into()
// or 
if gstring.to_string() == "text"

you can now write:

if gstring == "text"

And the best part, it's not only shorter but also faster.

Performance

The Eq impl doesn't convert or allocate, as it can reuse the GString::chars() method and combine it with iterators.
The implementation is very simple:

fn eq(&self, other: &&str) -> bool {
    self.chars().iter().copied().eq(other.chars())
}

We might add a similar Eq for StringName and NodePath in 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-used NodePath).

Compatibility

This change nicely highlights yet another reason why From traits 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 for GString::from("str") or an explicit "str".to_gstring().

This might be one of the last nails in the From coffin for string types in godot-rust. It might be time to call it a failed experiment and rely on explicit constructors and/or named to_gstring() converters instead. I despise breaking such code, but From really stands in the way of progress, and this not for the first time.

@Bromeon Bromeon added this to the 0.5 milestone Nov 24, 2025
@Bromeon Bromeon added c: core Core components breaking-change Requires SemVer bump performance Performance problems and optimizations labels Nov 24, 2025
@Bromeon Bromeon force-pushed the feature/gstring-str-eq branch from 7ae5767 to e51030f Compare November 24, 2025 22:22
@Bromeon Bromeon added the feature Adds functionality to the library label Nov 24, 2025
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1415

Easier than converting to String/GString, and requires no allocation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Requires SemVer bump c: core Core components feature Adds functionality to the library performance Performance problems and optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants