-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Merged by Bors] - expose ambiguities of ScheduleGraph
#7716
[Merged by Bors] - expose ambiguities of ScheduleGraph
#7716
Conversation
Alternative: #7719 |
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 honestly like this better than #7719 due to it's simplicity and more controlled API surface. If we need to alter the implementation in the future, this seems like a much better approach.
/// Returns a mutable reference to the [`ScheduleGraph`]. | ||
pub fn graph_mut(&mut self) -> &mut ScheduleGraph { | ||
&mut self.graph | ||
} | ||
|
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.
is this needed in this PR?
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.
It's needed to call build_schedule, otherwise you won't have the ambiguities
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.
bors r+
# Objective - other tools (bevy_mod_debugdump) would like to have access to the ambiguities so that they can do their own reporting/visualization ## Solution - store `conflicting_systems` in `ScheduleGraph` after calling `build_schedule` The solution isn't very pretty and as pointed out it #7522, there may be a better way of exposing this, but this is the quick and dirty way if we want to have this functionality exposed in 0.10.
Pull request successfully merged into main. Build succeeded:
|
ScheduleGraph
ScheduleGraph
follow-up to #7716 # Objective System access is only populated in `System::initialize`, so without calling `initialize` it's actually impossible to see most ambiguities. ## Solution - make `initialize` public. The method is idempotent, so calling it multiple times doesn't hurt
# Objective - other tools (bevy_mod_debugdump) would like to have access to the ambiguities so that they can do their own reporting/visualization ## Solution - store `conflicting_systems` in `ScheduleGraph` after calling `build_schedule` The solution isn't very pretty and as pointed out it bevyengine#7522, there may be a better way of exposing this, but this is the quick and dirty way if we want to have this functionality exposed in 0.10.
follow-up to bevyengine#7716 # Objective System access is only populated in `System::initialize`, so without calling `initialize` it's actually impossible to see most ambiguities. ## Solution - make `initialize` public. The method is idempotent, so calling it multiple times doesn't hurt
Objective
Solution
conflicting_systems
inScheduleGraph
after callingbuild_schedule
The solution isn't very pretty and as pointed out it #7522, there may be a better way of exposing this, but this is the quick and dirty way if we want to have this functionality exposed in 0.10.