From 77ae7cf822958b7ee1d9d6a072a680638c717376 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 3 Sep 2024 19:02:56 -0700 Subject: [PATCH] Added non-panicking method for adding overloads --- .../bevy_reflect/src/func/dynamic_function.rs | 41 ++++++++++++++++--- .../src/func/dynamic_function_mut.rs | 41 +++++++++++++++++-- crates/bevy_reflect/src/func/function_map.rs | 38 +++++++++-------- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/crates/bevy_reflect/src/func/dynamic_function.rs b/crates/bevy_reflect/src/func/dynamic_function.rs index 8344f138f6..c685cf88a0 100644 --- a/crates/bevy_reflect/src/func/dynamic_function.rs +++ b/crates/bevy_reflect/src/func/dynamic_function.rs @@ -3,8 +3,8 @@ use crate::{ __macro_exports::RegisterForReflection, func::{ args::ArgList, function_map::FunctionMap, info::FunctionInfoType, - signature::ArgumentSignature, DynamicFunctionMut, Function, FunctionError, FunctionResult, - IntoFunction, IntoFunctionMut, + signature::ArgumentSignature, DynamicFunctionMut, Function, FunctionError, + FunctionOverloadError, FunctionResult, IntoFunction, IntoFunctionMut, }, serde::Serializable, ApplyError, MaybeTyped, PartialReflect, Reflect, ReflectKind, ReflectMut, ReflectOwned, @@ -150,6 +150,8 @@ impl<'env> DynamicFunction<'env> { /// /// Panics if the function, `F`, contains a signature already found in this function. /// + /// For a non-panicking version, see [`try_with_overload`]. + /// /// # Examples /// /// ``` @@ -227,6 +229,7 @@ impl<'env> DynamicFunction<'env> { /// /// [argument signature]: ArgumentSignature /// [name]: Self::name + /// [`try_with_overload`]: Self::try_with_overload pub fn with_overload<'a, F: IntoFunction<'a, Marker>, Marker>( self, function: F, @@ -237,16 +240,44 @@ impl<'env> DynamicFunction<'env> { let function = function.into_function(); let name = self.name.clone(); - let mut function_map = self.function_map; - function_map + let function_map = self + .function_map .merge(function.function_map) - .unwrap_or_else(|err| { + .unwrap_or_else(|(_, err)| { panic!("{}", err); }); DynamicFunction { name, function_map } } + /// Attempt to add an overload to this function. + /// + /// If the function, `F`, contains a signature already found in this function, + /// an error will be returned along with the original function. + /// + /// For a panicking version, see [`with_overload`]. + /// + /// [`with_overload`]: Self::with_overload + pub fn try_with_overload, Marker>( + self, + function: F, + ) -> Result, FunctionOverloadError)> { + let function = function.into_function(); + + let name = self.name.clone(); + + match self.function_map.merge(function.function_map) { + Ok(function_map) => Ok(Self { name, function_map }), + Err((function_map, err)) => Err(( + Box::new(Self { + name, + function_map: *function_map, + }), + err, + )), + } + } + /// Call the function with the given arguments. /// /// # Example diff --git a/crates/bevy_reflect/src/func/dynamic_function_mut.rs b/crates/bevy_reflect/src/func/dynamic_function_mut.rs index c934977b05..af92a21547 100644 --- a/crates/bevy_reflect/src/func/dynamic_function_mut.rs +++ b/crates/bevy_reflect/src/func/dynamic_function_mut.rs @@ -6,7 +6,7 @@ use alloc::{boxed::Box, format, vec}; use crate::func::{ args::ArgList, function_map::FunctionMap, signature::ArgumentSignature, DynamicFunction, - FunctionError, FunctionInfoType, FunctionResult, IntoFunctionMut, + FunctionError, FunctionInfoType, FunctionOverloadError, FunctionResult, IntoFunctionMut, }; use bevy_utils::HashMap; @@ -158,6 +158,8 @@ impl<'env> DynamicFunctionMut<'env> { /// /// Panics if the function, `F`, contains a signature already found in this function. /// + /// For a non-panicking version, see [`try_with_overload`]. + /// /// # Example /// /// ``` @@ -189,6 +191,7 @@ impl<'env> DynamicFunctionMut<'env> { /// /// [argument signature]: ArgumentSignature /// [name]: Self::name + /// [`try_with_overload`]: Self::try_with_overload pub fn with_overload<'a, F: IntoFunctionMut<'a, Marker>, Marker>( self, function: F, @@ -199,14 +202,44 @@ impl<'env> DynamicFunctionMut<'env> { let function = function.into_function_mut(); let name = self.name.clone(); - let mut function_map = self.function_map; - function_map + let function_map = self + .function_map .merge(function.function_map) - .unwrap_or_else(|_| todo!()); + .unwrap_or_else(|(_, err)| { + panic!("{}", err); + }); DynamicFunctionMut { name, function_map } } + /// Attempt to add an overload to this function. + /// + /// If the function, `F`, contains a signature already found in this function, + /// an error will be returned along with the original function. + /// + /// For a panicking version, see [`with_overload`]. + /// + /// [`with_overload`]: Self::with_overload + pub fn try_with_overload, Marker>( + self, + function: F, + ) -> Result, FunctionOverloadError)> { + let function = function.into_function_mut(); + + let name = self.name.clone(); + + match self.function_map.merge(function.function_map) { + Ok(function_map) => Ok(Self { name, function_map }), + Err((function_map, err)) => Err(( + Box::new(Self { + name, + function_map: *function_map, + }), + err, + )), + } + } + /// Call the function with the given arguments. /// /// Variables that are captured mutably by this function diff --git a/crates/bevy_reflect/src/func/function_map.rs b/crates/bevy_reflect/src/func/function_map.rs index aaab049647..832598edeb 100644 --- a/crates/bevy_reflect/src/func/function_map.rs +++ b/crates/bevy_reflect/src/func/function_map.rs @@ -59,14 +59,14 @@ impl FunctionMap { /// Merge another [`FunctionMap`] into this one. /// /// If the other map contains any functions with the same signature as this one, - /// an error will be returned and the original map will remain unchanged. - pub fn merge(&mut self, other: Self) -> Result<(), FunctionOverloadError> { + /// an error will be returned along with the original, unchanged map. + pub fn merge(mut self, other: Self) -> Result, FunctionOverloadError)> { // === Function Map === // let mut other_indices = HashMap::new(); for (sig, index) in other.indices { if self.indices.contains_key(&sig) { - return Err(FunctionOverloadError { signature: sig }); + return Err((Box::new(self), FunctionOverloadError { signature: sig })); } other_indices.insert(sig, self.functions.len() + index); @@ -78,7 +78,7 @@ impl FunctionMap { for info in other.info.into_iter() { let sig = ArgumentSignature::from(&info); if self.indices.contains_key(&sig) { - return Err(FunctionOverloadError { signature: sig }); + return Err((Box::new(self), FunctionOverloadError { signature: sig })); } other_infos.push(info); } @@ -86,16 +86,16 @@ impl FunctionMap { // === Update === // self.indices.extend(other_indices); self.functions.extend(other.functions); - self.info = match &self.info { - FunctionInfoType::Standard(info) => FunctionInfoType::Overloaded( - std::iter::once(info.clone()).chain(other_infos).collect(), - ), - FunctionInfoType::Overloaded(infos) => { - FunctionInfoType::Overloaded(infos.iter().cloned().chain(other_infos).collect()) + self.info = match self.info { + FunctionInfoType::Standard(info) => { + FunctionInfoType::Overloaded(std::iter::once(info).chain(other_infos).collect()) } + FunctionInfoType::Overloaded(infos) => FunctionInfoType::Overloaded( + IntoIterator::into_iter(infos).chain(other_infos).collect(), + ), }; - Ok(()) + Ok(self) } } @@ -107,7 +107,7 @@ mod tests { #[test] fn should_merge_function_maps() { - let mut map_a = FunctionMap { + let map_a = FunctionMap { info: FunctionInfoType::Overloaded(Box::new([ FunctionInfo::anonymous().with_arg::("arg0"), FunctionInfo::anonymous().with_arg::("arg0"), @@ -135,11 +135,11 @@ mod tests { ]), }; - map_a.merge(map_b).unwrap(); + let map = map_a.merge(map_b).unwrap(); - assert_eq!(map_a.functions, vec!['a', 'b', 'c', 'd', 'e', 'f']); + assert_eq!(map.functions, vec!['a', 'b', 'c', 'd', 'e', 'f']); assert_eq!( - map_a.indices, + map.indices, HashMap::from([ (ArgumentSignature::from_iter([Type::of::()]), 0), (ArgumentSignature::from_iter([Type::of::()]), 1), @@ -153,7 +153,7 @@ mod tests { #[test] fn should_return_error_on_duplicate_signature() { - let mut map_a = FunctionMap { + let map_a = FunctionMap { info: FunctionInfoType::Overloaded(Box::new([ FunctionInfo::anonymous().with_arg::("arg0"), FunctionInfo::anonymous().with_arg::("arg0"), @@ -181,9 +181,11 @@ mod tests { ]), }; - let result = map_a.merge(map_b); + let Err((map_a, error)) = map_a.merge(map_b) else { + panic!("expected an error"); + }; assert_eq!( - result.unwrap_err(), + error, FunctionOverloadError { signature: ArgumentSignature::from_iter([Type::of::()]) }