hadashiA / VContainer

The extra fast, minimum code size, GC-free DI (Dependency Injection) library running on Unity Game Engine.
https://vcontainer.hadashikick.jp
MIT License
1.89k stars 165 forks source link

Add Awaitable Support for IStartable #584

Closed lolhans closed 9 months ago

lolhans commented 9 months ago

Since Unity 2023 will support Awaitables (a UniTask-Like behaviour for asynchronous programming in Unity: https://docs.unity3d.com/2023.1/Documentation/ScriptReference/Awaitable.html), I think it would be great to have this for IStartable as well.

Like the UniTask Integration already allows IAsyncStartable (https://vcontainer.hadashikick.jp/integrations/unitask)

UniTask

public class FooController : IAsyncStartable
{
    public async UniTask StartAsync(CancellationToken cancellation)
    {
        await LoadSomethingAsync(cancellation);
        await ...
        ...
    }
}

Unity Awaitable

public class FooController : IAsyncStartable
{
    public async Awaitable StartAsync(CancellationToken cancellation)
    {
        await LoadSomethingAsync(cancellation);
        await ...
        ...
    }
}
slimshader commented 9 months ago

Question: What will that enable? Are StartAsync calls even awaited by VContainer runtime? Awaiting is enabled by "async" keayword so only advantage of StartAsync over Start when doing async operations is CancelationToken, no? My point being: AsyncStart() should just return "void" (or preferably UniTaskVoid). Or am I missing something?

hadashiA commented 9 months ago

I would consider IAsyncStartable with Awaitable. Currentrly, Awaitable is a bit poor, so I will continue to give priority to UniTask in environments where UniTask is available.

My point being: AsyncStart() should just return "void" (or preferably UniTaskVoid). Or am I missing something?

I generally consider async void to be deprecated, even outside of Unity.

The main reason is that it doesn't compensate for errors. If an exception is thrown in an async void, internally the exception is held by AsyncVoidMethodBuilder.SetException. However, there is no way to catch it. Because the return value is void, there is no way to await it or to chain it with ContinueWith or other methods.

Also, the AsyncVoidMethodBuilder has some overhead in the way it does things, which may be unnecessary on Unity. UniTask has created its own MethodBuilder to address this.

lolhans commented 9 months ago

Currentrly, Awaitable is a bit poor, so I will continue to give priority to UniTask in environments where UniTask is available.

You are right about that, maybe it will be more feature rich with Unity 6 and a community around it. I think the advantage of Awaitable right now is about the catching time of the exception, because it is integrated in the default unity loops and not in additional loops like unitask does it.

The main reason is that it doesn't compensate for errors.

That is also the main reason for me. With async void you would need to write a wrapper Task around it, that includes the actual logic to be able to catch the errors accordingly.

I consider this issue as closed.