diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs index 19796c72b..f18ba1b54 100644 --- a/godot-core/src/builtin/string/gstring.rs +++ b/godot-core/src/builtin/string/gstring.rs @@ -328,7 +328,11 @@ impl From<&str> for GString { unsafe { Self::new_with_string_uninit(|string_ptr| { + #[cfg(before_api = "4.3")] let ctor = interface_fn!(string_new_with_utf8_chars_and_len); + #[cfg(since_api = "4.3")] + let ctor = interface_fn!(string_new_with_utf8_chars_and_len2); + ctor( string_ptr, bytes.as_ptr() as *const std::ffi::c_char, diff --git a/godot-core/src/classes/class_runtime.rs b/godot-core/src/classes/class_runtime.rs index 7ac021592..4e39ba262 100644 --- a/godot-core/src/classes/class_runtime.rs +++ b/godot-core/src/classes/class_runtime.rs @@ -170,11 +170,15 @@ pub(crate) fn construct_engine_object() -> Gd where T: GodotClass + Bounds, { - // SAFETY: adhere to Godot API; valid class name and returned pointer is an object. - unsafe { - let object_ptr = sys::interface_fn!(classdb_construct_object)(T::class_name().string_sys()); - Gd::from_obj_sys(object_ptr) - } + let mut obj = unsafe { + let object_ptr = sys::classdb_construct_object(T::class_name().string_sys()); + Gd::::from_obj_sys(object_ptr) + }; + #[cfg(since_api = "4.4")] + obj.upcast_object_mut() + .notify(crate::classes::notify::ObjectNotification::POSTINITIALIZE); + + obj } pub(crate) fn ensure_object_alive( diff --git a/godot-core/src/obj/base.rs b/godot-core/src/obj/base.rs index d6c2740e0..20ae60c7c 100644 --- a/godot-core/src/obj/base.rs +++ b/godot-core/src/obj/base.rs @@ -166,15 +166,15 @@ impl Base { (*self.obj).clone() } - /// Returns a [`Gd`] referencing the base object, for exclusive use during object initialization. + /// Returns a [`Gd`] referencing the base object, for exclusive use during object initialization and `NOTIFICATION_POSTINITIALIZE`. /// - /// Can be used during an initialization function [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`]. + /// Can be used during an initialization function [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`], or [`POSTINITIALIZE`][crate::classes::notify::ObjectNotification::POSTINITIALIZE]. /// /// The base pointer is only pointing to a base object; you cannot yet downcast it to the object being constructed. /// The instance ID is the same as the one the in-construction object will have. /// /// # Lifecycle for ref-counted classes - /// If `T: Inherits`, then the ref-counted object is not yet fully-initialized at the time of the `init` function running. + /// If `T: Inherits`, then the ref-counted object is not yet fully-initialized at the time of the `init` function and [`POSTINITIALIZE`][crate::classes::notify::ObjectNotification::POSTINITIALIZE] running. /// Accessing the base object without further measures would be dangerous. Here, godot-rust employs a workaround: the `Base` object (which /// holds a weak pointer to the actual instance) is temporarily upgraded to a strong pointer, preventing use-after-free. /// diff --git a/godot-core/src/obj/bounds.rs b/godot-core/src/obj/bounds.rs index 8e351665d..348a4f154 100644 --- a/godot-core/src/obj/bounds.rs +++ b/godot-core/src/obj/bounds.rs @@ -398,11 +398,7 @@ impl Declarer for DeclEngine { where T: GodotDefault + Bounds, { - unsafe { - let object_ptr = - sys::interface_fn!(classdb_construct_object)(T::class_name().string_sys()); - Gd::from_obj_sys(object_ptr) - } + crate::classes::construct_engine_object() } } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 8b28d92b5..c8fd09b7d 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -155,7 +155,7 @@ where where F: FnOnce(crate::obj::Base) -> T, { - let object_ptr = callbacks::create_custom(init) // or propagate panic. + let object_ptr = callbacks::create_custom(init, true) // or propagate panic. .unwrap_or_else(|payload| PanicPayload::repanic(payload)); unsafe { Gd::from_obj_sys(object_ptr) } @@ -335,6 +335,14 @@ impl Gd { Some(rc.map(|i| i as usize)) } + /// Drop without decrementing ref-counter. + /// + /// Needed in situations where the instance should effectively be forgotten, but without leaking other associated data. + pub(crate) fn drop_weak(self) { + // As soon as fields need custom Drop, this won't be enough anymore. + std::mem::forget(self); + } + #[cfg(feature = "trace")] // itest only. #[doc(hidden)] pub fn test_refcount(&self) -> Option { diff --git a/godot-core/src/registry/callbacks.rs b/godot-core/src/registry/callbacks.rs index d8e704535..d78720244 100644 --- a/godot-core/src/registry/callbacks.rs +++ b/godot-core/src/registry/callbacks.rs @@ -35,14 +35,19 @@ pub unsafe extern "C" fn create( _class_userdata: *mut std::ffi::c_void, _notify_postinitialize: sys::GDExtensionBool, ) -> sys::GDExtensionObjectPtr { - create_custom(T::__godot_user_init).unwrap_or(std::ptr::null_mut()) + create_custom( + T::__godot_user_init, + sys::conv::bool_from_sys(_notify_postinitialize), + ) + .unwrap_or(std::ptr::null_mut()) } #[cfg(before_api = "4.4")] pub unsafe extern "C" fn create( _class_userdata: *mut std::ffi::c_void, ) -> sys::GDExtensionObjectPtr { - create_custom(T::__godot_user_init).unwrap_or(std::ptr::null_mut()) + // `notify_postinitialize` doesn't matter before 4.4, it's sent by Godot when constructing object and we don't send it. + create_custom(T::__godot_user_init, true).unwrap_or(std::ptr::null_mut()) } /// Workaround for before Godot 4.5. @@ -71,7 +76,7 @@ pub unsafe extern "C" fn recreate( _class_userdata: *mut std::ffi::c_void, object: sys::GDExtensionObjectPtr, ) -> sys::GDExtensionClassInstancePtr { - create_rust_part_for_existing_godot_part(T::__godot_user_init, object) + create_rust_part_for_existing_godot_part(T::__godot_user_init, object, |_| {}) .unwrap_or(std::ptr::null_mut()) } @@ -88,15 +93,26 @@ pub unsafe extern "C" fn recreate_null( pub(crate) fn create_custom( make_user_instance: F, + notify_postinitialize: bool, ) -> Result where T: GodotClass, F: FnOnce(Base) -> T, { let base_class_name = T::Base::class_name(); - let base_ptr = unsafe { interface_fn!(classdb_construct_object)(base_class_name.string_sys()) }; + let base_ptr = unsafe { sys::classdb_construct_object(base_class_name.string_sys()) }; + + let postinit = |base_ptr| { + #[cfg(since_api = "4.4")] + if notify_postinitialize { + // Should notify it with a weak pointer, during `NOTIFICATION_POSTINITIALIZE`, ref-counted object is not yet fully-initialized. + let mut obj = unsafe { Gd::::from_obj_sys_weak(base_ptr) }; + obj.notify(crate::classes::notify::ObjectNotification::POSTINITIALIZE); + obj.drop_weak(); + } + }; - match create_rust_part_for_existing_godot_part(make_user_instance, base_ptr) { + match create_rust_part_for_existing_godot_part(make_user_instance, base_ptr, postinit) { Ok(_extension_ptr) => Ok(base_ptr), Err(payload) => { // Creation of extension object failed; we must now also destroy the base object to avoid leak. @@ -115,13 +131,15 @@ where /// With godot-rust, custom objects consist of two parts: the Godot object and the Rust object. This method takes the Godot part by pointer, /// creates the Rust part with the supplied state, and links them together. This is used for both brand-new object creation and hot reload. /// During hot reload, Rust objects are disposed of and then created again with updated code, so it's necessary to re-link them to Godot objects. -fn create_rust_part_for_existing_godot_part( +fn create_rust_part_for_existing_godot_part( make_user_instance: F, base_ptr: sys::GDExtensionObjectPtr, + postinit: P, ) -> Result where T: GodotClass, F: FnOnce(Base) -> T, + P: Fn(sys::GDExtensionObjectPtr), { let class_name = T::class_name(); //out!("create callback: {}", class_name.backing); @@ -153,6 +171,8 @@ where ); } + postinit(base_ptr); + // Mark initialization as complete, now that user constructor has finished. base_copy.mark_initialized(); diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index 614f73de5..510f1c4de 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -471,6 +471,21 @@ pub unsafe fn discover_main_thread() { } } +/// Construct Godot object. +/// +/// "NOTIFICATION_POSTINITIALIZE" must be sent after construction since 4.4. +/// +/// # Safety +/// `class_name` is assumed to be valid. +pub unsafe fn classdb_construct_object( + class_name: GDExtensionConstStringNamePtr, +) -> GDExtensionObjectPtr { + #[cfg(before_api = "4.4")] + return interface_fn!(classdb_construct_object)(class_name); + #[cfg(since_api = "4.4")] + return interface_fn!(classdb_construct_object2)(class_name); +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Macros to access low-level function bindings diff --git a/itest/rust/src/object_tests/base_init_test.rs b/itest/rust/src/object_tests/base_init_test.rs index 452fccdfb..107fc4cba 100644 --- a/itest/rust/src/object_tests/base_init_test.rs +++ b/itest/rust/src/object_tests/base_init_test.rs @@ -7,8 +7,11 @@ use godot::builtin::real_consts::FRAC_PI_3; use godot::builtin::Vector2; -use godot::classes::{ClassDb, RefCounted}; -use godot::obj::{Gd, InstanceId, NewAlloc, NewGd, WithBaseField}; +use godot::classes::notify::ObjectNotification; +use godot::classes::{ClassDb, IRefCounted, RefCounted}; +use godot::meta::ToGodot; +use godot::obj::{Base, Gd, InstanceId, NewAlloc, NewGd, WithBaseField}; +use godot::register::{godot_api, GodotClass}; use godot::task::TaskHandle; use crate::framework::{expect_panic, itest, next_frame}; @@ -168,3 +171,29 @@ fn base_init_to_gd() { }); }); } + +#[derive(GodotClass)] +#[class(init)] +struct RefcPostinit { + pub base: Base, +} + +#[godot_api] +impl IRefCounted for RefcPostinit { + fn on_notification(&mut self, what: ObjectNotification) { + if what == ObjectNotification::POSTINITIALIZE { + self.base + .to_init_gd() + .set_meta("meta", &"postinited".to_variant()); + } + } +} + +#[cfg(since_api = "4.4")] +#[itest(async)] +fn base_postinit_refcounted() -> TaskHandle { + let obj = RefcPostinit::new_gd(); + assert_eq!(obj.get_meta("meta"), "postinited".to_variant()); + assert_eq!(obj.get_reference_count(), 2); + next_frame(move || assert_eq!(obj.get_reference_count(), 1, "eventual dec-ref happens")) +} diff --git a/itest/rust/src/object_tests/virtual_methods_test.rs b/itest/rust/src/object_tests/virtual_methods_test.rs index 9ca73e7e9..3cbe3f3cf 100644 --- a/itest/rust/src/object_tests/virtual_methods_test.rs +++ b/itest/rust/src/object_tests/virtual_methods_test.rs @@ -520,6 +520,8 @@ fn test_notifications() { assert_eq!( obj.bind().sequence, vec![ + #[cfg(since_api = "4.4")] + ReceivedEvent::Notification(NodeNotification::POSTINITIALIZE), ReceivedEvent::Notification(NodeNotification::UNPAUSED), ReceivedEvent::Notification(NodeNotification::EDITOR_POST_SAVE), ReceivedEvent::Ready,