-
-
Notifications
You must be signed in to change notification settings - Fork 12
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: Support for Android #35
feat: Support for Android #35
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.
Lgtm, good job!
Just some very minor nits.
Co-authored-by: Lukas Klingsbo <[email protected]>
Co-authored-by: Lukas Klingsbo <[email protected]>
...ads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/GamepadsAndroidPlugin.kt
Outdated
Show resolved
Hide resolved
...ads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/GamepadsAndroidPlugin.kt
Outdated
Show resolved
Hide resolved
...ads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/GamepadsAndroidPlugin.kt
Outdated
Show resolved
Hide resolved
...ads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/GamepadsAndroidPlugin.kt
Outdated
Show resolved
Hide resolved
...es/gamepads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/EventListener.kt
Outdated
Show resolved
Hide resolved
...es/gamepads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/EventListener.kt
Outdated
Show resolved
Hide resolved
...es/gamepads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/EventListener.kt
Outdated
Show resolved
Hide resolved
...es/gamepads_android/android/src/main/kotlin/dev/markvideon/gamepads_android/EventListener.kt
Outdated
Show resolved
Hide resolved
README.md
Outdated
override fun isGamepadsInputDevice(device: InputDevice): Boolean { | ||
return device.sources and InputDevice.SOURCE_GAMEPAD == InputDevice.SOURCE_GAMEPAD | ||
|| device.sources and InputDevice.SOURCE_JOYSTICK == InputDevice.SOURCE_JOYSTICK | ||
} |
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.
why would users need to override this method from GamepadsCompatibleActivity?
can't we just provide the impl for this?
I would prefer to minimize any code that users have to add manually to use the libary
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.
on a similar line of thought, I wonder if we can provide a GamepadsCompatibleActivityAdapter, that implements all these required methods on behalf of the user just requiring the minimum hooks to be added:
- wire in registerKeyEventHandler and registerMotionEventHandler
- access to getSystemService(INPUT_SERVICE)
it appears that everything else is boilerplate that would be the same for all (or most) users using these 3 variables and thus could be hidden away by us
but I might be missing something, I am only looking at the code on github. I can try to pull on IJ later to take a look
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.
Overall looks good to me. I just wonde why make this a standalone plugin instead of adding it under the gamepads one that already exists, in a federated structure
It's being integrated, I'll just pass it on |
Didn't look much into the code, but testing on a Samsung s21 ultra and a Sony xperia 1 V, both with android 14,it seems like events are not being registered and list gamepads never resolves? Am i missing anything when setting it up? |
Did you perform the steps in the updated README? |
…on/gamepads_android/EventListener.kt Co-authored-by: Luan Nico <[email protected]>
…on/gamepads_android/EventListener.kt Co-authored-by: Luan Nico <[email protected]>
…on/gamepads_android/GamepadsAndroidPlugin.kt Co-authored-by: Luan Nico <[email protected]>
…ios to match `gamepads`.
…nst, adjustment to lambda definition for readability as per PR feedback
… not handle events (and should bubble them up) when the listeners have not been setup.
…. Removed from integration example in README.
Now that the README on the feature branch has been altered, you can either point to the head of this feature branch and use those README instructions, or you can follow these: Sample integration for gamepads_android 0.1.1
|
Addressing #17.
Note that the example project as-is will fail due to the published release of gamepads 0.1.1 not depending on gamepads_android. Manually adding gamepads_android: ^0.1.1 as a dependency to the example project will allow you to test everything links as expected.
Edit: Also note that there are required integration steps for consuming apps on Android. Essentially this implementation relies on MainActivity forwarding [KeyEvent]s and [MotionEvent]s to the plug-in class.