From 7de157003280741f39d97a8cd1b7fa52fd8b62a2 Mon Sep 17 00:00:00 2001 From: Yarvin Date: Mon, 6 Jan 2025 11:59:13 +0100 Subject: [PATCH] Derive Var and Export for DynGd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refactor `join_with` function to make it usable with &str and `Cow<'static, str>', make use of formatting with a `write!(…)` macro - Split class registration process in two - Derive Var and Export for `DynGd` – Use `PropertyHint::NODE_TYPE` and `PropertyHint::RESOURCE_TYPE` to mark what can and what can't be exported via the editor - Add tests --- godot-core/src/obj/dyn_gd.rs | 36 ++++++++++++- godot-core/src/registry/class.rs | 63 +++++++++++++++++++--- godot-ffi/src/toolbox.rs | 17 +++--- itest/godot/ManualFfiTests.gd | 30 +++++++++++ itest/rust/src/object_tests/dyn_gd_test.rs | 12 +++++ 5 files changed, 142 insertions(+), 16 deletions(-) diff --git a/godot-core/src/obj/dyn_gd.rs b/godot-core/src/obj/dyn_gd.rs index 96611ea76..6f0d3591e 100644 --- a/godot-core/src/obj/dyn_gd.rs +++ b/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 Var for DynGd +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, since we have to set `erased_obj` as well. + *self = ::from_godot(value); + } +} + +impl Export for DynGd +where + T: GodotClass + Bounds, + D: ?Sized + 'static, +{ + fn export_hint() -> PropertyHintInfo { + PropertyHintInfo { + hint_string: get_dyn_property_hint_string::(), + .. as Export>::export_hint() + } + } + fn as_node_class() -> Option { + as Export>::as_node_class() + } +} diff --git a/godot-core/src/registry/class.rs b/godot-core/src/registry/class.rs index 45a381281..31d852677 100644 --- a/godot-core/src/registry/class.rs +++ b/godot-core/src/registry/class.rs @@ -5,9 +5,11 @@ * 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; @@ -15,7 +17,7 @@ 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. @@ -218,11 +220,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, + 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:?}`"); @@ -249,13 +270,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) { @@ -337,6 +352,38 @@ pub(crate) fn try_dynify_object( Err(error.into_error(object)) } +/// Responsible for creating hint_string for [`DynGd`][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`'s `D` trait. +/// +/// See also [Godot docs for PropertyHint](https://docs.godotengine.org/en/stable/classes/class_@globalscope.html#enum-globalscope-propertyhint). +pub(crate) fn get_dyn_property_hint_string() -> GString +where + D: ?Sized + 'static, +{ + let typeid = any::TypeId::of::(); + 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::(); + 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::() + ); + + 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); diff --git a/godot-ffi/src/toolbox.rs b/godot-ffi/src/toolbox.rs index 1d1f82164..806c0fd03 100644 --- a/godot-ffi/src/toolbox.rs +++ b/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(mut iter: I, sep: &str, mut format_elem: F) -> String +pub fn join_with(mut iter: I, sep: &str, mut format_elem: F) -> String where I: Iterator, - 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 } diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index 8eeaa9c95..88023ad07 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/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 diff --git a/itest/rust/src/object_tests/dyn_gd_test.rs b/itest/rust/src/object_tests/dyn_gd_test.rs index 72394acfb..777694287 100644 --- a/itest/rust/src/object_tests/dyn_gd_test.rs +++ b/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>, + #[export] + second: Option>, +}