From 820669791415d3fbd09e3a3a234c5785a073d6b2 Mon Sep 17 00:00:00 2001 From: Lili Zoey Date: Mon, 10 Apr 2023 22:28:22 +0200 Subject: [PATCH] Fix return virtual method type Add another virtual method return test Make test that causes memory leak use `#[itest(skip)]` Move the logic for determining whether to use `Ref` or not entirely into `Mem` Remove some unnecessary manual ffi tests Rename CallType --- godot-codegen/src/central_generator.rs | 4 +- godot-codegen/src/lib.rs | 4 + godot-codegen/src/util.rs | 2 + godot-core/src/builtin/aabb.rs | 2 +- godot-core/src/builtin/array.rs | 17 +-- godot-core/src/builtin/basis.rs | 2 +- godot-core/src/builtin/color.rs | 2 +- godot-core/src/builtin/dictionary.rs | 18 +-- godot-core/src/builtin/meta/signature.rs | 4 +- godot-core/src/builtin/node_path.rs | 18 +-- godot-core/src/builtin/packed_array.rs | 2 +- godot-core/src/builtin/plane.rs | 2 +- godot-core/src/builtin/projection.rs | 2 +- godot-core/src/builtin/quaternion.rs | 2 +- godot-core/src/builtin/rect2.rs | 2 +- godot-core/src/builtin/rect2i.rs | 2 +- godot-core/src/builtin/rid.rs | 2 +- godot-core/src/builtin/string.rs | 22 +-- godot-core/src/builtin/string_name.rs | 18 +-- godot-core/src/builtin/transform2d.rs | 2 +- godot-core/src/builtin/transform3d.rs | 2 +- godot-core/src/builtin/vector2.rs | 4 +- godot-core/src/builtin/vector2i.rs | 4 +- godot-core/src/builtin/vector3.rs | 4 +- godot-core/src/builtin/vector3i.rs | 4 +- godot-core/src/builtin/vector4.rs | 4 +- godot-core/src/builtin/vector4i.rs | 4 +- godot-core/src/macros.rs | 4 +- godot-core/src/obj/gd.rs | 45 +++--- godot-core/src/obj/instance_id.rs | 2 +- godot-core/src/obj/traits.rs | 12 +- godot-ffi/src/godot_ffi.rs | 105 +++++++++----- godot-ffi/src/lib.rs | 2 +- itest/godot/ManualFfiTests.gd | 40 ------ itest/godot/TestRunner.gd | 2 - itest/rust/src/array_test.rs | 15 -- itest/rust/src/lib.rs | 7 +- itest/rust/src/node_test.rs | 4 +- itest/rust/src/object_test.rs | 4 +- itest/rust/src/runner.rs | 23 +--- itest/rust/src/variant_test.rs | 2 +- itest/rust/src/virtual_methods_test.rs | 168 +++++++++++++++++++---- 42 files changed, 339 insertions(+), 251 deletions(-) diff --git a/godot-codegen/src/central_generator.rs b/godot-codegen/src/central_generator.rs index b98383a03..2b145f68b 100644 --- a/godot-codegen/src/central_generator.rs +++ b/godot-codegen/src/central_generator.rs @@ -177,7 +177,7 @@ fn make_sys_code(central_items: &CentralItems) -> String { } // SAFETY: - // This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. + // This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for VariantType { ffi_methods! { type GDExtensionTypePtr = *mut Self; .. } } @@ -208,7 +208,7 @@ fn make_sys_code(central_items: &CentralItems) -> String { } // SAFETY: - // This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. + // This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for VariantOperator { ffi_methods! { type GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-codegen/src/lib.rs b/godot-codegen/src/lib.rs index 470af9672..e9ec07c30 100644 --- a/godot-codegen/src/lib.rs +++ b/godot-codegen/src/lib.rs @@ -258,6 +258,7 @@ const SELECTED_CLASSES: &[&str] = &[ "AudioStreamPlayer", "BaseButton", "Button", + "BoxMesh", "Camera2D", "Camera3D", "CanvasItem", @@ -276,6 +277,7 @@ const SELECTED_CLASSES: &[&str] = &[ "Label", "MainLoop", "Marker2D", + "Mesh", "Node", "Node2D", "Node3D", @@ -285,9 +287,11 @@ const SELECTED_CLASSES: &[&str] = &[ "PackedScene", "PathFollow2D", "PhysicsBody2D", + "PrimitiveMesh", "RefCounted", "RenderingServer", "Resource", + "ResourceFormatLoader", "ResourceLoader", "RigidBody2D", "SceneTree", diff --git a/godot-codegen/src/util.rs b/godot-codegen/src/util.rs index 08c8bd18f..ace6a7ea1 100644 --- a/godot-codegen/src/util.rs +++ b/godot-codegen/src/util.rs @@ -99,6 +99,8 @@ pub fn make_enum_definition(enum_: &Enum) -> TokenStream { self.ord } } + // SAFETY: + // The enums are transparently represented as an `i32`, so `*mut Self` is sound. unsafe impl sys::GodotFfi for #enum_name { sys::ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/aabb.rs b/godot-core/src/builtin/aabb.rs index e1d62767f..e6d7505bc 100644 --- a/godot-core/src/builtin/aabb.rs +++ b/godot-core/src/builtin/aabb.rs @@ -83,7 +83,7 @@ impl Aabb { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Aabb { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index e1bfee88e..ec0ec3ecd 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -566,22 +566,23 @@ impl Array { // ... // } +// SAFETY: +// - `move_return_ptr` +// Nothing special needs to be done beyond a `std::mem::swap` when returning an Array. +// So we can just use `ffi_methods`. +// +// - `from_arg_ptr` +// Arrays are properly initialized through a `from_sys` call, but the ref-count should be incremented +// as that is the callee's responsibility. Which we do by calling `std::mem::forget(array.share())`. unsafe impl GodotFfi for Array { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; fn from_sys; fn sys; fn from_sys_init; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning an Array. fn move_return_ptr; } - // SAFETY: - // Arrays are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // Using `std::mem::forget(array.share())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let array = Self::from_sys(ptr); std::mem::forget(array.share()); array diff --git a/godot-core/src/builtin/basis.rs b/godot-core/src/builtin/basis.rs index bcf204c9a..5c2d15353 100644 --- a/godot-core/src/builtin/basis.rs +++ b/godot-core/src/builtin/basis.rs @@ -571,7 +571,7 @@ impl Mul for Basis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Basis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/color.rs b/godot-core/src/builtin/color.rs index 5714378df..c93d97489 100644 --- a/godot-core/src/builtin/color.rs +++ b/godot-core/src/builtin/color.rs @@ -312,7 +312,7 @@ impl Color { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Color { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index bf1845cee..57a6cec56 100644 --- a/godot-core/src/builtin/dictionary.rs +++ b/godot-core/src/builtin/dictionary.rs @@ -239,22 +239,24 @@ impl Dictionary { // ---------------------------------------------------------------------------------------------------------------------------------------------- // Traits +// SAFETY: +// - `move_return_ptr` +// Nothing special needs to be done beyond a `std::mem::swap` when returning an Dictionary. +// So we can just use `ffi_methods`. +// +// - `from_arg_ptr` +// Dictionaries are properly initialized through a `from_sys` call, but the ref-count should be +// incremented as that is the callee's responsibility. Which we do by calling +// `std::mem::forget(dictionary.share())`. unsafe impl GodotFfi for Dictionary { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; fn from_sys; fn from_sys_init; fn sys; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning a dictionary. fn move_return_ptr; } - // SAFETY: - // Dictionaries are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // Using `std::mem::forget(dictionary.share())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let dictionary = Self::from_sys(ptr); std::mem::forget(dictionary.share()); dictionary diff --git a/godot-core/src/builtin/meta/signature.rs b/godot-core/src/builtin/meta/signature.rs index f7c25d7a6..bb219438b 100644 --- a/godot-core/src/builtin/meta/signature.rs +++ b/godot-core/src/builtin/meta/signature.rs @@ -34,7 +34,7 @@ pub trait SignatureTuple { ret: sys::GDExtensionTypePtr, func: fn(&mut C, Self::Params) -> Self::Ret, method_name: &str, - call_type: sys::CallType, + call_type: sys::PtrcallType, ); } @@ -144,7 +144,7 @@ macro_rules! impl_signature_for_tuple { ret: sys::GDExtensionTypePtr, func: fn(&mut C, Self::Params) -> Self::Ret, method_name: &str, - call_type: sys::CallType, + call_type: sys::PtrcallType, ) { $crate::out!("ptrcall: {}", method_name); diff --git a/godot-core/src/builtin/node_path.rs b/godot-core/src/builtin/node_path.rs index c6c032094..fb0b7a759 100644 --- a/godot-core/src/builtin/node_path.rs +++ b/godot-core/src/builtin/node_path.rs @@ -22,22 +22,24 @@ impl NodePath { } } +// SAFETY: +// - `move_return_ptr` +// Nothing special needs to be done beyond a `std::mem::swap` when returning a NodePath. +// So we can just use `ffi_methods`. +// +// - `from_arg_ptr` +// NodePaths are properly initialized through a `from_sys` call, but the ref-count should be +// incremented as that is the callee's responsibility. Which we do by calling +// `std::mem::forget(node_path.share())`. unsafe impl GodotFfi for NodePath { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; fn from_sys; fn sys; fn from_sys_init; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning a NodePath. fn move_return_ptr; } - // SAFETY: - // NodePaths are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // Using `std::mem::forget(node_path.share())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let node_path = Self::from_sys(ptr); std::mem::forget(node_path.clone()); node_path diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index 54eb4850b..0ba83a5b1 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -405,7 +405,7 @@ macro_rules! impl_packed_array { // incremented as that is the callee's responsibility. // // Using `std::mem::forget(array.clone())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let array = Self::from_sys(ptr); std::mem::forget(array.clone()); array diff --git a/godot-core/src/builtin/plane.rs b/godot-core/src/builtin/plane.rs index c54e16b35..83209fa4d 100644 --- a/godot-core/src/builtin/plane.rs +++ b/godot-core/src/builtin/plane.rs @@ -135,7 +135,7 @@ impl Neg for Plane { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Plane { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/projection.rs b/godot-core/src/builtin/projection.rs index 81260f779..8625152e0 100644 --- a/godot-core/src/builtin/projection.rs +++ b/godot-core/src/builtin/projection.rs @@ -487,7 +487,7 @@ impl GlamConv for Projection { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Projection { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/quaternion.rs b/godot-core/src/builtin/quaternion.rs index 98b76b548..1a1f34884 100644 --- a/godot-core/src/builtin/quaternion.rs +++ b/godot-core/src/builtin/quaternion.rs @@ -255,7 +255,7 @@ impl Mul for Quaternion { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Quaternion { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/rect2.rs b/godot-core/src/builtin/rect2.rs index 82828c613..492a86e1a 100644 --- a/godot-core/src/builtin/rect2.rs +++ b/godot-core/src/builtin/rect2.rs @@ -105,7 +105,7 @@ impl Rect2 { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Rect2 { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/rect2i.rs b/godot-core/src/builtin/rect2i.rs index 4945c49a8..ea560e796 100644 --- a/godot-core/src/builtin/rect2i.rs +++ b/godot-core/src/builtin/rect2i.rs @@ -94,7 +94,7 @@ impl Rect2i { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Rect2i { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/rid.rs b/godot-core/src/builtin/rid.rs index 7dbd99fee..57eeaf394 100644 --- a/godot-core/src/builtin/rid.rs +++ b/godot-core/src/builtin/rid.rs @@ -82,7 +82,7 @@ impl Rid { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Rid { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/string.rs b/godot-core/src/builtin/string.rs index 498f48f71..99050a4d5 100644 --- a/godot-core/src/builtin/string.rs +++ b/godot-core/src/builtin/string.rs @@ -37,12 +37,12 @@ impl GodotString { fn string_sys = sys; } - /// Move `self` into a system pointer. + /// Move `self` into a system pointer. This transfers ownership and thus does not call the destructor. /// /// # Safety /// `dst` must be a pointer to a `GodotString` which is suitable for ffi with Godot. pub unsafe fn move_string_ptr(self, dst: sys::GDExtensionStringPtr) { - self.move_return_ptr(dst as *mut _, sys::CallType::Standard); + self.move_return_ptr(dst as *mut _, sys::PtrcallType::Standard); } /// Gets the internal chars slice from a [`GodotString`]. @@ -75,22 +75,24 @@ impl GodotString { } } +// SAFETY: +// - `move_return_ptr` +// Nothing special needs to be done beyond a `std::mem::swap` when returning a String. +// So we can just use `ffi_methods`. +// +// - `from_arg_ptr` +// Strings are properly initialized through a `from_sys` call, but the ref-count should be +// incremented as that is the callee's responsibility. Which we do by calling +// `std::mem::forget(string.share())`. unsafe impl GodotFfi for GodotString { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; fn from_sys; fn sys; fn from_sys_init; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning a GodotString. fn move_return_ptr; } - // SAFETY: - // GodotStrings are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // Using `std::mem::forget(string.share())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let string = Self::from_sys(ptr); std::mem::forget(string.clone()); string diff --git a/godot-core/src/builtin/string_name.rs b/godot-core/src/builtin/string_name.rs index 43b75fc3f..bb78daa8b 100644 --- a/godot-core/src/builtin/string_name.rs +++ b/godot-core/src/builtin/string_name.rs @@ -31,22 +31,24 @@ impl StringName { } } +// SAFETY: +// - `move_return_ptr` +// Nothing special needs to be done beyond a `std::mem::swap` when returning a StringName. +// So we can just use `ffi_methods`. +// +// - `from_arg_ptr` +// StringNames are properly initialized through a `from_sys` call, but the ref-count should be +// incremented as that is the callee's responsibility. Which we do by calling +// `std::mem::forget(string_name.share())`. unsafe impl GodotFfi for StringName { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; fn from_sys; fn sys; fn from_sys_init; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning a StringName. fn move_return_ptr; } - // SAFETY: - // StringNames are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // Using `std::mem::forget(string_name.share())` increments the ref count. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self { + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { let string_name = Self::from_sys(ptr); std::mem::forget(string_name.clone()); string_name diff --git a/godot-core/src/builtin/transform2d.rs b/godot-core/src/builtin/transform2d.rs index 1d7ffe36d..b6b04d7d5 100644 --- a/godot-core/src/builtin/transform2d.rs +++ b/godot-core/src/builtin/transform2d.rs @@ -359,7 +359,7 @@ impl GlamConv for Transform2D { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Transform2D { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/transform3d.rs b/godot-core/src/builtin/transform3d.rs index f2459e4b6..4c7c61985 100644 --- a/godot-core/src/builtin/transform3d.rs +++ b/godot-core/src/builtin/transform3d.rs @@ -353,7 +353,7 @@ impl GlamConv for Transform3D { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Transform3D { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector2.rs b/godot-core/src/builtin/vector2.rs index 310f3d15b..850a9679b 100644 --- a/godot-core/src/builtin/vector2.rs +++ b/godot-core/src/builtin/vector2.rs @@ -311,7 +311,7 @@ impl_vector_operators!(Vector2, real, (x, y)); impl_vector_index!(Vector2, real, (x, y), Vector2Axis, (X, Y)); // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector2 { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -327,7 +327,7 @@ pub enum Vector2Axis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector2Axis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector2i.rs b/godot-core/src/builtin/vector2i.rs index b1109e7c5..83ec4563f 100644 --- a/godot-core/src/builtin/vector2i.rs +++ b/godot-core/src/builtin/vector2i.rs @@ -89,7 +89,7 @@ impl_vector_operators!(Vector2i, i32, (x, y)); impl_vector_index!(Vector2i, i32, (x, y), Vector2iAxis, (X, Y)); // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector2i { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -105,7 +105,7 @@ pub enum Vector2iAxis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector2iAxis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector3.rs b/godot-core/src/builtin/vector3.rs index cf83337c5..a7569bc2e 100644 --- a/godot-core/src/builtin/vector3.rs +++ b/godot-core/src/builtin/vector3.rs @@ -330,7 +330,7 @@ impl_vector_operators!(Vector3, real, (x, y, z)); impl_vector_index!(Vector3, real, (x, y, z), Vector3Axis, (X, Y, Z)); // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector3 { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -349,7 +349,7 @@ pub enum Vector3Axis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector3Axis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector3i.rs b/godot-core/src/builtin/vector3i.rs index 707f20bb4..6ae8048e4 100644 --- a/godot-core/src/builtin/vector3i.rs +++ b/godot-core/src/builtin/vector3i.rs @@ -98,7 +98,7 @@ impl_vector_operators!(Vector3i, i32, (x, y, z)); impl_vector_index!(Vector3i, i32, (x, y, z), Vector3iAxis, (X, Y, Z)); // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector3i { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -116,7 +116,7 @@ pub enum Vector3iAxis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector3iAxis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector4.rs b/godot-core/src/builtin/vector4.rs index de9f195a7..65758808d 100644 --- a/godot-core/src/builtin/vector4.rs +++ b/godot-core/src/builtin/vector4.rs @@ -90,7 +90,7 @@ impl fmt::Display for Vector4 { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector4 { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -110,7 +110,7 @@ pub enum Vector4Axis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector4Axis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/builtin/vector4i.rs b/godot-core/src/builtin/vector4i.rs index 7e3bfb5b5..494aca9b8 100644 --- a/godot-core/src/builtin/vector4i.rs +++ b/godot-core/src/builtin/vector4i.rs @@ -83,7 +83,7 @@ impl fmt::Display for Vector4i { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector4i { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -103,7 +103,7 @@ pub enum Vector4iAxis { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for Vector4iAxis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/macros.rs b/godot-core/src/macros.rs index 1c7464f0a..5daf041a1 100644 --- a/godot-core/src/macros.rs +++ b/godot-core/src/macros.rs @@ -112,7 +112,7 @@ macro_rules! gdext_register_method_inner { inst.$method_name( $( $param, )* ) }, stringify!($method_name), - sys::CallType::Standard + sys::PtrcallType::Standard ); } ); @@ -401,7 +401,7 @@ macro_rules! gdext_ptrcall { $ret_ptr, |__instance, ($($arg,)*)| __instance.$method_name($($arg,)*), stringify!($method_name), - sys::CallType::Virtual, + sys::PtrcallType::Virtual, ) }; } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 3b7ec0882..e001b1b08 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -12,7 +12,7 @@ use std::ptr; use godot_ffi as sys; use godot_ffi::VariantType; use sys::types::OpaqueObject; -use sys::{ffi_methods, interface_fn, static_assert_eq_size, CallType, GodotFfi}; +use sys::{ffi_methods, interface_fn, static_assert_eq_size, GodotFfi, PtrcallType}; use crate::builtin::meta::{ClassName, VariantMetadata}; use crate::builtin::{FromVariant, ToVariant, Variant, VariantConversionError}; @@ -499,7 +499,14 @@ where unsafe { std::mem::transmute::<&mut OpaqueObject, &mut T>(&mut self.opaque) } } } - +// SAFETY: +// - `move_return_ptr` +// When the `call_type` is `PtrcallType::Virtual`, and the current type is known to inherit from `RefCounted` +// then we use `ref_get_object`. Otherwise we use `Gd::from_obj_sys`. +// - `from_arg_ptr` +// When the `call_type` is `PtrcallType::Virtual`, and the current type is known to inherit from `RefCounted` +// then we use `ref_set_object`. Otherwise we use `std::ptr::write`. Finally we forget `self` as we pass +// ownership to the caller. unsafe impl GodotFfi for Gd where T: GodotClass, @@ -513,31 +520,21 @@ where // For more context around `ref_get_object` and `ref_set_object`, see: // https://github.com/godotengine/godot-cpp/issues/954 - // SAFETY: - // When the `call_type` is `CallType::Virtual`, and the current type is known to inherit from `RefCounted` - // then we use `ref_get_object`. Otherwise we use `Gd::from_obj_sys`. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: CallType) -> Self { - match (T::Mem::is_static_ref_counted(), call_type) { - (true, CallType::Virtual) => { - let obj_ptr = interface_fn!(ref_get_object)(ptr as sys::GDExtensionRefPtr); - // ref_get_object increments the ref_count for us - Self::from_obj_sys_weak(obj_ptr) - } - _ => Self::from_obj_sys(ptr as sys::GDExtensionObjectPtr), + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self { + if T::Mem::pass_as_ref(call_type) { + let obj_ptr = interface_fn!(ref_get_object)(ptr as sys::GDExtensionRefPtr); + // ref_get_object increments the ref_count for us + Self::from_obj_sys_weak(obj_ptr) + } else { + Self::from_obj_sys(ptr as sys::GDExtensionObjectPtr) } } - // SAFETY: - // When the `call_type` is `CallType::Virtual`, and the current type is known to inherit from `RefCounted` - // then we use `ref_set_object`. Otherwise we use `std::ptr::write`. Finally we forget `self` as we pass - // ownership to the caller. - unsafe fn move_return_ptr(self, ptr: sys::GDExtensionTypePtr, call_type: CallType) { - match (T::Mem::is_static_ref_counted(), call_type) { - (true, CallType::Virtual) => { - interface_fn!(ref_set_object)(ptr as sys::GDExtensionRefPtr, self.obj_sys()) - } - - _ => std::ptr::write(ptr as *mut _, self.opaque), + unsafe fn move_return_ptr(self, ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) { + if T::Mem::pass_as_ref(call_type) { + interface_fn!(ref_set_object)(ptr as sys::GDExtensionRefPtr, self.obj_sys()) + } else { + std::ptr::write(ptr as *mut _, self.opaque) } // We've passed ownership to caller. std::mem::forget(self); diff --git a/godot-core/src/obj/instance_id.rs b/godot-core/src/obj/instance_id.rs index 4144bdbb8..b169c7070 100644 --- a/godot-core/src/obj/instance_id.rs +++ b/godot-core/src/obj/instance_id.rs @@ -78,7 +78,7 @@ impl Debug for InstanceId { } // SAFETY: -// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. +// This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for InstanceId { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index abb1c0cba..b0f36508a 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -229,6 +229,8 @@ pub mod dom { // ---------------------------------------------------------------------------------------------------------------------------------------------- pub mod mem { + use godot_ffi::PtrcallType; + use super::private::Sealed; use crate::obj::{Gd, GodotClass}; use crate::out; @@ -246,7 +248,11 @@ pub mod mem { /// Check if ref-counted, return `None` if information is not available (dynamic and obj dead) fn is_ref_counted(obj: &Gd) -> Option; - fn is_static_ref_counted() -> bool { + /// Returns `true` if argument and return pointers are passed as `Ref` pointers given this + /// [`PtrcallType`]. + /// + /// See [`PtrcallType::Virtual`] for information about `Ref` objects. + fn pass_as_ref(_call_type: PtrcallType) -> bool { false } } @@ -286,8 +292,8 @@ pub mod mem { Some(true) } - fn is_static_ref_counted() -> bool { - true + fn pass_as_ref(call_type: PtrcallType) -> bool { + matches!(call_type, PtrcallType::Virtual) } } diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index dbb8778d6..127bc2af6 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/godot-ffi/src/godot_ffi.rs @@ -13,7 +13,7 @@ use std::fmt::Debug; /// # Safety /// /// [`from_arg_ptr`](GodotFfi::from_arg_ptr) and [`move_return_ptr`](GodotFfi::move_return_ptr) -/// must properly initialize and clean up values given the [`CallType`] provided by the caller. +/// must properly initialize and clean up values given the [`PtrcallType`] provided by the caller. pub unsafe trait GodotFfi { /// Construct from Godot opaque pointer. /// @@ -77,24 +77,26 @@ pub unsafe trait GodotFfi { /// Construct from a pointer to an argument in a call. /// /// # Safety - /// `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, - /// which is different depending on the type. - /// `ptr` must encode `Self` according to the given `call_type`'s encoding of argument values. - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: CallType) -> Self; + /// * `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, + /// which is different depending on the type. + /// + /// * `ptr` must encode `Self` according to the given `call_type`'s encoding of argument values. + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self; /// Move self into the pointer in pointer `dst`, dropping what is already in `dst. /// /// # Safety - /// `dst` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, - /// hich is different depending on the type. - /// `dst` must be able to accept a value of type `Self` encoded according to the given - /// `call_type`'s encoding of return values. - unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: CallType); + /// * `dst` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, + /// which is different depending on the type. + /// + /// * `dst` must be able to accept a value of type `Self` encoded according to the given + /// `call_type`'s encoding of return values. + unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: PtrcallType); } /// An indication of what type of pointer call is being made. #[derive(Default, Copy, Clone, Eq, PartialEq, Debug)] -pub enum CallType { +pub enum PtrcallType { /// Standard pointer call. /// /// In a standard ptrcall, every argument is passed in as a pointer to a value of that type, and the @@ -103,8 +105,8 @@ pub enum CallType { Standard, /// Virtual pointer call. /// - /// A virtual call behaves like [`CallType::Standard`], except for `RefCounted` objects. - /// `RefCounted` objects are instead passed in and returned as `Ref` objects. + /// A virtual call behaves like [`PtrcallType::Standard`], except for `RefCounted` objects. + /// `RefCounted` objects are instead passed in and returned as `Ref` objects in Godot. /// /// To properly get a value from an argument in a pointer call, you must use `ref_get_object`. And to /// return a value you must use `ref_set_object`. @@ -126,7 +128,7 @@ pub trait GodotFuncMarshal: Sized { /// See also [`GodotFfi::from_arg_ptr`]. unsafe fn try_from_arg( ptr: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result; /// Used for function return values. On failure, `self` which can't be converted to Via is returned. @@ -138,7 +140,7 @@ pub trait GodotFuncMarshal: Sized { unsafe fn try_return( self, dst: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result<(), Self>; } @@ -175,13 +177,13 @@ macro_rules! ffi_methods_one { }; (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { $( #[$attr] )? $vis - unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::CallType) -> Self { + unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::PtrcallType) -> Self { Self::from_sys(ptr as *mut _) } }; (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { $( #[$attr] )? $vis - unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::CallType) { + unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::PtrcallType) { std::ptr::swap(dst as *mut _, std::ptr::addr_of_mut!(self.opaque)) } }; @@ -210,13 +212,13 @@ macro_rules! ffi_methods_one { }; (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { $( #[$attr] )? $vis - unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::CallType) -> Self { + unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::PtrcallType) -> Self { Self::from_sys(ptr as *mut _) } }; (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { $( #[$attr] )? $vis - unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::CallType) { + unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::PtrcallType) { std::ptr::swap(dst, std::mem::transmute::<_, $Ptr>(self.opaque)) } }; @@ -245,13 +247,13 @@ macro_rules! ffi_methods_one { }; (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { $( #[$attr] )? $vis - unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::CallType) -> Self { + unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::PtrcallType) -> Self { *(ptr as *mut Self) } }; (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { $( #[$attr] )? $vis - unsafe fn $move_return_ptr(self, dst: $Ptr, _call_type: $crate::CallType) { + unsafe fn $move_return_ptr(self, dst: $Ptr, _call_type: $crate::PtrcallType) { *(dst as *mut Self) = self } }; @@ -313,7 +315,7 @@ macro_rules! ffi_methods_rest { /// /// ## Using `Opaque` /// -/// Turning pointer call arguments into a value is simply calling `from_opaue` on the argument pointer. +/// Turning pointer call arguments into a value is simply calling `from_opaque` on the argument pointer. /// Returning a value from a pointer call is simply calling [`std::ptr::swap`] on the return pointer /// and the `opaque` field transmuted into a pointer. /// @@ -348,14 +350,14 @@ macro_rules! ffi_methods { // ---------------------------------------------------------------------------------------------------------------------------------------------- // Implementation for common types (needs to be this crate due to orphan rule) mod scalars { - use super::{CallType, GodotFfi, GodotFuncMarshal}; + use super::{GodotFfi, GodotFuncMarshal, PtrcallType}; use crate as sys; use std::convert::Infallible; macro_rules! impl_godot_marshalling { ($T:ty) => { // SAFETY: - // This type is transparently represented as `Self` in Godot, so `*mut Self` is sound. + // This type is represented as `Self` in Godot, so `*mut Self` is sound. unsafe impl GodotFfi for $T { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } @@ -370,21 +372,44 @@ mod scalars { unsafe fn try_from_arg( ptr: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result { - let via = <$Via as GodotFfi>::from_arg_ptr(ptr, call_type); - // Godot does not always give us a valid `i64`/`f64`, nor `Self` when it does a pointer - // call, so we cannot use `try_from` here. + let via = <$Via>::from_arg_ptr(ptr, call_type); + Self::try_from(via).map_err(|_| via) + } + + unsafe fn try_return( + self, + dst: sys::GDExtensionTypePtr, + call_type: PtrcallType, + ) -> Result<(), Self> { + <$Via>::from(self).move_return_ptr(dst, call_type); + Ok(()) + } + } + }; + + ($T:ty as $Via:ty; lossy) => { + // implicit bounds: + // T: TryFrom, Copy + // Via: TryFrom, GodotFfi + impl GodotFuncMarshal for $T { + type Via = $Via; + + unsafe fn try_from_arg( + ptr: sys::GDExtensionTypePtr, + call_type: PtrcallType, + ) -> Result { + let via = <$Via>::from_arg_ptr(ptr, call_type); Ok(via as Self) } unsafe fn try_return( self, dst: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result<(), Self> { - let via = <$Via>::try_from(self).map_err(|_| self)?; - via.move_return_ptr(dst, call_type); + (self as $Via).move_return_ptr(dst, call_type); Ok(()) } } @@ -398,12 +423,13 @@ mod scalars { // Only implements GodotFuncMarshal impl_godot_marshalling!(i32 as i64); - impl_godot_marshalling!(i16 as i64); - impl_godot_marshalling!(i8 as i64); impl_godot_marshalling!(u32 as i64); + impl_godot_marshalling!(i16 as i64); impl_godot_marshalling!(u16 as i64); + impl_godot_marshalling!(i8 as i64); impl_godot_marshalling!(u8 as i64); - impl_godot_marshalling!(f32 as f64); + + impl_godot_marshalling!(f32 as f64; lossy); unsafe impl GodotFfi for () { unsafe fn from_sys(_ptr: sys::GDExtensionTypePtr) -> Self { @@ -421,7 +447,10 @@ mod scalars { // SAFETY: // We're not accessing the value in `_ptr`. - unsafe fn from_arg_ptr(_ptr: sys::GDExtensionTypePtr, _call_type: super::CallType) -> Self { + unsafe fn from_arg_ptr( + _ptr: sys::GDExtensionTypePtr, + _call_type: super::PtrcallType, + ) -> Self { } // SAFETY: @@ -429,7 +458,7 @@ mod scalars { unsafe fn move_return_ptr( self, _dst: sys::GDExtensionTypePtr, - _call_type: super::CallType, + _call_type: super::PtrcallType, ) { // Do nothing } @@ -443,7 +472,7 @@ mod scalars { unsafe fn try_from_arg( ptr: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result { Ok(Self::from_arg_ptr(ptr, call_type)) } @@ -451,7 +480,7 @@ mod scalars { unsafe fn try_return( self, dst: sys::GDExtensionTypePtr, - call_type: CallType, + call_type: PtrcallType, ) -> Result<(), Self> { self.move_return_ptr(dst, call_type); Ok(()) diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index a4a07478f..fa94018cb 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -28,7 +28,7 @@ mod plugins; #[doc(hidden)] pub use paste; -pub use crate::godot_ffi::{CallType, GodotFfi, GodotFuncMarshal}; +pub use crate::godot_ffi::{GodotFfi, GodotFuncMarshal, PtrcallType}; pub use gen::central::*; pub use gen::gdextension_interface::*; diff --git a/itest/godot/ManualFfiTests.gd b/itest/godot/ManualFfiTests.gd index 4b184c72c..73b56f8f9 100644 --- a/itest/godot/ManualFfiTests.gd +++ b/itest/godot/ManualFfiTests.gd @@ -72,46 +72,6 @@ func test_export(): obj.free() node.free() -func test_untyped_array_pass_to_user_func_varcall(): - var obj = ArrayTest.new() - var array: Array = [42, "answer"] - assert_eq(obj.pass_untyped_array(array), 2) - -func test_untyped_array_pass_to_user_func_ptrcall(): - var obj: ArrayTest = ArrayTest.new() - var array: Array = [42, "answer"] - assert_eq(obj.pass_untyped_array(array), 2) - -func test_untyped_array_return_from_user_func_varcall(): - var obj = ArrayTest.new() - var array: Array = obj.return_untyped_array() - assert_eq(array, [42, "answer"]) - -func test_untyped_array_return_from_user_func_ptrcall(): - var obj: ArrayTest = ArrayTest.new() - var array: Array = obj.return_untyped_array() - assert_eq(array, [42, "answer"]) - -func test_typed_array_pass_to_user_func_varcall(): - var obj = ArrayTest.new() - var array: Array[int] = [1, 2, 3] - assert_eq(obj.pass_typed_array(array), 6) - -func test_typed_array_pass_to_user_func_ptrcall(): - var obj: ArrayTest = ArrayTest.new() - var array: Array[int] = [1, 2, 3] - assert_eq(obj.pass_typed_array(array), 6) - -func test_typed_array_return_from_user_func_varcall(): - var obj = ArrayTest.new() - var array: Array[int] = obj.return_typed_array(3) - assert_eq(array, [1, 2, 3]) - -func test_typed_array_return_from_user_func_ptrcall(): - var obj: ArrayTest = ArrayTest.new() - var array: Array[int] = obj.return_typed_array(3) - assert_eq(array, [1, 2, 3]) - class MockObjGd extends Object: var i: int = 0 diff --git a/itest/godot/TestRunner.gd b/itest/godot/TestRunner.gd index 9466ecf99..3e28abe2f 100644 --- a/itest/godot/TestRunner.gd +++ b/itest/godot/TestRunner.gd @@ -38,9 +38,7 @@ func _ready(): gdscript_tests, gdscript_suites.size(), allow_focus, - get_tree().root, self, - get_tree() ) var exit_code: int = 0 if success else 1 diff --git a/itest/rust/src/array_test.rs b/itest/rust/src/array_test.rs index 9f830d327..e8ac4e4a9 100644 --- a/itest/rust/src/array_test.rs +++ b/itest/rust/src/array_test.rs @@ -427,21 +427,6 @@ struct ArrayTest; #[godot_api] impl ArrayTest { - #[func] - fn pass_untyped_array(&self, array: VariantArray) -> i64 { - array.len().try_into().unwrap() - } - - #[func] - fn return_untyped_array(&self) -> VariantArray { - varray![42, "answer"] - } - - #[func] - fn pass_typed_array(&self, array: Array) -> i64 { - array.iter_shared().sum() - } - #[func] fn return_typed_array(&self, n: i64) -> Array { (1..(n + 1)).collect() diff --git a/itest/rust/src/lib.rs b/itest/rust/src/lib.rs index 17670ef25..9596878f6 100644 --- a/itest/rust/src/lib.rs +++ b/itest/rust/src/lib.rs @@ -4,10 +4,9 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use godot::engine::{Engine, Node, Window}; +use godot::engine::{Engine, Node}; use godot::init::{gdextension, ExtensionLibrary}; use godot::obj::Gd; -use godot::prelude::SceneTree; use godot::sys; mod array_test; @@ -105,9 +104,7 @@ fn collect_rust_tests() -> (Vec, usize, bool) { } pub struct TestContext { - _scene_tree: Gd, - root_node: Gd, - test_node: Gd, + scene_tree: Gd, } #[derive(Copy, Clone)] diff --git a/itest/rust/src/node_test.rs b/itest/rust/src/node_test.rs index 2daf9189d..5099ed647 100644 --- a/itest/rust/src/node_test.rs +++ b/itest/rust/src/node_test.rs @@ -58,7 +58,7 @@ fn node_get_node_fail() { #[itest] fn node_path_from_str(ctx: &TestContext) { - let child = ctx.test_node.share(); + let child = ctx.scene_tree.share(); assert_eq!( child.get_path().to_string(), NodePath::from_str("/root/TestRunner").unwrap().to_string() @@ -96,7 +96,7 @@ fn node_scene_tree() { // FIXME: call_group() crashes #[itest(skip)] fn node_call_group(ctx: &TestContext) { - let mut node = ctx.test_node.share(); + let mut node = ctx.scene_tree.share(); let mut tree = node.get_tree().unwrap(); node.add_to_group("group".into(), true); diff --git a/itest/rust/src/object_test.rs b/itest/rust/src/object_test.rs index b1dd8de2a..4443490e3 100644 --- a/itest/rust/src/object_test.rs +++ b/itest/rust/src/object_test.rs @@ -95,7 +95,7 @@ fn object_user_roundtrip_write() { assert_eq!(obj.bind().value, value); let obj2 = unsafe { - Gd::::from_sys_init(|ptr| obj.move_return_ptr(ptr, sys::CallType::Standard)) + Gd::::from_sys_init(|ptr| obj.move_return_ptr(ptr, sys::PtrcallType::Standard)) }; assert_eq!(obj2.bind().value, value); } // drop @@ -615,7 +615,7 @@ fn object_call_with_args() { fn object_get_scene_tree(ctx: &TestContext) { let node = Node3D::new_alloc(); - let mut tree = ctx.test_node.share(); + let mut tree = ctx.scene_tree.share(); tree.add_child(node.upcast(), false, InternalMode::INTERNAL_MODE_DISABLED); let count = tree.get_child_count(false); diff --git a/itest/rust/src/runner.rs b/itest/rust/src/runner.rs index 30369b475..70c452f73 100644 --- a/itest/rust/src/runner.rs +++ b/itest/rust/src/runner.rs @@ -8,9 +8,8 @@ use std::time::{Duration, Instant}; use godot::bind::{godot_api, GodotClass}; use godot::builtin::{ToVariant, Variant, VariantArray}; -use godot::engine::{Node, Window}; +use godot::engine::Node; use godot::obj::Gd; -use godot::prelude::SceneTree; use crate::{RustTestCase, TestContext}; @@ -32,9 +31,7 @@ impl IntegrationTests { gdscript_tests: VariantArray, gdscript_file_count: i64, allow_focus: bool, - root_node: Gd, - test_node: Gd, - scene_tree: Gd, + scene_tree: Gd, ) -> bool { println!("{}Run{} Godot integration tests...", FMT_CYAN_BOLD, FMT_END); @@ -57,7 +54,7 @@ impl IntegrationTests { } let clock = Instant::now(); - self.run_rust_tests(rust_tests, root_node, test_node, scene_tree); + self.run_rust_tests(rust_tests, scene_tree); let rust_time = clock.elapsed(); let gdscript_time = if !focus_run { self.run_gdscript_tests(gdscript_tests); @@ -69,18 +66,8 @@ impl IntegrationTests { self.conclude(rust_time, gdscript_time, allow_focus) } - fn run_rust_tests( - &mut self, - tests: Vec, - root_node: Gd, - test_node: Gd, - scene_tree: Gd, - ) { - let ctx = TestContext { - _scene_tree: scene_tree, - root_node, - test_node, - }; + fn run_rust_tests(&mut self, tests: Vec, scene_tree: Gd) { + let ctx = TestContext { scene_tree }; let mut last_file = None; for test in tests { diff --git a/itest/rust/src/variant_test.rs b/itest/rust/src/variant_test.rs index 06d0e1197..9809e669a 100644 --- a/itest/rust/src/variant_test.rs +++ b/itest/rust/src/variant_test.rs @@ -257,7 +257,7 @@ fn variant_sys_conversion2() { unsafe { v.clone().move_return_ptr( buffer.as_mut_ptr() as sys::GDExtensionTypePtr, - sys::CallType::Standard, + sys::PtrcallType::Standard, ) }; diff --git a/itest/rust/src/virtual_methods_test.rs b/itest/rust/src/virtual_methods_test.rs index e17c5be60..03edf03ed 100644 --- a/itest/rust/src/virtual_methods_test.rs +++ b/itest/rust/src/virtual_methods_test.rs @@ -8,14 +8,20 @@ use crate::TestContext; use godot::bind::{godot_api, GodotClass}; -use godot::builtin::GodotString; +use godot::builtin::{ + is_equal_approx, real, varray, Color, GodotString, PackedByteArray, PackedColorArray, + PackedFloat32Array, PackedInt32Array, PackedStringArray, PackedVector2Array, + PackedVector3Array, RealConv, StringName, ToVariant, Variant, VariantArray, Vector2, Vector3, +}; use godot::engine::node::InternalMode; +use godot::engine::resource_loader::CacheMode; use godot::engine::{ - InputEvent, InputEventAction, Node, Node2D, Node2DVirtual, NodeVirtual, RefCounted, - RefCountedVirtual, + BoxMesh, InputEvent, InputEventAction, Node, Node2D, Node2DVirtual, PrimitiveMesh, + PrimitiveMeshVirtual, RefCounted, RefCountedVirtual, ResourceFormatLoader, + ResourceFormatLoaderVirtual, ResourceLoader, Window, }; use godot::obj::{Base, Gd, Share}; -use godot::prelude::PackedStringArray; +use godot::private::class_macros::assert_eq_approx; use godot::test::itest; /// Simple class, that deliberately has no constructor accessible from GDScript @@ -96,25 +102,34 @@ impl Node2DVirtual for TreeVirtualTest { } #[derive(GodotClass, Debug)] -#[class(base=Node)] +#[class(init, base=PrimitiveMesh)] struct ReturnVirtualTest { #[base] - base: Base, + base: Base, } #[godot_api] -impl NodeVirtual for ReturnVirtualTest { - fn init(base: Base) -> Self { - ReturnVirtualTest { base } - } - - fn get_configuration_warnings(&self) -> PackedStringArray { - let mut output = PackedStringArray::new(); - output.push("Hello".into()); - output +impl PrimitiveMeshVirtual for ReturnVirtualTest { + fn create_mesh_array(&self) -> VariantArray { + varray![ + PackedVector3Array::from_iter([Vector3::LEFT].into_iter()), + PackedVector3Array::from_iter([Vector3::LEFT].into_iter()), + PackedFloat32Array::from_iter([0.0, 0.0, 0.0, 1.0].into_iter()), + PackedColorArray::from_iter([Color::from_rgb(1.0, 1.0, 1.0)]), + PackedVector2Array::from_iter([Vector2::LEFT]), + PackedVector2Array::from_iter([Vector2::LEFT]), + PackedByteArray::from_iter([0, 1, 2, 3].into_iter()), + PackedByteArray::from_iter([0, 1, 2, 3].into_iter()), + PackedByteArray::from_iter([0, 1, 2, 3].into_iter()), + PackedByteArray::from_iter([0, 1, 2, 3].into_iter()), + PackedInt32Array::from_iter([0, 1, 2, 3].into_iter()), + PackedFloat32Array::from_iter([0.0, 1.0, 2.0, 3.0].into_iter()), + PackedInt32Array::from_iter([0].into_iter()), + ] } } +#[derive(GodotClass, Debug)] #[class(base=Node2D)] struct InputVirtualTest { #[base] @@ -129,11 +144,52 @@ impl Node2DVirtual for InputVirtualTest { } fn input(&mut self, event: Gd) { - println!("nya"); self.event = Some(event); } } +#[derive(GodotClass, Debug)] +#[class(init, base=ResourceFormatLoader)] +struct FormatLoaderTest { + #[base] + base: Base, +} + +impl FormatLoaderTest { + fn resource_type() -> GodotString { + GodotString::from("foo") + } +} + +#[godot_api] +impl ResourceFormatLoaderVirtual for FormatLoaderTest { + fn get_recognized_extensions(&self) -> PackedStringArray { + [GodotString::from("extension")].into_iter().collect() + } + + fn handles_type(&self, type_: StringName) -> bool { + type_.to_string() == Self::resource_type().to_string() + } + + fn get_resource_type(&self, _path: GodotString) -> GodotString { + Self::resource_type() + } + + fn exists(&self, _path: GodotString) -> bool { + true + } + + fn load( + &self, + _path: GodotString, + _original_path: GodotString, + _use_sub_threads: bool, + _cache_mode: i64, + ) -> Variant { + BoxMesh::new().to_variant() + } +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- #[itest] @@ -147,7 +203,7 @@ fn test_ready(test_context: &TestContext) { assert_eq!(obj.bind().implementation_value, 0); // Add to scene tree - let mut test_node = test_context.test_node.share(); + let mut test_node = test_context.scene_tree.share(); test_node.add_child( obj.share().upcast(), false, @@ -163,7 +219,7 @@ fn test_ready_multiple_fires(test_context: &TestContext) { let obj = Gd::::new_default(); assert_eq!(obj.bind().implementation_value, 0); - let mut test_node = test_context.test_node.share(); + let mut test_node = test_context.scene_tree.share(); // Add to scene tree test_node.add_child( @@ -192,7 +248,7 @@ fn test_ready_request_ready(test_context: &TestContext) { let obj = Gd::::new_default(); assert_eq!(obj.bind().implementation_value, 0); - let mut test_node = test_context.test_node.share(); + let mut test_node = test_context.scene_tree.share(); // Add to scene tree test_node.add_child( @@ -235,7 +291,7 @@ fn test_tree_enters_exits(test_context: &TestContext) { let obj = Gd::::new_default(); assert_eq!(obj.bind().tree_enters, 0); assert_eq!(obj.bind().tree_exits, 0); - let mut test_node = test_context.test_node.share(); + let mut test_node = test_context.scene_tree.share(); // Add to scene tree test_node.add_child( @@ -262,17 +318,76 @@ fn test_tree_enters_exits(test_context: &TestContext) { #[itest] fn test_virtual_method_with_return(_test_context: &TestContext) { let obj = Gd::::new_default(); - let output = obj.bind().get_configuration_warnings(); - assert!(output.contains("Hello".into())); - assert_eq!(output.len(), 1); - obj.free(); + let arr = obj.share().upcast::().get_mesh_arrays(); + let arr_rust = obj.bind().create_mesh_array(); + assert_eq!(arr.len(), arr_rust.len()); + // can't just assert_eq because the values of some floats change slightly + assert_eq_approx!( + arr.get(0).to::().get(0), + arr_rust.get(0).to::().get(0), + Vector3::is_equal_approx + ); + assert_eq_approx!( + arr.get(2).to::().get(3), + arr_rust.get(2).to::().get(3), + |a, b| is_equal_approx(real::from_f32(a), real::from_f32(b)) + ); + assert_eq_approx!( + arr.get(3).to::().get(0), + arr_rust.get(3).to::().get(0), + Color::is_equal_approx + ); + assert_eq_approx!( + arr.get(4).to::().get(0), + arr_rust.get(4).to::().get(0), + Vector2::is_equal_approx + ); + assert_eq!( + arr.get(6).to::(), + arr_rust.get(6).to::(), + ); + assert_eq!( + arr.get(10).to::(), + arr_rust.get(10).to::() + ); } +// TODO: Fix memory leak in this test. +#[itest(skip)] +fn test_format_loader(_test_context: &TestContext) { + let format_loader = Gd::::new_default(); + let mut loader = ResourceLoader::singleton(); + loader.add_resource_format_loader(format_loader.share().upcast(), true); + + let extensions = loader.get_recognized_extensions_for_type(FormatLoaderTest::resource_type()); + let mut extensions_rust = format_loader.bind().get_recognized_extensions(); + extensions_rust.push("tres".into()); + assert_eq!(extensions, extensions_rust); + let resource = loader + .load( + "path.extension".into(), + "".into(), + CacheMode::CACHE_MODE_IGNORE, + ) + .unwrap(); + assert!(resource.try_cast::().is_some()); + + loader.remove_resource_format_loader(format_loader.upcast()); +} + +#[itest] fn test_input_event(test_context: &TestContext) { let obj = Gd::::new_default(); assert_eq!(obj.bind().event, None); + let test_viewport = Window::new_alloc(); + + test_context.scene_tree.share().add_child( + test_viewport.share().upcast(), + false, + InternalMode::INTERNAL_MODE_DISABLED, + ); - test_context.test_node.share().add_child( + test_viewport.share().add_child( obj.share().upcast(), false, InternalMode::INTERNAL_MODE_DISABLED, @@ -283,8 +398,7 @@ fn test_input_event(test_context: &TestContext) { event.set_pressed(true); // We're running in headless mode, so Input.parse_input_event does not work - test_context - .root_node + test_viewport .share() .push_input(event.share().upcast(), false);