Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - impl Reflect for &'static Path #6755

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions crates/bevy_reflect/src/impls/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use bevy_utils::{Duration, Instant};
use bevy_utils::{HashMap, HashSet};
#[cfg(any(unix, windows))]
use std::ffi::OsString;
use std::path::Path;
use std::{
any::Any,
borrow::Cow,
Expand Down Expand Up @@ -745,6 +746,86 @@ impl Reflect for Cow<'static, str> {
}
}

impl Reflect for &'static Path {
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
}

fn get_type_info(&self) -> &'static TypeInfo {
<Self as Typed>::type_info()
}

fn into_any(self: Box<Self>) -> Box<dyn Any> {
self
}

fn as_any(&self) -> &dyn Any {
self
}

fn as_any_mut(&mut self) -> &mut dyn Any {
self
}

fn into_reflect(self: Box<Self>) -> Box<dyn Reflect> {
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>() {
*self = value;
} else {
panic!("Value is not a {}.", std::any::type_name::<Self>());
}
}

fn set(&mut self, value: Box<dyn Reflect>) -> Result<(), Box<dyn Reflect>> {
*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<Self>) -> ReflectOwned {
ReflectOwned::Value(self)
}

fn clone_value(&self) -> Box<dyn Reflect> {
Box::new(*self)
}

fn reflect_hash(&self) -> Option<u64> {
let mut hasher = crate::ReflectHasher::default();
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<bool> {
let value = value.as_any();
if let Some(value) = value.downcast_ref::<Self>() {
Some(std::cmp::PartialEq::eq(self, value))
} else {
Some(false)
}
}
}

impl<T: FromReflect> GetTypeRegistration for Option<T> {
fn get_type_registration() -> TypeRegistration {
TypeRegistration::of::<Option<T>>()
Expand Down Expand Up @@ -1019,6 +1100,27 @@ impl FromReflect for Cow<'static, str> {
}
}

impl Typed for &'static Path {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these impls separated from the Reflect impl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, I just copied how the other impls in the file are organized.

Copy link
Member

@MrGVSV MrGVSV Nov 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh lol looks like I someone messed up in placing the Option<T> impls 🤦

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to leave it for now. I'll follow up with a quick cleanup PR!

fn type_info() -> &'static TypeInfo {
static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new();
CELL.get_or_set(|| TypeInfo::Value(ValueInfo::new::<Self>()))
}
}

impl GetTypeRegistration for &'static Path {
fn get_type_registration() -> TypeRegistration {
let mut registration = TypeRegistration::of::<Self>();
registration.insert::<ReflectFromPtr>(FromType::<Self>::from_type());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other registrations we should include here? I think we could do ReflectSerialize but I'm not sure ReflectDeserialize is possible for a reference (?) so maybe we shouldn't include either? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I don't think there is anything else to register apart from those. So this is probably fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make more sense to support serialization for Cow<'static, Path> instead.

registration
}
}

impl FromReflect for &'static Path {
fn from_reflect(reflect: &dyn crate::Reflect) -> Option<Self> {
reflect.as_any().downcast_ref::<Self>().copied()
}
}

#[cfg(test)]
mod tests {
use crate as bevy_reflect;
Expand All @@ -1029,6 +1131,7 @@ mod tests {
use bevy_utils::HashMap;
use bevy_utils::{Duration, Instant};
use std::f32::consts::{PI, TAU};
use std::path::Path;

#[test]
fn can_serialize_duration() {
Expand Down Expand Up @@ -1228,4 +1331,11 @@ mod tests {
let output = <Instant as FromReflect>::from_reflect(&expected).unwrap();
assert_eq!(expected, output);
}

#[test]
fn path_should_from_reflect() {
let path = Path::new("hello_world.rs");
let output = <&'static Path as FromReflect>::from_reflect(&path).unwrap();
assert_eq!(path, output);
}
}