Implement GString concatenation operator

[PierceLBrooks]

This will allow for users to add together their GString references with plus sign notation from within the Rust runtime of a gdextension, like so:

let concat = &GString::from("12") + &GString::from(" is a ") + &GString::from("true") + &GString::from(" number");

[Bromeon]

Hi, and thanks for your contribution! 👍

I'm not sure the + operator is something I want to add to godot-rust. Your example is rather verbose and encourages inefficient code by creating lots of temporary allocations/conversions that are then immediately thrown away.

Instead of

let a = GString::from(" is a ");
let b = GString::from("true");
let concat = &GString::from("12") + &a + &b + &GString::from(" number");

one can write

let a = "is a";
let b = true;
let concat: GString = format!("12{a}{b} number").into();

Which has multiple advantages:

• more readable
• more performant (Rust string is concatenated, only one Rust->Godot conversion)
• works with any concatenation type (here bool), not just other GStrings.

[PierceLBrooks]

Hi @Bromeon,

The verbosity of my example usage was mostly just due to to mirroring the logic originally employed by the utilities_test.rs unit test file's utilities_str function.

In regard to performance, I do understand your concerns. My overall intention is to allow for a less lexically frictional experience for those adjusting to Rust from GDScript. Though operator overloading in this way may not be strictly so idiomatic for the avid Rust user, it does offer marginally greater parity with GDScript's syntax, which if that is not a primary priority for the GDExt project, then the closure of this pull request may indeed be appropriate.

[PierceLBrooks]

Please let me know if PierceLBrooks@7acaafc helps at all to minimize any of your performance concerns.

[Bromeon]

Please let me know if PierceLBrooks@7acaafc helps at all to minimize any of your performance concerns.

Using + still means that only 2 operands can be passed at a time, so a + b + c + d amounts to 3 str() roundtrips via FFI, plus the involved variant conversions and marshalling.

Using a single str() call is strictly better here. It also accepts non-string arguments.

---

In regard to performance, I do understand your concerns. My overall intention is to allow for a less lexically frictional experience for those adjusting to Rust from GDScript. Though operator overloading in this way may not be strictly so idiomatic for the avid Rust user, it does offer marginally greater parity with GDScript's syntax, which if that is not a primary priority for the GDExt project, then the closure of this pull request may indeed be appropriate.

I understand, but in godot-rust we need to strike a balance between what's idiomatic in Rust and what's idiomatic in Godot. The + operator just has too many disadvantages... and it's not even great in GDScript because it only supports string operands (I almost always use str myself).

The str function is already available, as a relatively straightforward migration path from GDScript to Rust. I agree it could be more discoverable, which is something I plan to address in godot-rust/book#11.

Potentially, we could also think about providing a format-style macro that internally calls str and can be used like this:

let a = "is a";
let b = true;
let concat = godot_str!("12{a}{b} number");

What do you think about this?

[Bromeon]

@PierceLBrooks would you be willing to change this PR to implement godot_str! as described above?

There is some prior art with the godot_print! macro; I could imagine the implementation to look very similar.

[PierceLBrooks]

@Bromeon yes, I believe I will have enough free time to make a proper attempt at doing so before the weekend is over here. Thanks for the opportunity!

[Bromeon]

Thanks to you! 😊

As mentioned, the already existing godot_print! macro could be a good start, as it follows the same principle. Let me know if you need any support!

[Bromeon]

@PierceLBrooks have you had a chance to look into this yet? Let us know in case you get stuck somewhere! 🙂

[PierceLBrooks]

@Bromeon , yes I have taken a look this week and will likely have conclusive progressive by the end of this weekend. The past week ended up being a little more chaotic than originally anticipated :P

[PierceLBrooks]

@Bromeon , the following commit hash reference should successfully address your suggestion... :)
PierceLBrooks@a6fc980

[GodotRust]

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

[Bromeon]

Thanks a lot, this looks already great!

I added some comments; the import is currently wrong (which is why CI fails).
You can run ./check.sh locally, which runs part of the test suite on your machine.

Would be great if you could squash the commits to a single one.

[Bromeon]

It would be good to add this at the end of the file https://github.com/godot-rust/gdext/blob/master/godot-core/src/global/print.rs, since it's so similar to the others.

Could you also add a small documentation, maybe even with an doctest example that shows just 2-3 things being concatenated?

[Bromeon]

The macro should be in godot::global, not godot, see
https://godot-rust.github.io/docs/gdext/master/godot/global/index.html#macros

This is likely automatically the case when you apply the previous suggestion of moving the godot_str! declaration to print.rs.

[PierceLBrooks]

@Bromeon How's this? PierceLBrooks@2446cc8

[Bromeon]

I really meant print.rs. While the string is technically not "print", what matters much more is that closely related code is co-located. Having one function in its own file just makes it harder to discover and requires extra navigation if e.g. a refactor across all these macros happens.

So please keep it in the same file, it's absolutely fine if that is called print.rs 🙂

[Bromeon]

After updating the code (see comment), please run ./check.sh which should show you errors about formatting.
Fix them with rustfmt.

Once ./check.sh passes, please do this:

Would be great if you could squash the commits to a single one.

Thanks a lot!

[PierceLBrooks]

@Bromeon , I believe the formatting, utility function consolidation migration, and squashing concerns have all now been addressed! :) Thank you for your patience.