Add setter to native structure's object pointer, allow to specify ref-count semantics

Author: Bromeon

godot-codegen/src/generator/native_structures.rs

@@ -173,20 +173,44 @@ fn make_native_structure_field_and_accessor(
     let (field_name, accessor);
     if is_object_ptr {
         // Highlight that the pointer field is internal/opaque.
-        field_name = format_ident!("__unused_{}_ptr", snake_field_name);
+        field_name = format_ident!("raw_{}_ptr", snake_field_name);
 
         // Generate method that converts from instance ID.
+        let getter_name = &snake_field_name;
+        let setter_name = format_ident!("set_{}", snake_field_name);
         let id_field_name = format_ident!("{}_id", snake_field_name);
+
         accessor = Some(quote! {
             /// Returns the object as a `Gd<T>`, or `None` if it no longer exists.
-            pub fn #snake_field_name(&self) -> Option<Gd<Object>> {
+            pub fn #getter_name(&self) -> Option<Gd<Object>> {
                 crate::obj::InstanceId::try_from_u64(self.#id_field_name.id)
                     .and_then(|id| Gd::try_from_instance_id(id).ok())
 
                 // Sanity check for consistency (if Some(...)):
                 // let ptr = self.#field_name as sys::GDExtensionObjectPtr;
                 // unsafe { Gd::from_obj_sys(ptr) }
             }
+
+            /// Sets the object from a `Gd<T>` pointer.
+            ///
+            /// `increment_refcount` is only relevant for ref-counted objects (inheriting `RefCounted`). It is ignored otherwise.
+            /// - Set it to true if you transfer `self` to Godot, e.g. via output parameter in a virtual function call.
+            ///   In this case, you can drop your own references and the object will remain alive.
+            ///   However, if you drop the native structure `self` without handing it over to Godot, you'll have a memory leak.
+            /// - Set it to false if you just manage the native structure yourself.
+            pub fn #setter_name(&mut self, mut obj: Gd<Object>, increment_refcount: bool) {
+                use crate::meta::GodotType as _;
+
+                assert!(obj.is_instance_valid(), "provided object is dead");
+
+                let id = obj.instance_id().to_u64();
+                if increment_refcount {
+                    obj = obj.with_inc_refcount();
+                }
+
+                self.#id_field_name = ObjectId { id };
+                self.#field_name = obj.obj_sys() as *mut std::ffi::c_void;
+            }
         });
     } else {
         field_name = snake_field_name;

godot-core/src/obj/gd.rs

@@ -380,6 +380,13 @@ impl<T: GodotClass> Gd<T> {
         }
     }
 
+    /// Returns a callable referencing a method from this object named `method_name`.
+    ///
+    /// This is shorter syntax for [`Callable::from_object_method(self, method_name)`][Callable::from_object_method].
+    pub fn callable<S: Into<StringName>>(&self, method_name: S) -> Callable {
+        Callable::from_object_method(self, method_name)
+    }
+
     pub(crate) unsafe fn from_obj_sys_or_none(
         ptr: sys::GDExtensionObjectPtr,
     ) -> Result<Self, ConvertError> {
@@ -418,11 +425,10 @@ impl<T: GodotClass> Gd<T> {
         self.raw.script_sys()
     }
 
-    /// Returns a callable referencing a method from this object named `method_name`.
-    ///
-    /// This is shorter syntax for [`Callable::from_object_method(self, method_name)`][Callable::from_object_method].
-    pub fn callable<S: Into<StringName>>(&self, method_name: S) -> Callable {
-        Callable::from_object_method(self, method_name)
+    pub(crate) fn with_inc_refcount(self) -> Self {
+        Self {
+            raw: self.raw.with_inc_refcount(),
+        }
     }
 }
 

godot-core/src/obj/raw.rs

@@ -82,7 +82,8 @@ impl<T: GodotClass> RawGd<T> {
     }
 
     /// Returns `self` but with initialized ref-count.
-    fn with_inc_refcount(mut self) -> Self {
+    // Could be private if not for Gd::with_inc_refcount(), which is used by native structure setters.
+    pub(crate) fn with_inc_refcount(mut self) -> Self {
         // Note: use init_ref and not inc_ref, since this might be the first reference increment.
         // Godot expects RefCounted::init_ref to be called instead of RefCounted::reference in that case.
         // init_ref also doesn't hurt (except 1 possibly unnecessary check).

itest/rust/src/engine_tests/native_structures_test.rs

@@ -12,8 +12,8 @@ use godot::classes::native::{
     AudioFrame, CaretInfo, Glyph, ObjectId, PhysicsServer2DExtensionShapeResult,
 };
 use godot::classes::text_server::Direction;
-use godot::classes::{ITextServerExtension, Node3D, TextServer, TextServerExtension};
-use godot::obj::{Base, NewAlloc, NewGd};
+use godot::classes::{ITextServerExtension, Node3D, RefCounted, TextServer, TextServerExtension};
+use godot::obj::{Base, Gd, NewAlloc, NewGd};
 use godot::register::{godot_api, GodotClass};
 
 use std::cell::Cell;
@@ -194,21 +194,52 @@ fn native_structure_debug() {
 
 #[itest]
 fn native_structure_object_pointers() {
+    // Object.
     let object = Node3D::new_alloc();
-    let raw_object_ptr: *mut std::ffi::c_void = std::ptr::null_mut();
 
-    let result = PhysicsServer2DExtensionShapeResult {
+    let mut result = PhysicsServer2DExtensionShapeResult {
         rid: Rid::new(12),
-        collider_id: ObjectId {
-            id: object.instance_id().to_i64() as u64,
-        },
-        __unused_collider_ptr: raw_object_ptr,
+        collider_id: ObjectId { id: 0 },
+        raw_collider_ptr: std::ptr::null_mut(),
         shape: 0,
     };
 
+    result.set_collider(object.clone().upcast(), true);
+    assert_eq!(result.collider_id.id, object.instance_id().to_i64() as u64);
+
     let retrieved = result.collider();
     assert_eq!(retrieved, Some(object.clone().upcast()));
 
     object.free();
     assert_eq!(result.collider(), None);
+
+    // RefCounted, increment ref-count.
+    let object = RefCounted::new_gd();
+    let id = object.instance_id();
+    result.set_collider(object.clone().upcast(), true);
+    assert_eq!(result.collider_id.id, id.to_i64() as u64);
+
+    drop(object); // Test if Godot keeps ref-count.
+
+    let retrieved = result
+        .collider()
+        .expect("Ref-counted objects don't drop if ref-count is incremented");
+    assert_eq!(retrieved.instance_id(), id);
+
+    // Manually decrement refcount (method unexposed).
+    Gd::<RefCounted>::from_instance_id(id).call("unreference".into(), &[]);
+
+    // RefCounted, do NOT increment ref-count.
+    let object = RefCounted::new_gd();
+    let id = object.instance_id();
+    result.set_collider(object.clone().upcast(), false);
+    assert_eq!(result.collider_id.id, id.to_i64() as u64);
+
+    drop(object); // Test if Godot keeps ref-count.
+
+    let retrieved = result.collider();
+    assert!(
+        retrieved.is_none(),
+        "Ref-counted objects drop if ref-count is not incremented"
+    );
 }