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

Inconsistent usage of event attributes #6

Open
therioko opened this issue Mar 28, 2020 · 3 comments
Open

Inconsistent usage of event attributes #6

therioko opened this issue Mar 28, 2020 · 3 comments

Comments

@therioko
Copy link

Again, great work! I've got a minor suggestion when it comes to consistency of event attributes:

Under drag events like InputEventSingleScreenDrag, event.relative relates to event.position. Under InputEventScreenPinch and InputEventScreenTwist, however, event.relative relates to event.distance and angle (the former undocumented btw) while event.position is still present.

I think it'd be preferable to keep event.relative related to event.position and introduce different attributes for distance and angle.

@Federico-Ciuffardi
Copy link
Owner

Hi again 😃!

The consistency comes from the relation between the relative attribute and the attribute that is most important/useful/defining to the event, as the angle to a Twist, the distance between fingers for a Pinch, or the center for Drag. I think this must change if we reach the point that there are two or more of these attributes but for now I think is the best way of handling this.

Of course as I might be wrong, if anyone else reading this agrees with ichbindu or with me please let me know so I can reconsider it.

P.S What do you mean when you say that the angle is undocumented, there is not an angle attribute on InputEventScreenTwist (yet) and the relative attribute of InputEventScreenTwist is documented.

@therioko
Copy link
Author

That makes sense to me. Let's see what others think.

In my specific use case I wanted to use event.position and an attribute for the relative position (which doesn't exist) to also move the camera's x/y while catching InputEventScreenPinch or InputEventScreenTwist. Imagine pinching the screen while simultaneously moving your fingers up/down. With the current implementation I'll need to store event.position separately to have access to it during the next event. No big deal, just something that I stumbled upon as I assumed event.relative would be the one I needed. :)

P.S What do you mean when you say that the angle is undocumented, there is not an angle attribute on InputEventScreenTwist (yet) and the relative attribute of InputEventScreenTwist is documented.

Sorry I was a bit unclear with what I wrote. I meant that event.distance in InputEventScreenPinch is undocumented: https://github.com/Federico-Ciuffardi/Godot-Touch-Input-Manager#inputeventscreenpinch

@Federico-Ciuffardi
Copy link
Owner

Ok. Anyways, if you implement that workaround to get the pinch relative movement and find it useful let me know.

Sorry I was a bit unclear with what I wrote. I meant that event.distance in InputEventScreenPinch is undocumented: https://github.com/Federico-Ciuffardi/Godot-Touch-Input-Manager#inputeventscreenpinch

Just updated the documentation, thanks!

@Federico-Ciuffardi Federico-Ciuffardi added help wanted Extra attention is needed discussion and removed help wanted Extra attention is needed labels Apr 2, 2020
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

2 participants