feat: Add prefab loading

[Rhuagh]

---

This change is 

[torkleyy]

Wow could we merge those formatting changes in a separate PR?

[torkleyy]

Wow could we merge those formatting changes in a separate PR?

[Xaeroxe]

Nicely done!

[Rhuagh]

This should be cleaner now with the formatting shit out of the way.

[Rhuagh]

This should be cleaner now with the formatting shit out of the way.

[torkleyy]

Is this ready for review?

[torkleyy]

Is this ready for review?

[torkleyy]

Reviewed 7 of 18 files at r1.
Review status: 6 of 17 files reviewed at latest revision, all discussions resolved.

---

amethyst_assets/Cargo.toml, line 33 at r1 (raw file):

rayon = "1.0.1"
serde = { version = "1", features = ["serde_derive"] }
shred = { version = "0.7" }

Ah, so the strategy with amethyst_core doesn't work everywhere..

---

amethyst_core/src/transform/components/mod.rs, line 13 at r1 (raw file):

/// Prefab component data for Transform
#[derive(Default, Deserialize, Serialize, Debug, Clone)]
pub struct TransformPrefabData {

Is this extra struct necessary?

---

examples/prefab/README.md, line 1 at r1 (raw file):

# Prefab Example

I don't see why we'd need a readme in every example directory. It's more important to have them all listed in examples/README.md

---

examples/prefab/resources/display_config.ron, line 7 at r1 (raw file):

  fullscreen: false,
  multisampling: 0,
  title: "Assets example",

"Prefab example"

---

examples/prefab/resources/display_config.ron, line 9 at r1 (raw file):

  title: "Assets example",
  visibility: true,
  vsync: true,

This was causing problems on some platforms, I think we turned them off by default because of that. Maybe it's fixed though?

---

Comments from Reviewable

[torkleyy]

Reviewed 7 of 18 files at r1.
Review status: 6 of 17 files reviewed at latest revision, all discussions resolved.

---

amethyst_assets/Cargo.toml, line 33 at r1 (raw file):

rayon = "1.0.1"
serde = { version = "1", features = ["serde_derive"] }
shred = { version = "0.7" }

Ah, so the strategy with amethyst_core doesn't work everywhere..

---

amethyst_core/src/transform/components/mod.rs, line 13 at r1 (raw file):

/// Prefab component data for Transform
#[derive(Default, Deserialize, Serialize, Debug, Clone)]
pub struct TransformPrefabData {

Is this extra struct necessary?

---

examples/prefab/README.md, line 1 at r1 (raw file):

# Prefab Example

I don't see why we'd need a readme in every example directory. It's more important to have them all listed in examples/README.md

---

examples/prefab/resources/display_config.ron, line 7 at r1 (raw file):

  fullscreen: false,
  multisampling: 0,
  title: "Assets example",

"Prefab example"

---

examples/prefab/resources/display_config.ron, line 9 at r1 (raw file):

