search

fishfolk / bones

An easy-to-use game engine for making real games.
https://fishfolk.org/development/bones/introduction/
Other
210 stars 20 forks source link

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

Open zicklag opened 3 months ago

zicklag commented 3 months ago

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.

https://github.com/fishfolk/bones/blob/8240aea4bc444fea1b5a3ef6e2b4287edf36fa58/framework_crates/bones_asset/src/server.rs#L228-L231

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

MaxCWhitehead commented 3 months ago

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 commented 3 months ago

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.