track interaction from all mouse buttons in UI

[mockersf]

Objective

Track interaction from all mouse buttons
Fixes #2322

Solution

I added an enum to the clicked state that specify which mouse button clicked it.

[Weibye]

What about mice with additional buttons like what is commonly seen with gaming-mice?

[mockersf]

Thanks, forgot about those... They are now added

[alice-i-cecile]

This is an excellent stop-gap and should be merged.

[NathanSWard]

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

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

[mockersf]

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

[billyb2]

Why not just make it:

pub enum ClickedWith {
    MouseButton,
    Touch,
}

?

[mockersf]

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

[billyb2]

Oh alright

[NathanSWard]

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)

[billyb2]

Hey, just checking on the PR. Is there anything that needs to be fixed before it can be merged?

[alice-i-cecile]

Hey, just checking on the PR. Is there anything that needs to be fixed before it can be merged?

I think we're good to; we're just in a bit of a PR backlog as Cart focuses on getting the rendering rework done. @mockersf has merge power as well now, but I don't think this qualifies as "completely uncontroversial", especially since it's his own PR :)

[NathanSWard]

I'm still a fan of reusing MouseButton even if it does cost a few most characters to type out.
However other than that, this PR should be good to go

[cart]

This doesn't behave quite how I would expect it to. If you "press left click", "press right click", then "release right click", it will be in the "hovered" state, despite left click still being clicked. We would need a "stack" to support this correctly (or poll current click state every frame).

[cart]

Alternatively, maybe we should rely on event queues instead of component state.

[cart]

But that would also be a pretty major change to a system that we plan on changing anyway. Not sure its worth developing new things here when we could be investing that energy in the "next" api.