From b29d5d16d8683b08521e0836f4ce3139c7382885 Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 14:58:40 +0900 Subject: [PATCH 1/6] Make sure that RegisterInstance is not managed by the container --- .../Assets/VContainer/Runtime/Container.cs | 4 +- .../Runtime/Internal/InstanceRegistration.cs | 21 +------- .../Internal/InstanceRegistrationBuilder.cs | 8 +-- .../Unity/ComponentRegistrationBuilder.cs | 49 ++++++++++--------- .../Unity/ContainerBuilderUnityExtensions.cs | 11 ++--- .../Unity/FindComponentRegistration.cs | 2 +- .../Unity/InstanceComponentRegistration.cs | 37 ++++++++++++++ .../InstanceComponentRegistration.cs.meta | 3 ++ .../VContainer/Tests/ScopedContainerTest.cs | 4 +- 9 files changed, 80 insertions(+), 59 deletions(-) create mode 100644 VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs create mode 100644 VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs.meta diff --git a/VContainer/Assets/VContainer/Runtime/Container.cs b/VContainer/Assets/VContainer/Runtime/Container.cs index c1b8b665..e102322d 100644 --- a/VContainer/Assets/VContainer/Runtime/Container.cs +++ b/VContainer/Assets/VContainer/Runtime/Container.cs @@ -107,7 +107,7 @@ public void Dispose() object CreateTrackedInstance(IRegistration registration) { var lazy = sharedInstances.GetOrAdd(registration, createInstance); - if (!lazy.IsValueCreated && lazy.Value is IDisposable disposable) + if (!lazy.IsValueCreated && lazy.Value is IDisposable disposable && !(registration is InstanceRegistration)) { disposables.Add(disposable); } @@ -165,7 +165,7 @@ public object Resolve(IRegistration registration) case Lifetime.Singleton: var singleton = sharedInstances.GetOrAdd(registration, createInstance); - if (!singleton.IsValueCreated && singleton.Value is IDisposable disposable) + if (!singleton.IsValueCreated && singleton.Value is IDisposable disposable && !(registration is InstanceRegistration)) { disposables.Add(disposable); } diff --git a/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistration.cs b/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistration.cs index 79d8e0fa..86b27542 100644 --- a/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistration.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistration.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Threading; namespace VContainer.Internal { @@ -11,35 +10,19 @@ sealed class InstanceRegistration : IRegistration public Lifetime Lifetime { get; } readonly object implementationInstance; - readonly IInjector injector; - readonly IReadOnlyList parameters; - - long injected; public InstanceRegistration( object implementationInstance, Type implementationType, Lifetime lifetime, - IReadOnlyList interfaceTypes, - IReadOnlyList parameters, - IInjector injector) + IReadOnlyList interfaceTypes) { ImplementationType = implementationType; Lifetime = lifetime; InterfaceTypes = interfaceTypes; - this.implementationInstance = implementationInstance; - this.injector = injector; - this.parameters = parameters; } - public object SpawnInstance(IObjectResolver resolver) - { - if (Interlocked.CompareExchange(ref injected, 1, 0) == 0) - { - injector.Inject(implementationInstance, resolver, parameters); - } - return implementationInstance; - } + public object SpawnInstance(IObjectResolver resolver) => implementationInstance; } } \ No newline at end of file diff --git a/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistrationBuilder.cs b/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistrationBuilder.cs index 8ecd51a2..924bb688 100644 --- a/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistrationBuilder.cs +++ b/VContainer/Assets/VContainer/Runtime/Internal/InstanceRegistrationBuilder.cs @@ -12,15 +12,11 @@ public InstanceRegistrationBuilder(object implementationInstance) public override IRegistration Build() { - var injector = InjectorCache.GetOrBuild(ImplementationType); - return new InstanceRegistration( implementationInstance, ImplementationType, Lifetime, - InterfaceTypes, - Parameters, - injector); + InterfaceTypes); } } -} \ No newline at end of file +} diff --git a/VContainer/Assets/VContainer/Runtime/Unity/ComponentRegistrationBuilder.cs b/VContainer/Assets/VContainer/Runtime/Unity/ComponentRegistrationBuilder.cs index 4cb67313..f1278d97 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/ComponentRegistrationBuilder.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/ComponentRegistrationBuilder.cs @@ -22,18 +22,30 @@ public Transform GetParent() public sealed class ComponentRegistrationBuilder : RegistrationBuilder { - readonly bool finding; - readonly Scene scene; + readonly object instance; readonly Component prefab; readonly string gameObjectName; - + ComponentDestination destination; + Scene scene; + + internal ComponentRegistrationBuilder(object instance) + : base(instance.GetType(), Lifetime.Singleton) + { + this.instance = instance; + } + + internal ComponentRegistrationBuilder(in Scene scene, Type implementationType) + : base(implementationType, Lifetime.Scoped) + { + this.scene = scene; + } internal ComponentRegistrationBuilder( Component prefab, Type implementationType, Lifetime lifetime) - : this(false, default, implementationType, lifetime) + : base(implementationType, lifetime) { this.prefab = prefab; } @@ -42,32 +54,25 @@ internal ComponentRegistrationBuilder( string gameObjectName, Type implementationType, Lifetime lifetime) - : this(false, default, implementationType, lifetime) - { - this.gameObjectName = gameObjectName; - } - - internal ComponentRegistrationBuilder(Scene scene, Type implementationType) - : this(true, scene, implementationType, Lifetime.Scoped) - { - } - - ComponentRegistrationBuilder( - bool finding, - Scene scene, - Type implementationType, - Lifetime lifetime) : base(implementationType, lifetime) { - this.finding = finding; - this.scene = scene; + this.gameObjectName = gameObjectName; } public override IRegistration Build() { var injector = InjectorCache.GetOrBuild(ImplementationType); - if (finding) + if (instance != null) + { + return new InstanceComponentRegistration( + instance, + ImplementationType, + InterfaceTypes, + Parameters, + injector); + } + if (scene.IsValid()) { return new FindComponentRegistration( ImplementationType, diff --git a/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs b/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs index de9a42c3..cce8be3b 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs @@ -108,13 +108,10 @@ public static void RegisterEntryPointExceptionHandler( public static RegistrationBuilder RegisterComponent(this IContainerBuilder builder, TInterface component) { - var registrationBuilder = builder.RegisterInstance(component).As(typeof(TInterface)); - if (component is MonoBehaviour) - { - // Force inject execution - registrationBuilder - .OnAfterBuild((registration, container) => registration.SpawnInstance(container)); - } + var registrationBuilder = new ComponentRegistrationBuilder(component).As(typeof(TInterface)); + // Force inject execution + registrationBuilder.OnAfterBuild((registration, container) => registration.SpawnInstance(container)); + builder.Register(registrationBuilder); return registrationBuilder; } diff --git a/VContainer/Assets/VContainer/Runtime/Unity/FindComponentRegistration.cs b/VContainer/Assets/VContainer/Runtime/Unity/FindComponentRegistration.cs index ad2731fb..8ef5f9a8 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/FindComponentRegistration.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/FindComponentRegistration.cs @@ -9,7 +9,7 @@ sealed class FindComponentRegistration : IRegistration { public Type ImplementationType { get; } public IReadOnlyList InterfaceTypes { get; } - public Lifetime Lifetime => Lifetime.Singleton; + public Lifetime Lifetime => Lifetime.Scoped; readonly IReadOnlyList parameters; readonly IInjector injector; diff --git a/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs new file mode 100644 index 00000000..bb77e9d3 --- /dev/null +++ b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using UnityEngine; + +namespace VContainer.Unity +{ + sealed class InstanceComponentRegistration : IRegistration + { + public Type ImplementationType { get; } + public IReadOnlyList InterfaceTypes { get; } + public Lifetime Lifetime => Lifetime.Singleton; + + readonly object instance; + readonly IReadOnlyList parameters; + readonly IInjector injector; + + public InstanceComponentRegistration( + object instance, + Type implementationType, + IReadOnlyList interfaceTypes, + IReadOnlyList parameters, + IInjector injector) + { + ImplementationType = implementationType; + InterfaceTypes = interfaceTypes; + this.instance = instance; + this.parameters = parameters; + this.injector = injector; + } + + public object SpawnInstance(IObjectResolver resolver) + { + injector.Inject(instance, resolver, parameters); + return instance; + } + } +} \ No newline at end of file diff --git a/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs.meta b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs.meta new file mode 100644 index 00000000..b6ba0ad3 --- /dev/null +++ b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 3eaf1bb53ea4494484290e95207cc8f3 +timeCreated: 1620020530 \ No newline at end of file diff --git a/VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs b/VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs index df118cd1..81f77970 100644 --- a/VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs +++ b/VContainer/Assets/VContainer/Tests/ScopedContainerTest.cs @@ -217,7 +217,7 @@ public void Inject() [Test] - public void RegisterDisposableInstance() + public void InstanceRegistrationDoesNotDisposal() { var instance1 = new DisposableServiceA(); @@ -238,7 +238,7 @@ public void RegisterDisposableInstance() Assert.That(resolveFromParent.Disposed, Is.False); container.Dispose(); - Assert.That(resolveFromParent.Disposed, Is.True); + Assert.That(resolveFromParent.Disposed, Is.False); } } } From f846944ee570ab9fdef948505008ac5af4ca9e8e Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 15:07:51 +0900 Subject: [PATCH 2/6] Add tests --- .../Tests/Unity/UnityContainerBuilderTest.cs | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/VContainer/Assets/VContainer/Tests/Unity/UnityContainerBuilderTest.cs b/VContainer/Assets/VContainer/Tests/Unity/UnityContainerBuilderTest.cs index 8b4e89bc..d914e115 100644 --- a/VContainer/Assets/VContainer/Tests/Unity/UnityContainerBuilderTest.cs +++ b/VContainer/Assets/VContainer/Tests/Unity/UnityContainerBuilderTest.cs @@ -41,25 +41,40 @@ public void RegisterEntryPointMultiple() public void RegisterComponent() { var component = new GameObject("SampleBehaviour").AddComponent(); - { - var builder = new ContainerBuilder(); - builder.Register(Lifetime.Transient); - builder.RegisterComponent(component); + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.RegisterComponent(component); - var container = builder.Build(); + var container = builder.Build(); - Assert.That(container.Resolve(), Is.EqualTo(component)); - Assert.That(container.Resolve().ServiceA, Is.InstanceOf()); - } - { - var builder = new ContainerBuilder(); - builder.Register(Lifetime.Transient); - builder.RegisterComponent(component); + Assert.That(container.Resolve(), Is.EqualTo(component)); + Assert.That(container.Resolve().ServiceA, Is.InstanceOf()); + } - var container = builder.Build(); + [Test] + public void RegisterComponentAsInterface() + { + var component = new GameObject("SampleBehaviour").AddComponent(); + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.RegisterComponent(component); - Assert.That(container.Resolve(), Is.EqualTo(component)); - } + var container = builder.Build(); + + Assert.That(container.Resolve(), Is.EqualTo(component)); + } + + [Test] + public void RegisterComponentWithAutoInjection() + { + var component = new GameObject("SampleBehaviour").AddComponent(); + var builder = new ContainerBuilder(); + builder.Register(Lifetime.Transient); + builder.RegisterComponent(component); + + var container = builder.Build(); + + Assert.That(component.ServiceA, Is.InstanceOf()); } [Test] From 26ea57a7f20bb3735942efbceec7d4f561764b5e Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 15:51:21 +0900 Subject: [PATCH 3/6] [docs] Add note for RegisterInstance --- website/docs/registering/register-type.mdx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/website/docs/registering/register-type.mdx b/website/docs/registering/register-type.mdx index 17993ea7..030e30fb 100644 --- a/website/docs/registering/register-type.mdx +++ b/website/docs/registering/register-type.mdx @@ -166,7 +166,7 @@ builder.RegisterInstance(obj); ``` :::note -`RegisterIntance` always has a `Scoped` lifetime, so it has no arguments. +`RegisterIntance` always has a `Singleton` lifetime, so it has no arguments. ::: It can resolve like this: @@ -178,6 +178,18 @@ class ClassA } ``` +:::note +Instances registered with RegisterInstance are not managed by the container. + +- Dispose will not be executed automatically. +- Method Injection will not be executed automatically + +If you want the container to manage the created instances, consider using the following instead + +- [Register(_ => instance, ...)](./register-using-delegate) +- [RegisterComponent(...)](./register-monobehaviour) +::: + ## Register instance as interface Use `As*` declarations. From fcf65d3e0d76829c4227be68dd83fd503577c994 Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 16:34:33 +0900 Subject: [PATCH 4/6] Fix a test that sometimes fail --- .../VContainer/Tests/Unity/EntryPointTest.cs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs b/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs index a4c72ef7..6f25e470 100644 --- a/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs +++ b/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs @@ -1,5 +1,6 @@ using System.Collections; using NUnit.Framework; +using UnityEngine; using UnityEngine.TestTools; using VContainer.Unity; @@ -16,8 +17,6 @@ public IEnumerator Create() builder.Register(Lifetime.Scoped); }); - yield return null; - yield return null; yield return null; var entryPoint = lifetimeScope.Container.Resolve(); @@ -28,6 +27,11 @@ public IEnumerator Create() Assert.That(entryPoint.StartCalled, Is.EqualTo(1)); Assert.That(entryPoint.PostStartCalled, Is.EqualTo(1)); + yield return new WaitForFixedUpdate(); + + Assert.That(entryPoint.FixedTickCalls, Is.GreaterThan(0)); + Assert.That(entryPoint.PostFixedTickCalls, Is.GreaterThan(0)); + var fixedTickCalls = entryPoint.FixedTickCalls; var postFixedTickCalls = entryPoint.PostFixedTickCalls; var tickCalls = entryPoint.TickCalls; @@ -38,23 +42,16 @@ public IEnumerator Create() var disposable = lifetimeScope.Container.Resolve(); lifetimeScope.Dispose(); - Assert.That(fixedTickCalls, Is.GreaterThan(0)); - Assert.That(postFixedTickCalls, Is.GreaterThan(0)); - Assert.That(tickCalls, Is.GreaterThan(1)); - Assert.That(postTickCalls, Is.GreaterThan(1)); - Assert.That(lateTickCalls, Is.GreaterThan(1)); - Assert.That(postLateTickCalls, Is.GreaterThan(1)); - - yield return null; yield return null; + yield return new WaitForFixedUpdate(); + Assert.That(fixedTickCalls, Is.EqualTo(fixedTickCalls)); + Assert.That(postFixedTickCalls, Is.EqualTo(postFixedTickCalls)); + Assert.That(tickCalls, Is.EqualTo(tickCalls)); + Assert.That(postTickCalls, Is.EqualTo(postTickCalls)); + Assert.That(lateTickCalls, Is.EqualTo(lateTickCalls)); + Assert.That(postLateTickCalls, Is.EqualTo(postLateTickCalls)); Assert.That(disposable.Disposed, Is.True); - Assert.That(entryPoint.FixedTickCalls, Is.EqualTo(fixedTickCalls)); - Assert.That(entryPoint.PostFixedTickCalls, Is.EqualTo(postFixedTickCalls)); - Assert.That(entryPoint.TickCalls, Is.EqualTo(tickCalls)); - Assert.That(entryPoint.PostTickCalls, Is.EqualTo(postTickCalls)); - Assert.That(entryPoint.LateTickCalls, Is.EqualTo(lateTickCalls)); - Assert.That(entryPoint.PostLateTickCalls, Is.EqualTo(postLateTickCalls)); } [UnityTest] From 3400bb7024d54ebca48769cea8aa867105841295 Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 16:50:12 +0900 Subject: [PATCH 5/6] Fix ecs behaviour --- .../Runtime/Unity/ContainerBuilderUnityExtensions.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs b/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs index cce8be3b..9024d2cf 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs @@ -208,9 +208,8 @@ public static RegistrationBuilder RegisterSystemFromWorld(this IContainerBuil if (system is null) throw new ArgumentException($"{typeof(T).FullName} is not in the world {world}"); - return builder.RegisterInstance(system) - .As(typeof(ComponentSystemBase)) - .AsSelf(); + return builder.RegisterComponent(system) + .As(typeof(ComponentSystemBase), typeof(T)); } // Use custom world From bfe83e86303961b1b06f2dca740e63f0e13d4c58 Mon Sep 17 00:00:00 2001 From: hadashi Date: Mon, 3 May 2021 16:59:57 +0900 Subject: [PATCH 6/6] Fix test --- .../VContainer/Runtime/Unity/InstanceComponentRegistration.cs | 1 - VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs index bb77e9d3..0d2144bf 100644 --- a/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs +++ b/VContainer/Assets/VContainer/Runtime/Unity/InstanceComponentRegistration.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using UnityEngine; namespace VContainer.Unity { diff --git a/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs b/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs index 6f25e470..22270bd4 100644 --- a/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs +++ b/VContainer/Assets/VContainer/Tests/Unity/EntryPointTest.cs @@ -27,6 +27,7 @@ public IEnumerator Create() Assert.That(entryPoint.StartCalled, Is.EqualTo(1)); Assert.That(entryPoint.PostStartCalled, Is.EqualTo(1)); + yield return new WaitForFixedUpdate(); yield return new WaitForFixedUpdate(); Assert.That(entryPoint.FixedTickCalls, Is.GreaterThan(0));