-
Notifications
You must be signed in to change notification settings - Fork 922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] event: Unwrap user event in map_nonuser_event Err() #1761
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would anyone use map_nonuser_event
to separate Event<T>
into Event<()>
and T
? The purpose is to map Event<T>
to Event<()>
, why would that ever return the user event when you're clearly trying to get rid of it?
@chrisduerr Who says that I'm trying to get rid of them? I want to separate these events out and
It... already does? Just not in a very friendly way? As explained my usecase is identical to what is going on in if let winit::event::Event::UserEvent(thread_event) = event {
// Received message from thread
} else if let Some(event) = event.to_static() {
match event.map_nonuser_event() {
Ok(event) => {
// Send event to thread
let _ = event_send.send(event);
}
Err(_) => unreachable!(),
}
} Or match event.map_nonuser_event() {
Ok(event) => {
// Send event to thread
if let Some(event) = event.to_static() {
let _ = event_send.send(event);
}
}
Err(user_event) => match user_event {
winit::event::Event::UserEvent(thread_event) => {
// Received message from thread
}
_ => unreachable!(),
}
} Or any other variation of With this change I can get rid of that unnecessary match event.map_nonuser_event() {
Ok(event) => {
// Send event to thread
if let Some(event) = event.to_static() {
let _ = event_send.send(event);
}
}
Err(thread_event) => {
// Received message from thread
}
} EDIT: Sure, you can get rid of the |
Which doesn't use It seems like your problem is fundamentally with the combination of |
If it were to use user events, then it'd need it to send an
There is not really a "problem" that this PR attempts to tackle, merely making the resulting |
Currently separating an Event<T> into a user event T and Event<()> is not ergonomic: when map_nonuser_event returns Err() the caller "knows" this can only contain UserEvent(T), yet has to write additional unpacking logic to extract the event, with an unnecessary unreachable!(). Solve this by returning T directly in the Err case, instead of Event<'a, T>.
4ec0b4a
to
769e8b6
Compare
Currently separating an
Event<T>
into a user eventT
andEvent<()>
is not ergonomic: whenmap_nonuser_event
returnsErr()
the caller "knows" this can only containUserEvent(T)
, yet has to write additional unpacking logic to extract the event, with an unnecessaryunreachable!()
.Solve this by returning
T
directly in the Err case, instead ofEvent<'a, T>
.cargo fmt
has been run on this branchcargo doc
builds successfullyCHANGELOG.md
if knowledge of this change could be valuable to usersmultithreaded.rs
? That is the exact use case we need this for: handle user events (from the thread) in the event loop, and send nonuser events into the thread.