gdx-gamesvcs icon indicating copy to clipboard operation
gdx-gamesvcs copied to clipboard

Feature/interface changes

Open karivatj opened this issue 3 years ago • 6 comments

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

  1. 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.

  1. 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.

  • Timespan declares if for example daily, weekly or all time results are fetched.
  • Collection defines if the results retrieved are in public or social context.

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.

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

karivatj avatar Dec 05 '21 19:12 karivatj

  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. :)

MrStahlfelge avatar Dec 06 '21 18:12 MrStahlfelge

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 avatar Dec 07 '21 20:12 karivatj

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 avatar Dec 10 '21 11:12 karivatj

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

karivatj avatar Dec 15 '21 13:12 karivatj

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.

MrStahlfelge avatar Dec 15 '21 14:12 MrStahlfelge

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

karivatj avatar Dec 16 '21 07:12 karivatj