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

feat: Add custom long tap delay #3101

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@ mixin TapCallbacks on Component {
void onTapUp(TapUpEvent event) {}
void onTapCancel(TapCancelEvent event) {}

/// The delay (in milliseconds) after which a tap is considered a long tap.
int get longTapDelay => 300;
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit strange that it is coming from this mixin, since it'll only have effect the first time a component is added. I think for this case it might be worth adding a static variable to the dispatcher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean a mutable static? I don't like the ideia. Is there any alternative?

Copy link
Member

Choose a reason for hiding this comment

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

There are few times where static variables are useful, but since passing this value in from the mixin just makes it more confusing for the user since they'll believe that they can set it per tappable component, I think it is a better option.
However, one downside of it is that it does affect the possibility of having multiple different games in the same app with different long tap delays (a very rare use-case). I don't have any other ideas for how to solve this, if you do, feel free to suggest them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spydon Added an alternative implementation here: #3110


@override
@mustCallSuper
void onMount() {
super.onMount();
final game = findRootGame()!;
if (game.findByKey(const MultiTapDispatcherKey()) == null) {
final dispatcher = MultiTapDispatcher();
final dispatcher = MultiTapDispatcher(
longTapDelay: longTapDelay,
);
game.registerKey(const MultiTapDispatcherKey(), dispatcher);
game.add(dispatcher);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,16 @@ class MultiTapDispatcherKey implements ComponentKey {

@internal
class MultiTapDispatcher extends Component implements MultiTapListener {
MultiTapDispatcher({
int longTapDelay = 300,
}) : _longTapDelay = longTapDelay;

/// The record of all components currently being touched.
final Set<TaggedComponent<TapCallbacks>> _record = {};

/// The delay (in milliseconds) after which a tap is considered a long tap.
final int _longTapDelay;

FlameGame get game => parent! as FlameGame;

/// Called when the user touches the device screen within the game canvas,
Expand All @@ -50,7 +57,7 @@ class MultiTapDispatcher extends Component implements MultiTapListener {
}

/// Called after the user has been touching the screen for [longTapDelay]
/// seconds without the tap being cancelled.
/// milliseconds without the tap being cancelled.
///
/// This event will be delivered to all the components that previously
/// received the `onTapDown` event, and who remain at the point where the user
Expand Down Expand Up @@ -118,9 +125,9 @@ class MultiTapDispatcher extends Component implements MultiTapListener {

//#region MultiTapListener API

/// The delay (in seconds) after which a tap is considered a long tap.
/// The delay (in milliseconds) after which a tap is considered a long tap.
@override
double get longTapDelay => 0.300;
int get longTapDelay => _longTapDelay;
Copy link
Member

Choose a reason for hiding this comment

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

In Flame we use seconds everywhere, so this should not be changed to milliseconds


@override
void handleTap(int pointerId) {}
Expand Down Expand Up @@ -157,7 +164,7 @@ class MultiTapDispatcher extends Component implements MultiTapListener {
MultiTapGestureRecognizer.new,
(MultiTapGestureRecognizer instance) {
instance.longTapDelay = Duration(
milliseconds: (longTapDelay * 1000).toInt(),
milliseconds: longTapDelay,
);
instance.onTap = handleTap;
instance.onTapDown = handleTapDown;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ mixin MultiTouchTapDetector on Game implements MultiTapListener {

//#region MultiTapListener API
@override
double get longTapDelay => 0.300;
int get longTapDelay => 300;

@override
void handleTap(int pointerId) => onTap(pointerId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import 'package:flutter/gestures.dart';
/// - [MultiTouchTapDetector] for a custom `Game`
abstract class MultiTapListener {
/// The amount of time before the "long tap down" event is triggered.
double get longTapDelay;
int get longTapDelay;

/// A tap has occurred.
void handleTap(int pointerId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class GestureDetectorBuilder {
(MultiTapGestureRecognizer instance) {
final g = game as MultiTapListener;
instance.longTapDelay = Duration(
milliseconds: (g.longTapDelay * 1000).toInt(),
milliseconds: g.longTapDelay,
);
instance.onTap = g.handleTap;
instance.onTapDown = g.handleTapDown;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ void main() {
},
);

testWidgets(
'TapCallbacks default long tap delay is 300ms',
(tester) async {
expect(_TapCallbacksComponent().longTapDelay, 300);
},
);

testWidgets(
'Custom TapCallbacks can override default long tap delay',
(tester) async {
expect(_CustomDelayTapCallbacksComponent().longTapDelay, 500);
},
);

testWidgets(
'tap outside of component is not registered as handled',
(tester) async {
Expand Down Expand Up @@ -511,4 +525,10 @@ mixin _TapCounter on TapCallbacks {
class _TapCallbacksComponent extends PositionComponent
with TapCallbacks, _TapCounter {}

class _CustomDelayTapCallbacksComponent extends PositionComponent
with TapCallbacks, _TapCounter {
@override
int get longTapDelay => 500;
}

class _TapCallbacksGame extends FlameGame with TapCallbacks, _TapCounter {}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,23 @@ void main() {
expect(game.nOnTapCancel, 2);
},
);

test(
'default longTapDelay is 300ms',
() {
final detector = _GameWithMultiTouchTapDetector();
expect(detector.longTapDelay, 300);
},
);

test(
'longTapDelay can be customized',
() {
final detector = _GameWithMultiTouchTapDetector();
detector.longTapDelayValue = 500;
expect(detector.longTapDelay, 500);
},
);
});
}

Expand All @@ -97,6 +114,11 @@ class _GameWithMultiTouchTapDetector extends Game with MultiTouchTapDetector {
int nOnTap = 0;
int nOnTapCancel = 0;

int? longTapDelayValue;

@override
int get longTapDelay => longTapDelayValue ?? super.longTapDelay;

@override
void render(Canvas canvas) => rendered = true;

Expand Down
Loading