  title: "Assets example",
  visibility: true,
  vsync: true,

This was causing problems on some platforms, I think we turned them off by default because of that. Maybe it's fixed though?

---

Comments from Reviewable

[Rhuagh]

You can review, but it's not even close to complete :)

[Rhuagh]

You can review, but it's not even close to complete :)

[Rhuagh]

Review status: 6 of 17 files reviewed at latest revision, 4 unresolved discussions.

---

amethyst_core/src/transform/components/mod.rs, line 13 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Is this extra struct necessary?

Not really, no, I could make the impl PrefabData for Transform add GlobalTransform::default() instead, and just use Transform directly.

---

examples/prefab/README.md, line 1 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I don't see why we'd need a readme in every example directory. It's more important to have them all listed in examples/README.md

I just copied the assets example, not cleaned up yet, but I agree.

---

examples/prefab/resources/display_config.ron, line 9 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

This was causing problems on some platforms, I think we turned them off by default because of that. Maybe it's fixed though?

They're still on in most examples? If you don't use vsync some platforms get thread starvation problems I believe.

---

Comments from Reviewable

[Rhuagh]

Review status: 6 of 17 files reviewed at latest revision, 4 unresolved discussions.

---

amethyst_core/src/transform/components/mod.rs, line 13 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Is this extra struct necessary?

Not really, no, I could make the impl PrefabData for Transform add GlobalTransform::default() instead, and just use Transform directly.

---

examples/prefab/README.md, line 1 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

I don't see why we'd need a readme in every example directory. It's more important to have them all listed in examples/README.md

I just copied the assets example, not cleaned up yet, but I agree.

---

examples/prefab/resources/display_config.ron, line 9 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

This was causing problems on some platforms, I think we turned them off by default because of that. Maybe it's fixed though?

They're still on in most examples? If you don't use vsync some platforms get thread starvation problems I believe.

---

Comments from Reviewable

[Rhuagh]

Reviewed 18 of 18 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable

[Rhuagh]

Reviewed 18 of 18 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable

[Rhuagh]

Review status: 11 of 17 files reviewed at latest revision, 4 unresolved discussions.

---

amethyst_assets/Cargo.toml, line 33 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Ah, so the strategy with amethyst_core doesn't work everywhere..

No, derive requires it to be at base level :/ We should look at how serde do derive for shred.

---

Comments from Reviewable

[Rhuagh]

Review status: 11 of 17 files reviewed at latest revision, 4 unresolved discussions.

---

amethyst_assets/Cargo.toml, line 33 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

Ah, so the strategy with amethyst_core doesn't work everywhere..

No, derive requires it to be at base level :/ We should look at how serde do derive for shred.

---

Comments from Reviewable

[omni-viral]

I like it mostly. But it seems a bit more complex than necessary.

[omni-viral]

Can't you implement PrefabData for Transform directly?

[Rhuagh]

The complexity needs to be there I believe, I will demonstrate later why. The first prefab data entries are simple to load, the animation stuff as an example is much more complex

[Rhuagh]

The complexity needs to be there I believe, I will demonstrate later why. The first prefab data entries are simple to load, the animation stuff as an example is much more complex

[torkleyy]

👍 I really love this PR, looks like a lot of work to review though. This will be really helpful for the usability of the engine. Please tell me if there's any part of this that can be reviewed already.

[torkleyy]

👍 I really love this PR, looks like a lot of work to review though. This will be really helpful for the usability of the engine. Please tell me if there's any part of this that can be reviewed already.

[Rhuagh]

I believe most of this is ready for review now. I'm gonna add a chapter to the book as part of the PR also. Will write that tomorrow hopefully.

---

Review status: 1 of 81 files reviewed at latest revision, 5 unresolved discussions.

---

examples/prefab/resources/display_config.ron, line 7 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

"Prefab example"

Done.

---

Comments from Reviewable

[Rhuagh]

I believe most of this is ready for review now. I'm gonna add a chapter to the book as part of the PR also. Will write that tomorrow hopefully.

---

Review status: 1 of 81 files reviewed at latest revision, 5 unresolved discussions.

---

examples/prefab/resources/display_config.ron, line 7 at r1 (raw file):

Previously, torkleyy (Thomas Schaller) wrote…

"Prefab example"

Done.

---

Comments from Reviewable

[torkleyy]

Review status: 7 of 81 files reviewed at latest revision, 5 unresolved discussions.

---

Comments from Reviewable

[torkleyy]

Review status: 7 of 81 files reviewed at latest revision, 5 unresolved discussions.

---

Comments from Reviewable

[torkleyy]

A book chapter would be tops :)

[omni-viral]

Could you write how loading of complex prefab will look like?

[omni-viral]

Please, minimize trait bounds on struct and enum definitions.
Those for<'a> Deserialize<'a> + Serialize seems unnecessary here.

[Rhuagh]

They are required for deriving Deserialize/Serialize, without them it won't build.

[omni-viral]

Maybe DeserializeOwned?

[Xaeroxe]

That would probably be preferable as for<'b> Deserialize<'b> is effectively synonymous with DeserializeOwned. You can't meet that criteria without meeting DeserializeOwned.

[omni-viral]

This confuses me.
Why not perform conversion manually?

[Rhuagh]

Not sure if this is actually required anymore, it's a remnant of the old gltf loader system.

[omni-viral]

How exactly PrefabData must know how to interpret those entities?

[Rhuagh]

This should be documented on Prefab and PrefabData. If not, I need to add it.

[Rhuagh]

Most of the features needed are in place now, the big glaring weaknesses are that it's not possible to target existing entities from inside a prefab (except for the entity the prefab handle is placed on), and there's no caching of assets for the most part. Some of the prefab impls can cache internally, but not all of them. I'd also like to consolidate some of the assets prefab implementations.

But all of those can be deferred for later PRs, the system is possible to work with now.

[Rhuagh]

Most of the features needed are in place now, the big glaring weaknesses are that it's not possible to target existing entities from inside a prefab (except for the entity the prefab handle is placed on), and there's no caching of assets for the most part. Some of the prefab impls can cache internally, but not all of them. I'd also like to consolidate some of the assets prefab implementations.

But all of those can be deferred for later PRs, the system is possible to work with now.

[Rhuagh]

Conceptually, a prefab is a list of entities, where the first entry in the list refers to the "main" entity, i.e. the entity the prefab handle is placed. All other entries in the list will spawn a new entity when instantiated.

Each entry in the list contains an optional parent index and an optional data container. The data container is a generic type that implement PrefabData.

When a prefab is loaded it is processed in two stages:

Sub asset loading:

This is kicked off during AssetStorage::process in the PrefabLoaderSystem, and basically do a depth first visit in the data container and starts any secondary asset loading needed, such as reading other assets from disc, or transferring data to GPU memory etc. When doing so it creates an internal ProgressCounter, and if any secondary loads were started, it will not finish the process, but instead return a result that signifies to the storage that it needs to reprocessed at a later stage. During this visit walk, the data can morph inside the prefab, as an example, if there are AssetPrefabs inside, it will trigger a load of the asset, and replace itself with the returned handle.

Once all sub loading has completed (it will use the internally created ProgressCounter to track this), it will signal done to AssetStorage::process, which in turn will update the progress tracker on the higher level prefab load etc up to the user requested level. Once this is complete all data has been read from disc, and resides somewhere in memory (either on gfx card or ram).

Prefab instantiation:

This happens when a prefab handle is added onto an Entity, and at this point the data in the prefab is basically cloned and an entity hierarchy is created with the main entity as the root. The prefab data is immutable at this point.

[Rhuagh]

Conceptually, a prefab is a list of entities, where the first entry in the list refers to the "main" entity, i.e. the entity the prefab handle is placed. All other entries in the list will spawn a new entity when instantiated.

Each entry in the list contains an optional parent index and an optional data container. The data container is a generic type that implement PrefabData.

When a prefab is loaded it is processed in two stages:

Sub asset loading:

This is kicked off during AssetStorage::process in the PrefabLoaderSystem, and basically do a depth first visit in the data container and starts any secondary asset loading needed, such as reading other assets from disc, or transferring data to GPU memory etc. When doing so it creates an internal ProgressCounter, and if any secondary loads were started, it will not finish the process, but instead return a result that signifies to the storage that it needs to reprocessed at a later stage. During this visit walk, the data can morph inside the prefab, as an example, if there are AssetPrefabs inside, it will trigger a load of the asset, and replace itself with the returned handle.

Once all sub loading has completed (it will use the internally created ProgressCounter to track this), it will signal done to AssetStorage::process, which in turn will update the progress tracker on the higher level prefab load etc up to the user requested level. Once this is complete all data has been read from disc, and resides somewhere in memory (either on gfx card or ram).

Prefab instantiation:

This happens when a prefab handle is added onto an Entity, and at this point the data in the prefab is basically cloned and an entity hierarchy is created with the main entity as the root. The prefab data is immutable at this point.

[Rhuagh]

After #760 .

[Rhuagh]

After #760 .

[Rhuagh]

This is now ready, this is just the first stage, I have atleast 4 followup PRs that add additional things on top of this:

• Convert GLTF loader to use this
• UI specific ron based format
• Shape prefabs
• Basic scene utility prefabs
• Examples improvements

[Rhuagh]

This is now ready, this is just the first stage, I have atleast 4 followup PRs that add additional things on top of this:

• Convert GLTF loader to use this
• UI specific ron based format
• Shape prefabs
• Basic scene utility prefabs
• Examples improvements

[Rhuagh]

r? @torkleyy @omni-viral @Xaeroxe @jojolepro

[Rhuagh]

r? @torkleyy @omni-viral @Xaeroxe @jojolepro

[Xaeroxe]

This looks great! Thanks!

I've got a few doc nits here and there. My biggest concern with this as it stands is that the prefab interpretation requires advance knowledge on the component types in the prefab files. I don't have a good solution for this right now so I can't block on it but I'd really like to be able to create a loader that can "discover" the type of the component as it goes, even for custom user components.

I'm aware this isn't a reasonable ask at this time though. I'll probably refactor this as soon as I come up with a viable way to do this.

[Xaeroxe]

Missed a "b"

[Xaeroxe]

That would probably be preferable as for<'b> Deserialize<'b> is effectively synonymous with DeserializeOwned. You can't meet that criteria without meeting DeserializeOwned.

[Xaeroxe]

Missing doc

[Xaeroxe]

Remove commented out code please

[Xaeroxe]

What does it take to remove this restriction? It's pretty easy to run into this in say an RTS game.

[Rhuagh]

I don't know at this point, I've not thought about it a lot. You'll need some kind of persistent addressing scheme for the entities, probably Marker from saveload. It is also fairly simple to cause UB with that, because you can easily create hierarchy loops.

[Xaeroxe]

This run function is no longer necessary, main() can return results. This should mimic the other examples.

[Rhuagh]

@Xaeroxe i don't see a way to do dynamic loading. At some point you'll need to insert into component storage and there you will need the type, and there's no way to create types at runtime. You'd need to do things completely unsafe, transmuting raw pointers most likely.

[Rhuagh]

@Xaeroxe i don't see a way to do dynamic loading. At some point you'll need to insert into component storage and there you will need the type, and there's no way to create types at runtime. You'd need to do things completely unsafe, transmuting raw pointers most likely.

[Xaeroxe]

@Rhuagh I was thinking more along the lines of trait objects and a HashMap between a type identifier string and the function required to insert the component. Said function would have to take a Deserializer parameter and perform its own deserialization of the data. Since the type can't be predicted in advance it'd probably rely on lazy updates very heavily. I'm not sure how many problems I'd run into while trying to implement this but it's such a big add-on to the existing work that I'd rather not block this PR on it.

[Xaeroxe]

@Rhuagh I was thinking more along the lines of trait objects and a HashMap between a type identifier string and the function required to insert the component. Said function would have to take a Deserializer parameter and perform its own deserialization of the data. Since the type can't be predicted in advance it'd probably rely on lazy updates very heavily. I'm not sure how many problems I'd run into while trying to implement this but it's such a big add-on to the existing work that I'd rather not block this PR on it.

[Rhuagh]

That would make it difficult to deserialize the data in advance of usage I believe.

[Rhuagh]

That would make it difficult to deserialize the data in advance of usage I believe.

[Xaeroxe]

That's true. So perhaps what you'd want to do instead is deserialize it using similar techniques but rather than inserting the components you keep them in the prefab structure wrapped in a Vec of Box<Fn(&LazyUpdate, Entity)> where each Fn(&LazyUpdate, Entity) is a closure that captures the component and inserts it with LazyUpdate when the closure is called.

[Xaeroxe]

That's true. So perhaps what you'd want to do instead is deserialize it using similar techniques but rather than inserting the components you keep them in the prefab structure wrapped in a Vec of Box<Fn(&LazyUpdate, Entity)> where each Fn(&LazyUpdate, Entity) is a closure that captures the component and inserts it with LazyUpdate when the closure is called.

[Xaeroxe]

LGTM! Thanks!

[Xaeroxe]

bors r+

[Xaeroxe]

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push
• continuous-integration/appveyor/branch

[bors]

Build succeeded

• continuous-integration/travis-ci/push
• continuous-integration/appveyor/branch