Loader/Asset API Ergonomics

[kazimuth]

The current loader API is (subjectively) a little heavy.

For example, to load a GLTF file:

let loader = world.read_resource();
let progress = ();
let format = GltfSceneFormat;
let options = Default::default();
let storage = &world.read_resource();

let asset = loader.load("path/to/gltf.gltf", GltfSceneFormat, options, progress, storage);

And to load a GLTF file from a custom source:

let loader = world.read_resource();
let progress = ();
let format = GltfSceneFormat;
let options = Default::default();
let source = /*...*/;
let storage = &world.read_resource();

let asset = loader.load_from("path/to/gltf.gltf", GltfSceneFormat, options, source, progress, storage);

I think this API could be made slightly cleaner by doing (any subset of) a few things:

• Dynamically dispatch on the resource URL to determine the source and format (maybe integrating with the vnodes system)
• Put the storages within the Loader / give the Loader some way to get handles to the storages, so that the user doesn't have to
• Use the builder pattern

Then, a simple asset load can look like just:

let asset = loader.asset("/io/assets/path/to/texture.png").load();

And a complex load can look like:

let asset = loader
    .asset("/io/assets/special_source/path_to_scene.gltf")
    .progress(progress_counter)
    .options(GltfSceneOptions { /* ... */ })
    .custom_storage(custom_storage) // not sure if this would be needed
    .load();

Since loading assets is something that you do a lot, I think it's worth it to make the API nice to use.

Downsides:

• Makes things slightly more prone to runtime errors (can have file-type mismatches)
• Gives Loader more responsibilities

[minecrawler]

I like the builder pattern! Though I think that the storage selection might be a bit difficult, as you can load meshes, textures, material files, .... with the same method at the moment. In order to solve that problem, maybe we can introduce specialized methods?

vnodes.insert(
  "/assets/dummy/texture",
  loader.texture("/io/assets/path/to/texture.png").load()?
);
vnodes.insert(
  "/assets/dummy/mesh",
  loader.mesh("/io/assets/path/to/mesh.gltf").load()?
);

// ...

world
  .create_entity()
  .with(vnodes.get("/assets/dummy/texture")?.clone())
  .with(vnodes.get("/assets/dummy/mesh")?.clone())
  /* ... */
  .build()
;

As for deciding on the format, I think we might learn a bit from how Assimp does that.

[Rhuagh]

I don't see a way that we could add the storage lookup inside Loader, without mutably borrowing World, which you don't have access to in the Systems.

I believe that if we want to do something like this we should build it on top of Loader to do proper separation of concerns.

Dynamic lookup of formats is also quite hard without boxing.

[kazimuth]

I don't see a way that we could add the storage lookup inside Loader, without mutably borrowing World, which you don't have access to in the Systems.

Yeah :/ I don't know what the right way to do this is.

Dynamic lookup of formats is also quite hard without boxing.

If you box the Format impls, that shouldn't have any overhead, right? It'll just be a vtable lookup when you load the file:

let texture_formats: HashMap<String, Box<Format<Texture>>> = hashmap! {
  "jpg" => Box::new(JpegFormat),
  "png" => Box::new(PngFormat),
  // ...
};

// `asset` is unboxed
let asset = formats[file.file_extension()].import(name, file, ...);

So, import can't be inlined, but it probably shouldn't be inlined anyway. This has minimal overhead, the time compared to do the vtable lookup is tiny compared to the time to actually load the asset.

Options could work by using some Any magic:

trait Format<A: Asset> {
    fn import(&self, options: &Any, /*...*/)
       -> Result<FormatValue<A>>;
}
impl Format<Texture> for JpegFormat {
   fn import(&self, options: &Any, /*...*/) {
       let options = options.downcast_ref::<JpegOptions>().or_else(|| {
          error!("mismatched option type");
          Default::default()
       });
       // ...
   }

This is a pretty awkward api, but it does work, and won't be used by users too often unless they implement their own formats.

(It's unclear to me whether the added complexity is worth it here tbh.)

[torkleyy]

I think we already improved this with the prefab system. Do you still see an issue here @kazimuth? I share your concerns that this needs a couple of lines, but I wanted things to be very flexible and that's just the best way I found. I think what's more important than the number of lines is how easy it is to grasp, and I think that we're good here.

[Rhuagh]

By providing wrapper "Formats" like TextureFormat this is a smaller issue atleast.

[AnneKitsune]

Dynamically dispatch on the resource URL to determine the source and format (maybe integrating with the vnodes system

I tried to do that last month, and it didn't work very well...
Maybe it can give you some ideas: https://github.com/jojolepro/amethyst-extra/blob/master/src/lib.rs#L144

[Moxinilian]

Regarding the amount of resources requested, we can't add all the storages inside the loader. However, we can have the loader inside every asset storages, and we would have storage.load(&mut self, ...). Afaik, nobody requires Write<Loader> so this won't break dispatching either.
When you're just loading textures, it saves instructions and makes it more natural to think about.

[kabergstrom]

I think a key insight is that you will get much simpler runtime game code by offloading parameters such as format and options to a build step,

With persistent asset identifiers and a multi-stage asset build pipeline you can reduce the game code to a load of an asset identifer. The runtime asset representation can then fill in the rest.
https://github.com/amethyst/amethyst/issues/875

[fhaynes]

Moving this to RFC repo