-
Notifications
You must be signed in to change notification settings - Fork 279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Async Binding Support #155
Async Binding Support #155
Conversation
An extra information for Addressables. I have tested addressable loading in an additional test. This is included as a zip folder inside Async tests. I didn't distribute it outside zip folder because that would require projects to import addresables package. It contains following test file that shows how to load addressables with this feature. using System;
using Zenject;
using System.Collections;
using NUnit.Framework;
using UnityEngine;
using UnityEngine.AddressableAssets;
using UnityEngine.ResourceManagement.AsyncOperations;
using UnityEngine.ResourceManagement.ResourceLocations;
using UnityEngine.TestTools;
public class TestAddressable : ZenjectIntegrationTestFixture
{
[UnityTest]
public IEnumerator TestAddressableAsyncLoad()
{
PreInstall();
AsyncOperationHandle<GameObject> handle = default;
Container.BindAsync<GameObject>().FromMethod(async () =>
{
try
{
var locationsHandle = Addressables.LoadResourceLocationsAsync("TestAddressablePrefab");
await locationsHandle.Task;
Assert.Greater(locationsHandle.Result.Count, 0, "Key required for test is not configured. Check Readme.txt in addressable test folder");
IResourceLocation location = locationsHandle.Result[0];
handle = Addressables.LoadAsset<GameObject>(location);
await handle.Task;
return handle.Result;
}
catch (InvalidKeyException)
{
}
return null;
}).AsCached();
PostInstall();
yield return null;
AsyncInject<GameObject> asycFoo = Container.Resolve<AsyncInject<GameObject>>();
int frameCounter = 0;
while (!asycFoo.HasResult && !asycFoo.IsFaulted)
{
frameCounter++;
if (frameCounter > 10000)
{
Addressables.Release(handle);
Assert.Fail();
}
yield return null;
}
Addressables.Release(handle);
Assert.Pass();
}
} |
Added awaiter support as suggested in gitter group and test related to it. And simple documentation page. However did not update main RaidMe page. |
@Mathijs-Bakker Added typeless base AsyncInject so that injections can be grouped in list bindings CI tests failed, but I am suspecting it is unrelated. My intention is to make final addition with Addressable flavored Thanks. |
Used version defines to add EXTENJECT_INCLUDE_ADDRESSABLE_BINDINGS define when Addressables package is added to the project
@Mathijs-Bakker Feature is ready for review. Final Addressable addition is not added to the documentation. I can finalize that after the review. |
Based on comment in gitter, I updated addressable method to manually throw error when addressable fails to load. By default addressable return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've looked at the code and mentioned everything I noticed. This isn't a lot, since I'm currently not very deep into the internal implementation of Extenject.
Besides that, I made some suggestions for the documentation in order to improve the grammar a bit. Also, I'd use the async
keyword as a keyword, and thus with the Markdown formatting for inline code (example: async
).
All of the suggestions are up for debate.
UnityProject/Assets/Plugins/Zenject/OptionalExtras/Async/Runtime/AsyncInject.cs
Outdated
Show resolved
Hide resolved
Documentation/Async.md
Outdated
|
||
In dependency injection, the injector resolves dependencies of the target class only once, often after class is first created. In other words, injection is a one time process that does not track the injected dependencies to update them later on. If a dependency is not ready at the moment of injection; either binding would not resolve in case of optional bindings or fail completely throwing an error. | ||
|
||
This is creates a dilemma when implementing dependencies that are resolved asyncroniously. You can design around the DI limitations by carefully architecting your code so that the injection happens after async process is completed. This requires careful planning, increased complexity in setup. It is also prone to errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is creates a dilemma when implementing dependencies that are resolved asyncroniously. You can design around the DI limitations by carefully architecting your code so that the injection happens after async process is completed. This requires careful planning, increased complexity in setup. It is also prone to errors. | |
This creates a dilemma while implementing dependencies that are resolved asynchronous. You can design around the DI limitations by carefully designing your code so that the injection happens after the `async` process is completed. This requires careful planning, which leads to an increased complexity in the setup, and is also prone to errors. |
@RichardWepner thanks for the review. I applied all suggestions. Did not apply documentation changes directly here to have single commit. So I did those locally too. |
🙇 |
Did this ever make it into the Unity asset store package? I'm on the latest version (9.2.0) and there's no |
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
Issue Number
Issue Number: #153
Create or search an issue here: Extenject/Issues
What is the current behavior?
LazyInject<T>
. An intermediary object is injected instead of typeT
, so in implementation binding can be resolved later when the value is actually needed.What is the new behavior?
AsyncInject<T>
similar toLazyInject<T>
. However unlikeLazyInject<T>
this is not automatic. The dependency needs to be bound explicitly through a specific binder.Container.BindAsync<T>().FromMethod( async () => {...})
binding format for binding async dependencieshandle
which is used in Addressable system to release a loaded resource from memory after use.Does this introduce a breaking change?
Other information
Container.Bind<T>().FromAsyncMethod( async ()=> {...})
. While it is technically possible to do it this way, I felt that it is better to be explicit. Also since feature is not supporting factories or memory pools yet, it may have cause confusion or bugs. So I kept it separate.On which Unity version has this been tested?
Scripting backend: