Support OnEditor<T>/Required<T>for #[export] similar to OnReady<T>

[Yarwin]

Currently the pattern for structs with Gd references to other nodes looks like:

#[derive(GodotClass)]
#[class(init, base=Node)]
pub struct SomeNode {
    #[export]
    required_node_a: Option<Gd<Node>>,
    #[export]
     required_node_b: Option<Gd<Node>>,
    #[export]
    optional_node: Option<Gd<Node>>,
    ...
}

#[godot_api]
impl INode for SomeNode {
    fn ready(&mut self) {
        let Some(required_node_a) = self.required_node_a.as_ref()  else { panic!("required node must be set!")};
        let Some(required_node_b) = self.required_node_b.as_ref()  else { panic!("required node must be set!")};
        if let Some(optional) = self.optional_node.as_ref() {
                godot_print!("optional node has been set!");
       }
    }
}

Thus forcing user to abuse the .unwrap().

Similar case in gdscript can be represented as:

extends Node

@export var required_node_a: Node
@export var required_node_b: Node
@export var optional_node: Node

func _ready() -> void:
	print(required_node_a.name)  # errors if required_node_a is not present
	print(required_node_b.name)  # errors if required_node_b is not present
	if optional_node:
		print(optional_node.name)

It would be great to create abstraction similar to OnReady<Gd> for required exports. The inner workings would be similar to OnReady, while the exported property itself could be represented by simple Required<Gd<Node>>.

[Bromeon]

Agree with this; I noticed the need for something like that and even started experimenting. One thing where I wasn't sure, and it's not obvious from your code either:

Should the initialization happen lazily or during ready? Because the latter would just be a special-cased OnReady<T> which also allows Export, no? Whereas the former could panic on Deref/DerefMut.

[Yarwin]

Personally I would prefer having it in ready – not supplying the required property is a logic error and user should be notified such. In broader context – I'm unsure if it wouldn't cause issues with various editor tools and plugins 🤷.

[Bromeon]

Is it guaranteed that #[export] fields are always initialized before ready()?
Also thinking of hot-reloading scenarios etc... 🤔

[Yarwin]

I did some digging and it seems that lazy Deref/DerefMut might be better option 🤔. I can move on to implementation if nobody has any objections

[Bromeon]

So the only reason for OnEditor to exist would be to avoid the explicit .as_ref().unwrap()?

I'm not saying that's not worth it -- I think these common cases should be as ergonomic as possible -- but I'd like to be clear about the goals 🙂 also then we can find the best possible name.

Furthermore, would this only exist for Gd<T> or also fields of other types?

[Yarwin]

So the only reason for OnEditor to exist would be to avoid the explicit .as_ref().unwrap()?

Yep. Moreover it would be handy if it could be used for Resources and Refcounted/Objects (almost half of my potential use cases are related to this).

Furthermore, would this only exist for Gd or also fields of other types?

For any T:ffi that implements GodotNullableFfi, 1:1 to option.

I have second thoughts though; it doesn't need to be a part of the core to work (can be external package instead), doesn't point to godot's abstraction directly (read - hard to discover) and implementing such wrapper for Option is trivial (shouldn't take more than 5 to 10 minutes? I pasted my wrapper below anyway).

here is my current wrapper, definitively naive (literally an Option<T> that deferences to T) albeit works.

pub struct Required<T> {
    inner: Option<T>
}

impl<T> Default for Required<T> {
    fn default() -> Self {
        Required { inner: None }
    }
}

impl<T> std::ops::Deref for Required<T>
{
    type Target = T;
    fn deref(&self) -> &Self::Target {
        match &self.inner {
            None => panic!(),
            Some(v) => v
        }
    }
}

impl<T> std::ops::DerefMut for Required<T>
{
    fn deref_mut(&mut self) -> &mut Self::Target {
        match &mut self.inner {
            None => panic!(),
            Some(v) => v
        }
    }
}


impl<T: GodotConvert> GodotConvert for Required<T>
    where
        Option<T::Via>: GodotType,
{
    type Via = Option<T::Via>;
}

impl<T: ToGodot> ToGodot for Required<T>
    where
        Option<T::Via>: GodotType,
{
    fn to_godot(&self) -> Self::Via {
        self.inner.to_godot()
    }

    fn into_godot(self) -> Self::Via {
        self.inner.into_godot()
    }
    fn to_variant(&self) -> Variant {
        self.inner.to_variant()
    }

}

impl<T: FromGodot> FromGodot for Required<T>
    where
        Option<T::Via>: GodotType,
{
    fn try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
        match Option::<T>::try_from_godot(via) {
            Ok(val) => Ok( Required { inner: val }),
            Err(e) => Err(e)
        }
    }

    fn from_godot(via: Self::Via) -> Self {
        Required { inner: Option::<T>::from_godot(via) }
    }

    fn try_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
        Ok(Required {inner: Option::<T>::try_from_variant(variant)?})
    }

    fn from_variant(variant: &Variant) -> Self {
        return Required {inner: Option::<T>::from_variant(variant)}
    }
}

impl<T> Var for Required<T>
    where
        T: Var + FromGodot,
        Option<T>: GodotConvert<Via = Option<T::Via>>,
        Required<T>: GodotConvert<Via = Option<T::Via>>
{
    fn get_property(&self) -> Self::Via {
        Option::<T>::get_property(&self.inner)
    }

    fn set_property(&mut self, value: Self::Via) {
        Option::<T>::set_property(&mut self.inner, value as Self::Via);
    }
}

impl<T> Export for Required<T>
    where
        T: Export + GodotType,
        T::Ffi: GodotNullableFfi,
        Option<T>: Var,
        Required<T>: Var
{
    fn default_export_info() -> PropertyHintInfo {
        Option::<T>::default_export_info()
    }
}

[bcolloran]

So the only reason for OnEditor to exist would be to avoid the explicit .as_ref().unwrap()?

I'm not saying that's not worth it -- I think these common cases should be as ergonomic as possible -- but I'd like to be clear about the goals 🙂 also then we can find the best possible name.

Just a quick note here: for me it's not about avoiding writing out .as_ref().unwrap() so much as correctly modeling the semantics of my nodes (which is the big reason I choose rust over GDscript! 🙂).

As @Yarwin mentions above, I have some cases where some an exported item is legitimately optional, and so is well modeled by:

    #[export]
    optional_node: Option<Gd<Node>>,

But I'm also making heavy use of a pattern where I'm using Resources to configure nodes, and the resource is required for the node to function correctly. I think an approved/recommended way of doing this would be nice, whether that means including a wrapper type, an export attribute, or even just mentioning Yarvin's recipe above in the "Recipes" section of the book.

Thanks Bromeon, loving gdext!

[Yarwin]

note: The declaration of mentioned recipe changed slightly, I'll update it later 😅

I have some cases where some an exported item is legitimately optional

More usecases can be found in: godotengine/godot-proposals#162

I think an approved/recommended way of doing this would be nice, whether that means including a wrapper type, and export attribute

@bcolloran the main issues are discoverability, consistency with Gdscript's user API and avoiding cognitive overload. I think I can made PR with said recipe for a book and then we'll see how wildly it's used?

[bcolloran]

Thanks @Yarwin!