diff --git a/godot-core/src/init/mod.rs b/godot-core/src/init/mod.rs index 195f7f359..b268dda94 100644 --- a/godot-core/src/init/mod.rs +++ b/godot-core/src/init/mod.rs @@ -17,7 +17,7 @@ use crate::out; pub use sys::GdextBuild; #[doc(hidden)] -// TODO consider body safe despite unsafe function, and explicitly mark unsafe {} locations +#[deny(unsafe_op_in_unsafe_fn)] pub unsafe fn __gdext_load_library( get_proc_address: sys::GDExtensionInterfaceGetProcAddress, library: sys::GDExtensionClassLibraryPtr, @@ -41,7 +41,10 @@ pub unsafe fn __gdext_load_library( let config = sys::GdextConfig::new(tool_only_in_editor); - sys::initialize(get_proc_address, library, config); + // SAFETY: no custom code has run yet + no other thread is accessing global handle. + unsafe { + sys::initialize(get_proc_address, library, config); + } // Currently no way to express failure; could be exposed to E if necessary. // No early exit, unclear if Godot still requires output parameters to be set. @@ -54,7 +57,10 @@ pub unsafe fn __gdext_load_library( deinitialize: Some(ffi_deinitialize_layer::), }; - *init = godot_init_params; + // SAFETY: Godot is responsible for passing us a valid pointer. + unsafe { + *init = godot_init_params; + } success as u8 }; diff --git a/godot-macros/src/class/data_models/inherent_impl.rs b/godot-macros/src/class/data_models/inherent_impl.rs index c0638f8fc..4eeff41f2 100644 --- a/godot-macros/src/class/data_models/inherent_impl.rs +++ b/godot-macros/src/class/data_models/inherent_impl.rs @@ -4,6 +4,7 @@ * 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/. */ + use crate::class::{ into_signature_info, make_constant_registration, make_method_registration, make_signal_registrations, ConstDefinition, FuncDefinition, RpcAttr, RpcMode, SignalDefinition, @@ -66,7 +67,7 @@ struct FuncAttr { pub struct InherentImplAttr { /// For implementation reasons, there can be a single 'primary' impl block and 0 or more 'secondary' impl blocks. - /// For now this is controlled by a key in the the 'godot_api' attribute + /// For now, this is controlled by a key in the 'godot_api' attribute. pub secondary: bool, } @@ -80,7 +81,7 @@ pub fn transform_inherent_impl( let prv = quote! { ::godot::private }; // Can add extra functions to the end of the impl block. - let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block)?; + let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block, meta.secondary)?; let consts = process_godot_constants(&mut impl_block)?; #[cfg(all(feature = "register-docs", since_api = "4.3"))] @@ -107,16 +108,13 @@ pub fn transform_inherent_impl( let fill_storage = quote! { ::godot::sys::plugin_execute_pre_main!({ - #method_storage_name.lock().unwrap().push(||{ - + #method_storage_name.lock().unwrap().push(|| { #( #method_registrations )* #( #signal_registrations )* - }); - #constants_storage_name.lock().unwrap().push(||{ + #constants_storage_name.lock().unwrap().push(|| { #constant_registration - }); }); }; @@ -155,7 +153,6 @@ pub fn transform_inherent_impl( }; let class_registration = quote! { - ::godot::sys::plugin_add!(__GODOT_PLUGIN_REGISTRY in #prv; #prv::ClassPlugin { class_name: #class_name_obj, item: #prv::PluginItem::InherentImpl(#prv::InherentImpl { @@ -169,7 +166,6 @@ pub fn transform_inherent_impl( }), init_level: <#class_name as ::godot::obj::GodotClass>::INIT_LEVEL, }); - }; let result = quote! { @@ -182,7 +178,7 @@ pub fn transform_inherent_impl( Ok(result) } else { - // We are in a secondary `impl` block, so most of the work has already been done + // We are in a secondary `impl` block, so most of the work has already been done, // and we just need to add our registration functions in the storage defined by the primary `impl` block. let result = quote! { @@ -197,6 +193,7 @@ pub fn transform_inherent_impl( fn process_godot_fns( class_name: &Ident, impl_block: &mut venial::Impl, + is_secondary_impl: bool, ) -> ParseResult<(Vec, Vec)> { let mut func_definitions = vec![]; let mut signal_definitions = vec![]; @@ -286,9 +283,16 @@ fn process_godot_fns( rpc_info, }); } + ItemAttrType::Signal(ref _attr_val) => { + if is_secondary_impl { + return attr.bail( + "#[signal] is not currently supported in secondary impl blocks", + function, + ); + } if function.return_ty.is_some() { - return attr.bail("return types are not supported", function); + return attr.bail("return types in #[signal] are not supported", function); } let external_attributes = function.attributes.clone(); @@ -301,6 +305,7 @@ fn process_godot_fns( removed_indexes.push(index); } + ItemAttrType::Const(_) => { return attr.bail( "#[constant] can only be used on associated constant", @@ -541,12 +546,7 @@ where } // #[signal] - name if name == "signal" => { - // TODO once parameters are supported, this should probably be moved to the struct definition - // E.g. a zero-sized type Signal<(i32, String)> with a provided emit(i32, String) method - // This could even be made public (callable on the struct obj itself) - AttrParseResult::Signal(attr.value.clone()) - } + name if name == "signal" => AttrParseResult::Signal(attr.value.clone()), // #[constant] name if name == "constant" => AttrParseResult::Const(attr.value.clone()), diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index 08f5cac45..19a221fdf 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -4,13 +4,13 @@ * 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/. */ + use godot::builtin::inner::InnerCallable; use godot::builtin::{varray, Callable, GString, StringName, Variant}; use godot::classes::{Node2D, Object}; use godot::meta::ToGodot; use godot::obj::{NewAlloc, NewGd}; use godot::register::{godot_api, GodotClass}; -use std::fmt::{Display, Formatter}; use std::hash::Hasher; use std::sync::atomic::{AtomicU32, Ordering}; @@ -364,7 +364,7 @@ pub mod custom_callable { } impl Hash for Adder { - fn hash(&self, state: &mut H) { + fn hash(&self, state: &mut H) { let mut guard = self.tracker.lock().unwrap(); guard.hash_counter += 1; @@ -378,7 +378,7 @@ pub mod custom_callable { } } - impl godot::builtin::RustCallable for Adder { + impl RustCallable for Adder { fn invoke(&mut self, args: &[&Variant]) -> Result { for arg in args { self.sum += arg.to::(); @@ -410,6 +410,7 @@ pub mod custom_callable { tracker.lock().unwrap().hash_counter } + // Also used in signal_test. pub struct PanicCallable(pub Arc); impl PartialEq for PanicCallable { @@ -424,8 +425,8 @@ pub mod custom_callable { } } - impl Display for PanicCallable { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + impl fmt::Display for PanicCallable { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "test") } } diff --git a/itest/rust/src/builtin_tests/containers/signal_test.rs b/itest/rust/src/builtin_tests/containers/signal_test.rs index 1d038c9bd..f6ebcad7d 100644 --- a/itest/rust/src/builtin_tests/containers/signal_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_test.rs @@ -5,67 +5,18 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use godot::builtin::{Callable, GString, Signal, StringName}; -use godot::meta::ToGodot; -use godot::register::{godot_api, GodotClass}; -use std::cell::Cell; - +use godot::builtin::{GString, Signal, StringName}; use godot::classes::{Object, RefCounted}; +use godot::meta::ToGodot; use godot::obj::{Base, Gd, NewAlloc, NewGd, WithBaseField}; +use godot::register::{godot_api, GodotClass}; use godot::sys; +use std::cell::Cell; use crate::framework::itest; -#[derive(GodotClass)] -#[class(init, base=Object)] -struct Emitter {} - -#[godot_api] -impl Emitter { - #[signal] - fn signal_0_arg(); - - #[signal] - fn signal_1_arg(arg1: i64); - - #[signal] - fn signal_2_arg(arg1: Gd, arg2: GString); -} - -#[derive(GodotClass)] -#[class(init, base=Object)] -struct Receiver { - used: [Cell; 3], - base: Base, -} - -#[godot_api] -impl Receiver { - #[func] - fn receive_0_arg(&self) { - self.used[0].set(true); - } - - #[func] - fn receive_1_arg(&self, arg1: i64) { - self.used[1].set(true); - assert_eq!(arg1, 987); - } - - #[func] - fn receive_2_arg(&self, arg1: Gd, arg2: GString) { - assert_eq!(self.base().clone(), arg1); - assert_eq!(SIGNAL_ARG_STRING, arg2.to_string()); - - self.used[2].set(true); - } -} - -const SIGNAL_ARG_STRING: &str = "Signal string arg"; - #[itest] -/// Test that godot can call a method that is connect with a signal -fn signals() { +fn signal_basic_connect_emit() { let mut emitter = Emitter::new_alloc(); let receiver = Receiver::new_alloc(); @@ -76,8 +27,8 @@ fn signals() { ]; for (i, arg) in args.iter().enumerate() { - let signal_name = format!("signal_{i}_arg"); - let receiver_name = format!("receive_{i}_arg"); + let signal_name = format!("emitter_{i}"); + let receiver_name = format!("receiver_{i}"); emitter.connect(&signal_name, &receiver.callable(&receiver_name)); emitter.emit_signal(&signal_name, arg); @@ -90,7 +41,7 @@ fn signals() { } #[itest] -fn instantiate_signal() { +fn signal_construction_and_id() { let mut object = RefCounted::new_gd(); let object_id = object.instance_id(); @@ -109,60 +60,73 @@ fn instantiate_signal() { assert_eq!(signal.object(), None); } -#[itest] -fn emit_signal() { - let mut object = RefCounted::new_gd(); +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Helper types - object.add_user_signal("test_signal"); +#[derive(GodotClass)] +#[class(init, base=Object)] +struct Emitter {} - let signal = Signal::from_object_signal(&object, "test_signal"); - let receiver = Receiver::new_alloc(); +#[godot_api] +impl Emitter { + #[signal] + fn emitter_0(); - object.connect( - &StringName::from("test_signal"), // explicit StringName - &Callable::from_object_method(&receiver, "receive_1_arg"), - ); - assert_eq!(signal.connections().len(), 1); + #[signal] + fn emitter_1(arg1: i64); - signal.emit(&[987i64.to_variant()]); - assert!(receiver.bind().used[1].get()); + #[signal] + fn emitter_2(arg1: Gd, arg2: GString); +} - receiver.free(); +#[derive(GodotClass)] +#[class(init, base=Object)] +struct Receiver { + used: [Cell; 3], + base: Base, } -#[itest] -fn connect_signal() { - let mut object = RefCounted::new_gd(); +#[godot_api] +impl Receiver { + #[func] + fn receiver_0(&self) { + self.used[0].set(true); + } - object.add_user_signal("test_signal"); + #[func] + fn receiver_1(&self, arg1: i64) { + self.used[1].set(true); + assert_eq!(arg1, 987); + } - let signal = Signal::from_object_signal(&object, "test_signal"); - let receiver = Receiver::new_alloc(); + #[func] + fn receiver_2(&self, arg1: Gd, arg2: GString) { + assert_eq!(self.base().clone(), arg1); + assert_eq!(SIGNAL_ARG_STRING, arg2.to_string()); - signal.connect(&Callable::from_object_method(&receiver, "receive_1_arg"), 0); - assert_eq!(signal.connections().len(), 1); + self.used[2].set(true); + } +} - object.emit_signal("test_signal", &[987i64.to_variant()]); - assert!(receiver.bind().used[1].get()); +const SIGNAL_ARG_STRING: &str = "Signal string arg"; - receiver.free(); -} +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// 4.2+ custom callables #[cfg(since_api = "4.2")] mod custom_callable { use godot::builtin::{Callable, Signal}; + use godot::classes::Node; use godot::meta::ToGodot; + use godot::obj::{Gd, NewAlloc}; use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; - use godot::classes::Node; - use godot::obj::{Gd, NewAlloc}; - use crate::builtin_tests::containers::callable_test::custom_callable::PanicCallable; use crate::framework::{itest, TestContext}; #[itest] - fn connect_signal_panic_user_from_fn() { + fn signal_panic_user_from_fn() { connect_signal_panic_shared( "test_signal", connect_signal_panic_from_fn, @@ -176,7 +140,7 @@ mod custom_callable { } #[itest] - fn connect_signal_panic_user_from_custom() { + fn signal_panic_user_from_custom() { connect_signal_panic_shared( "test_signal", connect_signal_panic_from_custom, @@ -190,41 +154,32 @@ mod custom_callable { } #[itest] - fn connect_signal_panic_from_fn_tree_entered(ctx: &TestContext) { + fn signal_panic_from_fn_tree_entered(ctx: &TestContext) { connect_signal_panic_shared( "tree_entered", connect_signal_panic_from_fn, |_node| {}, - |node| { - ctx.scene_tree.clone().add_child(&*node); - ctx.scene_tree.clone().remove_child(&*node); - }, + |node| add_remove_child(ctx, node), ); } #[itest] - fn connect_signal_panic_from_custom_tree_entered(ctx: &TestContext) { + fn signal_panic_from_custom_tree_entered(ctx: &TestContext) { connect_signal_panic_shared( "tree_entered", connect_signal_panic_from_custom, |_node| {}, - |node| { - ctx.scene_tree.clone().add_child(&*node); - ctx.scene_tree.clone().remove_child(&*node); - }, + |node| add_remove_child(ctx, node), ); } #[itest] - fn connect_signal_panic_from_fn_tree_exiting(ctx: &TestContext) { + fn signal_panic_from_fn_tree_exiting(ctx: &TestContext) { connect_signal_panic_shared( "tree_exiting", connect_signal_panic_from_fn, |_node| {}, - |node| { - ctx.scene_tree.clone().add_child(&*node); - ctx.scene_tree.clone().remove_child(&*node); - }, + |node| add_remove_child(ctx, node), ); } @@ -234,28 +189,22 @@ mod custom_callable { "tree_exiting", connect_signal_panic_from_custom, |_node| {}, - |node| { - ctx.scene_tree.clone().add_child(&*node); - ctx.scene_tree.clone().remove_child(&*node); - }, + |node| add_remove_child(ctx, node), ); } #[itest] - fn connect_signal_panic_from_fn_tree_exited(ctx: &TestContext) { + fn signal_panic_from_fn_tree_exited(ctx: &TestContext) { connect_signal_panic_shared( "tree_exited", connect_signal_panic_from_fn, |_node| {}, - |node| { - ctx.scene_tree.clone().add_child(&*node); - ctx.scene_tree.clone().remove_child(&*node); - }, + |node| add_remove_child(ctx, node), ); } #[itest] - fn connect_signal_panic_from_custom_tree_exited(ctx: &TestContext) { + fn signal_panic_from_custom_tree_exited(ctx: &TestContext) { connect_signal_panic_shared( "tree_exited", connect_signal_panic_from_custom, @@ -267,6 +216,15 @@ mod custom_callable { ); } + // ------------------------------------------------------------------------------------------------------------------------------------------ + // 4.2+ custom callables - helper functions + + fn add_remove_child(ctx: &TestContext, node: &mut Gd) { + let mut tree = ctx.scene_tree.clone(); + tree.add_child(&*node); + tree.remove_child(&*node); + } + fn connect_signal_panic_shared( signal: &str, callable: impl FnOnce(Arc) -> Callable, @@ -283,7 +241,6 @@ mod custom_callable { signal.connect(&callable, 0); emit(&mut node); - assert_eq!(1, received.load(Ordering::SeqCst)); node.free(); diff --git a/itest/rust/src/framework/mod.rs b/itest/rust/src/framework/mod.rs index 6f1ed6d95..4bc0fdfe7 100644 --- a/itest/rust/src/framework/mod.rs +++ b/itest/rust/src/framework/mod.rs @@ -135,7 +135,7 @@ pub fn suppress_godot_print(mut f: impl FnMut()) { } /// Some tests are disabled, as they rely on Godot checks which are only available in Debug builds. -/// See https://github.com/godotengine/godot/issues/86264. +/// See . pub fn runs_release() -> bool { !Os::singleton().is_debug_build() }