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 event for clicks #9240

Closed
wants to merge 32 commits into from
Closed

Conversation

AnbyKatz
Copy link
Contributor

Objective

Solution

  • Add API to send Click events when an Interaction goes from Interaction::Hovered to Interaction::Pressed
  • Incorporate Click event sends into UiPlugin by default
  • Refactor game_menu.rs example to use Click rather then manually checking the Interaction

As an aside, I haven't changed any other bevy example that could use Click just yet. If my implementation is along the lines of what people want then happy to skim through the UI examples and migrate them to use Click if wanted.

Possibly interested party @Shatur :)

crates/bevy_ui/src/lib.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/lib.rs Outdated Show resolved Hide resolved
examples/games/game_menu.rs Outdated Show resolved Hide resolved
examples/games/game_menu.rs Outdated Show resolved Hide resolved
@AnbyKatz
Copy link
Contributor Author

Made some changes @Shatur, let me know what you think. Found some other examples that I'm happy to change to Click's as well

crates/bevy_ui/src/lib.rs Outdated Show resolved Hide resolved
examples/games/game_menu.rs Outdated Show resolved Hide resolved
@mockersf mockersf added A-Input Player input via keyboard, mouse, gamepad, and more A-UI Graphical user interfaces, styles, layouts, and widgets labels Jul 23, 2023
Copy link
Contributor

@Shatur Shatur left a comment

Choose a reason for hiding this comment

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

A few more suggestions

examples/ecs/state.rs Outdated Show resolved Hide resolved
examples/ecs/state.rs Outdated Show resolved Hide resolved
examples/ecs/state.rs Outdated Show resolved Hide resolved
examples/ecs/state.rs Outdated Show resolved Hide resolved
examples/ecs/state.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Shatur Shatur left a comment

Choose a reason for hiding this comment

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

Looks like we have more examples, could you update them too?

examples/ui/size_constraints.rs
examples/ui/display_and_visibility.rs
examples/ui/button.rs
examples/mobile/src/lib.rs
crates/bevy_ui/src/focus.rs

@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Jul 31, 2023
@AnbyKatz
Copy link
Contributor Author

AnbyKatz commented Aug 4, 2023

Apologies still working on this and will hopefully have it done in a week probably. Real life nonsense currently imposing. :(

@Shatur
Copy link
Contributor

Shatur commented Aug 5, 2023

Take your time, hope everything is alright!

@Shatur
Copy link
Contributor

Shatur commented Aug 27, 2023

@AnthonyKalaitzis hope that you are doing okay! Let me know if you are not longer interested, I will finish this PR for you.

@AnbyKatz
Copy link
Contributor Author

AnbyKatz commented Sep 2, 2023

@Shatur Mega apologies, fell of the face of the earth there for a bit (employment interviews zzzzz). Will have the other examples finished by today or tomorrow hopefully. Just pushed one to let people know I'm still alive and working on it. Sorry again for the delay 😢

@Bytekeeper
Copy link

Just a small question: Why not send the click event in ui_focus_system? That way LastInteraction would not even be required?

@Shatur
Copy link
Contributor

Shatur commented Sep 3, 2023

Just a small question: Why not send the click event in ui_focus_system?

This sounds like a great suggestion!
I suggested LastInteraction because this is what I used in my game because without touching Bevy code.

@AnthonyKalaitzis could you try the suggested approach? I think you can do it here:
https://github.com/AnthonyKalaitzis/bevy/blob/f07e153682b6c4129343db09c4ff8c938019c9fd/crates/bevy_ui/src/focus.rs#L178

crates/bevy_ui/src/focus.rs Outdated Show resolved Hide resolved
examples/games/game_menu.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Shatur Shatur left a comment

Choose a reason for hiding this comment

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

Thank you a lot!
One minor nitpick that you can apply from browser and we are good to go

crates/bevy_ui/src/focus.rs Outdated Show resolved Hide resolved
@AnbyKatz
Copy link
Contributor Author

AnbyKatz commented Sep 4, 2023

Big thanks @Shatur for sticking with me through that painful process 😄! Very much a learning experience for me, haven't done much open source stuff before. Big thanks 🥇

Copy link
Contributor

@nicoburns nicoburns left a comment

Choose a reason for hiding this comment

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

I think at some point we'll want better handling of cases where there are multiple elements on top of each other (so the top element captures the event, and it doesn't fire on all overlapping elements). But this seems like an improvement over what we have.

examples/ecs/state.rs Outdated Show resolved Hide resolved
@Shatur
Copy link
Contributor

Shatur commented Oct 8, 2023

@AnthonyKalaitzis could you resolve conflicts?

@Shatur Shatur mentioned this pull request Oct 16, 2023
@alice-i-cecile
Copy link
Member

Adopted in #10141. Don't worry, credit will be shared if this gets merged and your ongoing participation in that thread would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants