Skip to content
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.

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

Asset Changes Are Handled Without Waiting for Asset Load to Finish #408

Open
zicklag opened this issue Jun 5, 2024 · 2 comments
Open

Asset Changes Are Handled Without Waiting for Asset Load to Finish #408

zicklag opened this issue Jun 5, 2024 · 2 comments

Comments

@zicklag
Copy link
Member

zicklag commented Jun 5, 2024

While going over the asset server with @TekhnaeRaav, he helped me realize that the asset server was responding to changed assets, triggering a forced asset reload, but then handing the asset change without waiting for the actual asset to be re-loaded.

match changed {
ChangedAsset::Loc(loc) => {
let handle = self.load_asset_forced(loc.as_ref());
pending_asset_changes.push(handle);

My brain is only partially consuming this right now, but that definitely seems wrong.

@MaxCWhitehead
Copy link
Collaborator

Maybe we need to wait on handle_change on pending_asset_changes until AssetLoadProgressis finished? Would need to persist pending_asset_changes somewhere.

Could wait until all pending are loaded and then flush handling changes, or figure out how to use a callback on each asset completing? I'm not too familiar with this code - but hopefully this might resolve issues with hot reloading sometimes crashing. Good find!

@zicklag
Copy link
Member Author

zicklag commented Jun 5, 2024

I feel like we might need another channel to listen to, maybe, that gets handles pushed to it when they finish loading. So one channel for asset source changes, which come from filesystem watchers or similar things, and another channel for asset load changes, that gets pushed to when an asset data has been loaded.

Beyond this, @TekhnaeRaav had a good point, while looking through the renderer, that we could use the asset load changes exclusively to trigger this bones_bevy_renderer image load step, instead of what we are doing now where we either have to check every frame, or we have to do all the bevy image loading up-front.

I don't want to do any major changes to the asset loader if we don't need to, because I think we probably want to integrate Iroh blobs pretty deeply into the asset server, which will be a big change that will hopefully replace our usage of dashmaps and some other things that might have been causing some problems.

So I'm not sure if it's worth trying to fix immediately, but if it's a small change, it'd probably be worth doing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants