Add OnEditor<T>, remove Export implementations for DynGd and Gd

- Restore Var implementation.
- Allow to use `OnEditor<T>` for types that are represented by `BuiltinExport` (like converted enums represented in godot as i64 and whatnot)
- Add proper bounds for `impl<T> BuiltinExport for Option<T>`
- Remove `Sealed` for `OnEditor` since it uses Option implementation
- Extend & tweak docs according to CR comments
- Add information about skipping before-ready checks for `#[class(tool)]`
- Include `compile_fail` doctests and examples.

Author: Yarwin

godot-core/src/meta/sealed.rs

@@ -11,7 +11,7 @@
 use crate::builtin::*;
 use crate::meta;
 use crate::meta::traits::{ArrayElement, GodotNullableFfi, GodotType};
-use crate::obj::{DynGd, Gd, GodotClass, OnEditor, RawGd};
+use crate::obj::{DynGd, Gd, GodotClass, RawGd};
 
 pub trait Sealed {}
 impl Sealed for Aabb {}
@@ -72,4 +72,3 @@ where
     T::Ffi: GodotNullableFfi,
 {
 }
-impl<T> Sealed for OnEditor<T> where T: GodotType {}

godot-core/src/obj/dyn_gd.rs

@@ -139,12 +139,51 @@ use std::{fmt, ops};
 ///
 /// # `#[export]` for `DynGd<T, D>`
 ///
-/// Exporting `DynGd<T, D>` is possible only via algebraic types such as [`OnEditor`] or [`Option`].
+/// Exporting `DynGd<T, D>` is possible only via [`OnEditor`] or [`Option`].
 /// `DynGd<T, D>` can also be exported directly as an element of an array such as `Array<DynGd<T, D>>`.
 ///
 /// Since `DynGd<T, D>` expresses shared functionality `D` among classes inheriting `T`,
 /// `#[export]` for `DynGd<T, D>` where `T` is a concrete Rust class is not allowed.
-/// Consider using `Gd<T>` instead in such cases.
+///
+/// ```compile_fail
+///
+///  use godot::prelude::*;
+///
+/// trait Health { /* ... */ }
+///
+/// #[derive(GodotClass)]
+/// # #[class(init)]
+/// struct Monster { /* ... */ }
+///
+/// #[godot_dyn]
+/// impl Health for Monster { /* ... */ }
+///
+/// #[derive(GodotClass)]
+/// #[class(init, base = Node)]
+/// struct MyClass {
+///     #[export]
+///     dyn_concrete: Option<DynGd<Monster, dyn Health>>,
+/// }
+/// ```
+///
+/// Consider using `Gd<T>` instead in such cases:
+///
+/// ```
+///  use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// #[class(init, base = Node)]
+/// struct Monster { /* ... */ }
+///
+/// /* ... */
+///
+/// #[derive(GodotClass)]
+/// #[class(init, base = Node)]
+/// struct MyClass {
+///     #[export]
+///     dyn_concrete: Option<Gd<Monster>>,
+/// }
+/// ```
 ///
 /// ## Node based classes
 ///
@@ -154,7 +193,7 @@ use std::{fmt, ops};
 /// ## Resource based classes
 ///
 /// `#[export]` for a `DynGd<T, D>` allows you to limit the available choices to implementors of a given trait `D` whose base inherits the specified `T`
-/// (for example, `#[export] DynGd<Resource, dyn MyTrait>` won't include Rust classes with an Object base, even if they implement `MyTrait`).
+/// (for example, `#[export] Option<DynGd<Resource, dyn MyTrait>>` won't include Rust classes with an Object base, even if they implement `MyTrait`).
 pub struct DynGd<T, D>
 where
     // T does _not_ require AsDyn<D> here. Otherwise, it's impossible to upcast (without implementing the relation for all base classes).
@@ -292,22 +331,6 @@ where
     pub fn into_gd(self) -> Gd<T> {
         self.obj
     }
