heroiclabs / nakama-unity

Unity client for Nakama server.
https://heroiclabs.com/docs/unity-client-guide
Other
411 stars 75 forks source link

Synchronous API #87

Closed xvnnnk closed 4 years ago

xvnnnk commented 4 years ago

Please provide synchronous version of the methods in the library. It's hard to use async/await methods in new Unity ECS.

novabyte commented 4 years ago

@xvnnnk Interesting. Do you have a code example on what makes using Task objects harder with Unity's new ECS?

You can execute a task synchronously with its .Wait() method:

https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.wait?view=netframework-4.6

For example you could fetch a player account synchronously.

var interval = TimeSpan.FromSeconds(5);
var account = client.GetAccountAsync(session).Wait(interval);
var user = account.User;
Debug.LogFormat("User id '{0}' username '{1}'", user.Id, user.Username);
Debug.LogFormat("User wallet: '{0}'", account.Wallet);

What sort of synchronous API would you want that's different to the one above?

xvnnnk commented 4 years ago

Never mind. I think I have to find the solution in ECS part not in the Nakama part. Because calling synchronous web requests can become quite a problem for itself.

But anyway, Just to add more information, as far as I know, blocking Tasks with Wait or Result can easily create deadlocks. You can find more information about that (and some examples) on Stackoverflow. The solution for deadlocks is to await all the way through.

Specifically with Unity ECS, what I have encountered are two issues.

First, The main purpose of ECS is performance. Unity ECS has its own task library (Job System). Using two task libraries at the same time is not a good idea for performance.

Second, which is a bit annoying, is that in ECS you should not use any callbacks. Data should drive your code. Now imagine you want to read data from database as soon as connection has been established. You have a code similar to this:

protected override async void OnUpdate()
{
    if( IsConnectedToNakama() && PlayerData == null)
    {
          await ReadPlayerDataFromDatabase();
     }
}

OnUpdate will be called each frame. PlayerData will not be set until the end of method. If ReadPlayerDataFromDatabase() takes longer than a frame, you end up calling ReadPlayerDataFromDatabase() twice. The solution is to set a flag that shows we are already calling the API. And that's the problem. I have to create a flag for every request.

This was a FYI. I have to find the solution for this problem in the ECS land. I'm closing this issue.

novabyte commented 4 years ago

@xvnnnk I'm confused by your response.

But anyway, Just to add more information, as far as I know, blocking Tasks with Wait or Result can easily create deadlocks. You can find more information about that (and some examples) on Stackoverflow.

We've built an asynchronous API optimized for network operations. You asked about a synchronous API so I gave you the option and now you warn me against synchronous code. I'm aware that it can cause deadlocks which is why we did not build the client sdk in that way.

An alternative approach is to use the callback style with Tasks which we already show an example for at the bottom of the README:

https://github.com/heroiclabs/nakama-unity/blob/master/README.md

Using two task libraries at the same time is not a good idea for performance.

Do you have any statistical data to back up this concern about performance or is it anecdotal?

Second, which is a bit annoying, is that in ECS you should not use any callbacks. Data should drive your code. Now imagine you want to read data from database as soon as connection has been established.

I don't think this has anything to do with networked code, sockets, or request/response. The problem appears to be what to do to handle the asynchronous execution of some logic inside the new ECS system.

Please let me know if you find a recommended best practice on how to do that.