From 5b00af01d756cc9f03f6058f2634ebd56c2a7296 Mon Sep 17 00:00:00 2001 From: Mark Schmale Date: Tue, 27 Sep 2022 18:11:38 +0000 Subject: [PATCH] Make arrays behave like lists in reflection (#5987) # Objective Currently, arrays cannot indexed using the reflection path API. This change makes them behave like lists so `x.get_path("list[0]")` will behave the same way, whether x.list is a "List" (e.g. a Vec) or an array. ## Solution When syntax is encounterd `[ ]` we check if the referenced type is either a `ReflectRef::List` or `ReflectRef::Array` (or `ReflectMut` for the mutable case). Since both provide the identical API for accessing entries, we do the same for both, although it requires code duplication as far as I can tell. This was born from working on #5764, but since this seems to be an easier fix (and I am not sure if I can actually solve #5812) I figured it might be worth to split this out. --- crates/bevy_reflect/src/path.rs | 101 ++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 17 deletions(-) diff --git a/crates/bevy_reflect/src/path.rs b/crates/bevy_reflect/src/path.rs index e749f0d4c247d..0b508fa0f749e 100644 --- a/crates/bevy_reflect/src/path.rs +++ b/crates/bevy_reflect/src/path.rs @@ -1,6 +1,6 @@ use std::num::ParseIntError; -use crate::{Reflect, ReflectMut, ReflectRef}; +use crate::{Array, Reflect, ReflectMut, ReflectRef}; use thiserror::Error; /// An error returned from a failed path string query. @@ -122,14 +122,10 @@ impl GetPath for dyn Reflect { if let Some(Token::Ident(value)) = next_token(path, &mut index) { match current.reflect_ref() { ReflectRef::List(reflect_list) => { - let list_index = value.parse::()?; - let list_item = reflect_list.get(list_index).ok_or( - ReflectPathError::InvalidListIndex { - index: current_index, - list_index, - }, - )?; - current = list_item; + current = read_array_entry(reflect_list, value, current_index)?; + } + ReflectRef::Array(reflect_arr) => { + current = read_array_entry(reflect_arr, value, current_index)?; } _ => { return Err(ReflectPathError::ExpectedList { @@ -188,14 +184,10 @@ impl GetPath for dyn Reflect { if let Some(Token::Ident(value)) = next_token(path, &mut index) { match current.reflect_mut() { ReflectMut::List(reflect_list) => { - let list_index = value.parse::()?; - let list_item = reflect_list.get_mut(list_index).ok_or( - ReflectPathError::InvalidListIndex { - index: current_index, - list_index, - }, - )?; - current = list_item; + current = read_array_entry_mut(reflect_list, value, current_index)?; + } + ReflectMut::Array(reflect_arr) => { + current = read_array_entry_mut(reflect_arr, value, current_index)?; } _ => { return Err(ReflectPathError::ExpectedStruct { @@ -233,6 +225,38 @@ impl GetPath for dyn Reflect { } } +fn read_array_entry<'r, 'p, T>( + list: &'r T, + value: &'p str, + current_index: usize, +) -> Result<&'r dyn Reflect, ReflectPathError<'p>> +where + T: Array + ?Sized, +{ + let list_index = value.parse::()?; + list.get(list_index) + .ok_or(ReflectPathError::InvalidListIndex { + index: current_index, + list_index, + }) +} + +fn read_array_entry_mut<'r, 'p, T>( + list: &'r mut T, + value: &'p str, + current_index: usize, +) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>> +where + T: Array + ?Sized, +{ + let list_index = value.parse::()?; + list.get_mut(list_index) + .ok_or(ReflectPathError::InvalidListIndex { + index: current_index, + list_index, + }) +} + fn read_field<'r, 'p>( current: &'r dyn Reflect, field: &'p str, @@ -341,6 +365,49 @@ mod tests { use super::GetPath; use crate as bevy_reflect; use crate::*; + + #[test] + fn reflect_array_behaves_like_list() { + #[derive(Reflect)] + struct A { + list: Vec, + array: [u8; 10], + } + + let a = A { + list: vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9], + array: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9], + }; + + assert_eq!(*a.get_path::("list[5]").unwrap(), 5); + assert_eq!(*a.get_path::("array[5]").unwrap(), 5); + assert_eq!(*a.get_path::("list[0]").unwrap(), 0); + assert_eq!(*a.get_path::("array[0]").unwrap(), 0); + } + + #[test] + fn reflect_array_behaves_like_list_mut() { + #[derive(Reflect)] + struct A { + list: Vec, + array: [u8; 10], + } + + let mut a = A { + list: vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9], + array: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9], + }; + + assert_eq!(*a.get_path_mut::("list[5]").unwrap(), 5); + assert_eq!(*a.get_path_mut::("array[5]").unwrap(), 5); + + *a.get_path_mut::("list[5]").unwrap() = 10; + *a.get_path_mut::("array[5]").unwrap() = 10; + + assert_eq!(*a.get_path_mut::("list[5]").unwrap(), 10); + assert_eq!(*a.get_path_mut::("array[5]").unwrap(), 10); + } + #[test] fn reflect_path() { #[derive(Reflect)]