Skip to content

Allow adding and removing asset sources at runtime. #21890

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Allow adding and removing asset sources at runtime. #21890

wants to merge 15 commits into from
+962 −440

Conversation

@andriyDev
Copy link
Contributor

@andriyDev andriyDev commented Nov 20, 2025 •
edited
Loading

Objective

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!

@andriyDev andriyDev force-pushed the runtime-sources branch 2 times, most recently from 3be29fa to 5528a8c Compare November 20, 2025 04:11
@andriyDev andriyDev added C-Feature A new feature, making something new possible A-Assets Load files from disk to use for things like images, models, and sounds D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Release-Note Work that should be called out in the blog due to impact labels Nov 20, 2025
@andriyDev andriyDev added this to the 0.18 milestone Nov 20, 2025
greeble-dev
greeble-dev approved these changes Nov 20, 2025
View reviewed changes
Copy link
Contributor

@greeble-dev greeble-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

crates/bevy_asset/src/lib.rs
Comment on lines 336 to 370
pub enum DefaultAssetSource {
/// Create the default asset source given these file paths.
FromPaths {
/// The path to the unprocessed assets.
file_path: String,
/// The path to the processed assets.
///
/// If [`None`], the default file path is used when in [`AssetMode::Processed`].
processed_file_path: Option<String>,
},
/// Create the default asset source from the provided builder.
///
/// Note: The Mutex is just an implementation detail for applying the
/// plugin.
FromBuilder(Mutex<AssetSourceBuilder>),
}
Copy link
Contributor

@greeble-dev greeble-dev Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could consider making this DefaultAssetSource::Paths/Builder and have a function DefaultAssetSource::from_paths/from_builder? Feels slightly nicer to me, but not a big deal, and using the struct directly does make it easier to plug in default values.

Copy link
Contributor Author

@andriyDev andriyDev Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! This is especially good for the builder path, so users don't need to deal with the Mutex.

crates/bevy_asset/src/processor/mod.rs Outdated
/// A bool indicating whether the processor has started or not.
///
/// This is different from `state` since `state` is async, and we only assign to it once, so we
/// should ~never block on it.
Copy link
Contributor

@greeble-dev greeble-dev Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// should ~never block on it.
/// should never block on it.

Copy link
Contributor Author

@andriyDev andriyDev Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expanded on this since it's possible to block on this.

andriyDev and others added 14 commits November 21, 2025 00:00
@andriyDev
Store AssetSource as Arc in AssetSources.
eddf3a3
@andriyDev
Wrap the AssetSources in a RwLock.
fdd6a3b
@andriyDev
Allow adding and removing assets, and allow registering sources after…
6a2f33d 
… adding the AssetPlugin.
@andriyDev
Fix up regular asset tests to register sources after the AssetPlugin.
0809669
@andriyDev
Rewrite asset processing tests to allow adding sources piecemeal.
a027728
@andriyDev
Prevent adding or removing processed asset sources after processing h…
7853d99 
…as started.
@andriyDev
Create a test to show that an asset can be loaded from a runtime adde…
a4839e2 
…d source.
@andriyDev
Fix up all the dependents and examples to add the default source in t…
71bc5be 
…he AssetPlugin.
@andriyDev
Write a test to show that we can't add or remove processed sources af…
8bd59ab 
…ter the processor starts.
@andriyDev
Create a migration guide for registering asset sources.
6c360c8
@andriyDev
Create release notes for runtime asset sources.
86a92a0
@andriyDev @greeble-dev
Reword release notes.
4ac516a 
Co-authored-by: Greeble <[email protected]>
@andriyDev @greeble-dev
Reword error to be a more clear when adding sources at the wrong time.
ddd8634 
Co-authored-by: Greeble <[email protected]>
@andriyDev
Clarify blocking on the RwLock around the started flag.
efa0bcb
@andriyDev andriyDev force-pushed the runtime-sources branch 5 times, most recently from 64e3aac to b8059fa Compare November 21, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greeble-dev greeble-dev greeble-dev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Assets Load files from disk to use for things like images, models, and sounds C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Milestone

0.18

Development

Successfully merging this pull request may close these issues.

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

2 participants

@andriyDev @greeble-dev