Consider adding non-draining event reader iterator

[djeedai]

What problem does this solve or what need does it fill?

Trying to loop over received events of an EventReader<E> twice or more in the same system on the same frame. Currently iter() drains, so all but the first iteration time return 0 event.

This is also confusing since iter() on say, Vev<T>, will not drain, and since EventReader::iter() is the only method available it's easy to not think and misuse it.

What solution would you like?

Add a non-draining iterator with an explicit name, making it obvious this should not be your default choice for new users.
Something like:

EventReader::iter_peek();

or

EventReader::peek().iter();

fn peek() -> PeekingEventIterator;

This not only solves the non-draining issue, but also shows in docs that there are 2 types of iterators, which prompt the user to question which one they need, avoiding the confusion with the non-draining iter() found in the standard library.

What alternative(s) have you considered?

Manual event clearing. This is overkill and error-prone, much more so than calling (N-1) times the non-draining iterator proposed and one last time the draining one. Or doing N non-draining and a manual clear().

Additional context

I understand the default iter() being draining to avoid user error, so avoiding on purpose any method name already existing on standard Rust iterators like drain() to avoid confusion. iter_peek() sounds good and longer (so less enticing) than iter() but we can even make it even less likely to be used like iter_without_draining_advanced_use_only(), as long as we provide an escape hatch out of the always-drain default we have now.

[MrGVSV]

EventReader::peek().iter();

One thing with this method actually is that we might be able to use a Drop impl on PeekingEventIterator to automatically handle the drain and event counter once dropped (if the user hasn't already).

[djeedai]

One thing with this method actually is that we might be able to use a Drop impl on PeekingEventIterator to automatically handle the drain and event counter once dropped (if the user hasn't already).

That would be strange, if I scope my peek, then I cannot peek again after that because the Drop will have cleared the reader. I don't see how the reader would know that this peek was the last one for the scope. Or are you suggesting that the clear be in the Drop of EventReader itself? Or something else?

[alice-i-cecile]

What if we implemented Clone on the EventReader?

[DJMcNab]

Indeed, I have just started a slight rework of EventReader related infrastructure, which as a side point adds Clone to ManualEventReader. This use case is the perfect case to drop down to ManualEventReader

[MrGVSV]

That would be strange, if I scope my peek, then I cannot peek again after that because the Drop will have cleared the reader. I don't see how the reader would know that this peek was the last one for the scope.

That makes sense. I guess I was thinking more for simple systems. But yeah, this wouldn't work if you created the peek in an inner function call, for example.

Or are you suggesting that the clear be in the Drop of EventReader itself?

That might be possible too, since the reader should only drop at the end of the main system. Although, that might be bad too if you need to take advantage of the 1-frame delay...

[MrGVSV]

What if we implemented Clone on the EventReader?

Would that require T be Clone as well?

[alice-i-cecile]

Would that require T be Clone as well?

I don't think so: we could clone the internal state, and the references to the events. The events themselves are never moved into the EventReader at all: they stay put in Res<Events<T>> (which, incidentally, is the current workaround).

[djeedai]

which, incidentally, is the current workaround

@alice-i-cecile can you explain that statement please?

[alice-i-cecile]

The underlying event information can be accessed through Res<Events<T>>. This allows users to create multiple ManualEventReader within a single system, avoiding the need to drain the events.

The other workaround that comes to mind is a bit sillier, but quite a lot simpler: simply add two EventReader<T> system parameters. Each will have their own internal state, and will be able to be iterated independently.

[MrGVSV]

The other workaround that comes to mind is a bit sillier, but quite a lot simpler: simply add two EventReader<T> system parameters. Each will have their own internal state, and will be able to be iterated independently.

Doesn't it rely on Local, though? So having two should still affect the same local system state, right? Or does Local work differently than the way I think it does?

[DJMcNab]

Each local is a different value. Consider:

fn sys(a: Local<u32>, b: Local<u32>){
    let a: &mut u32 = &mut *a;
    let b: &mut u32 = &mut *b;
}

Which compiles and runs fine.

[rparrett]

Looks like this was resolved by #12777.