Merge pull request #396 from mhoff12358/mhoff12358/gd_as_self_argument

Allow godot-visible functions to receive a Gd instead of self.

Author: Bromeon

godot-core/src/obj/base.rs

@@ -35,6 +35,12 @@ pub struct Base<T: GodotClass> {
 }
 
 impl<T: GodotClass> Base<T> {
+    /// # Safety
+    /// The returned Base is a weak pointer, so holding it will not keep the object alive. It must not be accessed after the object is destroyed.
+    pub(crate) unsafe fn from_base(base: &Base<T>) -> Base<T> {
+        Base::from_obj(Gd::from_obj_sys_weak(base.obj_sys()))
+    }
+
     // Note: not &mut self, to only borrow one field and not the entire struct
     pub(crate) unsafe fn from_sys(base_ptr: sys::GDExtensionObjectPtr) -> Self {
         assert!(!base_ptr.is_null(), "instance base is null pointer");

godot-core/src/registry.rs

@@ -347,9 +347,9 @@ pub mod callbacks {
             unsafe { interface_fn!(classdb_construct_object)(base_class_name.string_sys()) };
 
         let base = unsafe { Base::from_sys(base_ptr) };
-        let user_instance = make_user_instance(base);
+        let user_instance = make_user_instance(unsafe { Base::from_base(&base) });
 
-        let instance = InstanceStorage::<T>::construct(user_instance);
+        let instance = InstanceStorage::<T>::construct(user_instance, base);
         let instance_ptr = instance.into_raw();
         let instance_ptr = instance_ptr as sys::GDExtensionClassInstancePtr;
 

godot-core/src/storage.rs

@@ -29,14 +29,15 @@ mod single_threaded {
     use std::any::type_name;
     use std::cell;
 
-    use crate::obj::GodotClass;
+    use crate::obj::{Base, Gd, GodotClass, Inherits};
     use crate::out;
 
     use super::Lifecycle;
 
     /// Manages storage and lifecycle of user's extension class instances.
     pub struct InstanceStorage<T: GodotClass> {
         user_instance: cell::RefCell<T>,
+        base: Base<T::Base>,
 
         // Declared after `user_instance`, is dropped last
         pub lifecycle: cell::Cell<Lifecycle>,
@@ -45,11 +46,12 @@ mod single_threaded {
 
     /// For all Godot extension classes
     impl<T: GodotClass> InstanceStorage<T> {
-        pub fn construct(user_instance: T) -> Self {
+        pub fn construct(user_instance: T, base: Base<T::Base>) -> Self {
             out!("    Storage::construct             <{}>", type_name::<T>());
 
             Self {
                 user_instance: cell::RefCell::new(user_instance),
+                base,
                 lifecycle: cell::Cell::new(Lifecycle::Alive),
                 godot_ref_count: cell::Cell::new(1),
             }
@@ -101,6 +103,13 @@ mod single_threaded {
             })
         }
 
+        pub fn get_gd(&self) -> Gd<T>
+        where
+            T: Inherits<<T as GodotClass>::Base>,
+        {
+            self.base.clone().cast()
+        }
+
         pub(super) fn godot_ref_count(&self) -> u32 {
             self.godot_ref_count.get()
         }
@@ -113,7 +122,7 @@ mod multi_threaded {
     use std::sync;
     use std::sync::atomic::{AtomicU32, Ordering};
 
-    use crate::obj::GodotClass;
+    use crate::obj::{Base, Gd, GodotClass, Inherits, Share};
     use crate::out;
 
     use super::Lifecycle;
@@ -146,6 +155,7 @@ mod multi_threaded {
     /// Manages storage and lifecycle of user's extension class instances.
     pub struct InstanceStorage<T: GodotClass> {
         user_instance: sync::RwLock<T>,
+        base: Base<T::Base>,
 
         // Declared after `user_instance`, is dropped last
         pub lifecycle: AtomicLifecycle,
@@ -154,11 +164,12 @@ mod multi_threaded {
 
     /// For all Godot extension classes
     impl<T: GodotClass> InstanceStorage<T> {
-        pub fn construct(user_instance: T) -> Self {
+        pub fn construct(user_instance: T, base: Base<T::Base>) -> Self {
             out!("    Storage::construct             <{}>", type_name::<T>());
 
             Self {
                 user_instance: sync::RwLock::new(user_instance),
+                base,
                 lifecycle: AtomicLifecycle::new(Lifecycle::Alive),
                 godot_ref_count: AtomicU32::new(1),
             }
@@ -206,6 +217,13 @@ mod multi_threaded {
             })
         }
 
+        pub fn get_gd(&self) -> Gd<T>
+        where
+            T: Inherits<<T as GodotClass>::Base>,
+        {
+            self.base.clone().cast()
+        }
+
         pub(super) fn godot_ref_count(&self) -> u32 {
             self.godot_ref_count.load(Ordering::Relaxed)
         }

godot-macros/src/class/data_models/field_var.rs

@@ -195,6 +195,7 @@ impl GetterSetterImpl {
             FuncDefinition {
                 func: signature,
                 rename: None,
+                has_gd_self: false,
             },
         );
 

godot-macros/src/class/data_models/func.rs

@@ -14,6 +14,7 @@ pub struct FuncDefinition {
     pub func: venial::Function,
     /// The name the function will be exposed as in Godot. If `None`, the Rust function name is used.
     pub rename: Option<String>,
+    pub has_gd_self: bool,
 }
 
 /// Returns a C function which acts as the callback when a virtual method of this instance is invoked.
@@ -24,7 +25,7 @@ pub fn make_virtual_method_callback(
     class_name: &Ident,
     method_signature: &venial::Function,
 ) -> TokenStream {
-    let signature_info = get_signature_info(method_signature);
+    let signature_info = get_signature_info(method_signature, false);
     let method_name = &method_signature.name;
 
     let wrapped_method = make_forwarding_closure(class_name, &signature_info);
@@ -54,7 +55,7 @@ pub fn make_method_registration(
     class_name: &Ident,
     func_definition: FuncDefinition,
 ) -> TokenStream {
-    let signature_info = get_signature_info(&func_definition.func);
+    let signature_info = get_signature_info(&func_definition.func, func_definition.has_gd_self);
     let sig_tuple =
         util::make_signature_tuple_type(&signature_info.ret_type, &signature_info.param_types);
 
@@ -127,6 +128,7 @@ pub fn make_method_registration(
 enum ReceiverType {
     Ref,
     Mut,
+    GdSelf,
     Static,
 }
 
@@ -167,6 +169,18 @@ fn make_forwarding_closure(class_name: &Ident, signature_info: &SignatureInfo) -
                 }
             }
         }
+        ReceiverType::GdSelf => {
+            quote! {
+                |instance_ptr, params| {
+                    let ( #(#params,)* ) = params;
+
+                    let storage =
+                        unsafe { ::godot::private::as_storage::<#class_name>(instance_ptr) };
+
+                    <#class_name>::#method_name(storage.get_gd(), #(#params),*)
+                }
+            }
+        }
         ReceiverType::Static => {
             quote! {
                 |_, params| {
@@ -178,9 +192,13 @@ fn make_forwarding_closure(class_name: &Ident, signature_info: &SignatureInfo) -
     }
 }
 
-fn get_signature_info(signature: &venial::Function) -> SignatureInfo {
+fn get_signature_info(signature: &venial::Function, has_gd_self: bool) -> SignatureInfo {
     let method_name = signature.name.clone();
-    let mut receiver_type = ReceiverType::Static;
+    let mut receiver_type = if has_gd_self {
+        ReceiverType::GdSelf
+    } else {
+        ReceiverType::Static
+    };
     let mut param_idents: Vec<Ident> = Vec::new();
     let mut param_types = Vec::new();
     let ret_type = match &signature.return_ty {
@@ -192,6 +210,11 @@ fn get_signature_info(signature: &venial::Function) -> SignatureInfo {
     for (arg, _) in &signature.params.inner {
         match arg {
             venial::FnParam::Receiver(recv) => {
+                if receiver_type == ReceiverType::GdSelf {
+                    // This shouldn't happen, as when has_gd_self is true the first function parameter should have been removed.
+                    // And the first parameter should be the only one that can be a Receiver.
+                    panic!("has_gd_self is true for a signature starting with a Receiver param.");
+                }
                 receiver_type = if recv.tk_mut.is_some() {
                     ReceiverType::Mut
                 } else if recv.tk_ref.is_some() {
@@ -228,7 +251,7 @@ fn get_signature_info(signature: &venial::Function) -> SignatureInfo {
 
 fn make_method_flags(method_type: ReceiverType) -> TokenStream {
     match method_type {
-        ReceiverType::Ref | ReceiverType::Mut => {
+        ReceiverType::Ref | ReceiverType::Mut | ReceiverType::GdSelf => {
             quote! { ::godot::engine::global::MethodFlags::METHOD_FLAGS_DEFAULT }
         }
         ReceiverType::Static => {

godot-macros/src/class/godot_api.rs

@@ -47,7 +47,10 @@ pub fn attribute_godot_api(input_decl: Declaration) -> Result<TokenStream, Error
 
 /// Attribute for user-declared function
 enum BoundAttrType {
-    Func { rename: Option<String> },
+    Func {
+        rename: Option<String>,
+        has_gd_self: bool,
+    },
     Signal(AttributeValue),
     Const(AttributeValue),
 }
@@ -230,11 +233,25 @@ fn process_godot_fns(decl: &mut Impl) -> Result<(Vec<FuncDefinition>, Vec<Functi
                 return attr.bail("generic fn parameters are not supported", method);
             }
 
-            match attr.ty {
-                BoundAttrType::Func { rename } => {
+            match &attr.ty {
+                BoundAttrType::Func {
+                    rename,
+                    has_gd_self,
+                } => {
                     // Signatures are the same thing without body
-                    let sig = util::reduce_to_signature(method);
-                    func_definitions.push(FuncDefinition { func: sig, rename });
+                    let mut sig = util::reduce_to_signature(method);
+                    if *has_gd_self {
+                        if sig.params.is_empty() {
+                            return attr.bail("with attribute key `gd_self`, the method must have a first parameter of type Gd<Self>", method);
+                        } else {
+                            sig.params.inner.remove(0);
+                        }
+                    }
+                    func_definitions.push(FuncDefinition {
+                        func: sig,
+                        rename: rename.clone(),
+                        has_gd_self: *has_gd_self,
+                    });
                 }
                 BoundAttrType::Signal(ref _attr_val) => {
                     if method.return_ty.is_some() {
@@ -317,11 +334,15 @@ where
                 let mut parser = KvParser::parse(attributes, "func")?.unwrap();
 
                 let rename = parser.handle_expr("rename")?.map(|ts| ts.to_string());
+                let has_gd_self = parser.handle_alone("gd_self")?;
 
                 Some(BoundAttr {
                     attr_name: attr_name.clone(),
                     index,
-                    ty: BoundAttrType::Func { rename },
+                    ty: BoundAttrType::Func {
+                        rename,
+                        has_gd_self,
+                    },
                 })
             }
             name if name == "signal" => {

itest/godot/ManualFfiTests.gd

@@ -304,3 +304,25 @@ func test_func_rename():
 	assert_eq(func_rename.has_method("renamed_static"), false)
 	assert_eq(func_rename.has_method("spell_static"), true)
 	assert_eq(func_rename.spell_static(), "static")
+
+var gd_self_reference: GdSelfReference
+func update_self_reference(value):
+	gd_self_reference.update_internal(value)
+
+# Todo: Once there is a way to assert for a SCRIPT ERROR failure this can be reenabled.
+"""
+func test_gd_self_reference_fails():
+	# Create the gd_self_reference and connect its signal to a gdscript method that calls back into it.
+	gd_self_reference = GdSelfReference.new()
+	gd_self_reference.update_internal_signal.connect(update_self_reference)
+	
+	# The returned value will still be 0 because update_internal can't be called in update_self_reference due to a borrowing issue.
+	assert_eq(gd_self_reference.fail_to_update_internal_value_due_to_conflicting_borrow(10), 0)
+"""
+
+func test_gd_self_reference_succeeds():
+	# Create the gd_self_reference and connect its signal to a gdscript method that calls back into it.
+	gd_self_reference = GdSelfReference.new()
+	gd_self_reference.update_internal_signal.connect(update_self_reference)
+
+	assert_eq(gd_self_reference.succeed_at_updating_internal_value(10), 10)

itest/rust/src/register_tests/func_test.rs

@@ -7,7 +7,7 @@
 use godot::prelude::*;
 
 #[derive(GodotClass)]
-#[class(base=RefCounted)]
+#[class(init, base=RefCounted)]
 struct FuncRename;
 
 #[godot_api]
@@ -40,9 +40,70 @@ impl FuncRename {
     }
 }
 
+#[derive(GodotClass)]
+#[class(base=RefCounted)]
+struct GdSelfReference {
+    internal_value: i32,
+
+    #[base]
+    base: Base<RefCounted>,
+}
+
+#[godot_api]
+impl GdSelfReference {
+    // A signal that will be looped back to update_internal through gdscript.
+    #[signal]
+    fn update_internal_signal(new_internal: i32);
+
+    #[func]
+    fn update_internal(&mut self, new_value: i32) {
+        self.internal_value = new_value;
+    }
+
+    #[func]
+    fn fail_to_update_internal_value_due_to_conflicting_borrow(
+        &mut self,
+        new_internal: i32,
+    ) -> i32 {
+        // Since a self reference is held while the signal is emitted, when
+        // GDScript tries to call update_internal(), there will be a failure due
+        // to the double borrow and self.internal_value won't be changed.
+        self.base.emit_signal(
+            "update_internal_signal".into(),
+            &[new_internal.to_variant()],
+        );
+        self.internal_value
+    }
+
+    #[func(gd_self)]
+    fn succeed_at_updating_internal_value(mut this: Gd<Self>, new_internal: i32) -> i32 {
+        // Since this isn't bound while the signal is emitted, GDScript will succeed at calling
+        // update_internal() and self.internal_value will be changed.
+        this.emit_signal(
+            "update_internal_signal".into(),
+            &[new_internal.to_variant()],
+        );
+        return this.bind().internal_value;
+    }
+
+    #[func(gd_self)]
+    fn takes_gd_as_equivalent(mut this: Gd<GdSelfReference>) -> bool {
+        this.bind_mut();
+        true
+    }
+
+    #[func(gd_self)]
+    fn takes_gd_as_self_no_return_type(this: Gd<GdSelfReference>) {
+        this.bind();
+    }
+}
+
 #[godot_api]
-impl RefCountedVirtual for FuncRename {
-    fn init(_base: Base<Self::Base>) -> Self {
-        Self
+impl RefCountedVirtual for GdSelfReference {
+    fn init(base: Base<Self::Base>) -> Self {
+        Self {
+            internal_value: 0,
+            base,
+        }
     }
 }