Possible implementations of rust convenience traits

[lilizoey]

Many standard rust types use various convenience traits to extend their functionality, we should consider which of these convenience traits we can and should implement.

An attempt at an exhaustive list of such traits:

• Borrow/BorrowMut/ToOwned
• Clone
• Eq/PartialEq
• Ord/PartialOrd
• From/TryFrom (Into shouldn't be directly implemented)
• AsRef/AsMut
• Default
• Error
• Various fmt traits. Debug, Display, and Write being most important.
• Future
• IntoFuture
• Hash
• The iterator traits
• ToSocketAddrs
• Operator overloading
• SliceIndex
• FromStr
• Wake

[lilizoey]

As a general thought, i think we should try to implement most of these where it makes sense. As that helps the bindings be used throughout the rust ecosystem.

For some specific thoughts:

We could maybe implement Borrow/BorrowMut/ToOwned for converting between T and Gd<T>.

Do we need our own Share trait? it basically functions like Clone but is not as flexible as Clone because other crates dont know about it. Could we just replace it with Clone? There are already types in the rust std whose Clone impl functions semantically like share does, like Rc/Arc.

We could implement From/TryFrom more eagerly than we currently do. which could help with cross compatability of crates.

It might be possible to implement Future for signals. We should explore this eventually.

[Ogeon]

The lack of Clone for Gd is a bit inconvenient since it prevents deriving Clone for types that directly contain a Gd. Not to mention generic code that doesn't know about Share.

[Bromeon]

Yep, there are quite a few hints toward providing impl Clone for Gd<T>. The current design is a consequence of the issues encountered in godot-rust/gdnative#712, but we may revisit this as more and more functionality is mapped to gdext.

The question is of course what we would do for deep-copies. It could be argued that someone cloning a Vec<Gd<EntityData>> would want a duplicate of the vector, not just new references to the same data. This is especially true for "data bundle" types EntityData, not full-blown classes.

[Ogeon]

I can definitely see how it's tricky to decide on the right API and semantics and thanks for linking the previous discussion. My spontaneous expectation, from my own use and understanding so far, would be that Clone would be as close as possible to what gdscript does when assigning a value to a variable. 🤔 I mean such that

var a := MyType.new()
var b := a

basically corresponds to

let a: Gd<_> = MyType::new(); // made up example fn
let b = a.clone();

But I'm not here to revive the discussion and I'm sure I'm missing a ton of weird cases with my anecdotes, anyway. 😬

[Bromeon]

That's an interesting perspective. It would mean that copy-on-write types like GodotString or PackedArray would be deep-copied (upon first write), whereas reference-counted ones like Array, Dictionary or Gd<T: RefCounted> would be shared.

It means the trait (Clone or Share) no longer expresses what types of copy semantics are used; however a Godot user would likely need to know this anyway when working with the types.

[Ogeon]

This is specifically for Clone, which I'm thinking is the "default" way of copying. I'm hoping it would be easier to memorize things and prevent mistakes if the rules are the same in both rust and gdscript. But I don't know if it's too messy or hard to implement.

I think there's still room for more explicit methods or traits where it makes sense, so Gd could in theory also have a bikeshed_clone_inner function that clones the data. And Share can maybe stay for things that explicitly can be shared, if that's useful. Hopefully not in a way that results in multipe similar options again...

Would Clone even work normally on a GodotClass, considering it has the base reference that I'm assuming would need to be replaced? 👀 I suppose Base can have a special clone implementation?

[Bromeon]

Would Clone even work normally on a GodotClass, considering it has the base reference that I'm assuming would need to be replaced? 👀 I suppose Base can have a special clone implementation?

Not in the general case -- it works well for types that are "data bundles", i.e. classes that:

• inherit RefCounted
• don't have a #[base] attribute
• can thus live outside a Gd<T>, and be constructed using Gd::new(obj)

They typically have quite different usage than regular classes. One thing that can be useful for them is value semantics. But maybe there should be a dedicated DeepClone trait in such cases.

For nodes, there's also the special case of duplicate(), but I'm not sure that should be mixed with Clone 🤔

[Ogeon]

Oh, I didn't realize it was possible to omit the #[base] attribute, but I guess I didn't try either. 🤔 But it makes sense to want to treat those types like more regular Rust types, at least outside the Gd.

[bluenote10]

We could maybe implement Borrow/BorrowMut/ToOwned for converting between T and Gd.

Due to rust-lang/rust#41906 I'm always a bit scared of using a use std::borrow::Borrow; at module level (because it will break typical usage Rc<RefCell<T>>). So we should be a bit careful that this doesn't lead to similar ambiguity issues.

[Bromeon]

I'm also a proponent of "just because we can, doesn't mean we should" 🙂

Before implementing traits like Borrow, I'd like to see practical use cases in the context of gamedev, not theoretical generic scenarios. In case of doubt, we should wait and see if people truly run into scenarios, where these would solve something that's not already possible with the existing API.

[Bromeon]

As it is, this issue is too open-ended and has no clear actionable or condition after which it can be closed. It thus becomes a candidate for "perpetually open", which I'd like to avoid.

Plus, I'm not a big fan of the approach, as stated before. These traits should be justified by use cases, like every other feature. Doing use-case-driven development ensures that all code in existence is actually useful (by definition) and has gone through consideration by maintainers. The alternative is that we end up adding a lot of code that might not benefit anyone at all, yet we keep paying the maintenance cost until eternity.

Of course, simple things like Debug don't need a huge justification every time. But for many of the above traits, the added value is not immediately obvious.

There have already been several issues and PRs toward specific traits:

• Implement Display and Debug for Builtins #553
• Implement FromStr for NodePath #201
• GFile: read/write with fa.*_buffer #528
• Godot vs. Rust hashes #505
• Consistent error APIs #536

And some should likely be tracked around the time when a specific feature is going to be implemented (e.g. Future/IntoFuture/Wake if we are starting an async implementation, far in the future -- no pun intended).

If you see traits in the above list that are missing and would add a lot of value right now, please speak up. We can then discuss and track those separately. I will close this issue by end of next week, so we keep the tracker focused on more concrete and higher-priority issues -- of which there are a lot 😀