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

track interaction from all mouse buttons in UI #2324

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions crates/bevy_ui/src/focus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,18 @@ use bevy_transform::components::GlobalTransform;
use bevy_window::Windows;
use smallvec::SmallVec;

#[derive(Copy, Clone, Eq, PartialEq, Debug)]
pub enum ClickedWith {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of redefining the mouse buttons, can we just reuse the MouseButton enum?

pub enum ClickedWith {
    Mouse(MouseButton),
    Touch,
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do that to avoid adding an extra layer to deconstruct when matching... I would prefer not to, but ok to change if people really feel it's better to reuse and add a layer

Currently it's

Interaction::Clicked(ClickedWith::MouseLeftButton)

With this it would be

Interaction::Clicked(ClickedWith::Mouse(MouseButton::Left))

Which starts to be painful when matching

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just make it:

pub enum ClickedWith {
    MouseButton,
    Touch,
}

?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would only work if you want to react the same way to any mouse button click, Bevy shouldn't force that on users

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh alright

Copy link
Contributor

@NathanSWard NathanSWard Jun 9, 2021

Choose a reason for hiding this comment

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

I would prefer not to, but ok to change if people really feel it's better to reuse and add a layer

I'm personally a big fan of reusing stuff we already have.
It's fair that it is more code to write however, it comes at the benefit of reusing existing code which is a net gain. (Especially if MouseButton were to change for some reason)

Also you could probably change ClickedWith to just Clicked to reduce a few characters :)
And you could also then do impl From<MouseButton> for Clicked

However at the end of the day, if most people don't want to reuse MouseButton I'm fine with leaving it as is. (I just have a preference for re-usability)

MouseLeftButton,
MouseMiddleButton,
MouseRightButton,
MouseOtherButton(u16),
Touch,
}

#[derive(Copy, Clone, Eq, PartialEq, Debug)]
pub enum Interaction {
Clicked,
Clicked(ClickedWith),
Hovered,
None,
}
Expand Down Expand Up @@ -70,20 +79,28 @@ pub fn ui_focus_system(
}

let mouse_released =
mouse_button_input.just_released(MouseButton::Left) || touches_input.just_released(0);
mouse_button_input.get_just_released().last().is_some() || touches_input.just_released(0);
if mouse_released {
for (_entity, _node, _global_transform, interaction, _focus_policy) in node_query.iter_mut()
{
if let Some(mut interaction) = interaction {
if *interaction == Interaction::Clicked {
if matches!(*interaction, Interaction::Clicked(_)) {
*interaction = Interaction::None;
}
}
}
}

let mouse_clicked =
mouse_button_input.just_pressed(MouseButton::Left) || touches_input.just_released(0);
let interacting_from = mouse_button_input
.get_just_pressed()
.last()
.map(|button| match button {
MouseButton::Left => ClickedWith::MouseLeftButton,
MouseButton::Middle => ClickedWith::MouseMiddleButton,
MouseButton::Right => ClickedWith::MouseRightButton,
MouseButton::Other(button) => ClickedWith::MouseOtherButton(*button),
})
.or_else(|| touches_input.just_released(0).then(|| ClickedWith::Touch));

let mut moused_over_z_sorted_nodes = node_query
.iter_mut()
Expand Down Expand Up @@ -118,10 +135,10 @@ pub fn ui_focus_system(
// set Clicked or Hovered on top nodes
for (entity, focus_policy, interaction, _) in moused_over_z_sorted_nodes.by_ref() {
if let Some(mut interaction) = interaction {
if mouse_clicked {
if let Some(interacting_from) = interacting_from {
// only consider nodes with Interaction "clickable"
if *interaction != Interaction::Clicked {
*interaction = Interaction::Clicked;
if !matches!(*interaction, Interaction::Clicked(_)) {
*interaction = Interaction::Clicked(interacting_from);
// if the mouse was simultaneously released, reset this Interaction in the next
// frame
if mouse_released {
Expand Down
2 changes: 1 addition & 1 deletion examples/ecs/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn menu(
) {
for (interaction, mut material) in interaction_query.iter_mut() {
match *interaction {
Interaction::Clicked => {
Interaction::Clicked(_) => {
*material = button_materials.pressed.clone();
state.set(AppState::InGame).unwrap();
}
Expand Down
6 changes: 3 additions & 3 deletions examples/ui/button.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ fn button_system(
for (interaction, mut material, children) in interaction_query.iter_mut() {
let mut text = text_query.get_mut(children[0]).unwrap();
match *interaction {
Interaction::Clicked => {
text.sections[0].value = "Press".to_string();
Interaction::Clicked(button) => {
text.sections[0].value = format!("Press {:?}", button);
*material = button_materials.pressed.clone();
}
Interaction::Hovered => {
Expand All @@ -65,7 +65,7 @@ fn setup(
commands
.spawn_bundle(ButtonBundle {
style: Style {
size: Size::new(Val::Px(150.0), Val::Px(65.0)),
size: Size::new(Val::Px(450.0), Val::Px(65.0)),
// center button
margin: Rect::all(Val::Auto),
// horizontally center child text
Expand Down