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

Conversation

lucasnlm
Copy link
Contributor

Description

Currently longTapDelay is hardcoded to 300ms.
This is good for most of the games. But same games required custom long press values.
This PR allows it.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • 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 or docs.

Breaking Change?

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

Related Issues

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

It's a good idea, we just need to discuss the implementation details

@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

@@ -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

@lucasnlm
Copy link
Contributor Author

lucasnlm commented Apr 1, 2024

Canceled in favor of #3110

@lucasnlm lucasnlm closed this Apr 1, 2024
@lucasnlm lucasnlm deleted the custom_long_tap_delay branch April 1, 2024 14:39
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.

2 participants