Merge #115

115: Implement static methods for engine + builtin classes r=Bromeon a=Bromeon

Also fixes a memory leak when `RefCounted` instances are returned from engine methods.

Closes #43.


Co-authored-by: Jan Haller <[email protected]>

Author: bors[bot]

godot-codegen/src/api_parser.rs

@@ -172,7 +172,7 @@ pub struct ClassMethod {
     pub name: String,
     pub is_const: bool,
     pub is_vararg: bool,
-    //pub is_static: bool,
+    pub is_static: bool,
     pub is_virtual: bool,
     pub hash: Option<i64>,
     pub return_value: Option<MethodReturn>,

godot-codegen/src/class_generator.rs

@@ -510,20 +510,21 @@ fn make_method_definition(
     /*if method.map_args(|args| args.is_empty()) {
         // Getters (i.e. 0 arguments) will be stripped of their `get_` prefix, to conform to Rust convention
         if let Some(remainder) = method_name.strip_prefix("get_") {
-            // Do not apply for get_16 etc
-            // TODO also not for get_u16 etc, in StreamPeer
+            // TODO Do not apply for FileAccess::get_16, StreamPeer::get_u16, etc
             if !remainder.chars().nth(0).unwrap().is_ascii_digit() {
                 method_name = remainder;
             }
         }
     }*/
 
     let method_name_str = special_cases::maybe_renamed(class_name, &method.name);
-    let receiver = if method.is_const {
-        quote! { &self, }
-    } else {
-        quote! { &mut self, }
-    };
+
+    let (receiver, receiver_arg) = make_receiver(
+        method.is_static,
+        method.is_const,
+        quote! { self.object_ptr },
+    );
+
     let hash = method.hash;
     let is_varcall = method.is_vararg;
 
@@ -546,10 +547,10 @@ fn make_method_definition(
         let __call_fn = sys::interface_fn!(#function_provider);
     };
     let varcall_invocation = quote! {
-        __call_fn(__method_bind, self.object_ptr, __args_ptr, __args.len() as i64, return_ptr, std::ptr::addr_of_mut!(__err));
+        __call_fn(__method_bind, #receiver_arg, __args_ptr, __args.len() as i64, return_ptr, std::ptr::addr_of_mut!(__err));
     };
     let ptrcall_invocation = quote! {
-        __call_fn(__method_bind, self.object_ptr, __args_ptr, return_ptr);
+        __call_fn(__method_bind, #receiver_arg, __args_ptr, return_ptr);
     };
 
     make_function_definition(
@@ -579,11 +580,8 @@ fn make_builtin_method_definition(
 
     let method_name_str = &method.name;
 
-    let receiver = if method.is_const {
-        quote! { &self, }
-    } else {
-        quote! { &mut self, }
-    };
+    let (receiver, receiver_arg) =
+        make_receiver(method.is_static, method.is_const, quote! { self.sys_ptr });
 
     let return_value = method.return_type.as_deref().map(MethodReturn::from_type);
     let hash = method.hash;
@@ -602,7 +600,7 @@ fn make_builtin_method_definition(
         let __call_fn = __call_fn.unwrap_unchecked();
     };
     let ptrcall_invocation = quote! {
-        __call_fn(self.sys_ptr, __args_ptr, return_ptr, __args.len() as i32);
+        __call_fn(#receiver_arg, __args_ptr, return_ptr, __args.len() as i32);
     };
 
     make_function_definition(
@@ -746,6 +744,28 @@ fn make_function_definition(
     }
 }
 
+fn make_receiver(
+    is_static: bool,
+    is_const: bool,
+    receiver_arg: TokenStream,
+) -> (TokenStream, TokenStream) {
+    let receiver = if is_static {
+        quote! {}
+    } else if is_const {
+        quote! { &self, }
+    } else {
+        quote! { &mut self, }
+    };
+
+    let receiver_arg = if is_static {
+        quote! { std::ptr::null_mut() }
+    } else {
+        receiver_arg
+    };
+
+    (receiver, receiver_arg)
+}
+
 fn make_params(
     method_args: &Option<Vec<MethodArg>>,
     is_varcall: bool,

godot-core/src/builtin/color.rs

@@ -17,7 +17,6 @@ pub struct Color {
 }
 
 impl Color {
-    #[allow(dead_code)]
     pub fn new(r: f32, g: f32, b: f32, a: f32) -> Self {
         Self { r, g, b, a }
     }

godot-core/src/obj/gd.rs

@@ -488,6 +488,9 @@ impl<T: GodotClass> Gd<T> {
     /// Runs `init_fn` on the address of a pointer (initialized to null). If that pointer is still null after the `init_fn` call,
     /// then `None` will be returned; otherwise `Gd::from_obj_sys(ptr)`.
     ///
+    /// This method will **NOT** increment the reference-count of the object, as it assumes the input to come from a Godot API
+    /// return value.
+    ///
     /// # Safety
     /// `init_fn` must be a function that correctly handles a _type pointer_ pointing to an _object pointer_.
     #[doc(hidden)]
@@ -496,7 +499,9 @@ impl<T: GodotClass> Gd<T> {
 
         // Initialize pointer with given function, return Some(ptr) on success and None otherwise
         let object_ptr = raw_object_init(init_fn);
-        sys::ptr_then(object_ptr, |ptr| Gd::from_obj_sys(ptr))
+
+        // Do not increment ref-count; assumed to be return value from FFI.
+        sys::ptr_then(object_ptr, |ptr| Gd::from_obj_sys_weak(ptr))
     }
 }
 

itest/rust/src/codegen_test.rs

@@ -4,17 +4,20 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-// This file tests the presence and naming of generated symbols, not their functionality.
+// This file tests the presence, naming and accessibility of generated symbols.
+// Functionality is only tested on a superficial level (to make sure general FFI mechanisms work).
 
 use crate::itest;
-
-use godot::engine::HttpRequest;
+use godot::builtin::inner::{InnerColor, InnerString};
+use godot::engine::{FileAccess, HttpRequest};
 use godot::prelude::*;
 
 pub fn run() -> bool {
     let mut ok = true;
     ok &= codegen_class_renamed();
     ok &= codegen_base_renamed();
+    ok &= codegen_static_builtin_method();
+    ok &= codegen_static_class_method();
     ok
 }
 
@@ -36,6 +39,28 @@ fn codegen_base_renamed() {
     obj.free();
 }
 
+#[itest]
+fn codegen_static_builtin_method() {
+    let pi = InnerString::num(std::f64::consts::PI, 3);
+    assert_eq!(pi, GodotString::from("3.142"));
+
+    let col = InnerColor::html("#663399cc".into());
+    assert_eq!(col, Color::new(0.4, 0.2, 0.6, 0.8));
+}
+
+#[itest]
+fn codegen_static_class_method() {
+    let exists = FileAccess::file_exists("inexistent".into());
+    assert!(!exists);
+
+    let exists = FileAccess::file_exists("res://itest.gdextension".into());
+    assert!(exists);
+
+    // see also object_test for reference count verification
+}
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
 #[derive(GodotClass)]
 #[class(base=HttpRequest)]
 pub struct TestBaseRenamed {

itest/rust/src/object_test.rs

@@ -7,7 +7,7 @@
 use crate::{expect_panic, itest};
 use godot::bind::{godot_api, GodotClass, GodotExt};
 use godot::builtin::{FromVariant, GodotString, StringName, ToVariant, Variant, Vector3};
-use godot::engine::{Node, Node3D, Object, RefCounted};
+use godot::engine::{file_access, FileAccess, Node, Node3D, Object, RefCounted};
 use godot::obj::Share;
 use godot::obj::{Base, Gd, InstanceId};
 use godot::sys::GodotFfi;
@@ -38,6 +38,7 @@ pub fn run() -> bool {
     ok &= object_engine_convert_variant();
     ok &= object_user_convert_variant_refcount();
     ok &= object_engine_convert_variant_refcount();
+    ok &= object_engine_returned_refcount();
     ok &= object_engine_up_deref();
     ok &= object_engine_up_deref_mut();
     ok &= object_engine_upcast();
@@ -265,6 +266,17 @@ fn check_convert_variant_refcount(obj: Gd<RefCounted>) {
     assert_eq!(obj.get_reference_count(), 1);
 }
 
+#[itest]
+fn object_engine_returned_refcount() {
+    let Some(file) = FileAccess::open("res://itest.gdextension".into(), file_access::ModeFlags::READ) else {
+        panic!("failed to open file used to test FileAccess")
+    };
+    assert!(file.is_open());
+
+    // There was a bug which incremented ref-counts of just-returned objects, keep this as regression test.
+    assert_eq!(file.get_reference_count(), 1);
+}
+
 #[itest]
 fn object_engine_up_deref() {
     let node3d: Gd<Node3D> = Node3D::new_alloc();