-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update to Bevy 0.12. #43
Update to Bevy 0.12. #43
Conversation
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the queue -> prepare set change is required then I'd prefer if the systems were renamed.
Other than that LGTM. I'd prefer if the bind group entries api was used, but I'll do that once this PR is merged.
@@ -724,7 +723,8 @@ pub fn render_app_builder(app: &mut App) { | |||
) | |||
.add_systems( | |||
Render, | |||
(queue_infinite_grids, queue_grid_view_bind_groups).in_set(RenderSet::Queue), | |||
(queue_infinite_grids, queue_grid_view_bind_groups) | |||
.in_set(RenderSet::PrepareBindGroups), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand why this is necessary. I understand that the sets were reordered but the Queue set should still be used to queue things. I haven't touched this code that much though so it's possible it's doing something incompatible with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem was that you put bind group creation and queuing into the same system, while Bevy 0.12 wants them to be in separate systems. I've split up the logic appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thank you for the investigation, I didn't implement the crate originally so I wasn't aware of that.
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
Everything seems to work in both the simple example and in my own app.
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
OK, I discovered that the shadow was broken, so this PR fixes that too. That required some fairly large surgery. I've also updated the PR to use the This should be ready to go now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for all the work!
5897bb6
into
ForesightMiningSoftwareCorporation:main
Thanks @pcwalton! |
Everything seems to work in both the simple example and in my own app.
The one thing I wasn't sure about was whether to rename the
queue_
methods toprepare_
now that they've been moved to thePrepareBindGroups
render set. They had to be moved there because in Bevy 0.12Queue
executes beforePrepare
, whereas this crate expected them to execute in the other order.