-
-    /// Implementation shared between `OnEditor<DynGd<T, D>>` and `Option<DynGd<T, D>>`.
-    #[doc(hidden)]
-    fn set_property(&mut self, value: <Self as GodotConvert>::Via)
-    where
-        D: 'static,
-    {
-        // `set_property` can't be delegated to Gd<T>, since we have to set `erased_obj` as well.
-        *self = <Self as FromGodot>::from_godot(value);
-    }
-
-    /// Implementation shared between `OnEditor<DynGd<T, D>>` and `Option<DynGd<T, D>>`.
-    #[doc(hidden)]
-    fn get_property(&self) -> <Self as GodotConvert>::Via {
-        self.obj.to_godot()
-    }
 }
 
 impl<T, D> DynGd<T, D>
@@ -519,21 +542,18 @@ where
     }
 }
 
-impl<T, D> Var for Option<DynGd<T, D>>
+impl<T, D> Var for DynGd<T, D>
 where
     T: GodotClass,
     D: ?Sized + 'static,
 {
     fn get_property(&self) -> Self::Via {
-        self.as_ref().map(|this| this.get_property())
+        self.obj.get_property()
     }
 
     fn set_property(&mut self, value: Self::Via) {
-        match (value, self.as_mut()) {
-            (Some(new_value), Some(current_value)) => current_value.set_property(new_value),
-            (Some(new_value), _) => *self = Some(<DynGd<T, D> as FromGodot>::from_godot(new_value)),
-            (None, _) => *self = None,
-        }
+        // `set_property` can't be delegated to Gd<T>, since we have to set `erased_obj` as well.
+        *self = <Self as FromGodot>::from_godot(value);
     }
 }
 
@@ -579,12 +599,12 @@ where
     D: ?Sized + 'static,
 {
     fn get_property(&self) -> Self::Via {
-        OnEditor::<DynGd<T, D>>::get_property_inner(self, <DynGd<T, D>>::get_property)
+        OnEditor::<DynGd<T, D>>::get_property_inner(self)
     }
 
     fn set_property(&mut self, value: Self::Via) {
         // `set_property` can't be delegated to Gd<T>, since we have to set `erased_obj` as well.
-        OnEditor::<DynGd<T, D>>::set_property_inner(self, value, <DynGd<T, D>>::set_property)
+        OnEditor::<DynGd<T, D>>::set_property_inner(self, value)
     }
 }
 

godot-core/src/obj/gd.rs

@@ -549,18 +549,6 @@ impl<T: GodotClass> Gd<T> {
         // Do not increment ref-count; assumed to be return value from FFI.
         sys::ptr_then(object_ptr, |ptr| Gd::from_obj_sys_weak(ptr))
     }
-
-    /// Implementation shared between `OnEditor<Gd<T>>` and `Option<Gd<T>`.
-    #[doc(hidden)]
-    fn set_property(&mut self, new_value: <Self as GodotConvert>::Via) {
-        *self = FromGodot::from_godot(new_value);
-    }
-
-    /// Implementation shared between `OnEditor<Gd<T>>` and `Option<Gd<T>`.
-    #[doc(hidden)]
-    fn get_property(&self) -> <Self as GodotConvert>::Via {
-        self.to_godot()
-    }
 }
 
 /// _The methods in this impl block are only available for objects `T` that are manually managed,
@@ -907,17 +895,13 @@ impl<T: GodotClass> Clone for Gd<T> {
     }
 }
 
