Fix native structures exposing *mut Gd<Object>, provide accessor

Author: Bromeon

godot-codegen/src/conv/type_conversions.rs

@@ -84,13 +84,24 @@ fn to_hardcoded_rust_enum(ty: &str) -> Option<&str> {
 /// Maps an input type to a Godot type with the same C representation. This is subtly different than [`to_rust_type`],
 /// which maps to an appropriate corresponding Rust type. This function should be used in situations where the C ABI for
 /// a type must match the Godot equivalent exactly, such as when dealing with pointers.
-pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> RustTy {
-    match ty {
+pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> (RustTy, bool) {
+    let mut is_obj = false;
+    let ty = match ty {
+        // In native structures, object pointers are mapped to opaque entities. Instead, an accessor function is provided.
+        "Object*" => {
+            is_obj = true;
+            RustTy::RawPointer {
+                inner: Box::new(RustTy::BuiltinIdent(ident("c_void"))),
+                is_const: false,
+            }
+        }
         "int" => RustTy::BuiltinIdent(ident("i32")),
         "float" => RustTy::BuiltinIdent(ident("f32")),
         "double" => RustTy::BuiltinIdent(ident("f64")),
         _ => to_rust_type(ty, None, ctx),
-    }
+    };
+
+    (ty, is_obj)
 }
 
 /// Maps an _input_ type from the Godot JSON to the corresponding Rust type (wrapping some sort of a token stream).

godot-codegen/src/generator/native_structures.rs

@@ -11,7 +11,7 @@ use crate::models::domain::{ExtensionApi, ModName, NativeStructure, TyName};
 use crate::util::ident;
 use crate::{conv, util, SubmitFn};
 use proc_macro2::TokenStream;
-use quote::quote;
+use quote::{format_ident, quote};
 use std::path::Path;
 
 pub fn generate_native_structures_files(
@@ -52,8 +52,15 @@ pub fn generate_native_structures_files(
 
 #[derive(Clone, Eq, PartialEq, Debug)]
 pub(crate) struct NativeStructuresField {
+    /// Identifier for the field name, e.g. `collider`.
     pub field_name: String,
+
+    /// Type which has a raw format that is latter mapped to `RustTy`.
+    ///
+    /// Corresponds to other Godot type names, e.g. `Object*` or `enum::TextServer.Direction`.
     pub field_type: String,
+
+    /// If the field is an array, this contains the number of elements.
     pub array_size: Option<usize>,
 }
 
@@ -65,12 +72,13 @@ fn make_native_structure(
     let class_name = &class_name.rust_ty;
 
     let imports = util::make_imports();
-    let fields = make_native_structure_fields(&structure.format, ctx);
+    let (fields, methods) = make_native_structure_fields_and_methods(&structure.format, ctx);
     let doc = format!("[`ToGodot`] and [`FromGodot`] are implemented for `*mut {class_name}` and `*const {class_name}`.");
 
     // mod re_export needed, because class should not appear inside the file module, and we can't re-export private struct as pub
     let tokens = quote! {
         #imports
+        use std::ffi::c_void; // for opaque object pointers
         use crate::meta::{GodotConvert, FromGodot, ToGodot};
 
         /// Native structure; can be passed via pointer in APIs that are not exposed to GDScript.
@@ -82,6 +90,10 @@ fn make_native_structure(
             #fields
         }
 
+        impl #class_name {
+            #methods
+        }
+
         impl GodotConvert for *mut #class_name {
             type Via = i64;
         }
@@ -119,25 +131,35 @@ fn make_native_structure(
     builtins::GeneratedBuiltin { code: tokens }
 }
 
-fn make_native_structure_fields(format_str: &str, ctx: &mut Context) -> TokenStream {
+fn make_native_structure_fields_and_methods(
+    format_str: &str,
+    ctx: &mut Context,
+) -> (TokenStream, TokenStream) {
     let fields = parse_native_structures_format(format_str)
         .expect("Could not parse native_structures format field");
 
-    let field_definitions = fields
-        .into_iter()
-        .map(|field| make_native_structure_field_definition(field, ctx));
+    let mut field_definitions = vec![];
+    let mut accessors = vec![];
 
-    quote! {
-        #( #field_definitions )*
+    for field in fields {
+        let (field_def, accessor) = make_native_structure_field_and_accessor(field, ctx);
+        field_definitions.push(field_def);
+        if let Some(accessor) = accessor {
+            accessors.push(accessor);
+        }
     }
+
+    let fields = quote! { #( #field_definitions )* };
+    let methods = quote! { #( #accessors )* };
+    (fields, methods)
 }
 
-fn make_native_structure_field_definition(
+fn make_native_structure_field_and_accessor(
     field: NativeStructuresField,
     ctx: &mut Context,
-) -> TokenStream {
+) -> (TokenStream, Option<TokenStream>) {
     let field_type = normalize_native_structure_field_type(&field.field_type);
-    let field_type = conv::to_rust_type_abi(&field_type, ctx);
+    let (field_type, is_object_ptr) = conv::to_rust_type_abi(&field_type, ctx);
 
     // Make array if needed.
     let field_type = if let Some(size) = field.array_size {
@@ -146,11 +168,36 @@ fn make_native_structure_field_definition(
         quote! { #field_type }
     };
 
-    let field_name = ident(&conv::to_snake_case(&field.field_name));
+    let snake_field_name = ident(&conv::to_snake_case(&field.field_name));
+
+    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);
+
+        // Generate method that converts from instance ID.
+        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>> {
+                crate::obj::InstanceId::try_from_u64(self.#id_field_name.id)
+                    .and_then(|id| Gd::try_from_instance_id(id).ok())
 
-    quote! {
+                // Sanity check for consistency (if Some(...)):
+                // let ptr = self.#field_name as sys::GDExtensionObjectPtr;
+                // unsafe { Gd::from_obj_sys(ptr) }
+            }
+        });
+    } else {
+        field_name = snake_field_name;
+        accessor = None;
+    };
+
+    let field_def = quote! {
         pub #field_name: #field_type,
-    }
+    };
+
+    (field_def, accessor)
 }
 
 fn normalize_native_structure_field_type(field_type: &str) -> String {

itest/rust/src/engine_tests/native_structures_test.rs

@@ -8,10 +8,12 @@
 use crate::framework::itest;
 
 use godot::builtin::{Rect2, Rid, Variant};
-use godot::classes::native::{AudioFrame, CaretInfo, Glyph, ObjectId};
+use godot::classes::native::{
+    AudioFrame, CaretInfo, Glyph, ObjectId, PhysicsServer2DExtensionShapeResult,
+};
 use godot::classes::text_server::Direction;
-use godot::classes::{ITextServerExtension, TextServer, TextServerExtension};
-use godot::obj::{Base, NewGd};
+use godot::classes::{ITextServerExtension, Node3D, TextServer, TextServerExtension};
+use godot::obj::{Base, NewAlloc, NewGd};
 use godot::register::{godot_api, GodotClass};
 
 use std::cell::Cell;
@@ -71,7 +73,7 @@ impl ITextServerExtension for TestTextServer {
 }
 
 #[itest]
-fn test_native_structures_codegen() {
+fn native_structure_codegen() {
     // Test construction of a few simple types.
     let _ = AudioFrame {
         left: 0.0,
@@ -93,7 +95,7 @@ fn test_native_structures_codegen() {
 }
 
 #[itest]
-fn test_native_structure_out_parameter() {
+fn native_structure_out_parameter() {
     // Instantiate a TextServerExtension and then have Godot call a
     // function which uses an 'out' pointer parameter.
     let mut ext = TestTextServer::new_gd();
@@ -127,7 +129,7 @@ fn test_native_structure_out_parameter() {
 }
 
 #[itest]
-fn test_native_structure_pointer_to_array_parameter() {
+fn native_structure_pointer_to_array_parameter() {
     // Instantiate a TextServerExtension.
     let ext = TestTextServer::new_gd();
     let result = ext
@@ -142,7 +144,7 @@ fn test_native_structure_pointer_to_array_parameter() {
 }
 
 #[itest]
-fn test_native_structure_clone() {
+fn native_structure_clone() {
     // Instantiate CaretInfo directly.
     let caret1 = CaretInfo {
         leading_caret: Rect2::from_components(0.0, 0.0, 0.0, 0.0),
@@ -169,15 +171,15 @@ fn test_native_structure_clone() {
 }
 
 #[itest]
-fn test_native_structure_partialeq() {
+fn native_structure_partialeq() {
     // Test basic equality between two identically-constructed
     // (but distinct) native structures.
     assert_eq!(sample_glyph(5), sample_glyph(5));
     assert_ne!(sample_glyph(1), sample_glyph(2));
 }
 
 #[itest]
-fn test_native_structure_debug() {
+fn native_structure_debug() {
     // Test debug output, both pretty-printed and not.
     let object_id = ObjectId { id: 256 };
     assert_eq!(
@@ -189,3 +191,24 @@ fn test_native_structure_debug() {
         String::from("ObjectId {\n    id: 256,\n}")
     );
 }
+
+#[itest]
+fn native_structure_object_pointers() {
+    let object = Node3D::new_alloc();
+    let raw_object_ptr: *mut std::ffi::c_void = std::ptr::null_mut();
+
+    let result = PhysicsServer2DExtensionShapeResult {
+        rid: Rid::new(12),
+        collider_id: ObjectId {
+            id: object.instance_id().to_i64() as u64,
+        },
+        __unused_collider_ptr: raw_object_ptr,
+        shape: 0,
+    };
+
+    let retrieved = result.collider();
+    assert_eq!(retrieved, Some(object.clone().upcast()));
+
+    object.free();
+    assert_eq!(result.collider(), None);
+}