Skip to content

prototype: UI event system#431

Closed
tommyminds wants to merge 5 commits intobevyengine:masterfrom
tommyminds:prototype/ui_events
Closed

prototype: UI event system#431
tommyminds wants to merge 5 commits intobevyengine:masterfrom
tommyminds:prototype/ui_events

Conversation

@tommyminds
Copy link
Copy Markdown

@tommyminds tommyminds commented Sep 3, 2020

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.

@Moxinilian Moxinilian added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets labels Sep 3, 2020
@tommyminds
Copy link
Copy Markdown
Author

tommyminds commented Sep 5, 2020

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.

ui_events

…d hover/move positions. Use enums to lower amount of events.
Fix a bug where we would not reset the pressed state when releasing outside 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)| {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@alice-i-cecile
Copy link
Copy Markdown
Member

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.

@tommyminds
Copy link
Copy Markdown
Author

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

@tommyminds tommyminds closed this Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants