Merge pull request #1211 from beicause/update-postinit

Emit `POSTINITIALIZE` notification after `init()`

Author: Bromeon

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,

godot-core/src/classes/class_runtime.rs

@@ -170,11 +170,15 @@ pub(crate) fn construct_engine_object<T>() -> Gd<T>
 where
     T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
 {
-    // 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::<T>::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(

godot-core/src/obj/base.rs

@@ -166,15 +166,15 @@ impl<T: GodotClass> Base<T> {
         (*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<RefCounted>`, then the ref-counted object is not yet fully-initialized at the time of the `init` function running.
+    /// If `T: Inherits<RefCounted>`, 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.
     ///

godot-core/src/obj/bounds.rs

@@ -398,11 +398,7 @@ impl Declarer for DeclEngine {
     where
         T: GodotDefault + Bounds<Declarer = Self>,
     {
-        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()
     }
 }
 

godot-core/src/obj/gd.rs

@@ -155,7 +155,7 @@ where
     where
         F: FnOnce(crate::obj::Base<T::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<T: GodotClass> Gd<T> {
         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<usize> {

godot-core/src/registry/callbacks.rs

@@ -35,14 +35,19 @@ pub unsafe extern "C" fn create<T: cap::GodotDefault>(
     _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<T: cap::GodotDefault>(
     _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 <https://github.com/godot-rust/gdext/issues/874> before Godot 4.5.
@@ -71,7 +76,7 @@ pub unsafe extern "C" fn recreate<T: cap::GodotDefault>(
     _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<T>(
 
 pub(crate) fn create_custom<T, F>(
     make_user_instance: F,
+    notify_postinitialize: bool,
 ) -> Result<sys::GDExtensionObjectPtr, PanicPayload>
 where
     T: GodotClass,
     F: FnOnce(Base<T::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::<Object>::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<T, F>(
+fn create_rust_part_for_existing_godot_part<T, F, P>(
     make_user_instance: F,
     base_ptr: sys::GDExtensionObjectPtr,
+    postinit: P,
 ) -> Result<sys::GDExtensionClassInstancePtr, PanicPayload>
 where
     T: GodotClass,
     F: FnOnce(Base<T::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();
 

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
 

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<RefCounted>,
+}
+
+#[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"))
+}

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,