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

Improve and redesign mouse Input Event #9136

Closed
toger5 opened this issue Jun 12, 2017 · 5 comments
Closed

Improve and redesign mouse Input Event #9136

toger5 opened this issue Jun 12, 2017 · 5 comments

Comments

@toger5
Copy link
Contributor

toger5 commented Jun 12, 2017

Mouse input Event

@pixelpicosean recently and Myself a couple of month ago both put effort in making trackpad support better.
This issue should sum up the ideas to discuss them. There are a couple of discussions in pr's which talk about the same topic but I though a dedicated place would be nice.

current implementation:

most mouse events are just "hacked" into the mouse button event. With custom mouse buttons like:
scrollUp / ScrollDown / scrollLeft / scrollRight and recently Magnify. Precision input is than achieved by a float called factor which is set to the value how far the mouse was scrolled/magnified.

All this made sense When trackpads were simpler and mouses only used scroll wheels which had increments and fired events on each increment. So the wheel actually behaved like a button.

Current Issues:

the issue with this implementation is that it is hard to figure out what event to use for scrolling and magnifying, Because mouse button does not seem to be the most obvious thing (if you don't know about it)
In addition to that it does not fit at all into the new introduced class based event system. Each event type should be another class. not the same class with different types.

Proposal for redesign:

1. move different mouse events to classes

Events all of them Inherit From InpuEventMouse so you have access to position...

InputEvnetMouseScroll
    Vector2 direction
InputEvnetMouseGesturePinch
    float zoom
InputEvnetMouseGestureRotate
    float angle
InputEvnetMouseGestureSwipe
    Vector2i direction
    float speed
InputEventMouseButton
    int button
    bool pressed
...

This system is much more straight forward to use:

comparison:

example for moving node with a trackpad scroll

old:

if event is InputEventMouseButton:
    if event.buttonIndex == MOUSE_BUTTON_SCROLL_LEFT:
        node.position.x -= event.factor
    else If event.buttonIndex == MOUSE_BUTTON_SCROLL_RIGHT:
        node.position.x += event.factor
    if event.buttonIndex == MOUSE_BUTTON_SCROLL_UP:
        node.position.y -= event.factor
    else If event.buttonIndex == MOUSE_BUTTON_SCROLL_DOWN:
        node.position.y += event.factor

new:

if event is InputMouseScroll:
    node.position += event.direction

example for sizing node with a Pinch gesture

old:

if event is InputEventMouseButton:
    if event.button_index == MOUSE_BUTTON_GESTURE_PINCH:
        node.size += event.factor

new:

if event is InputEventMouseGesturePinch:
    node.size += event.zoom

2. Add device enum

in InputEventMouse:

enum Device{
    DEVICE_TRACKPAD
    DEVICE_MOUSE
    ... and maybe drawing table or touchscreen (although that is handled by compleatly different events)     or special apple mouse with touch ;)
}
InputEventMouse
    Device device

This allows to determine the input device and execute different actions for different devices.

example

scroll up in 3d could be panning or orbiting when it is emitted by a touchpad and zoom when it is emitted by a mouse

scroll down in 2d: panning when using DEVICE_TRACKPAD and zoom DEVICE_MOUSE

if event is InputEventMouseScroll:
   if event.device == DEVICE_MOUSE:
        viewport.zoom += event.direction.y
   else if event.device == DEVICE_TRACKPAD:
       vieport.scrollOffset += event.direction

To Discuss

  • of course as always everything (it's godot community, ppl like feedback ;) ).
  • naming: InputEvnetMouseGesturePinch or InputEvnetMousePinch
    • is the Gesture keyword necessary?
  • Is a swipe gesture actually beneficial or should it just be implemented with scroll left/right and some threshold logic (I love using swiping to switch scripts so in general that would be a nice feature. although I think it might be better to implement it as an signal listener for controls. Since every swipe also needs setup when it triggers...)
  • naming of direction, zoom, angle. What would you type in when trying to get the scroll value of a class named InputEventMouseScroll ?

the code I posed is more or less pseudocode. I don't event know if I used correct class and constant names... (pls don't judge)

@toger5
Copy link
Contributor Author

toger5 commented Jun 13, 2017

reference:

- (void)scrollWheel:(NSEvent *)theEvent
   {
      if(([theEvent momentumPhase] != NSEventPhaseNone) || [theEvent phase] != NSEventPhaseNone))
      {
         //theEvent is from trackpad           
      }
      else 
      {
        //theEvent is from mouse
      }
   }

to figure out if the input is coming from mouse or trackpad on OSX

@kubecz3k
Copy link
Contributor

kubecz3k commented Apr 4, 2018

First of all thank you for your report and sorry for the delay.

We released Godot 3.0 in January 2018 after 18 months of work, fixing many old issues either directly, or by obsoleting/replacing the features they were referring to.

We still have hundreds of issues whose relevance/reproducibility needs to be checked against the current stable version, and that's where you can help us.
Could you check if the issue that you described initially is still relevant/reproducible in Godot 3.0 or any newer version, and comment about its current status here?

For bug reports, please also make sure that the issue contains detailed steps to reproduce the bug and, if possible, a zipped project that can be used to reproduce it right away. This greatly speeds up debugging and bugfixing tasks for our contributors.

Our Bugsquad will review this issue more in-depth in 15 days, and potentially close it if its relevance could not be confirmed.

Thanks in advance.

Note: This message is being copy-pasted to many "stale" issues (90+ days without activity). It might happen that it is not meaningful for this specific issue or appears oblivious of the issue's context, if so please comment to notify the Bugsquad about it.

@toger5
Copy link
Contributor Author

toger5 commented Apr 5, 2018

The mouse input definitly has been improoved especially for devices like mac books

I still see advantages when in using the proposed system.
Although it would be another api chagne after 3.0

@Calinou
Copy link
Member

Calinou commented Apr 1, 2020

#36953 is adding a more extensive input gestures system. If that pull request is merged, we may consider this proposal fulfilled.

@akien-mga
Copy link
Member

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants