Skip to content

Commit

Permalink
Improve Array, List, and Map methods (#136)
Browse files Browse the repository at this point in the history
### Checklist

* [x] I have read the [Contributor Guide](../../CONTRIBUTING.md)
* [x] I have read and agree to the [Code of
Conduct](../../CODE_OF_CONDUCT.md)
* [x] I have added a description of my changes and why I'd like them
included in the section below

### Description of Changes

Pass over methods on `Array`, `List`, and `Map` that makes things more
explicit and adds a few new useful methods:
- Add `Array::swap`
- Add `List::try_insert`
- `List::push` renamed to `List::try_push` and now returns a result
indicating that `FromReflect::from_reflect` failed
- `Map::insert` renamed to `Map::try_insert` and also returns result
- `Map::remove` renamed to `Map::try_remove` and also returns result

Also deduplicated the `Map` impls.

### Related Issues

- Fixes #132
  • Loading branch information
davidpdrsn authored Mar 4, 2024
1 parent 01ddb17 commit 3650736
Show file tree
Hide file tree
Showing 13 changed files with 195 additions and 244 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **fixed:** Iterating over fields and variants in type descriptors is now done
in the same order as defined in the code ([#119])
- **fixed:** Support derive reflect for `HashMap` ([#119])
- **breaking:** Add `Array::swap` ([#136])
- **breaking:** Add `List::try_insert` ([#136])
- **breaking:** `List::push` renamed to `List::try_push` ([#136])
- **breaking:** `Map::insert` renamed to `Map::try_insert` ([#136])
- **breaking:** `Map::remove` renamed to `Map::try_remove` ([#136])

[#90]: https://github.com/EmbarkStudios/mirror-mirror/pull/90
[#110]: https://github.com/EmbarkStudios/mirror-mirror/pull/110
[#109]: https://github.com/EmbarkStudios/mirror-mirror/pull/109
[#119]: https://github.com/EmbarkStudios/mirror-mirror/pull/119
[#136]: https://github.com/EmbarkStudios/mirror-mirror/pull/136

# 0.1.19 (26. February, 2023)

Expand Down
7 changes: 7 additions & 0 deletions crates/mirror-mirror/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ pub trait Array: Reflect {
fn iter(&self) -> Iter<'_>;

fn iter_mut(&mut self) -> ValueIterMut<'_>;

/// Swaps two elements in the array.
///
/// # Panics
///
/// Panics if `a` or `b` are out of bounds.
fn swap(&mut self, a: usize, b: usize);
}

impl fmt::Debug for dyn Array {
Expand Down
4 changes: 4 additions & 0 deletions crates/mirror-mirror/src/foreign_impls/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ where
.map(|value| value.as_reflect_mut());
Box::new(iter)
}

fn swap(&mut self, a: usize, b: usize) {
self.as_mut_slice().swap(a, b);
}
}

impl<T, const N: usize> FromReflect for [T; N]
Expand Down
48 changes: 2 additions & 46 deletions crates/mirror-mirror/src/foreign_impls/btree_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use core::any::Any;
use core::fmt;

use crate::iter::PairIterMut;
use crate::map::MapError;
use crate::type_info::graph::MapNode;
use crate::type_info::graph::NodeId;
use crate::type_info::graph::TypeGraph;
Expand All @@ -21,52 +22,7 @@ where
K: FromReflect + DescribeType + Ord,
V: FromReflect + DescribeType,
{
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get(&key)?;
Some(value.as_reflect())
}

fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get_mut(&key)?;
Some(value.as_reflect_mut())
}

fn insert(&mut self, key: &dyn Reflect, value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let value = V::from_reflect(value)?;
let previous = BTreeMap::insert(self, key, value)?;
Some(Box::new(previous))
}

fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let previous = BTreeMap::remove(self, &key)?;
Some(Box::new(previous))
}

fn len(&self) -> usize {
self.len()
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn iter(&self) -> crate::map::Iter<'_> {
let iter = self
.iter()
.map(|(key, value)| (key.as_reflect(), value.as_reflect()));
Box::new(iter)
}

fn iter_mut(&mut self) -> PairIterMut<'_, dyn Reflect> {
let iter = self
.iter_mut()
.map(|(key, value)| (key.as_reflect(), value.as_reflect_mut()));
Box::new(iter)
}
map_methods!();
}

impl<K, V> DescribeType for BTreeMap<K, V>
Expand Down
48 changes: 2 additions & 46 deletions crates/mirror-mirror/src/foreign_impls/hash_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use core::hash::Hash;
use std::collections::HashMap;

use crate::iter::PairIterMut;
use crate::map::MapError;
use crate::type_info::graph::MapNode;
use crate::type_info::graph::NodeId;
use crate::type_info::graph::TypeGraph;
Expand Down Expand Up @@ -105,52 +106,7 @@ where
V: FromReflect + DescribeType,
S: Default + BuildHasher + Send + 'static,
{
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get(&key)?;
Some(value.as_reflect())
}

fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get_mut(&key)?;
Some(value.as_reflect_mut())
}

fn insert(&mut self, key: &dyn Reflect, value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let value = V::from_reflect(value)?;
let previous = self.insert(key, value)?;
Some(Box::new(previous))
}

fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let previous = self.remove(&key)?;
Some(Box::new(previous))
}

fn len(&self) -> usize {
self.len()
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn iter(&self) -> crate::map::Iter<'_> {
let iter = self
.iter()
.map(|(key, value)| (key.as_reflect(), value.as_reflect()));
Box::new(iter)
}

fn iter_mut(&mut self) -> PairIterMut<'_, dyn Reflect> {
let iter = self
.iter_mut()
.map(|(key, value)| (key.as_reflect(), value.as_reflect_mut()));
Box::new(iter)
}
map_methods!();
}

impl<K, V, S> DescribeType for HashMap<K, V, S>
Expand Down
48 changes: 2 additions & 46 deletions crates/mirror-mirror/src/foreign_impls/kollect/linear_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use core::fmt;
use kollect::LinearMap;

use crate::iter::PairIterMut;
use crate::map::MapError;
use crate::type_info::graph::MapNode;
use crate::type_info::graph::NodeId;
use crate::type_info::graph::TypeGraph;
Expand Down Expand Up @@ -98,52 +99,7 @@ where
K: FromReflect + DescribeType + Eq,
V: FromReflect + DescribeType,
{
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get(&key)?;
Some(value.as_reflect())
}

fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get_mut(&key)?;
Some(value.as_reflect_mut())
}

fn insert(&mut self, key: &dyn Reflect, value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let value = V::from_reflect(value)?;
let previous = self.insert(key, value)?;
Some(Box::new(previous))
}

fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let previous = self.remove(&key)?;
Some(Box::new(previous))
}

fn len(&self) -> usize {
self.len()
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn iter(&self) -> crate::map::Iter<'_> {
let iter = self
.iter()
.map(|(key, value)| (key.as_reflect(), value.as_reflect()));
Box::new(iter)
}

fn iter_mut(&mut self) -> PairIterMut<'_, dyn Reflect> {
let iter = self
.iter_mut()
.map(|(key, value)| (key.as_reflect(), value.as_reflect_mut()));
Box::new(iter)
}
map_methods!();
}

impl<K, V> DescribeType for LinearMap<K, V>
Expand Down
48 changes: 2 additions & 46 deletions crates/mirror-mirror/src/foreign_impls/kollect/ordered_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use core::hash::Hash;
use kollect::OrderedMap;

use crate::iter::PairIterMut;
use crate::map::MapError;
use crate::type_info::graph::MapNode;
use crate::type_info::graph::NodeId;
use crate::type_info::graph::TypeGraph;
Expand Down Expand Up @@ -103,52 +104,7 @@ where
V: FromReflect + DescribeType,
S: Default + BuildHasher + Send + 'static,
{
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get(&key)?;
Some(value.as_reflect())
}

fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get_mut(&key)?;
Some(value.as_reflect_mut())
}

fn insert(&mut self, key: &dyn Reflect, value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let value = V::from_reflect(value)?;
let previous = self.insert(key, value)?;
Some(Box::new(previous))
}

fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let previous = self.remove(&key)?;
Some(Box::new(previous))
}

fn len(&self) -> usize {
self.len()
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn iter(&self) -> crate::map::Iter<'_> {
let iter = self
.iter()
.map(|(key, value)| (key.as_reflect(), value.as_reflect()));
Box::new(iter)
}

fn iter_mut(&mut self) -> PairIterMut<'_, dyn Reflect> {
let iter = self
.iter_mut()
.map(|(key, value)| (key.as_reflect(), value.as_reflect_mut()));
Box::new(iter)
}
map_methods!();
}

impl<K, V, S> DescribeType for OrderedMap<K, V, S>
Expand Down
48 changes: 2 additions & 46 deletions crates/mirror-mirror/src/foreign_impls/kollect/unordered_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use core::hash::Hash;
use kollect::UnorderedMap;

use crate::iter::PairIterMut;
use crate::map::MapError;
use crate::type_info::graph::MapNode;
use crate::type_info::graph::NodeId;
use crate::type_info::graph::TypeGraph;
Expand Down Expand Up @@ -103,52 +104,7 @@ where
V: FromReflect + DescribeType,
S: Default + BuildHasher + Send + 'static,
{
fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get(&key)?;
Some(value.as_reflect())
}

fn get_mut(&mut self, key: &dyn Reflect) -> Option<&mut dyn Reflect> {
let key = K::from_reflect(key)?;
let value = self.get_mut(&key)?;
Some(value.as_reflect_mut())
}

fn insert(&mut self, key: &dyn Reflect, value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let value = V::from_reflect(value)?;
let previous = self.insert(key, value)?;
Some(Box::new(previous))
}

fn remove(&mut self, key: &dyn Reflect) -> Option<Box<dyn Reflect>> {
let key = K::from_reflect(key)?;
let previous = self.remove(&key)?;
Some(Box::new(previous))
}

fn len(&self) -> usize {
self.len()
}

fn is_empty(&self) -> bool {
self.is_empty()
}

fn iter(&self) -> crate::map::Iter<'_> {
let iter = self
.iter()
.map(|(key, value)| (key.as_reflect(), value.as_reflect()));
Box::new(iter)
}

fn iter_mut(&mut self) -> PairIterMut<'_, dyn Reflect> {
let iter = self
.iter_mut()
.map(|(key, value)| (key.as_reflect(), value.as_reflect_mut()));
Box::new(iter)
}
map_methods!();
}

impl<K, V, S> DescribeType for UnorderedMap<K, V, S>
Expand Down
Loading

0 comments on commit 3650736

Please sign in to comment.