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

Remove Option from any cursor position API #8865

Open
djeedai opened this issue Jun 17, 2023 · 5 comments
Open

Remove Option from any cursor position API #8865

djeedai opened this issue Jun 17, 2023 · 5 comments
Labels
A-Input Player input via keyboard, mouse, gamepad, and more A-UI Graphical user interfaces, styles, layouts, and widgets A-Windowing Platform-agnostic interface layer to run your app in C-Docs An addition or correction to our documentation C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR

Comments

@djeedai
Copy link
Contributor

djeedai commented Jun 17, 2023

What problem does this solve or what need does it fill?

I'm trying to write a simple UI for some example. Nothing fancy. I keep hitting a wall against all the cursor position APIs returning an Option<Vec2>.

  1. Why is None ever possible? This is documented nowhere. What does it ever mean to get an Interaction::Clicked with a None mouse cursor position?
  2. It's extremely annoying to have to deal with None. This leaks into all code dealing with UI: button click, drag and drop, etc. and there's no good behavior to assign to it.

What solution would you like?

Remove the Option from all those APIs, just use Vec2 directly. If there's really a case where the mouse cursor position is not available, document why and possibly add an escape hatch for handling that, our try our best guess to find a position anyway.

What alternative(s) have you considered?

User has to wrap all APIs into their own helper function(s) to safely unwrap that Option or find a default based on the (primary) window and other factors, which systematically adds several queries to all systems and is far from trivial (see Additional context).

Additional context

  • Interaction has no cursor information on click
  • RelativeCursorPosition stores an Option<Vec2> but it's unused for cases where the cursor is outside the widget, which would be the only case where I find this would make sense.
  • It's unclear how you get the current window where the interaction occurred (vs. the primary one) to be able to retrieve its position and cursor position.
@djeedai djeedai added C-Feature A new feature, making something new possible A-Windowing Platform-agnostic interface layer to run your app in A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jun 17, 2023
@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jun 17, 2023

Why is None ever possible?

I think it's supposed to represent the cursor being outside of the window.

This is documented nowhere

I added a line in #8858 for Window.physical_cursor_position and Window.cursor_position. If it should appear somewhere else in the doc, or if it doesn't have the behaviour I think it has, please comment there!

What does it ever mean to get an Interaction::Clicked with a None mouse cursor position?

I think it's not supposed to happen. If it happened to you it might be linked to this bug: #8840

@djeedai
Copy link
Contributor Author

djeedai commented Jun 17, 2023

Thanks @Selene-Amanita for the docs. I approved your PR. I still think that None is a bad idea, the info it provides is already available from the Window, and the info it loses instead is valuable. Whether the cursor is inside the window should be a separate piece of information from the relative position of the cursor.

@Aceeri
Copy link
Member

Aceeri commented Jun 17, 2023

We don't get cursor position updates outside of the window from winit, so to implement this we'd have to manually track cursor position using DeviceEvent::Mouse, which still will not give us a path where we would always know the position of the cursor.

It's unclear how you get the current window where the interaction occurred (vs. the primary one) to be able to retrieve its position and cursor position.

This is a separate issue that is being solved in #8852.

I feel like this is being too extreme and better solved by improving the API surfaces in other areas. Something that would be helpful is seeing examples of what you are trying to do here and why this is a pain. For example it might be a good idea to add a Vec2 position to Interaction::Clicked or similar.

@djeedai
Copy link
Contributor Author

djeedai commented Jun 17, 2023

For example it might be a good idea to add a Vec2 position to Interaction::Clicked or similar.

Yes that was my initial reaction. But then I remembered Interaction is a component attached to many UI elements, and increasing the size of ALL of them just for the position of a single unique cursor feels like a waste. I don't have a good solution on top of my head.

@djeedai
Copy link
Contributor Author

djeedai commented Jun 17, 2023

About what I'm trying to do:

  1. On right click, open a small menu UI that I wrote at the position of the mouse cursor. This requires knowing which Window the cursor is on, in case there's multiple (not really a blocker as I have a single one now, but the general issue remains).

  2. I have a widget similar to a window or dialog, and when hovering over the "title bar" I want to be able to left click and drag the widget around. This requires knowing the exact position of the cursor on click relative to that widget, so that cursor moves can drag the widget around. Using delta cursor moves doesn't work from experience; it's prone to drift. So this requires knowing at all times the cursor position, subtracting the delta from when the left button was clicked, and moving the widget there.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Jun 17, 2023
@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation and removed C-Feature A new feature, making something new possible labels Aug 7, 2023
@alice-i-cecile alice-i-cecile added the A-Input Player input via keyboard, mouse, gamepad, and more label May 13, 2024
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 A-Windowing Platform-agnostic interface layer to run your app in C-Docs An addition or correction to our documentation C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

4 participants