From 686f08d924cd1eeec4c79c30854ead9e5b50071d Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 5 Jan 2025 22:44:20 +0100 Subject: [PATCH] Integrate connect/emit with Rust named function pointers --- godot-core/src/builtin/mod.rs | 3 - godot-core/src/classes/class_runtime.rs | 2 +- godot-core/src/obj/gd.rs | 5 + godot-core/src/obj/raw_gd.rs | 7 +- godot-core/src/private.rs | 2 - godot-core/src/registry/as_func.rs | 46 ++++++ godot-core/src/registry/mod.rs | 2 + .../src/{builtin => registry}/typed_signal.rs | 149 +++++++++++++++--- godot-macros/src/class/data_models/func.rs | 8 +- godot-macros/src/class/data_models/signal.rs | 83 +++++----- godot/src/lib.rs | 5 +- .../builtin_tests/containers/signal_test.rs | 34 +--- 12 files changed, 247 insertions(+), 99 deletions(-) create mode 100644 godot-core/src/registry/as_func.rs rename godot-core/src/{builtin => registry}/typed_signal.rs (52%) diff --git a/godot-core/src/builtin/mod.rs b/godot-core/src/builtin/mod.rs index a2310769d..80bc5056d 100644 --- a/godot-core/src/builtin/mod.rs +++ b/godot-core/src/builtin/mod.rs @@ -63,8 +63,6 @@ pub mod __prelude_reexport { pub use string::{GString, NodePath, StringName}; pub use transform2d::*; pub use transform3d::*; - // TODO move to register? - pub use typed_signal::{Func, TypedSignal}; pub use variant::*; pub use vectors::*; @@ -113,7 +111,6 @@ mod signal; mod string; mod transform2d; mod transform3d; -mod typed_signal; mod variant; mod vectors; diff --git a/godot-core/src/classes/class_runtime.rs b/godot-core/src/classes/class_runtime.rs index a6f72a1ea..ba487b03d 100644 --- a/godot-core/src/classes/class_runtime.rs +++ b/godot-core/src/classes/class_runtime.rs @@ -47,7 +47,7 @@ pub(crate) fn display_string( obj: &Gd, f: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { - let string: GString = obj.raw.as_object().to_string(); + let string: GString = obj.raw.as_object_ref().to_string(); ::fmt(&string, f) } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 436de6520..7a93898b4 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -327,6 +327,11 @@ impl Gd { .expect("Upcast to Object failed. This is a bug; please report it.") } + /// Equivalent to [`upcast_mut::()`][Self::upcast_mut], but without bounds. + pub(crate) fn upcast_object_mut(&mut self) -> &mut classes::Object { + self.raw.as_object_mut() + } + /// **Upcast shared-ref:** access this object as a shared reference to a base class. /// /// This is semantically equivalent to multiple applications of [`Self::deref()`]. Not really useful on its own, but combined with diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index 1801d5620..2ce3bce22 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -207,11 +207,16 @@ impl RawGd { // self.as_target_mut() // } - pub(crate) fn as_object(&self) -> &classes::Object { + pub(crate) fn as_object_ref(&self) -> &classes::Object { // SAFETY: Object is always a valid upcast target. unsafe { self.as_upcast_ref() } } + pub(crate) fn as_object_mut(&mut self) -> &mut classes::Object { + // SAFETY: Object is always a valid upcast target. + unsafe { self.as_upcast_mut() } + } + /// # Panics /// If this `RawGd` is null. In Debug mode, sanity checks (valid upcast, ID comparisons) can also lead to panics. /// diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index e87eda72a..157d274db 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -17,11 +17,9 @@ pub use sys::out; #[cfg(feature = "trace")] pub use crate::meta::trace; -use crate::builtin::Variant; use crate::global::godot_error; use crate::meta::error::CallError; use crate::meta::CallContext; -use crate::obj::{bounds, BaseMut, GodotClass, Inherits}; use crate::sys; use std::sync::atomic; #[cfg(debug_assertions)] diff --git a/godot-core/src/registry/as_func.rs b/godot-core/src/registry/as_func.rs new file mode 100644 index 000000000..f5769e36c --- /dev/null +++ b/godot-core/src/registry/as_func.rs @@ -0,0 +1,46 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +// https://geo-ant.github.io/blog/2021/rust-traits-and-variadic-functions +// +// Could be generalized with R return type, and not special-casing `self`. But keep simple until actually needed. + +pub trait AsFunc { + fn call(&mut self, maybe_instance: I, params: Ps); +} + +// pub trait AsMethod { +// fn call(&mut self, instance: &mut C, params: Ps); +// } + +// Now generalize via macro: +macro_rules! impl_signal_recipient { + ($( $args:ident : $Ps:ident ),*) => { + // Global and associated functions. + impl AsFunc<(), ( $($Ps,)* )> for F + where F: FnMut( $($Ps,)* ) -> R + { + fn call(&mut self, _no_instance: (), ($($args,)*): ( $($Ps,)* )) { + self($($args,)*); + } + } + + // Methods. + impl AsFunc<&mut C, ( $($Ps,)* )> for F + where F: FnMut( &mut C, $($Ps,)* ) -> R + { + fn call(&mut self, instance: &mut C, ($($args,)*): ( $($Ps,)* )) { + self(instance, $($args,)*); + } + } + }; +} + +impl_signal_recipient!(); +impl_signal_recipient!(arg0: P0); +impl_signal_recipient!(arg0: P0, arg1: P1); +impl_signal_recipient!(arg0: P0, arg1: P1, arg2: P2); diff --git a/godot-core/src/registry/mod.rs b/godot-core/src/registry/mod.rs index 9ce2a155d..83ca83804 100644 --- a/godot-core/src/registry/mod.rs +++ b/godot-core/src/registry/mod.rs @@ -8,12 +8,14 @@ // Note: final re-exports from godot-core are in lib.rs, mod register::private. // These are public here for simplicity, but many are not imported by the main crate. +pub mod as_func; pub mod callbacks; pub mod class; pub mod constant; pub mod method; pub mod plugin; pub mod property; +pub mod typed_signal; // RpcConfig uses MultiplayerPeer::TransferMode and MultiplayerApi::RpcMode, which are only enabled in `codegen-full` feature. #[cfg(feature = "codegen-full")] diff --git a/godot-core/src/builtin/typed_signal.rs b/godot-core/src/registry/typed_signal.rs similarity index 52% rename from godot-core/src/builtin/typed_signal.rs rename to godot-core/src/registry/typed_signal.rs index 11ee86fa8..abf6b8f49 100644 --- a/godot-core/src/builtin/typed_signal.rs +++ b/godot-core/src/registry/typed_signal.rs @@ -7,56 +7,163 @@ // Maybe move this to builtin::functional module? -use crate::builtin::{Callable, Signal, Variant}; -use crate::obj::Gd; +use crate::builtin::{Callable, Variant}; +use crate::obj::{Gd, GodotClass, WithBaseField}; +use crate::registry::as_func::AsFunc; use crate::{classes, meta, sys}; use std::borrow::Cow; use std::fmt; pub trait ParamTuple { fn to_variant_array(&self) -> Vec; + fn from_variant_array(array: &[&Variant]) -> Self; } -impl ParamTuple for () { - fn to_variant_array(&self) -> Vec { - Vec::new() - } +macro_rules! impl_param_tuple { + // Recursive case for tuple with N elements + ($($args:ident : $Ps:ident),*) => { + impl<$($Ps),*> ParamTuple for ($($Ps,)*) + where + $($Ps: meta::ToGodot + meta::FromGodot),* + { + fn to_variant_array(&self) -> Vec { + let ($($args,)*) = self; + + vec![ + $( $args.to_variant(), )* + ] + } + + #[allow(unused_variables, unused_mut, clippy::unused_unit)] + fn from_variant_array(array: &[&Variant]) -> Self { + let mut iter = array.iter(); + ( $( + <$Ps>::from_variant( + iter.next().unwrap_or_else(|| panic!("ParamTuple: {} access out-of-bounds (len {})", stringify!($args), array.len())) + ), + )* ) + } + } + }; } -impl ParamTuple for (T,) +impl_param_tuple!(); +impl_param_tuple!(arg0: P0); +impl_param_tuple!(arg0: P0, arg1: P1); +impl_param_tuple!(arg0: P0, arg1: P1, arg2: P2); + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + +#[doc(hidden)] +pub enum ObjectRef<'a, C: GodotClass> { + /// Helpful for emit: reuse `&self` from within the `impl` block, goes through `base()` re-borrowing and thus allows re-entrant calls + /// through Godot. + Internal { obj_mut: &'a mut C }, + + /// From outside, based on `Gd` pointer. + External { gd: Gd }, +} + +impl ObjectRef<'_, C> where - T: meta::ToGodot, + C: WithBaseField, { - fn to_variant_array(&self) -> Vec { - vec![self.0.to_variant()] + fn with_object_mut(&mut self, f: impl FnOnce(&mut classes::Object)) { + match self { + ObjectRef::Internal { obj_mut } => f(obj_mut.base_mut().upcast_object_mut()), + ObjectRef::External { gd } => f(gd.upcast_object_mut()), + } + } + + fn to_owned(&self) -> Gd { + match self { + ObjectRef::Internal { obj_mut } => WithBaseField::to_gd(*obj_mut), + ObjectRef::External { gd } => gd.clone(), + } } } // ---------------------------------------------------------------------------------------------------------------------------------------------- -pub struct TypedSignal { - signal: Signal, +pub struct TypedSignal<'a, C: GodotClass, Ps> { + //signal: Signal, + /// In Godot, valid signals (unlike funcs) are _always_ declared in a class and become part of each instance. So there's always an object. + owner: ObjectRef<'a, C>, + name: Cow<'static, str>, _signature: std::marker::PhantomData, } -impl TypedSignal { - pub(crate) fn from_untyped(signal: Signal) -> Self { +impl<'a, C: WithBaseField, Ps: ParamTuple> TypedSignal<'a, C, Ps> { + #[doc(hidden)] + pub fn new(owner: ObjectRef<'a, C>, name: &'static str) -> Self { Self { - signal, + owner, + name: Cow::Borrowed(name), _signature: std::marker::PhantomData, } } - pub fn emit(&self, params: Ps) { - self.signal.emit(¶ms.to_variant_array()); + pub fn emit(&mut self, params: Ps) { + let name = self.name.as_ref(); + + self.owner.with_object_mut(|obj| { + obj.emit_signal(name, ¶ms.to_variant_array()); + }); } - pub fn connect_untyped(&mut self, callable: &Callable, flags: i64) { - self.signal.connect(callable, flags); + /// Connect a method (member function) with `&mut self` as the first parameter. + pub fn connect_self(&mut self, mut function: F) + where + for<'c> F: AsFunc<&'c mut C, Ps> + 'static, + { + // When using sys::short_type_name() in the future, make sure global "func" and member "MyClass::func" are rendered as such. + // PascalCase heuristic should then be good enough. + let callable_name = std::any::type_name_of_val(&function); + + let object = self.owner.to_owned(); + let godot_fn = move |variant_args: &[&Variant]| -> Result { + let args = Ps::from_variant_array(variant_args); + + // let mut function = function; + // function.call(instance, args); + let mut object = object.clone(); + + // TODO: how to avoid another bind, when emitting directly from Rust? + let mut instance = object.bind_mut(); + let instance = &mut *instance; + function.call(instance, args); + + Ok(Variant::nil()) + }; + + let name = self.name.as_ref(); + let callable = Callable::from_local_fn(callable_name, godot_fn); + + self.owner.with_object_mut(|obj| { + obj.connect(name, &callable); + }); } - pub fn to_untyped(&self) -> Signal { - self.signal.clone() + /// Connect a static function (global or associated function). + pub fn connect(&mut self, mut function: F) + where + F: AsFunc<(), Ps> + 'static, + { + let callable_name = std::any::type_name_of_val(&function); + + let godot_fn = move |variant_args: &[&Variant]| -> Result { + let args = Ps::from_variant_array(variant_args); + function.call((), args); + + Ok(Variant::nil()) + }; + + let name = self.name.as_ref(); + let callable = Callable::from_local_fn(callable_name, godot_fn); + + self.owner.with_object_mut(|obj| { + obj.connect(name, &callable); + }); } } diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 3d16542a4..0e9e368dd 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -193,16 +193,16 @@ pub fn make_func_collection( static_collection_methods.push(quote! { #(#cfg_attrs)* // Use `&self` here to enable `.` chaining, such as in MyClass::static_funcs().my_func(). - fn #rust_func_name(self) -> ::godot::builtin::Func<#generic_args> { + fn #rust_func_name(self) -> ::godot::register::Func<#generic_args> { let class_name = <#class_name as ::godot::obj::GodotClass>::class_name(); - ::godot::builtin::Func::from_static_function(class_name.to_cow_str(), #godot_func_name) + ::godot::register::Func::from_static_function(class_name.to_cow_str(), #godot_func_name) } }); } else { instance_collection_methods.push(quote! { #(#cfg_attrs)* - fn #rust_func_name(self) -> ::godot::builtin::Func<#generic_args> { - ::godot::builtin::Func::from_instance_method(self.obj, #godot_func_name) + fn #rust_func_name(self) -> ::godot::register::Func<#generic_args> { + ::godot::register::Func::from_instance_method(self.obj, #godot_func_name) } }); } diff --git a/godot-macros/src/class/data_models/signal.rs b/godot-macros/src/class/data_models/signal.rs index f7564b24c..9bee17bb4 100644 --- a/godot-macros/src/class/data_models/signal.rs +++ b/godot-macros/src/class/data_models/signal.rs @@ -50,7 +50,7 @@ impl<'a> SignalDetails<'a> { pub fn extract( original_decl: &'a venial::Function, class_name: &'a Ident, - external_attributes: &'a Vec, + external_attributes: &'a [venial::Attribute], ) -> ParseResult> { let mut param_types = vec![]; let mut param_names = vec![]; @@ -109,8 +109,10 @@ pub fn make_signal_registrations( has_builder, } = signal; - let details = SignalDetails::extract(&signature, class_name, external_attributes)?; + let details = SignalDetails::extract(signature, class_name, external_attributes)?; + // Callable custom functions are only supported in 4.2+, upon which custom signals rely. + #[cfg(since_api = "4.2")] if *has_builder { collection_api.extend_with(&details); } @@ -136,7 +138,7 @@ fn make_signal_registration(details: &SignalDetails, class_name_obj: &TokenStrea .. } = details; - let signature_tuple = util::make_signature_tuple_type("e! { () }, ¶m_types); + let signature_tuple = util::make_signature_tuple_type("e! { () }, param_types); let indexes = 0..param_types.len(); let param_property_infos = quote! { @@ -185,28 +187,24 @@ struct SignalCollection { /// The actual signal definitions, including both `struct` and `impl` blocks. individual_structs: Vec, - // signals_fields: Vec, } impl SignalCollection { fn extend_with(&mut self, details: &SignalDetails) { let SignalDetails { signal_name, + signal_name_str, signal_cfg_attrs, individual_struct_name, .. } = details; - // self.signals_fields.push(quote! { - // #(#signal_cfg_attrs)* - // #signal_name: ::godot::builtin::TypedSignal<#param_tuple> - // }); - self.collection_methods.push(quote! { #(#signal_cfg_attrs)* - fn #signal_name(&mut self) -> #individual_struct_name<'_> { - let object_mut = &mut *self.object_base; - #individual_struct_name { object_mut } + fn #signal_name(self) -> #individual_struct_name<'a> { + #individual_struct_name { + typed: ::godot::register::TypedSignal::new(self.object, #signal_name_str) + } } }); @@ -226,44 +224,53 @@ fn make_signal_individual_struct(details: &SignalDetails) -> TokenStream { class_name, param_names, param_tuple, - signal_name_str, + // signal_name, signal_cfg_attrs, individual_struct_name, .. } = details; + // let module_name = format_ident!("__godot_signal_{class_name}_{signal_name}"); + + // Module + re-export. + // Could also keep contained in module to reduce namespace pollution, but might make docs a bit more nested. quote! { + // #(#signal_cfg_attrs)* + // mod #module_name { + + // TODO make pub without running into "private type `MySignal` in public interface" errors. #(#signal_cfg_attrs)* #[allow(non_camel_case_types)] - pub struct #individual_struct_name<'a> { - object_mut: &'a mut ::godot::classes::Object - //object_base: ::godot::obj::BaseMut<'a, #class_name>, - //signal: ::godot::builtin::TypedSignal<#param_tuple> + struct #individual_struct_name<'a> { + typed: ::godot::register::TypedSignal<'a, #class_name, #param_tuple>, } + // Concrete convenience API is macro-based; many parts are delegated to TypedSignal via Deref/DerefMut. #(#signal_cfg_attrs)* impl #individual_struct_name<'_> { pub fn emit(&mut self, #emit_params) { - use ::godot::meta::ToGodot; - // Potential optimization: encode args as signature-tuple and use direct ptrcall. - let varargs = [ - #( #param_names.to_variant(), )* - ]; - self.object_mut.emit_signal(#signal_name_str, &varargs); + self.typed.emit((#( #param_names, )*)); } + } - fn connect_fn(&mut self, f: impl FnMut #param_tuple) { + #(#signal_cfg_attrs)* + impl<'a> std::ops::Deref for #individual_struct_name<'a> { + type Target = ::godot::register::TypedSignal<'a, #class_name, #param_tuple>; + fn deref(&self) -> &Self::Target { + &self.typed } + } - fn connect(mut self, registered_func: ::godot::register::Func) -> Self { - // connect() return value is ignored -- do not write `let _ = ...`, so we can revisit this when adding #[must_use] to Error. - - let callable = registered_func.to_callable(); - self.object_mut.connect(#signal_name_str, &callable); - self + #(#signal_cfg_attrs)* + impl std::ops::DerefMut for #individual_struct_name<'_> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.typed } } + + // #(#signal_cfg_attrs)* + // pub(crate) use #module_name::#individual_struct_name; } } @@ -273,27 +280,27 @@ fn make_signal_collection(class_name: &Ident, collection: SignalCollection) -> O return None; } - let struct_name = format_ident!("{}Signals", class_name); - let signals_struct_methods = &collection.collection_methods; + let collection_struct_name = format_ident!("{}Signals", class_name); + let collection_struct_methods = &collection.collection_methods; let individual_structs = collection.individual_structs; let code = quote! { #[allow(non_camel_case_types)] - pub struct #struct_name<'a> { + pub struct #collection_struct_name<'a> { // To allow external call in the future (given Gd, not self), this could be an enum with either BaseMut or &mut Gd/&mut T. - object_base: ::godot::obj::BaseMut<'a, #class_name>, + object: ::godot::register::ObjectRef<'a, #class_name> } - impl #struct_name<'_> { - #( #signals_struct_methods )* + impl<'a> #collection_struct_name<'a> { + #( #collection_struct_methods )* } impl ::godot::obj::cap::WithSignals for #class_name { - type SignalCollection<'a> = #struct_name<'a>; + type SignalCollection<'a> = #collection_struct_name<'a>; fn signals(&mut self) -> Self::SignalCollection<'_> { Self::SignalCollection { - object_base: self.base_mut(), + object: ::godot::register::ObjectRef::Internal { obj_mut: self } } } } diff --git a/godot/src/lib.rs b/godot/src/lib.rs index a2f0a8af6..c0028896b 100644 --- a/godot/src/lib.rs +++ b/godot/src/lib.rs @@ -183,12 +183,11 @@ pub mod init { /// Register/export Rust symbols to Godot: classes, methods, enums... pub mod register { + pub use godot_core::registry::as_func::*; pub use godot_core::registry::property; + pub use godot_core::registry::typed_signal::*; pub use godot_macros::{godot_api, godot_dyn, Export, GodotClass, GodotConvert, Var}; - // FIXME - pub use godot_core::builtin::Func; - #[cfg(feature = "__codegen-full")] pub use godot_core::registry::RpcConfig; diff --git a/itest/rust/src/builtin_tests/containers/signal_test.rs b/itest/rust/src/builtin_tests/containers/signal_test.rs index 25252d8d6..d9a207f74 100644 --- a/itest/rust/src/builtin_tests/containers/signal_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_test.rs @@ -9,7 +9,7 @@ use crate::framework::itest; use godot::builtin::{GString, Signal, StringName}; use godot::classes::{Object, RefCounted}; use godot::meta::ToGodot; -use godot::obj::cap::{WithFuncs, WithSignals}; +use godot::obj::cap::WithSignals; use godot::obj::{Base, Gd, NewAlloc, NewGd, WithBaseField}; use godot::register::{godot_api, GodotClass}; use godot::sys; @@ -49,24 +49,20 @@ fn signal_symbols_api() { let mut internal = emitter.bind_mut(); internal.connect_signals_internal(); - //internal.signals().emitter_1() drop(internal); // let check = Signal::from_object_signal(&emitter, "emitter_1"); // dbg!(check.connections()); - println!("PRE_EMIT"); - emitter.emit_signal("emitter_1", &[1234.to_variant()]); - println!("POST_EMIT"); - emitter.bind_mut().emit_signals_internal(); + // Check that instance method is invoked. let received_arg = LAST_METHOD_ARG.lock(); assert_eq!(*received_arg, Some(1234), "Emit failed (method)"); - // This is currently broken: - // let received_arg = LAST_STATIC_FUNCTION_ARG.lock(); - // assert_eq!(*received_arg, Some(1234), "Emit failed (static function)"); + // Check that static function is invoked. + let received_arg = LAST_STATIC_FUNCTION_ARG.lock(); + assert_eq!(*received_arg, Some(1234), "Emit failed (static function)"); receiver.free(); emitter.free(); @@ -118,30 +114,20 @@ impl Emitter { #[func] fn self_receive(&mut self, arg1: i64) { - println!("Received instance: {}", arg1); *LAST_METHOD_ARG.lock() = Some(arg1); } #[func] fn self_receive_static(arg1: i64) { - let x = Self::self_receive; - let y = Self::self_receive_static; - - println!("Received static: {}", arg1); *LAST_STATIC_FUNCTION_ARG.lock() = Some(arg1); } // "Internal" means connect/emit happens from within the class (via &mut self). fn connect_signals_internal(&mut self) { - self.signals().emitter_2().connect_fn(|obj, s| { - println!("emitter_2({obj}, {s})"); - }); - - let m = self.funcs().self_receive(); - let s = Self::static_funcs().self_receive_static(); - - self.signals().emitter_1().connect(m).connect(s); + let mut sig = self.signals().emitter_1(); + sig.connect_self(Self::self_receive); + sig.connect(Self::self_receive_static); } fn emit_signals_internal(&mut self) { @@ -180,10 +166,6 @@ impl Receiver { // This should probably have a dedicated key such as #[godot_api(func_refs)] or so... #[signal] fn _just_here_to_generate_funcs(); - - fn func(&self) { - // let f = self.signals().receiver_2(); - } } const SIGNAL_ARG_STRING: &str = "Signal string arg";