amazon-archives / aws-sdk-unity

ARCHIVED: The aws sdk for unity is now distributed as a part of aws sdk for dotnet:
https://github.com/aws/aws-sdk-net
Other
105 stars 43 forks source link

Synchronize Dataset on suspend #97

Closed seaders closed 6 years ago

seaders commented 8 years ago

I've already asked this a while ago in the /aws/amazon-cognito-unity repository (https://github.com/aws/amazon-cognito-unity/issues/4), but I want to ask here, is there any way to Synchronize Datasets on suspend?

When you suspend, on mobile, both on Android and iOS, you've a short time to do other things before your app enters the background. The caveat of this is, the tasks you need to do must occur on the main thread available to you. Issue here, with the way the AWS Unity SDK is built, it relies on Unity's WWW class which means, unless you block somewhere, you're going to yield that WWW, which will mean the actual call of the WWW won't happen until the app is resumed.

Obviously if a user opens our app up in another device, their data won't have synced by then. With how the SDK is built, even if we did block on the WWW call, there's so many different threaded things happening, it's quite hacky to try and force it to save at that point.

For CognitoSync, to offer this type of storage for mobile, we think it's quite strange that this (standard required) feature is missing, as you normally choose this solution to 1) minimise costs, only save when you need to, and 2) disrupt a user's experience least, save when the app is going into the background, and won't harm a user's experience if it sticks a little due to file, or network, I/O.

karthiksaligrama commented 8 years ago

As you have explained it yourself, the sdk uses WWW which only can make http calls on foreground. This is more of a Unity Limitation for WWW than SDK. Other Http clients either are paid(hence would be limited by license ) or do not implement full feature set (like Correct SSL implementation). Unity does seem to working on experimental new API called UnityWebRequest. But this is still in experimental stage and wont be available on Android/iOS/Standalone till dec 8 as per their roadmap.

The other option we debated was to have a feature which allows customizing the http client easily via some configuration, we might do this at some point, so you can have you own http client without any of these limitations ( You can still do it with some minor modifications to the code).

Regarding point no. 2 ) have you observed any issues with the performance of your game with the current implementation? cause the synchronize is an async operation and ot should run in the threadpool thread and not disrupt game thread.

seaders commented 8 years ago

The final WWW call is actually easy enough to get in, you just have some flag somewhere that sets whether it should yield the return, or block, it's the rest of the SDK method that I found the most difficult. I brought a lot of the stack from the .NET libraries which offer the synchronous versions of the SDK into 2.0.0.0 but just upgraded now to 2.0.0.5 and so much has changed that it's like starting from scratch bringing it in again :/

As for the other point, yeah we have noticed some drops, but mainly on poor performing Android devices. On those guys, we're struggling to maintain a good-enough FPS so every bit of things you do outside of the game can and most likely will affect how the game operates.

My point is more about offering a system like this, that allows remote syncing. Sure, you want to allow devs to save at particular points (we do), but a very standard feature is syncing when the app is put into suspend. If you put your phone through a network monitoring Proxy (Charles on Mac is our choice), you'll see that happen so much when you suspend apps.

karthiksaligrama commented 8 years ago

The reason why we don't allow synchronous versions is that what we have observed sometimes it tends to block game thread and that cannot be good for the game. We have some Internal Synchronous API (Cognito Identity, STS) but that's because writing high level logic over an Asynchronous API can get complicated with too many callbacks. So unless user is explicitly making the synchronous call within a new thread it is bound to cause problems and that might not be evident directly.

I do understand your point about trying to reduce the number of synchronize calls. If you have suggestions on how we can use WWW on background. Do let us know or send a pull request and i'll consider adding that as a standard feature in the sdk.

seaders commented 8 years ago

Honestly, the WWW part really is the easiest part of it all. When suspending the app, the only method you have to do things on are on pre-existing threads, until the end of their natural, or forced execution time (this is OS dependant), and it's perfectly acceptable in this situation, to block the main thread.

From the work that I had done previously, the crux of it (for the WWW request) was in UnityWebRequestFactory.cs

    ...

    public IWebResponseData GetResponse()
    {
    if (UnityInitializer.IsMainThread())
        {
            if(UnityInitializer._forceOnThread)
            {
                InvokeRequest();

                if (this.Exception != null)
                    throw this.Exception;

                return this.Response;
            }
            throw new Exception("Network on game thread");
        }
            ....

    void InvokeRequest()
    {
        // Fire the request            
        this.WwwRequest = new WWW(this.RequestUri.AbsoluteUri,
                                  this.RequestContent, this.Headers);

        while(!this.WwwRequest.isDone)
        {
            Thread.Sleep(100);
        }
        // Debug.Log(this.WwwRequest.text);

        this.Response = new UnityWebResponseData(this.WwwRequest);
    }
    ...

Now, with how the WWW class is handled by Unity, this doesn't work on iOS, but they have plans to sort this out. But yeah, the main areas of difficulty I had was even getting to this situation on suspend.

seaders commented 8 years ago
  The reason why we don't allow synchronous versions is that what we have observed sometimes it tends to block game thread and that cannot be good for the game.

And sorry, just on this, I understand the reasons behind, but I'd always prefer to be able to make that decision ourselves, like we can do with every other aspect that may affect the game. If only for the debugging aspect. For when we're trying to deal with a bug, or a complicated, edge situation, with the current heavily-threaded setup, it's near impossible to debug through it without Unity crashing consistently (as I've highlighted to both yourself, and the Unity devs before already).

karthiksaligrama commented 8 years ago

i don't really mind opening up the synchronous API at this point but i still do think that there are people who would come back with issues related to usage of synchronous API.

As per the WWW code that you have attached.i'll take a look at it and get back to you

seaders commented 8 years ago

So long as people have an option, you'll just provide a better SDK that people can use whichever way fits their purposes, rather than limiting folks like us that needs a specific usage due to others misusing / misunderstanding how to use the SDK correctly.

hyandell commented 6 years ago

Closing as this repository is being archived.