Merge pull request #1436 from godot-rust/qol/uniform-ffi-rawptr

Unify raw pointer FFI passing with `RawPtr<P>` type

Author: Bromeon

godot-codegen/src/context.rs

@@ -254,6 +254,7 @@ impl<'a> Context<'a> {
 
     /// Yields cached sys pointer types – various pointer types declared in `gdextension_interface`
     /// and used as parameters in exposed Godot APIs.
+    #[allow(dead_code)] // Currently unused, as RawPtr<P> covers all raw pointers.
     pub fn cached_sys_pointer_types(&self) -> impl Iterator<Item = &RustTy> {
         self.cached_rust_types
             .values()

godot-codegen/src/generator/central_files.rs

@@ -10,7 +10,6 @@ use quote::{format_ident, quote, ToTokens};
 
 use crate::context::Context;
 use crate::conv;
-use crate::generator::sys_pointer_types::make_godotconvert_for_systypes;
 use crate::generator::{enums, gdext_build_struct};
 use crate::models::domain::{ExtensionApi, FlowDirection};
 use crate::util::ident;
@@ -61,7 +60,6 @@ pub fn make_core_central_code(api: &ExtensionApi, ctx: &mut Context) -> TokenStr
 
     let (global_enum_defs, global_reexported_enum_defs) = make_global_enums(api);
     let variant_type_traits = make_variant_type_enum(api, false);
-    let sys_types_godotconvert_impl = make_godotconvert_for_systypes(ctx);
 
     // TODO impl Clone, Debug, PartialEq, PartialOrd, Hash for VariantDispatch
     // TODO could use try_to().unwrap_unchecked(), since type is already verified. Also directly overload from_variant().
@@ -119,15 +117,10 @@ pub fn make_core_central_code(api: &ExtensionApi, ctx: &mut Context) -> TokenStr
             #( #global_enum_defs )*
         }
 
-       pub mod global_reexported_enums {
+        pub mod global_reexported_enums {
             use crate::sys;
             #( #global_reexported_enum_defs )*
         }
-
-        pub mod sys_pointer_types {
-            use crate::sys;
-            #( #sys_types_godotconvert_impl )*
-        }
     }
 }
 

godot-codegen/src/generator/mod.rs

@@ -28,7 +28,6 @@ pub mod method_tables;
 pub mod native_structures;
 pub mod notifications;
 pub mod signals;
-pub mod sys_pointer_types;
 pub mod utility_functions;
 pub mod virtual_definitions;
 pub mod virtual_traits;

godot-codegen/src/generator/native_structures.rs

@@ -93,7 +93,6 @@ fn make_native_structure(
     let tokens = quote! {
         #imports
         use std::ffi::c_void; // for opaque object pointer fields
-        use crate::meta::{GodotConvert, EngineFromGodot, EngineToGodot};
 
         /// Native structure; can be passed via pointer in APIs that are not exposed to GDScript.
         ///
@@ -107,58 +106,7 @@ fn make_native_structure(
             #methods
         }
 
-        impl GodotConvert for *mut #class_name {
-            type Via = i64;
-        }
-
-        // Native structure pointers implement internal-only conversion traits for use in engine APIs.
-        impl EngineToGodot for *mut #class_name {
-            type Pass = crate::meta::ByValue;
-
-            fn engine_to_godot(&self) -> crate::meta::ToArg<'_, Self::Via, Self::Pass> {
-                *self as i64
-            }
-
-            fn engine_to_variant(&self) -> crate::builtin::Variant {
-                crate::builtin::Variant::from(*self as i64)
-            }
-        }
-
-        impl EngineFromGodot for *mut #class_name {
-            fn engine_try_from_godot(via: Self::Via) -> Result<Self, crate::meta::error::ConvertError> {
-                Ok(via as Self)
-            }
-
-            fn engine_try_from_variant(variant: &crate::builtin::Variant) -> Result<Self, crate::meta::error::ConvertError> {
-                variant.try_to::<i64>().map(|i| i as Self)
-            }
-        }
-
-        impl GodotConvert for *const #class_name {
-            type Via = i64;
-        }
-
-        impl EngineToGodot for *const #class_name {
-            type Pass = crate::meta::ByValue;
-
-            fn engine_to_godot(&self) -> crate::meta::ToArg<'_, Self::Via, Self::Pass> {
-                *self as i64
-            }
-
-            fn engine_to_variant(&self) -> crate::builtin::Variant {
-                crate::builtin::Variant::from(*self as i64)
-            }
-        }
-
-        impl EngineFromGodot for *const #class_name {
-            fn engine_try_from_godot(via: Self::Via) -> Result<Self, crate::meta::error::ConvertError> {
-                Ok(via as Self)
-            }
-
-            fn engine_try_from_variant(variant: &crate::builtin::Variant) -> Result<Self, crate::meta::error::ConvertError> {
-                variant.try_to::<i64>().map(|i| i as Self)
-            }
-        }
+        // Pointer conversions are now handled by RawPtr<P>, no direct ToGodot/FromGodot impls.
     };
     // note: TypePtr -> ObjectPtr conversion OK?
 
