Skip to content

Commit

Permalink
reflect: add insert and remove methods to List (bevyengine#7063)
Browse files Browse the repository at this point in the history
# Objective

- Fixes bevyengine#7061

## Solution

- Add and implement `insert` and `remove` methods for `List`.

---

## Changelog

- Added `insert` and `remove` methods to `List`.
- Changed the `push` and `pop` methods on `List` to have default implementations.

## Migration Guide

- Manual implementors of `List` need to implement the new methods `insert` and `remove` and 
consider whether to use the new default implementation of `push` and `pop`.

Co-authored-by: radiish <[email protected]>
  • Loading branch information
2 people authored and james7132 committed Jan 21, 2023
1 parent d823da0 commit 4343f2e
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 6 deletions.
16 changes: 16 additions & 0 deletions crates/bevy_reflect/src/impls/smallvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,22 @@ impl<T: smallvec::Array + Send + Sync + 'static> List for SmallVec<T>
where
T::Item: FromReflect,
{
fn insert(&mut self, index: usize, value: Box<dyn Reflect>) {
let value = value.take::<T::Item>().unwrap_or_else(|value| {
<T as smallvec::Array>::Item::from_reflect(&*value).unwrap_or_else(|| {
panic!(
"Attempted to insert invalid value of type {}.",
value.type_name()
)
})
});
SmallVec::insert(self, index, value);
}

fn remove(&mut self, index: usize) -> Box<dyn Reflect> {
Box::new(self.remove(index))
}

fn push(&mut self, value: Box<dyn Reflect>) {
let value = value.take::<T::Item>().unwrap_or_else(|value| {
<T as smallvec::Array>::Item::from_reflect(&*value).unwrap_or_else(|| {
Expand Down
22 changes: 20 additions & 2 deletions crates/bevy_reflect/src/impls/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl_from_reflect_value!(NonZeroU8);
impl_from_reflect_value!(NonZeroI8);

macro_rules! impl_reflect_for_veclike {
($ty:ty, $push:expr, $pop:expr, $sub:ty) => {
($ty:ty, $insert:expr, $remove:expr, $push:expr, $pop:expr, $sub:ty) => {
impl<T: FromReflect> Array for $ty {
#[inline]
fn get(&self, index: usize) -> Option<&dyn Reflect> {
Expand Down Expand Up @@ -213,6 +213,22 @@ macro_rules! impl_reflect_for_veclike {
}

impl<T: FromReflect> List for $ty {
fn insert(&mut self, index: usize, value: Box<dyn Reflect>) {
let value = value.take::<T>().unwrap_or_else(|value| {
T::from_reflect(&*value).unwrap_or_else(|| {
panic!(
"Attempted to insert invalid value of type {}.",
value.type_name()
)
})
});
$insert(self, index, value);
}

fn remove(&mut self, index: usize) -> Box<dyn Reflect> {
Box::new($remove(self, index))
}

fn push(&mut self, value: Box<dyn Reflect>) {
let value = value.take::<T>().unwrap_or_else(|value| {
T::from_reflect(&*value).unwrap_or_else(|| {
Expand Down Expand Up @@ -328,9 +344,11 @@ macro_rules! impl_reflect_for_veclike {
};
}

impl_reflect_for_veclike!(Vec<T>, Vec::push, Vec::pop, [T]);
impl_reflect_for_veclike!(Vec<T>, Vec::insert, Vec::remove, Vec::push, Vec::pop, [T]);
impl_reflect_for_veclike!(
VecDeque<T>,
VecDeque::insert,
VecDeque::remove,
VecDeque::push_back,
VecDeque::pop_back,
VecDeque::<T>
Expand Down
41 changes: 37 additions & 4 deletions crates/bevy_reflect/src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,43 @@ use crate::{

/// An ordered, mutable list of [Reflect] items. This corresponds to types like [`std::vec::Vec`].
///
/// This is a sub-trait of [`Array`] as it implements a [`push`](List::push) function, allowing
/// it's internal size to grow.
/// This is a sub-trait of [`Array`], however as it implements [insertion](List::insert) and [removal](List::remove),
/// it's internal size may change.
///
/// This trait expects index 0 to contain the _front_ element.
/// The _back_ element must refer to the element with the largest index.
/// These two rules above should be upheld by manual implementors.
///
/// [`push`](List::push) and [`pop`](List::pop) have default implementations,
/// however it may be faster to implement them manually.
pub trait List: Reflect + Array {
/// Inserts an element at position `index` within the list,
/// shifting all elements after it towards the back of the list.
///
/// # Panics
/// Panics if `index > len`.
fn insert(&mut self, index: usize, element: Box<dyn Reflect>);

/// Removes and returns the element at position `index` within the list,
/// shifting all elements before it towards the front of the list.
///
/// # Panics
/// Panics if `index` is out of bounds.
fn remove(&mut self, index: usize) -> Box<dyn Reflect>;

/// Appends an element to the _back_ of the list.
fn push(&mut self, value: Box<dyn Reflect>);
fn push(&mut self, value: Box<dyn Reflect>) {
self.insert(self.len(), value);
}

/// Removes the _back_ element from the list and returns it, or [`None`] if it is empty.
fn pop(&mut self) -> Option<Box<dyn Reflect>>;
fn pop(&mut self) -> Option<Box<dyn Reflect>> {
if self.is_empty() {
None
} else {
Some(self.remove(self.len() - 1))
}
}

/// Clones the list, producing a [`DynamicList`].
fn clone_dynamic(&self) -> DynamicList {
Expand Down Expand Up @@ -174,6 +199,14 @@ impl Array for DynamicList {
}

impl List for DynamicList {
fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {
self.values.insert(index, element);
}

fn remove(&mut self, index: usize) -> Box<dyn Reflect> {
self.values.remove(index)
}

fn push(&mut self, value: Box<dyn Reflect>) {
DynamicList::push_box(self, value);
}
Expand Down

0 comments on commit 4343f2e

Please sign in to comment.