[Merged by Bors] - Resolve most remaining execution-order ambiguities#6341
Closed
joseph-gio wants to merge 18 commits intobevyengine:mainfrom
Closed
[Merged by Bors] - Resolve most remaining execution-order ambiguities#6341joseph-gio wants to merge 18 commits intobevyengine:mainfrom
joseph-gio wants to merge 18 commits intobevyengine:mainfrom
Conversation
alice-i-cecile
added
A-Rendering
alice-i-cecile
approved these changes
Oct 22, 2022
hymm
approved these changes
Oct 25, 2022
Contributor
hymm
left a comment
hymm
left a comment
There was a problem hiding this comment.
Needing to suppress Assets<Image> so many times makes me worried about how ambiguity detection will work at a larger scale. Hopefully won't be too bad as most games won't need to get mutable access to Assets too much. Either way not the concern of this PR.
Co-authored-by: Mike <mike.hsu@gmail.com>
Member
Author
|
Misclick, my bad. |
alice-i-cecile
added
the
S-Ready-For-Final-Review
Member
|
Yeah the problem with Assets is that we need to have a pattern for "order independent writes". |
Member
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Oct 27, 2022
# Objective Bevy's internal plugins have lots of execution-order ambiguities, which makes the ambiguity detection tool very noisy for our users. ## Solution Silence every last ambiguity that can currently be resolved. Each time an ambiguity is silenced, it is accompanied by a comment describing why it is correct. This description should be based on the public API of the respective systems. Thus, I have added documentation to some systems describing how they use some resources. # Future work Some ambiguities remain, due to issues out of scope for this PR. * The ambiguity checker does not respect `Without<>` filters, leading to false positives. * Ambiguities between `bevy_ui` and `bevy_animation` cannot be resolved, since neither crate knows that the other exists. We will need a general solution to this problem.
Contributor
|
Pull request successfully merged into main. Build succeeded:
|
bors
bot
changed the title
james7132
pushed a commit
to james7132/bevy
that referenced
this pull request
Oct 28, 2022
# Objective Bevy's internal plugins have lots of execution-order ambiguities, which makes the ambiguity detection tool very noisy for our users. ## Solution Silence every last ambiguity that can currently be resolved. Each time an ambiguity is silenced, it is accompanied by a comment describing why it is correct. This description should be based on the public API of the respective systems. Thus, I have added documentation to some systems describing how they use some resources. # Future work Some ambiguities remain, due to issues out of scope for this PR. * The ambiguity checker does not respect `Without<>` filters, leading to false positives. * Ambiguities between `bevy_ui` and `bevy_animation` cannot be resolved, since neither crate knows that the other exists. We will need a general solution to this problem.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective Bevy's internal plugins have lots of execution-order ambiguities, which makes the ambiguity detection tool very noisy for our users. ## Solution Silence every last ambiguity that can currently be resolved. Each time an ambiguity is silenced, it is accompanied by a comment describing why it is correct. This description should be based on the public API of the respective systems. Thus, I have added documentation to some systems describing how they use some resources. # Future work Some ambiguities remain, due to issues out of scope for this PR. * The ambiguity checker does not respect `Without<>` filters, leading to false positives. * Ambiguities between `bevy_ui` and `bevy_animation` cannot be resolved, since neither crate knows that the other exists. We will need a general solution to this problem.
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
4 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
Bevy's internal plugins have lots of execution-order ambiguities, which makes the ambiguity detection tool very noisy for our users.
Solution
Silence every last ambiguity that can currently be resolved.
Each time an ambiguity is silenced, it is accompanied by a comment describing why it is correct. This description should be based on the public API of the respective systems. Thus, I have added documentation to some systems describing how they use some resources.
Future work
Some ambiguities remain, due to issues out of scope for this PR.
Without<>filters, leading to false positives.bevy_uiandbevy_animationcannot be resolved, since neither crate knows that the other exists. We will need a general solution to this problem.