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

Interaction component refactor #7257

Closed
wants to merge 62 commits into from

Conversation

Pietrek14
Copy link
Contributor

@Pietrek14 Pietrek14 commented Jan 18, 2023

This PR's original objective was only to fix #7239. The original solution was to introduce a new InteractionPolicy component that would regulate the Interaction component behaviour.

Objective

Decouples the Hovered and the Clicked state of the Interaction component. Also fixes #7239.

Solution

  • The Interaction component has been removed
  • Interaction's purpose is now fulfilled by the Pressed and the RelativeCursorPosition components
  • To keep the simple interface of the Interaction component, I introduced an InteractionState enum, which is not a component, but contains the same variants as the Interaction component. To get the node's InteractionState the user can call interaction_state method on (&Pressed, &RelativeCursorPosition).
  • Also introduced a InteractionStateChangedFilter, which looks for changes in both Pressed and RelativeCursorPosition components, although it's not as performant as the previous Changed<Interaction>, because it also fires then the cursor moves

Changelog

  • Added:
    • Pressed component
    • PressPolicy struct used inside the Pressed component
    • an example showcasing how the PressPolicy works
    • InteractionState enum
    • InteractionStateHandler trait implemented for (&Pressed, &RelativeCursorPosition)
  • Modified
    • ButtonBundle replacing the Interaction component with Pressed and RelativeCursorPosition components
    • all the examples using the old Interaction component
  • Removed
    • the Interaction component

Migration guide

  • If you queried for Interaction, you will now have to query for Pressed and RelativeCursorPosition and call the interaction_state method on (&Pressed, &RelativeCursorPosition)
  • If you only used the Clicked state from Interaction you can query only for the Pressed component
  • If you only used the Hovered state from Interaction you can query only for the RelativeCursorPosition component and call its mouse_over method

@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-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jan 18, 2023
@@ -53,6 +53,27 @@ impl Default for Interaction {
}
}

/// Describes whether [`Interaction`] component should remain in the `Clicked` state after
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Describes whether [`Interaction`] component should remain in the `Clicked` state after
/// Describes whether [`Interaction`] component should remain in the [`Pressed`](Interaction::Pressed) state after

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Interaction state is called Clicked though. Should I change the state name to Pressed or keep the comment saying Clicked and link the Interaction::Clicked?

Copy link
Contributor

@Weibye Weibye Jan 21, 2023

Choose a reason for hiding this comment

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

