Merge pull request #998 from Yarwin/derive-var-and-export-for-dyn-gd

Implement `Var` and `Export` for `DynGd<T, D>`

godot-core/src/obj/dyn_gd.rs

@@ -7,10 +7,11 @@
 
 use crate::builtin::Variant;
 use crate::meta::error::ConvertError;
-use crate::meta::{FromGodot, GodotConvert, ToGodot};
+use crate::meta::{ClassName, FromGodot, GodotConvert, PropertyHintInfo, ToGodot};
 use crate::obj::guards::DynGdRef;
 use crate::obj::{bounds, AsDyn, Bounds, DynGdMut, Gd, GodotClass, Inherits};
-use crate::registry::class::try_dynify_object;
+use crate::registry::class::{get_dyn_property_hint_string, try_dynify_object};
+use crate::registry::property::{Export, Var};
 use crate::{meta, sys};
 use std::{fmt, ops};
 
@@ -478,3 +479,34 @@ where
     D: ?Sized + 'static,
 {
 }
+
+impl<T, D> Var for DynGd<T, D>
+where
+    T: GodotClass,
+    D: ?Sized + 'static,
+{
+    fn get_property(&self) -> Self::Via {
+        self.obj.get_property()
+    }
+
+    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.
+        *self = <Self as FromGodot>::from_godot(value);
+    }
+}
+
+impl<T, D> Export for DynGd<T, D>
+where
+    T: GodotClass + Bounds<Exportable = bounds::Yes>,
+    D: ?Sized + 'static,
+{
+    fn export_hint() -> PropertyHintInfo {
+        PropertyHintInfo {
+            hint_string: get_dyn_property_hint_string::<D>(),
+            ..<Gd<T> as Export>::export_hint()
+        }
+    }
+    fn as_node_class() -> Option<ClassName> {
+        <Gd<T> as Export>::as_node_class()
+    }
+}

godot-core/src/registry/class.rs

@@ -5,17 +5,19 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use godot_ffi::join_with;
 use std::collections::HashMap;
 use std::{any, ptr};
 
+use crate::builtin::GString;
 use crate::init::InitLevel;
 use crate::meta::error::{ConvertError, FromGodotError};
 use crate::meta::ClassName;
 use crate::obj::{cap, DynGd, Gd, GodotClass};
 use crate::private::{ClassPlugin, PluginItem};
 use crate::registry::callbacks;
 use crate::registry::plugin::{ErasedDynifyFn, ErasedRegisterFn, InherentImpl};
-use crate::{classes, godot_error, sys};
+use crate::{classes, godot_error, godot_warn, sys};
 use sys::{interface_fn, out, Global, GlobalGuard, GlobalLockError};
 
 /// Returns a lock to a global map of loaded classes, by initialization level.
@@ -215,11 +217,30 @@ pub fn auto_register_classes(init_level: InitLevel) {
         fill_class_info(elem.item.clone(), class_info);
     });
 
