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).

[dubrowgn]

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?

I got the impression that the current behavior was intended since you don't know ahead of time if anyone will consume the events. If you wait for "all readers" to consume their events, and there are no readers (or they don't care about all of the data), you have a memory leak.

I haven't looked at the current implementation that much, but I kind of assumed that an EventReader's lifetime was about as long as it took to call the receiving system (if it's just a cursor). So the count of "all readers" would usually be zero at runtime.

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).

Fair. I was more worried about simulation lag, but some programs are probably more prone to graphics lag. I'm not 100% convinced what you describe is what I would want to do though, because you effectively inject additional input latency by moving the handling of some inputs back by one simulation frame in your example. Under high load, that latency could be significant. If I'm several seconds behind, and trying to open the menu, I don't necessarily want to wait for the simulation to very slowly and painfully replay all my inputs with frame perfect timing before it catches up enough to realize I'm trying to do something.

What you describe may very well be the "right" way to handle it, but I would probably have to actually try both under problematic conditions to decide.

[maniwani]

I got the impression that the current behavior was intended since you don't know ahead of time if anyone will consume the events. If you wait for "all readers" to [read] the events, [and they never do], you have a memory leak.

Right, we clear the queue to prevent memory leaks. But that behavior isn't valid, or we wouldn't be here. So we just can't store events forever. There needs to be some limit to the capacity. Making the event queue into a large ring buffer is one option. In any case, nailing down the right time to drop events is a better problem to have than events being fundamentally broken because we drop them way too soon.

I'm not 100% convinced what you describe is what I would want to do though, because you effectively inject additional input latency by moving the handling of some inputs back by one simulation frame in your example.

A fixed timestep exists so that you can have a simulation whose behavior is consistent regardless of framerate. It basically emulates a dedicated core that runs steps with perfect spacing. The fact is, the steps are actually anything BUT perfectly-spaced, but that is supposed to have no observable effect on the results. Therefore, it follows that the exact time we run a step shouldn't affect which input events that step sees.

Each simulation step should see the same events it would have seen under perfect-spacing.

If I'm several seconds behind, and trying to open the menu, I don't necessarily want to wait for the simulation to very slowly and painfully replay all my inputs with frame perfect timing before it catches up enough to realize I'm trying to do something.

A few things:

• If you're several seconds worth of steps behind, you're in a death spiral or you have like 0.5 FPS.
• You shouldn't run UI logic inside of FixedUpdate. If you want to open a menu, just do it in Update.
• Framerate-independence is necessary to truly cancel-out delay.
  • Timestamps would not only ensure each step sees the appropriate input events (so that none get squashed/lost), but would also enable things like doing hit detection against the state as it was at the time the event was captured.
  • To see that in action, see this and this.

What I'm trying to get at is "fixed update" and "frame update" will read the same input events, just independently. Each frame will see the events captured during the previous frame. Each simulation step will see the events captured "during" the previous step.

[dubrowgn]

If you're several seconds worth of steps behind, you're in a death spiral or you have like 0.5 FPS.

This is true with the current Bevy implementation, but it is not a characteristic of all implementations of a fixed update loop. This is why I mentioned there are alternative implementations.

It seems like we are talking past each other a bit here. Part of that is definitely my fault, since I jumped to potential implementation details before aligning on requirements. I don't mean to argue, but I do hope to get across my original point so that it is considered in whatever eventual solution that is reached: A solution that is directly coupled to the Bevy implementation of a fixed update loop is only useful in that specific context.

Alternative implementations of a fixed update loop would continue to have all the same input event problems seen today. But, when the events API is defined in terms of (for example) "since I last asked", the question of update cadence doesn't even arise. This means such a solution (or some similar solution) would be flexible enough to be used in any implementation of a fixed update loop (or even any update loop). For example:

What I'm trying to get at is "fixed update" and "frame update" will read the same input events, just independently. Each frame will see the events captured during the previous frame. Each simulation step will see the events captured "during" the previous step.

