prototype: UI event system #431
Conversation
tommyminds
force-pushed
the
prototype/ui_events
branch
from
9061df0
to
5c4d265
Compare
Moxinilian
added
C-Feature
|
As discussed on discord, I updated the prototype to group certain UI events into enum to preserve order and lower the amount of global events. Combined interaction updates with the event system so that both get properly updated in the same frame and simplify the code for it. Also implemented hover/move and removed doubleclick.
|
tommyminds
force-pushed
the
prototype/ui_events
branch
from
4db247d
to
e208794
Compare
…d hover/move positions. Use enums to lower amount of events.
tommyminds
force-pushed
the
prototype/ui_events
branch
from
e208794
to
9df757e
Compare
…utside of the entity.
| let mut query_iter = node_query.iter(); | ||
| let mut moused_over_z_sorted_nodes = query_iter | ||
| .iter() | ||
| .filter_map(|(entity, node, interaction, transform, propagate_policy)| { |
There was a problem hiding this comment.
Avoid Multi-Line Closures. In this case, it makes the code harder to read, especially if we add another operation after the filter_map. It’s difficult to understand what the closure is doing upon first glance. Also, there’s a good chance that we will want to reuse the behavior of the closure elsewhere in our program. Simple extracting to a named function can help. And while doing that consider refactoring ui_event_system for better readability. Looking at the code we can see 10 if statements, one of which check more than one condition. It helps naming the conditions by extracting them to a private functions. Also great way of reducing the size of ui_event_system could be by extracting the if/else statement part into a smaller private functions.
|
From my reading of this PR, it looks like this approach will suffer from difficulties with event-propagation speed ("deep UI") and boilerplate that makes it hard to share logic between widgets (which is pushing current designs towards using a channels wrapper for events). See recent comments on #254 for more details. |
|
Agreed. This PR hasn’t aged well. I think it is still an improvement over the current UI events, but should be closed in favor of a complete revamp of UI |
I made a quick prototype of what a UI event system could potentially look like.
This could potentially fix some of the open issues mentioned here.
I am not sure if I like the API of handling these events for particular entities this way, but interested to get opinions on if we want to explore this further rather than using Interactions for things like Click/DoubleClick/MouseDown/MouseUp/MouseHover.