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

Internal rewrite of bevy_asset with slight api changes. #272

Closed

Conversation

lachlansneff
Copy link
Contributor

@lachlansneff lachlansneff commented Aug 21, 2020

These changes started as an async (async as in an async runtime) rewrite of the bevy_asset crate. During that process, I removed AssetLoadRequestHandler, redesigned how AssetLoaders are stored and accessed, and changed the bevy_asset api slightly, making it a little more typesafe and consistent. In the end, I removed the async runtime because async filesystem io isn't actually async and instead added a rayon::ThreadPool to do work in the background. Since the threadpool is isolated from the global pool, it should be alright to run io jobs on it. It would be easy enough to add back support for "async", but I don't think it's really that useful in this case.

The most notable api change is that AssetLoader now has Asset as an associated type, rather than as a type parameter. I think this is better, but I may be able to find a way to change it back if preferred.

@karroffel karroffel added A-Assets Load files from disk to use for things like images, models, and sounds C-Code-Quality A section of code that is hard to understand or change labels Aug 21, 2020
@StarArawn
Copy link
Contributor

It would be easy enough to add back support for "async", but I don't think it's really that useful in this case.

If we want to support web we likely will need async loading.

@erlend-sh
Copy link

Are you aware of #26 and the ongoing discussion about possibly adopting atelier-assets?

@lachlansneff
Copy link
Contributor Author

lachlansneff commented Aug 21, 2020

@StarArawn Good point, it probably should stay in.

@erlend-sh I didn't! atelier-assets looks good, if a little complex. Would be great to see what @cart thinks about it.

@cart
Copy link
Member

cart commented Aug 21, 2020

The Scenes focus area also intentionally includes Asset features that will force us to make a decision about atelier-assets. The changes in this pr all seem like nice iterative improvements to the current system, but I don't want to go too far down the rabbit hole of making the current system better, when we might throw it all away 😄

@lachlansneff
Copy link
Contributor Author

Okay, cool, I'll close this then.

@cart
Copy link
Member

cart commented Aug 22, 2020

Cool. Lets remember that this is here and we can pick it up if / when the time comes 😄

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants