Include errors along side successful components in BRP bevy/get method (#15516)
				
					
				
			## Objective I am using BRP for a web inspector. To get components from a entity is first do a `bevy/list` on the specific entity and then use the result in a `bevy/get` request. The problem with this is `bevy/list` returns all components even if they aren't reflect-able (which is what I expect) but when I then do a `bevy/get` request even if all bar one of the components are reflect-able the request will fail. ## Solution Update the `bevy/get` response to include a map of components like it did for successful request and a map of errors. This means if one or more components are not present on the entity or cannot be reflected it will not fail the entire request. I also only did `bevy/get` as I don't think any of the other methods would benefit from this. ## Testing I tested this with my inspector and with a http client and it worked as expected. --------- Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
This commit is contained in:
		
							parent
							
								
									eaa37f3b45
								
							
						
					
					
						commit
						85dfd72631
					
				| @ -18,7 +18,7 @@ use bevy_reflect::{ | ||||
| }; | ||||
| use bevy_utils::HashMap; | ||||
| use serde::{de::DeserializeSeed as _, Deserialize, Serialize}; | ||||
| use serde_json::Value; | ||||
| use serde_json::{Map, Value}; | ||||
| 
 | ||||
| use crate::{error_codes, BrpError, BrpResult}; | ||||
| 
 | ||||
| @ -64,6 +64,11 @@ pub struct BrpGetParams { | ||||
|     ///
 | ||||
|     /// [full paths]: bevy_reflect::TypePath::type_path
 | ||||
|     pub components: Vec<String>, | ||||
| 
 | ||||
|     /// An optional flag to fail when encountering an invalid component rather
 | ||||
|     /// than skipping it. Defaults to false.
 | ||||
|     #[serde(default)] | ||||
|     pub strict: bool, | ||||
| } | ||||
| 
 | ||||
| /// `bevy/query`: Performs a query over components in the ECS, returning entities
 | ||||
| @ -228,7 +233,20 @@ pub struct BrpSpawnResponse { | ||||
| } | ||||
| 
 | ||||
| /// The response to a `bevy/get` request.
 | ||||
| pub type BrpGetResponse = HashMap<String, Value>; | ||||
| #[derive(Debug, Serialize, Deserialize, Clone)] | ||||
| #[serde(untagged)] | ||||
| pub enum BrpGetResponse { | ||||
|     /// The non-strict response that reports errors separately without failing the entire request.
 | ||||
|     Lenient { | ||||
|         /// A map of successful components with their values.
 | ||||
|         components: HashMap<String, Value>, | ||||
|         /// A map of unsuccessful components with their errors.
 | ||||
|         errors: HashMap<String, Value>, | ||||
|     }, | ||||
|     /// The strict response that will fail if any components are not present or aren't
 | ||||
|     /// reflect-able.
 | ||||
|     Strict(HashMap<String, Value>), | ||||
| } | ||||
| 
 | ||||
| /// The response to a `bevy/list` request.
 | ||||
| pub type BrpListResponse = Vec<String>; | ||||
| @ -273,26 +291,65 @@ fn parse_some<T: for<'de> Deserialize<'de>>(value: Option<Value>) -> Result<T, B | ||||
| 
 | ||||
| /// Handles a `bevy/get` request coming from a client.
 | ||||
| pub fn process_remote_get_request(In(params): In<Option<Value>>, world: &World) -> BrpResult { | ||||
|     let BrpGetParams { entity, components } = parse_some(params)?; | ||||
|     let BrpGetParams { | ||||
|         entity, | ||||
|         components, | ||||
|         strict, | ||||
|     } = parse_some(params)?; | ||||
| 
 | ||||
|     let app_type_registry = world.resource::<AppTypeRegistry>(); | ||||
|     let type_registry = app_type_registry.read(); | ||||
|     let entity_ref = get_entity(world, entity)?; | ||||
| 
 | ||||
|     let mut response = BrpGetResponse::default(); | ||||
|     let mut response = if strict { | ||||
|         BrpGetResponse::Strict(Default::default()) | ||||
|     } else { | ||||
|         BrpGetResponse::Lenient { | ||||
|             components: Default::default(), | ||||
|             errors: Default::default(), | ||||
|         } | ||||
|     }; | ||||
| 
 | ||||
|     for component_path in components { | ||||
|         let reflect_component = get_reflect_component(&type_registry, &component_path) | ||||
|             .map_err(BrpError::component_error)?; | ||||
|         match handle_get_component(&component_path, entity, entity_ref, &type_registry) { | ||||
|             Ok(serialized_object) => match response { | ||||
|                 BrpGetResponse::Strict(ref mut components) | ||||
|                 | BrpGetResponse::Lenient { | ||||
|                     ref mut components, .. | ||||
|                 } => { | ||||
|                     components.extend(serialized_object.into_iter()); | ||||
|                 } | ||||
|             }, | ||||
|             Err(err) => match response { | ||||
|                 BrpGetResponse::Strict(_) => return Err(err), | ||||
|                 BrpGetResponse::Lenient { ref mut errors, .. } => { | ||||
|                     let err_value = serde_json::to_value(err).map_err(BrpError::internal)?; | ||||
|                     errors.insert(component_path, err_value); | ||||
|                 } | ||||
|             }, | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|     serde_json::to_value(response).map_err(BrpError::internal) | ||||
| } | ||||
| 
 | ||||
| /// Handle a single component for [`process_remote_get_request`].
 | ||||
| fn handle_get_component( | ||||
|     component_path: &str, | ||||
|     entity: Entity, | ||||
|     entity_ref: EntityRef, | ||||
|     type_registry: &TypeRegistry, | ||||
| ) -> Result<Map<String, Value>, BrpError> { | ||||
|     let reflect_component = | ||||
|         get_reflect_component(type_registry, component_path).map_err(BrpError::component_error)?; | ||||
| 
 | ||||
|     // Retrieve the reflected value for the given specified component on the given entity.
 | ||||
|     let Some(reflected) = reflect_component.reflect(entity_ref) else { | ||||
|             return Err(BrpError::component_not_present(&component_path, entity)); | ||||
|         return Err(BrpError::component_not_present(component_path, entity)); | ||||
|     }; | ||||
| 
 | ||||
|     // Each component value serializes to a map with a single entry.
 | ||||
|         let reflect_serializer = | ||||
|             ReflectSerializer::new(reflected.as_partial_reflect(), &type_registry); | ||||
|     let reflect_serializer = ReflectSerializer::new(reflected.as_partial_reflect(), type_registry); | ||||
|     let Value::Object(serialized_object) = | ||||
|         serde_json::to_value(&reflect_serializer).map_err(|err| BrpError { | ||||
|             code: error_codes::COMPONENT_ERROR, | ||||
| @ -307,10 +364,7 @@ pub fn process_remote_get_request(In(params): In<Option<Value>>, world: &World) | ||||
|         }); | ||||
|     }; | ||||
| 
 | ||||
|         response.extend(serialized_object.into_iter()); | ||||
|     } | ||||
| 
 | ||||
|     serde_json::to_value(response).map_err(BrpError::internal) | ||||
|     Ok(serialized_object) | ||||
| } | ||||
| 
 | ||||
| /// Handles a `bevy/query` request coming from a client.
 | ||||
|  | ||||
| @ -109,6 +109,17 @@ | ||||
| //! `params`:
 | ||||
| //! - `entity`: The ID of the entity whose components will be fetched.
 | ||||
| //! - `components`: An array of [fully-qualified type names] of components to fetch.
 | ||||
| //! - `strict` (optional): A flag to enable strict mode which will fail if any one of the
 | ||||
| //!   components is not present or can not be reflected. Defaults to false.
 | ||||
| //!
 | ||||
| //! If `strict` is false:
 | ||||
| //!
 | ||||
| //! `result`:
 | ||||
| //! - `components`: A map associating each type name to its value on the requested entity.
 | ||||
| //! - `errors`: A map associating each type name with an error if it was not on the entity
 | ||||
| //!   or could not be reflected.
 | ||||
| //!
 | ||||
| //! If `strict` is true:
 | ||||
| //!
 | ||||
| //! `result`: A map associating each type name to its value on the requested entity.
 | ||||
| //!
 | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Liam Gallagher
						Liam Gallagher