Skip to content

Commit

Permalink
Merge pull request #838 from godot-rust/qol/register-traits
Browse files Browse the repository at this point in the history
Simplify property hint APIs
  • Loading branch information
Bromeon authored Aug 4, 2024
2 parents b4ffe6f + 5fbde3b commit b28cc97
Show file tree
Hide file tree
Showing 18 changed files with 519 additions and 520 deletions.
56 changes: 29 additions & 27 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ use std::marker::PhantomData;
use crate::builtin::*;
use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
use crate::meta::{
ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
};
use crate::obj::EngineEnum;
use crate::registry::property::{
builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var,
ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType,
PropertyHintInfo, ToGodot,
};
use crate::registry::property::{Export, Var};
use godot_ffi as sys;
use sys::{ffi_methods, interface_fn, GodotFfi};

Expand Down Expand Up @@ -778,6 +776,14 @@ impl<T: ArrayElement> Array<T> {
}
}
}

/// Whether this array is untyped and holds `Variant` elements (compile-time check).
///
/// Used as `if` statement in trait impls. Avoids defining yet another trait or non-local overridden function just for this case;
/// `Variant` is the only Godot type that has variant type NIL and can be used as an array element.
fn has_variant_t() -> bool {
T::Ffi::variant_type() == VariantType::NIL
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -886,18 +892,6 @@ impl<T: ArrayElement> Clone for Array<T> {
}
}

impl<T: ArrayElement + TypeStringHint> TypeStringHint for Array<T> {
fn type_string() -> String {
format!("{}:{}", VariantType::ARRAY.ord(), T::type_string())
}
}

impl TypeStringHint for VariantArray {
fn type_string() -> String {
builtin_type_string::<VariantArray>()
}
}

