Mouse movement accumulation is broken #538
Comments
|
This was broken in the Bevy 0.14 port. I'm pretty sure that this is just tech debt from the terrible existing hack biting us. This should be rewritten from scratch. |
alice-i-cecile
added a commit
that referenced
this issue
Jun 10, 2024
* Reimplement keyboard inputs * Update document * Migrate to new input processors and refactor the trait * Implement mouse inputs * Prefer `FromIter<*AxisProcessor>` over `From<Vec<*AxisProcessor>>` * Implement `GamepadButtonType` and `GamepadAxisType` * Add a `pipeline` method to create input processing pipelines * Typo * Replace old `UserInput` and `InputKind` with the new ones * RELEASES.md * Fix clashing inputs * Typo * Docs * Rename * Docs * Docs * RELEASES.md * Fix wrong gamepad button value * Rename `InputChord::from_multiple` to `new` * Fix ci * Simple compiler fixes * Fix modifier import * Fix bevy_egui dep * Ignore and fix failing tests #538 * Reuse try_apply logic --------- Co-authored-by: Shute052 <sinoctis@163.com>
|
There's a good chance that this has the same root cause as bevyengine/bevy#13758, but I think we should be able to fix this without the associated fix by simply writing this code correctly. |
|
https://github.com/bevyengine/bevy/commits/release-0.14.0/crates/bevy_ecs/src/event.rs This is the relevant history of the event files: nothing else is plausible. |
|
|
This was referenced Jun 11, 2024
Merged
github-merge-queue bot
pushed a commit
to bevyengine/bevy
that referenced
this issue
Jun 12, 2024
…13808) # Objective 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. ## Solution 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! ## Testing 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. ## Outstanding - [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: Mike <mike.hsu@gmail.com>
mnmaita
pushed a commit
to mnmaita/bevy
that referenced
this issue
Jun 14, 2024
…evyengine#13808) 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 bevyengine#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 bevyengine#11528, and introduced by bevyengine#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: Mike <mike.hsu@gmail.com>
Merged
6 tasks
mnmaita
pushed a commit
to mnmaita/bevy
that referenced
this issue
Jun 14, 2024
…evyengine#13808) 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 bevyengine#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 bevyengine#11528, and introduced by bevyengine#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: Mike <mike.hsu@gmail.com>
mockersf
pushed a commit
to bevyengine/bevy
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Version
Just after a513ad0
What you did
Many tests are failing. See https://github.com/Leafwing-Studios/leafwing-input-manager/actions/runs/9452237797/job/26034969670?pr=534
This isn't particularly suprising: this code was always very jank and fragile.
The text was updated successfully, but these errors were encountered: