Remake PRs targeting 0.14 branch #13825
Labels
A-Cross-Cutting
Impacts the entire engine
D-Straightforward
Simple bug fixes and API improvements, docs, test and examples
S-Ready-For-Implementation
This issue is ready for an implementation PR. Go for it!
Milestone
Comments
alice-i-cecile
added
S-Ready-For-Implementation
|
This issue mostly exists to make sure we don't forget to do this, but help here would be lovely. |
6 tasks
mockersf
pushed a commit
that referenced
this issue
Jun 14, 2024
…13808) (#13842) # Objective - Related to #13825 ## Solution - Cherry picked the merged PR and performed the necessary changes to adapt it to the 0.14 release branch. --------- As discovered in Leafwing-Studios/leafwing-input-manager#538, there appears to be some real weirdness going on in how event updates are processed between Bevy 0.13 and Bevy 0.14. To identify the cause and prevent regression, I've added tests to validate the intended behavior. My initial suspicion was that this would be fixed by #13762, but that doesn't seem to be the case. Instead, events appear to never be updated at all when using `bevy_app` by itself. This is part of the problem resolved by #11528, and introduced by #10077. After some investigation, it appears that `signal_event_update_system` is never added using a bare-bones `App`, and so event updates are always skipped. This can be worked around by adding your own copy to a later-in-the-frame schedule, but that's not a very good fix. Ensure that if we're not using a `FixedUpdate` schedule, events are always updated every frame. To do this, I've modified the logic of `event_update_condition` and `event_update_system` to clearly and correctly differentiate between the two cases: where we're waiting for a "you should update now" signal and where we simply don't care. To encode this, I've added the `ShouldUpdateEvents` enum, replacing a simple `bool` in `EventRegistry`'s `needs_update` field. Now, both tests pass as expected, without having to manually add a system! I've written two parallel unit tests to cover the intended behavior: 1. Test that `iter_current_update_events` works as expected in `bevy_ecs`. 2. Test that `iter_current_update_events` works as expected in `bevy_app` I've also added a test to verify that event updating works correctly in the presence of a fixed main schedule, and a second test to verify that fixed updating works at all to help future authors narrow down failures. - [x] figure out why the `bevy_app` version of this test fails but the `bevy_ecs` version does not - [x] figure out why `EventRegistry::run_updates` isn't working properly - [x] figure out why `EventRegistry::run_updates` is never getting called - [x] figure out why `event_update_condition` is always returning false - [x] figure out why `EventRegistry::needs_update` is always false - [x] verify that the problem is a missing `signal_events_update_system` Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Mike <mike.hsu@gmail.com>
alice-i-cecile
added a commit
that referenced
this issue
Jun 16, 2024
# Objective - Fixes #13825 ## Solution - Cherry picked and fixed non-trivial conflicts to be able to merge #10839 into the 0.14 release branch. Link to PR: #10839 Co-authored-by: James O'Brien <james.obrien@drafly.net> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: MiniaczQ <xnetroidpl@gmail.com> Co-authored-by: Carter Anderson <mcanders1@gmail.com>
|
I suppose this can be closed now. |
|
Yep! Thanks again for tackling this: it really helped Francois and I move the release forward. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Some PRs can't be trivially cherrypicked by @mockersf to the 0.14 release due to merge conflicts. These are:
To fix each of these, redo their changes, branched off of https://github.com/bevyengine/bevy/tree/release-0.14.0 and then make a PR on the
bevyengine/bevyrepo targeting the release branch.The text was updated successfully, but these errors were encountered: