-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Type-safe signals #1000
base: master
Are you sure you want to change the base?
Type-safe signals #1000
Conversation
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just looking through this a bit and it's really cool from what i can tell! as it's a draft im not really gonna do a proper review but it'll be cool to see the more finished product.
(also happy 1000th issue/pr :p)
pub trait ParamTuple { | ||
fn to_variant_array(&self) -> Vec<Variant>; | ||
fn from_variant_array(array: &[&Variant]) -> Self; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if this is something we could reuse in Ptr/VarcallSignatureTuple
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was wondering the same, the similarities are hard to overlook 🙂 It could even be a foundation for a potential builder API one day, if we allow users to register their own methods....
One thing that would be interesting, but very hard to measure, is how compile-time is impacted by different approaches (e.g. more code in declarative macros vs. procedural macros vs. traits/generics) 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that would be interesting, but very hard to measure, is how compile-time is impacted by different approaches (e.g. more code in declarative macros vs. procedural macros vs. traits/generics) 🤔
I can imagine it would also vary a lot depending on exactly what lies where too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i experimented a bit with this using a trait combination like this:
pub trait Params {
const PARAM_LEN: usize;
}
pub trait InParams: Params + Sized {
unsafe fn varcall_args(
args_ptr: *const sys::GDExtensionConstVariantPtr,
call_ctx: &CallContext,
) -> Result<Self, crate::meta::error::CallError>;
unsafe fn ptrcall_args(
args_ptr: *const sys::GDExtensionConstTypePtr,
call_type: sys::PtrcallType,
call_ctx: &CallContext,
) -> Self;
}
pub trait OutParams: Params + Sized {
fn with_variant_array<F: FnOnce(&[Variant]) -> R, R>(&self, f: F) -> R;
fn with_ptr_array<F: FnOnce(&[sys::GDExtensionConstTypePtr]) -> R, R>(&self, f: F) -> R;
}
and i did manage to write signature impls that compile using it:
`VarcallSignatureTuple`
impl<R: ToGodot + FromGodot + Debug, Ps: InParams + OutParams> VarcallSignatureTuple
for SignatureThing<R, Ps>
{
const PARAM_COUNT: usize = <Ps as Params>::PARAM_LEN;
fn param_property_info(index: usize, param_name: &str) -> PropertyInfo {
todo!()
}
fn param_info(index: usize, param_name: &str) -> Option<MethodParamOrReturnInfo> {
todo!()
}
fn return_info() -> Option<MethodParamOrReturnInfo> {
todo!()
}
unsafe fn in_varcall(
instance_ptr: godot_ffi::GDExtensionClassInstancePtr,
call_ctx: &CallContext,
args_ptr: *const godot_ffi::GDExtensionConstVariantPtr,
arg_count: i64,
ret: godot_ffi::GDExtensionVariantPtr,
err: *mut godot_ffi::GDExtensionCallError,
func: fn(godot_ffi::GDExtensionClassInstancePtr, Self::Params) -> Self::Ret,
) -> Result<(), CallError> {
//$crate::out!("in_varcall: {call_ctx}");
CallError::check_arg_count(
call_ctx,
arg_count as usize,
<Self::Params as Params>::PARAM_LEN,
)?;
#[cfg(feature = "trace")]
trace::push(true, false, &call_ctx);
let args = unsafe { <Self::Params as InParams>::varcall_args(args_ptr, call_ctx)? };
let rust_result = func(instance_ptr, args);
varcall_return::<R>(rust_result, ret, err);
Ok(())
}
unsafe fn out_class_varcall(
method_bind: ClassMethodBind,
// Separate parameters to reduce tokens in generated class API.
class_name: &'static str,
method_name: &'static str,
object_ptr: godot_ffi::GDExtensionObjectPtr,
maybe_instance_id: Option<InstanceId>, // if not static
args: Self::Params,
varargs: &[Variant],
) -> Result<Self::Ret, CallError> {
let call_ctx = CallContext::outbound(class_name, method_name);
//$crate::out!("out_class_varcall: {call_ctx}");
// Note: varcalls are not safe from failing, if they happen through an object pointer -> validity check necessary.
if let Some(instance_id) = maybe_instance_id {
crate::classes::ensure_object_alive(instance_id, object_ptr, &call_ctx);
}
let class_fn = sys::interface_fn!(object_method_bind_call);
let variant: Result<Variant, CallError> = args.with_variant_array(|explicit_args| {
let mut variant_ptrs = Vec::with_capacity(explicit_args.len() + varargs.len());
variant_ptrs.extend(explicit_args.iter().map(Variant::var_sys));
variant_ptrs.extend(varargs.iter().map(Variant::var_sys));
Variant::new_with_var_uninit_result(|return_ptr| {
let mut err = sys::default_call_error();
class_fn(
method_bind.0,
object_ptr,
variant_ptrs.as_ptr(),
variant_ptrs.len() as i64,
return_ptr,
std::ptr::addr_of_mut!(err),
);
sys::CallError::try_from_sys(err).map_err(|err| {
CallError::check_out_varcall(&call_ctx, err, &explicit_args, varargs)
})
})
});
variant.and_then(|v| {
v.try_to::<Self::Ret>()
.map_err(|e| CallError::failed_return_conversion::<Self::Ret>(&call_ctx, e))
})
}
#[cfg(since_api = "4.3")]
unsafe fn out_script_virtual_call(
// Separate parameters to reduce tokens in macro-generated API.
class_name: &'static str,
method_name: &'static str,
method_sname_ptr: godot_ffi::GDExtensionConstStringNamePtr,
object_ptr: godot_ffi::GDExtensionObjectPtr,
args: Self::Params,
) -> Self::Ret {
// Assumes that caller has previously checked existence of a virtual method.
let call_ctx = CallContext::outbound(class_name, method_name);
//$crate::out!("out_script_virtual_call: {call_ctx}");
let object_call_script_method = sys::interface_fn!(object_call_script_method);
let variant = args.with_variant_array(|explicit_args| {
let variant_ptrs = explicit_args
.iter()
.map(Variant::var_sys)
.collect::<Vec<_>>();
Variant::new_with_var_uninit(|return_ptr| {
let mut err = sys::default_call_error();
object_call_script_method(
object_ptr,
method_sname_ptr,
variant_ptrs.as_ptr(),
variant_ptrs.len() as i64,
return_ptr,
std::ptr::addr_of_mut!(err),
);
})
});
let result = <Self::Ret as FromGodot>::try_from_variant(&variant);
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
unsafe fn out_utility_ptrcall_varargs(
utility_fn: UtilityFunctionBind,
function_name: &'static str,
args: Self::Params,
varargs: &[Variant],
) -> Self::Ret {
let call_ctx = CallContext::outbound("", function_name);
//$crate::out!("out_utility_ptrcall_varargs: {call_ctx}");
let result = args.with_ptr_array(|explicit_args| {
let mut type_ptrs = Vec::with_capacity(explicit_args.len() + varargs.len());
type_ptrs.extend(explicit_args);
type_ptrs.extend(varargs.iter().map(sys::GodotFfi::sys));
// Important: this calls from_sys_init_default().
new_from_ptrcall::<Self::Ret>(|return_ptr| {
utility_fn(return_ptr, type_ptrs.as_ptr(), type_ptrs.len() as i32);
})
});
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
unsafe fn out_builtin_ptrcall_varargs(
builtin_fn: BuiltinMethodBind,
class_name: &'static str,
method_name: &'static str,
type_ptr: godot_ffi::GDExtensionTypePtr,
args: Self::Params,
varargs: &[Variant],
) -> Self::Ret {
let call_ctx = CallContext::outbound(class_name, method_name);
//$crate::out!("out_builtin_ptrcall_varargs: {call_ctx}");
let result = args.with_ptr_array(|explicit_args| {
let mut type_ptrs = Vec::with_capacity(explicit_args.len() + varargs.len());
type_ptrs.extend(explicit_args);
type_ptrs.extend(varargs.iter().map(sys::GodotFfi::sys));
// Important: this calls from_sys_init_default().
new_from_ptrcall::<Self::Ret>(|return_ptr| {
builtin_fn(
type_ptr,
type_ptrs.as_ptr(),
return_ptr,
type_ptrs.len() as i32,
);
})
});
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
fn format_args(args: &Self::Params) -> String {
todo!()
}
}
`PtrcallSignatureTuple`
impl<R: ToGodot + FromGodot + Debug, Params: InParams + OutParams> PtrcallSignatureTuple
for SignatureThing<R, Params>
{
type Params = Params;
type Ret = R;
unsafe fn in_ptrcall(
instance_ptr: godot_ffi::GDExtensionClassInstancePtr,
call_ctx: &CallContext<'static>,
args_ptr: *const godot_ffi::GDExtensionConstTypePtr,
ret: godot_ffi::GDExtensionTypePtr,
func: fn(godot_ffi::GDExtensionClassInstancePtr, Self::Params) -> Self::Ret,
call_type: godot_ffi::PtrcallType,
) {
// $crate::out!("in_ptrcall: {call_ctx}");
#[cfg(feature = "trace")]
trace::push(true, true, &call_ctx);
let args = unsafe { InParams::ptrcall_args(args_ptr, call_type, call_ctx) };
// SAFETY:
// `ret` is always a pointer to an initialized value of type $R
// TODO: double-check the above
ptrcall_return(func(instance_ptr, args), ret, call_ctx, call_type)
}
unsafe fn out_class_ptrcall(
method_bind: ClassMethodBind,
// Separate parameters to reduce tokens in generated class API.
class_name: &'static str,
method_name: &'static str,
object_ptr: godot_ffi::GDExtensionObjectPtr,
maybe_instance_id: Option<InstanceId>, // if not static
args: Self::Params,
) -> Self::Ret {
let call_ctx = CallContext::outbound(class_name, method_name);
// $crate::out!("out_class_ptrcall: {call_ctx}");
if let Some(instance_id) = maybe_instance_id {
crate::classes::ensure_object_alive(instance_id, object_ptr, &call_ctx);
}
let class_fn = sys::interface_fn!(object_method_bind_ptrcall);
let result = args.with_ptr_array(|type_ptrs| {
new_from_ptrcall::<Self::Ret>(|return_ptr| {
class_fn(method_bind.0, object_ptr, type_ptrs.as_ptr(), return_ptr);
})
});
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
unsafe fn out_builtin_ptrcall(
builtin_fn: BuiltinMethodBind,
// Separate parameters to reduce tokens in generated class API.
class_name: &'static str,
method_name: &'static str,
type_ptr: godot_ffi::GDExtensionTypePtr,
args: Self::Params,
) -> Self::Ret {
let call_ctx = CallContext::outbound(class_name, method_name);
// $crate::out!("out_builtin_ptrcall: {call_ctx}");
let result = args.with_ptr_array(|type_ptrs| {
new_from_ptrcall::<Self::Ret>(|return_ptr| {
builtin_fn(
type_ptr,
type_ptrs.as_ptr(),
return_ptr,
type_ptrs.len() as i32,
);
})
});
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
unsafe fn out_utility_ptrcall(
utility_fn: UtilityFunctionBind,
function_name: &'static str,
args: Self::Params,
) -> Self::Ret {
let call_ctx = CallContext::outbound("", function_name);
// $crate::out!("out_utility_ptrcall: {call_ctx}");
let result = args.with_ptr_array(|type_ptrs| {
new_from_ptrcall::<Self::Ret>(|return_ptr| {
utility_fn(return_ptr, type_ptrs.as_ptr(), type_ptrs.len() as i32);
})
});
result.unwrap_or_else(|err| return_error::<Self::Ret>(&call_ctx, err))
}
}
223a47c
to
686f08d
Compare
73f520f
to
ba17d4a
Compare
|
||
/// Extracted syntax info for a declared signal. | ||
struct SignalDetails<'a> { | ||
/// `fn my_signal(i: i32, s: GString)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change these from docstrings ///
to normal comments //
- IDE highlights might be a bit too confusing in isolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how you mean this -- these types are only used during codegen of the macro and won't be visible to the user.
Using ///
has the advantage that hovering over the fields (elsewhere) shows some example formats, plus there's syntax highlighting for the `...`
backticked parts 🙂
Seeing this MR gave me a smile for the whole day. Thanks. :) |
Adds three APIs to classes that contain at least one #[signal] decl: - self.signals() - self.funcs() - Self::static_funcs() The idea is to use generated symbols for type-safe connecting (signal -> func) as well as emitting. This is still WIP.
ba17d4a
to
5777979
Compare
Closes #8.
Closes #620.
Early draft for a type-safe signal system. Largely backwards compatible with the current syntax.
The following...
...expands to code that can then be used like this (inside
impl MyClass
):Note that this is explicitly not limited to functions declared via
#[func]
. I had an earlier draft with a generatedfuncs()
API which would route the calls via Godot reflection, but this seems more flexible, since you can keep signal recipients private in Rust. On the downside, it may be harder to disconnect such signals.Compatibility
#[signal]
now requires aBase<T>
field.As far as I can see, that's the only breakage. It might also be possible to opt in to the new type-safe signals via something like
#[godot_api(signals)]
or so.This feature is only available on Godot 4.2+, since it requires custom callables.
Work to do
There's a ton that can still be added, in either this or future PRs:
sig.connect_obj(obj, Class::method)
impl
)gd.signals().my_signal()
#[signal]
pub
etc).AsArg
inemit()
&self
methods