Skip to content

Commit

Permalink
Refactor GodotFfi a bit to use const vs mut pointers better and hav…
Browse files Browse the repository at this point in the history
…e more clear names
  • Loading branch information
lilizoey committed Feb 24, 2024
1 parent 8ecd619 commit c4c3181
Show file tree
Hide file tree
Showing 39 changed files with 625 additions and 340 deletions.
4 changes: 2 additions & 2 deletions godot-codegen/src/generator/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn make_builtin_class(class: &BuiltinClass, ctx: &mut Context) -> GeneratedBuilt
pub fn from_outer(outer: &#outer_class) -> Self {
Self {
_outer_lifetime: std::marker::PhantomData,
sys_ptr: outer.sys(),
sys_ptr: sys::AsConst::force_mut(outer.sys()),
}
}
#special_constructors
Expand Down Expand Up @@ -154,7 +154,7 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr
{
Self {
_outer_lifetime: std::marker::PhantomData,
sys_ptr: outer.sys(),
sys_ptr: sys::AsConst::force_mut(outer.sys()),
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions godot-core/src/builtin/aabb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,12 @@ unsafe impl GodotFfi for Aabb {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}

impl sys::GodotFfiBorrowable for Aabb {
unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self {
&*(ptr as *const Self)
}
}

impl_godot_as_self!(Aabb);

impl ApproxEq for Aabb {
Expand Down
70 changes: 47 additions & 23 deletions godot-core/src/builtin/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::builtin::*;
use crate::property::{Export, PropertyHintInfo, TypeStringHint, Var};
use std::fmt;
use std::marker::PhantomData;
use sys::{ffi_methods, interface_fn, GodotFfi};
use sys::{ffi_methods, interface_fn, GodotFfi, GodotFfiBorrowable};

use super::meta::{
ConvertError, FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant,
Expand Down Expand Up @@ -54,7 +54,7 @@ use super::meta::{
// for `T: GodotType` because `drop()` requires `sys_mut()`, which is on the `GodotFfi`
// trait, whose `from_sys_init()` requires `Default`, which is only implemented for `T:
// GodotType`. Whew. This could be fixed by splitting up `GodotFfi` if desired.
#[repr(C)]
#[repr(transparent)]
pub struct Array<T: GodotType> {
// Safety Invariant: The type of all values in `opaque` matches the type `T`.
opaque: sys::types::OpaqueArray,
Expand Down Expand Up @@ -253,7 +253,7 @@ impl<T: GodotType> Array<T> {
/// # Panics
///
/// If `index` is out of bounds.
fn ptr_mut(&self, index: usize) -> *mut Variant {
fn ptr_mut(&mut self, index: usize) -> *mut Variant {
let ptr = self.ptr_mut_or_null(index);
assert!(
!ptr.is_null(),
Expand All @@ -264,11 +264,11 @@ impl<T: GodotType> Array<T> {
}

/// Returns a pointer to the element at the given index, or null if out of bounds.
fn ptr_mut_or_null(&self, index: usize) -> *mut Variant {
fn ptr_mut_or_null(&mut self, index: usize) -> *mut Variant {
// SAFETY: array_operator_index returns null for invalid indexes.
let variant_ptr = unsafe {
let index = to_i64(index);
interface_fn!(array_operator_index)(self.sys(), index)
interface_fn!(array_operator_index)(self.sys_mut(), index)
};

Variant::ptr_from_sys_mut(variant_ptr)
Expand Down Expand Up @@ -456,7 +456,7 @@ impl<T: GodotType> Array<T> {
// SAFETY: The array is a newly created empty untyped array.
unsafe {
interface_fn!(array_set_typed)(
self.sys(),
self.sys_mut(),
type_info.variant_type.sys(),
type_info.class_name.string_sys(),
script.var_sys(),
Expand Down Expand Up @@ -745,22 +745,46 @@ unsafe impl<T: GodotType> GodotFfi for Array<T> {
}

ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn new_from_sys;
fn sys;
fn from_sys_init;
fn sys_mut;
fn new_with_uninit;
fn new_with_init;
fn move_return_ptr;
}

unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self {
let mut result = Self::default();
init_fn(result.sys_mut());
result
unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let array = Self::borrow_sys(ptr);
array.clone()
}
}

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
impl<T: GodotType> sys::HasOpaque for Array<T> {
type Opaque = sys::types::OpaqueArray;

unsafe fn borrow_opaque<'a>(ptr: *const Self::Opaque) -> &'a Self {
&*(ptr as *mut Self)
}

fn from_opaque(opaque: Self::Opaque) -> Self {
Self {
opaque,
_phantom: PhantomData,
}
}

fn opaque(&self) -> &Self::Opaque {
&self.opaque
}

fn opaque_mut(&mut self) -> &mut Self::Opaque {
&mut self.opaque
}
}

impl<T: GodotType> GodotFfiBorrowable for Array<T> {
unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self {
sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueArray)
}
}

Expand Down Expand Up @@ -825,9 +849,9 @@ impl<T: GodotType> Clone for Array<T> {
fn clone(&self) -> Self {
// SAFETY: `self` is a valid array, since we have a reference that keeps it alive.
let array = unsafe {
Self::from_sys_init(|self_ptr| {
Self::new_with_uninit(|self_ptr| {
let ctor = ::godot_ffi::builtin_fn!(array_construct_copy);
let args = [self.sys_const()];
let args = [self.sys()];
ctor(self_ptr, args.as_ptr());
})
};
Expand Down Expand Up @@ -891,7 +915,7 @@ impl<T: GodotType> Default for Array<T> {
#[inline]
fn default() -> Self {
let mut array = unsafe {
Self::from_sys_init(|self_ptr| {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(array_construct_default);
ctor(self_ptr, std::ptr::null_mut())
})
Expand Down Expand Up @@ -937,9 +961,9 @@ impl<T: GodotType> GodotType for Array<T> {
impl<T: GodotType> GodotFfiVariant for Array<T> {
fn ffi_to_variant(&self) -> Variant {
unsafe {
Variant::from_var_sys_init(|variant_ptr| {
Variant::new_with_uninit_var_sys(|variant_ptr| {
let array_to_variant = sys::builtin_fn!(array_to_variant);
array_to_variant(variant_ptr, self.sys());
array_to_variant(variant_ptr, sys::AsConst::force_mut(self.sys()));
})
}
}
Expand All @@ -956,7 +980,7 @@ impl<T: GodotType> GodotFfiVariant for Array<T> {
let array = unsafe {
sys::from_sys_init_or_init_default::<Self>(|self_ptr| {
let array_from_variant = sys::builtin_fn!(array_from_variant);
array_from_variant(self_ptr, variant.var_sys());
array_from_variant(self_ptr, sys::AsConst::force_mut(variant.var_sys()));
})
};

Expand All @@ -977,7 +1001,7 @@ impl<T: GodotType + ToGodot, const N: usize> From<&[T; N]> for Array<T> {
/// Creates a `Array` from the given slice.
impl<T: GodotType + ToGodot> From<&[T]> for Array<T> {
fn from(slice: &[T]) -> Self {
let array = Self::new();
let mut array = Self::new();
let len = slice.len();
if len == 0 {
return array;
Expand Down
6 changes: 6 additions & 0 deletions godot-core/src/builtin/basis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,12 @@ unsafe impl GodotFfi for Basis {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}

impl sys::GodotFfiBorrowable for Basis {
unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self {
&*(ptr as *const Self)
}
}

impl_godot_as_self!(Basis);

/// The ordering used to interpret a set of euler angles as extrinsic
Expand Down
40 changes: 27 additions & 13 deletions godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::obj::bounds::DynMemory;
use crate::obj::Bounds;
use crate::obj::{Gd, GodotClass, InstanceId};
use std::{fmt, ptr};
use sys::{ffi_methods, GodotFfi};
use sys::{ffi_methods, GodotFfi, GodotFfiBorrowable};

/// A `Callable` represents a function in Godot.
///
Expand All @@ -25,7 +25,7 @@ use sys::{ffi_methods, GodotFfi};
/// Currently it is impossible to use `bind` and `unbind` in GDExtension, see [godot-cpp#802].
///
/// [godot-cpp#802]: https://github.com/godotengine/godot-cpp/issues/802
#[repr(C, align(8))]
#[repr(transparent)]
pub struct Callable {
opaque: sys::types::OpaqueCallable,
}
Expand All @@ -51,7 +51,7 @@ impl Callable {
sys::from_sys_init_or_init_default::<Self>(|self_ptr| {
let ctor = sys::builtin_fn!(callable_from_object_method);
let raw = object.to_ffi();
let args = [raw.as_arg_ptr(), method.sys_const()];
let args = [raw.as_arg_ptr(), method.sys()];
ctor(self_ptr, args.as_ptr());
})
}
Expand Down Expand Up @@ -140,7 +140,7 @@ impl Callable {
fn from_custom_info(mut info: sys::GDExtensionCallableCustomInfo) -> Callable {
// SAFETY: callable_custom_create() is a valid way of creating callables.
unsafe {
Callable::from_sys_init(|type_ptr| {
Callable::new_with_uninit(|type_ptr| {
sys::interface_fn!(callable_custom_create)(type_ptr, ptr::addr_of_mut!(info))
})
}
Expand All @@ -151,7 +151,7 @@ impl Callable {
/// _Godot equivalent: `Callable()`_
pub fn invalid() -> Self {
unsafe {
Self::from_sys_init(|self_ptr| {
Self::new_with_uninit(|self_ptr| {
let ctor = sys::builtin_fn!(callable_construct_default);
ctor(self_ptr, ptr::null_mut())
})
Expand Down Expand Up @@ -293,22 +293,36 @@ unsafe impl GodotFfi for Callable {
}

ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque;
fn from_sys;
fn new_from_sys;
fn sys;
fn from_sys_init;
fn sys_mut;
fn new_with_uninit;
fn move_return_ptr;
}

unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
let callable = Self::from_sys(ptr);
callable.inc_ref();
let callable = Self::borrow_sys(ptr);
callable.clone()
}

fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self
where
Self: Sized,
{
let mut callable = Callable::invalid();
init_fn(&mut callable);

callable
}
}

sys::unsafe_impl_has_opaque!(Callable {
opaque: sys::types::OpaqueCallable
});

unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self {
let mut result = Self::invalid();
init_fn(result.sys_mut());
result
impl GodotFfiBorrowable for Callable {
unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self {
sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueCallable)
}
}

Expand Down
6 changes: 6 additions & 0 deletions godot-core/src/builtin/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,12 @@ unsafe impl GodotFfi for Color {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. }
}

impl sys::GodotFfiBorrowable for Color {
unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self {
&*(ptr as *const Self)
}
}

impl_godot_as_self!(Color);

impl ApproxEq for Color {
Expand Down
Loading

0 comments on commit c4c3181

Please sign in to comment.