Feature/interface changes #48

karivatj · 2021-12-05T19:53:26Z

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:

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

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.

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.

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

android-amazongc/src/de/golfgl/gdxgamesvcs/GameCircleClient.java

core/src/de/golfgl/gdxgamesvcs/IGameServiceClient.java

core/src/de/golfgl/gdxgamesvcs/IGameServiceListener.java

core/src/de/golfgl/gdxgamesvcs/player/IPlayerData.java

android-gpgs/src/de/golfgl/gdxgamesvcs/GpgsPlayerData.java

core/src/de/golfgl/gdxgamesvcs/IGameServiceClient.java

MrStahlfelge · 2021-12-06T18:13:45Z

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 · 2021-12-07T20:52:09Z

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

core/src/de/golfgl/gdxgamesvcs/IGameServiceClient.java

karivatj · 2021-12-10T11:13:09Z

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 · 2021-12-10T11:15:09Z

html-gpgs/src/de/golfgl/gdxgamesvcs/GpgsClient.java

@@ -267,17 +267,15 @@ private native void refreshDisplayname() /*-{

    protected void onDisplayName(String displayName) {
        this.displayName = displayName;
-        if (gsListener != null)


Why is this OnSessionActive callback here? Seems weird when we receive a new displayname we trigger a session active event? Is this something that is needed in HTML GPGS stuff? I decided to remove this because I thought it is illogical for this to be here. Let me know if this needs to be reverted.

Before your change, player display name was guarantueed to be available when game service was connected. Thus, the connection state was set to connected after the display name was received.

Okay, I will revert that change. Thanks for the clarification.

karivatj · 2021-12-10T11:17:06Z

core/src/de/golfgl/gdxgamesvcs/IGameServiceListener.java


-    public enum GsErrorType {errorLoginFailed, errorUnknown, errorServiceUnreachable, errorLogoutFailed}


Rename GsErrorType to GsResultCode. Makes it usable in success cases also. Relevant to for example on session change events.

What different success types can we have?

Instead of defining only some generic error types for example errorLoginFailed or errorLogoutFailed which we use to inform the client when an error occurs, we could define some other status codes as well like signedIn, signedOut, connectionPaused, connectionResumed etc and rename the enum type to GsResultCode. We could then use it with OnSessionAcrive and OnSessionInactive methods to inform the client of the reason the session state changed.

Does this idea sound reasonable or is this completely useless thing to consider? What do you think?

In my game project I needed this information to know if the user has cancelled the sign in process or signed out explicitly. With no result code present, there's no way to know why the session state change happened. And I believe there is other use cases for this kind of information as well. I don't see a reason there shouldn't be a specific reason code included in the callback when a session state change occurs.

Not useless, but at the state GPGS was there was no reliable information what exactly happened, might have changed now.

karivatj · 2021-12-15T13:45:14Z

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 · 2021-12-15T14:03:25Z

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.

If supported by the Game Service, this method is used to invoke a call to fetch player related data. Implement method stubs to submodules which just returns false when called. Add IPlayerData interface which describes the PlayerData data model. Add IPlayerDataResponseListener interface which declares a callback method when player data is received from the Game Service.

Modification includes two extra int parameters timespan and collection If supported, these integers 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.

karivatj · 2021-12-16T07:33:54Z

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

MrStahlfelge reviewed Dec 6, 2021

View reviewed changes

karivatj force-pushed the feature/interface-changes branch from 2fe8ef3 to 2dc0284 Compare December 10, 2021 11:04

karivatj commented Dec 10, 2021

View reviewed changes

core/src/de/golfgl/gdxgamesvcs/IGameServiceClient.java Show resolved Hide resolved

karivatj commented Dec 10, 2021

View reviewed changes

karivatj force-pushed the feature/interface-changes branch from 2dc0284 to cd18623 Compare December 10, 2021 12:18

karivatj force-pushed the feature/interface-changes branch from cd18623 to 4c77c80 Compare December 16, 2021 07:20

karivatj added 2 commits December 16, 2021 09:27

Add missing Javadoc statements

f20b2f6

karivatj force-pushed the feature/interface-changes branch from 4c77c80 to f20b2f6 Compare December 16, 2021 07:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/interface changes #48

Feature/interface changes #48

karivatj commented Dec 5, 2021 •

edited

Loading

MrStahlfelge commented Dec 6, 2021

karivatj commented Dec 7, 2021

karivatj commented Dec 10, 2021

karivatj Dec 10, 2021 •

edited

Loading

MrStahlfelge Dec 10, 2021

karivatj Dec 11, 2021

karivatj Dec 10, 2021

MrStahlfelge Dec 10, 2021

karivatj Dec 11, 2021 •

edited

Loading

MrStahlfelge Dec 11, 2021

karivatj commented Dec 15, 2021 •

edited

Loading

MrStahlfelge commented Dec 15, 2021

karivatj commented Dec 16, 2021 •

edited

Loading


		public enum GsErrorType {errorLoginFailed, errorUnknown, errorServiceUnreachable, errorLogoutFailed}

Feature/interface changes #48

Are you sure you want to change the base?

Feature/interface changes #48

Conversation

karivatj commented Dec 5, 2021 • edited Loading

MrStahlfelge commented Dec 6, 2021

karivatj commented Dec 7, 2021

karivatj commented Dec 10, 2021

karivatj Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

MrStahlfelge Dec 10, 2021

Choose a reason for hiding this comment

karivatj Dec 11, 2021

Choose a reason for hiding this comment

karivatj Dec 10, 2021

Choose a reason for hiding this comment

MrStahlfelge Dec 10, 2021

Choose a reason for hiding this comment

karivatj Dec 11, 2021 • edited Loading

Choose a reason for hiding this comment

MrStahlfelge Dec 11, 2021

Choose a reason for hiding this comment

karivatj commented Dec 15, 2021 • edited Loading

MrStahlfelge commented Dec 15, 2021

karivatj commented Dec 16, 2021 • edited Loading

karivatj commented Dec 5, 2021 •

edited

Loading

karivatj Dec 10, 2021 •

edited

Loading

karivatj Dec 11, 2021 •

edited

Loading

karivatj commented Dec 15, 2021 •

edited

Loading

karivatj commented Dec 16, 2021 •

edited

Loading