Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add implementation of Callable #231

Merged
merged 1 commit into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions godot-codegen/src/class_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,11 +629,6 @@ fn make_builtin_method_definition(
type_info: &BuiltinTypeInfo,
ctx: &mut Context,
) -> TokenStream {
// TODO implement varcalls
if method.is_vararg {
return TokenStream::new();
}

let method_name_str = &method.name;

let (receiver, receiver_arg) =
Expand Down
201 changes: 201 additions & 0 deletions godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
/*
* 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 godot_ffi as sys;

use crate::builtin::{inner, ToVariant, Variant};
use crate::engine::Object;
use crate::obj::mem::Memory;
use crate::obj::{Gd, GodotClass, InstanceId};
use std::fmt;
use sys::{ffi_methods, GodotFfi};

use super::{StringName, VariantArray};

/// A `Callable` represents a function in Godot.
///
/// Usually a callable is a reference to an `Object` and a method name, this is a standard callable. But can
/// also be a custom callable, which is usually created from `bind`, `unbind`, or a GDScript lambda. See
/// [`Callable::is_custom`].
///
/// 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))]
pub struct Callable {
opaque: sys::types::OpaqueCallable,
}

impl Callable {
fn from_opaque(opaque: sys::types::OpaqueCallable) -> Self {
Self { opaque }
}

/// Create a callable for the method `object::method_name`.
///
/// _Godot equivalent: `Callable(Object object, StringName method)`_
pub fn from_object_method<T, S>(object: Gd<T>, method_name: S) -> Self
where
T: GodotClass, // + Inherits<Object>,
S: Into<StringName>,
{
// upcast not needed
let method = method_name.into();
unsafe {
Self::from_sys_init_default(|self_ptr| {
let ctor = sys::builtin_fn!(callable_from_object_method);
let args = [object.sys_const(), method.sys_const()];
ctor(self_ptr, args.as_ptr());
})
}
}

/// Calls the method represented by this callable.
///
/// Arguments passed should match the method's signature.
///
/// - If called with more arguments than expected by the method, the extra arguments will be ignored and
/// the call continues as normal.
/// - If called with fewer arguments than expected it will crash Godot, without triggering UB.
/// - If called with arguments of the wrong type then an error will be printed and the call will return
/// `NIL`.
/// - If called on an invalid Callable then no error is printed, and `NIL` is returned.
///
/// _Godot equivalent: `callv`_
pub fn callv(&self, arguments: VariantArray) -> Variant {
self.as_inner().callv(arguments)
}

/// Returns the name of the method represented by this callable. If the callable is a lambda function,
/// returns the function's name.
Comment on lines +72 to +73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the callable is a lambda function, returns the function's name.

What does that mean? Lambdas have no name, at least not from the user perspective.

Since we have Option return type, would it be possible to return None in cases where the function name is not meaningful for the user?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lambdas can in fact have names in GDScript, which is kinda counter-intuitive since that's not what lambda means. Here is an example of a lambda with a name:

func _ready():
	var foo = func bleh():
		pass
	print(foo)

this will print

bleh(lambda)

Though i realize now that the godot docs are actually wrong and trying to use get_method on a lambda does not in fact return the function's name but rather just errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

godotengine/godot#73052 this is a known bug apparently, not sure how we should handle that in our case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see! Regarding the bug, it's probably OK to rely on it working, hopefully it will be fixed at some point. We could maybe add a mention in the doc.

///
/// ## Known Bugs
///
/// Getting the name of a lambda errors instead of returning its name, see [godot#73052].
///
/// _Godot equivalent: `get_method`_
///
/// [godot#73052]: https://github.com/godotengine/godot/issues/73052
pub fn method_name(&self) -> Option<StringName> {
let method_name = self.as_inner().get_method();
if method_name.is_empty() {
None
} else {
Some(method_name)
}
}

/// Returns the object on which this callable is called.
///
/// Returns `None` when this callable doesn't have any target object to call a method on, regardless of
/// if the method exists for that target or not.
///
/// _Godot equivalent: `get_object`_
pub fn object(&self) -> Option<Gd<Object>> {
// Increment refcount because we're getting a reference, and `InnerCallable::get_object` doesn't
// increment the refcount.
self.as_inner().get_object().map(|object| {
<Object as GodotClass>::Mem::maybe_inc_ref(&object);
object
})
}

/// Returns the ID of this callable's object, see also [`Gd::instance_id`].
///
/// Returns `None` when this callable doesn't have any target to call a method on.
///
/// _Godot equivalent: `get_object_id`_
Comment on lines +106 to +110
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also None when the state is invalid?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

pub fn object_id(&self) -> Option<InstanceId> {
let id = self.as_inner().get_object_id();
InstanceId::try_from_i64(id)
}

/// Returns the 32-bit hash value of this callable's object.
///
/// _Godot equivalent: `hash`_
pub fn hash(&self) -> u32 {
self.as_inner().hash().try_into().unwrap()
}
Comment on lines +116 to +121
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guaranteed not to panic? The values returned by Godot are unsigned?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to Array::hash at least yeah. i think i checked this myself at some point too.

Copy link
Member Author

@lilizoey lilizoey Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also if this ever breaks/changes then it's likely gonna break for every call to hash because of how hashes generally work. so we'd notice in for instance the callable_hash itest.


/// Returns true if this callable is a custom callable.
///
/// Custom callables are mainly created from bind or unbind. In GDScript, lambda functions are also
/// custom callables.
///
/// If a callable is not a custom callable, then it is considered a standard callable, this function is
/// the opposite of [`Callable.is_standard`].
///
/// _Godot equivalent: `is_custom`_
///
/// [`Callable.is_standard`]: https://docs.godotengine.org/en/stable/classes/class_callable.html#class-callable-method-is-standard
#[doc(alias = "is_standard")]
pub fn is_custom(&self) -> bool {
self.as_inner().is_custom()
}

/// Returns true if this callable has no target to call the method on.
///
/// This is not the negated form of [`is_valid`], as `is_valid` will return `false` if the callable has a
/// target but the method does not exist.
///
/// _Godot equivalent: `is_null`_
pub fn is_null(&self) -> bool {
self.as_inner().is_null()
}

/// Returns true if the callable's object exists and has a valid method name assigned, or is a custom
/// callable.
///
/// _Godot equivalent: `is_valid`_
pub fn is_valid(&self) -> bool {
self.as_inner().is_valid()
}

#[doc(hidden)]
pub fn as_inner(&self) -> inner::InnerCallable {
inner::InnerCallable::from_outer(self)
}
}

impl_builtin_traits! {
for Callable {
Default => callable_construct_default;
// Equality for custom callables depend on the equality implementation of that custom callable. This
// is from what i can tell currently implemented as total equality in all cases, but i dont believe
// there are any guarantees that all implementations of equality for custom callables will be.
//
// So we cannot implement `Eq` here and be confident equality will be total for all future custom
// callables.
PartialEq => callable_operator_equal;
Clone => callable_construct_copy;
Drop => callable_destroy;
}
}

// SAFETY:
// The `opaque` in `Callable` is just a pair of pointers, and requires no special initialization or cleanup
// beyond what is done in `from_opaque` and `drop`. So using `*mut Opaque` is safe.
unsafe impl GodotFfi for Callable {
ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. }
}

impl std::fmt::Debug for Callable {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let method = self.method_name();
let object = self.object();

f.debug_struct("Callable")
.field("method", &method)
.field("object", &object)
.finish()
}
}

impl std::fmt::Display for Callable {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.to_variant())
}
}
19 changes: 0 additions & 19 deletions godot-core/src/builtin/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,25 +107,6 @@ macro_rules! impl_builtin_traits_inner {
}
}
};

// TODO remove; use godot-core/src/builtin/variant/impls.rs instead (this one is only used for Callable)
( FromVariant for $Type:ty => $gd_method:ident ) => {
impl $crate::builtin::variant::FromVariant for $Type {
fn try_from_variant(variant: &$crate::builtin::Variant) -> Result<Self, $crate::builtin::variant::VariantConversionError> {
if variant.get_type() != <Self as $crate::builtin::meta::VariantMetadata>::variant_type() {
return Err($crate::builtin::variant::VariantConversionError)
}
let result = unsafe {
Self::from_sys_init_default(|self_ptr| {
let converter = sys::builtin_fn!($gd_method);
converter(self_ptr, variant.var_sys());
})
};

Ok(result)
}
}
};
}

macro_rules! impl_builtin_traits {
Expand Down
9 changes: 7 additions & 2 deletions godot-core/src/builtin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub use crate::{array, dict, varray};
pub use aabb::*;
pub use array_inner::{Array, VariantArray};
pub use basis::*;
pub use callable::*;
pub use color::*;
pub use dictionary_inner::Dictionary;
pub use math::*;
Expand Down Expand Up @@ -90,6 +91,7 @@ mod dictionary_inner;

mod aabb;
mod basis;
mod callable;
mod color;
mod glam_helpers;
mod math;
Expand Down Expand Up @@ -403,7 +405,10 @@ mod export {
impl_export_by_clone!(Vector3i);
impl_export_by_clone!(Vector4);

// TODO investigate whether these should impl Export at all, and if so, how
// impl_export_by_clone!(Callable);
// Callables can be exported, however you can't do anything with them in the editor.
// But we do need to be able to export them since we can't make something a property without exporting.
// And it should be possible to access Callables by property from for instance GDScript.
impl_export_by_clone!(Callable);
// TODO investigate whether Signal should impl Export at all, and if so, how
// impl_export_by_clone!(Signal);
}
33 changes: 0 additions & 33 deletions godot-core/src/builtin/others.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,9 @@

// Stub for various other built-in classes, which are currently incomplete, but whose types
// are required for codegen
use crate::builtin::{inner, StringName};
use crate::obj::{Gd, GodotClass};
use godot_ffi as sys;
use sys::{ffi_methods, GodotFfi};

// TODO: Swap more inner math types with glam types
// Note: ordered by enum ord in extension JSON
impl_builtin_stub!(Callable, OpaqueCallable);
impl_builtin_stub!(Signal, OpaqueSignal);

impl Callable {
pub fn from_object_method<T, S>(object: Gd<T>, method: S) -> Self
where
T: GodotClass, // + Inherits<Object>,
S: Into<StringName>,
{
// upcast not needed
let method = method.into();
unsafe {
Self::from_sys_init_default(|self_ptr| {
let ctor = sys::builtin_fn!(callable_from_object_method);
let args = [object.sys_const(), method.sys_const()];
ctor(self_ptr, args.as_ptr());
})
}
}

#[doc(hidden)]
pub fn as_inner(&self) -> inner::InnerCallable {
inner::InnerCallable::from_outer(self)
}
}

impl_builtin_traits! {
for Callable {
// Default => callable_construct_default;
FromVariant => callable_from_variant;
}
}
21 changes: 21 additions & 0 deletions godot-core/src/builtin/string_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use sys::{ffi_methods, GodotFfi};
use std::fmt;
use std::hash::{Hash, Hasher};

use super::inner;

#[repr(C)]
pub struct StringName {
opaque: sys::types::OpaqueStringName,
Expand All @@ -21,6 +23,25 @@ impl StringName {
Self { opaque }
}

/// Returns the number of characters in the string.
///
/// _Godot equivalent: `length`_
pub fn len(&self) -> usize {
self.as_inner().length() as usize
}

/// Returns `true` if this is the empty string.
///
/// _Godot equivalent: `is_empty`_
pub fn is_empty(&self) -> bool {
self.as_inner().is_empty()
}

#[doc(hidden)]
pub fn as_inner(&self) -> inner::InnerStringName {
inner::InnerStringName::from_outer(self)
}

ffi_methods! {
type sys::GDExtensionStringNamePtr = *mut Opaque;

Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/variant/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ mod impls {
impl_variant_traits!(Aabb, aabb_to_variant, aabb_from_variant, Aabb);
impl_variant_traits!(bool, bool_to_variant, bool_from_variant, Bool);
impl_variant_traits!(Basis, basis_to_variant, basis_from_variant, Basis);
impl_variant_traits!(Callable, callable_to_variant, callable_from_variant, Callable);
impl_variant_traits!(Vector2, vector2_to_variant, vector2_from_variant, Vector2);
impl_variant_traits!(Vector3, vector3_to_variant, vector3_from_variant, Vector3);
impl_variant_traits!(Vector4, vector4_to_variant, vector4_from_variant, Vector4);
Expand All @@ -157,7 +158,6 @@ mod impls {
impl_variant_traits!(StringName, string_name_to_variant, string_name_from_variant, StringName);
impl_variant_traits!(NodePath, node_path_to_variant, node_path_from_variant, NodePath);
// TODO use impl_variant_traits!, as soon as `Default` is available. Also consider auto-generating.
impl_variant_metadata!(Callable, /* callable_to_variant, callable_from_variant, */ Callable);
impl_variant_metadata!(Signal, /* signal_to_variant, signal_from_variant, */ Signal);
impl_variant_traits!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant, PackedByteArray);
impl_variant_traits!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant, PackedInt32Array);
Expand Down
9 changes: 8 additions & 1 deletion godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use sys::types::OpaqueObject;
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};
use crate::builtin::{
Callable, FromVariant, StringName, ToVariant, Variant, VariantConversionError,
};
use crate::obj::dom::Domain as _;
use crate::obj::mem::Memory as _;
use crate::obj::{cap, dom, mem, Export, GodotClass, Inherits, Share};
Expand Down Expand Up @@ -416,6 +418,11 @@ impl<T: GodotClass> Gd<T> {
T::Mem::maybe_init_ref(&self);
self
}

/// Returns a callable referencing a method from this object named `method_name`.
pub fn callable<S: Into<StringName>>(&self, method_name: S) -> Callable {
Callable::from_object_method(self.share(), method_name)
}
Comment on lines +421 to +425
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the convenience of this method; though over time, as we're adding more and more interactions between Gd and other types, we might want to decide where those should live (and I'd tend to "outside of Gd").

It's fine for now, but this may still change in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's convenient for now but we should have a proper discussion of what the API for using callable should be.

}

/// _The methods in this impl block are only available for objects `T` that are manually managed,
Expand Down
Loading