diff --git a/crates/bevy_reflect/src/func/dynamic_function.rs b/crates/bevy_reflect/src/func/dynamic_function.rs index fba0031489..dcaa051c00 100644 --- a/crates/bevy_reflect/src/func/dynamic_function.rs +++ b/crates/bevy_reflect/src/func/dynamic_function.rs @@ -1,9 +1,9 @@ use crate as bevy_reflect; use crate::__macro_exports::RegisterForReflection; -use crate::func::args::{ArgInfo, ArgList}; +use crate::func::args::ArgList; use crate::func::info::FunctionInfo; use crate::func::{ - DynamicFunctionMut, Function, FunctionResult, IntoFunction, IntoFunctionMut, ReturnInfo, + DynamicFunctionMut, Function, FunctionError, FunctionResult, IntoFunction, IntoFunctionMut, }; use crate::serde::Serializable; use crate::{ @@ -64,8 +64,10 @@ impl<'env> DynamicFunction<'env> { /// The given function can be used to call out to any other callable, /// including functions, closures, or methods. /// - /// It's important that the function signature matches the provided [`FunctionInfo`]. - /// This info may be used by consumers of this function for validation and debugging. + /// It's important that the function signature matches the provided [`FunctionInfo`] + /// as this will be used to validate arguments when [calling] the function. + /// + /// [calling]: DynamicFunction::call pub fn new Fn(ArgList<'a>) -> FunctionResult<'a> + Send + Sync + 'env>( func: F, info: FunctionInfo, @@ -89,21 +91,6 @@ impl<'env> DynamicFunction<'env> { self } - /// Set the argument information of the function. - /// - /// It's important that the arguments match the intended function signature, - /// as this can be used by consumers of this function for validation and debugging. - pub fn with_args(mut self, args: Vec) -> Self { - self.info = self.info.with_args(args); - self - } - - /// Set the return information of the function. - pub fn with_return_info(mut self, return_info: ReturnInfo) -> Self { - self.info = self.info.with_return_info(return_info); - self - } - /// Call the function with the given arguments. /// /// # Example @@ -120,8 +107,25 @@ impl<'env> DynamicFunction<'env> { /// let result = func.call(args).unwrap().unwrap_owned(); /// assert_eq!(result.try_take::().unwrap(), 123); /// ``` + /// + /// # Errors + /// + /// This method will return an error if the number of arguments provided does not match + /// the number of arguments expected by the function's [`FunctionInfo`]. + /// + /// The function itself may also return any errors it needs to. pub fn call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> { - (self.func)(args) + let expected_arg_count = self.info.arg_count(); + let received_arg_count = args.len(); + + if expected_arg_count != received_arg_count { + Err(FunctionError::ArgCountMismatch { + expected: expected_arg_count, + received: received_arg_count, + }) + } else { + (self.func)(args) + } } /// Returns the function info. @@ -321,6 +325,21 @@ mod tests { let _: DynamicFunction = make_closure(function); } + #[test] + fn should_return_error_on_arg_count_mismatch() { + let func = (|a: i32, b: i32| a + b).into_function(); + + let args = ArgList::default().push_owned(25_i32); + let error = func.call(args).unwrap_err(); + assert!(matches!( + error, + FunctionError::ArgCountMismatch { + expected: 2, + received: 1 + } + )); + } + #[test] fn should_clone_dynamic_function() { let hello = String::from("Hello"); @@ -379,7 +398,10 @@ mod tests { } }, // The `FunctionInfo` doesn't really matter for this test - FunctionInfo::anonymous(), + // so we can just give it dummy information. + FunctionInfo::anonymous() + .with_arg::("curr") + .with_arg::<()>("this"), ); let args = ArgList::new().push_ref(&factorial).push_owned(5_i32); diff --git a/crates/bevy_reflect/src/func/dynamic_function_mut.rs b/crates/bevy_reflect/src/func/dynamic_function_mut.rs index 06d1e0f71c..d39202a1f6 100644 --- a/crates/bevy_reflect/src/func/dynamic_function_mut.rs +++ b/crates/bevy_reflect/src/func/dynamic_function_mut.rs @@ -1,9 +1,9 @@ use alloc::borrow::Cow; use core::fmt::{Debug, Formatter}; -use crate::func::args::{ArgInfo, ArgList}; +use crate::func::args::ArgList; use crate::func::info::FunctionInfo; -use crate::func::{DynamicFunction, FunctionResult, IntoFunctionMut, ReturnInfo}; +use crate::func::{DynamicFunction, FunctionError, FunctionResult, IntoFunctionMut}; /// A dynamic representation of a function. /// @@ -72,8 +72,10 @@ impl<'env> DynamicFunctionMut<'env> { /// The given function can be used to call out to any other callable, /// including functions, closures, or methods. /// - /// It's important that the function signature matches the provided [`FunctionInfo`]. - /// This info may be used by consumers of this function for validation and debugging. + /// It's important that the function signature matches the provided [`FunctionInfo`] + /// as this will be used to validate arguments when [calling] the function. + /// + /// [calling]: DynamicFunctionMut::call pub fn new FnMut(ArgList<'a>) -> FunctionResult<'a> + 'env>( func: F, info: FunctionInfo, @@ -97,21 +99,6 @@ impl<'env> DynamicFunctionMut<'env> { self } - /// Set the argument information of the function. - /// - /// It's important that the arguments match the intended function signature, - /// as this can be used by consumers of this function for validation and debugging. - pub fn with_args(mut self, args: Vec) -> Self { - self.info = self.info.with_args(args); - self - } - - /// Set the return information of the function. - pub fn with_return_info(mut self, return_info: ReturnInfo) -> Self { - self.info = self.info.with_return_info(return_info); - self - } - /// Call the function with the given arguments. /// /// Variables that are captured mutably by this function @@ -135,9 +122,26 @@ impl<'env> DynamicFunctionMut<'env> { /// assert_eq!(result.try_take::().unwrap(), 100); /// ``` /// + /// # Errors + /// + /// This method will return an error if the number of arguments provided does not match + /// the number of arguments expected by the function's [`FunctionInfo`]. + /// + /// The function itself may also return any errors it needs to. + /// /// [`call_once`]: DynamicFunctionMut::call_once pub fn call<'a>(&mut self, args: ArgList<'a>) -> FunctionResult<'a> { - (self.func)(args) + let expected_arg_count = self.info.arg_count(); + let received_arg_count = args.len(); + + if expected_arg_count != received_arg_count { + Err(FunctionError::ArgCountMismatch { + expected: expected_arg_count, + received: received_arg_count, + }) + } else { + (self.func)(args) + } } /// Call the function with the given arguments and consume it. @@ -161,8 +165,25 @@ impl<'env> DynamicFunctionMut<'env> { /// increment_function.call_once(args).unwrap(); /// assert_eq!(count, 5); /// ``` + /// + /// # Errors + /// + /// This method will return an error if the number of arguments provided does not match + /// the number of arguments expected by the function's [`FunctionInfo`]. + /// + /// The function itself may also return any errors it needs to. pub fn call_once(mut self, args: ArgList) -> FunctionResult { - (self.func)(args) + let expected_arg_count = self.info.arg_count(); + let received_arg_count = args.len(); + + if expected_arg_count != received_arg_count { + Err(FunctionError::ArgCountMismatch { + expected: expected_arg_count, + received: received_arg_count, + }) + } else { + (self.func)(args) + } } /// Returns the function info. @@ -252,4 +273,30 @@ mod tests { let closure: DynamicFunctionMut = make_closure(|a: i32, b: i32| total = a + b); let _: DynamicFunctionMut = make_closure(closure); } + + #[test] + fn should_return_error_on_arg_count_mismatch() { + let mut total = 0; + let mut func = (|a: i32, b: i32| total = a + b).into_function_mut(); + + let args = ArgList::default().push_owned(25_i32); + let error = func.call(args).unwrap_err(); + assert!(matches!( + error, + FunctionError::ArgCountMismatch { + expected: 2, + received: 1 + } + )); + + let args = ArgList::default().push_owned(25_i32); + let error = func.call_once(args).unwrap_err(); + assert!(matches!( + error, + FunctionError::ArgCountMismatch { + expected: 2, + received: 1 + } + )); + } } diff --git a/examples/reflection/function_reflection.rs b/examples/reflection/function_reflection.rs index b004d2020f..d2334ae4a0 100644 --- a/examples/reflection/function_reflection.rs +++ b/examples/reflection/function_reflection.rs @@ -7,7 +7,7 @@ //! processing deserialized reflection data, or even just storing type-erased versions of your functions. use bevy::reflect::func::{ - ArgList, DynamicFunction, DynamicFunctionMut, FunctionError, FunctionInfo, IntoFunction, + ArgList, DynamicFunction, DynamicFunctionMut, FunctionInfo, FunctionResult, IntoFunction, IntoFunctionMut, Return, }; use bevy::reflect::{PartialReflect, Reflect}; @@ -129,16 +129,10 @@ fn main() { } let get_or_insert_function = dbg!(DynamicFunction::new( - |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(), - }); - } - + |mut args: ArgList| -> FunctionResult { // The `ArgList` contains the arguments in the order they were pushed. + // The `DynamicFunction` will validate that the list contains + // exactly the number of arguments we expect. // We can retrieve them out in order (note that this modifies the `ArgList`): let value = args.take::()?; let container = args.take::<&mut Option>()?; @@ -160,8 +154,8 @@ fn main() { // such as by using its type name or by prefixing it with your crate name. .with_name("my_crate::get_or_insert") // 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. + // This is used to validate arguments when calling the function. + // And it aids consumers of the function with their own validation and debugging. // Arguments should be provided in the order they are defined in the function. .with_arg::("value") .with_arg::<&mut Option>("container")