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

Fix ambiguous queue_material_tilemap_meshes ordering #523

Merged
merged 2 commits into from
May 28, 2024

Conversation

MeoMix
Copy link
Contributor

@MeoMix MeoMix commented Mar 19, 2024

It was possible for queue_material_tilemap_meshes to run prior to prepare which would result in nothing rendering.

This would only occur in scenarios where other code inserted a system dependent on mut commands: Commands into the Queue set. The bevy-egui library was doing so and was thus introducing behavior where bevy-ecs-tilemap would intermittently not render anything due to improper system ordering.

Fixes: #521

Additional Details

djeedai/bevy_hanabi#297 Bevy Hanabi experienced a similar issue. Their fix looks similar to this proposed fix.

The Bevy notes hint that this sort of change is necessary, too:

image

This is not directly an issue with automatically applying command buffers. It is related, but the key issue here is one of improper system ordering. The existing codebase happened to work but was not guaranteed to work. It worked only because the scheduler was choosing to resolve ambiguous system ordering in a way which was compatible with bevy_ecs_tilemap architecture. This invited the problem where third party code could introduce systems that would affect the scheduler's ordering decision and result in rendering failures.

It was possible for `queue_material_tilemap_meshes` to run prior to `prepare` which would result in nothing rendering.

This would only occur in scenarios where other code inserted a system dependent on `mut commands: Commands` into the `Queue` set. The `bevy-egui` library was doing so and was thus introducing behavior where `bevy-ecs-tilemap` would intermittently not render anything due to improper system ordering.
@StarArawn
Copy link
Owner

@MeoMix can you run cargo fmt on this and then it's good to merge!

@MeoMix
Copy link
Contributor Author

MeoMix commented Apr 17, 2024

@StarArawn Feel free to merge? I don't have the permission to do so

@jgayfer jgayfer mentioned this pull request Apr 21, 2024
@StarArawn StarArawn merged commit e4f3cc6 into StarArawn:main May 28, 2024
8 checks passed
@MeoMix MeoMix deleted the patch-2 branch June 15, 2024 16:43
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

Successfully merging this pull request may close these issues.

Bevy 0.13 race condition resulting in tiles not rendering
2 participants