Skip to content

Commit

Permalink
bevy_reflect: Get owned fields (#5728)
Browse files Browse the repository at this point in the history
# Objective

Sometimes it's useful to be able to retrieve all the fields of a container type so that they may be processed separately. With reflection, however, we typically only have access to references.

The only alternative is to "clone" the value using `Reflect::clone_value`. This, however, returns a Dynamic type in most cases. The solution there would be to use `FromReflect` instead, but this also has a problem in that it means we need to add `FromReflect` as an additional bound.

## Solution

Add a `drain` method to all container traits. This returns a `Vec<Box<dyn Reflect>>` (except for `Map` which returns `Vec<(Box<dyn Reflect>, Box<dyn Reflect>)>`).

This allows us to do things a lot simpler. For example, if we finished processing a struct and just need a particular value:

```rust
// === OLD === //
/// May or may not return a Dynamic*** value (even if `container` wasn't a `DynamicStruct`)
fn get_output(container: Box<dyn Struct>, output_index: usize) -> Box<dyn Reflect> {
  container.field_at(output_index).unwrap().clone_value()
}

// === NEW === //
/// Returns _exactly_ whatever was in the given struct
fn get_output(container: Box<dyn Struct>, output_index: usize) -> Box<dyn Reflect> {
  container.drain().remove(output_index).unwrap()
}
```

### Discussion

* Is `drain` the best method name? It makes sense that it "drains" all the fields and that it consumes the container in the process, but I'm open to alternatives.

---

## Changelog

* Added a `drain` method to the following traits:
  * `Struct`
  * `TupleStruct`
  * `Tuple`
  * `Array`
  * `List`
  * `Map`
  * `Enum`
  • Loading branch information
MrGVSV committed Aug 30, 2022
1 parent bb2303a commit ecc584f
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 0 deletions.
7 changes: 7 additions & 0 deletions crates/bevy_reflect/src/array.rs
Expand Up @@ -30,6 +30,8 @@ pub trait Array: Reflect {
}
/// Returns an iterator over the collection.
fn iter(&self) -> ArrayIter;
/// Drain the elements of this array to get a vector of owned values.
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>>;

fn clone_dynamic(&self) -> DynamicArray {
DynamicArray {
Expand Down Expand Up @@ -246,6 +248,11 @@ impl Array for DynamicArray {
}
}

#[inline]
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.values.into_vec()
}

#[inline]
fn clone_dynamic(&self) -> DynamicArray {
DynamicArray {
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_reflect/src/impls/smallvec.rs
Expand Up @@ -37,6 +37,12 @@ where
index: 0,
}
}

fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.into_iter()
.map(|value| Box::new(value) as Box<dyn Reflect>)
.collect()
}
}

impl<T: smallvec::Array + Send + Sync + 'static> List for SmallVec<T>
Expand Down
25 changes: 25 additions & 0 deletions crates/bevy_reflect/src/impls/std.rs
Expand Up @@ -123,6 +123,13 @@ impl<T: FromReflect> Array for Vec<T> {
index: 0,
}
}

#[inline]
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.into_iter()
.map(|value| Box::new(value) as Box<dyn Reflect>)
.collect()
}
}

impl<T: FromReflect> List for Vec<T> {
Expand Down Expand Up @@ -261,6 +268,17 @@ impl<K: FromReflect + Eq + Hash, V: FromReflect> Map for HashMap<K, V> {
}
}

fn drain(self: Box<Self>) -> Vec<(Box<dyn Reflect>, Box<dyn Reflect>)> {
self.into_iter()
.map(|(key, value)| {
(
Box::new(key) as Box<dyn Reflect>,
Box::new(value) as Box<dyn Reflect>,
)
})
.collect()
}

fn clone_dynamic(&self) -> DynamicMap {
let mut dynamic_map = DynamicMap::default();
dynamic_map.set_name(self.type_name().to_string());
Expand Down Expand Up @@ -409,6 +427,13 @@ impl<T: Reflect, const N: usize> Array for [T; N] {
index: 0,
}
}

#[inline]
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.into_iter()
.map(|value| Box::new(value) as Box<dyn Reflect>)
.collect()
}
}

impl<T: Reflect, const N: usize> Reflect for [T; N] {
Expand Down
23 changes: 23 additions & 0 deletions crates/bevy_reflect/src/lib.rs
Expand Up @@ -519,6 +519,29 @@ mod tests {
assert_eq!(foo, *foo2.downcast::<Foo>().unwrap());
}

#[test]
fn should_drain_fields() {
let array_value: Box<dyn Array> = Box::new([123_i32, 321_i32]);
let fields = array_value.drain();
assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());

let list_value: Box<dyn List> = Box::new(vec![123_i32, 321_i32]);
let fields = list_value.drain();
assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());

let tuple_value: Box<dyn Tuple> = Box::new((123_i32, 321_i32));
let fields = tuple_value.drain();
assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());

let map_value: Box<dyn Map> = Box::new(HashMap::from([(123_i32, 321_i32)]));
let fields = map_value.drain();
assert!(fields[0].0.reflect_partial_eq(&123_i32).unwrap_or_default());
assert!(fields[0].1.reflect_partial_eq(&321_i32).unwrap_or_default());
}

#[test]
fn reflect_take() {
#[derive(Reflect, Debug, PartialEq)]
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/src/list.rs
Expand Up @@ -137,6 +137,10 @@ impl Array for DynamicList {
}
}

fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.values
}

fn clone_dynamic(&self) -> DynamicArray {
DynamicArray {
name: self.name.clone(),
Expand Down
7 changes: 7 additions & 0 deletions crates/bevy_reflect/src/map.rs
Expand Up @@ -42,6 +42,9 @@ pub trait Map: Reflect {
/// Returns an iterator over the key-value pairs of the map.
fn iter(&self) -> MapIter;

/// Drain the key-value pairs of this map to get a vector of owned values.
fn drain(self: Box<Self>) -> Vec<(Box<dyn Reflect>, Box<dyn Reflect>)>;

/// Clones the map, producing a [`DynamicMap`].
fn clone_dynamic(&self) -> DynamicMap;

Expand Down Expand Up @@ -226,6 +229,10 @@ impl Map for DynamicMap {
}
}
}

fn drain(self: Box<Self>) -> Vec<(Box<dyn Reflect>, Box<dyn Reflect>)> {
self.values
}
}

impl Reflect for DynamicMap {
Expand Down
15 changes: 15 additions & 0 deletions crates/bevy_reflect/src/tuple.rs
Expand Up @@ -40,6 +40,9 @@ pub trait Tuple: Reflect {
/// Returns an iterator over the values of the tuple's fields.
fn iter_fields(&self) -> TupleFieldIter;

/// Drain the fields of this tuple to get a vector of owned values.
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>>;

/// Clones the struct into a [`DynamicTuple`].
fn clone_dynamic(&self) -> DynamicTuple;
}
Expand Down Expand Up @@ -253,6 +256,11 @@ impl Tuple for DynamicTuple {
}
}

#[inline]
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
self.fields
}

#[inline]
fn clone_dynamic(&self) -> DynamicTuple {
DynamicTuple {
Expand Down Expand Up @@ -451,6 +459,13 @@ macro_rules! impl_reflect_tuple {
}
}

#[inline]
fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
vec![
$(Box::new(self.$index),)*
]
}

#[inline]
fn clone_dynamic(&self) -> DynamicTuple {
let mut dyn_tuple = DynamicTuple {
Expand Down

0 comments on commit ecc584f

Please sign in to comment.