Events aren't being cleared properly

[gwafotapa]

Bevy version 0.12.1

Events are not silently dropped the turn after they're sent but instead accumulate in the EventReader's buffer.

What I did

Here's an example:

use std::cmp::Ordering;

use bevy::prelude::*;

#[derive(Event)]
struct Ev;

fn send(mut evw: EventWriter<Ev>) {
    evw.send(Ev);
}

fn read(mut evr: EventReader<Ev>) {
    println!("{}", evr.iter().count());
}

// Generates a condition returning true on the nth frame
fn nth_frame(n: u32) -> impl FnMut() -> bool + Clone {
    let mut run = 1;
    move || match run.cmp(&n) {
        Ordering::Greater => false,
        Ordering::Equal => {
            run += 1;
            true
        }
        Ordering::Less => {
            run += 1;
            false
        }
    }
}

fn main() {
    App::new()
        .add_event::<Ev>()
        .add_plugins(DefaultPlugins)
        .add_systems(Update, send)
        .add_systems(Update, read.after(send).run_if(nth_frame(10)))
        .run();
}

What went wrong

Under version 0.11.3, this prints 2 as expected but under 0.12.1 I'm getting 10.

[torsteingrindvik]

I think I've found this too and I was confused if I had misunderstood events or if it was a bug.

For me it was this:

• Some system in frame n generates an event
• A system that rarely runs because of run conditions still gets the event much later, in e.g. frame n+100

Sounds like this can be caused if events are buffered instead of cleared

[5tr1k3r]

#10860

Seems like a decent amount of people stumbled over it, maybe something needs to be done.

[jnhyatt]

I agree something needs to be done, this is a tremendous footgun, and not just because the behavior changes. I know there was discussion about this being the "right" way to do this in the ecs dev channel somewhere, but at this point I think it's obvious that this isn't what users are expecting.

[tim-blackbird]

Agreed. Even experienced Bevy devs are struggling to use events correctly, so something must be done.

It is easier said than done, however.
EventReaders keep track of which events they've read with a cursor stored in the system. We'll somehow need to update these cursors when the systems aren't even running or change how events are tracked entirely.

[uvizhe]

I wish this change in behavior was announced 😔

Now, shall I adopt my code to the new behavior or wait for a final decision on this?

[tim-blackbird]

The issue description is a little strange.

Under version 0.11.3, this prints 2 as expected

Wouldn't you expect it to be 1? That is what gets printed with nth_frame(1) i.e reading events every frame.

The example given reads old events in both 0.11.3 and 0.12.1
The difference is that 0.12.1 can buffer events for more than 2 frames but with 0.11.3 you're still getting an outdated event from the previous frame.

[gwafotapa]

According to the doc, this is the expected behavior.

When an event is generated, some systems may already have run in the current frame. Thus, it's already too late for them to read that event this frame. That's why events persist until the end of the next frame. And this is why 2 is the expected behavior in the example.

[tim-blackbird]

I see, it was more clearly documented than I thought. fair enough.

[Azorlogh]

This new behavior synergizes pretty poorly with .run_if 😕
Would it be possible to have a method on EventReader to read events' emission timestamp or frame-count? That way we would be able to easily filter out the older ones.

[teodosin]

Adding my 2 cents that a change like this should be documented, with suggested workarounds if it is intended to stay this way and not just a bug. I am surprised this isn't talked about more, since problems arise the moment any systems with run conditions read from the same events.