From af865e76a323fc87dd16157e7f4d7593d02c2b95 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Tue, 16 Jul 2024 06:01:52 -0700 Subject: [PATCH] bevy_reflect: Improve `DynamicFunction` ergonomics (#14201) # Objective Many functions can be converted to `DynamicFunction` using `IntoFunction`. Unfortunately, we are limited by Rust itself and the implementations are far from exhaustive. For example, we can't convert functions with more than 16 arguments. Additionally, we can't handle returns with lifetimes not tied to the lifetime of the first argument. In such cases, users will have to create their `DynamicFunction` manually. Let's take the following function: ```rust fn get(index: usize, list: &Vec) -> &String { &list[index] } ``` This function cannot be converted to a `DynamicFunction` via `IntoFunction` due to the lifetime of the return value being tied to the second argument. Therefore, we need to construct the `DynamicFunction` manually: ```rust DynamicFunction::new( |mut args, info| { let list = args .pop() .unwrap() .take_ref::>(&info.args()[1])?; let index = args.pop().unwrap().take_owned::(&info.args()[0])?; Ok(Return::Ref(get(index, list))) }, FunctionInfo::new() .with_name("get") .with_args(vec![ ArgInfo::new::(0).with_name("index"), ArgInfo::new::<&Vec>(1).with_name("list"), ]) .with_return_info(ReturnInfo::new::<&String>()), ); ``` While still a small and straightforward snippet, there's a decent amount going on here. There's a lot of room for improvements when it comes to ergonomics and readability. The goal of this PR is to address those issues. ## Solution Improve the ergonomics and readability of manually created `DynamicFunction`s. Some of the major changes: 1. Removed the need for `&ArgInfo` when reifying arguments (i.e. the `&info.args()[1]` calls) 2. Added additional `pop` methods on `ArgList` to handle both popping and casting 3. Added `take` methods on `ArgList` for taking the arguments out in order 4. Removed the need for `&FunctionInfo` in the internal closure (Change 1 made it no longer necessary) 5. Added methods to automatically handle generating `ArgInfo` and `ReturnInfo` With all these changes in place, we get something a lot nicer to both write and look at: ```rust DynamicFunction::new( |mut args| { let index = args.take::()?; let list = args.take::<&Vec>()?; Ok(Return::Ref(get(index, list))) }, FunctionInfo::new() .with_name("get") .with_arg::("index") .with_arg::<&Vec>("list") .with_return::<&String>(), ); ``` Alternatively, to rely on type inference for taking arguments, you could do: ```rust DynamicFunction::new( |mut args| { let index = args.take_owned()?; let list = args.take_ref()?; Ok(Return::Ref(get(index, list))) }, FunctionInfo::new() .with_name("get") .with_arg::("index") .with_arg::<&Vec>("list") .with_return::<&String>(), ); ``` ## Testing You can test locally by running: ``` cargo test --package bevy_reflect ``` --- ## Changelog - Removed `&ArgInfo` argument from `FromArg::from_arg` trait method - Removed `&ArgInfo` argument from `Arg::take_***` methods - Added `ArgValue` - `Arg` is now a struct containing an `ArgValue` and an argument `index` - `Arg::take_***` methods now require `T` is also `TypePath` - Added `Arg::new`, `Arg::index`, `Arg::value`, `Arg::take_value`, and `Arg::take` methods - Replaced `ArgId` in `ArgError` with just the argument `index` - Added `ArgError::EmptyArgList` - Renamed `ArgList::push` to `ArgList::push_arg` - Added `ArgList::pop_arg`, `ArgList::pop_owned`, `ArgList::pop_ref`, and `ArgList::pop_mut` - Added `ArgList::take_arg`, `ArgList::take_owned`, `ArgList::take_ref`, `ArgList::take_mut`, and `ArgList::take` - `ArgList::pop` is now generic - Renamed `FunctionError::InvalidArgCount` to `FunctionError::ArgCountMismatch` - The closure given to `DynamicFunction::new` no longer has a `&FunctionInfo` argument - Added `FunctionInfo::with_arg` - Added `FunctionInfo::with_return` ## Internal Migration Guide > [!important] > Function reflection was introduced as part of the 0.15 dev cycle. This migration guide was written for developers relying on `main` during this cycle, and is not a breaking change coming from 0.14. * The `FromArg::from_arg` trait method and the `Arg::take_***` methods no longer take a `&ArgInfo` argument. * What used to be `Arg` is now `ArgValue`. `Arg` is now a struct which contains an `ArgValue`. * `Arg::take_***` methods now require `T` is also `TypePath` * Instances of `id: ArgId` in `ArgError` have been replaced with `index: usize` * `ArgList::push` is now `ArgList::push_arg`. It also takes the new `ArgValue` type. * `ArgList::pop` has become `ArgList::pop_arg` and now returns `ArgValue`. `Arg::pop` now takes a generic type and downcasts to that type. It's recommended to use `ArgList::take` and friends instead since they allow removing the arguments from the list in the order they were pushed (rather than reverse order). * `FunctionError::InvalidArgCount` is now `FunctionError::ArgCountMismatch` * The closure given to `DynamicFunction::new` no longer has a `&FunctionInfo` argument. This argument can be removed. --- .../derive/src/impls/func/from_arg.rs | 27 +- crates/bevy_reflect/src/func/args/arg.rs | 201 +++++++-- crates/bevy_reflect/src/func/args/error.rs | 17 +- crates/bevy_reflect/src/func/args/from_arg.rs | 44 +- crates/bevy_reflect/src/func/args/info.rs | 4 +- crates/bevy_reflect/src/func/args/list.rs | 381 ++++++++++++++++-- .../bevy_reflect/src/func/args/ownership.rs | 6 + .../src/func/closures/dynamic_closure.rs | 12 +- .../src/func/closures/dynamic_closure_mut.rs | 14 +- .../src/func/closures/into_closure.rs | 5 +- .../src/func/closures/into_closure_mut.rs | 2 +- crates/bevy_reflect/src/func/error.rs | 2 +- crates/bevy_reflect/src/func/function.rs | 61 ++- crates/bevy_reflect/src/func/info.rs | 37 +- crates/bevy_reflect/src/func/into_function.rs | 5 +- crates/bevy_reflect/src/func/mod.rs | 12 +- crates/bevy_reflect/src/func/reflect_fn.rs | 99 ++--- .../bevy_reflect/src/func/reflect_fn_mut.rs | 103 ++--- crates/bevy_reflect/src/func/return_type.rs | 4 + examples/reflection/function_reflection.rs | 49 +-- 20 files changed, 766 insertions(+), 319 deletions(-) diff --git a/crates/bevy_reflect/derive/src/impls/func/from_arg.rs b/crates/bevy_reflect/derive/src/impls/func/from_arg.rs index a52f3e2fb4..9757dd415c 100644 --- a/crates/bevy_reflect/derive/src/impls/func/from_arg.rs +++ b/crates/bevy_reflect/derive/src/impls/func/from_arg.rs @@ -15,32 +15,23 @@ pub(crate) fn impl_from_arg( quote! { impl #impl_generics #bevy_reflect::func::args::FromArg for #type_path #ty_generics #where_reflect_clause { - type Item<'from_arg> = #type_path #ty_generics; - fn from_arg<'from_arg>( - arg: #bevy_reflect::func::args::Arg<'from_arg>, - info: &#bevy_reflect::func::args::ArgInfo, - ) -> #FQResult, #bevy_reflect::func::args::ArgError> { - arg.take_owned(info) + type This<'from_arg> = #type_path #ty_generics; + fn from_arg(arg: #bevy_reflect::func::args::Arg) -> #FQResult, #bevy_reflect::func::args::ArgError> { + arg.take_owned() } } impl #impl_generics #bevy_reflect::func::args::FromArg for &'static #type_path #ty_generics #where_reflect_clause { - type Item<'from_arg> = &'from_arg #type_path #ty_generics; - fn from_arg<'from_arg>( - arg: #bevy_reflect::func::args::Arg<'from_arg>, - info: &#bevy_reflect::func::args::ArgInfo, - ) -> #FQResult, #bevy_reflect::func::args::ArgError> { - arg.take_ref(info) + type This<'from_arg> = &'from_arg #type_path #ty_generics; + fn from_arg(arg: #bevy_reflect::func::args::Arg) -> #FQResult, #bevy_reflect::func::args::ArgError> { + arg.take_ref() } } impl #impl_generics #bevy_reflect::func::args::FromArg for &'static mut #type_path #ty_generics #where_reflect_clause { - type Item<'from_arg> = &'from_arg mut #type_path #ty_generics; - fn from_arg<'from_arg>( - arg: #bevy_reflect::func::args::Arg<'from_arg>, - info: &#bevy_reflect::func::args::ArgInfo, - ) -> #FQResult, #bevy_reflect::func::args::ArgError> { - arg.take_mut(info) + type This<'from_arg> = &'from_arg mut #type_path #ty_generics; + fn from_arg(arg: #bevy_reflect::func::args::Arg) -> #FQResult, #bevy_reflect::func::args::ArgError> { + arg.take_mut() } } } diff --git a/crates/bevy_reflect/src/func/args/arg.rs b/crates/bevy_reflect/src/func/args/arg.rs index bc523af285..e89ce72598 100644 --- a/crates/bevy_reflect/src/func/args/arg.rs +++ b/crates/bevy_reflect/src/func/args/arg.rs @@ -1,81 +1,200 @@ -use crate::func::args::{ArgError, ArgInfo, Ownership}; -use crate::Reflect; +use crate::func::args::{ArgError, FromArg, Ownership}; +use crate::{Reflect, TypePath}; +use std::ops::Deref; -/// Represents an argument that can be passed to a [`DynamicFunction`] or [`DynamicClosure`]. +/// Represents an argument that can be passed to a [`DynamicFunction`], [`DynamicClosure`], +/// or [`DynamicClosureMut`]. /// /// [`DynamicFunction`]: crate::func::DynamicFunction /// [`DynamicClosure`]: crate::func::DynamicClosure +/// [`DynamicClosureMut`]: crate::func::DynamicClosureMut #[derive(Debug)] -pub enum Arg<'a> { - Owned(Box), - Ref(&'a dyn Reflect), - Mut(&'a mut dyn Reflect), +pub struct Arg<'a> { + index: usize, + value: ArgValue<'a>, } impl<'a> Arg<'a> { - /// Returns `Ok(T)` if the argument is [`Arg::Owned`]. - pub fn take_owned(self, info: &ArgInfo) -> Result { - match self { - Arg::Owned(arg) => arg.take().map_err(|arg| ArgError::UnexpectedType { - id: info.id().clone(), - expected: ::std::borrow::Cow::Borrowed(info.type_path()), - received: ::std::borrow::Cow::Owned(arg.reflect_type_path().to_string()), + /// Create a new [`Arg`] with the given index and value. + pub fn new(index: usize, value: ArgValue<'a>) -> Self { + Self { index, value } + } + + /// The index of the argument. + pub fn index(&self) -> usize { + self.index + } + + /// Set the index of the argument. + pub(crate) fn set_index(&mut self, index: usize) { + self.index = index; + } + + /// The value of the argument. + pub fn value(&self) -> &ArgValue<'a> { + &self.value + } + + /// Take the value of the argument. + pub fn take_value(self) -> ArgValue<'a> { + self.value + } + + /// Take the value of the argument and attempt to convert it to a concrete value, `T`. + /// + /// This is a convenience method for calling [`FromArg::from_arg`] on the argument. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let a = 1u32; + /// let b = 2u32; + /// let mut c = 3u32; + /// let mut args = ArgList::new().push_owned(a).push_ref(&b).push_mut(&mut c); + /// + /// let a = args.take::().unwrap(); + /// assert_eq!(a, 1); + /// + /// let b = args.take::<&u32>().unwrap(); + /// assert_eq!(*b, 2); + /// + /// let c = args.take::<&mut u32>().unwrap(); + /// assert_eq!(*c, 3); + /// ``` + pub fn take(self) -> Result, ArgError> { + T::from_arg(self) + } + + /// Returns `Ok(T)` if the argument is [`ArgValue::Owned`]. + /// + /// If the argument is not owned, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_owned(value); + /// let value = args.take_owned::().unwrap(); + /// assert_eq!(value, 123); + /// ``` + pub fn take_owned(self) -> Result { + match self.value { + ArgValue::Owned(arg) => arg.take().map_err(|arg| ArgError::UnexpectedType { + index: self.index, + expected: std::borrow::Cow::Borrowed(T::type_path()), + received: std::borrow::Cow::Owned(arg.reflect_type_path().to_string()), }), - Arg::Ref(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + ArgValue::Ref(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Owned, received: Ownership::Ref, }), - Arg::Mut(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + ArgValue::Mut(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Owned, received: Ownership::Mut, }), } } - /// Returns `Ok(&T)` if the argument is [`Arg::Ref`]. - pub fn take_ref(self, info: &ArgInfo) -> Result<&'a T, ArgError> { - match self { - Arg::Owned(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + /// Returns `Ok(&T)` if the argument is [`ArgValue::Ref`]. + /// + /// If the argument is not a reference, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_ref(&value); + /// let value = args.take_ref::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn take_ref(self) -> Result<&'a T, ArgError> { + match self.value { + ArgValue::Owned(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Ref, received: Ownership::Owned, }), - Arg::Ref(arg) => Ok(arg.downcast_ref().ok_or_else(|| ArgError::UnexpectedType { - id: info.id().clone(), - expected: ::std::borrow::Cow::Borrowed(info.type_path()), - received: ::std::borrow::Cow::Owned(arg.reflect_type_path().to_string()), - })?), - Arg::Mut(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + ArgValue::Ref(arg) => { + Ok(arg.downcast_ref().ok_or_else(|| ArgError::UnexpectedType { + index: self.index, + expected: std::borrow::Cow::Borrowed(T::type_path()), + received: std::borrow::Cow::Owned(arg.reflect_type_path().to_string()), + })?) + } + ArgValue::Mut(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Ref, received: Ownership::Mut, }), } } - /// Returns `Ok(&mut T)` if the argument is [`Arg::Mut`]. - pub fn take_mut(self, info: &ArgInfo) -> Result<&'a mut T, ArgError> { - match self { - Arg::Owned(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + /// Returns `Ok(&mut T)` if the argument is [`ArgValue::Mut`]. + /// + /// If the argument is not a mutable reference, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let mut value = 123u32; + /// let mut args = ArgList::new().push_mut(&mut value); + /// let value = args.take_mut::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn take_mut(self) -> Result<&'a mut T, ArgError> { + match self.value { + ArgValue::Owned(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Mut, received: Ownership::Owned, }), - Arg::Ref(_) => Err(ArgError::InvalidOwnership { - id: info.id().clone(), + ArgValue::Ref(_) => Err(ArgError::InvalidOwnership { + index: self.index, expected: Ownership::Mut, received: Ownership::Ref, }), - Arg::Mut(arg) => { - let received = ::std::borrow::Cow::Owned(arg.reflect_type_path().to_string()); + ArgValue::Mut(arg) => { + let received = std::borrow::Cow::Owned(arg.reflect_type_path().to_string()); Ok(arg.downcast_mut().ok_or_else(|| ArgError::UnexpectedType { - id: info.id().clone(), - expected: ::std::borrow::Cow::Borrowed(info.type_path()), + index: self.index, + expected: std::borrow::Cow::Borrowed(T::type_path()), received, })?) } } } } + +/// Represents an argument that can be passed to a [`DynamicFunction`]. +/// +/// [`DynamicFunction`]: crate::func::DynamicFunction +#[derive(Debug)] +pub enum ArgValue<'a> { + Owned(Box), + Ref(&'a dyn Reflect), + Mut(&'a mut dyn Reflect), +} + +impl<'a> Deref for ArgValue<'a> { + type Target = dyn Reflect; + + fn deref(&self) -> &Self::Target { + match self { + ArgValue::Owned(arg) => arg.as_ref(), + ArgValue::Ref(arg) => *arg, + ArgValue::Mut(arg) => *arg, + } + } +} diff --git a/crates/bevy_reflect/src/func/args/error.rs b/crates/bevy_reflect/src/func/args/error.rs index 83794b81d6..13ebc0a7d3 100644 --- a/crates/bevy_reflect/src/func/args/error.rs +++ b/crates/bevy_reflect/src/func/args/error.rs @@ -2,25 +2,30 @@ use alloc::borrow::Cow; use thiserror::Error; -use crate::func::args::{ArgId, Ownership}; +use crate::func::args::Ownership; /// An error that occurs when converting an [argument]. /// -/// [argument]: crate::func::Arg +/// [argument]: crate::func::args::Arg #[derive(Debug, Error, PartialEq)] pub enum ArgError { /// The argument is not the expected type. - #[error("expected `{expected}` but received `{received}` (@ {id:?})")] + #[error("expected `{expected}` but received `{received}` (@ argument index {index})")] UnexpectedType { - id: ArgId, + index: usize, expected: Cow<'static, str>, received: Cow<'static, str>, }, /// The argument has the wrong ownership. - #[error("expected {expected} value but received {received} value (@ {id:?})")] + #[error("expected {expected} value but received {received} value (@ argument index {index})")] InvalidOwnership { - id: ArgId, + index: usize, expected: Ownership, received: Ownership, }, + /// Occurs when attempting to access an argument from an empty [`ArgList`]. + /// + /// [`ArgList`]: crate::func::args::ArgList + #[error("expected an argument but received none")] + EmptyArgList, } diff --git a/crates/bevy_reflect/src/func/args/from_arg.rs b/crates/bevy_reflect/src/func/args/from_arg.rs index 7ae61e6856..9478650165 100644 --- a/crates/bevy_reflect/src/func/args/from_arg.rs +++ b/crates/bevy_reflect/src/func/args/from_arg.rs @@ -1,23 +1,32 @@ -use crate::func::args::{Arg, ArgError, ArgInfo}; +use crate::func::args::{Arg, ArgError}; /// A trait for types that can be created from an [`Arg`]. /// +/// This trait exists so that types can be automatically converted into an [`Arg`] +/// by [`IntoFunction`] so they can be passed to a [`DynamicFunction`] in an [`ArgList`]. +/// /// This trait is used instead of a blanket [`From`] implementation due to coherence issues: /// we can't implement `From` for both `T` and `&T`/`&mut T`. /// /// This trait is automatically implemented when using the `Reflect` [derive macro]. /// +/// [`IntoFunction`]: crate::func::IntoFunction +/// [`DynamicFunction`]: crate::func::DynamicFunction +/// [`ArgList`]: crate::func::args::ArgList /// [derive macro]: derive@crate::Reflect pub trait FromArg { - /// The type of the item created from the argument. + /// The type to convert into. /// /// This should almost always be the same as `Self`, but with the lifetime `'a`. - type Item<'a>; + /// + /// The reason we use a separate associated type is to allow for the lifetime + /// to be tied to the argument, rather than the type itself. + type This<'a>; /// Creates an item from an argument. /// /// The argument must be of the expected type and ownership. - fn from_arg<'a>(arg: Arg<'a>, info: &ArgInfo) -> Result, ArgError>; + fn from_arg(arg: Arg) -> Result, ArgError>; } /// Implements the [`FromArg`] trait for the given type. @@ -54,12 +63,9 @@ macro_rules! impl_from_arg { $($U $(: $U1 $(+ $U2)*)?),* )? { - type Item<'from_arg> = $ty; - fn from_arg<'from_arg>( - arg: $crate::func::args::Arg<'from_arg>, - info: &$crate::func::args::ArgInfo, - ) -> Result, $crate::func::args::ArgError> { - arg.take_owned(info) + type This<'from_arg> = $ty; + fn from_arg(arg: $crate::func::args::Arg) -> Result, $crate::func::args::ArgError> { + arg.take_owned() } } @@ -72,12 +78,9 @@ macro_rules! impl_from_arg { $($U $(: $U1 $(+ $U2)*)?),* )? { - type Item<'from_arg> = &'from_arg $ty; - fn from_arg<'from_arg>( - arg: $crate::func::args::Arg<'from_arg>, - info: &$crate::func::args::ArgInfo, - ) -> Result, $crate::func::args::ArgError> { - arg.take_ref(info) + type This<'from_arg> = &'from_arg $ty; + fn from_arg(arg: $crate::func::args::Arg) -> Result, $crate::func::args::ArgError> { + arg.take_ref() } } @@ -90,12 +93,9 @@ macro_rules! impl_from_arg { $($U $(: $U1 $(+ $U2)*)?),* )? { - type Item<'from_arg> = &'from_arg mut $ty; - fn from_arg<'from_arg>( - arg: $crate::func::args::Arg<'from_arg>, - info: &$crate::func::args::ArgInfo, - ) -> Result, $crate::func::args::ArgError> { - arg.take_mut(info) + type This<'from_arg> = &'from_arg mut $ty; + fn from_arg(arg: $crate::func::args::Arg) -> Result, $crate::func::args::ArgError> { + arg.take_mut() } } }; diff --git a/crates/bevy_reflect/src/func/args/info.rs b/crates/bevy_reflect/src/func/args/info.rs index 34f10cda53..f55643dcc7 100644 --- a/crates/bevy_reflect/src/func/args/info.rs +++ b/crates/bevy_reflect/src/func/args/info.rs @@ -3,11 +3,13 @@ use alloc::borrow::Cow; use crate::func::args::{GetOwnership, Ownership}; use crate::TypePath; -/// Type information for an [`Arg`] used in a [`DynamicFunction`] or [`DynamicClosure`]. +/// Type information for an [`Arg`] used in a [`DynamicFunction`], [`DynamicClosure`], +/// or [`DynamicClosureMut`]. /// /// [`Arg`]: crate::func::args::Arg /// [`DynamicFunction`]: crate::func::function::DynamicFunction /// [`DynamicClosure`]: crate::func::closures::DynamicClosure +/// [`DynamicClosureMut`]: crate::func::closures::DynamicClosureMut #[derive(Debug, Clone)] pub struct ArgInfo { /// The index of the argument within its function. diff --git a/crates/bevy_reflect/src/func/args/list.rs b/crates/bevy_reflect/src/func/args/list.rs index 319d6f8853..67703cebc0 100644 --- a/crates/bevy_reflect/src/func/args/list.rs +++ b/crates/bevy_reflect/src/func/args/list.rs @@ -1,12 +1,15 @@ -use crate::func::args::Arg; -use crate::Reflect; +use crate::func::args::{Arg, ArgValue, FromArg}; +use crate::func::ArgError; +use crate::{Reflect, TypePath}; +use std::collections::VecDeque; -/// A list of arguments that can be passed to a [`DynamicFunction`] or [`DynamicClosure`]. +/// A list of arguments that can be passed to a [`DynamicFunction`], [`DynamicClosure`], +/// or [`DynamicClosureMut`]. /// /// # Example /// /// ``` -/// # use bevy_reflect::func::{Arg, ArgList}; +/// # use bevy_reflect::func::{ArgValue, ArgList}; /// let foo = 123; /// let bar = 456; /// let mut baz = 789; @@ -20,63 +23,385 @@ use crate::Reflect; /// // Push a mutable reference argument /// .push_mut(&mut baz) /// // Push a manually constructed argument -/// .push(Arg::Ref(&3.14)); +/// .push_arg(ArgValue::Ref(&3.14)); /// ``` /// +/// [arguments]: Arg /// [`DynamicFunction`]: crate::func::DynamicFunction /// [`DynamicClosure`]: crate::func::DynamicClosure +/// [`DynamicClosureMut`]: crate::func::DynamicClosureMut #[derive(Default, Debug)] -pub struct ArgList<'a>(Vec>); +pub struct ArgList<'a> { + list: VecDeque>, + /// A flag that indicates if the list needs to be re-indexed. + /// + /// This flag should be set when an argument is removed from the beginning of the list, + /// so that any future push operations will re-index the arguments. + needs_reindex: bool, +} impl<'a> ArgList<'a> { /// Create a new empty list of arguments. pub fn new() -> Self { - Self(Vec::new()) + Self { + list: VecDeque::new(), + needs_reindex: false, + } } - /// Push an [`Arg`] onto the list. - pub fn push(mut self, arg: Arg<'a>) -> Self { - self.0.push(arg); + /// Push an [`ArgValue`] onto the list. + /// + /// If an argument was previously removed from the beginning of the list, + /// this method will also re-index the list. + pub fn push_arg(mut self, arg: ArgValue<'a>) -> Self { + if self.needs_reindex { + for (index, arg) in self.list.iter_mut().enumerate() { + arg.set_index(index); + } + self.needs_reindex = false; + } + + let index = self.list.len(); + self.list.push_back(Arg::new(index, arg)); self } - /// Push an [`Arg::Ref`] onto the list with the given reference. + /// Push an [`ArgValue::Ref`] onto the list with the given reference. + /// + /// If an argument was previously removed from the beginning of the list, + /// this method will also re-index the list. pub fn push_ref(self, arg: &'a dyn Reflect) -> Self { - self.push(Arg::Ref(arg)) + self.push_arg(ArgValue::Ref(arg)) } - /// Push an [`Arg::Mut`] onto the list with the given mutable reference. + /// Push an [`ArgValue::Mut`] onto the list with the given mutable reference. + /// + /// If an argument was previously removed from the beginning of the list, + /// this method will also re-index the list. pub fn push_mut(self, arg: &'a mut dyn Reflect) -> Self { - self.push(Arg::Mut(arg)) + self.push_arg(ArgValue::Mut(arg)) } - /// Push an [`Arg::Owned`] onto the list with the given owned value. + /// Push an [`ArgValue::Owned`] onto the list with the given owned value. + /// + /// If an argument was previously removed from the beginning of the list, + /// this method will also re-index the list. pub fn push_owned(self, arg: impl Reflect) -> Self { - self.push(Arg::Owned(Box::new(arg))) + self.push_arg(ArgValue::Owned(Box::new(arg))) } - /// Push an [`Arg::Owned`] onto the list with the given boxed value. + /// Push an [`ArgValue::Owned`] onto the list with the given boxed value. + /// + /// If an argument was previously removed from the beginning of the list, + /// this method will also re-index the list. pub fn push_boxed(self, arg: Box) -> Self { - self.push(Arg::Owned(arg)) + self.push_arg(ArgValue::Owned(arg)) } - /// Pop the last argument from the list, if there is one. - pub fn pop(&mut self) -> Option> { - self.0.pop() + /// Remove the first argument in the list and return it. + /// + /// It's generally preferred to use [`Self::take`] instead of this method + /// as it provides a more ergonomic way to immediately downcast the argument. + pub fn take_arg(&mut self) -> Result, ArgError> { + self.needs_reindex = true; + self.list.pop_front().ok_or(ArgError::EmptyArgList) + } + + /// Remove the first argument in the list and return `Ok(T::This)`. + /// + /// If the list is empty or the [`FromArg::from_arg`] call fails, returns an error. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let a = 1u32; + /// let b = 2u32; + /// let mut c = 3u32; + /// let mut args = ArgList::new().push_owned(a).push_ref(&b).push_mut(&mut c); + /// + /// let a = args.take::().unwrap(); + /// assert_eq!(a, 1); + /// + /// let b = args.take::<&u32>().unwrap(); + /// assert_eq!(*b, 2); + /// + /// let c = args.take::<&mut u32>().unwrap(); + /// assert_eq!(*c, 3); + /// ``` + pub fn take(&mut self) -> Result, ArgError> { + self.take_arg()?.take::() + } + + /// Remove the first argument in the list and return `Ok(T)` if the argument is [`ArgValue::Owned`]. + /// + /// If the list is empty or the argument is not owned, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_owned(value); + /// let value = args.take_owned::().unwrap(); + /// assert_eq!(value, 123); + /// ``` + pub fn take_owned(&mut self) -> Result { + self.take_arg()?.take_owned() + } + + /// Remove the first argument in the list and return `Ok(&T)` if the argument is [`ArgValue::Ref`]. + /// + /// If the list is empty or the argument is not a reference, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_ref(&value); + /// let value = args.take_ref::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn take_ref(&mut self) -> Result<&'a T, ArgError> { + self.take_arg()?.take_ref() + } + + /// Remove the first argument in the list and return `Ok(&mut T)` if the argument is [`ArgValue::Mut`]. + /// + /// If the list is empty or the argument is not a mutable reference, returns an error. + /// + /// It's generally preferred to use [`Self::take`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let mut value = 123u32; + /// let mut args = ArgList::new().push_mut(&mut value); + /// let value = args.take_mut::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn take_mut(&mut self) -> Result<&'a mut T, ArgError> { + self.take_arg()?.take_mut() + } + + /// Remove the last argument in the list and return it. + /// + /// It's generally preferred to use [`Self::pop`] instead of this method + /// as it provides a more ergonomic way to immediately downcast the argument. + pub fn pop_arg(&mut self) -> Result, ArgError> { + self.list.pop_back().ok_or(ArgError::EmptyArgList) + } + + /// Remove the last argument in the list and return `Ok(T::This)`. + /// + /// If the list is empty or the [`FromArg::from_arg`] call fails, returns an error. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let a = 1u32; + /// let b = 2u32; + /// let mut c = 3u32; + /// let mut args = ArgList::new().push_owned(a).push_ref(&b).push_mut(&mut c); + /// + /// let c = args.pop::<&mut u32>().unwrap(); + /// assert_eq!(*c, 3); + /// + /// let b = args.pop::<&u32>().unwrap(); + /// assert_eq!(*b, 2); + /// + /// let a = args.pop::().unwrap(); + /// assert_eq!(a, 1); + /// ``` + pub fn pop(&mut self) -> Result, ArgError> { + self.pop_arg()?.take::() + } + + /// Remove the last argument in the list and return `Ok(T)` if the argument is [`ArgValue::Owned`]. + /// + /// If the list is empty or the argument is not owned, returns an error. + /// + /// It's generally preferred to use [`Self::pop`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_owned(value); + /// let value = args.pop_owned::().unwrap(); + /// assert_eq!(value, 123); + /// ``` + pub fn pop_owned(&mut self) -> Result { + self.pop_arg()?.take_owned() + } + + /// Remove the last argument in the list and return `Ok(&T)` if the argument is [`ArgValue::Ref`]. + /// + /// If the list is empty or the argument is not a reference, returns an error. + /// + /// It's generally preferred to use [`Self::pop`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let value = 123u32; + /// let mut args = ArgList::new().push_ref(&value); + /// let value = args.pop_ref::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn pop_ref(&mut self) -> Result<&'a T, ArgError> { + self.pop_arg()?.take_ref() + } + + /// Remove the last argument in the list and return `Ok(&mut T)` if the argument is [`ArgValue::Mut`]. + /// + /// If the list is empty or the argument is not a mutable reference, returns an error. + /// + /// It's generally preferred to use [`Self::pop`] instead of this method. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::func::ArgList; + /// let mut value = 123u32; + /// let mut args = ArgList::new().push_mut(&mut value); + /// let value = args.pop_mut::().unwrap(); + /// assert_eq!(*value, 123); + /// ``` + pub fn pop_mut(&mut self) -> Result<&'a mut T, ArgError> { + self.pop_arg()?.take_mut() } /// Returns the number of arguments in the list. pub fn len(&self) -> usize { - self.0.len() + self.list.len() } /// Returns `true` if the list of arguments is empty. pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - /// Take ownership of the list of arguments. - pub fn take(self) -> Vec> { - self.0 + self.list.is_empty() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn should_push_arguments_in_order() { + let args = ArgList::new() + .push_owned(123) + .push_owned(456) + .push_owned(789); + + assert_eq!(args.len(), 3); + assert_eq!(args.list[0].index(), 0); + assert_eq!(args.list[1].index(), 1); + assert_eq!(args.list[2].index(), 2); + } + + #[test] + fn should_push_arg_with_correct_ownership() { + let a = String::from("a"); + let b = String::from("b"); + let mut c = String::from("c"); + let d = String::from("d"); + let e = String::from("e"); + let f = String::from("f"); + let mut g = String::from("g"); + + let args = ArgList::new() + .push_arg(ArgValue::Owned(Box::new(a))) + .push_arg(ArgValue::Ref(&b)) + .push_arg(ArgValue::Mut(&mut c)) + .push_owned(d) + .push_boxed(Box::new(e)) + .push_ref(&f) + .push_mut(&mut g); + + assert!(matches!(args.list[0].value(), &ArgValue::Owned(_))); + assert!(matches!(args.list[1].value(), &ArgValue::Ref(_))); + assert!(matches!(args.list[2].value(), &ArgValue::Mut(_))); + assert!(matches!(args.list[3].value(), &ArgValue::Owned(_))); + assert!(matches!(args.list[4].value(), &ArgValue::Owned(_))); + assert!(matches!(args.list[5].value(), &ArgValue::Ref(_))); + assert!(matches!(args.list[6].value(), &ArgValue::Mut(_))); + } + + #[test] + fn should_take_args_in_order() { + let a = String::from("a"); + let b = 123_i32; + let c = 456_usize; + let mut d = 5.78_f32; + + let mut args = ArgList::new() + .push_owned(a) + .push_ref(&b) + .push_ref(&c) + .push_mut(&mut d); + + assert_eq!(args.len(), 4); + assert_eq!(args.take_owned::().unwrap(), String::from("a")); + assert_eq!(args.take::<&i32>().unwrap(), &123); + assert_eq!(args.take_ref::().unwrap(), &456); + assert_eq!(args.take_mut::().unwrap(), &mut 5.78); + assert_eq!(args.len(), 0); + } + + #[test] + fn should_pop_args_in_reverse_order() { + let a = String::from("a"); + let b = 123_i32; + let c = 456_usize; + let mut d = 5.78_f32; + + let mut args = ArgList::new() + .push_owned(a) + .push_ref(&b) + .push_ref(&c) + .push_mut(&mut d); + + assert_eq!(args.len(), 4); + assert_eq!(args.pop_mut::().unwrap(), &mut 5.78); + assert_eq!(args.pop_ref::().unwrap(), &456); + assert_eq!(args.pop::<&i32>().unwrap(), &123); + assert_eq!(args.pop_owned::().unwrap(), String::from("a")); + assert_eq!(args.len(), 0); + } + + #[test] + fn should_reindex_on_push_after_take() { + let mut args = ArgList::new() + .push_owned(123) + .push_owned(456) + .push_owned(789); + + assert!(!args.needs_reindex); + + args.take_arg().unwrap(); + assert!(args.needs_reindex); + assert!(args.list[0].value().reflect_partial_eq(&456).unwrap()); + assert_eq!(args.list[0].index(), 1); + assert!(args.list[1].value().reflect_partial_eq(&789).unwrap()); + assert_eq!(args.list[1].index(), 2); + + let args = args.push_owned(123); + assert!(!args.needs_reindex); + assert!(args.list[0].value().reflect_partial_eq(&456).unwrap()); + assert_eq!(args.list[0].index(), 0); + assert!(args.list[1].value().reflect_partial_eq(&789).unwrap()); + assert_eq!(args.list[1].index(), 1); + assert!(args.list[2].value().reflect_partial_eq(&123).unwrap()); + assert_eq!(args.list[2].index(), 2); } } diff --git a/crates/bevy_reflect/src/func/args/ownership.rs b/crates/bevy_reflect/src/func/args/ownership.rs index 554126d295..e12ca5440a 100644 --- a/crates/bevy_reflect/src/func/args/ownership.rs +++ b/crates/bevy_reflect/src/func/args/ownership.rs @@ -2,8 +2,14 @@ use core::fmt::{Display, Formatter}; /// A trait for getting the ownership of a type. /// +/// This trait exists so that [`IntoFunction`] can automatically generate +/// [`FunctionInfo`] containing the proper [`Ownership`] for its [argument] types. +/// /// This trait is automatically implemented when using the `Reflect` [derive macro]. /// +/// [`IntoFunction`]: crate::func::IntoFunction +/// [`FunctionInfo`]: crate::func::FunctionInfo +/// [argument]: crate::func::args::Arg /// [derive macro]: derive@crate::Reflect pub trait GetOwnership { /// Returns the ownership of [`Self`]. diff --git a/crates/bevy_reflect/src/func/closures/dynamic_closure.rs b/crates/bevy_reflect/src/func/closures/dynamic_closure.rs index 8fbdbbfd2b..bbfe234cae 100644 --- a/crates/bevy_reflect/src/func/closures/dynamic_closure.rs +++ b/crates/bevy_reflect/src/func/closures/dynamic_closure.rs @@ -47,7 +47,7 @@ use crate::func::{FunctionResult, IntoClosure, ReturnInfo}; /// [`DynamicFunction`]: crate::func::DynamicFunction pub struct DynamicClosure<'env> { info: FunctionInfo, - func: Box Fn(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'env>, + func: Box Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>, } impl<'env> DynamicClosure<'env> { @@ -56,8 +56,8 @@ impl<'env> DynamicClosure<'env> { /// The given function can be used to call out to a regular function, closure, or method. /// /// It's important that the closure signature matches the provided [`FunctionInfo`]. - /// This info is used to validate the arguments and return value. - pub fn new Fn(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'env>( + /// This info may be used by consumers of the function for validation and debugging. + pub fn new Fn(ArgList<'a>) -> FunctionResult<'a> + 'env>( func: F, info: FunctionInfo, ) -> Self { @@ -83,8 +83,8 @@ impl<'env> DynamicClosure<'env> { /// Set the arguments of the closure. /// - /// It is very important that the arguments match the intended closure signature, - /// as this is used to validate arguments passed to the closure. + /// It's important that the arguments match the intended closure signature, + /// as this can be used by consumers of the function for validation and debugging. pub fn with_args(mut self, args: Vec) -> Self { self.info = self.info.with_args(args); self @@ -113,7 +113,7 @@ impl<'env> DynamicClosure<'env> { /// assert_eq!(result.take::().unwrap(), 123); /// ``` pub fn call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> { - (self.func)(args, &self.info) + (self.func)(args) } /// Returns the closure info. diff --git a/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs b/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs index eb63463d9a..3111b74760 100644 --- a/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs +++ b/crates/bevy_reflect/src/func/closures/dynamic_closure_mut.rs @@ -56,7 +56,7 @@ use crate::func::{FunctionResult, IntoClosureMut, ReturnInfo}; /// [`DynamicFunction`]: crate::func::DynamicFunction pub struct DynamicClosureMut<'env> { info: FunctionInfo, - func: Box FnMut(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'env>, + func: Box FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>, } impl<'env> DynamicClosureMut<'env> { @@ -65,8 +65,8 @@ impl<'env> DynamicClosureMut<'env> { /// The given function can be used to call out to a regular function, closure, or method. /// /// It's important that the closure signature matches the provided [`FunctionInfo`]. - /// This info is used to validate the arguments and return value. - pub fn new FnMut(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'env>( + /// This info may be used by consumers of the function for validation and debugging. + pub fn new FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>( func: F, info: FunctionInfo, ) -> Self { @@ -92,8 +92,8 @@ impl<'env> DynamicClosureMut<'env> { /// Set the arguments of the closure. /// - /// It is very important that the arguments match the intended closure signature, - /// as this is used to validate arguments passed to the closure. + /// It's important that the arguments match the intended closure signature, + /// as this can be used by consumers of the function for validation and debugging. pub fn with_args(mut self, args: Vec) -> Self { self.info = self.info.with_args(args); self @@ -130,7 +130,7 @@ impl<'env> DynamicClosureMut<'env> { /// /// [`call_once`]: DynamicClosureMut::call_once pub fn call<'a>(&mut self, args: ArgList<'a>) -> FunctionResult<'a> { - (self.func)(args, &self.info) + (self.func)(args) } /// Call the closure with the given arguments and consume the closure. @@ -155,7 +155,7 @@ impl<'env> DynamicClosureMut<'env> { /// assert_eq!(count, 5); /// ``` pub fn call_once(mut self, args: ArgList) -> FunctionResult { - (self.func)(args, &self.info) + (self.func)(args) } /// Returns the closure info. diff --git a/crates/bevy_reflect/src/func/closures/into_closure.rs b/crates/bevy_reflect/src/func/closures/into_closure.rs index 3d191a1e11..eddf762af8 100644 --- a/crates/bevy_reflect/src/func/closures/into_closure.rs +++ b/crates/bevy_reflect/src/func/closures/into_closure.rs @@ -21,10 +21,7 @@ where F: ReflectFn<'env, Marker1> + TypedFunction + 'env, { fn into_closure(self) -> DynamicClosure<'env> { - DynamicClosure::new( - move |args, info| self.reflect_call(args, info), - Self::function_info(), - ) + DynamicClosure::new(move |args| self.reflect_call(args), Self::function_info()) } } diff --git a/crates/bevy_reflect/src/func/closures/into_closure_mut.rs b/crates/bevy_reflect/src/func/closures/into_closure_mut.rs index a3daff6caa..05a6d8e599 100644 --- a/crates/bevy_reflect/src/func/closures/into_closure_mut.rs +++ b/crates/bevy_reflect/src/func/closures/into_closure_mut.rs @@ -23,7 +23,7 @@ where { fn into_closure_mut(mut self) -> DynamicClosureMut<'env> { DynamicClosureMut::new( - move |args, info| self.reflect_call_mut(args, info), + move |args| self.reflect_call_mut(args), Self::function_info(), ) } diff --git a/crates/bevy_reflect/src/func/error.rs b/crates/bevy_reflect/src/func/error.rs index 81d9f535d4..636b32c826 100644 --- a/crates/bevy_reflect/src/func/error.rs +++ b/crates/bevy_reflect/src/func/error.rs @@ -13,7 +13,7 @@ pub enum FunctionError { ArgError(#[from] ArgError), /// The number of arguments provided does not match the expected number. #[error("expected {expected} arguments but received {received}")] - InvalidArgCount { expected: usize, received: usize }, + ArgCountMismatch { expected: usize, received: usize }, } /// The result of calling a dynamic [`DynamicFunction`] or [`DynamicClosure`]. diff --git a/crates/bevy_reflect/src/func/function.rs b/crates/bevy_reflect/src/func/function.rs index a84e5dca1f..f602f0f25e 100644 --- a/crates/bevy_reflect/src/func/function.rs +++ b/crates/bevy_reflect/src/func/function.rs @@ -62,19 +62,15 @@ use crate::func::{FunctionResult, IntoFunction, ReturnInfo}; /// // We start by defining the shape of the function: /// let info = FunctionInfo::new() /// .with_name("append") -/// .with_args(vec![ -/// ArgInfo::new::(0).with_name("value"), -/// ArgInfo::new::<&mut Vec>(1).with_name("list"), -/// ]) -/// .with_return_info( -/// ReturnInfo::new::<&mut String>() -/// ); +/// .with_arg::("value") +/// .with_arg::<&mut Vec>("list") +/// .with_return::<&mut String>(); /// /// // Then we define the dynamic function, which will be used to call our `append` function: -/// let mut func = DynamicFunction::new(|mut args, info| { -/// // Arguments are popped from the list in reverse order: -/// let arg1 = args.pop().unwrap().take_mut::>(&info.args()[1]).unwrap(); -/// let arg0 = args.pop().unwrap().take_owned::(&info.args()[0]).unwrap(); +/// let mut func = DynamicFunction::new(|mut args| { +/// // Arguments are removed from the list in order: +/// let arg0 = args.take::()?; +/// let arg1 = args.take::<&mut Vec>()?; /// /// // Then we can call our function and return the result: /// Ok(Return::Mut(append(arg0, arg1))) @@ -97,7 +93,7 @@ use crate::func::{FunctionResult, IntoFunction, ReturnInfo}; /// [module-level documentation]: crate::func pub struct DynamicFunction { info: FunctionInfo, - func: Arc Fn(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'static>, + func: Arc Fn(ArgList<'a>) -> FunctionResult<'a> + 'static>, } impl DynamicFunction { @@ -106,8 +102,8 @@ impl DynamicFunction { /// The given function can be used to call out to a regular function, closure, or method. /// /// It's important that the function signature matches the provided [`FunctionInfo`]. - /// This info is used to validate the arguments and return value. - pub fn new Fn(ArgList<'a>, &FunctionInfo) -> FunctionResult<'a> + 'static>( + /// This info may be used by consumers of the function for validation and debugging. + pub fn new Fn(ArgList<'a>) -> FunctionResult<'a> + 'static>( func: F, info: FunctionInfo, ) -> Self { @@ -130,8 +126,8 @@ impl DynamicFunction { /// Set the arguments of the function. /// - /// It is very important that the arguments match the intended function signature, - /// as this is used to validate arguments passed to the function. + /// It's important that the arguments match the intended function signature, + /// as this can be used by consumers of the function for validation and debugging. pub fn with_args(mut self, args: Vec) -> Self { self.info = self.info.with_args(args); self @@ -159,7 +155,7 @@ impl DynamicFunction { /// assert_eq!(result.take::().unwrap(), 100); /// ``` pub fn call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> { - (self.func)(args, &self.info) + (self.func)(args) } /// Returns the function info. @@ -212,6 +208,7 @@ impl IntoFunction<()> for DynamicFunction { #[cfg(test)] mod tests { use super::*; + use crate::func::Return; #[test] fn should_overwrite_function_name() { @@ -230,4 +227,34 @@ mod tests { let function: DynamicFunction = make_function(|| {}); let _: DynamicFunction = make_function(function); } + + #[test] + fn should_allow_manual_function_construction() { + #[allow(clippy::ptr_arg)] + fn get(index: usize, list: &Vec) -> &String { + &list[index] + } + + let func = DynamicFunction::new( + |mut args| { + let list = args.pop::<&Vec>()?; + let index = args.pop::()?; + Ok(Return::Ref(get(index, list))) + }, + FunctionInfo::new() + .with_name("get") + .with_arg::("index") + .with_arg::<&Vec>("list") + .with_return::<&String>(), + ); + + let list = vec![String::from("foo")]; + let value = func + .call(ArgList::new().push_owned(0_usize).push_ref(&list)) + .unwrap() + .unwrap_ref() + .downcast_ref::() + .unwrap(); + assert_eq!(value, "foo"); + } } diff --git a/crates/bevy_reflect/src/func/info.rs b/crates/bevy_reflect/src/func/info.rs index 6793b654a4..059a05373a 100644 --- a/crates/bevy_reflect/src/func/info.rs +++ b/crates/bevy_reflect/src/func/info.rs @@ -48,18 +48,47 @@ impl FunctionInfo { self } + /// Push an argument onto the function's argument list. + /// + /// The order in which this method is called matters as it will determine the index of the argument + /// based on the current number of arguments. + pub fn with_arg( + mut self, + name: impl Into>, + ) -> Self { + let index = self.args.len(); + self.args.push(ArgInfo::new::(index).with_name(name)); + self + } + /// Set the arguments of the function. /// - /// Arguments passed to the function will be validated against the info provided here. - /// Mismatched arguments may result in the function call returning an [error]. + /// This will completely replace any existing arguments. /// - /// [error]: crate::func::FunctionError + /// It's preferable to use [`Self::with_arg`] to add arguments to the function + /// as it will automatically set the index of the argument. pub fn with_args(mut self, args: Vec) -> Self { self.args = args; self } - /// Set the return information of the function. + /// Set the [return information] of the function. + /// + /// To manually set the [`ReturnInfo`] of the function, see [`Self::with_return_info`]. + /// + /// [return information]: ReturnInfo + pub fn with_return(mut self) -> Self { + self.return_info = ReturnInfo::new::(); + self + } + + /// Set the [return information] of the function. + /// + /// This will completely replace any existing return information. + /// + /// For a simpler, static version of this method, see [`Self::with_return`]. + /// + /// [return information]: ReturnInfo pub fn with_return_info(mut self, return_info: ReturnInfo) -> Self { self.return_info = return_info; self diff --git a/crates/bevy_reflect/src/func/into_function.rs b/crates/bevy_reflect/src/func/into_function.rs index 7104eb1a72..dad36867a7 100644 --- a/crates/bevy_reflect/src/func/into_function.rs +++ b/crates/bevy_reflect/src/func/into_function.rs @@ -47,10 +47,7 @@ where // However, we don't do this because it would prevent users from // converting function pointers into `DynamicFunction`s. - DynamicFunction::new( - move |args, info| self.reflect_call(args, info), - Self::function_info(), - ) + DynamicFunction::new(move |args| self.reflect_call(args), Self::function_info()) } } diff --git a/crates/bevy_reflect/src/func/mod.rs b/crates/bevy_reflect/src/func/mod.rs index 0c741cf048..ce1340cd76 100644 --- a/crates/bevy_reflect/src/func/mod.rs +++ b/crates/bevy_reflect/src/func/mod.rs @@ -99,7 +99,7 @@ //! [lack of variadic generics]: https://poignardazur.github.io/2024/05/25/report-on-rustnl-variadics/ //! [coherence issues]: https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#coherence-leak-check -pub use args::{Arg, ArgError, ArgList}; +pub use args::{ArgError, ArgList, ArgValue}; pub use closures::*; pub use error::*; pub use function::*; @@ -124,7 +124,7 @@ mod return_type; mod tests { use alloc::borrow::Cow; - use crate::func::args::{ArgError, ArgId, ArgList, Ownership}; + use crate::func::args::{ArgError, ArgList, Ownership}; use crate::TypePath; use super::*; @@ -138,7 +138,7 @@ mod tests { let result = func.call(args); assert_eq!( result.unwrap_err(), - FunctionError::InvalidArgCount { + FunctionError::ArgCountMismatch { expected: 1, received: 0 } @@ -154,7 +154,7 @@ mod tests { let result = func.call(args); assert_eq!( result.unwrap_err(), - FunctionError::InvalidArgCount { + FunctionError::ArgCountMismatch { expected: 0, received: 1 } @@ -171,7 +171,7 @@ mod tests { assert_eq!( result.unwrap_err(), FunctionError::ArgError(ArgError::UnexpectedType { - id: ArgId::Index(0), + index: 0, expected: Cow::Borrowed(i32::type_path()), received: Cow::Borrowed(u32::type_path()) }) @@ -188,7 +188,7 @@ mod tests { assert_eq!( result.unwrap_err(), FunctionError::ArgError(ArgError::InvalidOwnership { - id: ArgId::Index(0), + index: 0, expected: Ownership::Ref, received: Ownership::Owned }) diff --git a/crates/bevy_reflect/src/func/reflect_fn.rs b/crates/bevy_reflect/src/func/reflect_fn.rs index 7a80f4cbd1..83c77b6b2b 100644 --- a/crates/bevy_reflect/src/func/reflect_fn.rs +++ b/crates/bevy_reflect/src/func/reflect_fn.rs @@ -2,8 +2,8 @@ use bevy_utils::all_tuples; use crate::func::args::FromArg; use crate::func::macros::count_tokens; -use crate::func::{ArgList, FunctionError, FunctionInfo, FunctionResult, IntoReturn, ReflectFnMut}; -use crate::Reflect; +use crate::func::{ArgList, FunctionError, FunctionResult, IntoReturn, ReflectFnMut}; +use crate::{Reflect, TypePath}; /// A reflection-based version of the [`Fn`] trait. /// @@ -33,16 +33,15 @@ use crate::Reflect; /// # Example /// /// ``` -/// # use bevy_reflect::func::{ArgList, FunctionInfo, ReflectFn, TypedFunction}; +/// # use bevy_reflect::func::{ArgList, FunctionInfo, ReflectFn}; /// # /// fn add(a: i32, b: i32) -> i32 { /// a + b /// } /// /// let args = ArgList::new().push_owned(25_i32).push_owned(75_i32); -/// let info = add.get_function_info(); /// -/// let value = add.reflect_call(args, &info).unwrap().unwrap_owned(); +/// let value = add.reflect_call(args).unwrap().unwrap_owned(); /// assert_eq!(value.take::().unwrap(), 100); /// ``` /// @@ -62,7 +61,7 @@ use crate::Reflect; /// [unconstrained type parameters]: https://doc.rust-lang.org/error_codes/E0207.html pub trait ReflectFn<'env, Marker>: ReflectFnMut<'env, Marker> { /// Call the function with the given arguments and return the result. - fn reflect_call<'a>(&self, args: ArgList<'a>, info: &FunctionInfo) -> FunctionResult<'a>; + fn reflect_call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a>; } /// Helper macro for implementing [`ReflectFn`] on Rust closures. @@ -81,27 +80,22 @@ macro_rules! impl_reflect_fn { // This clause allows us to convert `ReturnType` into `Return` ReturnType: IntoReturn + Reflect, Function: Fn($($Arg),*) -> ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> Fn($($Arg::Item<'a>),*) -> ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> Fn($($Arg::This<'a>),*) -> ReturnType + 'env, { - fn reflect_call<'a>(&self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + #[allow(unused_mut)] + fn reflect_call<'a>(&self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!($($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [$($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - #[allow(unused_mut)] - let mut _index = 0; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + $(let $arg = args.take::<$Arg>()?;)* Ok((self)($($arg,)*).into_return()) } @@ -110,35 +104,28 @@ macro_rules! impl_reflect_fn { // === (&self, ...) -> &ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFn<'env, fn(&Receiver, $($Arg),*) -> &ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&ReturnType` into `Return` for<'a> &'a ReturnType: IntoReturn, Function: for<'a> Fn(&'a Receiver, $($Arg),*) -> &'a ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> Fn(&'a Receiver, $($Arg::Item<'a>),*) -> &'a ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> Fn(&'a Receiver, $($Arg::This<'a>),*) -> &'a ReturnType + 'env, { - fn reflect_call<'a>(&self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call<'a>(&self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_ref::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_ref::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } @@ -147,35 +134,28 @@ macro_rules! impl_reflect_fn { // === (&mut self, ...) -> &mut ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFn<'env, fn(&mut Receiver, $($Arg),*) -> &mut ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&mut ReturnType` into `Return` for<'a> &'a mut ReturnType: IntoReturn, Function: for<'a> Fn(&'a mut Receiver, $($Arg),*) -> &'a mut ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> Fn(&'a mut Receiver, $($Arg::Item<'a>),*) -> &'a mut ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> Fn(&'a mut Receiver, $($Arg::This<'a>),*) -> &'a mut ReturnType + 'env, { - fn reflect_call<'a>(&self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call<'a>(&self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_mut::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_mut::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } @@ -184,35 +164,28 @@ macro_rules! impl_reflect_fn { // === (&mut self, ...) -> &ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFn<'env, fn(&mut Receiver, $($Arg),*) -> &ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&ReturnType` into `Return` for<'a> &'a ReturnType: IntoReturn, Function: for<'a> Fn(&'a mut Receiver, $($Arg),*) -> &'a ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> Fn(&'a mut Receiver, $($Arg::Item<'a>),*) -> &'a ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> Fn(&'a mut Receiver, $($Arg::This<'a>),*) -> &'a ReturnType + 'env, { - fn reflect_call<'a>(&self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call<'a>(&self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_mut::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_mut::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } diff --git a/crates/bevy_reflect/src/func/reflect_fn_mut.rs b/crates/bevy_reflect/src/func/reflect_fn_mut.rs index a396d56262..ae109832e4 100644 --- a/crates/bevy_reflect/src/func/reflect_fn_mut.rs +++ b/crates/bevy_reflect/src/func/reflect_fn_mut.rs @@ -2,8 +2,8 @@ use bevy_utils::all_tuples; use crate::func::args::FromArg; use crate::func::macros::count_tokens; -use crate::func::{ArgList, FunctionError, FunctionInfo, FunctionResult, IntoReturn}; -use crate::Reflect; +use crate::func::{ArgList, FunctionError, FunctionResult, IntoReturn}; +use crate::{Reflect, TypePath}; /// A reflection-based version of the [`FnMut`] trait. /// @@ -35,7 +35,7 @@ use crate::Reflect; /// # Example /// /// ``` -/// # use bevy_reflect::func::{ArgList, FunctionInfo, ReflectFnMut, TypedFunction}; +/// # use bevy_reflect::func::{ArgList, FunctionInfo, ReflectFnMut}; /// # /// let mut list: Vec = vec![1, 3]; /// @@ -45,9 +45,8 @@ use crate::Reflect; /// }; /// /// let args = ArgList::new().push_owned(1_usize).push_owned(2_i32); -/// let info = insert.get_function_info(); /// -/// insert.reflect_call_mut(args, &info).unwrap(); +/// insert.reflect_call_mut(args).unwrap(); /// assert_eq!(list, vec![1, 2, 3]); /// ``` /// @@ -68,11 +67,7 @@ use crate::Reflect; /// [unconstrained type parameters]: https://doc.rust-lang.org/error_codes/E0207.html pub trait ReflectFnMut<'env, Marker> { /// Call the function with the given arguments and return the result. - fn reflect_call_mut<'a>( - &mut self, - args: ArgList<'a>, - info: &FunctionInfo, - ) -> FunctionResult<'a>; + fn reflect_call_mut<'a>(&mut self, args: ArgList<'a>) -> FunctionResult<'a>; } /// Helper macro for implementing [`ReflectFnMut`] on Rust closures. @@ -91,27 +86,22 @@ macro_rules! impl_reflect_fn_mut { // This clause allows us to convert `ReturnType` into `Return` ReturnType: IntoReturn + Reflect, Function: FnMut($($Arg),*) -> ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> FnMut($($Arg::Item<'a>),*) -> ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> FnMut($($Arg::This<'a>),*) -> ReturnType + 'env, { - fn reflect_call_mut<'a>(&mut self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + #[allow(unused_mut)] + fn reflect_call_mut<'a>(&mut self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!($($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [$($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - #[allow(unused_mut)] - let mut _index = 0; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + $(let $arg = args.take::<$Arg>()?;)* Ok((self)($($arg,)*).into_return()) } @@ -120,35 +110,28 @@ macro_rules! impl_reflect_fn_mut { // === (&self, ...) -> &ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFnMut<'env, fn(&Receiver, $($Arg),*) -> &ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&ReturnType` into `Return` for<'a> &'a ReturnType: IntoReturn, Function: for<'a> FnMut(&'a Receiver, $($Arg),*) -> &'a ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> FnMut(&'a Receiver, $($Arg::Item<'a>),*) -> &'a ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> FnMut(&'a Receiver, $($Arg::This<'a>),*) -> &'a ReturnType + 'env, { - fn reflect_call_mut<'a>(&mut self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call_mut<'a>(&mut self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_ref::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_ref::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } @@ -157,35 +140,28 @@ macro_rules! impl_reflect_fn_mut { // === (&mut self, ...) -> &mut ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFnMut<'env, fn(&mut Receiver, $($Arg),*) -> &mut ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&mut ReturnType` into `Return` for<'a> &'a mut ReturnType: IntoReturn, Function: for<'a> FnMut(&'a mut Receiver, $($Arg),*) -> &'a mut ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> FnMut(&'a mut Receiver, $($Arg::Item<'a>),*) -> &'a mut ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> FnMut(&'a mut Receiver, $($Arg::This<'a>),*) -> &'a mut ReturnType + 'env, { - fn reflect_call_mut<'a>(&mut self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call_mut<'a>(&mut self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_mut::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_mut::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } @@ -194,35 +170,28 @@ macro_rules! impl_reflect_fn_mut { // === (&mut self, ...) -> &ReturnType === // impl<'env, Receiver, $($Arg,)* ReturnType, Function> ReflectFnMut<'env, fn(&mut Receiver, $($Arg),*) -> &ReturnType> for Function where - Receiver: Reflect, + Receiver: Reflect + TypePath, $($Arg: FromArg,)* ReturnType: Reflect, // This clause allows us to convert `&ReturnType` into `Return` for<'a> &'a ReturnType: IntoReturn, Function: for<'a> FnMut(&'a mut Receiver, $($Arg),*) -> &'a ReturnType + 'env, - // This clause essentially asserts that `Arg::Item` is the same type as `Arg` - Function: for<'a> FnMut(&'a mut Receiver, $($Arg::Item<'a>),*) -> &'a ReturnType + 'env, + // This clause essentially asserts that `Arg::This` is the same type as `Arg` + Function: for<'a> FnMut(&'a mut Receiver, $($Arg::This<'a>),*) -> &'a ReturnType + 'env, { - fn reflect_call_mut<'a>(&mut self, args: ArgList<'a>, _info: &FunctionInfo) -> FunctionResult<'a> { + fn reflect_call_mut<'a>(&mut self, mut args: ArgList<'a>) -> FunctionResult<'a> { const COUNT: usize = count_tokens!(Receiver $($Arg)*); if args.len() != COUNT { - return Err(FunctionError::InvalidArgCount { + return Err(FunctionError::ArgCountMismatch { expected: COUNT, received: args.len(), }); } - let [receiver, $($arg,)*] = args.take().try_into().expect("invalid number of arguments"); - - let receiver = receiver.take_mut::(_info.args().get(0).expect("argument index out of bounds"))?; - - #[allow(unused_mut)] - let mut _index = 1; - let ($($arg,)*) = ($($Arg::from_arg($arg, { - _index += 1; - _info.args().get(_index - 1).expect("argument index out of bounds") - })?,)*); + // Extract all arguments (in order) + let receiver = args.take_mut::()?; + $(let $arg = args.take::<$Arg>()?;)* Ok((self)(receiver, $($arg,)*).into_return()) } diff --git a/crates/bevy_reflect/src/func/return_type.rs b/crates/bevy_reflect/src/func/return_type.rs index 59a0f36adc..b20dd62d1c 100644 --- a/crates/bevy_reflect/src/func/return_type.rs +++ b/crates/bevy_reflect/src/func/return_type.rs @@ -61,11 +61,15 @@ impl<'a> Return<'a> { /// A trait for types that can be converted into a [`Return`] value. /// +/// This trait exists so that types can be automatically converted into a [`Return`] +/// by [`IntoFunction`]. +/// /// This trait is used instead of a blanket [`Into`] implementation due to coherence issues: /// we can't implement `Into` for both `T` and `&T`/`&mut T`. /// /// This trait is automatically implemented when using the `Reflect` [derive macro]. /// +/// [`IntoFunction`]: crate::func::IntoFunction /// [derive macro]: derive@crate::Reflect pub trait IntoReturn { /// Converts [`Self`] into a [`Return`] value. diff --git a/examples/reflection/function_reflection.rs b/examples/reflection/function_reflection.rs index 4d1faf7c1a..8935c2096a 100644 --- a/examples/reflection/function_reflection.rs +++ b/examples/reflection/function_reflection.rs @@ -6,10 +6,9 @@ //! This can be used for things like adding scripting support to your application, //! processing deserialized reflection data, or even just storing type-erased versions of your functions. -use bevy::reflect::func::args::ArgInfo; use bevy::reflect::func::{ - ArgList, DynamicClosure, DynamicClosureMut, DynamicFunction, FunctionInfo, IntoClosure, - IntoClosureMut, IntoFunction, Return, ReturnInfo, + ArgList, DynamicClosure, DynamicClosureMut, DynamicFunction, FunctionError, FunctionInfo, + IntoClosure, IntoClosureMut, IntoFunction, Return, }; use bevy::reflect::Reflect; @@ -129,33 +128,37 @@ fn main() { } let get_or_insert_function = dbg!(DynamicFunction::new( - |mut args, info| { - let container_info = &info.args()[1]; - let value_info = &info.args()[0]; + |mut args| { + // We can optionally add a check to ensure we were given the correct number of arguments. + if args.len() != 2 { + return Err(FunctionError::ArgCountMismatch { + expected: 2, + received: args.len(), + }); + } // The `ArgList` contains the arguments in the order they were pushed. - // Therefore, we need to pop them in reverse order. - let container = args - .pop() - .unwrap() - .take_mut::>(container_info) - .unwrap(); - let value = args.pop().unwrap().take_owned::(value_info).unwrap(); + // We can retrieve them out in order (note that this modifies the `ArgList`): + let value = args.take::()?; + let container = args.take::<&mut Option>()?; + + // We could have also done the following to make use of type inference: + // let value = args.take_owned()?; + // let container = args.take_mut()?; Ok(Return::Ref(get_or_insert(value, container))) }, FunctionInfo::new() - // We can optionally provide a name for the function + // We can optionally provide a name for the function. .with_name("get_or_insert") - // Since our function takes arguments, we MUST provide that argument information. - // The arguments should be provided in the order they are defined in the function. - // This is used to validate any arguments given at runtime. - .with_args(vec![ - ArgInfo::new::(0).with_name("value"), - ArgInfo::new::<&mut Option>(1).with_name("container"), - ]) - // We can optionally provide return information as well. - .with_return_info(ReturnInfo::new::<&i32>()), + // Since our function takes arguments, we should provide that argument information. + // This helps ensure that consumers of the function can validate the arguments they + // pass into the function and helps for debugging. + // Arguments should be provided in the order they are defined in the function. + .with_arg::("value") + .with_arg::<&mut Option>("container") + // We can provide return information as well. + .with_return::<&i32>(), )); let mut container: Option = None;