Skip to content

Commit b5e74b3

Browse files
committed
Fix various issues with pointer calls
Make `Gd` in virtual method-calls use `ref_get_object`/`ref_set_object` if it inherits from `RefCounted` Add some tests of objects in pointer calls Add tests for Array passing through pointer calls Add more tests for refcounted Add a test for _input Rename some stuff in TestContext Add unsafe to rect and such GodotFfi impl along with `SAFETY` comments
1 parent 3d0e3d2 commit b5e74b3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+695
-166
lines changed

godot-codegen/src/central_generator.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,9 @@ fn make_sys_code(central_items: &CentralItems) -> String {
176176
}
177177
}
178178

179-
impl GodotFfi for VariantType {
179+
// SAFETY:
180+
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
181+
unsafe impl GodotFfi for VariantType {
180182
ffi_methods! { type GDExtensionTypePtr = *mut Self; .. }
181183
}
182184

@@ -205,7 +207,9 @@ fn make_sys_code(central_items: &CentralItems) -> String {
205207
}
206208
}
207209

208-
impl GodotFfi for VariantOperator {
210+
// SAFETY:
211+
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
212+
unsafe impl GodotFfi for VariantOperator {
209213
ffi_methods! { type GDExtensionTypePtr = *mut Self; .. }
210214
}
211215
};

godot-codegen/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,8 @@ const SELECTED_CLASSES: &[&str] = &[
271271
"Image",
272272
"ImageTextureLayered",
273273
"Input",
274+
"InputEvent",
275+
"InputEventAction",
274276
"Label",
275277
"MainLoop",
276278
"Marker2D",
@@ -296,4 +298,6 @@ const SELECTED_CLASSES: &[&str] = &[
296298
"TextureLayered",
297299
"Time",
298300
"Timer",
301+
"Window",
302+
"Viewport",
299303
];

godot-codegen/src/util.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ pub fn make_enum_definition(enum_: &Enum) -> TokenStream {
9999
self.ord
100100
}
101101
}
102-
impl sys::GodotFfi for #enum_name {
102+
unsafe impl sys::GodotFfi for #enum_name {
103103
sys::ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
104104
}
105105
#bitfield_ops

godot-core/src/builtin/aabb.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ impl Aabb {
8282
*/
8383
}
8484

85-
impl GodotFfi for Aabb {
85+
// SAFETY:
86+
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
87+
unsafe impl GodotFfi for Aabb {
8688
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
8789
}

godot-core/src/builtin/array.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,26 @@ impl<T: VariantMetadata + ToVariant> Array<T> {
566566
// ...
567567
// }
568568

569-
impl<T: VariantMetadata> GodotFfi for Array<T> {
570-
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. }
569+
unsafe impl<T: VariantMetadata> GodotFfi for Array<T> {
570+
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
571+
fn from_sys;
572+
fn sys;
573+
fn from_sys_init;
574+
// SAFETY:
575+
// Nothing special needs to be done beyond a `std::mem::swap` when returning an Array.
576+
fn move_return_ptr;
577+
}
578+
579+
// SAFETY:
580+
// Arrays are properly initialized through a `from_sys` call, but the ref-count should be
581+
// incremented as that is the callee's responsibility.
582+
//
583+
// Using `std::mem::forget(array.share())` increments the ref count.
584+
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
585+
let array = Self::from_sys(ptr);
586+
std::mem::forget(array.share());
587+
array
588+
}
571589

572590
unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self {
573591
let mut result = Self::default();

godot-core/src/builtin/basis.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,9 @@ impl Mul<Vector3> for Basis {
570570
}
571571
}
572572

573-
impl GodotFfi for Basis {
573+
// SAFETY:
574+
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
575+
unsafe impl GodotFfi for Basis {
574576
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
575577
}
576578

godot-core/src/builtin/color.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,9 @@ impl Color {
311311
}
312312
}
313313

314-
impl GodotFfi for Color {
314+
// SAFETY:
315+
// This type is transparently represented as `Self` in Godot, so `*mut Self` is sound.
316+
unsafe impl GodotFfi for Color {
315317
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
316318
}
317319

godot-core/src/builtin/dictionary.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,26 @@ impl Dictionary {
239239
// ----------------------------------------------------------------------------------------------------------------------------------------------
240240
// Traits
241241

242-
impl GodotFfi for Dictionary {
243-
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. }
242+
unsafe impl GodotFfi for Dictionary {
243+
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
244+
fn from_sys;
245+
fn from_sys_init;
246+
fn sys;
247+
// SAFETY:
248+
// Nothing special needs to be done beyond a `std::mem::swap` when returning a dictionary.
249+
fn move_return_ptr;
250+
}
251+
252+
// SAFETY:
253+
// Dictionaries are properly initialized through a `from_sys` call, but the ref-count should be
254+
// incremented as that is the callee's responsibility.
255+
//
256+
// Using `std::mem::forget(dictionary.share())` increments the ref count.
257+
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::CallType) -> Self {
258+
let dictionary = Self::from_sys(ptr);
259+
std::mem::forget(dictionary.share());
260+
dictionary
261+
}
244262

245263
unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self {
246264
let mut result = Self::default();

godot-core/src/builtin/macros.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ macro_rules! impl_builtin_stub {
157157
}
158158
}
159159

160-
impl GodotFfi for $Class {
160+
// SAFETY:
161+
// This is simply a wrapper around an `Opaque` value representing a value of the type.
162+
// So this is safe.
163+
unsafe impl GodotFfi for $Class {
161164
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. }
162165
}
163166
};

godot-core/src/builtin/meta/signature.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub trait SignatureTuple {
3434
ret: sys::GDExtensionTypePtr,
3535
func: fn(&mut C, Self::Params) -> Self::Ret,
3636
method_name: &str,
37+
call_type: sys::CallType,
3738
);
3839
}
3940

@@ -143,6 +144,7 @@ macro_rules! impl_signature_for_tuple {
143144
ret: sys::GDExtensionTypePtr,
144145
func: fn(&mut C, Self::Params) -> Self::Ret,
145146
method_name: &str,
147+
call_type: sys::CallType,
146148
) {
147149
$crate::out!("ptrcall: {}", method_name);
148150

@@ -151,20 +153,20 @@ macro_rules! impl_signature_for_tuple {
151153

152154
let args = ( $(
153155
unsafe {
154-
<$Pn as sys::GodotFuncMarshal>::try_from_sys(
155-
sys::force_mut_ptr(*args_ptr.offset($n))
156+
<$Pn as sys::GodotFuncMarshal>::try_from_arg(
157+
sys::force_mut_ptr(*args_ptr.offset($n)),
158+
call_type
156159
)
157160
}
158161
.unwrap_or_else(|e| param_error::<$Pn>(method_name, $n, &e)),
159162
)* );
160163

161164
let ret_val = func(&mut *instance, args);
162-
unsafe { <$R as sys::GodotFuncMarshal>::try_write_sys(&ret_val, ret) }
163-
.unwrap_or_else(|e| return_error::<$R>(method_name, &e));
164-
165-
//Note: we want to prevent the destructor from being run, since we pass ownership to the caller.
166-
//https://github.com/godot-rust/gdext/pull/165
167-
std::mem::forget(ret_val);
165+
// SAFETY:
166+
// `ret` is always a pointer to an initialized value of type $R
167+
// TODO: double-check the above
168+
<$R as sys::GodotFuncMarshal>::try_return(ret_val, ret, call_type)
169+
.unwrap_or_else(|ret_val| return_error::<$R>(method_name, &ret_val));
168170
}
169171
}
170172
};

0 commit comments

Comments
 (0)