feat: Component Mixin to Receive Cursor Hover Events by garysm · Pull Request #1834 · flame-engine/flame

garysm · 2022-08-05T06:05:59Z

Description

This PR adds the HasCursorHandlerComponents mixin on FlameGame, and the CursorHandler mixin on Component. Heavily inspired by HasKeyboardHandlerComponents.

HasCursorHandlerComponents propagates cursor hover events to child CursorHandler Components, making these events easier to handle.

Checklist

The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
I have read the Contributor Guide and followed the process outlined for submitting PRs.
I have updated/added tests for ALL new/updated/fixed functionality.
I have updated/added relevant documentation in docs and added dartdoc comments with ///.
I have updated/added relevant examples in examples.

Breaking Change

Yes, this is a breaking change.
No, this is not a breaking change.

Related Issues

Closes #1818

spydon

Lgtm except for the too long lines!

spydon · 2022-08-12T20:08:23Z

@garysm I fixed the too long lines, could you add some docs for the functionality?
Possibly also an example for the examples directory.

garysm · 2022-08-13T04:17:04Z

@spydon Yes, I will work on docs and provide an example. Thank you for fixing the too long lines.

alestiago · 2022-08-14T19:04:40Z

Would we like to remove this mixin if #1733 gets done?

garysm · 2022-08-20T13:42:47Z

@alestiago, that would probably make sense. I can finish up with this, then convert to a class once #1733 is implemented.

…/flame into cursor-handler-components

garysm · 2022-08-30T18:44:29Z

Added docs and example @spydon . Let me know if you'd like a test to be written as well.

spydon · 2022-08-30T18:56:41Z

Good job @garysm, I'll hopefully have time to have a look in the coming days!

@st-pasha didn't you have some feedback on this PR?

st-pasha · 2022-08-31T00:06:26Z

Yes, I'll re-review this PR. But first we need to fix up this branch -- currently it shows 54 files changed.

garysm · 2022-08-31T00:10:30Z

Yes, I'll re-review this PR. But first we need to fix up this branch -- currently it shows 54 files changed.

I performed a rebase, that is probably why.

spydon · 2022-08-31T05:06:18Z

I performed a rebase, that is probably why.

GitHub should be able to understand rebases, but maybe not when the branch is outdated, can you press "Update branch" in the UI and see if one of those merges help?

garysm · 2022-08-31T18:50:33Z

@spydon that suggestion worked, thanks!

st-pasha

Some general comments:

Tests are currently missing. Generally, in order to test mouse movement events in a test, you would testWidgets(), and then tester.createGesture() + gesture.moveBy().
The interaction between this mixin and HasDraggableComponents is unclear: would they coexist peacefully, or will one preempt the other? This needs to be documented. Also, if both mixins are present and the user drags the mouse - would both mixins process this event?
We're slowly trying to move towards more uniform approach for event handling, see the lib/src/events/ folder. I'm not sure whether you'd want to modify your PR to use this newer approach, but that would save us some work in the future.

st-pasha · 2022-09-17T00:28:35Z

+      'game also mixed with HasCursorHandlerComponents. Do not mix with both, '
+      'HasCursorHandlerComponents removes the necessity of '
+      'MouseMovementDetector.',
+    );


This assert is probably unnecessary.
If the user did have MouseMovementDetector on their Game, and then added HasCursorHandlerComponent, then the assert wouldn't trigger since onMouseMove would be overridden anyway.

Did you see this comment @garysm?

garysm · 2022-09-20T12:59:10Z

Thanks for the feedback @st-pasha , I'll work on this.

Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>

luanpotter · 2023-01-28T15:49:07Z