This sounds great! As long as "during the previous step" isn't directly coupled to Bevy's fixed update implementation. Instead (again, as an example), you could expose an API that allows you to get new events up to a given end timestamp. That kind of API could be used in any update loop strategy. If you want to process all of the new events, you can do that. If you want to process only new events from a specific slice of time, you can do that too. You can even do a combination of both in any combination of loops with different cadences to handle different kinds of inputs, all without Bevy having to know anything about how your loops actually work.

[maniwani]

But, when the events API is defined in terms of (for example) "since I last asked", the question of update cadence doesn't even arise.

I agree. What I tried to explain is that EventReader<T> behavior is "since I last asked", the Events::<T>::update_system just disrupts that by clearing the queue of events.

bevy/crates/bevy_ecs/src/event.rs

Lines 544 to 552 in 3f6367d

	pub fn update(&mut self) {
	std::mem::swap(&mut self.events_a, &mut self.events_b);
	self.events_b.clear();
	self.events_b.start_event_count = self.event_count;
	debug_assert_eq!(
	self.events_a.start_event_count + self.events_a.len(),
	self.events_b.start_event_count
	);
	}

As long as "during the previous step" isn't directly coupled to Bevy's fixed update implementation. [...] You could expose an API that allows you to get new events up to a given end timestamp.

This is what I've been trying to say. To close this issue, FixedUpdate requires timestamps and an API to read events "up to given timestamp".

I never meant to imply that such things would be coupled to or only usable by FixedUpdate.

"during" the previous step

It sounds like you understand what I meant, but I'll try to give a more thorough explanation.

Each frame sees the input events that the app received during the previous frame. The same should be true for fixed updates, but what "during" actually means is a bit subtle.

As I explained earlier, what makes fixed update "fixed" is that it ignores the IRL time that elapses between updates. Each fixed update advances the simulation by the same fixed amount of time.

What might be unintuitive is that every step does correspond to an actual span of IRL time, it's just that their execution is delayed.

In this diagram, the fixed updates are marked above the axis. The t0, t1, and t2 points below the axis are three points where a new frame begins. The boxes indicate that step 4 is run by the frame at t1 and that steps 5-8 are run by the frame at t2.

Now, regardless of when step N is actually run, it logically happened at t = N * Δt in IRL time, and so it should see the input events the app received between t = (N - 1) * Δt and t = N * Δt.

That's what requires event timestamps and an API to read events "up to given timestamp."

[richardhozak]

For those that run into this, and are using Res<Input<KeyCode>> in their fixed update systems, you will still get missed inputs, the solution is to use events in fixed update as fixed in linked PR (#10077). If you want to use Input<KeyCode> you can use local system param (Local<Input<KeyCode>>) and feed it events, like so:

fn handle_input(
    mut keys: Local<Input<KeyCode>>,
    mut keyboard_input_events: EventReader<KeyboardInput>,
) {
    keys.clear();
    for event in keyboard_input_events.read() {
        if let Some(key_code) = event.key_code {
            match event.state {
                ButtonState::Pressed => keys.press(key_code),
                ButtonState::Released => keys.release(key_code),
            }
        }
    }

    // do something with inputs

    let up = keys.pressed(KeyCode::Up);
    let down = keys.pressed(KeyCode::Down);
    let left = keys.pressed(KeyCode::Left);
    let right = keys.pressed(KeyCode::Right);
    let respawn = keys.just_pressed(KeyCode::Return);
}

[alice-i-cecile]

I've chewed on this a bit, and I think I'd actually like to handle this a lot like how Time works upstream in Bevy.

There's no canonical location for when to update the just_pressed information: both update and fixed update are defensible. And some games might even want to do both.

So instead, swap the behavior of just_pressed based on which schedule we're in. Provide some fallback methods to opt out of this behavior, but make the obvious thing to do just work using the same pattern.

We'll probably need to implement this by keeping track of its pressed state in both the main and fixed schedule.

[Henrique194]

Any updates on this issue?

[alice-i-cecile]

We've worked to improve event handling in fixed updates, per #10077 and #13808.

Over at leafwing-input-manager, we've also tackled this problem via a (not yet released) state-switching solution in Leafwing-Studios/leafwing-input-manager#522 by @cBournhonesque. While that is slated for upstreaming, we should consider implementing a parallel solution for ButtonInput for those who want to operate on raw inputs.