Add configure_schedules to App and Schedules to apply ScheduleBuildSettings to all schedules#9514
Merged
alice-i-cecile merged 4 commits intobevyengine:mainfrom Aug 28, 2023
Conversation
alice-i-cecile
added
A-ECS
DevinLeamy
changed the title
ScheduleBuildSettings to all schedulesScheduleBuildSettings to all schedules
alice-i-cecile
approved these changes
Aug 20, 2023
Member
|
Thanks! |
hymm
approved these changes
Aug 21, 2023
Contributor
hymm
left a comment
hymm
left a comment
There was a problem hiding this comment.
I worry a little that there isn't a way to turn on say ambiguity detection for all schedules, but leave the rest of the settings untouched. But I think the predominant usage will be to just set all the schedules the same anyways, so going to approve and see what happens.
alice-i-cecile
added
the
S-Ready-For-Final-Review
Member
|
That can still be done manually, using the workaround in the linked issue plus struct update syntax :) If that turns out to be a common pattern in practice though we can add a new method. |
Contributor
|
There's no getter for the current build settings, so you can't use the struct update syntax. |
DevinLeamy
commented
Aug 22, 2023
|
|
||
| /// Returns the schedule's current `ScheduleBuildSettings`. | ||
| pub fn get_build_settings(&self) -> ScheduleBuildSettings { | ||
| self.graph.settings.clone() |
Contributor
Author
There was a problem hiding this comment.
Clone because it's a light struct and we're not mutating the return value.
Contributor
Author
|
Makes sense to add the ability to chain App::new()
// ...
.configure_schedules(ScheduleBuildSettings {
ambiguity_detection: LogLevel::Error,
..schedule.get_build_settings()
})
// ...
.run() |
hymm
approved these changes
Aug 22, 2023
rdrpenguin04
pushed a commit
to rdrpenguin04/bevy
that referenced
this pull request
Jan 9, 2024
…ettings` to all schedules (bevyengine#9514) # Objective - Fixes: bevyengine#9508 - Fixes: bevyengine#9526 ## Solution - Adds ```rust fn configure_schedules(&mut self, schedule_build_settings: ScheduleBuildSettings) ``` to `Schedules`, and `App` to simplify applying `ScheduleBuildSettings` to all schedules. --- ## Migration Guide - No breaking changes. - Adds `Schedule::get_build_settings()` getter for the schedule's `ScheduleBuildSettings`. - Can replaced manual configuration of all schedules: ```rust // Old for (_, schedule) in app.world.resource_mut::<Schedules>().iter_mut() { schedule.set_build_settings(build_settings); } // New app.configure_schedules(build_settings); ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
AppandSchedulesto configure all schedules #9508Solution
to
Schedules, andAppto simplify applyingScheduleBuildSettingsto all schedules.Migration Guide
Schedule::get_build_settings()getter for the schedule'sScheduleBuildSettings.