Skip to content

Commit

Permalink
bevy_reflect: IntoIter for DynamicList and DynamicMap (#4108)
Browse files Browse the repository at this point in the history
# Objective

In some cases, you may want to take ownership of the values in `DynamicList` or `DynamicMap`. 

I came across this need while trying to implement a custom deserializer, but couldn't get ownership of the values in the list.

## Solution

Implemented `IntoIter` for both `DynamicList` and `DynamicMap`.
  • Loading branch information
MrGVSV committed Apr 25, 2022
1 parent 4aa5605 commit c203c36
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 4 deletions.
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ Bevy also currently has the following "development process" goals:
* **Focus**: The Bevy Org should focus on building a small number of features excellently over merging every new community-contributed feature quickly. Sometimes this means pull requests will sit unmerged for a long time. This is the price of focus and we are willing to pay it. Fortunately Bevy is modular to its core. 3rd party plugins are a great way to work around this policy.
* **User-facing API ergonomics come first**: Solid user experience should receive significant focus and investment. It should rarely be compromised in the interest of internal implementation details.
* **Modularity over deep integration**: Individual crates and features should be "pluggable" whenever possible. Don't tie crates, features, or types together that don't need to be.
* **Don't merge everything ... don't merge too early**: Every feature we add increases maintenance burden and compile times. Only merge features that are "generally" useful. Don't merge major changes or new features unless we have relative consensus that the design is correct *and* that we have the developer capacity to support it. When possible, make a 3rd party Plugin / crate first, then consider merging once the API has been tested in the wild. Bevy's modular structure means that the only difference between "official engine features" and "third party plugins" is our endorsement and the repo the code lives in. We should take advantage of that whenever possible.
* **Control and consistency over 3rd party code reuse**: Only add a dependency if it is *absolutely* necessary. Every dependency we add decreases our autonomy and consistency. Dependencies also have the potential to increase compile times and risk pulling in sub-dependencies we don't want / need.
* **Don't merge everything ... don't merge too early**: Every feature we add increases maintenance burden and compile times. Only merge features that are "generally" useful. Don't merge major changes or new features unless we have relative consensus that the design is correct _and_ that we have the developer capacity to support it. When possible, make a 3rd party Plugin / crate first, then consider merging once the API has been tested in the wild. Bevy's modular structure means that the only difference between "official engine features" and "third party plugins" is our endorsement and the repo the code lives in. We should take advantage of that whenever possible.
* **Control and consistency over 3rd party code reuse**: Only add a dependency if it is _absolutely_ necessary. Every dependency we add decreases our autonomy and consistency. Dependencies also have the potential to increase compile times and risk pulling in sub-dependencies we don't want / need.
* **Don't re-invent every wheel**: As a counter to the previous point, don't re-invent everything at all costs. If there is a crate in the Rust ecosystem that is the "de-facto" standard (ex: wgpu, winit, cpal), we should heavily consider using it. Bevy should be a positive force in the ecosystem. We should drive the improvements we need into these core ecosystem crates.
* **Rust-first**: Engine and user-facing code should optimize and encourage Rust-only workflows. Adding additional languages increases internal complexity, fractures the Bevy ecosystem, and makes it harder for users to understand the engine. Never compromise a Rust interface in the interest of compatibility with other languages.
* **Thoughtful public interfaces over maximal configurability**: Symbols and apis should be private by default. Every public API should be thoughtfully and consistently designed. Don't expose unnecessary internal implementation details. Don't allow users to "shoot themselves in the foot". Favor one "happy path" api over multiple apis for different use cases.
* **Welcome new contributors**: Invest in new contributors. Help them fill knowledge and skill gaps. Don't ever gatekeep Bevy development according to notions of required skills or credentials. Help new developers find their niche.
* **Civil discourse**: We need to collectively discuss ideas and the best ideas *should* win. But conversations need to remain respectful at all times. Remember that we're all in this together. Always follow our [Code of Conduct](https://github.com/bevyengine/bevy/blob/main/CODE_OF_CONDUCT.md).
* **Test what you need to**: Write useful tests. Don't write tests that aren't useful. We *generally* aren't strict about unit testing every line of code. We don't want you to waste your time. But at the same time:
* **Civil discourse**: We need to collectively discuss ideas and the best ideas _should_ win. But conversations need to remain respectful at all times. Remember that we're all in this together. Always follow our [Code of Conduct](https://github.com/bevyengine/bevy/blob/main/CODE_OF_CONDUCT.md).
* **Test what you need to**: Write useful tests. Don't write tests that aren't useful. We _generally_ aren't strict about unit testing every line of code. We don't want you to waste your time. But at the same time:
* Most new features should have at least one minimal [example](https://github.com/bevyengine/bevy/tree/main/examples). These also serve as simple integration tests, as they are run as part of our CI process.
* The more complex or "core" a feature is, the more strict we are about unit tests. Use your best judgement here. We will let you know if your pull request needs more tests. We use [Rust's built in testing framework](https://doc.rust-lang.org/book/ch11-01-writing-tests.html).

Expand Down
27 changes: 27 additions & 0 deletions crates/bevy_reflect/src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ impl<'a> Iterator for ListIter<'a> {
}
}

impl IntoIterator for DynamicList {
type Item = Box<dyn Reflect>;
type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
self.values.into_iter()
}
}

impl<'a> ExactSizeIterator for ListIter<'a> {}

/// Applies the elements of `b` to the corresponding elements of `a`.
Expand Down Expand Up @@ -244,3 +253,21 @@ pub fn list_partial_eq<L: List>(a: &L, b: &dyn Reflect) -> Option<bool> {

Some(true)
}

#[cfg(test)]
mod tests {
use super::DynamicList;

#[test]
fn test_into_iter() {
let mut list = DynamicList::default();
list.push(0usize);
list.push(1usize);
list.push(2usize);
let items = list.into_iter();
for (index, item) in items.into_iter().enumerate() {
let value = item.take::<usize>().expect("couldn't downcast to usize");
assert_eq!(index, value);
}
}
}
34 changes: 34 additions & 0 deletions crates/bevy_reflect/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,15 @@ impl<'a> Iterator for MapIter<'a> {
}
}

impl IntoIterator for DynamicMap {
type Item = (Box<dyn Reflect>, Box<dyn Reflect>);
type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
self.values.into_iter()
}
}

impl<'a> ExactSizeIterator for MapIter<'a> {}

/// Compares a [`Map`] with a [`Reflect`] value.
Expand Down Expand Up @@ -253,3 +262,28 @@ pub fn map_partial_eq<M: Map>(a: &M, b: &dyn Reflect) -> Option<bool> {

Some(true)
}

#[cfg(test)]
mod tests {
use super::DynamicMap;

#[test]
fn test_into_iter() {
let expected = vec!["foo", "bar", "baz"];

let mut map = DynamicMap::default();
map.insert(0usize, expected[0].to_string());
map.insert(1usize, expected[1].to_string());
map.insert(2usize, expected[2].to_string());

for (index, item) in map.into_iter().enumerate() {
let key = item.0.take::<usize>().expect("couldn't downcast to usize");
let value = item
.1
.take::<String>()
.expect("couldn't downcast to String");
assert_eq!(index, key);
assert_eq!(expected[index], value);
}
}
}

0 comments on commit c203c36

Please sign in to comment.