Skip to content

Invalid From<&VariantArray> for Packed*Array impls #1186

Open
@Bromeon

Description

@Bromeon

There are lots of From implementations from VariantArray to Packed*Array:

impl From<&Array<Variant>> for PackedByteArray {...}
impl From<&Array<Variant>> for PackedColorArray {...}
...

This is of course incorrect, as the variants may hold different types and From must be infallible.
They are implemented here:

impl_builtin_froms!($PackedArray; VariantArray => $from_array);

macro_rules! impl_builtin_froms {
($To:ty; $($From:ty => $from_fn:ident),* $(,)?) => {
$(impl From<&$From> for $To {
fn from(other: &$From) -> Self {
unsafe {
// TODO should this be from_sys_init_default()?
Self::new_with_uninit(|ptr| {
let args = [other.sys()];
::godot_ffi::builtin_call! {
$from_fn(ptr, args.as_ptr())
}
})
}
}
})*
};
}

Instead, we should use TryFrom or a named conversion method. Also, we could add infallible conversions from concretely typed arrays, e.g. Array<Color> <-> PackedColorArray in both directions. Right now, there is only PackedColorArray -> VariantArray.

Overall, From/TryFrom is probably OK here, but named methods are a potential alternative. We need to keep in mind that there are other conversions, e.g. between slices, Vec and so on, so maybe enabling generic programming is beneficial here.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions