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

Mouse and Pointer Events #577

Merged
merged 11 commits into from
Jul 30, 2023
Merged

Mouse and Pointer Events #577

merged 11 commits into from
Jul 30, 2023

Conversation

hobnob
Copy link
Member

@hobnob hobnob commented Jul 18, 2023

This is a breaking change, and I'm not sure I'm happy about it, so a second (or third) pair of eyes would be very welcome.
This change adds a majority of the properties found in both Mouse Events and Pointer Events. The upshot is that in doing so a simple Click(position) match now becomes Click(position, _, _, _, _, _, _, _), however we now have a wealth of new information about the event itself. some key changes are:

Key Down Properties

Various helper properties (isAltKeyDown etc.) to check if a key was held down during the event. The difference here compared to, say, keeping a track of key up/down events within Indigo, is that if the button is held down outside of the scope of the game (such as in another window) this property will still be correct, whereas Indigo will think the button is not held down.

Buttons Properties

The buttons proeprty was previously just on Pointer events, but is also available for the Mouse. As such, this property has now been moved to the Mouse event and updated to be a Batch of MouseButton

Mouse Enter/Leave

We already have a Pointer Enter/Leave - we now have the same for the mouse, which fixes #512

Tilting, Twisting, and Preassure

Pointer events have been updated with Tilt X/Y, Twist, and Preassure properties. More details on these can be found on the MDN docs. The main change from the docs here is that Twist and Tilt properties are in Radians not degrees.

Limited Firing of Events

Previously we fired mouse events every time we registered a pointer event. We now only do this if the pointer type is Mouse.

Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

Broadly looks good. Going to have another look with fresh eyes.

I'm mostly concerned about the pointerType == PointerType.Mouse filters. Another person was last working on this stuff and I'd been convinced that we should be moving to Pointer events rather than mouse since they were more broadly applicable and allowed us to support additional input types. This looks like we're filtering them out again, but I could be wrong, what's the deal?

MouseButton.fromOrdinalOpt(e.button).foreach { button =>
globalEventStream.pushGlobalEvent(MouseEvent.MouseDown(position, button))

if pointerType == PointerType.Mouse then {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we want that do we? I was under the impression that pointer events were also used for things like touch input?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes they are, but we should really only fire things like MouseDown if the pointer is actually a mouse. To be honest this is a deviation from the browser, which will fire a MouseDown even if what actually occured was a touch event. Happy to keep the more standard browser behaviour of course, I personally feel this is more 'correct', but there's an argument I think for both approaches

indigo/docs/quickstart/hello-indigo.md Outdated Show resolved Hide resolved
@hobnob
Copy link
Member Author

hobnob commented Jul 21, 2023

I'm mostly concerned about the pointerType == PointerType.Mouse filters. Another person was last working on this stuff and I'd been convinced that we should be moving to Pointer events rather than mouse since they were more broadly applicable and allowed us to support additional input types. This looks like we're filtering them out again, but I could be wrong, what's the deal?

There's definitely an argument to be had here for removing mouse events altogether, especially as PointerEvents inherit from MouseEvents, so you're getting no additional information with regards to the Mouse. The difficulty then comes later if you want to deal with Touch. TouchEvents (as fired by touchStart etc) have additional information about which Touches changed and where they started but don't inherit from PointerEvents. The question then becomes do we just ignore this and continue only supporting Pointer, or do we also support Touch, and in that case shouldn't we also support Mouse for completeness? For now, I think we should carry on supporting the mouse, and if we filter on the pointer type then that means the event itself can be used as a differentiator rather than having to do a match based on pointer type... but I could be wrong, it could be that people would prefer to filter on the pointer type rather than the event itself.

@hobnob hobnob force-pushed the features/mouseLeaveEvents branch from 6edb94f to 02ac3ca Compare July 27, 2023 07:47
It was removed accidentally in a rebase
@davesmith00000
Copy link
Member

Right, I'm with you now, thanks for the explanation. 👍

Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hobnob!

@davesmith00000 davesmith00000 merged commit bea2cc2 into main Jul 30, 2023
@davesmith00000 davesmith00000 deleted the features/mouseLeaveEvents branch July 30, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Mouse/Pointer Enter/Leave events
2 participants