AltimitSystems / mv-android-client

RPG Maker MV unofficial Android client
http://www.hbgames.org/forums/viewtopic.php?f=48&t=79391
Apache License 2.0
115 stars 36 forks source link

Google Play Games Functionality - Achievements #1

Closed JTM-rootstorm closed 6 years ago

JTM-rootstorm commented 6 years ago

Package-Private Additions via GooglePlayHandler

Modifications to WebPlayerActivity

The Important Additions for the End-User/Game Dev

The following functions are really only to be called via Script/Plugin from the running MV game and, as such, are JavascriptInterfaces

Possible TODOs

Getting Games Services and achievements working was first and foremost to this pull request, but the framework is kinda there for other things such as

Issues

Currently, com.android.support:appcompat-v7:27.0.2 has issues with a support library brought in by the play-services packages due to version mismatch. This does not seem to affect anything from my testing but I believe it's worth noting, just in case.

This is really my first pull request ever, so hopefully I've done things fairly decently. Changes can be made as needed.

felixjones commented 6 years ago

I'll do an in-depth review of these changes, but in the meantime there's one thing I'd like to see changed.

This line here: https://github.com/tehguy/mv-android-client/blob/43cb098b34098382b4bc81a5c6465843721fb1bd/app/src/main/java/systems/altimit/rpgmakermv/WebPlayerActivity.java#L134

I don't want end users to have any reason to navigate the Java code - unless they know what they're doing. I think either remove the silent option or make it a def switch in the app build.gradle file as such: https://github.com/tehguy/mv-android-client/blob/43cb098b34098382b4bc81a5c6465843721fb1bd/app/build.gradle#L22

The other thing I want to bring up is; I can't immediately see where initGooglePlayHandler is being used. Was this accidentally removed during a merge?

If this adds dependencies, then this entire feature should probably be toggled by a def switch in gradle. I can imagine that most people won't be using this particular feature, so having the dependencies pulled in for everyone is probably not the kindest thing to do.


This may rain on your parade a bit, but I've been working on the android-api branch for looking into adding Android specific functionality, including a safe (and guaranteed) way to add new Javascript interfaces to the MV Android Client. https://github.com/AltimitSystems/mv-android-client/tree/android-api

I'm thinking that this model may be suitable for adding your feature. We could make it a dedicated google-api feature that is optionally added to projects based on a def switch.

For now, I'll get to fixing the issues with the android-api so I can clean that up and have it committed to master so you can see how I've added this extension feature. I'll be changing how the Javascript interface for that is loaded, too - I'll make it so it's more suitable for your Google Play features.


I'd like to hear your thoughts on all this. I'll make an "API extension" issue dedicated to discussing this.