From 1b9c156479c60c2b791081f4b5d7db5edf38726b Mon Sep 17 00:00:00 2001 From: radiish Date: Mon, 9 Jan 2023 19:47:07 +0000 Subject: [PATCH] reflect: add `insert` and `remove` methods to `List` (#7063) # Objective - Fixes #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 --- crates/bevy_reflect/src/impls/smallvec.rs | 16 +++++++++ crates/bevy_reflect/src/impls/std.rs | 22 ++++++++++-- crates/bevy_reflect/src/list.rs | 41 ++++++++++++++++++++--- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index f44957175f..4a4c64e4ff 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -49,6 +49,22 @@ impl List for SmallVec where T::Item: FromReflect, { + fn insert(&mut self, index: usize, value: Box) { + let value = value.take::().unwrap_or_else(|value| { + ::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 { + Box::new(self.remove(index)) + } + fn push(&mut self, value: Box) { let value = value.take::().unwrap_or_else(|value| { ::Item::from_reflect(&*value).unwrap_or_else(|| { diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index ca7ad9c871..e80859ddbd 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -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 Array for $ty { #[inline] fn get(&self, index: usize) -> Option<&dyn Reflect> { @@ -213,6 +213,22 @@ macro_rules! impl_reflect_for_veclike { } impl List for $ty { + fn insert(&mut self, index: usize, value: Box) { + let value = value.take::().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 { + Box::new($remove(self, index)) + } + fn push(&mut self, value: Box) { let value = value.take::().unwrap_or_else(|value| { T::from_reflect(&*value).unwrap_or_else(|| { @@ -328,9 +344,11 @@ macro_rules! impl_reflect_for_veclike { }; } -impl_reflect_for_veclike!(Vec, Vec::push, Vec::pop, [T]); +impl_reflect_for_veclike!(Vec, Vec::insert, Vec::remove, Vec::push, Vec::pop, [T]); impl_reflect_for_veclike!( VecDeque, + VecDeque::insert, + VecDeque::remove, VecDeque::push_back, VecDeque::pop_back, VecDeque:: diff --git a/crates/bevy_reflect/src/list.rs b/crates/bevy_reflect/src/list.rs index 6a2c2a8767..2d67a62f24 100644 --- a/crates/bevy_reflect/src/list.rs +++ b/crates/bevy_reflect/src/list.rs @@ -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); + + /// 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; + /// Appends an element to the _back_ of the list. - fn push(&mut self, value: Box); + fn push(&mut self, value: Box) { + 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>; + fn pop(&mut self) -> Option> { + if self.is_empty() { + None + } else { + Some(self.remove(self.len() - 1)) + } + } /// Clones the list, producing a [`DynamicList`]. fn clone_dynamic(&self) -> DynamicList { @@ -174,6 +199,14 @@ impl Array for DynamicList { } impl List for DynamicList { + fn insert(&mut self, index: usize, element: Box) { + self.values.insert(index, element); + } + + fn remove(&mut self, index: usize) -> Box { + self.values.remove(index) + } + fn push(&mut self, value: Box) { DynamicList::push_box(self, value); }