Merge #970

970: Generate accessor methods for indexed properties r=Bromeon a=chitoyuu

Getters and setters are now generated for indexed properties that are naturally accessible in GDScript (i.e. do not have '/' in the name), like `SpatialMaterial::albedo_texture`. Doc generation for the underlying accessors has also been improved.

Close #689

Co-authored-by: Chitose Yuuzaki <chitoyuu@potatoes.gay>

bindings-generator/src/class_docs.rs

@@ -44,15 +44,17 @@ impl GodotXmlDocs {
             .find(|node| node.tag_name().name() == "class")
         {
             if let Some(class_name) = class.attribute("name") {
-                let methods_node = class
-                    .descendants()
-                    .find(|node| node.tag_name().name() == "methods");
-                self.parse_methods(class_name, methods_node);
-
+                // Parse members first, so more general docs for indexed accessors can be used
+                // if they exist.
                 let members_node = class
                     .descendants()
                     .find(|node| node.tag_name().name() == "members");
                 self.parse_members(class_name, members_node);
+
+                let methods_node = class
+                    .descendants()
+                    .find(|node| node.tag_name().name() == "methods");
+                self.parse_methods(class_name, methods_node);
             }
         }
     }
@@ -62,6 +64,26 @@ impl GodotXmlDocs {
             for node in members.descendants() {
                 if node.tag_name().name() == "member" {
                     if let Some(desc) = node.text() {
+                        if let Some(property_name) = node.attribute("name") {
+                            if !property_name.contains('/') {
+                                if node.has_attribute("setter") {
+                                    self.add_fn(
+                                        class,
+                                        &format!("set_{}", property_name),
+                                        desc,
+                                        &[],
+                                    );
+                                }
+                                if node.has_attribute("getter") {
+                                    self.add_fn(
+                                        class,
+                                        &format!("get_{}", property_name),
+                                        desc,
+                                        &[],
+                                    );
+                                }
+                            }
+                        }
                         if let Some(func) = node.attribute("setter") {
                             self.add_fn(class, func, desc, &[]);
                         }

bindings-generator/src/methods.rs

@@ -6,9 +6,9 @@ use proc_macro2::TokenStream;
 use quote::{format_ident, quote};
 
 use std::collections::HashMap;
-use std::collections::HashSet;
 
 /// Types of icalls.
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 pub(crate) enum IcallType {
     Ptr,
     Varargs,
@@ -265,6 +265,14 @@ pub(crate) fn generate_methods(
     icalls: &mut HashMap<String, MethodSig>,
     docs: Option<&GodotXmlDocs>,
 ) -> TokenStream {
+    /// Memorized information about generated methods. Used to generate indexed property accessors.
+    struct Generated {
+        icall: proc_macro2::Ident,
+        icall_ty: IcallType,
+        maybe_unsafe: TokenStream,
+        maybe_unsafe_reason: &'static str,
+    }
+
     // Brings values of some types to a type with less information.
     fn arg_erase(ty: &Ty, name: &proc_macro2::Ident) -> TokenStream {
         match ty {
@@ -280,11 +288,14 @@ pub(crate) fn generate_methods(
 
             Ty::Object(_) => quote! { #name.as_arg_ptr() },
 
+            // Allow lossy casting of numeric types into similar primitives, see also to_return_post
+            Ty::I64 | Ty::F64 | Ty::Bool => quote! { #name as _ },
+
             _ => quote! { #name },
         }
     }
 
-    let mut method_set = HashSet::new();
+    let mut generated = HashMap::new();
     let mut result = TokenStream::new();
 
     for method in &class.methods {
@@ -301,11 +312,9 @@ pub(crate) fn generate_methods(
         let mut rust_ret_type = ret_type.to_rust();
 
         // Ensure that methods are not injected several times.
-        let method_name_string = method_name.to_string();
-        if method_set.contains(&method_name_string) {
+        if generated.contains_key(method_name) {
             continue;
         }
-        method_set.insert(method_name_string);
 
         let mut params_decl = TokenStream::new();
         let mut params_use = TokenStream::new();
@@ -387,7 +396,126 @@ pub(crate) fn generate_methods(
                 }
             }
         };
+
         result.extend(output);
+
+        generated.insert(
+            method_name.to_string(),
+            Generated {
+                icall,
+                icall_ty,
+                maybe_unsafe,
+                maybe_unsafe_reason,
+            },
+        );
+    }
+
+    for property in &class.properties {
+        if property.index < 0 || property.name.contains('/') {
+            continue;
+        }
+
+        let property_index = property.index;
+        let ty = Ty::from_src(&property.type_);
+
+        if let Some(Generated {
+            icall,
+            icall_ty,
+            maybe_unsafe,
+            maybe_unsafe_reason,
+        }) = generated.get(&property.getter)
+        {
+            let rusty_name = rust_safe_name(&property.name);
+            let rust_ret_type = ty.to_rust();
+
+            let method_bind_fetch = {
+                let method_table = format_ident!("{}MethodTable", class.name);
+                let rust_method_name = format_ident!("{}", property.getter);
+
+                quote! {
+                    let method_bind: *mut sys::godot_method_bind = #method_table::get(get_api()).#rust_method_name;
+                }
+            };
+
+            let doc_comment = docs
+                .and_then(|docs| {
+                    docs.get_class_method_desc(
+                        class.name.as_str(),
+                        &format!("get_{}", property.name),
+                    )
+                })
+                .unwrap_or("");
+
+            let recover = ret_recover(&ty, *icall_ty);
+
+            let output = quote! {
+                #[doc = #doc_comment]
+                #[doc = #maybe_unsafe_reason]
+                #[inline]
+                pub #maybe_unsafe fn #rusty_name(&self) -> #rust_ret_type {
+                    unsafe {
+                        #method_bind_fetch
+
+                        let ret = crate::icalls::#icall(method_bind, self.this.sys().as_ptr(), #property_index);
+
+                        #recover
+                    }
+                }
+            };
+
+            result.extend(output);
+        }
+
+        if let Some(Generated {
+            icall,
+            icall_ty,
+            maybe_unsafe,
+            maybe_unsafe_reason,
+        }) = generated.get(&property.setter)
+        {
+            let rusty_name = rust_safe_name(&format!("set_{}", property.name));
+
+            let rust_arg_ty = ty.to_rust_arg();
+            let arg_ident = format_ident!("value");
+            let arg_erased = arg_erase(&ty, &arg_ident);
+
+            let method_bind_fetch = {
+                let method_table = format_ident!("{}MethodTable", class.name);
+                let rust_method_name = format_ident!("{}", property.setter);
+
+                quote! {
+                    let method_bind: *mut sys::godot_method_bind = #method_table::get(get_api()).#rust_method_name;
+                }
+            };
+
+            let doc_comment = docs
+                .and_then(|docs| {
+                    docs.get_class_method_desc(
+                        class.name.as_str(),
+                        &format!("set_{}", property.name),
+                    )
+                })
+                .unwrap_or("");
+
+            let recover = ret_recover(&Ty::Void, *icall_ty);
+
+            let output = quote! {
+                #[doc = #doc_comment]
+                #[doc = #maybe_unsafe_reason]
+                #[inline]
+                pub #maybe_unsafe fn #rusty_name(&self, #arg_ident: #rust_arg_ty) {
+                    unsafe {
+                        #method_bind_fetch
+
+                        let ret = crate::icalls::#icall(method_bind, self.this.sys().as_ptr(), #property_index, #arg_erased);
+
+                        #recover
+                    }
+                }
+            };
+
+            result.extend(output);
+        }
     }
 
     result

test/src/lib.rs

@@ -9,6 +9,7 @@ mod test_async;
 mod test_constructor;
 mod test_derive;
 mod test_free_ub;
+mod test_indexed_props;
 mod test_map_owned;
 mod test_register;
 mod test_return_leak;
@@ -76,6 +77,7 @@ pub extern "C" fn run_tests(
     status &= test_constructor::run_tests();
     status &= test_derive::run_tests();
     status &= test_free_ub::run_tests();
+    status &= test_indexed_props::run_tests();
     status &= test_map_owned::run_tests();
     status &= test_register::run_tests();
     status &= test_return_leak::run_tests();
@@ -238,6 +240,7 @@ fn init(handle: InitHandle) {
     test_constructor::register(handle);
     test_derive::register(handle);
     test_free_ub::register(handle);
+    test_indexed_props::register(handle);
     test_map_owned::register(handle);
     test_register::register(handle);
     test_return_leak::register(handle);

test/src/test_indexed_props.rs

@@ -0,0 +1,32 @@
+use gdnative::core_types::Margin;
+use gdnative::prelude::*;
+
+pub(crate) fn run_tests() -> bool {
+    let mut status = true;
+
+    status &= test_indexed_props();
+
+    status
+}
+
+pub(crate) fn register(_handle: InitHandle) {}
+
+crate::godot_itest! { test_indexed_props {
+    let control = Control::new();
+
+    assert_eq!(0, control.margin_top());
+    assert_eq!(0, control.margin_left());
+
+    control.set_margin_top(42);
+
+    assert_eq!(42, control.margin_top());
+    assert_eq!(42, control.margin(Margin::Top.into()) as i64);
+    assert_eq!(0, control.margin_left());
+    assert_eq!(0, control.margin(Margin::Left.into()) as i64);
+
+    control.set_margin(Margin::Left.into(), 24.0);
+
+    assert_eq!(24, control.margin_left());
+
+    control.free();
+}}