Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified scene and reflection serialization #4153

Closed
AronDerenyi opened this issue Mar 8, 2022 · 15 comments
Closed

Simplified scene and reflection serialization #4153

AronDerenyi opened this issue Mar 8, 2022 · 15 comments
Labels
A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@AronDerenyi
Copy link
Contributor

What problem does this solve or what need does it fill?

Right now serialized scene files are too big and verbose. Take for example the scene example file:

[
  (
    entity: 0,
    components: [
      {
        "type": "bevy_transform::components::transform::Transform",
        "struct": {
          "translation": {
            "type": "glam::vec3::Vec3",
            "value": (0.0, 0.0, 0.0),
          },
          "rotation": {
            "type": "glam::quat::Quat",
            "value": (0.0, 0.0, 0.0, 1.0),
          },
          "scale": {
            "type": "glam::vec3::Vec3",
            "value": (1.0, 1.0, 1.0),
          },
        },
      },
      {
        "type": "scene::ComponentB",
        "struct": {
          "value": {
            "type": "alloc::string::String",
            "value": "hello",
          },
        },
      },
      {
        "type": "scene::ComponentA",
        "struct": {
          "x": {
            "type": "f32",
            "value": 1.0,
          },
          "y": {
            "type": "f32",
            "value": 2.0,
          },
        },
      },
    ],
  ),
  (
    entity: 1,
    components: [
      {
        "type": "scene::ComponentA",
        "struct": {
          "x": {
            "type": "f32",
            "value": 3.0,
          },
          "y": {
            "type": "f32",
            "value": 4.0,
          },
        },
      },
    ],
  ),
]

It takes 64 lines to describe 2 entities with a total of 4 components. It's also hard to read and understand with all the indentation. This is mostly because of the way reflected items are serialized so all of the above also apply to them.

What solution would you like?

Luckily the Rusty Object Notation (RON) has introduced named structs which can be used to merge the type and value into a single item like so:

changing

{
  "type": "glam::vec3::Vec3",
  "value": (1.0, 1.0, 1.0),
}

to

glam::vec3::Vec3(1.0, 1.0, 1.0)

This way we remove the "type" and "value" keys and also a layer of indentation. Applying all of this to the example file we get the following:

[
  (
    entity: 0,
    components: [
      bevy_transform::components::transform::Transform(
        translation: glam::vec3::Vec3(0.0, 0.0, 0.0),
        rotation: glam::quat::Quat(0.0, 0.0, 0.0, 1.0),
        scale: glam::vec3::Vec3(1.0, 1.0, 1.0),
      ),
      scene::ComponentB(
        value: alloc::string::String("hello"),
      ),
      scene::ComponentA(
        x: f32(1.0),
        y: f32(2.0),
      ),
    ],
  ),
  (
    entity: 1,
    components: [
      scene::ComponentA(
        x: f32(3.0),
        y: f32(4.0)
      )
    ],
  ),
]

This is only 28 lines long with a lot less indentation while keeping all of the information of the current file. This is subjective but to me it's just as if not more readable than the current one. And while we're at it we can further simplify by treating the scene as a map where each key is and entity and their corresponding values are lists of their components. Like so:

{
  0: [
    bevy_transform::components::transform::Transform(
      translation: glam::vec3::Vec3(0.0, 0.0, 0.0),
      rotation: glam::quat::Quat(0.0, 0.0, 0.0, 1.0),
      scale: glam::vec3::Vec3(1.0, 1.0, 1.0),
    ),
    scene::ComponentB(
      value: alloc::string::String("hello"),
    ),
    scene::ComponentA(
      x: f32(1.0),
      y: f32(2.0),
    ),
  ],
  1: [
    scene::ComponentA(
      x: f32(3.0),
      y: f32(4.0)
    )
  ],
}

I prefer the last version for the following reasons:

  • It's compact while still containing every bit of information
  • It more closely resembles the corresponding structs in rust
  • It's clear what each indentation level means (first: entities, second: components, third: component properties and so on)
@AronDerenyi AronDerenyi added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Mar 8, 2022
@alice-i-cecile alice-i-cecile added A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled C-Feature A new feature, making something new possible labels Mar 8, 2022
@alice-i-cecile
Copy link
Member

I like this a lot: it's shorter, clearer, and easier to both read and write! For me, 3 > 2 >> 1.

Are you interested in submitting a PR for this? We're about to take a serious look at scenes, and this is high on my list for improvements I'd like to see.

@IceSentry
Copy link
Contributor

I like the last version, but how would it work if we ever want to add more metadata to a scene that isn't just another entity?

@aevyrie
Copy link
Member

aevyrie commented Mar 8, 2022

I like the last version, but how would it work if we ever want to add more metadata to a scene that isn't just another entity?