Yes, I think we should rename it to Interaction::Pressed so that it makes more sense when we eventually start supporting Interaction::Released (#5769)

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, hmm. On second thought no.

Lets keep the name as Interaction::Clicked and do the rename in #7240 so that those two changes are kept together. Will be easier to review that way.

Cargo.toml Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@Pietrek14
Copy link
Contributor Author

How do I fix the check bans errors? Do I do something with Cargo.toml?

@Pietrek14
Copy link
Contributor Author

Hey @Weibye, I see you self-requested a review quite some time ago. Are you still up to it? If you aren't, it's obviously alright, I'm just checking in case you forgot.

@Selene-Amanita Selene-Amanita added this to the 0.12 milestone Jul 17, 2023
@UkoeHB
Copy link
Contributor

UkoeHB commented Sep 29, 2023

@alice-i-cecile asked the discord for feedback on this PR. I have not used bevy_ui so I can't evaluate in that context, but I did build an interaction system for bevy_lunex so I will discuss interaction from that perspective. The code lives in bevy_kot.

There are a lot of things to think about with interaction if you want what I would consider a production-grade solution. These features can be implemented in more than one way, but I describe them from the point of view of the system I built (which does not use the mainstream approach of event bubbling and pointer capture).

Mandatory features

  • Interaction sources: An interaction source (aka 'pointer') is composed of two pieces, a location and a click state (just clicked, is clicked, just unclicked). Interaction sources can be arbitrary: mouse + left button, mouse + right button, mouse + Space key, touchscreen input, console pointer + X button, etc. You can also have multiple interaction sources simultaneously - such as the multiple active cursors you find in the Super Smash Bros character-selection screen.
  • UI scope: Interactive elements are scoped to specific UI aggregations (in lunex these are UiTrees), which are in turn scoped to specific OS windows. Elements in non-focused windows should be ignored by the interaction system. [note: window focus tracking is not yet implemented in bevy_kot, and multiple UI trees is not yet implemented in bevy_lunex]
  • Interaction scoping: If you have many UI layers, you only want interactive elements at the top of the stack to respond to interactions. Visibility checks don't cut it, because you might want to disable interactions for elements while leaving them visible (e.g. for pop-ups that lock out other parts of the screen). You may also want to 'path' interactions through scoping elements to child interactive elements (this is useful to get O(logN) targeting instead of O(N), and gives you more control over the interactivity space of an element). [note: scoping elements is not yet implemented in bevy_kot]
  • Pressed vs Clicked: An interaction source has a click state, while an interactive element has a pressed state. Usually those states line up, but separating them means an element has more direct and flexible control over when it enters and exits a pressed state in response to the source's click state and location.
  • Press: Home vs Away: After an element becomes pressed, the cursor may move away from the element while leaving the element pressed. When above the element, the press state is 'pressed home', and if away it is 'pressed away'. Exactly when an element becomes unpressed has two possibilities related to home vs away: when a cursor unclicks (while home or possibly away), or when pressed away starts. A press on an element may also abort, with multiple possibilities: never (not recommended), when pressed away starts, or when the element becomes obstructed. All of these configurations should be user-selected.
  • Press Home Zone: The zone/area where an element becomes pressed may differ from the zone that you want to track 'pressed home'. For example, a rotatable 3D object may become pressed when clicking it, then you can drag the mouse around within an interaction window to rotate it, but if your mouse goes outside the interaction window (which may be embedded in a panel of windows (e.g. an editor)) then rotation stops, but if your mouse returns to the window then rotation resumes. In that case, the 'press home zone' would be the interaction window.
  • State-handling order: Interaction state handling is discretized into ticks, which means multiple state changes can be handled in a single update (e.g. click -> unclick or unclick -> click). The order that state-handlers are invoked should align as close as possible with the order of actual state changes so that edge conditions are handled robustly.
  • Efficient: Interaction targeting should be a minimal cost for your app, even with many interactive elements. I'd like to target 10k interactive elements at 125 FPS on modern hardware as a good stretch-goal for evaluating if an interaction system has a negligible footprint. Using O(logN) targeting can more-or-less eliminate perf issues for reasonable system designs, so the test should be done with an O(N) UI structure since that is the degenerate case. [note: bevy_kot is not there yet, pending efficiency improvements in bevy_lunex]

Nice-to-have features

These are useful features that can either be implemented on top of the mandatory stuff or aren't strictly necessary.

  • Automatic select: 'Selected' is a long-term state change in an element triggered by interactions.
  • Visibility control: An element may have different widgets for its appearance whose visibility should be automatically toggled as the element moves through default/pressed/selected/hovered/pressed+selected/etc. state changes.
  • Hover control: An element may have specific responses to being hovered that need to be disabled when pressed (or selected). You should be able to say, for example, 'disable hovering when pressed' so that the element becomes unhovered immediately when pressed and remains unhovered until unpressed.
  • Fine-grained callbacks: You should be able to define callbacks on an element that react to interaction events. The reason to embed these callbacks in the interaction system is some level of internal detail is lost when you translate interactions into component changes on an element entity, and callback data dependencies may have strict ordering requirements within the interaction-handling pipeline. In particular, due to the distinction between clicks and presses where only a Pressed component is added to an element entity, you cannot define callbacks outside the interaction pipeline for click state changes on a specific element (without duplicating targeting identification work done by the interaction pipeline).

bevy_kot explanation (links for commit 1ea3623 [09/28/23])

bevy_kot interactive elements uses a baked-in interaction handling pipeline that is generic over interaction sources (which are generic over cursor tags and UI tree tags). Elements are tagged with components that allow the pipeline to filter for elements that a specific interaction source may target. An interactive element builder constructs a set of action callbacks and interaction response callbacks based on user-specified configuration details, and adds those to the element entity. Interaction response callbacks internally call the entity's action callbacks as needed, and action callbacks internally modify Pressed/Selected/Hovered components on the entity. Both sets of callbacks internally invoke user-defined callbacks injected to the element builder, in addition to adjusting the hover state and widget visibility of the element based on related configuration details. Callbacks are NOT added to elements if they are not needed by any of the requested element builder configurations.

The interactive element builder has a large number of configuration details, including when to enter/exit the pressed state. All configuration behaviors are baked into the callbacks.

When a pipeline for an interaction source runs, it first iterates over targetable elements to identify the top-most element under the cursor. It assembles a pack of information about the source which is used by the rest of the pipeline. The remainder of the pipeline is a series of interaction response callback invocations based on the current state of the source.

The interaction system allows a cursor implementer to define how element intersections are detected. Elements may have a distinction between the element itself and the element's 'press home zone', in which case pressing (after press start) and unclicks are contextualized to the press home zone (via LunexCursor::cursor_intersects_press_home_zone()).

Finally, the interaction system allows you to tag entities with an interaction barrier (assuming they have a lunex Widget), which effectively blocks targeting on entities lower than the lunex widget associated with that barrier, if the barrier intersects with the cursor.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Sep 29, 2023
@alice-i-cecile alice-i-cecile removed this from the 0.12 milestone Sep 29, 2023
@alice-i-cecile
Copy link
Member

Talking it over some more, I don't think that this is the right next step. Like both @UkoeHB and @aevyrie (on Discord) have said, I think there's a lot of subtle complexity here and I think we need to take a step back and do some serious design rather than incrementally trying to fix the very real problems we have.

I'm going to close this out, but if we (or you!) do that design work (in a Github Discussion or RFC) and the solution ends up looking like this, please feel free to reopen or remake this PR.

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 M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ui Button is still considered pressed when holding down, but moving cursor out of the button Rect
7 participants