-impl<T: GodotClass> Var for Option<Gd<T>> {
+impl<T: GodotClass> Var for Gd<T> {
     fn get_property(&self) -> Self::Via {
-        self.as_ref().map(<Gd<T>>::get_property)
+        self.to_godot()
     }
 
     fn set_property(&mut self, value: Self::Via) {
-        match (value, self.as_mut()) {
-            (Some(new_value), Some(current_value)) => current_value.set_property(new_value),
-            (Some(new_value), _) => *self = Some(<Gd<T> as FromGodot>::from_godot(new_value)),
-            (None, _) => *self = None,
-        }
+        *self = FromGodot::from_godot(value)
     }
 }
 
@@ -956,11 +940,11 @@ where
     OnEditor<Gd<T>>: GodotConvert<Via = Option<<Gd<T> as GodotConvert>::Via>>,
 {
     fn get_property(&self) -> Self::Via {
-        OnEditor::<Gd<T>>::get_property_inner(self, <Gd<T>>::get_property)
+        OnEditor::<Gd<T>>::get_property_inner(self)
     }
 
     fn set_property(&mut self, value: Self::Via) {
-        OnEditor::<Gd<T>>::set_property_inner(self, value, <Gd<T>>::set_property)
+        OnEditor::<Gd<T>>::set_property_inner(self, value)
     }
 }
 

godot-core/src/obj/oneditor.rs

@@ -11,7 +11,7 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// Exported property that must be initialized in the editor (or associated code) before use.
 ///
 /// Allows to use `Gd<T>`, which by itself never holds null objects, as an `#[export]` that should not be null during runtime.
-/// As such, it can be used as a more ergonomic way of `Option<Gd<T>>` which _assumes_ initialization.
+/// As such, it can be used as a more ergonomic version of `Option<Gd<T>>` which _assumes_ initialization.
 ///
 /// Panics during access if uninitialized.
 /// When used inside a node class, `OnEditor` checks if a value has been set before `ready()` is run, and panics otherwise.
@@ -147,6 +147,11 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// }
 /// ```
 ///
+/// # Using `OnEditor<T>` with `#[class(tool)]`
+///
+/// When used with `#[class(tool)]`, the before-ready checks are omitted.
+/// Otherwise, `OnEditor<T>` behaves the same — accessing an uninitialized value
+/// will cause a panic.
 pub struct OnEditor<T> {
     inner: OnEditorState<T>,
 }
@@ -160,7 +165,7 @@ pub(crate) enum OnEditorState<T> {
     Initialized(T),
 }
 
-impl<T: GodotConvert + FromGodot + PartialEq> OnEditor<T> {
+impl<T: Var + FromGodot + PartialEq> OnEditor<T> {
     /// Initializes invalid `OnEditor<T>` with given value.
     ///
     /// # Panics
@@ -183,7 +188,7 @@ impl<T: GodotConvert + FromGodot + PartialEq> OnEditor<T> {
     /// If this value is not changed in the editor, accessing it from Rust will cause a panic.
     pub fn new_invalid(val: T) -> Self
     where
-        T: BuiltinExport,
+        T::Via: BuiltinExport,
     {
         OnEditor {
             inner: OnEditorState::Uninitialized(val),
@@ -208,14 +213,11 @@ impl<T: GodotConvert + FromGodot + PartialEq> OnEditor<T> {
     }
 
     /// `Var::get_property` implementation that works both for nullable and non-nullable types.
-    pub(crate) fn get_property_inner(
-        &self,
-        get_property_fn: impl FnOnce(&T) -> T::Via,
-    ) -> Option<T::Via> {
+    pub(crate) fn get_property_inner(&self) -> Option<T::Via> {
         match &self.inner {
             OnEditorState::Null => None,
             OnEditorState::Uninitialized(val) | OnEditorState::Initialized(val) => {
-                Some(get_property_fn(val))
+                Some(val.get_property())
             }
         }
     }
@@ -224,17 +226,11 @@ impl<T: GodotConvert + FromGodot + PartialEq> OnEditor<T> {
     ///
     /// All the state transitions are valid, since it is being run only in the editor.
     /// See also [`Option::set_property()`].
-    pub(crate) fn set_property_inner(
-        &mut self,
-        value: Option<T::Via>,
-        set_property_fn: impl FnOnce(&mut T, T::Via),
-    ) where
-        T::Via: PartialEq,
-    {
+    pub(crate) fn set_property_inner(&mut self, value: Option<T::Via>) {
         match (value, &mut self.inner) {
             (None, _) => self.inner = OnEditorState::Null,
             (Some(value), OnEditorState::Initialized(current_value)) => {
-                set_property_fn(current_value, value)
+                current_value.set_property(value);
             }
             (Some(value), OnEditorState::Null) => {
                 self.inner = OnEditorState::Initialized(FromGodot::from_godot(value))
@@ -274,31 +270,33 @@ impl<T> std::ops::DerefMut for OnEditor<T> {
 
 impl<T> GodotConvert for OnEditor<T>
 where
-    T: GodotType + GodotConvert<Via = T> + BuiltinExport,
+    T: GodotConvert,
+    T::Via: GodotType + BuiltinExport,
 {
     type Via = T::Via;
 }
 
 impl<T> Var for OnEditor<T>
 where
-    OnEditor<T>: GodotConvert<Via = T>,
-    T: GodotConvert<Via = T> + BuiltinExport + Var + FromGodot + PartialEq,
+    OnEditor<T>: GodotConvert<Via = T::Via>,
+    T: Var + FromGodot + PartialEq,
+    T::Via: BuiltinExport,
 {
     fn get_property(&self) -> Self::Via {
         // Will never fail – `PrimitiveGodotType` can not be represented by the `OnEditorState::Null`.
-        OnEditor::<T>::get_property_inner(self, T::get_property)
-            .expect("DirectExport is not nullable.")
+        OnEditor::<T>::get_property_inner(self).expect("DirectExport is not nullable.")
     }
 
-    fn set_property(&mut self, value: T) {
-        OnEditor::<T>::set_property_inner(self, Some(value), T::set_property);
+    fn set_property(&mut self, value: T::Via) {
+        OnEditor::<T>::set_property_inner(self, Some(value));
     }
 }
 
 impl<T> Export for OnEditor<T>
 where
     OnEditor<T>: Var,
-    T: GodotConvert<Via = T> + BuiltinExport + Export,
+    T: GodotConvert + Export,
+    T::Via: BuiltinExport,
 {
     fn export_hint() -> PropertyHintInfo {
         T::export_hint()

godot-core/src/registry/property.rs

@@ -10,7 +10,7 @@
 use crate::classes;
 use crate::global::PropertyHint;
 use godot_ffi as sys;
-use godot_ffi::VariantType;
+use godot_ffi::{GodotNullableFfi, VariantType};
 use std::fmt::Display;
 
 use crate::meta::{ClassName, FromGodot, GodotConvert, GodotType, PropertyHintInfo, ToGodot};
@@ -88,14 +88,22 @@ pub trait Export: Var {
 /// Provides a few blanket implementations and, by itself, has no implications
 /// for the [`Var`] or [`Export`] traits.
 ///
-/// Some Godot Types which are inherently non-nullable (e.g., `Gd<T>`),
-/// might have their value set to null by the editor. Additionally, Godot must generate
-/// initial, default value for such properties, causing memory leaks.
-///
-/// Non-algebraic types that don't implement `BuiltinExport` shouldn't be used directly as an `#[export]`
+/// Types which don't implement the `BuiltinExport` trait can't be used directly as an `#[export]`
 /// and must be handled using associated algebraic types, such as:
 /// * [`Option<T>`], which represents optional value that can be null when used.
 /// * [`OnEditor<T>`][crate::obj::OnEditor], which represents value that must not be null when used.
+// Some Godot Types which are inherently non-nullable (e.g., `Gd<T>`),
+// might have their value set to null by the editor. Additionally, Godot must generate
+// initial, default value for such properties, causing memory leaks.
+// Such `GodotType`s don't implement `BuiltinExport`.
+//
+// Note: This marker trait is required to create a blanket implementation
+// for `OnEditor<T>` where `T` is anything other than `GodotClass`.
+// An alternative approach would involve introducing an extra associated type
+// to `GodotType` trait. However, this would not be ideal — `GodotType` is used
+// in contexts unrelated to `#[export]`, and adding unnecessary complexity
+// should be avoided. Since Rust does not yet support specialization (i.e. negative trait bounds),
+// this `MarkerTrait` serves as the intended solution to recognize aforementioned types.
 pub trait BuiltinExport {}
 
 /// This function only exists as a place to add doc-tests for the `Export` trait.
@@ -126,6 +134,30 @@ pub trait BuiltinExport {}
 /// }
 /// ```
 ///
+/// Neither `Gd<T>` nor `DynGd<T, D>` can be used with an `#[export]` directly:
+///
+/// ```compile_fail
+///  use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// #[class(init, base = Node)]
+/// struct MyClass {
+///     #[export]
+///     editor_property: Gd<Resource>,
+/// }
+/// ```
+///
+/// ```compile_fail
+///  use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// #[class(init, base = Node)]
+/// struct MyClass {
+///     #[export]
+///     editor_property: DynGd<Node, dyn Display>,
+/// }
+/// ```
+///
 /// ```compile_fail
 /// use godot::prelude::*;
 ///
@@ -175,7 +207,12 @@ where
     }
 }
 
-impl<T> BuiltinExport for Option<T> {}
+impl<T> BuiltinExport for Option<T>
+where
+    T: GodotType,
+    T::Ffi: GodotNullableFfi,
+{
+}
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Export machinery