+[`PointerHoverEvent`](https://api.flutter.dev/flutter/gestures/PointerHoverEvent-class.html)
+
+```{warning}
+This should not be used on platforms that do not support mouse cursors.


can we make this warning more clear as to what happens?
is the handler ignored?
or will it error?
that is relevant to multi-platform games

luanpotter · 2023-01-28T15:50:01Z

+    add(
+      CursorFollowerCircle(
+        radius: 20.0,
+      )..position = Vector2(100.0, 100.0),


Vector2.all

Suggested change

)..position = Vector2(100.0, 100.0),

)..position = Vector2.all(100),

luanpotter · 2023-01-28T15:51:28Z

+      _radius,
+      _paint,
+    );
+    canvas.drawLine(


(not part of this PR)
we should have a renderLine ext similar to renderPoint
https://github.com/flame-engine/flame/blob/main/packages/flame/lib/src/extensions/canvas.dart#L21

erickzanardo · 2023-02-06T12:58:25Z

+    add(
+      CursorFollowerCircle(
+        radius: 20.0,
+      )..position = Vector2(100.0, 100.0),


Suggested change

)..position = Vector2(100.0, 100.0),

)..position = Vector2.all(100),

spydon · 2023-04-16T14:18:07Z

@garysm are you interested in continuing working on this? :)

garysm · 2023-04-19T02:22:50Z

@garysm are you interested in continuing working on this? :)

Yes, has been on the backburner for me @spydon, but I need to finish this off. I need to take a look at the event handling as @st-pasha mentioned and catch up on Flame in general, to save you all some effort :)

garysm · 2023-04-28T16:14:12Z

@spydon taking a look at #2450, this makes a lot more sense and aligns to what I'd like to achieve. Since there is no existing cursor component mixin, there shouldn't be a need for a bridge either. I can re-work this PR for "new-style" components and event handling. Or I can close and create a new one for that, whatever works.

spydon · 2023-04-28T16:16:45Z

@spydon taking a look at #2450, this makes a lot more sense and aligns to what I'd like to achieve. Since there is no existing cursor component mixin, there shouldn't be a need for a bridge either. I can re-work this PR for "new-style" components and event handling. Or I can close and create a new one for that, whatever works.

Whatever is the easiest for you :)

…ndler-components

Add CursorHoverEvent, CursorHoverCallbacks, CursorHoverDispatcher

garysm · 2023-06-13T06:05:57Z

It seems to me that I cannot add a hover listener directly to the game from my new CursorHoverDispatcher as (to my knowledge) Flutter does not have a built-in hover GestureRecognizer, which is required to add to the GestureRecognizerBuilder. Should there be a new method created which specifically adds only mouse detectors, and isn't dependent on a mixin on Game?

garysm · 2023-06-17T00:10:59Z

@spydon any thoughts on this?

spydon · 2023-06-17T03:32:58Z

@garysm I missed that you had written! If it is possible (which it should be) it should be done in the same manner as the other ones, i.e. without a mixin in the game, there should be some hints on what to connect the game to in the old hover functionality.

garysm · 2023-06-24T22:14:30Z

@garysm I missed that you had written! If it is possible (which it should be) it should be done in the same manner as the other ones, i.e. without a mixin in the game, there should be some hints on what to connect the game to in the old hover functionality.

I don't think that the current GestureDetectorBuilder is suitable to handle a PointerHoverEvent, as (to my knowledge) the RawGestureDetector returned by the builder will not accept non-contact gestures. The solution I think would be to create a separate MouseDetectorBuilder, which would similarly rebuild the game widget when a new listener is added:

class MouseDetectorBuilder {
  MouseDetectorBuilder([this._onChange]);

  final Map _listeners = {};
  final void Function()? _onChange;

  Widget build(Widget child) {
    // Return mouse listeners
  }
}

My question then would be, what should the widget returned from this builder be? I am unsure, since the listeners cannot be passed into a RawGestureDetector, but should still be consolidated into a single widget.

What are your thoughts @spydon ?

spydon · 2023-07-17T14:11:45Z

My question then would be, what should the widget returned from this builder be? I am unsure, since the listeners cannot be passed into a RawGestureDetector, but should still be consolidated into a single widget.

What are your thoughts @spydon ?

Sorry for the late reply, it's been very busy with FlutterCon!
Couldn't this be transformed to the new system: https://github.com/flame-engine/flame/blob/main/packages/flame/lib/src/game/game_widget/gesture_detector_builder.dart#L180-L194 ?

garysm · 2023-07-18T04:30:01Z

My question then would be, what should the widget returned from this builder be? I am unsure, since the listeners cannot be passed into a RawGestureDetector, but should still be consolidated into a single widget.
What are your thoughts @spydon ?

Sorry for the late reply, it's been very busy with FlutterCon! Couldn't this be transformed to the new system: https://github.com/flame-engine/flame/blob/main/packages/flame/lib/src/game/game_widget/gesture_detector_builder.dart#L180-L194 ?

No problem! The issue would be that there is no way to dynamically add onMouseMove or similar callbacks like GestureDetectorBuilder, as applyMouseDetectors still relies on the old system to apply each callback when needed. That's why I think it might be appropriate to create a separate builder with those capabilities, but I'm not sure what that might look like.

spydon · 2023-07-18T10:12:10Z

No problem! The issue would be that there is no way to dynamically add onMouseMove or similar callbacks like GestureDetectorBuilder, as applyMouseDetectors still relies on the old system to apply each callback when needed. That's why I think it might be appropriate to create a separate builder with those capabilities, but I'm not sure what that might look like.

Ah right, now I understand the problem.
Yeah, seems like a separate builder would be needed then, but that builder would contain pretty much everything (and some more, like in the other builder) from the snippet I sent previously I think?

spydon · 2023-09-08T10:58:42Z

Superseded by #2706, thanks a lot for this PR though @garysm even though not merged it gave a lot of insights!

garysm added 2 commits July 25, 2022 15:01

feat: Add cursor components mixin to Game

01f3bf4

Merge remote-tracking branch 'origin' into cursor-handler-components

77ac85d

spydon reviewed Aug 8, 2022

View reviewed changes

Comment thread packages/flame/lib/src/gestures/detectors.dart Outdated

spydon requested a review from a team August 8, 2022 16:25

spydon added 2 commits August 12, 2022 22:06

Fix too long lines

ceb61bc

Merge branch 'main' into cursor-handler-components

847ba14

garysm and others added 8 commits August 20, 2022 09:44

Merge branch 'main' into cursor-handler-components

01d5b83

Add example for CursorHandler components

84dc8b3

feat: Add cursor components mixin to Game

c70680b

Fix too long lines

aab7e10

Add example for CursorHandler components

8a52989

Merge branch 'cursor-handler-components' of https://github.com/garysm…

69af8bf

…/flame into cursor-handler-components

rename example

316dd3f

Add mouse movement to docs

b229f0c

Merge branch 'main' into cursor-handler-components

fa4d26a

garysm requested a review from spydon September 12, 2022 17:37

spydon added 2 commits September 12, 2022 20:28

Merge branch 'main' into cursor-handler-components

446e80b

Merge branch 'main' into cursor-handler-components

dde9dbb

st-pasha reviewed Sep 17, 2022

View reviewed changes

garysm and others added 4 commits November 27, 2022 00:28

Update doc/flame/inputs/gesture-input.md

ce40df1

Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>

Merge branch 'main' into cursor-handler-components

cadf718

update doc for linting and admonition

4293baa

Add blank line in doc

0ebb400

luanpotter reviewed Jan 28, 2023

View reviewed changes

erickzanardo approved these changes Feb 6, 2023

View reviewed changes

spydon marked this pull request as draft April 16, 2023 14:22

Merge branch 'main' into cursor-handler-components

eb6a8bd

garysm changed the title ~~feat: Propagate cursor hover events to child Components~~ feat: Component Mixin to Receive Cursor Hover Events Apr 28, 2023

garysm and others added 5 commits June 5, 2023 21:35

Merge branch 'main' of https://github.com/garysm/flame into cursor-ha…

fdfbb76

…ndler-components

Merge branch 'main' of https://github.com/garysm/flame into cursor-ha…

27f536a

…ndler-components

Wip

0f0db95

Add CursorHoverEvent, CursorHoverCallbacks, CursorHoverDispatcher

fix export error

135fc39

Merge branch 'main' into cursor-handler-components

1f5d375

spydon mentioned this pull request Sep 4, 2023

feat: Add HoverCallbacks #2706

Merged

6 tasks

spydon closed this Sep 8, 2023

	)..position = Vector2(100.0, 100.0),
	)..position = Vector2.all(100),

Uh oh!

Conversation

garysm commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Breaking Change

Related Issues

Uh oh!

spydon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spydon commented Aug 12, 2022

Uh oh!

garysm commented Aug 13, 2022

Uh oh!

alestiago commented Aug 14, 2022

Uh oh!

garysm commented Aug 20, 2022

Uh oh!

garysm commented Aug 30, 2022

Uh oh!

spydon commented Aug 30, 2022

Uh oh!

st-pasha commented Aug 31, 2022

Uh oh!

garysm commented Aug 31, 2022

Uh oh!

spydon commented Aug 31, 2022

Uh oh!

garysm commented Aug 31, 2022

Uh oh!

st-pasha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

st-pasha Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

spydon Nov 25, 2022

Choose a reason for hiding this comment

Uh oh!

garysm commented Sep 20, 2022

Uh oh!

luanpotter Jan 28, 2023

Choose a reason for hiding this comment

Uh oh!

luanpotter Jan 28, 2023

Choose a reason for hiding this comment

Uh oh!

erickzanardo Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

luanpotter Jan 28, 2023

Choose a reason for hiding this comment

Uh oh!

erickzanardo Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

spydon commented Apr 16, 2023

Uh oh!

garysm commented Apr 19, 2023

Uh oh!

garysm commented Apr 28, 2023

Uh oh!

spydon commented Apr 28, 2023

Uh oh!

garysm commented Jun 13, 2023

Uh oh!

garysm commented Jun 17, 2023

Uh oh!

spydon commented Jun 17, 2023

Uh oh!

garysm commented Jun 24, 2023

Uh oh!

spydon commented Jul 17, 2023

Uh oh!

garysm commented Jul 18, 2023

garysm commented Aug 5, 2022 •

edited

Loading