Skip to content

Commit 871c80c

Browse files
committed
Add TypeRegistrationDeserializer and remove BorrowedStr (#7094)
# Objective This a follow-up to #6894, see #6894 (comment) The goal is to avoid cloning any string when getting a `&TypeRegistration` corresponding to a string which is being deserialized. As a bonus code duplication is also reduced. ## Solution The manual deserialization of a string and lookup into the type registry has been moved into a separate `TypeRegistrationDeserializer` type, which implements `DeserializeSeed` with a `Visitor` that accepts any string with `visit_str`, even ones that may not live longer than that function call. `BorrowedStr` has been removed since it's no longer used. --- ## Changelog - The type `TypeRegistrationDeserializer` has been added, which simplifies getting a `&TypeRegistration` while deserializing a string.
1 parent 9be47e3 commit 871c80c

File tree

3 files changed

+64
-33
lines changed

3 files changed

+64
-33
lines changed

crates/bevy_reflect/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ downcast-rs = "1.2"
2929
parking_lot = "0.12.1"
3030
thiserror = "1.0"
3131
once_cell = "1.11"
32-
serde = { version = "1", features = ["derive"] }
32+
serde = "1"
3333
smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true }
3434
glam = { version = "0.22", features = ["serde"], optional = true }
3535

crates/bevy_reflect/src/serde/de.rs

+51-16
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use serde::de::{
1212
};
1313
use serde::Deserialize;
1414
use std::any::TypeId;
15-
use std::borrow::Cow;
1615
use std::fmt;
1716
use std::fmt::{Debug, Display, Formatter};
1817
use std::slice::Iter;
@@ -211,13 +210,6 @@ impl<'de> Visitor<'de> for U32Visitor {
211210
}
212211
}
213212

214-
/// Helper struct for deserializing strings without allocating (when possible).
215-
///
216-
/// Based on [this comment](https://github.com/bevyengine/bevy/pull/6894#discussion_r1045069010).
217-
#[derive(Deserialize)]
218-
#[serde(transparent)]
219-
struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>);
220-
221213
/// A general purpose deserializer for reflected types.
222214
///
223215
/// This will return a [`Box<dyn Reflect>`] containing the deserialized data.
@@ -265,6 +257,54 @@ impl<'a, 'de> DeserializeSeed<'de> for UntypedReflectDeserializer<'a> {
265257
}
266258
}
267259

260+
/// A deserializer for type registrations.
261+
///
262+
/// This will return a [`&TypeRegistration`] corresponding to the given type.
263+
/// This deserializer expects a string containing the _full_ [type name] of the
264+
/// type to find the `TypeRegistration` of.
265+
///
266+
/// [`&TypeRegistration`]: crate::TypeRegistration
267+
/// [type name]: std::any::type_name
268+
pub struct TypeRegistrationDeserializer<'a> {
269+
registry: &'a TypeRegistry,
270+
}
271+
272+
impl<'a> TypeRegistrationDeserializer<'a> {
273+
pub fn new(registry: &'a TypeRegistry) -> Self {
274+
Self { registry }
275+
}
276+
}
277+
278+
impl<'a, 'de> DeserializeSeed<'de> for TypeRegistrationDeserializer<'a> {
279+
type Value = &'a TypeRegistration;
280+
281+
fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
282+
where
283+
D: serde::Deserializer<'de>,
284+
{
285+
struct TypeRegistrationVisitor<'a>(&'a TypeRegistry);
286+
287+
impl<'de, 'a> Visitor<'de> for TypeRegistrationVisitor<'a> {
288+
type Value = &'a TypeRegistration;
289+
290+
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
291+
formatter.write_str("string containing `type` entry for the reflected value")
292+
}
293+
294+
fn visit_str<E>(self, type_name: &str) -> Result<Self::Value, E>
295+
where
296+
E: Error,
297+
{
298+
self.0.get_with_name(type_name).ok_or_else(|| {
299+
Error::custom(format_args!("No registration found for `{type_name}`"))
300+
})
301+
}
302+
}
303+
304+
deserializer.deserialize_str(TypeRegistrationVisitor(self.registry))
305+
}
306+
}
307+
268308
struct UntypedReflectDeserializerVisitor<'a> {
269309
registry: &'a TypeRegistry,
270310
}
@@ -280,14 +320,9 @@ impl<'a, 'de> Visitor<'de> for UntypedReflectDeserializerVisitor<'a> {
280320
where
281321
A: MapAccess<'de>,
282322
{
283-
let type_name = map
284-
.next_key::<BorrowableCowStr>()?
285-
.ok_or_else(|| Error::invalid_length(0, &"at least one entry"))?
286-
.0;
287-
288-
let registration = self.registry.get_with_name(&type_name).ok_or_else(|| {
289-
Error::custom(format_args!("No registration found for `{type_name}`"))
290-
})?;
323+
let registration = map
324+
.next_key_seed(TypeRegistrationDeserializer::new(self.registry))?
325+
.ok_or_else(|| Error::invalid_length(0, &"at least one entry"))?;
291326
let value = map.next_value_seed(TypedReflectDeserializer {
292327
registration,
293328
registry: self.registry,

crates/bevy_scene/src/serde.rs

+12-16
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
use crate::{DynamicEntity, DynamicScene};
22
use anyhow::Result;
33
use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer};
4-
use bevy_reflect::{serde::UntypedReflectDeserializer, Reflect, TypeRegistry, TypeRegistryArc};
4+
use bevy_reflect::{
5+
serde::{TypeRegistrationDeserializer, UntypedReflectDeserializer},
6+
Reflect, TypeRegistry, TypeRegistryArc,
7+
};
58
use bevy_utils::HashSet;
69
use serde::ser::SerializeMap;
710
use serde::{
811
de::{DeserializeSeed, Error, MapAccess, SeqAccess, Visitor},
912
ser::SerializeStruct,
1013
Deserialize, Deserializer, Serialize, Serializer,
1114
};
12-
use std::borrow::Cow;
1315
use std::fmt::Formatter;
1416

1517
pub const SCENE_STRUCT: &str = "Scene";
@@ -353,15 +355,16 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> {
353355
{
354356
let mut added = HashSet::new();
355357
let mut components = Vec::new();
356-
while let Some(BorrowableCowStr(key)) = map.next_key()? {
357-
if !added.insert(key.clone()) {
358-
return Err(Error::custom(format!("duplicate component: `{key}`")));
358+
while let Some(registration) =
359+
map.next_key_seed(TypeRegistrationDeserializer::new(self.registry))?
360+
{
361+
if !added.insert(registration.type_id()) {
362+
return Err(Error::custom(format_args!(
363+
"duplicate component: `{}`",
364+
registration.type_name()
365+
)));
359366
}
360367

361-
let registration = self
362-
.registry
363-
.get_with_name(&key)
364-
.ok_or_else(|| Error::custom(format!("no registration found for `{key}`")))?;
365368
components.push(
366369
map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?,
367370
);
@@ -385,13 +388,6 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> {
385388
}
386389
}
387390

388-
/// Helper struct for deserializing strings without allocating (when possible).
389-
///
390-
/// Based on [this comment](https://github.com/bevyengine/bevy/pull/6894#discussion_r1045069010).
391-
#[derive(Deserialize)]
392-
#[serde(transparent)]
393-
struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>);
394-
395391
#[cfg(test)]
396392
mod tests {
397393
use crate::serde::{SceneDeserializer, SceneSerializer};

0 commit comments

Comments
 (0)