-
Notifications
You must be signed in to change notification settings - Fork 424
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
Android input handlers refactor #5317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine otherwise.
@@ -30,6 +31,8 @@ public abstract class AndroidInputHandler : InputHandler | |||
/// </summary> | |||
protected readonly AndroidGameView View; | |||
|
|||
public override string Description => base.Description.Replace("Android", null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads a bit weird. Is this to make things display correctly in settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Settings use custom LocalizabeStrings instead of this.
This is used when logging unhandled events and takes inspiration from InputHandler.Description
:
Logger.Log($"Unknown {Description} {methodName} event: {inputEvent}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just leave the Android
in the description? If it's not displayed anywhere else, I think I'd prefer it being there in logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, the Description
can stay as-is, but logs will use GetType().ReadableName()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a bit confused here. Can we revert the Replace
part?
Adds complexity and is not really used anywhere, as some of the handlers override it.
Changes include:
FrameStatistics
when enqueuing inputTouchSource
validation for touch inputHistoric handling of touch input
Explainer: https://developer.android.com/reference/android/view/MotionEvent#batching
This is especially important if the game/device is lagging. Previously we were dropping a lot of events. I expect the UI to feel marginally smoother with historic handling, and scrolling/dragging to be more precise.