From cf33bdcb4c638789bcfd19012dca047fe9ad16c7 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 00:53:46 +0200 Subject: [PATCH 1/9] bevy_reflect: implement Reflect for SmolStr Prompted by winit's update https://github.com/bevyengine/bevy/pull/8745. --- crates/bevy_reflect/Cargo.toml | 15 ++- crates/bevy_reflect/src/impls/smol_str.rs | 134 ++++++++++++++++++++++ crates/bevy_reflect/src/lib.rs | 3 + 3 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 crates/bevy_reflect/src/impls/smol_str.rs diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index 2736045723d83..055e3149ef6f7 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -11,14 +11,18 @@ readme = "README.md" [features] default = [] -# Provides Bevy-related reflection implementations +# When enabled, Provides Bevy-related reflection implementations bevy = ["glam", "smallvec", "bevy_math"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] +# When enabled, allows to reflect SmolStr, which bevy can expose through the winit dependency. +winit = ["smol_str"] [dependencies] # bevy -bevy_math = { path = "../bevy_math", version = "0.11.0-dev", features = ["serialize"], optional = true } +bevy_math = { path = "../bevy_math", version = "0.11.0-dev", features = [ + "serialize", +], optional = true } bevy_reflect_derive = { path = "bevy_reflect_derive", version = "0.11.0-dev" } bevy_utils = { path = "../bevy_utils", version = "0.11.0-dev" } bevy_ptr = { path = "../bevy_ptr", version = "0.11.0-dev" } @@ -30,8 +34,13 @@ parking_lot = "0.12.1" thiserror = "1.0" once_cell = "1.11" serde = "1" -smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true } +smallvec = { version = "1.6", features = [ + "serde", + "union", + "const_generics", +], optional = true } glam = { version = "0.24", features = ["serde"], optional = true } +smol_str = { version = "0.2.0", optional = true } [dev-dependencies] ron = "0.8.0" diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs new file mode 100644 index 0000000000000..a0ffa1da1496d --- /dev/null +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -0,0 +1,134 @@ +use std::{ + any::Any, + hash::{Hash, Hasher}, +}; + +use smol_str::SmolStr; + +use crate::{ + utility::{reflect_hasher, NonGenericTypeInfoCell}, + FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize, ReflectFromPtr, + ReflectMut, ReflectOwned, ReflectRef, ReflectSerialize, TypeInfo, TypeRegistration, Typed, + ValueInfo, +}; + +impl Reflect for SmolStr { + fn type_name(&self) -> &str { + std::any::type_name::() + } + + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) + } + + fn into_any(self: Box) -> Box { + self + } + + fn as_any(&self) -> &dyn Any { + self + } + + fn as_any_mut(&mut self) -> &mut dyn Any { + self + } + + fn into_reflect(self: Box) -> Box { + self + } + + fn as_reflect(&self) -> &dyn Reflect { + self + } + + fn as_reflect_mut(&mut self) -> &mut dyn Reflect { + self + } + + fn apply(&mut self, value: &dyn Reflect) { + let value = value.as_any(); + if let Some(value) = value.downcast_ref::() { + *self = SmolStr::new(value); + } else { + panic!("Value is not a {}.", std::any::type_name::()); + } + } + + fn set(&mut self, value: Box) -> Result<(), Box> { + *self = value.take()?; + Ok(()) + } + + fn reflect_ref(&self) -> ReflectRef { + ReflectRef::Value(self) + } + + fn reflect_mut(&mut self) -> ReflectMut { + ReflectMut::Value(self) + } + + fn reflect_owned(self: Box) -> ReflectOwned { + ReflectOwned::Value(self) + } + + fn clone_value(&self) -> Box { + Box::new(self.clone()) + } + + fn reflect_hash(&self) -> Option { + let mut hasher = reflect_hasher(); + Hash::hash(&std::any::Any::type_id(self), &mut hasher); + Hash::hash(self, &mut hasher); + Some(hasher.finish()) + } + + fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option { + let value = value.as_any(); + if let Some(value) = value.downcast_ref::() { + Some(std::cmp::PartialEq::eq(self, value)) + } else { + Some(false) + } + } +} + +impl Typed for SmolStr { + fn type_info() -> &'static TypeInfo { + static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); + CELL.get_or_set(|| TypeInfo::Value(ValueInfo::new::())) + } +} + +impl GetTypeRegistration for SmolStr { + fn get_type_registration() -> TypeRegistration { + let mut registration = TypeRegistration::of::(); + #[cfg(feature = "serde")] + { + registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); + } + registration.insert::(FromType::::from_type()); + registration + } +} + +impl FromReflect for SmolStr { + fn from_reflect(reflect: &dyn crate::Reflect) -> Option { + Some(reflect.as_any().downcast_ref::()?.into()) + } +} + +#[cfg(test)] +mod tests { + use crate::Reflect; + use smol_str::SmolStr; + + #[test] + fn should_partial_eq_smolstr() { + let a: &dyn Reflect = &SmolStr::new("A"); + let b: &dyn Reflect = &SmolStr::new("A"); + let c: &dyn Reflect = &SmolStr::new("B"); + assert!(a.reflect_partial_eq(b).unwrap_or_default()); + assert!(!a.reflect_partial_eq(c).unwrap_or_default()); + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 13221427960d9..bda0fd48a5f30 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -459,6 +459,9 @@ mod impls { mod rect; #[cfg(feature = "smallvec")] mod smallvec; + #[cfg(feature = "smol_str")] + mod smol_str; + mod std; #[cfg(feature = "glam")] From 863314e4637c99d450507daa01233efd05cf0468 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 6 Jun 2023 21:28:13 -0400 Subject: [PATCH 2/9] Fix typo Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index 055e3149ef6f7..82c79c3a32e87 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -11,7 +11,7 @@ readme = "README.md" [features] default = [] -# When enabled, Provides Bevy-related reflection implementations +# When enabled, provides Bevy-related reflection implementations bevy = ["glam", "smallvec", "bevy_math"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] From 4ac35501f2c04368225613200b3894c72ba794ef Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 09:37:18 +0200 Subject: [PATCH 3/9] doc PR suggestion Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index 82c79c3a32e87..d8056b3486a82 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -15,7 +15,7 @@ default = [] bevy = ["glam", "smallvec", "bevy_math"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] -# When enabled, allows to reflect SmolStr, which bevy can expose through the winit dependency. +# When enabled, provides winit-related reflection implementations winit = ["smol_str"] [dependencies] From e3d0db05fd9a98f0759f3c9544cb721676f9a701 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 09:40:38 +0200 Subject: [PATCH 4/9] test cleanup Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/impls/smol_str.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index a0ffa1da1496d..738e64d773698 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -128,7 +128,7 @@ mod tests { let a: &dyn Reflect = &SmolStr::new("A"); let b: &dyn Reflect = &SmolStr::new("A"); let c: &dyn Reflect = &SmolStr::new("B"); - assert!(a.reflect_partial_eq(b).unwrap_or_default()); - assert!(!a.reflect_partial_eq(c).unwrap_or_default()); + assert_eq!(Some(true), a.reflect_partial_eq(b)); + assert_ne!(Some(false), a.reflect_partial_eq(c)); } } From 6faeef299779aec0f1bb77febad1bbac7eaf2d59 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 13:12:27 +0200 Subject: [PATCH 5/9] add TypePath + new test + fix inner representation to SmolStr --- crates/bevy_reflect/src/impls/smol_str.rs | 63 ++++++++++++++++++----- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index 738e64d773698..b5dc9442edc39 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -6,12 +6,13 @@ use std::{ use smol_str::SmolStr; use crate::{ - utility::{reflect_hasher, NonGenericTypeInfoCell}, - FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize, ReflectFromPtr, - ReflectMut, ReflectOwned, ReflectRef, ReflectSerialize, TypeInfo, TypeRegistration, Typed, - ValueInfo, + utility::{reflect_hasher, GenericTypePathCell, NonGenericTypeInfoCell}, + FromReflect, FromType, GetTypeRegistration, Reflect, ReflectFromPtr, ReflectMut, ReflectOwned, + ReflectRef, TypeInfo, TypePath, TypeRegistration, Typed, ValueInfo, }; +use crate::{ReflectDeserialize, ReflectSerialize}; + impl Reflect for SmolStr { fn type_name(&self) -> &str { std::any::type_name::() @@ -47,10 +48,17 @@ impl Reflect for SmolStr { fn apply(&mut self, value: &dyn Reflect) { let value = value.as_any(); - if let Some(value) = value.downcast_ref::() { + + if let Some(value) = value.downcast_ref::() { + *self = value.clone(); + } else if let Some(value) = value.downcast_ref::() { *self = SmolStr::new(value); } else { - panic!("Value is not a {}.", std::any::type_name::()); + panic!( + "Value is not a {} nor a {}.", + std::any::type_name::(), + std::any::type_name::() + ); } } @@ -99,6 +107,30 @@ impl Typed for SmolStr { } } +impl TypePath for SmolStr { + fn type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| "smol_str::SmolStr".to_owned()) + } + + fn short_type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| "SmolStr".to_owned()) + } + + fn type_ident() -> Option<&'static str> { + Some("SmolStr") + } + + fn crate_name() -> Option<&'static str> { + Some("smol_str") + } + + fn module_path() -> Option<&'static str> { + Some("SmolStr") + } +} + impl GetTypeRegistration for SmolStr { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); @@ -114,21 +146,28 @@ impl GetTypeRegistration for SmolStr { impl FromReflect for SmolStr { fn from_reflect(reflect: &dyn crate::Reflect) -> Option { - Some(reflect.as_any().downcast_ref::()?.into()) + Some(reflect.as_any().downcast_ref::()?.clone()) } } #[cfg(test)] mod tests { - use crate::Reflect; + use crate::{FromReflect, Reflect}; use smol_str::SmolStr; #[test] fn should_partial_eq_smolstr() { let a: &dyn Reflect = &SmolStr::new("A"); - let b: &dyn Reflect = &SmolStr::new("A"); - let c: &dyn Reflect = &SmolStr::new("B"); - assert_eq!(Some(true), a.reflect_partial_eq(b)); - assert_ne!(Some(false), a.reflect_partial_eq(c)); + let a2: &dyn Reflect = &SmolStr::new("A"); + let b: &dyn Reflect = &SmolStr::new("B"); + assert_eq!(Some(true), a.reflect_partial_eq(a2)); + assert_eq!(Some(false), a.reflect_partial_eq(b)); + } + + #[test] + fn smolstr_should_from_reflect() { + let smolstr = SmolStr::new("hello_world.rs"); + let output = ::from_reflect(&smolstr); + assert_eq!(Some(smolstr), output); } } From 8e26f032553b9ee4d48e2d6ab713cff4c982beae Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 17:30:37 +0200 Subject: [PATCH 6/9] removed serialization ; register FromReflect --- crates/bevy_reflect/src/impls/smol_str.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index b5dc9442edc39..8eeb2b191d81c 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -7,12 +7,10 @@ use smol_str::SmolStr; use crate::{ utility::{reflect_hasher, GenericTypePathCell, NonGenericTypeInfoCell}, - FromReflect, FromType, GetTypeRegistration, Reflect, ReflectFromPtr, ReflectMut, ReflectOwned, - ReflectRef, TypeInfo, TypePath, TypeRegistration, Typed, ValueInfo, + FromReflect, FromType, GetTypeRegistration, Reflect, ReflectFromPtr, ReflectFromReflect, + ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, TypeRegistration, Typed, ValueInfo, }; -use crate::{ReflectDeserialize, ReflectSerialize}; - impl Reflect for SmolStr { fn type_name(&self) -> &str { std::any::type_name::() @@ -134,12 +132,8 @@ impl TypePath for SmolStr { impl GetTypeRegistration for SmolStr { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); - #[cfg(feature = "serde")] - { - registration.insert::(FromType::::from_type()); - registration.insert::(FromType::::from_type()); - } registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); registration } } From 85ba7f54e7bf52096a7902fc9e2b64855e69bd21 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 7 Jun 2023 23:27:02 +0200 Subject: [PATCH 7/9] Update crates/bevy_reflect/src/impls/smol_str.rs Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/impls/smol_str.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index 8eeb2b191d81c..10cf0dd32910a 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -139,8 +139,8 @@ impl GetTypeRegistration for SmolStr { } impl FromReflect for SmolStr { - fn from_reflect(reflect: &dyn crate::Reflect) -> Option { - Some(reflect.as_any().downcast_ref::()?.clone()) + fn from_reflect(reflect: &dyn Reflect) -> Option { + Some(reflect.downcast_ref::()?.clone()) } } From 012bd90b3f7df8b135e316bc870241da9737f617 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Thu, 8 Jun 2023 09:40:38 +0200 Subject: [PATCH 8/9] aggregated feature smol_str to bevy, because SmolStr is exposed within bevy_input and needs Refrlect derives. --- crates/bevy_reflect/Cargo.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index d8056b3486a82..d7b193beed930 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -12,11 +12,9 @@ readme = "README.md" [features] default = [] # When enabled, provides Bevy-related reflection implementations -bevy = ["glam", "smallvec", "bevy_math"] +bevy = ["glam", "smallvec", "bevy_math", "smol_str"] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] -# When enabled, provides winit-related reflection implementations -winit = ["smol_str"] [dependencies] # bevy From cc73745471f4e1b012ccd7a1ac9cf4798e97798b Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Thu, 8 Jun 2023 09:41:39 +0200 Subject: [PATCH 9/9] use derive macros to simplify code and avoid silly manual mistakes We can revisit that when need be. --- crates/bevy_reflect/src/impls/smol_str.rs | 149 +--------------------- 1 file changed, 5 insertions(+), 144 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index 10cf0dd32910a..3dd617a06c711 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -1,148 +1,9 @@ -use std::{ - any::Any, - hash::{Hash, Hasher}, -}; +use crate::std_traits::ReflectDefault; +use crate::{self as bevy_reflect}; +use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value}; -use smol_str::SmolStr; - -use crate::{ - utility::{reflect_hasher, GenericTypePathCell, NonGenericTypeInfoCell}, - FromReflect, FromType, GetTypeRegistration, Reflect, ReflectFromPtr, ReflectFromReflect, - ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, TypeRegistration, Typed, ValueInfo, -}; - -impl Reflect for SmolStr { - fn type_name(&self) -> &str { - std::any::type_name::() - } - - fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { - Some(::type_info()) - } - - fn into_any(self: Box) -> Box { - self - } - - fn as_any(&self) -> &dyn Any { - self - } - - fn as_any_mut(&mut self) -> &mut dyn Any { - self - } - - fn into_reflect(self: Box) -> Box { - self - } - - fn as_reflect(&self) -> &dyn Reflect { - self - } - - fn as_reflect_mut(&mut self) -> &mut dyn Reflect { - self - } - - fn apply(&mut self, value: &dyn Reflect) { - let value = value.as_any(); - - if let Some(value) = value.downcast_ref::() { - *self = value.clone(); - } else if let Some(value) = value.downcast_ref::() { - *self = SmolStr::new(value); - } else { - panic!( - "Value is not a {} nor a {}.", - std::any::type_name::(), - std::any::type_name::() - ); - } - } - - fn set(&mut self, value: Box) -> Result<(), Box> { - *self = value.take()?; - Ok(()) - } - - fn reflect_ref(&self) -> ReflectRef { - ReflectRef::Value(self) - } - - fn reflect_mut(&mut self) -> ReflectMut { - ReflectMut::Value(self) - } - - fn reflect_owned(self: Box) -> ReflectOwned { - ReflectOwned::Value(self) - } - - fn clone_value(&self) -> Box { - Box::new(self.clone()) - } - - fn reflect_hash(&self) -> Option { - let mut hasher = reflect_hasher(); - Hash::hash(&std::any::Any::type_id(self), &mut hasher); - Hash::hash(self, &mut hasher); - Some(hasher.finish()) - } - - fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option { - let value = value.as_any(); - if let Some(value) = value.downcast_ref::() { - Some(std::cmp::PartialEq::eq(self, value)) - } else { - Some(false) - } - } -} - -impl Typed for SmolStr { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Value(ValueInfo::new::())) - } -} - -impl TypePath for SmolStr { - fn type_path() -> &'static str { - static CELL: GenericTypePathCell = GenericTypePathCell::new(); - CELL.get_or_insert::(|| "smol_str::SmolStr".to_owned()) - } - - fn short_type_path() -> &'static str { - static CELL: GenericTypePathCell = GenericTypePathCell::new(); - CELL.get_or_insert::(|| "SmolStr".to_owned()) - } - - fn type_ident() -> Option<&'static str> { - Some("SmolStr") - } - - fn crate_name() -> Option<&'static str> { - Some("smol_str") - } - - fn module_path() -> Option<&'static str> { - Some("SmolStr") - } -} - -impl GetTypeRegistration for SmolStr { - fn get_type_registration() -> TypeRegistration { - let mut registration = TypeRegistration::of::(); - registration.insert::(FromType::::from_type()); - registration.insert::(FromType::::from_type()); - registration - } -} - -impl FromReflect for SmolStr { - fn from_reflect(reflect: &dyn Reflect) -> Option { - Some(reflect.downcast_ref::()?.clone()) - } -} +impl_reflect_value!(::smol_str::SmolStr(Debug, Hash, PartialEq, Default)); +impl_from_reflect_value!(::smol_str::SmolStr); #[cfg(test)] mod tests {