SignIn callback returns before the user information are fetched

[movrajr]

Currently the code below results in a null avatar because User.AvatarURL has not yet been set through the User.BulkUpdate method.

        GameJolt.UI.Manager.Instance.ShowSignIn((bool success) =>
        {
            if (success)
            {
                GameJolt.API.Manager.Instance.CurrentUser.DownloadAvatar(GotAvatar);
            }
        }

The cause is likely in User.cs:124

    Core.Request.Get(Constants.API_USERS_AUTH, parameters, (Core.Response response) => {
        IsAuthenticated = response.success;

        if (response.success)
        {
            Manager.Instance.CurrentUser = this;
            Get();
        }

        if (callback != null)
        {
            callback(response.success);
        }

    }, false);

To fix it consider something like this

    Core.Request.Get(Constants.API_USERS_AUTH, parameters, (Core.Response response) => {
        IsAuthenticated = response.success;

        if (response.success)
        {
            Manager.Instance.CurrentUser = this;
            Get((user)=>{
                if (callback != null)
                {
                    callback(response.success);
                }
            });
        }
        else
        {
            if (callback != null)
            {
                callback(response.success);
            }
        }               

    }, false);

[loicteixeira]

It would delay the callback but I reckon it makes sense. Thanks for pointing that out.

I don't mind doing it but if you wish, you can submit a pull request for that.

[movrajr]

I forked it and committed the enhancement, but I messed up the formatting. I usually let CodeMaid clean up my code, but I had to disable it otherwise it would reformat the complete file and mark all lines as changed. But manually editing proves also to be error prone. So it's probably best if you do the change for consistent formatting.

[movrajr]

Also I'm not sure if there are any unwanted side-effects, no time to test all cases as I'm in currently in that game jam.

[loicteixeira]

No worries. Thanks for your help and good luck with the jam!

[loicteixeira]

Thinking of it, I wonder if I should delay the callback since some users won't care about the extra information or allow the SignIn method to accept two callbacks, one for login and one for the extra information. But then, I'm afraid of some users being confused about which to register to.

In addition, once downloaded images caching is in place (#8), I would like to download the user avatar automatically. Should the callback wait for the image to be done downloading as well? That's going to delay it even more...

[movrajr]

It's already confusing in its current state. When the SignedIn callback is called I expect all CurrentUser fields to be populated. What else can I do, poll CurrentUser.AvatarURL until it's non-null?

I think adding an optional second callback is a good, self-documenting compromise.

GameJolt.UI.Manager.Instance.ShowSignIn(SignedInCallback, ReceivedCurrentUserDataCallback)

An alternative would be an event.

GameJolt.UI.Manager.Instance.OnReceivedCurrentUserData += ReceivedCurrentUserData;

[OnlyFails]

Thank you for the post, it has been like 2 years later and I just ran into this problem. I downloaded the latest version and everything. I'm guessing this API is no longer supported or is not being worked on anymore?

[loicteixeira]

I haven't fixed it yet, mostly because I'm not sure I should, for the reasons explained above, and also because the same change should be applied to the User.SignIn too for it not to be too confusing.

Furthermore, the method never promised such thing. It is called SignIn (or ShowSignIn in the case of the UI manager), not SignInAndGetInfo.

It seems that the vast majority of developers only care about signing the user in and don't display any information about them so delaying the response doesn't seem like the right thing to do.

Lastly, we're talking about fetching the user info so the AvatarURL will be available for the Avatar to be fetched. Next request will be about waiting for the Avatar to be downloaded automatically, because why on earth would it be the only field not set?

[loicteixeira]

I actually realise now that it does get the user info after singing in. Maybe it shouldn't at all, or have a separate SignInAndFetch method..

As for your other question, yes, the API isn't really being worked on anymore. I do bug fixes because I don't want to leave people out, but I'm looking for someone to take over the project.

[loicteixeira]

I'm finally implementing this as it seems that a growing number of people get confused by it.

I'm going with 2 callbacks as going with a single callback can return bad results (e.g. in the fix proposed above, the sign-in can success and the fetch fail but it will return success because it use response.success from the first call).

Furthermore, I'm not renaming the call to SignInAndGetInfo or else.