From 57ad44e034d4eccbffde88a69577db8340d4cc55 Mon Sep 17 00:00:00 2001 From: Henry Conklin Date: Thu, 5 Oct 2023 18:10:55 -0400 Subject: [PATCH 1/2] Use the class_name for the field when building PropertyInfo Godot uses the class_name to construct an instance of the right type in a few places, in particular for the PROPERTY_USAGE_EDITOR_INSTANTIATE_OBJECT flag. --- .../src/class/data_models/property.rs | 8 +- godot-macros/src/util/mod.rs | 10 ++ itest/rust/src/object_tests/property_test.rs | 110 +++++++++++++++--- 3 files changed, 112 insertions(+), 16 deletions(-) diff --git a/godot-macros/src/class/data_models/property.rs b/godot-macros/src/class/data_models/property.rs index 734ab83d5..d843d62a2 100644 --- a/godot-macros/src/class/data_models/property.rs +++ b/godot-macros/src/class/data_models/property.rs @@ -67,6 +67,8 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { continue; }; + let field_variant_type = util::property_variant_type(field_type); + let field_class_name = util::property_variant_class_name(field_type); let field_name = field_ident.to_string(); // rustfmt wont format this if we put it in the let-else. @@ -178,8 +180,8 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { let usage = #usage_flags; let property_info = ::godot::builtin::meta::PropertyInfo { - variant_type: <<#field_type as ::godot::bind::property::Property>::Intermediate as ::godot::builtin::meta::VariantMetadata>::variant_type(), - class_name: #class_name_obj, + variant_type: #field_variant_type, + class_name: #field_class_name, property_name: #field_name.into(), hint, hint_string, @@ -194,7 +196,7 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { unsafe { ::godot::sys::interface_fn!(classdb_register_extension_class_property)( ::godot::sys::get_library(), - #class_name::class_name().string_sys(), + #class_name_obj.string_sys(), std::ptr::addr_of!(property_info_sys), setter_name.string_sys(), getter_name.string_sys(), diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index d24ecf8f1..22738be05 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -32,6 +32,16 @@ pub fn class_name_obj(class: &impl ToTokens) -> TokenStream { quote! { <#class as ::godot::obj::GodotClass>::class_name() } } +pub fn property_variant_type(property_type: &impl ToTokens) -> TokenStream { + let property_type = property_type.to_token_stream(); + quote! {<<#property_type as ::godot::bind::property::Property>::Intermediate as ::godot::builtin::meta::VariantMetadata>::variant_type()} +} + +pub fn property_variant_class_name(property_type: &impl ToTokens) -> TokenStream { + let property_type = property_type.to_token_stream(); + quote! {<<#property_type as ::godot::bind::property::Property>::Intermediate as ::godot::builtin::meta::VariantMetadata>::class_name()} +} + pub fn bail_fn(msg: impl AsRef, tokens: T) -> ParseResult where T: Spanned, diff --git a/itest/rust/src/object_tests/property_test.rs b/itest/rust/src/object_tests/property_test.rs index 01f28322c..494807cfc 100644 --- a/itest/rust/src/object_tests/property_test.rs +++ b/itest/rust/src/object_tests/property_test.rs @@ -6,7 +6,10 @@ use godot::{ bind::property::ExportInfo, - engine::{global::PropertyHint, Texture}, + engine::{ + global::{PropertyHint, PropertyUsageFlags}, + Texture, + }, prelude::*, test::itest, }; @@ -351,20 +354,101 @@ fn derive_export() { .iter_shared() .find(|c| c.get_or_nil("name") == "foo".to_variant()) .unwrap(); - assert_eq!( - property.get_or_nil("class_name"), - "DeriveExport".to_variant() + // `class_name` should be empty for non-Object variants. + check_property(&property, "class_name", ""); + check_property(&property, "type", VariantType::Int as i32); + check_property(&property, "hint", PropertyHint::PROPERTY_HINT_ENUM.ord()); + check_property(&property, "hint_string", "A:0,B:1,C:2"); + check_property( + &property, + "usage", + PropertyUsageFlags::PROPERTY_USAGE_DEFAULT.ord(), ); - assert_eq!( - property.get_or_nil("type"), - (VariantType::Int as i32).to_variant() +} + +#[derive(GodotClass)] +#[class(init, base=Resource)] +pub struct CustomResource {} + +#[godot_api] +impl CustomResource {} + +#[godot_api] +impl ResourceVirtual for CustomResource {} + +#[derive(GodotClass)] +#[class(init, base=Resource, rename=NewNameCustomResource)] +pub struct RenamedCustomResource {} + +#[godot_api] +impl RenamedCustomResource {} + +#[godot_api] +impl ResourceVirtual for RenamedCustomResource {} + +#[derive(GodotClass)] +#[class(init, base=Node)] +pub struct ExportResource { + #[export] + #[var(usage_flags=[PROPERTY_USAGE_DEFAULT, PROPERTY_USAGE_EDITOR_INSTANTIATE_OBJECT])] + pub foo: Option>, + + #[export] + pub bar: Option>, +} + +#[godot_api] +impl ExportResource {} + +#[godot_api] +impl NodeVirtual for ExportResource {} + +#[itest] +fn export_resource() { + let class: Gd = Gd::new_default(); + + let property = class + .get_property_list() + .iter_shared() + .find(|c| c.get_or_nil("name") == "foo".to_variant()) + .unwrap(); + check_property(&property, "class_name", "CustomResource"); + check_property(&property, "type", VariantType::Object as i32); + check_property( + &property, + "hint", + PropertyHint::PROPERTY_HINT_RESOURCE_TYPE.ord(), + ); + check_property(&property, "hint_string", "CustomResource"); + check_property( + &property, + "usage", + PropertyUsageFlags::PROPERTY_USAGE_DEFAULT.ord() + | PropertyUsageFlags::PROPERTY_USAGE_EDITOR_INSTANTIATE_OBJECT.ord(), ); - assert_eq!( - property.get_or_nil("hint"), - (PropertyHint::PROPERTY_HINT_ENUM.ord()).to_variant() + + let property = class + .get_property_list() + .iter_shared() + .find(|c| c.get_or_nil("name") == "bar".to_variant()) + .unwrap(); + check_property(&property, "class_name", "NewNameCustomResource"); + check_property(&property, "type", VariantType::Object as i32); + check_property( + &property, + "hint", + PropertyHint::PROPERTY_HINT_RESOURCE_TYPE.ord(), ); - assert_eq!( - property.get_or_nil("hint_string"), - "A:0,B:1,C:2".to_variant() + check_property(&property, "hint_string", "NewNameCustomResource"); + check_property( + &property, + "usage", + PropertyUsageFlags::PROPERTY_USAGE_DEFAULT.ord(), ); + + class.free(); +} + +fn check_property(property: &Dictionary, key: &str, expected: impl ToVariant) { + assert_eq!(property.get_or_nil(key), expected.to_variant()); } From f7217c27c96f2c45d6407cbf5588739d31103bd4 Mon Sep 17 00:00:00 2001 From: Henry Conklin Date: Fri, 6 Oct 2023 09:38:23 -0400 Subject: [PATCH 2/2] Clippy fixes for Rust v1.73.0 --- godot-core/src/builtin/macros.rs | 22 +++++++++------------- godot-macros/src/util/list_parser.rs | 2 +- itest/rust/src/framework/runner.rs | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/godot-core/src/builtin/macros.rs b/godot-core/src/builtin/macros.rs index 26e4214ce..2f8d52b63 100644 --- a/godot-core/src/builtin/macros.rs +++ b/godot-core/src/builtin/macros.rs @@ -68,10 +68,10 @@ macro_rules! impl_builtin_traits_inner { impl Eq for $Type {} }; - ( PartialOrd for $Type:ty => $gd_method:ident ) => { - impl PartialOrd for $Type { + ( Ord for $Type:ty => $gd_method:ident ) => { + impl Ord for $Type { #[inline] - fn partial_cmp(&self, other: &Self) -> Option { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { let op_less = |lhs, rhs| unsafe { let mut result = false; ::godot_ffi::builtin_call! { @@ -81,22 +81,18 @@ macro_rules! impl_builtin_traits_inner { }; if op_less(self.sys(), other.sys()) { - Some(std::cmp::Ordering::Less) + std::cmp::Ordering::Less } else if op_less(other.sys(), self.sys()) { - Some(std::cmp::Ordering::Greater) + std::cmp::Ordering::Greater } else { - Some(std::cmp::Ordering::Equal) + std::cmp::Ordering::Equal } } } - }; - - ( Ord for $Type:ty => $gd_method:ident ) => { - impl_builtin_traits_inner!(PartialOrd for $Type => $gd_method); - impl Ord for $Type { + impl PartialOrd for $Type { #[inline] - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - PartialOrd::partial_cmp(self, other).expect("PartialOrd::partial_cmp") + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) } } }; diff --git a/godot-macros/src/util/list_parser.rs b/godot-macros/src/util/list_parser.rs index 9f7866fe2..70149efdc 100644 --- a/godot-macros/src/util/list_parser.rs +++ b/godot-macros/src/util/list_parser.rs @@ -167,7 +167,7 @@ impl ListParser { /// Take the next element of the list, if it is a key-value pair of the form `key = expression`. pub(crate) fn try_next_key_value(&mut self) -> Option<(Ident, KvValue)> { - let Some(kv) = self.peek() else { return None }; + let kv = self.peek()?; if let Ok((key, value)) = kv.as_key_value() { _ = self.pop_next(); diff --git a/itest/rust/src/framework/runner.rs b/itest/rust/src/framework/runner.rs index 1d96b04a8..878046afd 100644 --- a/itest/rust/src/framework/runner.rs +++ b/itest/rust/src/framework/runner.rs @@ -364,7 +364,7 @@ fn get_execution_time(test: &Variant) -> Option { fn get_errors(test: &Variant) -> Array { test.call("get", &["errors".to_variant()]) .try_to::>() - .unwrap_or(Array::new()) + .unwrap_or_default() } #[must_use]