Replace all different load variants in AssetServer with a builder.

[andriyDev]

Objective

• In 0.18, we had 10 different functions that load assets (I'm not even counting load_folder).
• In 0.19, we've even added load_erased - but it unfortunately doesn't support all the features that the other variants support.
• We apparently needed load_acquire_with_settings_override which 1) loads the asset, 2) uses the settings provided, 3) allows reading unapproved asset paths, and 4) drops a guard once the load completes.
  • That's fine if that's necessary. But we needed to create an explicit variant for that.
• We need fewer load paths!

Solution

• Create a builder.
• Store all these options dynamically instead of statically handling each case.
• Have the caller choose a particular "kind" of load when they are ready: load, load_erased, load_untyped, or load_untyped_async.
  • I intentionally didn't provide a load_async or load_erased_async, since those can be replicated using load/load_erased + AssetServer::wait_for_asset_id to get the exact same effect.

I am also intentionally leaving NestedLoader untouched in this PR, but a followup will duplicate this API for NestedLoader, which should make it easier to understand.

Unlike the NestedLoader API, we aren't doing any type-state craziness, so the docs are much more clear: users don't need to understand how type-state stuff works, they just call the handful of methods on the type. The "cost" here is we now need to be careful about including the cross product of loads between static asset type, runtime asset type, or dynamic asset type, crossed with deferred or async. In theory, if we added more kinds on either side, we would need to expand this cross product a lot. In practice though, it seems unlikely there will be any more variants there. (maybe there could be a blocking variant? I don't think this is a popular opinion though).

A big con here is some somewhat common calls are now more verbose. Specifically, asset_server.load_with_settings() has become asset_server.load_builder().with_settings().load(). I am not really concerned about this though, since it really isn't that painful.

Testing

• Tests all pass!

---

Showcase

Now instead of:

asset_server.load_acquire_with_settings_override("some_path", |settings: &mut GltfLoaderSettings| { ... }, my_lock_guard);

You can instead do:

asset_server.load_builder()
    .with_guard(my_lock_guard)
    .with_settings(|settings: &mut GltfLoaderSettings| { ... })
    .override_unapproved()
    .load("some_path");

We also now cover more variants! For example, you can now load an asset untyped with a guard, or with override_unapproved, etc.

[greeble-dev]

Looks good, just got some nitpicky comments.

Also thinking that load_builder could be bikeshedded as it's a teeny bit verbose? LoadContext uses loader. Maybe build is an option? I suspect both of these end up too ambiguous though.

[greeble-dev]

Suggested change

	title: Advanced AssetServer load variants are now expose through a builder pattern.
	title: Advanced AssetServer load variants are now exposed through a builder pattern.

[andriyDev]

Done.

[greeble-dev]

Suggested change

	Every load variant above can be reimplemented using `load_builder`, and each one of these methods
	has deprecation messages on them explaining their new equivalent.
	Every load variant above can be reimplemented using `load_builder`, and each one of these methods
	has deprecation messages on them explaining their new equivalent.
	For example, `load_with_settings_override` can be replaced by:
	```rust
	asset_server
	.load_builder()
	.with_settings(settings)
	.override_unapproved()
	.load(path)

I thought one example here might be nice so the user gets the flavor of things.

[andriyDev]

Done. Nice!

[greeble-dev]

Suggested change

	self.load_builder().load(path.into())
	self.load_builder().load(path)

I don't think the into is necessary? Unless there's an optimization reason?

[andriyDev]

You're correct it's not necessary, but I believe it should reduce the number of monomorphizations, since it will always call load where the Into is just AssetPath.

[greeble-dev]

Maybe load_erased and load_untyped should be kept? They're convenient and the duplication is modest.

[andriyDev]

Seeing as how load_erased was added this cycle, I don't think it's important. As for load_untyped, this also has a mega ugly API so I don't really want to make it convenient to use - load should be the front-door and all other loads are considered "advanced" IMO.

[alice-i-cecile]

Thoughts:

• wow, there are very much too many asset loading APIs
• I share @cart's distaste for builders
• this is a large improvement over the status quo
• this is well-executed! good docs, sensible API, clean migration story

I think we should fix this, and do so in time for 0.19, but instead of a builder pattern we should keep it simple:

• load(), which takes no settings at all
• load_with, which takes an LoadOptions struct. This impls Default for nice partial initialization, and wraps all over the other config that you might have.

load_with_settings is the tricky bit here, because it's generic, but I think we should be able to pass in a generic which defaults to () without too big of a hit.

[alice-i-cecile]

I've been convinced that my proposal is unworkable due to differing return types.

[andriyDev]

For any code-archaeologists, here was the discussion: https://discord.com/channels/691052431525675048/749332104487108618/1494030066940903434

TL;DR a descriptor style API would still leave a bunch of extra variants in the AssetServer to support the different return types of the load functions, which is what we're trying to avoid!