From 64b764eb463d46a02daaf8097b666a6df62c6f05 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Wed, 22 Mar 2023 22:56:37 +0100 Subject: [PATCH] More extensive tests of #[func] passing between GDScript and Rust --- godot-core/src/builtin/array.rs | 15 ++++++- godot-core/src/builtin/meta/mod.rs | 45 +++++++------------- godot-core/src/builtin/node_path.rs | 2 + godot-core/src/builtin/variant/impls.rs | 18 +++++++- godot-macros/src/derive_godot_class.rs | 16 +++---- itest/godot/TestSuite.gd | 8 ++-- itest/godot/input/GenFfiTests.template.gd | 27 +++++++++--- itest/rust/build.rs | 51 ++++++++++++++++++----- 8 files changed, 122 insertions(+), 60 deletions(-) diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index d13ce4566..61f09c0d1 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -855,7 +855,7 @@ macro_rules! varray { /// [`set_typed`](https://docs.godotengine.org/en/latest/classes/class_array.html#class-array-method-set-typed). /// /// We ignore the `script` parameter because it has no impact on typing in Godot. -#[derive(Debug, PartialEq, Eq)] +#[derive(PartialEq, Eq)] struct TypeInfo { variant_type: VariantType, class_name: StringName, @@ -880,3 +880,16 @@ impl TypeInfo { self.variant_type != VariantType::Nil } } + +impl fmt::Debug for TypeInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let class = self.class_name.to_string(); + let class_str = if class.is_empty() { + String::new() + } else { + format!(" (class={class})") + }; + + write!(f, "{:?}{}", self.variant_type, class_str) + } +} diff --git a/godot-core/src/builtin/meta/mod.rs b/godot-core/src/builtin/meta/mod.rs index 9006a8b84..534d8d31a 100644 --- a/godot-core/src/builtin/meta/mod.rs +++ b/godot-core/src/builtin/meta/mod.rs @@ -27,13 +27,14 @@ pub trait VariantMetadata { } fn property_info(property_name: &str) -> PropertyInfo { - PropertyInfo::new( - Self::variant_type(), - Self::class_name(), - StringName::from(property_name), - global::PropertyHint::PROPERTY_HINT_NONE, - GodotString::new(), - ) + PropertyInfo { + variant_type: Self::variant_type(), + class_name: Self::class_name(), + property_name: StringName::from(property_name), + hint: global::PropertyHint::PROPERTY_HINT_NONE, + hint_string: GodotString::new(), + usage: global::PropertyUsageFlags::PROPERTY_USAGE_DEFAULT, + } } fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { @@ -52,33 +53,17 @@ impl VariantMetadata for Option { /// Rusty abstraction of sys::GDExtensionPropertyInfo /// Keeps the actual allocated values (the sys equivalent only keeps pointers, which fall out of scope) #[derive(Debug)] +// Note: is not #[non_exhaustive], so adding fields is a breaking change. Mostly used internally at the moment though. pub struct PropertyInfo { - variant_type: VariantType, - class_name: ClassName, - property_name: StringName, - hint: global::PropertyHint, - hint_string: GodotString, - usage: global::PropertyUsageFlags, + pub variant_type: VariantType, + pub class_name: ClassName, + pub property_name: StringName, + pub hint: global::PropertyHint, + pub hint_string: GodotString, + pub usage: global::PropertyUsageFlags, } impl PropertyInfo { - pub fn new( - variant_type: VariantType, - class_name: ClassName, - property_name: StringName, - hint: global::PropertyHint, - hint_string: GodotString, - ) -> Self { - Self { - variant_type, - class_name, - property_name, - hint, - hint_string, - usage: global::PropertyUsageFlags::PROPERTY_USAGE_DEFAULT, - } - } - /// Converts to the FFI type. Keep this object allocated while using that! pub fn property_sys(&self) -> sys::GDExtensionPropertyInfo { use crate::obj::EngineEnum as _; diff --git a/godot-core/src/builtin/node_path.rs b/godot-core/src/builtin/node_path.rs index 8bfbbe603..1f5ae7f03 100644 --- a/godot-core/src/builtin/node_path.rs +++ b/godot-core/src/builtin/node_path.rs @@ -80,5 +80,7 @@ impl_builtin_traits! { Default => node_path_construct_default; Clone => node_path_construct_copy; Drop => node_path_destroy; + Eq => node_path_operator_equal; + // Ord => node_path_operator_less; } } diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index 177098d21..58b05d0a8 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -5,8 +5,9 @@ */ use super::*; -use crate::builtin::meta::VariantMetadata; +use crate::builtin::meta::{PropertyInfo, VariantMetadata}; use crate::builtin::*; +use crate::engine::global; use crate::obj::EngineEnum; use godot_ffi as sys; use sys::GodotFfi; @@ -224,6 +225,21 @@ impl VariantMetadata for Variant { // Arrays use the `NIL` type to indicate that they are untyped. VariantType::Nil } + + fn property_info(property_name: &str) -> PropertyInfo { + PropertyInfo { + variant_type: Self::variant_type(), + class_name: Self::class_name(), + property_name: StringName::from(property_name), + hint: global::PropertyHint::PROPERTY_HINT_NONE, + hint_string: GodotString::new(), + usage: global::PropertyUsageFlags::PROPERTY_USAGE_NIL_IS_VARIANT, + } + } + + fn param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { + sys::GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_INT8 + } } impl ToVariant for T { diff --git a/godot-macros/src/derive_godot_class.rs b/godot-macros/src/derive_godot_class.rs index 8660c0f2d..8407ae78e 100644 --- a/godot-macros/src/derive_godot_class.rs +++ b/godot-macros/src/derive_godot_class.rs @@ -377,14 +377,14 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream { use ::godot::builtin::meta::VariantMetadata; let class_name = ::godot::builtin::StringName::from(#class_name::CLASS_NAME); - - let property_info = ::godot::builtin::meta::PropertyInfo::new( - <#field_type>::variant_type(), - ::godot::builtin::meta::ClassName::of::<#class_name>(), - ::godot::builtin::StringName::from(#field_name), - ::godot::engine::global::PropertyHint::#hint_type, - ::godot::builtin::GodotString::from(#description), - ); + let property_info = ::godot::builtin::meta::PropertyInfo { + variant_type: <#field_type>::variant_type(), + class_name: ::godot::builtin::meta::ClassName::of::<#class_name>(), + property_name: ::godot::builtin::StringName::from(#name), + hint: ::godot::engine::global::PropertyHint::#hint_type, + hint_string: GodotString::from(#description), + usage: ::godot::engine::global::PropertyUsageFlags::PROPERTY_USAGE_DEFAULT, + }; let property_info_sys = property_info.property_sys(); let getter_name = ::godot::builtin::StringName::from(#getter_name); diff --git a/itest/godot/TestSuite.gd b/itest/godot/TestSuite.gd index bde7b4b3c..11a4be253 100644 --- a/itest/godot/TestSuite.gd +++ b/itest/godot/TestSuite.gd @@ -15,9 +15,9 @@ func assert_that(what: bool, message: String = "") -> bool: _assertion_failed = true if message: - print("assertion failed: %s" % message) + print("GDScript assertion failed: %s" % message) else: - print("assertion failed") + print("GDScript assertion failed.") return false func assert_eq(left, right, message: String = "") -> bool: @@ -26,7 +26,7 @@ func assert_eq(left, right, message: String = "") -> bool: _assertion_failed = true if message: - print("assertion failed: %s\n left: %s\n right: %s" % [message, left, right]) + print("GDScript assertion failed: %s\n left: %s\n right: %s" % [message, left, right]) else: - print("assertion failed: `(left == right)`\n left: %s\n right: %s" % [left, right]) + print("GDScript assertion failed: `(left == right)`\n left: %s\n right: %s" % [left, right]) return false diff --git a/itest/godot/input/GenFfiTests.template.gd b/itest/godot/input/GenFfiTests.template.gd index 91ee508f9..6a820b089 100644 --- a/itest/godot/input/GenFfiTests.template.gd +++ b/itest/godot/input/GenFfiTests.template.gd @@ -4,14 +4,31 @@ extends TestSuite +# Note: GDScript only uses ptrcalls if it has the full type information available at "compile" (parse) time. +# That includes all arguments (including receiver) as well as function signature (parameters + return type). +# Otherwise, GDScript will use varcall. Both are tested below. +# It is thus important that `ffi` is initialized using = for varcalls, and using := for ptrcalls. + #( func test_varcall_IDENT(): var ffi = GenFfi.new() - var from_rust = ffi.return_IDENT() - assert_that(ffi.accept_IDENT(from_rust)) + var from_rust: Variant = ffi.return_IDENT() + assert_that(ffi.accept_IDENT(from_rust), "ffi.accept_IDENT(from_rust)") + + var from_gdscript: Variant = VAL + var mirrored: Variant = ffi.mirror_IDENT(from_gdscript) + assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript") +#) + +#( +func test_ptrcall_IDENT(): + var ffi := GenFfi.new() + + var from_rust: TYPE = ffi.return_IDENT() + assert_that(ffi.accept_IDENT(from_rust), "ffi.accept_IDENT(from_rust)") - var from_gdscript = VAL - var mirrored = ffi.mirror_IDENT(from_gdscript) - assert_eq(mirrored, from_gdscript) + var from_gdscript: TYPE = VAL + var mirrored: TYPE = ffi.mirror_IDENT(from_gdscript) + assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript") #) diff --git a/itest/rust/build.rs b/itest/rust/build.rs index ec3ca4708..e6bfe1241 100644 --- a/itest/rust/build.rs +++ b/itest/rust/build.rs @@ -11,22 +11,33 @@ use std::path::Path; type IoResult = std::io::Result<()>; -macro_rules! push { - ($inputs:ident; $gdscript_ty:ident, $rust_ty:ty, $val:expr) => { - push!($inputs; $gdscript_ty, $rust_ty, $val, $val); - }; - - ($inputs:ident; $gdscript_ty:ident, $rust_ty:ty, $gdscript_val:expr, $rust_val:expr) => { +/// Push with GDScript expr in string +macro_rules! pushs { + ($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $gdscript_val:expr, $rust_val:expr) => { $inputs.push(Input { - ident: stringify!($rust_ty).to_ascii_lowercase(), - gdscript_ty: stringify!($gdscript_ty), - gdscript_val: stringify!($gdscript_val), - rust_ty: quote! { $rust_ty }, + ident: stringify!($RustTy) + .to_ascii_lowercase() + .replace("<", "_") + .replace(">", ""), + gdscript_ty: stringify!($GDScriptTy), + gdscript_val: $gdscript_val, + rust_ty: quote! { $RustTy }, rust_val: quote! { $rust_val }, }); }; } +/// Push simple GDScript expression, outside string +macro_rules! push { + ($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $val:expr) => { + push!($inputs; $GDScriptTy, $RustTy, $val, $val); + }; + + ($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $gdscript_val:expr, $rust_val:expr) => { + pushs!($inputs; $GDScriptTy, $RustTy, stringify!($gdscript_val), $rust_val); + }; +} + // Edit this to change involved types fn collect_inputs() -> Vec { let mut inputs = vec![]; @@ -41,15 +52,33 @@ fn collect_inputs() -> Vec { push!(inputs; bool, bool, true); push!(inputs; Color, Color, Color(0.7, 0.5, 0.3, 0.2), Color::from_rgba(0.7, 0.5, 0.3, 0.2)); push!(inputs; String, GodotString, "hello", "hello".into()); + push!(inputs; StringName, StringName, &"hello", "hello".into()); + pushs!(inputs; NodePath, NodePath, r#"^"hello""#, "hello".into()); push!(inputs; Vector2, Vector2, Vector2(12.5, -3.5), Vector2::new(12.5, -3.5)); push!(inputs; Vector3, Vector3, Vector3(117.5, 100.0, -323.25), Vector3::new(117.5, 100.0, -323.25)); push!(inputs; Vector4, Vector4, Vector4(-18.5, 24.75, -1.25, 777.875), Vector4::new(-18.5, 24.75, -1.25, 777.875)); push!(inputs; Vector2i, Vector2i, Vector2i(-2147483648, 2147483647), Vector2i::new(-2147483648, 2147483647)); push!(inputs; Vector3i, Vector3i, Vector3i(-1, -2147483648, 2147483647), Vector3i::new(-1, -2147483648, 2147483647)); - //push!(inputs; Variant, Variant, 123, 123i64.to_variant()); + + // Data structures + // TODO enable below, when GDScript has typed array literals, or find a hack with eval/lambdas + /*pushs!(inputs; Array[int], Array, + "(func() -> Array[int]: [-7, 12, 40])()", + array![-7, 12, 40] + );*/ + + push!(inputs; Array, VariantArray, + [-7, "godot", false, Vector2i(-77, 88)], + varray![-7, "godot", false, Vector2i::new(-77, 88)]); + + pushs!(inputs; Dictionary, Dictionary, + r#"{"key": 83, -3: Vector2(1, 2), 0.03: true}"#, + dict! { "key": 83, (-3): Vector2::new(1.0, 2.0), 0.03: true } + ); // Composite push!(inputs; int, InstanceId, -1, InstanceId::from_nonzero(0xFFFFFFFFFFFFFFF)); + push!(inputs; Variant, Variant, 123, 123i64.to_variant()); inputs }