Top level sections? e.g.:

scene::Metadata1 (

),
scene::Entities(
  0: [
    bevy_transform::components::transform::Transform(
      translation: glam::vec3::Vec3(0.0, 0.0, 0.0),
      # ...
    )
  ],
)

@DJMcNab
Copy link
Member

DJMcNab commented Mar 8, 2022

How is this issue different to #92?

It appears to me to be covering the same ground.

But I do agree, those formats are much nicer

@alice-i-cecile
Copy link
Member

I'd missed #92! IMO this issue covers a strict improvement to the syntax: #92 is much more vaguely scoped. Closing #92 will necessarily close this, but the reverse is not true.

@IceSentry
Copy link
Contributor

Top level sections?

I kinda assumed the scene format would support nested scene or even multiple scene in a file. Assuming it's only one per file it makes sense to only use top level sections though

@AronDerenyi
Copy link
Contributor Author

Are you interested in submitting a PR for this?

I'll give it a shot and we'll see what happens

how would it work if we ever want to add more metadata to a scene that isn't just another entity?

I like what @aevyrie suggested but I don't think it's possible to have multiple top level values. In that case we could simply wrap it all in a struct:

Scene( // optional
  metadata1: ...
  metadata2: ...
  entities: {
    0: [
      bevy_transform::components::transform::Transform(
        translation: glam::vec3::Vec3(0.0, 0.0, 0.0),
        rotation: glam::quat::Quat(0.0, 0.0, 0.0, 1.0),
        scale: glam::vec3::Vec3(1.0, 1.0, 1.0),
      ),
      ...
    ],
    ...
  }
)

@alice-i-cecile
Copy link
Member

Sounds good. @AronDerenyi, feel free to reach out on the PR thread or on Discord if you need a hand with the PR :)

@james7132
Copy link
Member

Just a small note about external compatibility: RON isn't all that well supported outside of the Rust ecosystem. Until a fully-capable scene editor is available in some format, some form of consideration for third-party tooling should be kept in mind.

@aevyrie
Copy link
Member

aevyrie commented Mar 8, 2022

Just a small note about external compatibility: RON isn't all that well supported outside of the Rust ecosystem. Until a fully-capable scene editor is available in some format, some form of consideration for third-party tooling should be kept in mind.

@james7132 I (think) we would always have the option to deserialize into another format like JSON, we would just need to implement ser/de for the scene type. Are there external tools you have in mind that would consume bevy's custom scene format?

I like what @aevyrie suggested but I don't think it's possible to have multiple top level values. In that case we could simply wrap it all in a struct:
...

@AronDerenyi FWIW this looks shockingly similar to the example in the ron-rs readme:
https://github.com/ron-rs/ron#same-example-in-ron

@AronDerenyi
Copy link
Contributor Author

@AronDerenyi FWIW this looks shockingly similar to the example in the ron-rs readme:
https://github.com/ron-rs/ron#same-example-in-ron

Not gonna lie, that gave me the idea to add the "Scene" to the beginning.

@cart
Copy link
Member

cart commented Mar 8, 2022

In the early days of bevy i tried making "struct name style" RON work for dynamic scenes, but couldn't do it, because RON (and serde for that matter) were designed under the assumption that we are serializing / deserializing "static" structs where you already know the type before you start deserializing. I have yet to see any RON that uses "fully qualified type names" in the struct-location, because (at least last time I checked) this is hard coded functionality that uses serde-provided short names. We will need to support fully qualified type names to resolve naming conflicts. After my initial investigation, I was also under the assumption that dynamically deserializing based on struct name alone wasn't possible without forking RON (which I did do 😄). Ultimately I decided it wasn't worth it because it couldn't support fully qualified type names.

@lassade has done some great work in this space here: https://github.com/lassade/bevy_prefab. It seems like they have somehow managed to dynamically load via struct names (see the arbitrary lists of prefab types and lists of component types in the .prefab files). But it also still appears to use the "serde provided short name". As soon as naming conflicts occur, I suspect that will become a problem. I do really like the UX and format they've come up with though.

@james7132
Copy link
Member

@james7132 I (think) we would always have the option to deserialize into another format like JSON, we would just need to implement ser/de for the scene type. Are there external tools you have in mind that would consume bevy's custom scene format?

So long as it works strictly with Reflect without needing explicit Serialize/Deserialize implementations, this could tentatively work.

@Metadorius
Copy link

GitHub didn't link the mention so I'll do it myself 😄
I made a similar suggestion on #4561 (comment), which doesn't allow multiple components (because why should it if ECS doesn't) is also not limited to RON only. While the proposed approach is certainly nice I think it's wrong to represent sets with arrays.

@alice-i-cecile
Copy link
Member

Completed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

8 participants