Skip to content
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

Add an Activate triggered event to bevy_ui #17267

Closed
wants to merge 7 commits into from

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Jan 9, 2025

Objective

While working on #17224, I realized that we need a blessed way to activate (trigger, use) UI elements. This needs to work for both pointer clicks and buttons that get dispatched ("A" or "Enter") to the currently focused element.

We could emulate a pointer click, as that PR does (well, it's a press, but same thing). That's what the web does after all!
That gives buttons a clear signal to respond to, regardless of the input device.

There's a few problems with this:

  1. Pointer events in Bevy carry a ton of data, most of which doesn't have a sensible default when they're being emulated in this way. See this permalink for the nonsense data I filled in.
  2. Sending pointer events is extremely verbose: this took 27 lines, while it should have taken 1.
  3. This privileges one input device over others, and makes it harder and weirder to use bevy_ui on devices without pointers (notably consoles).
  4. Application developers can't agree on whether "press" or "click" should be used for buttons: press is more responsive but click allows roll-off cancellation.

Solution

  • Introduce bevy_ui::actions::Activate, which is canonically used for "do something with a UI widget when I press enter"
  • Add simple hierarchical event bubbling to this action
  • Demonstrate how to use observers to respond to listen to the Activate action.
  • Add systems which listen to the standard inputs to trigger "Activate" to UiPlugin
  • Make sure users can disable these systems if they want to do something weirder or more sophisticated

To do (this PR)

  • Update the navigation examples to use this pattern
  • Cut the systems from this PR, to reduce controversy.
  • Remove bubbling: the idea is to have each widget listen for the correct inputs and send these events to themselves.

Future work

  • Migrate all of our examples to use this pattern. This has been split out for reviewability.
  • Use a more sophisticated bubbling strategy once our requirements are better known. Bubbling to the window adds a lot of complexity, and PickingTraversal could not be reused.
  • Add other actions: Cancel is definitely desired, but other actions are likely valuable

Testing

Additional context

  • This design was prompted by @aevyrie and @NthTensor, who were rightly confused that I was trying to make it easier to send stripped-down picking events.
  • This "send an event and bubble it up" action pattern is something that @viridia and I have been discussing as a general-purpose tool for focus management / contextual actions. LWIM (and other action managers) should be able to adapt to this pattern pretty comfortably, although users of those crates will want to disable the built-in systems here to enable rebinding.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets M-Needs-Release-Note Work that should be called out in the blog due to impact X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 9, 2025
@NthTensor NthTensor self-requested a review January 9, 2025 20:51
@@ -36,6 +36,8 @@ mod render;
mod stack;
mod ui_node;

pub mod actions;
Copy link
Member Author

Choose a reason for hiding this comment

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

The "pub use module::*`" style below is pretty terrible for documentation: both because "pile of stuff" and because it breaks module docs.

I've broken from convention here: I would like to change the other modules in this crate in another PR.

@alice-i-cecile alice-i-cecile requested a review from cart January 9, 2025 21:02
@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 9, 2025
@alice-i-cecile alice-i-cecile marked this pull request as draft January 9, 2025 21:12
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Jan 9, 2025
input_focus: Res<InputFocus>,
mut commands: Commands,
) {
if keyboard_input.just_pressed(KeyCode::Enter) {
Copy link
Member

Choose a reason for hiding this comment

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

Hard-coding these to specific inputs isn't ideal. Seems like this is begging for an input-manager-defined remappable action 😄

@@ -0,0 +1,152 @@
//! Semantically meaningful actions that can be performed on UI elements.
Copy link
Member

Choose a reason for hiding this comment

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

I see why you're adding this, but it feels like it might be too opinionated. A generic "activate" action seems like it would be context specific. For example, I can easily imagine a button that wants to "activate" on release, that only "activates" while held, or only if it is held for a period of time, or that wants to activate even if it is not focused, or a widget that cannot be "activated" at all.

If we're going to go this route, I think we should embrace it as a "high level focused button system" that uses familiar and less opinionated terminology and encapsulates the whole lifecycle (ex: something like Pressed / Released or FocusedPress/FocusedRelease), and makes that state poll-able (to allow people to distinguish between "just pressed" and "currently pressed" each frame). And it would need to be able to accommodate things like "firing the release event for an entity if focus changes while pressed".

I also think that a lot of the utility / UX improvement of this impl comes from performing an action based on multiple input types (aka this is providing the same utility that an action manager would provide).

Investing in an upstream action system would let each widget express things like:

if actions.just_pressed(UiSelect) {
  if let Some(entity) = input_focus.get() {
    // do_thing
  }
}

if actions.just_released(UiSelect) {
  if let Some(entity) = input_focus.get() {
    // do_thing
  }
}

From there, we could build the "high level entity focus based virtual button system" on top:

entity.observe(|trigger: Trigger<FocusedPress>| {
})

entity.observe(|trigger: Trigger<FocusedRelease>| {
})

However I think introducing this at all seriously complicates the "event handling story", as users now need to decide to listen for either Pointer<Pressed> or FocusedPress. These are two separate generic input event systems that both perform very similar roles (and Pointer<Pressed> directly drives the focus system). It also means that people that want pointer information in the context of FocusedPress would need to grab that from somewhere else. I'm thinking a "virtual input-action-driven Pointer", as you used #17224 is probably the better move, as it allows everyone to snap to the single, already-generic pointer API. All of the boilerplate of producing the generic pointer info would be internal bevy logic (so not user facing), and it would all serve a purpose (people want to able to write logic about pointer position, read hit data, etc).

Imagine a button that when pressed, applies a visual effect at the point it was pressed at. If a context-free event like FocusedPress is used, they would need to somehow grab the current pointer state separately (if it was driven by a pointer) and if it was driven by a button, pick some fall back logic. By embracing Pointer as the "one api" and implementing a virtual pointer system, we can make an opinionated choice about where it exists on screen (ex: the "middle" of the focused UI element). Then consumers of the API can blissfully code to the Pointer api (and receive all of its benefits). This also allows the "virtual pointer" behavior to be user-configurable. Ex: maybe they want to trigger the "middle pointer click" on a given input action. And by nature of supporting "multiple pointers", we get all of the same behaviors "for free" / they behave consistently (ex: the most recent pointer change "stealing" focus), without users needing to think about supporting both use cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in agreement with you here, at least on the first part. I'm OK with the concept of an Activate event, but I think that the decision as to when it should be triggered is widget-specific.

Consider the case of a checkbox: emitting an Activate event is not all that useful, what you really want is a ValueChanged(bool) event. A slider emits a ValueChanged(f32) and so on. Different widgets will emit different "semantic" events which are up-conversions of low-level "physical" events.

Different button behaviors that you describe (activate on middle button and so on) could easily be implemented as distinct widget types - it's just a set of observers. However, we don't want developers to have to constantly re-implement Button or Checkbox behaviors, because the "correct" behaviors are subtle and easy to get wrong, especially when we add a11y into the mix.

This is why I have been lobbying for a collection of "core widgets" in Bevy. These would be "headless" (unstyled) widgets that implement PushButton, Checkbox, Slider, Spinbox and other behaviors, and would include the proper focus handling and accessibility goodness.

These headless widgets would be designed to work in conjunction with BSN or whatever templating system - they are just components after all, with a handful of registered observers to update the component state and re-emit events.

Such widgets could use Activate and ValueChanged<T>, so I think it's helpful to define these events in a standard way; but I don't like the idea of triggering them broadly.

@alice-i-cecile
Copy link
Member Author

Thanks for the feedback :) I can live with "everything gets turned into mocked pointers" as a design principle. Let me close this out and make a PR to make that pattern easier to do.

@aevyrie
Copy link
Member

aevyrie commented Jan 10, 2025

Only had a few minutes to do a super fast drive by, some quick thoughts:

We could emulate a pointer click, as that PR does (well, it's a press, but same thing).

Presses and clicks are very distinct, and it's an important one! Clicks end on the same entity they started on. This is how you can "cancel" clicking a button by moving your pointer somewhere else before releasing. Edit: you noted this elsewhere.

Pointer events in Bevy carry a ton of data, most of which doesn't have a sensible default when they're being emulated in this way

I may have missed something important due to the quick look, but this shouldn't be true. If the UI element you are psuedo-clicking on is in the UI, all the required picking information should be able to be provided, and is useful information for anyone listening to that event. For example, the UI should exist on some render target, the UI node has some center position on that target you can use, and UI trees should be associated with a camera.

@aevyrie
Copy link
Member

aevyrie commented Jan 10, 2025

This privileges one input device over others, and makes it harder and weirder to use bevy_ui on devices without pointers (notably consoles).

It seems strange to me to tie picking to this at all. It would make mores sense to me if buttons sent "button_click" or something when they are clicked. It is up to the implementation to choose what causes that - could be an enter key on focus, could be a pointer press, or could be a pointer click. Now, we would want to provide a Button that has these things hooked up, but it means you aren't tying the inputs to the interactions. That's a pretty common pattern I follow to avoid these issues.

Instead of focus_enter -> pointer_click -> button click response logic
do (focus_enter | pointer_press) -> button_click -> button click response logic

Then it doesn't matter what is in that first part - it could be a gamepad plugin sending the button click, bevy_ui doesn't care.

Again, apologies if I missed things while skimming! I can come back to this later.

@alice-i-cecile
Copy link
Member Author

Closed in favor of mocking picking events; see #17399.

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 D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Contentious There are nontrivial implications that should be thought through
Projects
Status: Responded
Development

Successfully merging this pull request may close these issues.

4 participants