Merge pull request #1271 from godot-rust/qol/varia

Nightly rustfmt + internals (once-calls, robust refcounts)

Author: Bromeon

.github/workflows/full-ci.yml

@@ -49,6 +49,7 @@ jobs:
       - name: "Install Rust"
         uses: ./.github/composite/rust
         with:
+          rust: nightly
           components: rustfmt
 
       - name: "Check rustfmt"

.github/workflows/minimal-ci.yml

@@ -47,6 +47,7 @@ jobs:
       - name: "Install Rust"
         uses: ./.github/composite/rust
         with:
+          rust: nightly
           components: rustfmt
 
       - name: "Check rustfmt"

.github/workflows/release-version.yml

@@ -120,7 +120,7 @@ jobs:
           ref: ${{ github.ref }}
 
       - name: "Install Rust (uncached)"
-        run: rustup update stable
+        run: rustup update nightly && rustup default nightly
 
       - name: "Check rustfmt"
         run: cargo fmt --all -- --check

godot-core/src/builtin/callable.rs

@@ -11,10 +11,10 @@ use godot_ffi as sys;
 use sys::{ffi_methods, ExtVariantType, GodotFfi};
 
 use crate::builtin::{inner, GString, StringName, Variant, VariantArray};
-use crate::classes;
 use crate::meta::{GodotType, ToGodot};
 use crate::obj::bounds::DynMemory;
 use crate::obj::{Bounds, Gd, GodotClass, InstanceId};
+use crate::{classes, meta};
 
 #[cfg(all(since_api = "4.2", before_api = "4.3"))]
 type CallableCustomInfo = sys::GDExtensionCallableCustomInfo;
@@ -181,6 +181,40 @@ impl Callable {
         })
     }
 
+    /// Create callable from **single-threaded** Rust function or closure that can only be called once.
+    ///
+    /// `name` is used for the string representation of the closure, which helps debugging.
+    ///
+    /// After the first invocation, subsequent calls will panic with a message indicating the callable has already been consumed. This is
+    /// useful for deferred operations that should only execute once. For repeated execution, use [`from_local_fn()][Self::from_local_fn].
+    #[cfg(since_api = "4.2")]
+    pub(crate) fn from_once_fn<F, S>(name: S, rust_function: F) -> Self
+    where
+        F: 'static + FnOnce(&[&Variant]) -> Result<Variant, ()>,
+        S: meta::AsArg<GString>,
+    {
+        meta::arg_into_owned!(name);
+
+        let mut rust_fn_once = Some(rust_function);
+        Self::from_local_fn(&name, move |args| {
+            let rust_fn_once = rust_fn_once
+                .take()
+                .expect("callable created with from_once_fn() has already been consumed");
+            rust_fn_once(args)
+        })
+    }
+
+    #[cfg(feature = "trace")] // Test only.
+    #[cfg(since_api = "4.2")]
+    #[doc(hidden)]
+    pub fn __once_fn<F, S>(name: S, rust_function: F) -> Self
+    where
+        F: 'static + FnOnce(&[&Variant]) -> Result<Variant, ()>,
+        S: meta::AsArg<GString>,
+    {
+        Self::from_once_fn(name, rust_function)
+    }
+
     #[cfg(since_api = "4.2")]
     pub(crate) fn with_scoped_fn<S, F, Fc, R>(name: S, rust_function: F, callable_usage: Fc) -> R
     where
@@ -518,8 +552,6 @@ pub use custom_callable::RustCallable;
 #[cfg(since_api = "4.2")]
 use custom_callable::*;
 
-use crate::meta;
-
 #[cfg(since_api = "4.2")]
 mod custom_callable {
     use std::hash::Hash;

godot-core/src/classes/class_runtime.rs

@@ -49,9 +49,12 @@ pub(crate) fn debug_string_variant(
             .expect("get_class() must be compatible with StringName");
 
         let refcount = id.is_ref_counted().then(|| {
-            obj.call("get_reference_count", &[])
+            let count = obj
+                .call("get_reference_count", &[])
                 .try_to_relaxed::<i32>()
-                .expect("get_reference_count() must return integer") as usize
+                .expect("get_reference_count() must return integer");
+
+            Ok(count as usize)
         });
 
         debug_string_parts(f, ty, id, class, refcount, None)
@@ -75,7 +78,11 @@ pub(crate) fn debug_string_variant(
             let class = obj.dynamic_class_string();
 
             // Refcount is off-by-one due to now-created Gd<T> from conversion; correct by -1.
-            let refcount = obj.maybe_refcount().map(|rc| rc.saturating_sub(1));
+            let refcount = match obj.maybe_refcount() {
+                Some(Ok(rc)) => Some(Ok(rc.saturating_sub(1))),
+                Some(Err(e)) => Some(Err(e)),
+                None => None,
+            };
 
             debug_string_parts(f, ty, id, class, refcount, None)
         }
@@ -121,7 +128,7 @@ fn debug_string_parts(
     ty: &str,
     id: InstanceId,
     class: StringName,
-    refcount: Option<usize>,
+    refcount: Option<Result<usize, ()>>,
     trait_name: Option<&str>,
 ) -> std::fmt::Result {
     let mut builder = f.debug_struct(ty);
@@ -133,8 +140,14 @@ fn debug_string_parts(
         builder.field("trait", &format_args!("{trait_name}"));
     }
 
-    if let Some(refcount) = refcount {
-        builder.field("refc", &refcount);
+    match refcount {
+        Some(Ok(refcount)) => {
+            builder.field("refc", &refcount);
+        }
+        Some(Err(_)) => {
+            builder.field("refc", &"(N/A during init or drop)");
+        }
+        None => {}
     }
 
     builder.finish()

godot-core/src/obj/call_deferred.rs

@@ -61,14 +61,11 @@ where
             is_main_thread(),
             "`apply_deferred` must be called on the main thread"
         );
-        let mut rust_fn_once = Some(rust_function);
+
         let mut this = self.to_signal_obj().clone();
-        let callable = Callable::from_local_fn("apply_deferred", move |_| {
-            let rust_fn_once = rust_fn_once
-                .take()
-                .expect("rust_fn_once was already consumed");
+        let callable = Callable::from_once_fn("apply_deferred", move |_| {
             let mut this_mut = T::object_as_mut(&mut this);
-            rust_fn_once(this_mut.deref_mut());
+            rust_function(this_mut.deref_mut());
             Ok(Variant::nil())
         });
         callable.call_deferred(&[]);

godot-core/src/obj/gd.rs

@@ -315,18 +315,32 @@ impl<T: GodotClass> Gd<T> {
     }
 
     /// Returns the reference count, if the dynamic object inherits `RefCounted`; and `None` otherwise.
-    pub(crate) fn maybe_refcount(&self) -> Option<usize> {
+    ///
+    /// Returns `Err(())` if obtaining reference count failed, due to being called during init/drop.
+    pub(crate) fn maybe_refcount(&self) -> Option<Result<usize, ()>> {
+        // May become infallible if implemented via call() on Object, if ref-count bit of instance ID is set.
+        // This would likely be more efficient, too.
+
         // Fast check if ref-counted without downcast.
-        self.instance_id().is_ref_counted().then(|| {
-            let rc = self.raw.with_ref_counted(|refc| refc.get_reference_count());
-            rc as usize
-        })
+        if !self.instance_id().is_ref_counted() {
+            return None;
+        }
+
+        // Optimization: call `get_reference_count()` directly. Might also increase reliability and obviate the need for Result.
+
+        let rc = self
+            .raw
+            .try_with_ref_counted(|refc| refc.get_reference_count());
+
+        Some(rc.map(|i| i as usize))
     }
 
     #[cfg(feature = "trace")] // itest only.
     #[doc(hidden)]
     pub fn test_refcount(&self) -> Option<usize> {
         self.maybe_refcount()
+            .transpose()
+            .expect("failed to obtain refcount")
     }
 
     /// **Upcast:** convert into a smart pointer to a base class. Always succeeds.
@@ -359,11 +373,20 @@ impl<T: GodotClass> Gd<T> {
             .expect("Upcast to Object failed. This is a bug; please report it.")
     }
 
+    // /// Equivalent to [`upcast_mut::<Object>()`][Self::upcast_mut], but without bounds.
+    // pub(crate) fn upcast_object_ref(&self) -> &classes::Object {
+    //     self.raw.as_object_ref()
+    // }
+
     /// Equivalent to [`upcast_mut::<Object>()`][Self::upcast_mut], but without bounds.
     pub(crate) fn upcast_object_mut(&mut self) -> &mut classes::Object {
         self.raw.as_object_mut()
     }
 
+    // pub(crate) fn upcast_object_mut_from_ref(&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
@@ -554,7 +577,10 @@ impl<T: GodotClass> Gd<T> {
     pub(crate) unsafe fn from_obj_sys_or_none(
         ptr: sys::GDExtensionObjectPtr,
     ) -> Result<Self, ConvertError> {
-        Self::try_from_ffi(RawGd::from_obj_sys(ptr))
+        // Used to have a flag to select RawGd::from_obj_sys_weak(ptr) for Base::to_init_gd(), but solved differently in the end.
+        let obj = RawGd::from_obj_sys(ptr);
+
+        Self::try_from_ffi(obj)
     }
 
     /// Initializes this `Gd<T>` from the object pointer as a **strong ref**, meaning

godot-core/src/obj/raw_gd.rs

@@ -195,7 +195,7 @@ impl<T: GodotClass> RawGd<T> {
     ///
     /// # Panics
     /// If `self` does not inherit `RefCounted` or is null.
-    pub(crate) fn with_ref_counted<R>(&self, apply: impl Fn(&mut classes::RefCounted) -> R) -> R {
+    pub fn with_ref_counted<R>(&self, apply: impl Fn(&mut classes::RefCounted) -> R) -> R {
         // Note: this previously called Declarer::scoped_mut() - however, no need to go through bind() for changes in base RefCounted.
         // Any accesses to user objects (e.g. destruction if refc=0) would bind anyway.
         //
@@ -204,8 +204,8 @@ impl<T: GodotClass> RawGd<T> {
         //     self.as_target_mut()
         // }
 
-        let mut ref_counted = match self.ffi_cast::<classes::RefCounted>() {
-            Ok(cast_success) => cast_success,
+        match self.try_with_ref_counted(apply) {
+            Ok(result) => result,
             Err(()) if self.is_null() => {
                 panic!("RawGd::with_ref_counted(): expected to inherit RefCounted, encountered null pointer");
             }
@@ -216,14 +216,25 @@ impl<T: GodotClass> RawGd<T> {
 
                 // One way how this may panic is when invoked during destruction of a RefCounted object. The C++ `Object::object_cast_to()`
                 // function is virtual but cannot be dynamically dispatched in a C++ destructor.
-                panic!("RawGd::with_ref_counted(): expected to inherit RefCounted, but encountered {class}");
+                panic!(
+                    "Operation not permitted for object of class {class}:\n\
+                    class is either not RefCounted, or currently in construction/destruction phase"
+                );
             }
-        };
+        }
+    }
 
+    /// Fallible version of [`with_ref_counted()`](Self::with_ref_counted), for situations during init/drop when downcast no longer works.
+    #[expect(clippy::result_unit_err)]
+    pub fn try_with_ref_counted<R>(
+        &self,
+        apply: impl Fn(&mut classes::RefCounted) -> R,
+    ) -> Result<R, ()> {
+        let mut ref_counted = self.ffi_cast::<classes::RefCounted>()?;
         let return_val = apply(ref_counted.as_dest_mut().as_target_mut());
 
         // CastSuccess is forgotten when dropped, so no ownership transfer.
-        return_val
+        Ok(return_val)
     }
 
     /// Enables outer `Gd` APIs or bypasses additional null checks, in cases where `RawGd` is guaranteed non-null.
@@ -716,7 +727,7 @@ impl<T: GodotClass> Clone for RawGd<T> {
     fn clone(&self) -> Self {
         out!("RawGd::clone:     {self:?}  (before clone)");
 
-        if self.is_null() {
+        let cloned = if self.is_null() {
             Self::null()
         } else {
             self.check_rtti("clone");
@@ -728,7 +739,10 @@ impl<T: GodotClass> Clone for RawGd<T> {
                 cached_storage_ptr: self.cached_storage_ptr.clone(),
             };
             copy.with_inc_refcount()
-        }
+        };
+
+        out!("                  {self:?}  (after clone)");
+        cloned
     }
 }
 

itest/rust/src/builtin_tests/containers/callable_test.rs

@@ -648,6 +648,20 @@ pub mod custom_callable {
         assert!(signal.is_connected(&identical_callable));
     }
 
+    #[cfg(since_api = "4.2")]
+    #[itest]
+    fn callable_from_once_fn() {
+        let callable = Callable::__once_fn("once_test", move |_| Ok(42.to_variant()));
+
+        // First call should succeed.
+        let result = callable.call(&[]);
+        assert_eq!(result.to::<i32>(), 42);
+
+        // Second call should fail (panic currently isn't propagated, see other tests).
+        let result = callable.call(&[]);
+        assert!(result.is_nil());
+    }
+
     // ------------------------------------------------------------------------------------------------------------------------------------------
     // Helper structs and functions for custom callables