Make Gd::maybe_refcount() more robust

Previously panicked during construction/destruction, as the downcast to RefCounted
no longer worked. No returns Result.

The Debug impl for object now falls back to printing a "N/A" message for `refc` if
the value cannot be obtained.

Author: Bromeon

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/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
     }
 }