Handle just_pressed/just_released in the FixedUpdate schedule

[cBournhonesque]

Context

Fixes #252
(see description of the issue in bevyengine/bevy#6183)

It would also fix cBournhonesque/lightyear#349

(I did a write-up here: https://hackmd.io/_TGuaUTnRBeuisvUMr0QoQ?both)

i.e. that we cannot reliably use action.just_pressed() in FixedUpdate systems because:

1. in some cases,FixedUpdate runs 2 times in the same frame, which means that they would both had just_pressed() = True which is misleading

situation 1 (S1): F - FU - FU - F

2. in some cases, FixedUpdate runs 0 times in one frame, which means that just_pressed becomes pressed and the input is never seen by the FixedUpdate system.

situation 2 (S2): F - F - FU - F

Glossary: 

- `F` = `Frame start`
- `FU` = `FixedUpdate start`

Solution

Outline

This is a an initial version that can probably be improved, but it solves the issues with fixed timesteps.

We will have the same approach as the Time API: this is how it works: https://github.com/bevyengine/bevy/blob/main/crates/bevy_time/src/fixed.rs#L237
There are 3 resources Time<Fixed>, Time<Virtual>, Time<()>.
The resource Time<()> is set to either Time<Fixed> and Time<Virtual> depending on the schedule we are running in.

Here we do the same thing; the ActionData has 3 fields to represent state:

• update_state
• state
• fixed_update_state

We will update update_state in Update and fixed_update_state in FixedUpdate. The user-facing interface will be state which switches between update_state and fixed_update_state depending on the schedule; but maybe we could also expose update_state and fixed_update_state (similarly to how Time<Virtual> and Time<Res> are exposed)

We still keep state so that:

• all the current code is untouched (we will just swap state with either update_state or fixed_update_state depending on which schedule we are running
• the client-facing API is untouched; the user should just access state and they will access the correct data depending on whether they are running in FixedUpdate or Update

System order

So the order is

• Schedule = PreUpdate

  • tick the ActionState
  • apply the input events to state. (new button presses, etc.)
  • apply ManualControl

• before Schedule = RunFixedMainLoop (we use this because we don't want to run the systems once per FixedUpdate; there could be multiple FixedUpdate runs in a frame)

  • set update_state = state to apply the changes; then do state = fixed_update_state to load the new state
  • apply the input events to state (the input events need to be applied event because we essentially maintain 2 independent ActionState, one in Update and one in FixedUpdate)

• Schedule = FixedPreUpdate

  • apply release_on_disable
  • apply UI updates (i think this should run once per FixedUpdate because the button could be pressed in FixedUpdate? I might have misunderstood what the ui feature does)

• Schedule = FixedPostUpdate

  • tick the ActionState (we want to run this every FixedUpdate to go from just_pressed to pressed and have better timing information)

• after Schedule = RunFixedMainLoop

  • set fixed_update_state = state to save the changes, and do state = update_state to load the new state

I think having the fields inside ActionData instead of having 3 separate ActionData is a feature, not a bug:
some fields need to be updated only once (value, axis_pair), and should even be shared between the two schedules (consumed).

The timing information is also shared, but only updated in PreUpdate because Time<Real> is only updated in pre-updated. I don't think it's worth worrying about handling timing information separately in Time<Fixed> because:

• timing information also wasn't used in FixedUpdate before, so this PR is still a strict improvement
• we might need timing information from winit events to compute them accurately anyway

Test

Added 2 unit tests to show that the problems 1 and 2 described above are solved.

[atlv24]

This is quite straightforward and the test cases are confidence-inspiring. Definitely reduces footgunnyness, but I do also think that doing input in fixed update is inadvisable in the first place. I dont think that means it should explode on people who do that, but I do wonder if the copying around of structs has a performance impact that everyone will have to pay even if they only do input in update.

[cBournhonesque]

This is quite straightforward and the test cases are confidence-inspiring. Definitely reduces footgunnyness, but I do also think that doing input in fixed update is inadvisable in the first place. I dont think that means it should explode on people who do that, but I do wonder if the copying around of structs has a performance impact that everyone will have to pay even if they only do input in update.

I agree that there is a additional cost, so maybe we could gate this change behind a feature flag.
But in general there are a lot of cases where handling inputs in fixed-update is necessary; anything related to the simulation or to networking generally has to run in FixedUpdate (physics, simulation, etc.).

In my usecase, I maintain a networking library that has rollback networking; for rollback to work properly most of the simulation must happen in FixedUpdate, so users will have a lot of input-handling systems inside FixedUpdate.

[atlv24]

Are there any cases where you'd want input both in fixed update and normal update? If you're doing rollback netcode sim stuff you wouldnt want any input in normal update, right? Maybe the right approach is to just switch between fixed/notfixed entirely? In any case I think this can be merged as is, without feature gates. Gating would have to be motivated by benchmarks that show the additional complexity is warranted.

[cBournhonesque]

Are there any cases where you'd want input both in fixed update and normal update? If you're doing rollback netcode sim stuff you wouldnt want any input in normal update, right? Maybe the right approach is to just switch between fixed/notfixed entirely? In any case I think this can be merged as is, without feature gates. Gating would have to be motivated by benchmarks that show the additional complexity is warranted.

In most cases it would probably be: player inputs (move, fire weapons, etc.) in FixedUpdate, and admin inputs (UI, settings, chat) in Update.
So yeah maybe it's worth having a Mode enum where an input would be either in FixedUpdate or Update.

But it's probable that in some cases it would be useful to have inputs work in both FixedUpdate / Update, maybe we could provide 3 modes: Update, FixedUpdate and Both. Then it's a matter of ergonomics or UX whether we want to expose this kind of complexity to the user or hide it by always using Both. Maybe @alice-i-cecile has an opinion here

[atlv24]

Perfect thanks so much, intent is clearer like this

[alice-i-cecile]

Honestly, more elegant than I expected. I like how this mirrors the strategy used by Time.

There may be a better, more holistic solution with less cost one day, but for now, this is a mostly-invisible, simple fix.

Add release notes and I'll merge this in.

[alice-i-cecile]

Needs formatting now before I can merge 😄