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

Race condition in external level projects #232

Open
MScottMcBee opened this issue Sep 20, 2023 · 1 comment
Open

Race condition in external level projects #232

MScottMcBee opened this issue Sep 20, 2023 · 1 comment

Comments

@MScottMcBee
Copy link
Contributor

I have an LDTK project using external levels that has 34 levels. When I load a project file and select a level, process_ldtk_levels will sometimes fail to load the level. Inspecting the size of Assets<LdtkLevel> will show between 22 and 31.

What seems to be happening is that when the LdtkProject asset gets loaded it creates all the handles for the external levels, but loading asset dependencies isn't guaranteed to happen all at once. Since process_ldtk_levels is triggered by the creation of the handle, it's possible that your level just isn't there.

I was able to make a rough fix on my fork here by letting process_ldtk_levels run until the number of assets loaded matches the number of external files listed in a project.

Seeing as how Assets v2 is already in bevy's main branch, this might not be worth fixing at this exact moment.

@Trouv
Copy link
Owner

Trouv commented Sep 24, 2023

Hmm for some reason I was under the impression that AssetEvent::Created wouldn't fire for the parent asset until it and its dependencies were all loaded. I suppose that is not the case.

I was able to make a rough fix on my fork here by letting process_ldtk_levels run until the number of assets loaded matches the number of external files listed in a project.

An interesting idea. One thing I'd experiment with too is listening to the LdtkLevel asset events in addition to filtering for Added<Handle<LdtkLevel>>. Could maybe make a run condition for it.

Seeing as how Assets v2 is already in bevy's main branch, this might not be worth fixing at this exact moment.

I'm actually making a lot of changes to the asset types right now too, should be at a stopping point there soon.

Trouv pushed a commit that referenced this issue Jan 31, 2024
…ely loaded (#281)

Fixes #232 

Added a check so that when applying `LevelSelection`, there's a check to
make sure the external level is loaded.

Works for my game and all tests pass, not sure if this has any weird
knock-on effects.
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