@@ -233,7 +181,11 @@ fn make_native_structure_field_and_accessor(
             }
 
             /// Sets the object from a `Gd` pointer holding `Node` or a derived class.
-            pub fn #setter_name<T>(&mut self, #snake_field_name: Gd<T>)
+            ///
+            /// # Safety
+            /// You must ensure that the provided object remains alive while Godot accesses it.
+            /// See also [`RawPtr::new()`][crate::meta::RawPtr::new].
+            pub unsafe fn #setter_name<T>(&mut self, #snake_field_name: Gd<T>)
             where T: crate::obj::Inherits<Object> {
                 use crate::meta::GodotType as _;
 
@@ -245,7 +197,10 @@ fn make_native_structure_field_and_accessor(
                 let id = obj.instance_id().to_u64();
 
                 self.#id_field_name = ObjectId { id };
-                self.#field_name = obj.obj_sys() as *mut std::ffi::c_void;
+
+                // SAFETY: provided by method safety contract.
+                // Godot declares void* but expects GDExtensionObjectPtr.
+                self.#field_name = unsafe { RawPtr::new(obj.obj_sys().cast::<std::ffi::c_void>()) };
             }
         });
     } else {

godot-codegen/src/generator/sys_pointer_types.rs

@@ -902,11 +902,11 @@ impl ToTokens for RustTy {
             RustTy::RawPointer {
                 inner,
                 is_const: true,
-            } => quote! { *const #inner }.to_tokens(tokens),
+            } => quote! { crate::meta::RawPtr<*const #inner> }.to_tokens(tokens),
             RustTy::RawPointer {
                 inner,
                 is_const: false,
-            } => quote! { *mut #inner }.to_tokens(tokens),
+            } => quote! { crate::meta::RawPtr<*mut #inner> }.to_tokens(tokens),
             RustTy::EngineArray { tokens: path, .. } => path.to_tokens(tokens),
             RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens),
             RustTy::EngineClass {

godot-codegen/src/models/domain.rs

@@ -25,7 +25,7 @@ pub fn make_imports() -> TokenStream {
     quote! {
         use godot_ffi as sys;
         use crate::builtin::*;
-        use crate::meta::{AsArg, ClassId, CowArg, InParamTuple, OutParamTuple, ParamTuple, RefArg, Signature};
+        use crate::meta::{AsArg, ClassId, CowArg, InParamTuple, OutParamTuple, ParamTuple, RawPtr, RefArg, Signature};
         use crate::classes::native::*;
         use crate::classes::Object;
         use crate::obj::Gd;

godot-codegen/src/util.rs

@@ -67,7 +67,7 @@ impl AnyArray {
         }
     }
 
-    /// Returns the value at the specified index.
+    /// ⚠️ Returns the value at the specified index.
     ///
     /// This replaces the `Index` trait, which cannot be implemented for `Array`, as it stores variants and not references.
     ///

godot-core/src/builtin/collections/any_array.rs

@@ -255,7 +255,7 @@ impl<T: ArrayElement> Array<T> {
         Self::default()
     }
 
-    /// Returns the value at the specified index.
+    /// ⚠️ Returns the value at the specified index.
     ///
     /// This replaces the `Index` trait, which cannot be implemented for `Array`, as it stores variants and not references.
     ///
@@ -319,7 +319,7 @@ impl<T: ArrayElement> Array<T> {
         })
     }
 
-    /// Sets the value at the specified index.
+    ///  ⚠️ Sets the value at the specified index.
     ///
     /// # Panics
     ///

godot-core/src/builtin/collections/array.rs

@@ -165,7 +165,7 @@ impl<T: PackedArrayElement> PackedArray<T> {
     //     T::op_push_back(self.as_inner(), CowArg::Owned(value));
     // }
 
-    /// Inserts a new element at a given index in the array.
+    /// ⚠️ Inserts a new element at a given index in the array.
     ///
     /// The index must be valid, or at the end of the array (`index == len()`).
     ///
@@ -180,7 +180,7 @@ impl<T: PackedArrayElement> PackedArray<T> {
         T::op_insert(self.as_inner(), to_i64(index), value.into_arg());
     }
 
-    /// Removes and returns the element at the specified index. Similar to `remove_at` in
+    /// ⚠️ Removes and returns the element at the specified index. Similar to `remove_at` in
     /// GDScript, but also returns the removed value.
     ///
     /// On large arrays, this method is much slower than `pop_back` as it will move all the array's