Reading from an EventReader in an exclusive system doesn't clear events from the same tick

redpandamonium commented 2 years ago

Bevy version

0.7.0

What you did

I was playing around with EventReader in exclusive systems and found some inconsistencies. I have tested 7 scenarios that are inconsistent. Systems are not exclusive unless mentioned. Reading is done using an EventReader unless specified otherwise.

A: The event was written in CoreStage::PreUpdate. It is read in CoreStage::Update
B: The event was written in CoreStage::PreUpdate. It is read with an exclusive system in CoreStage::Update in at_begin using an EventReader
C: The event was written in CoreStage::PreUpdate. It is read with an exclusive system in CoreStage::Update in at_begin using an ResMut<Events<EventC>>
D: The event was written in CoreStage::PreUpdate. It is read with a normal system that is ordered to run after it was written, in the same stage.
E: The event was written in CoreStage::PreUpdate. It is read with an exclusive system in CoreStage::PreUpdate in at_end using an EventReader
F: The event was written in CoreStage::Update. It is read in the next tick by an exclusive system in CoreStage::Update in at_begin
G: The event was written in CoreStafe::Update. It is read in the same tick by an exclusive system in CoreStage::Update in at_end

The expected output order would thus be: D -> E -> (C and B) -> A -> G -> F (next tick)

What went wrong

However the actual order is: D -> E -> (C and B) -> A -> G -> E (next tick) -> (F and B) (next tick) -> G

E, G, and B were read twice. This is inconsistent because both systems before and after them worked as expected, and an exclusive system at the same time using Events<EventC>::drain works as expected. The exclusive system running in the next tick (F) is also working as expected. The issue only seems to happen with exclusive systems that read events using EventReader in the same tick as the event was fired. It does not seem to matter in which stage the event is fired or in which stage the exclusive system reads it, only the order seems to matter. This issue does not apply to non-exclusive systems.

I would expect all of the variants to only read the event the first time.

Additional information

Here's my test code to reproduce this issue:

use bevy::app::{App, CoreStage};
use bevy::ecs::event::Events;
use bevy::ecs::system::SystemState;
use bevy::prelude::{EventReader, EventWriter, ExclusiveSystemDescriptorCoercion, IntoExclusiveSystem, Local, ParallelSystemDescriptorCoercion, ResMut, World};

pub struct EventA(String); // read in CoreStage::Update with a normal system
pub struct EventB(String); // read in CoreStage::Update with an exclusive system in 'at_begin' using EventReader
pub struct EventC(String); // read in CoreStage::Update with an exclusive system in 'at_begin' using Events
pub struct EventD(String); // read in CoreStage::PreUpdate in a normal system after the event is fired
pub struct EventE(String); // read in CoreStage::PreUpdate with an exclusive system in 'at_end' using EventReader
pub struct EventF(String); // read in CodeStage::Update with an exclusive system in 'at_begin' using EventReader, written in CoreStage::Update
pub struct EventG(String); // read in CoreStage::Update with an exclusive system in 'at_end' using EventReader, written in CoreStage::Update

pub fn fire_event_a(mut writer: EventWriter<EventA>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventA("A".to_string()));
        *ran = true;
    }
}

pub fn fire_event_b(mut writer: EventWriter<EventB>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventB("B".to_string()));
        *ran = true;
    }
}

pub fn fire_event_c(mut writer: EventWriter<EventC>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventC("C".to_string()));
        *ran = true;
    }
}

pub fn fire_event_d(mut writer: EventWriter<EventD>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventD("D".to_string()));
        *ran = true;
    }
}

pub fn fire_event_e(mut writer: EventWriter<EventE>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventE("E".to_string()));
        *ran = true;
    }
}

pub fn fire_event_f(mut writer: EventWriter<EventF>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventF("F".to_string()));
        *ran = true;
    }
}

pub fn fire_event_g(mut writer: EventWriter<EventG>, mut ran: Local<bool>) {
    if !*ran {
        writer.send(EventG("G".to_string()));
        *ran = true;
    }
}

pub fn read_event_a(mut read: EventReader<EventA>) {
    for event in read.iter() {
        println!("{}", event.0);
    }
}

pub fn read_event_b(world: &mut World) {
    let mut event_system_state = SystemState::<(
        EventReader<EventB>
    )>::new(world);
    let (mut events) = event_system_state.get_mut(world);

    for event in events.iter() {
        println!("{}", event.0);
    }
}

pub fn read_event_c(world: &mut World) {
    let mut event_system_state = SystemState::<(
        ResMut<Events<EventC>>
    )>::new(world);
    let (mut events) = event_system_state.get_mut(world);

    for event in events.drain() {
        println!("{}", event.0);
    }
}

pub fn read_event_d(mut read: EventReader<EventD>) {
    for event in read.iter() {
        println!("{}", event.0);
    }
}

pub fn read_event_e(world: &mut World) {
    let mut event_system_state = SystemState::<(
    EventReader<EventE>
    )>::new(world);
    let (mut events) = event_system_state.get_mut(world);

    for event in events.iter() {
        println!("{}", event.0);
    }
}

pub fn read_event_f(world: &mut World) {
    let mut event_system_state = SystemState::<(
    EventReader<EventF>
    )>::new(world);
    let (mut events) = event_system_state.get_mut(world);

    for event in events.iter() {
        println!("{}", event.0);
    }
}

pub fn read_event_g(world: &mut World) {
    let mut event_system_state = SystemState::<(
    EventReader<EventG>
    )>::new(world);
    let (mut events) = event_system_state.get_mut(world);

    for event in events.iter() {
        println!("{}", event.0);
    }
}

fn main() {
    App::new()
        .add_plugin(bevy::core::CorePlugin::default())
        .add_plugin(bevy::app::ScheduleRunnerPlugin)
        .add_event::<EventA>()
        .add_event::<EventB>()
        .add_event::<EventC>()
        .add_event::<EventD>()
        .add_event::<EventE>()
        .add_event::<EventF>()
        .add_event::<EventG>()
        .add_system_to_stage(CoreStage::PreUpdate, fire_event_a)
        .add_system_to_stage(CoreStage::PreUpdate,fire_event_b)
        .add_system_to_stage(CoreStage::PreUpdate,fire_event_c)
        .add_system_to_stage(CoreStage::PreUpdate, fire_event_d.label("fire d"))
        .add_system_to_stage(CoreStage::PreUpdate,fire_event_e)
        .add_system(fire_event_f)
        .add_system(fire_event_g)
        .add_system(read_event_a)
        .add_system(read_event_b.exclusive_system())
        .add_system(read_event_c.exclusive_system())
        .add_system_to_stage(CoreStage::PreUpdate, read_event_d.after("fire d"))
        .add_system_to_stage(CoreStage::PreUpdate, read_event_e.exclusive_system().at_end())
        .add_system(read_event_f.exclusive_system())
        .add_system(read_event_g.exclusive_system().at_end())
        .run()
}

redpandamonium commented 2 years ago

Why does F work then?

hymm commented 2 years ago

You need to cache the system state values somewhere. See https://docs.rs/bevy/latest/bevy/ecs/system/struct.SystemState.html#warning

hymm commented 2 years ago

Why does F work then?

Probably something to do with events being double buffered and the event update systems running in First.

DJMcNab commented 2 years ago

Could we change our examples to not use uncached SystemState? It would be easier once we have Locals in exclusive systems

bevyengine / bevy