impl<T: ArrayElement> Var for Array<T> {
fn get_property(&self) -> Self::Via {
self.to_godot()
Expand All @@ -906,20 +900,28 @@ impl<T: ArrayElement> Var for Array<T> {
fn set_property(&mut self, value: Self::Via) {
*self = FromGodot::from_godot(value)
}
}

impl<T: ArrayElement + TypeStringHint> Export for Array<T> {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo {
hint: crate::global::PropertyHint::TYPE_STRING,
hint_string: T::type_string().into(),
fn var_hint() -> PropertyHintInfo {
// For array #[var], the hint string is "PackedInt32Array", "Node" etc. for typed arrays, and "" for untyped arrays.
if Self::has_variant_t() {
PropertyHintInfo::none()
} else if sys::GdextBuild::since_api("4.2") {
PropertyHintInfo::var_array_element::<T>()
} else {
// Godot 4.1 was not using PropertyHint::ARRAY_TYPE, but the empty hint instead.
PropertyHintInfo::none()
}
}
}

impl Export for Array<Variant> {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo::with_hint_none("Array")
impl<T: ArrayElement> Export for Array<T> {
fn export_hint() -> PropertyHintInfo {
// If T == Variant, then we return "Array" builtin type hint.
if Self::has_variant_t() {
PropertyHintInfo::type_name::<VariantArray>()
} else {
PropertyHintInfo::export_array_element::<T>()
}
}
}

Expand Down Expand Up @@ -975,7 +977,7 @@ impl<T: ArrayElement> GodotType for Array<T> {
#[cfg(since_api = "4.2")]
fn property_hint_info() -> PropertyHintInfo {
// Array<Variant>, aka untyped array, has no hints.
if T::Ffi::variant_type() == VariantType::NIL {
if Self::has_variant_t() {
return PropertyHintInfo::none();
}

Expand Down
25 changes: 0 additions & 25 deletions godot-core/src/builtin/collections/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ use godot_ffi as sys;

use crate::builtin::{inner, Variant, VariantArray};
use crate::meta::{FromGodot, ToGodot};
use crate::registry::property::{
builtin_type_string, Export, PropertyHintInfo, TypeStringHint, Var,
};
use sys::types::OpaqueDictionary;
use sys::{ffi_methods, interface_fn, GodotFfi};

Expand Down Expand Up @@ -402,28 +399,6 @@ impl Clone for Dictionary {
}
}

impl Var for Dictionary {
fn get_property(&self) -> Self::Via {
self.to_godot()
}

fn set_property(&mut self, value: Self::Via) {
*self = FromGodot::from_godot(value)
}
}

impl TypeStringHint for Dictionary {
fn type_string() -> String {
builtin_type_string::<Dictionary>()
}
}

impl Export for Dictionary {
fn default_export_info() -> PropertyHintInfo {
PropertyHintInfo::with_hint_none("Dictionary")
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Conversion traits

Expand Down
11 changes: 3 additions & 8 deletions godot-core/src/builtin/collections/packed_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,17 +545,12 @@ macro_rules! impl_packed_array {
$crate::meta::impl_godot_as_self!($PackedArray);

impl $crate::registry::property::Export for $PackedArray {
fn default_export_info() -> $crate::registry::property::PropertyHintInfo {
fn export_hint() -> $crate::meta::PropertyHintInfo {
// In 4.3 Godot can (and does) use type hint strings for packed arrays, see https://github.com/godotengine/godot/pull/82952.
if sys::GdextBuild::since_api("4.3") {
$crate::registry::property::PropertyHintInfo {
hint: $crate::global::PropertyHint::TYPE_STRING,
hint_string: <$Element as $crate::registry::property::TypeStringHint>::type_string().into(),
}
$crate::meta::PropertyHintInfo::export_array_element::<$Element>()
} else {
$crate::registry::property::PropertyHintInfo::with_hint_none(
<$PackedArray as $crate::meta::GodotType>::godot_type_name()
)
$crate::meta::PropertyHintInfo::type_name::<$PackedArray>()
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions godot-core/src/builtin/variant/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use super::*;
use crate::builtin::*;
use crate::global;
use crate::meta::error::{ConvertError, FromVariantError};
use crate::meta::{ArrayElement, GodotFfiVariant, GodotType, PropertyInfo};
use crate::registry::property::PropertyHintInfo;
use crate::meta::{ArrayElement, GodotFfiVariant, GodotType, PropertyHintInfo, PropertyInfo};
use godot_ffi as sys;
// For godot-cpp, see https://github.com/godotengine/godot-cpp/blob/master/include/godot_cpp/core/type_info.hpp.

Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/meta/godot_convert/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
use crate::builtin::Variant;
use crate::meta::error::{ConvertError, FromFfiError, FromVariantError};
use crate::meta::{
ArrayElement, ClassName, FromGodot, GodotConvert, GodotNullableFfi, GodotType, PropertyInfo,
ToGodot,
ArrayElement, ClassName, FromGodot, GodotConvert, GodotNullableFfi, GodotType,
PropertyHintInfo, PropertyInfo, ToGodot,
};
use crate::registry::method::MethodParamOrReturnInfo;
use crate::registry::property::PropertyHintInfo;
use godot_ffi as sys;

// The following ToGodot/FromGodot/Convert impls are auto-generated for each engine type, co-located with their definitions:
// - enum
// - const/mut pointer to native struct
Expand Down
143 changes: 143 additions & 0 deletions godot-core/src/meta/method_info.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Copyright (c) godot-rust; Bromeon and contributors.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/
use crate::builtin::{StringName, Variant};
use crate::global::MethodFlags;
use crate::meta::{ClassName, PropertyInfo};
use crate::sys;
use godot_ffi::conv::u32_to_usize;

/// Describes a method in Godot.
///
/// Abstraction of the low-level `sys::GDExtensionMethodInfo`.
// Currently used for ScriptInstance.
// TODO check overlap with (private) ClassMethodInfo.
#[derive(Debug, Clone)]
pub struct MethodInfo {
pub id: i32,
pub method_name: StringName,
pub class_name: ClassName,
pub return_type: PropertyInfo,
pub arguments: Vec<PropertyInfo>,
pub default_arguments: Vec<Variant>,
pub flags: MethodFlags,
}

impl MethodInfo {
/// Consumes self and turns it into a `sys::GDExtensionMethodInfo`, should be used together with
/// [`free_owned_method_sys`](Self::free_owned_method_sys).
///
/// This will leak memory unless used together with `free_owned_method_sys`.
pub fn into_owned_method_sys(self) -> sys::GDExtensionMethodInfo {
use crate::obj::EngineBitfield as _;

// Destructure self to ensure all fields are used.
let Self {
id,
method_name,
// TODO: Do we need this?
class_name: _class_name,
return_type,
arguments,
default_arguments,
flags,
} = self;

let argument_count: u32 = arguments
.len()
.try_into()
.expect("cannot have more than `u32::MAX` arguments");
let arguments = arguments
.into_iter()
.map(|arg| arg.into_owned_property_sys())
.collect::<Box<[_]>>();
let arguments = Box::leak(arguments).as_mut_ptr();

let default_argument_count: u32 = default_arguments
.len()
.try_into()
.expect("cannot have more than `u32::MAX` default arguments");
let default_argument = default_arguments
.into_iter()
.map(|arg| arg.into_owned_var_sys())
.collect::<Box<[_]>>();
let default_arguments = Box::leak(default_argument).as_mut_ptr();

sys::GDExtensionMethodInfo {
id,
name: method_name.into_owned_string_sys(),
return_value: return_type.into_owned_property_sys(),
argument_count,
arguments,
default_argument_count,
default_arguments,
flags: flags.ord().try_into().expect("flags should be valid"),
}
}

/// Properly frees a `sys::GDExtensionMethodInfo` created by [`into_owned_method_sys`](Self::into_owned_method_sys).
///
/// # Safety
///
/// * Must only be used on a struct returned from a call to `into_owned_method_sys`, without modification.
/// * Must not be called more than once on a `sys::GDExtensionMethodInfo` struct.
#[deny(unsafe_op_in_unsafe_fn)]
pub unsafe fn free_owned_method_sys(info: sys::GDExtensionMethodInfo) {
// Destructure info to ensure all fields are used.
let sys::GDExtensionMethodInfo {
name,
return_value,
flags: _flags,
id: _id,
argument_count,
arguments,
default_argument_count,
default_arguments,
} = info;

// SAFETY: `name` is a pointer that was returned from `StringName::into_owned_string_sys`, and has not been freed before this.
let _name = unsafe { StringName::from_owned_string_sys(name) };

// SAFETY: `return_value` is a pointer that was returned from `PropertyInfo::into_owned_property_sys`, and has not been freed before
// this.
unsafe { PropertyInfo::free_owned_property_sys(return_value) };

// SAFETY:
// - `from_raw_parts_mut`: `arguments` comes from `as_mut_ptr()` on a mutable slice of length `argument_count`, and no other
// accesses to the pointer happens for the lifetime of the slice.
// - `Box::from_raw`: The slice was returned from a call to `Box::leak`, and we have ownership of the value behind this pointer.
let arguments = unsafe {
let slice = std::slice::from_raw_parts_mut(arguments, u32_to_usize(argument_count));

Box::from_raw(slice)
};

for info in arguments.iter() {
// SAFETY: These infos were originally created from a call to `PropertyInfo::into_owned_property_sys`, and this method
// will not be called again on this pointer.
unsafe { PropertyInfo::free_owned_property_sys(*info) }
}

// SAFETY:
// - `from_raw_parts_mut`: `default_arguments` comes from `as_mut_ptr()` on a mutable slice of length `default_argument_count`, and no
// other accesses to the pointer happens for the lifetime of the slice.
// - `Box::from_raw`: The slice was returned from a call to `Box::leak`, and we have ownership of the value behind this pointer.
let default_arguments = unsafe {
let slice = std::slice::from_raw_parts_mut(
default_arguments,
u32_to_usize(default_argument_count),
);

Box::from_raw(slice)
};

for variant in default_arguments.iter() {
// SAFETY: These pointers were originally created from a call to `Variant::into_owned_var_sys`, and this method will not be
// called again on this pointer.
let _variant = unsafe { Variant::from_owned_var_sys(*variant) };
}
}
}
Loading

0 comments on commit b28cc97

Please sign in to comment.