gdext
gdext copied to clipboard
Feature : implement ToGodot for Vec<T>, Result<T>, Option<T> where T : ToGodot
For option Maybe returning T::to_variant()
if option is some else Variant::nil()
for Result dict!("Ok" : T::to_variant() )
for Vec
, returning a variant array
Option
is being worked on (at least for Option<Gd<T>>
) at #240, by the way. I'm not aware of any work being done for vectors or results.
I'd be interested to contribute if no one is on the case yet !
Vec<T>
definitely has a pretty obvious implementation, alongside other obvious collections. In fact we already have a From
impl to convert Array<T>
into Vec<T>
, though not the reverse which is weird, but i guess you can just do .iter().collect()
.
Result
i'm a bit more unsure about. i think we should base Result
on #246 when that's merged, rather than make a separate implementation.
Option
will be handled by #247 as mentioned, and it makes more sense to give it a separate implementation since it's used to represent nullable values.
What would the semantics for Result<T, E>
be?
Would any Err(...)
enumerator always map to Variant::nil()
?
Also, what's the motivation behind it? Might be more explicit to use Result::ok()
if that's desired 🤔
@astrale-sharp any comments? 🙂
Would any
Err(...)
enumerator always map toVariant::nil()
?
I don't understand what you mean there.
If V
and E
in Result<V,E>
are both ToGodot
I don't see what's lost in making the Result ToGodot as well, it can make handling Errors on the gdscript side a little more convenient.
Actually I don't really care about Result but Option and Vec are definitly something I came across making my game
Note: ToVariant
has now been renamed/re-purposed into ToGodot
Suggestion:
Assume here that T: GodotConvert/ToGodot/FromGodot
as appropriate.
GodotConvert
:
-
Option<T>
, withVia = Variant
(unless other more specific implementation exists, such as forOption<Gd<T>>
) -
Vec<T>
, withVia = Array<T::Via>
-
Result<T, ConvertError>
, withVia = T::Via
(see #467)
ToGodot
:
-
Option<T>
mapsNone => Variant::nil()
,Some(x) => x.to_variant()
-
Vec<T>
does effectivelyvec.iter().collect()
-
Result<T,E>
is either not implemented, or we just panic forErr
. In the future we may support usingResult
as a return type, whereErr
prints an error to Godot or something like that.
FromGodot
:
-
Option<T>
mapnull => None
andvalue => T::from_godot(value)
-
Vec<T>
does effectivelyarr.iter_shared().collect()
-
Result<T, ConvertError>
doesOk(T::try_from_godot(value))
(see #467)
We then have:
-
Option
always maps to a nullable variant, except when the type itself is nullable. -
Vec
isArray
as most people would expect -
Result
can be used to detect errors when a function is called
Additionally we should implement Property
and maybe Export
for at least Option<T>
, unless this causes godot to behave weirdly (such as the editor displaying weird values if you attempt to set it to the wrong value). We may consider only implementing it for Option<Variant>
if that's the case.
Sounds good, thanks for listing it up.
I'm not sure about Option<Variant>
though -- you would have two possible null states: None
and Some(Variant::nil())
.
It might be a necessity if we have an Option<T>
blanket impl though...
Sounds good, thanks for listing it up.
I'm not sure about
Option<Variant>
though -- you would have two possible null states:None
andSome(Variant::nil())
. It might be a necessity if we have anOption<T>
blanket impl though...
i think we'd probably implement it for each different option of Via
. Since there's only a limited number of possible Via
types. so we can choose to leave out Variant
if we want. (kinda necessary if we want Option<Gd<T>>
to be different)
Lately, I've made a wrapper for Vec<Gd<T>> where T: Inherits<Resource>
for my crate: https://github.com/StatisMike/gd-props/blob/master/gd-props-defs/src/types.rs#L88.
It implements GodotConvert + ToGodot + FromGodot
, with Array<Gd<T>>
as its Godot representation.
Also, it implements Property
and Export
, though with Property
its Intermediate
needed to be different: VariantArray
.
It's because there were problems with extending the vector in the editor - otherwise, there will be a need to either create default Gd<T>
and extend the Vec
on inputting another element, or make the Rust representation Vec<Option<Gd<T>>>
, making it much more verbose than current roundtrip from Array<Gd<T>>
to `Vec<Gd<T>>``
I think I could whip up a more universal wrapper for gdext
- I think that besides GodotConvert + ToGodot + FromGodot
the Property
and Export
should be also implemented for it to be able to expose it on the Godot side and make it exportable.
To be clear, Property
/Export
should be considered separately from ToGodot
/FromGodot
.
The former should not be implemented for things like Vec<T>
because they come with costly conversions every time a property is read or written on Godot side. This is not clear from simply declaring a field in a struct. We thus only implement these traits for native Godot types.
Returning/accepting values in #[func]
on the other hand already implies by-value semantics, so it's less surprising. ToGodot
/FromGodot
can be supported for different Rust types as indicated in the title. I don't see why there should be a special case for Resource
though.
@Bromeon The special case for Gd<Resource>
was only valid in the linked crate: it is for handling resources, so it was intended as serialization wrapper for subresource collections.
Though you are right, if any wrapper should be constructed at all, it should be done other way around.
I'm changing the scope of this issue to Vec<T>
.
Regarding the other types mentioned in the original title:
-
Option<T>
is not generally applicable since GDScript doesn't have nullability, so something likeOption<i32>
makes no sense. The types that can represent null states areVariant
andOption<Gd<T>>
, which are already supported.- Auto-downgrading
Option<T>
to useVariant
on the FFI comes at a loss of type safety and performance; as such I'd prefer if this were explicit for now.
- Auto-downgrading
-
Result<T, E>
is discussed separately in #425.