From 0a50c2a0cb21b270bdb0edf5dd201c0e09d8c891 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 1 Dec 2024 15:53:54 -0700 Subject: [PATCH] Add and clean up internal representation --- .../bevy_reflect/src/func/dynamic_function.rs | 93 ++---- ...on_map.rs => dynamic_function_internal.rs} | 277 ++++++++++++------ .../src/func/dynamic_function_mut.rs | 80 ++--- crates/bevy_reflect/src/func/mod.rs | 2 +- 4 files changed, 242 insertions(+), 210 deletions(-) rename crates/bevy_reflect/src/func/{function_map.rs => dynamic_function_internal.rs} (72%) diff --git a/crates/bevy_reflect/src/func/dynamic_function.rs b/crates/bevy_reflect/src/func/dynamic_function.rs index de780f32ae..265cf3b7db 100644 --- a/crates/bevy_reflect/src/func/dynamic_function.rs +++ b/crates/bevy_reflect/src/func/dynamic_function.rs @@ -2,9 +2,9 @@ use crate::{ self as bevy_reflect, __macro_exports::RegisterForReflection, func::{ - args::ArgList, function_map::FunctionMap, info::FunctionInfoType, - signature::ArgumentSignature, DynamicFunctionMut, Function, FunctionError, - FunctionOverloadError, FunctionResult, IntoFunction, IntoFunctionMut, + args::ArgList, dynamic_function_internal::DynamicFunctionInternal, info::FunctionInfoType, + signature::ArgumentSignature, DynamicFunctionMut, Function, FunctionOverloadError, + FunctionResult, IntoFunction, IntoFunctionMut, }, serde::Serializable, ApplyError, MaybeTyped, PartialReflect, Reflect, ReflectKind, ReflectMut, ReflectOwned, @@ -66,9 +66,9 @@ type ArcFn<'env> = Arc Fn(ArgList<'a>) -> FunctionResult<'a> + Send /// /// [`ReflectFn`]: crate::func::ReflectFn /// [module-level documentation]: crate::func +#[derive(Clone)] pub struct DynamicFunction<'env> { - pub(super) name: Option>, - pub(super) function_map: FunctionMap>, + pub(super) internal: DynamicFunctionInternal>, } impl<'env> DynamicFunction<'env> { @@ -92,25 +92,8 @@ impl<'env> DynamicFunction<'env> { func: F, info: impl TryInto, Error: Debug>, ) -> Self { - let info = info.try_into().unwrap(); - - let func: ArcFn = Arc::new(func); - Self { - name: match &info { - FunctionInfoType::Standard(info) => info.name().cloned(), - FunctionInfoType::Overloaded(_) => None, - }, - function_map: match info { - FunctionInfoType::Standard(info) => FunctionMap::Single(func, info.into_owned()), - FunctionInfoType::Overloaded(infos) => { - let indices = infos - .iter() - .map(|info| (ArgumentSignature::from(info), 0)) - .collect(); - FunctionMap::Overloaded(vec![func], infos.into_owned(), indices) - } - }, + internal: DynamicFunctionInternal::new(Arc::new(func), info.try_into().unwrap()), } } @@ -123,7 +106,7 @@ impl<'env> DynamicFunction<'env> { /// /// [`DynamicFunctions`]: DynamicFunction pub fn with_name(mut self, name: impl Into>) -> Self { - self.name = Some(name.into()); + self.internal.set_name(Some(name.into())); self } @@ -233,16 +216,14 @@ impl<'env> DynamicFunction<'env> { 'env: 'a, { let function = function.into_function(); - - let name = self.name.clone(); - let function_map = self - .function_map - .merge(function.function_map) + let internal = self + .internal + .merge(function.internal) .unwrap_or_else(|(_, err)| { panic!("{}", err); }); - DynamicFunction { name, function_map } + DynamicFunction { internal } } /// Attempt to add an overload to this function. @@ -259,14 +240,11 @@ impl<'env> DynamicFunction<'env> { ) -> 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(( + match self.internal.merge(function.internal) { + Ok(internal) => Ok(Self { internal }), + Err((internal, err)) => Err(( Box::new(Self { - name, - function_map: *function_map, + internal: *internal, }), err, )), @@ -297,23 +275,14 @@ impl<'env> DynamicFunction<'env> { /// /// The function itself may also return any errors it needs to. pub fn call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> { - let expected_arg_count = self.function_map.info().arg_count(); - let received_arg_count = args.len(); - - if !expected_arg_count.contains(&received_arg_count) { - Err(FunctionError::ArgCountMismatch { - expected: expected_arg_count, - received: received_arg_count, - }) - } else { - let func = self.function_map.get(&args)?; - func(args) - } + self.internal.validate_args(&args)?; + let func = self.internal.get(&args)?; + func(args) } /// Returns the function info. pub fn info(&self) -> FunctionInfoType { - self.function_map.info() + self.internal.info() } /// The name of the function. @@ -331,7 +300,7 @@ impl<'env> DynamicFunction<'env> { /// [`with_name`]: Self::with_name /// [overloaded]: Self::with_overload pub fn name(&self) -> Option<&Cow<'static, str>> { - self.name.as_ref() + self.internal.name() } /// Returns `true` if the function is [overloaded]. @@ -349,7 +318,7 @@ impl<'env> DynamicFunction<'env> { /// /// [overloaded]: Self::with_overload pub fn is_overloaded(&self) -> bool { - self.function_map.is_overloaded() + self.internal.is_overloaded() } /// Returns the number of arguments the function expects. @@ -372,17 +341,17 @@ impl<'env> DynamicFunction<'env> { /// /// [overloaded]: Self::with_overload pub fn arg_count(&self) -> RangeInclusive { - self.function_map.arg_count() + self.internal.arg_count() } } impl Function for DynamicFunction<'static> { fn name(&self) -> Option<&Cow<'static, str>> { - self.name.as_ref() + self.internal.name() } fn info(&self) -> FunctionInfoType { - self.function_map.info() + self.internal.info() } fn reflect_call<'a>(&self, args: ArgList<'a>) -> FunctionResult<'a> { @@ -494,19 +463,7 @@ impl_type_path!((in bevy_reflect) DynamicFunction<'env>); /// [overloaded]: DynamicFunction::with_overload impl<'env> Debug for DynamicFunction<'env> { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - let name = self.name().unwrap_or(&Cow::Borrowed("_")); - write!(f, "DynamicFunction(fn {name}")?; - self.function_map.debug(f)?; - write!(f, ")") - } -} - -impl<'env> Clone for DynamicFunction<'env> { - fn clone(&self) -> Self { - Self { - name: self.name.clone(), - function_map: self.function_map.clone(), - } + write!(f, "DynamicFunction({:?})", &self.internal) } } diff --git a/crates/bevy_reflect/src/func/function_map.rs b/crates/bevy_reflect/src/func/dynamic_function_internal.rs similarity index 72% rename from crates/bevy_reflect/src/func/function_map.rs rename to crates/bevy_reflect/src/func/dynamic_function_internal.rs index fb0edc1cb0..8048561152 100644 --- a/crates/bevy_reflect/src/func/function_map.rs +++ b/crates/bevy_reflect/src/func/dynamic_function_internal.rs @@ -8,6 +8,199 @@ use bevy_utils::hashbrown::HashMap; use core::fmt::{Debug, Formatter}; use core::ops::RangeInclusive; +/// An internal structure for storing a function and its corresponding [function information]. +/// +/// This is used to facilitate the sharing of functionality between [`DynamicFunction`] +/// and [`DynamicFunctionMut`]. +/// +/// [function information]: FunctionInfo +/// [`DynamicFunction`]: crate::func::DynamicFunction +/// [`DynamicFunctionMut`]: crate::func::DynamicFunctionMut +#[derive(Clone)] +pub(super) struct DynamicFunctionInternal { + name: Option>, + map: FunctionMap, +} + +impl DynamicFunctionInternal { + /// Create a new instance of [`DynamicFunctionInternal`] with the given function + /// and its corresponding information. + pub fn new(func: F, info: FunctionInfoType<'static>) -> Self { + Self { + name: match &info { + FunctionInfoType::Standard(info) => info.name().cloned(), + FunctionInfoType::Overloaded(_) => None, + }, + map: match info { + FunctionInfoType::Standard(info) => FunctionMap::Single(func, info.into_owned()), + FunctionInfoType::Overloaded(infos) => { + let indices = infos + .iter() + .map(|info| (ArgumentSignature::from(info), 0)) + .collect(); + FunctionMap::Overloaded(vec![func], infos.into_owned(), indices) + } + }, + } + } + + /// Sets the name of the function. + pub fn set_name(&mut self, name: Option>) { + self.name = name; + } + + /// The name of the function. + pub fn name(&self) -> Option<&Cow<'static, str>> { + self.name.as_ref() + } + + /// Returns `true` if the function is overloaded. + pub fn is_overloaded(&self) -> bool { + matches!(self.map, FunctionMap::Overloaded(..)) + } + + /// Get an immutable reference to the function. + /// + /// If the function is not overloaded, it will always be returned regardless of the arguments. + /// Otherwise, the function will be selected based on the arguments provided. + /// + /// If no overload matches the provided arguments, returns [`FunctionError::NoOverload`]. + pub fn get(&self, args: &ArgList) -> Result<&F, FunctionError> { + match &self.map { + FunctionMap::Single(function, _) => Ok(function), + FunctionMap::Overloaded(functions, _, indices) => { + let signature = ArgumentSignature::from(args); + indices + .get(&signature) + .map(|index| &functions[*index]) + .ok_or_else(|| FunctionError::NoOverload { + expected: indices.keys().cloned().collect(), + received: signature, + }) + } + } + } + + /// Get an mutable reference to the function. + /// + /// If the function is not overloaded, it will always be returned regardless of the arguments. + /// Otherwise, the function will be selected based on the arguments provided. + /// + /// If no overload matches the provided arguments, returns [`FunctionError::NoOverload`]. + pub fn get_mut(&mut self, args: &ArgList) -> Result<&mut F, FunctionError> { + match &mut self.map { + FunctionMap::Single(function, _) => Ok(function), + FunctionMap::Overloaded(functions, _, indices) => { + let signature = ArgumentSignature::from(args); + indices + .get(&signature) + .map(|index| &mut functions[*index]) + .ok_or_else(|| FunctionError::NoOverload { + expected: indices.keys().cloned().collect(), + received: signature, + }) + } + } + } + + /// Returns the function information contained in the map. + #[inline] + pub fn info(&self) -> FunctionInfoType { + match &self.map { + FunctionMap::Single(_, info) => FunctionInfoType::Standard(Cow::Borrowed(info)), + FunctionMap::Overloaded(_, info, _) => { + FunctionInfoType::Overloaded(Cow::Borrowed(info)) + } + } + } + + /// Returns the number of arguments the function expects. + /// + /// For[overloaded functions that can have a variable number of arguments, + /// this will return the minimum and maximum number of arguments. + /// + /// Otherwise, the range will have the same start and end. + pub fn arg_count(&self) -> RangeInclusive { + self.info().arg_count() + } + + /// Helper method for validating that a given set of arguments are _potentially_ valid for this function. + /// + /// Currently, this validates: + /// - The number of arguments is within the expected range + pub fn validate_args(&self, args: &ArgList) -> Result<(), FunctionError> { + let expected_arg_count = self.arg_count(); + let received_arg_count = args.len(); + + if !expected_arg_count.contains(&received_arg_count) { + Err(FunctionError::ArgCountMismatch { + expected: expected_arg_count, + received: received_arg_count, + }) + } else { + Ok(()) + } + } + + /// Merge another [`DynamicFunctionInternal`] into this one. + /// + /// If `other` contains any functions with the same signature as this one, + /// an error will be returned along with the original, unchanged instance. + /// + /// Therefore, this method should always return an overloaded function if the merge is successful. + /// + /// Additionally, if the merge succeeds, it should be guaranteed that the order + /// of the functions in the map will be preserved. + /// For example, merging `[func_a, func_b]` (self) with `[func_c, func_d]` (other) should result in + /// `[func_a, func_b, func_c, func_d]`. + /// And merging `[func_c, func_d]` (self) with `[func_a, func_b]` (other) should result in + /// `[func_c, func_d, func_a, func_b]`. + pub fn merge(self, other: Self) -> Result, FunctionOverloadError)> { + let map = self.map.merge(other.map).map_err(|(map, err)| { + ( + Box::new(Self { + name: self.name.clone(), + map: *map, + }), + err, + ) + })?; + + Ok(Self { + name: self.name, + map, + }) + } + + /// Convert the inner [`FunctionMap`] from holding `F` to holding `G`. + pub fn convert(self, f: fn(FunctionMap) -> FunctionMap) -> DynamicFunctionInternal { + DynamicFunctionInternal { + name: self.name, + map: f(self.map), + } + } +} + +impl Debug for DynamicFunctionInternal { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + let name = self.name.as_deref().unwrap_or("_"); + write!(f, "fn {name}")?; + + match &self.map { + // `(arg0: i32, arg1: i32) -> ()` + FunctionMap::Single(_, info) => PrettyPrintFunctionInfo::new(info).fmt(f), + // `{(arg0: i32, arg1: i32) -> (), (arg0: f32, arg1: f32) -> ()}` + FunctionMap::Overloaded(_, infos, _) => { + let mut set = f.debug_set(); + for info in infos.iter() { + set.entry(&PrettyPrintFunctionInfo::new(info)); + } + set.finish() + } + } + } +} + /// A helper type for storing a mapping of overloaded functions /// along with the corresponding [function information]. /// @@ -36,75 +229,6 @@ pub(super) enum FunctionMap { } impl FunctionMap { - /// Returns `true` if the map contains an overloaded function. - pub fn is_overloaded(&self) -> bool { - matches!(self, Self::Overloaded(..)) - } - - /// Get a reference to a function in the map. - /// - /// If there is only one function in the map, it will be returned. - /// Otherwise, the function will be selected based on the arguments provided. - /// - /// If no overload matches the provided arguments, an error will be returned. - pub fn get(&self, args: &ArgList) -> Result<&F, FunctionError> { - match self { - Self::Single(function, _) => Ok(function), - Self::Overloaded(functions, _, indices) => { - let signature = ArgumentSignature::from(args); - indices - .get(&signature) - .map(|index| &functions[*index]) - .ok_or_else(|| FunctionError::NoOverload { - expected: indices.keys().cloned().collect(), - received: signature, - }) - } - } - } - - /// Get a mutable reference to a function in the map. - /// - /// If there is only one function in the map, it will be returned. - /// Otherwise, the function will be selected based on the arguments provided. - /// - /// If no overload matches the provided arguments, an error will be returned. - pub fn get_mut(&mut self, args: &ArgList) -> Result<&mut F, FunctionError> { - match self { - Self::Single(function, _) => Ok(function), - Self::Overloaded(functions, _, indices) => { - let signature = ArgumentSignature::from(args); - indices - .get(&signature) - .map(|index| &mut functions[*index]) - .ok_or_else(|| FunctionError::NoOverload { - expected: indices.keys().cloned().collect(), - received: signature, - }) - } - } - } - - /// Returns the function information contained in the map. - pub fn info(&self) -> FunctionInfoType { - match self { - Self::Single(_, info) => FunctionInfoType::Standard(Cow::Borrowed(info)), - Self::Overloaded(_, info, _) => FunctionInfoType::Overloaded(Cow::Borrowed(info)), - } - } - - /// Returns the number of arguments the function expects. - /// - /// For [overloaded] functions that can have a variable number of arguments, - /// this will return the minimum and maximum number of arguments. - /// - /// Otherwise, the range will have the same start and end. - /// - /// [overloaded]: Self::Overloaded - pub fn arg_count(&self) -> RangeInclusive { - self.info().arg_count() - } - /// Merge another [`FunctionMap`] into this one. /// /// If the other map contains any functions with the same signature as this one, @@ -220,21 +344,6 @@ impl FunctionMap { } } } - - pub fn debug(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - match self { - // `(arg0: i32, arg1: i32) -> ()` - Self::Single(_, info) => PrettyPrintFunctionInfo::new(info).fmt(f), - // `{(arg0: i32, arg1: i32) -> (), (arg0: f32, arg1: f32) -> ()}` - Self::Overloaded(_, infos, _) => { - let mut set = f.debug_set(); - for info in infos.iter() { - set.entry(&PrettyPrintFunctionInfo::new(info)); - } - set.finish() - } - } - } } #[cfg(test)] diff --git a/crates/bevy_reflect/src/func/dynamic_function_mut.rs b/crates/bevy_reflect/src/func/dynamic_function_mut.rs index c6bce33145..4c94d8c745 100644 --- a/crates/bevy_reflect/src/func/dynamic_function_mut.rs +++ b/crates/bevy_reflect/src/func/dynamic_function_mut.rs @@ -5,8 +5,9 @@ use core::ops::RangeInclusive; #[cfg(not(feature = "std"))] use alloc::{boxed::Box, format, vec}; +use crate::func::dynamic_function_internal::FunctionMap; use crate::func::{ - args::ArgList, function_map::FunctionMap, signature::ArgumentSignature, DynamicFunction, + args::ArgList, dynamic_function_internal::DynamicFunctionInternal, DynamicFunction, FunctionError, FunctionInfoType, FunctionOverloadError, FunctionResult, IntoFunctionMut, }; @@ -70,8 +71,7 @@ type BoxFnMut<'env> = Box FnMut(ArgList<'a>) -> FunctionResult<'a> + /// [`ReflectFnMut`]: crate::func::ReflectFnMut /// [module-level documentation]: crate::func pub struct DynamicFunctionMut<'env> { - name: Option>, - function_map: FunctionMap>, + internal: DynamicFunctionInternal>, } impl<'env> DynamicFunctionMut<'env> { @@ -95,25 +95,8 @@ impl<'env> DynamicFunctionMut<'env> { func: F, info: impl TryInto, Error: Debug>, ) -> Self { - let info = info.try_into().unwrap(); - - let func: BoxFnMut = Box::new(func); - Self { - name: match &info { - FunctionInfoType::Standard(info) => info.name().cloned(), - FunctionInfoType::Overloaded(_) => None, - }, - function_map: match info { - FunctionInfoType::Standard(info) => FunctionMap::Single(func, info.into_owned()), - FunctionInfoType::Overloaded(infos) => { - let indices = infos - .iter() - .map(|info| (ArgumentSignature::from(info), 0)) - .collect(); - FunctionMap::Overloaded(vec![func], infos.into_owned(), indices) - } - }, + internal: DynamicFunctionInternal::new(Box::new(func), info.try_into().unwrap()), } } @@ -126,7 +109,7 @@ impl<'env> DynamicFunctionMut<'env> { /// /// [`DynamicFunctionMuts`]: DynamicFunctionMut pub fn with_name(mut self, name: impl Into>) -> Self { - self.name = Some(name.into()); + self.internal.set_name(Some(name.into())); self } @@ -196,15 +179,14 @@ impl<'env> DynamicFunctionMut<'env> { { let function = function.into_function_mut(); - let name = self.name.clone(); - let function_map = self - .function_map - .merge(function.function_map) + let internal = self + .internal + .merge(function.internal) .unwrap_or_else(|(_, err)| { panic!("{}", err); }); - DynamicFunctionMut { name, function_map } + DynamicFunctionMut { internal } } /// Attempt to add an overload to this function. @@ -221,14 +203,11 @@ impl<'env> DynamicFunctionMut<'env> { ) -> 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(( + match self.internal.merge(function.internal) { + Ok(internal) => Ok(Self { internal }), + Err((internal, err)) => Err(( Box::new(Self { - name, - function_map: *function_map, + internal: *internal, }), err, )), @@ -267,18 +246,9 @@ impl<'env> DynamicFunctionMut<'env> { /// /// [`call_once`]: DynamicFunctionMut::call_once pub fn call<'a>(&mut self, args: ArgList<'a>) -> FunctionResult<'a> { - let expected_arg_count = self.function_map.info().arg_count(); - let received_arg_count = args.len(); - - if !expected_arg_count.contains(&received_arg_count) { - Err(FunctionError::ArgCountMismatch { - expected: expected_arg_count, - received: received_arg_count, - }) - } else { - let func = self.function_map.get_mut(&args)?; - func(args) - } + self.internal.validate_args(&args)?; + let func = self.internal.get_mut(&args)?; + func(args) } /// Call the function with the given arguments and consume it. @@ -315,7 +285,7 @@ impl<'env> DynamicFunctionMut<'env> { /// Returns the function info. pub fn info(&self) -> FunctionInfoType { - self.function_map.info() + self.internal.info() } /// The name of the function. @@ -330,7 +300,7 @@ impl<'env> DynamicFunctionMut<'env> { /// [`DynamicFunctionMuts`]: DynamicFunctionMut /// [`with_name`]: Self::with_name pub fn name(&self) -> Option<&Cow<'static, str>> { - self.name.as_ref() + self.internal.name() } /// Returns `true` if the function is [overloaded]. @@ -350,7 +320,7 @@ impl<'env> DynamicFunctionMut<'env> { /// /// [overloaded]: Self::with_overload pub fn is_overloaded(&self) -> bool { - self.function_map.is_overloaded() + self.internal.is_overloaded() } /// Returns the number of arguments the function expects. @@ -373,7 +343,7 @@ impl<'env> DynamicFunctionMut<'env> { /// /// [overloaded]: Self::with_overload pub fn arg_count(&self) -> RangeInclusive { - self.function_map.arg_count() + self.internal.arg_count() } } @@ -389,10 +359,7 @@ impl<'env> DynamicFunctionMut<'env> { /// [overloaded]: DynamicFunctionMut::with_overload impl<'env> Debug for DynamicFunctionMut<'env> { fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { - let name = self.name().unwrap_or(&Cow::Borrowed("_")); - write!(f, "DynamicFunctionMut(fn {name}")?; - self.function_map.debug(f)?; - write!(f, ")") + write!(f, "DynamicFunctionMut({:?})", &self.internal) } } @@ -400,15 +367,14 @@ impl<'env> From> for DynamicFunctionMut<'env> { #[inline] fn from(function: DynamicFunction<'env>) -> Self { Self { - name: function.name, - function_map: match function.function_map { + internal: function.internal.convert(|map| match map { FunctionMap::Single(func, info) => FunctionMap::Single(arc_to_box(func), info), FunctionMap::Overloaded(funcs, infos, indices) => FunctionMap::Overloaded( funcs.into_iter().map(arc_to_box).collect(), infos, indices, ), - }, + }), } } } diff --git a/crates/bevy_reflect/src/func/mod.rs b/crates/bevy_reflect/src/func/mod.rs index 6545849d80..c5f6f2e657 100644 --- a/crates/bevy_reflect/src/func/mod.rs +++ b/crates/bevy_reflect/src/func/mod.rs @@ -173,10 +173,10 @@ pub use return_type::*; pub mod args; mod dynamic_function; +mod dynamic_function_internal; mod dynamic_function_mut; mod error; mod function; -mod function_map; mod info; mod into_function; mod into_function_mut;