Inputs can be missed (or duplicated) when using a fixed time step

[alice-i-cecile]

Bevy version

0.8.1

What you did

Create a system that responds to input events inside of a fixed times step.

What went wrong

Sometimes, input events are missed: Input::just_pressed and just_released are never picked up.

Other times, they're duplicated: we respond to the same input multiple times.

Explanation

Missed inputs

Fixed time step systems may not run every frame, but Input::just_pressed and friends is reset every frame (and events are only double buffered). If the frame rate is significantly faster than the time step, the fixed time step schedule will sometimes not run, completely missing the input (which will no longer be just-pressed).

Duplicated inputs

If the fixed timestep is faster than the frame rate, fixed time step systems sometimes need to run catch-up, and are evaluated multiple times in the same frame. However, this will cause just_pressed (or just_released) to be registered as true multiple times (as Input will not update between these system evaluations.

This can cause extremely confusing behavior. Note that, unlike missed inputs, events should not be affected by this failure mode, due to the internal cursor stored in EventReader.

Proposed fix

We should have a default fixed time step system set (see the stageless RFC). Input updating (and input event clearing) should occur in there, rather than as part of the main schedule.

Gameplay systems should effectively always be run in the fixed time step: only rendering-related code should be processed each frame.

Ping @maniwani for thoughts.

[maniwani]

Right, so the ideal situation is we can read from a buffer of timestamped input events (or state). Then in each iteration of the fixed timestep, we'd just yield the input that happened within that slice of time.

That's currently blocked on winit and gilrs. gilrs actually does timestamp its events, but using the wrong clock (SystemTime).

Input updating (and input event clearing) should occur in there, rather than as part of the main schedule.

So in lieu of handling it perfectly, yeah I agree that the next best thing would be to refresh events multiple times. That said, we might still want to do that in both places, once in each timestep iteration and again when we get to PreUpdate.

---

Side note, even once we have buffered, timestamped inputs, we probably still want the capability to peek ahead (observe inputs that arrived in the current frame) midway for XR things.

[alice-i-cecile]

Should be solved as part of #5984.

[jabuwu]

Gameplay systems should effectively always be run in the fixed time step: only rendering-related code should be processed each frame.

The impact of this statement is fairly substantial isn't it? The problem is moved from fixed updates to frame updates, and I think many newcomers to Bevy aren't even considering fixed updates. Suddenly it becomes mandatory, or potentially houses very tricky frame rate dependent bugs.

The default fixed timestep period is 1/60th of a second so it will seemingly be working properly in frame updates for anyone with a 60hz vsync app.

[dubrowgn]

I've been struggling with this for a while as well, and from a consumer perspective, what I really want is a way to get all the input events "since the last time I asked". In that context, the lifetime of the events is well defined, and no guesswork is needed. Just dispose of the events after they have been consumed.

This is different from the current implementation of "since the last rendered frame", but this is also potentially different from "since the last fixed update" as currently defined by bevy because there are valid reasons for rolling your own fixed update logic. For example, in some lockstep network models, the next "fixed update" can't happen until all client inputs are received. Or, you might want to enforce a fixed update time budget to keep the program interactive under load. Or, you might want to run the simulation at some multiple of real time. Etc. Etc.

In a typical evented architecture, consumers register for the events they are interested in. Just spit balling here, but maybe something similar could work here:

app.register_event_stream::<Input>(tag_name);

...

sys_handle_input(input_stream: ResMut<EventStream<Input>, With<tag_name>>) {
    input_stream.poll();
    for event in input_stream.drain() {
        ...
    }

    // or

    for event in input_stream.consume_new() {
        ...
    }
}

Conceptually, bevy would tee all the Input events into registered EventStreams of matching type, so if multiple consumers needed to handle Input events they wouldn't stomp on each other. The current events are effectively global, which makes them more difficult to reason about. With something like EventStream, each consumer effectively gets their own event queue, so they don't have to worry about what other systems are or aren't doing with the global events.

Something like this could theoretically be implemented fairly cheaply with one event queue, and a set of start and end "pointers" for each registered consumer. When a consumer polls, you check for new events, and move the consumer's end "pointer" to the end of the queue. When a consumer consumes, the consumer's start "pointer" moves forward until it reaches the corresponding end "pointer". When all start "pointers" have moved passed an event, drop it from the event queue.

[maniwani]

What I really want is a way to get all the input events "since the last time I asked".

EventReader<T> is already implemented like you described. EventReader<T> is a cursor over the real queue, Events<T>, and it picks up from where it stopped.

Readers don't "consume" events, the problem is that we clear Events<T> periodically.

When all start "pointers" have moved passed an event, drop it from the event queue.

Yep, that is one way we could handle this. Rather than clear the entire queue, drop events once they've been passed by all the readers. Are you interested in implementing that and making a PR?

Also, just so you know, even after that's resolved, fixed updates won't work quite right. There's another half to this issue that requires precise input event timestamps to solve.

For example, say the fixed timestep is 10ms and that the first frame starts at 0ms and ends at 20ms. So the next frame will run FixedUpdate twice. What should happen is the first run sees the events that came between [0ms, 10ms), and the second run sees the events that came between [10ms, 20ms). But without timestamps, readers can't know when to stop reading, so all the events will be read on the first run (which is not correct).