Skip to content

Commit

Permalink
bevy_scene: Use map for scene components (#6345)
Browse files Browse the repository at this point in the history
# Objective

Currently scenes define components using a list:

```rust
[
  (
    entity: 0,
    components: [
      {
        "bevy_transform::components::transform::Transform": (
          translation: (
            x: 0.0,
            y: 0.0,
            z: 0.0
          ),
          rotation: (0.0, 0.0, 0.0, 1.0),
          scale: (
            x: 1.0,
            y: 1.0,
            z: 1.0
          ),
        ),
      },
      {
        "my_crate::Foo": (
          text: "Hello World",
        ),
      },
      {
        "my_crate::Bar": (
          baz: 123,
        ),
      },
    ],
  ),
]
```

However, this representation has some drawbacks (as pointed out by @Metadorius in [this](#4561 (comment)) comment):

1. Increased nesting and more characters (minor effect on overall size)
2. More importantly, by definition, entities cannot have more than one instance of any given component. Therefore, such data is best stored as a map— where all values are meant to have unique keys.


## Solution

Change `components` to store a map of components rather than a list:

```rust
[
  (
    entity: 0,
    components: {
      "bevy_transform::components::transform::Transform": (
        translation: (
          x: 0.0,
          y: 0.0,
          z: 0.0
        ),
        rotation: (0.0, 0.0, 0.0, 1.0),
        scale: (
          x: 1.0,
          y: 1.0,
          z: 1.0
        ),
      ),
      "my_crate::Foo": (
        text: "Hello World",
      ),
      "my_crate::Bar": (
        baz: 123
      ),
    },
  ),
]
```

#### Code Representation

This change only affects the scene format itself. `DynamicEntity` still stores its components as a list. The reason for this is that storing such data as a map is not really needed since:
1. The "key" of each value is easily found by just calling `Reflect::type_name` on it
2. We should be generating such structs using the `World` itself which upholds the one-component-per-entity rule

One could in theory create manually create a `DynamicEntity` with duplicate components, but this isn't something I think we should focus on in this PR. `DynamicEntity` can be broken in other ways (i.e. storing a non-component in the components list), and resolving its issues can be done in a separate PR.

---

## Changelog

* The scene format now uses a map to represent the collection of components rather than a list

## Migration Guide

The scene format now uses a map to represent the collection of components. Scene files will need to update from the old list format.

<details>
<summary>Example Code</summary>

```rust
// OLD
[
  (
    entity: 0,
    components: [
      {
        "bevy_transform::components::transform::Transform": (
          translation: (
            x: 0.0,
            y: 0.0,
            z: 0.0
          ),
          rotation: (0.0, 0.0, 0.0, 1.0),
          scale: (
            x: 1.0,
            y: 1.0,
            z: 1.0
          ),
        ),
      },
      {
        "my_crate::Foo": (
          text: "Hello World",
        ),
      },
      {
        "my_crate::Bar": (
          baz: 123,
        ),
      },
    ],
  ),
]

// NEW
[
  (
    entity: 0,
    components: {
      "bevy_transform::components::transform::Transform": (
        translation: (
          x: 0.0,
          y: 0.0,
          z: 0.0
        ),
        rotation: (0.0, 0.0, 0.0, 1.0),
        scale: (
          x: 1.0,
          y: 1.0,
          z: 1.0
        ),
      ),
      "my_crate::Foo": (
        text: "Hello World",
      ),
      "my_crate::Bar": (
        baz: 123
      ),
    },
  ),
]
```

</details>
  • Loading branch information
MrGVSV committed Oct 27, 2022
1 parent 4d3d3c8 commit 894334b
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 47 deletions.
58 changes: 25 additions & 33 deletions assets/scenes/load_scene_example.scn.ron
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,37 @@
entities: [
(
entity: 0,
components: [
{
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
components: {
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
},
{
"scene::ComponentB": (
value: "hello",
),
},
{
"scene::ComponentA": (
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 2.0,
y: 1.0,
z: 1.0
),
},
],
),
"scene::ComponentB": (
value: "hello",
),
"scene::ComponentA": (
x: 1.0,
y: 2.0,
),
},
),
(
entity: 1,
components: [
{
"scene::ComponentA": (
x: 3.0,
y: 4.0,
),
},
],
components: {
"scene::ComponentA": (
x: 3.0,
y: 4.0,
),
},
),
]
)
53 changes: 39 additions & 14 deletions crates/bevy_scene/src/serde.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::{DynamicEntity, DynamicScene};
use anyhow::Result;
use bevy_reflect::{
serde::{ReflectSerializer, UntypedReflectDeserializer},
Reflect, TypeRegistry, TypeRegistryArc,
};
use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer};
use bevy_reflect::{serde::UntypedReflectDeserializer, Reflect, TypeRegistry, TypeRegistryArc};
use bevy_utils::HashSet;
use serde::ser::SerializeMap;
use serde::{
de::{DeserializeSeed, Error, MapAccess, SeqAccess, Visitor},
ser::{SerializeSeq, SerializeStruct},
Expand Down Expand Up @@ -100,10 +100,12 @@ impl<'a> Serialize for ComponentsSerializer<'a> {
where
S: serde::Serializer,
{
let mut state = serializer.serialize_seq(Some(self.components.len()))?;
let mut state = serializer.serialize_map(Some(self.components.len()))?;
for component in self.components {
state
.serialize_element(&ReflectSerializer::new(&**component, &self.registry.read()))?;
state.serialize_entry(
component.type_name(),
&TypedReflectSerializer::new(&**component, &*self.registry.read()),
)?;
}
state.end()
}
Expand Down Expand Up @@ -285,7 +287,7 @@ impl<'a, 'de> Visitor<'de> for SceneEntityVisitor<'a> {
return Err(Error::duplicate_field(ENTITY_FIELD_COMPONENTS));
}

components = Some(map.next_value_seed(ComponentVecDeserializer {
components = Some(map.next_value_seed(ComponentDeserializer {
registry: self.registry,
})?);
}
Expand All @@ -306,32 +308,55 @@ impl<'a, 'de> Visitor<'de> for SceneEntityVisitor<'a> {
}
}

pub struct ComponentVecDeserializer<'a> {
pub struct ComponentDeserializer<'a> {
pub registry: &'a TypeRegistry,
}

impl<'a, 'de> DeserializeSeed<'de> for ComponentVecDeserializer<'a> {
impl<'a, 'de> DeserializeSeed<'de> for ComponentDeserializer<'a> {
type Value = Vec<Box<dyn Reflect>>;

fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
where
D: serde::Deserializer<'de>,
{
deserializer.deserialize_seq(ComponentSeqVisitor {
deserializer.deserialize_map(ComponentVisitor {
registry: self.registry,
})
}
}

struct ComponentSeqVisitor<'a> {
struct ComponentVisitor<'a> {
pub registry: &'a TypeRegistry,
}

impl<'a, 'de> Visitor<'de> for ComponentSeqVisitor<'a> {
impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> {
type Value = Vec<Box<dyn Reflect>>;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("list of components")
formatter.write_str("map of components")
}

fn visit_map<A>(self, mut map: A) -> std::result::Result<Self::Value, A::Error>
where
A: MapAccess<'de>,
{
let mut added = HashSet::new();
let mut components = Vec::new();
while let Some(key) = map.next_key::<&str>()? {
if !added.insert(key) {
return Err(Error::custom(format!("duplicate component: `{}`", key)));
}

let registration = self
.registry
.get_with_name(key)
.ok_or_else(|| Error::custom(format!("no registration found for `{}`", key)))?;
components.push(
map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?,
);
}

Ok(components)
}

fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
Expand Down

0 comments on commit 894334b

Please sign in to comment.