+    // First register all the loaded classes and dyn traits.
+    // We need all the dyn classes in the registry to properly register DynGd properties;
+    // one can do it directly inside the loop – by locking and unlocking the mutex –
+    // but it is much slower and doesn't guarantee that all the dependent classes will be already loaded in most cases.
+    register_classes_and_dyn_traits(&mut map, init_level);
+
+    // actually register all the classes
+    for info in map.into_values() {
+        register_class_raw(info);
+        out!("Class {class_name} loaded.");
+    }
+
+    out!("All classes for level `{init_level:?}` auto-registered.");
+}
+
+fn register_classes_and_dyn_traits(
+    map: &mut HashMap<ClassName, ClassRegistrationInfo>,
+    init_level: InitLevel,
+) {
     let mut loaded_classes_by_level = global_loaded_classes_by_init_level();
     let mut loaded_classes_by_name = global_loaded_classes_by_name();
     let mut dyn_traits_by_typeid = global_dyn_traits_by_typeid();
 
-    for mut info in map.into_values() {
+    for info in map.values_mut() {
         let class_name = info.class_name;
         out!("Register class:   {class_name} at level `{init_level:?}`");
 
@@ -246,13 +267,7 @@ pub fn auto_register_classes(init_level: InitLevel) {
             .push(loaded_class);
 
         loaded_classes_by_name.insert(class_name, metadata);
-
-        register_class_raw(info);
-
-        out!("Class {class_name} loaded.");
     }
-
-    out!("All classes for level `{init_level:?}` auto-registered.");
 }
 
 pub fn unregister_classes(init_level: InitLevel) {
@@ -334,6 +349,38 @@ pub(crate) fn try_dynify_object<T: GodotClass, D: ?Sized + 'static>(
     Err(error.into_error(object))
 }
 
+/// Responsible for creating hint_string for [`DynGd<T, D>`][crate::obj::DynGd] properties which works with [`PropertyHint::NODE_TYPE`][crate::global::PropertyHint::NODE_TYPE] or [`PropertyHint::RESOURCE_TYPE`][crate::global::PropertyHint::RESOURCE_TYPE].
+///
+/// Godot offers very limited capabilities when it comes to validating properties in the editor if given class isn't a tool.
+/// Proper hint string combined with `PropertyHint::NODE_TYPE` or `PropertyHint::RESOURCE_TYPE` allows to limit selection only to valid classes - those registered as implementors of given `DynGd<T, D>`'s `D` trait.
+///
+/// See also [Godot docs for PropertyHint](https://docs.godotengine.org/en/stable/classes/[email protected]#enum-globalscope-propertyhint).
+pub(crate) fn get_dyn_property_hint_string<D>() -> GString
+where
+    D: ?Sized + 'static,
+{
+    let typeid = any::TypeId::of::<D>();
+    let dyn_traits_by_typeid = global_dyn_traits_by_typeid();
+    let Some(relations) = dyn_traits_by_typeid.get(&typeid) else {
+        let trait_name = sys::short_type_name::<D>();
+        godot_warn!(
+            "godot-rust: No class has been linked to trait {trait_name} with #[godot_dyn]."
+        );
+        return GString::default();
+    };
+    assert!(
+        !relations.is_empty(),
+        "Trait {trait_name} has been registered as DynGd Trait \
+        despite no class being related to it \n\
+        **this is a bug, please report it**",
+        trait_name = sys::short_type_name::<D>()
+    );
+
+    GString::from(join_with(relations.iter(), ", ", |relation| {
+        relation.implementing_class_name.to_cow_str()
+    }))
+}
+
 /// Populate `c` with all the relevant data from `component` (depending on component type).
 fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
     c.validate_unique(&item);

godot-ffi/src/toolbox.rs

@@ -8,6 +8,7 @@
 //! Functions and macros that are not very specific to gdext, but come in handy.
 
 use crate as sys;
+use std::fmt::{Display, Write};
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Macros
@@ -162,21 +163,25 @@ where
     join_with(iter, ", ", |item| format!("{item:?}"))
 }
 
-pub fn join_with<T, I, F>(mut iter: I, sep: &str, mut format_elem: F) -> String
+pub fn join_with<T, I, F, S>(mut iter: I, sep: &str, mut format_elem: F) -> String
 where
     I: Iterator<Item = T>,
-    F: FnMut(&T) -> String,
+    F: FnMut(&T) -> S,
+    S: Display,
 {
     let mut result = String::new();
 
     if let Some(first) = iter.next() {
-        result.push_str(&format_elem(&first));
+        // write! propagates error only if given formatter fails.
+        // String formatting by itself is an infallible operation.
+        // Read more at: https://doc.rust-lang.org/stable/std/fmt/index.html#formatting-traits
+        write!(&mut result, "{first}", first = format_elem(&first))
+            .expect("Formatter should not fail!");
         for item in iter {
-            result.push_str(sep);
-            result.push_str(&format_elem(&item));
+            write!(&mut result, "{sep}{item}", item = format_elem(&item))
+                .expect("Formatter should not fail!");
         }
     }
-
     result
 }
 

itest/godot/ManualFfiTests.gd

@@ -68,6 +68,36 @@ func test_export():
 	obj.free()
 	node.free()
 
+func test_export_dyn_gd():
+	var dyn_gd_exporter = RefcDynGdExporter.new()
+
+	# NodeHealth is valid candidate both for `empty` and `second` fields.
+	var node = NodeHealth.new()
+	dyn_gd_exporter.first = node
+	assert_eq(dyn_gd_exporter.first, node)
+
+	dyn_gd_exporter.second = node
+	assert_eq(dyn_gd_exporter.second, node)
+
+	# RefcHealth is valid candidate for `first` field.
+	var refc = RefcHealth.new()
+	dyn_gd_exporter.first = refc
+	assert_eq(dyn_gd_exporter.first, refc)
+	node.free()
+
+func test_export_dyn_gd_should_fail_for_wrong_type():
+	if runs_release():
+		return
+
+	var dyn_gd_exporter = RefcDynGdExporter.new()
+	var refc = RefcHealth.new()
+
+	disable_error_messages()
+	dyn_gd_exporter.second = refc # Should fail.
+	enable_error_messages()
+
+	assert_fail("`DynGdExporter.second` should only accept NodeHealth and only if it implements `InstanceIdProvider` trait")
+
 class MockObjGd extends Object:
 	var i: int = 0
 

itest/rust/src/object_tests/dyn_gd_test.rs

@@ -445,3 +445,15 @@ impl InstanceIdProvider for foreign::NodeHealth {
         self.base().instance_id()
     }
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Check if DynGd can be properly exported
+
+#[derive(GodotClass)]
+#[class(init)]
+struct RefcDynGdExporter {
+    #[var]
+    first: Option<DynGd<Object, dyn Health>>,
+    #[export]
+    second: Option<DynGd<foreign::NodeHealth, dyn InstanceIdProvider>>,
+}