Remove Interaction, and replace it with bevy_picking based fields on Button#15597
Remove Interaction, and replace it with bevy_picking based fields on Button#15597alice-i-cecile wants to merge 15 commits intobevyengine:mainfrom
Interaction, and replace it with bevy_picking based fields on Button#15597Conversation
Interaction, and replace it with a bevy_picking based alternativeInteraction, and replace it with bevy_picking based fields on Button
|
From game maker perspective- how to most easily get the "is this button just released" info? |
I think that the best strategy is to use an observer listening to |
|
The generated |
| pickable: Pickable { | ||
| should_block_lower: true, | ||
| ..Default::default() | ||
| }, |
There was a problem hiding this comment.
| pickable: Pickable { | |
| should_block_lower: true, | |
| ..Default::default() | |
| }, | |
| pickable: Pickable::default(), |
should_block_lower is true by default
There was a problem hiding this comment.
Great; that means we can swap to a pure derive then.
|
|
| let mut hovering_over_descendant = false; | ||
|
|
||
| for descendant in children_query.iter_descendants(entity) { | ||
| if map.contains_key(&descendant) { | ||
| hovering_over_descendant = true; | ||
| break; | ||
| } | ||
| } | ||
| hovering_over_descendant |
There was a problem hiding this comment.
Isn't it possible to use any() with DescendantsIter?
| #[derive(Component, Debug, Default, Clone, Copy, PartialEq, Eq, Reflect)] | ||
| #[reflect(Component, Default, Debug, PartialEq)] | ||
| pub struct Button; | ||
| pub struct Button { |
There was a problem hiding this comment.
Before I remembered to check if anyone had started on this yet, I started migrating a few of the examples and I went with the same identical Button { pressed: bool, hovered: bool } component implementation.
I think for the default Button representation, like MiniaczQ pointed out, I prefer the pressed-until-released behaviour. And then have something like a PushButton for the release-on-exit. But I don't think it's that important, this and a StickyButton component (or whatever we call it) would be fine too.
There was a problem hiding this comment.
With release-on-exit buttons, the button.pressed && !button.hovered state is invalid? maybe the fields should be private, with pressed() and hovered() accessors where pressed() returns self.pressed && self.hovered
| derive(serde::Serialize, serde::Deserialize), | ||
| reflect(Serialize, Deserialize) | ||
| )] | ||
| pub struct RelativeCursorPosition { |
There was a problem hiding this comment.
We could keep the RelativeCursorPosition component for now and just update it in update_hover_status or something. It's easier to use than sometimes than tracking move events.
| // the listed systems do not affect calculated size | ||
| .ambiguous_with(crate::ui_stack_system), | ||
| button_changed, | ||
| button_changed.after(UiSystem::Focus), |
There was a problem hiding this comment.
We should rename UiSystem::Focus to UiSystem::Picking I think.
| // Avoid triggering change detection spuriously | ||
| // We can't use `set_if_neq` here because we're only looking at a single component | ||
| if button.hovered { | ||
| button.hovered = false; | ||
| } |
There was a problem hiding this comment.
| // Avoid triggering change detection spuriously | |
| // We can't use `set_if_neq` here because we're only looking at a single component | |
| if button.hovered { | |
| button.hovered = false; | |
| } | |
| button.set_if_neq(Button { | |
| pressed: button.pressed, | |
| hovered: false, | |
| }); |
There was a problem hiding this comment.
Wait, if no buttons are hovered we don't need to preserve the pressed state either, so both fields should be set to false?
|
|
||
| // Avoid triggering change detection spuriously | ||
| // We can't use `set_if_neq` here because we're only looking at a single component | ||
| if button.hovered != is_hovered { |
There was a problem hiding this comment.
| if button.hovered != is_hovered { | |
| button.set_if_neq(Button { | |
| pressed: button.pressed, | |
| hovered: is_hovered, | |
| }); |
| let is_hovered = if map.contains_key(&entity) { | ||
| true | ||
| } else { | ||
| let mut hovering_over_descendant = false; |
There was a problem hiding this comment.
Users should just should_block_lower: false for this? Some interfaces use nested buttons.
As well with should_block_lower: true unless Down events are propagated upwards, won't it be possible that a button could be hovered without being pressable?
Pull request was converted to draft
Objective
Fixes #15550. See that issue and the linked issues within it about why the existing
Interactioncomponent is inadequate.Solution
Interactionand all engine systems that use itpressedandhoveredfields toButtonbevy_pickingbuttonexamplemake Text entities not pickable by defaultFocusPolicyin favor ofPickableRelativeCursorPosition. Widgets should compute this information themselves based onbevy_pickinginput.animation_graphexampleNote to reviewers: the previous solution has special-cased logic for hidden nodes to ensure that they can't be pressed. I would prefer not to implement that in this PR, as that's fundamentally a picking backend concern.
Testing
For basic functionality, run the
buttonexample.For something more holistic, try the
game_menuexample.Migration Guide
The
Interactioncomponent has been removed. Instead, use the newpressedandhoveredfields onButton. Bear in mind that you can also use observers to listen forbevy_pickingevents such asPointer<Down>andPointer<Over>directly as well.To migrate code that matches on
Interactionto change some property, follow the example below:Bevy 0.14:
Bevy 0.15:
NodeBundle(and their Bevy 0.15 equivalents) no longer track whether or not they are pressed / hovered by default. If you require this functionality, add theButtoncomponent to your entity.The
FocusPolicycomponent has been removed, and replaced with the universalPickablecomponent.The
RelativeCursorPositioncomponent has been removed. If you need this information, you should compute it frombevy_pickingevents in your widget's logic. See theanimation_graphexample for a demonstration of how this can be done.