Allow adding and removing asset sources at runtime.

[andriyDev]

Objective

• Fixes Allow adding and removing asset sources at runtime. #21758.
• Fixes Custom AssetSource(s) must be registered before DefaultPlugins in order to work #10162.

Solution

• Make the AssetPlugin include a DefaultAssetSource enum to initialize the default source when adding the AssetPlugin.
  • Note: DefaultAssetSource needs to contain a Mutex, since we need to be able to use the builder, but Plugin only provides us with a shared reference in the plugin.
• Put a RwLock around the asset sources.
• Add functions for adding and removing sources.
• Make the existing register_asset_source only work for non-default sources, and have them just use the add/remove functions.
• Temporary hack: prevent adding processed sources during runtime - we only allow unprocessed sources.
  • Practically, I don't expect users to want to add processed sources, but in the future they will. For this PR, I want to ignore that so we can solve 80% of the issues with 20% of the effort (I took a stab at making this work for processed sources too - it's possible but it's a pain).

One thing that is not addressed is the behavior after removing an asset source. This PR leaves it in a state where assets loaded from that asset source remain loaded. If you re-add the asset source, it does not reload the asset. But if the asset changes on the asset source, it should reload again. I'm kinda leaving it undefined since it's not clear what the correct behavior here should be anyway.

Testing

• Added tests!

[greeble-dev]

Couldn't spot any issues and tests passed. Just added some minor comments.

One thing I was left wondering about - do readers and writers still need to be passed to AssetSourceBuilder as closures that produce the value? I understand this was necessary when sources weren't shared. But is it true now that they're shared?

[kfc35]

I read the whole thing and could not find any logical issues. Comments are just suggestions to improve readability / any future debugging. I also checked out your branch and also verified that tests pass. Thanks for adding such clear tests!

In case you wanted my 2c on expected behavior for asset server removal, what you said sounds reasonable. If you waaaanted to, you could note the behavior in your release notes but say that this behavior may change depending on community input and see if anyone says anything / has suggestions, but I leave that to you!

[andriyDev]

@kfc35

If you waaaanted to, you could note the behavior in your release notes

Yeah I considered this. I decided to avoid including it just because I'm sure everyone will have their own "obviously correct behavior", where some will be varying degrees of painful-to-implement. I don't think anyone will notice this weirdness for a long time. Kinda a bad reason, but I don't feel like opening that can of worms lol.

[alice-i-cecile]

@andriyDev this isn't going to make 0.18, but when you get a chance can you resolve the merge conflicts?

[andriyDev]

@alice-i-cecile Rebased! I also added an additional commit to clean up some cases that didn't need their own test setup, thanks to this PR. Fewer separate test setup code means we're less likely to accidentally start reading the FS again.