MrStahlfelge / gdx-gamesvcs

Easy integration of gameservices in your libGDX game: Google Play Games, Apple Game Center, Amazon GameCircle and more
Apache License 2.0
113 stars 20 forks source link

Feature/interface changes #48

Open karivatj opened 2 years ago

karivatj commented 2 years ago

Hello,

And sorry for the long post.

I've been working on the refactoring of the android-gpgs module and I needed to make some interface changes to the code. This PR contains these proposed changes to the IGameService interface and all necessary changes to sub-modules which implement the interface. I've tested these changes locally and seems to work fine and builds ok too.

Here are the key changes of the PR:

1) Add resultCode Integer into gsOnSessionActive and gsOnSessionInactive methods. I found it is important to know the reason code why these session changes occur. For example, considering the new way to handle logins in Google Play Services, one should consider a situation when the user asks to explicitly sign out from the service or cancel the sign in flow. When this happens, one shouldn't try to sign in after that until the user explicitly wants to sign in by pressing a sign in button in the application.

Thus, I figured I'd add a resultCode information in to the callback methods that deliver this information to the caller. This way the caller can persist the status and can make a informed decision whether to sign the user in during app startup or during resume. This is described in Google Play Games Checklist ID 1.6

2) Add get player data method to IGameService interface and implement method stubs to sub-modules

This is used to fetch player data from the Game Service if it supports it. This was a missing feature, and I thought I'd implement it in android-gpgs module. This is purely optional feature but nice to have.

Add IPlayerData and IPlayerDataResponseListener interfaces, which define the data model and the callback methods when new player data is received from the Game Service.

3) Add timespan and collection parameters to fetchLeaderboardEntries in IGameService

If the Game Service supports it, these parameters can control how to refine the leaderboard results.

This feature is at least supported by Google Play Games and should be added to make more refined queries to leaderboards. Now it defaults to All Time scores and completely disables the possibility to fetch Daily or Weekly leaderboards.

4) Add missing Javadoc stuff to get rid of warnings during compile time.

MrStahlfelge commented 2 years ago
  1. Add resultCode Integer into gsOnSessionActive and gsOnSessionInactive methods. I found it is important to know the reason code why these session changes occur. For example, considering the new way to handle logins in Google Play Services, one should consider a situation when the user asks to explicitly sign out from the service or cancel the sign in flow. When this happens, one shouldn't try to sign in after that until the user explicitly wants to sign in by pressing a sign in button in the application.

This was handled implicitely by GpgsClient before, the behaviour should be kept. It's the difference between calling resumeSession and logIn. If it didn't work this way on one if the implementations, that was a bug. I know for sure it works on Gpgs Android though. :)

karivatj commented 2 years ago

Thank you for the comments. I have made some fixes and will update the PR soon. I will look into the GPGS login stuff a bit more to figure out a way to handle explicit sign out situations within the client code. I had some problems dealing with that stuff when updating the code to use the newer API. For example the old connect() method does not exist anymore and is replaced by silentSignIn() which seems to sign in the user no matter what is the situation (even if the user explicitly signed out previously).

karivatj commented 2 years ago

I've made the changes that were requested. I renamed one enum type GsErrorType to GsResultCode and added some success enums there also that can be used as session inactive / active resultcodes.

I will take a look at the sign in stuff discussed earlier a bit more thoroughly when I prepare the PR for the android-gpgs refactor.

karivatj commented 2 years ago

I managed to figure out the sign in process. If the user explicitly signs out it is handled within the GPGS client like previously.

The reason I added the status code to session state changes was to inform the client about the explicit sign out event so the client could persist the choice and make a decision during app startup whether or not to sign in the user. This was wrong, so I made a fix to the gpgs client code which handles this scenario behind the scenes. The original use case is not valid anymore (for my case) and thus the resultCode in onSessionActive and onSessionInactive methods is not used (for the time being)

It is up to you if you see this as useful. Should the session state change include a reason code? If not, I can remove the commit from the Pull Request in order to get this approved.

Cheers, Kari

MrStahlfelge commented 2 years ago

I think we should remove it, I can't see any use case and it breaks backwards compatibility (every game has to change the callbacks).

Ping me when the PR is ready to review.

karivatj commented 2 years ago

Alrighty, I modified the PR by removing the commit containing the result code stuff. @MrStahlfelge