Skip to content
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.

Sign up for GitHub

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

Fix an issue with Start being called multiple times with exception #734

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Fix an issue with Start being called multiple times with exception #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 83 additions & 65 deletions VContainer/Assets/Tests/Unity/PlayerLoopItemTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using NUnit.Framework;
using UnityEngine.TestTools.Constraints;
using VContainer.Unity;
Expand All @@ -8,7 +9,7 @@ namespace VContainer.Tests.Unity
{
public class PlayerLoopItemTest
{
private class Ticker : ITickable, IPostTickable, IPostLateTickable, IFixedTickable, IPostFixedTickable
class Ticker : ITickable, IPostTickable, IPostLateTickable, IFixedTickable, IPostFixedTickable
{
public void Tick() { }
public void FixedTick() { }
Expand All @@ -17,84 +18,101 @@ public void PostLateTick() { }
public void PostFixedTick() { }
}

private class TickableLoopItemTest
class TestStartable : IStartable
{
[Test]
public void MoveNextWithoutAllocation()
public int Executed { get; private set; }

public void Start()
{
var list = new List<ITickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new TickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
Executed++;
}
}
private class PostTickableLoopItemTest

class ThrowStartable : IStartable
{
[Test]
public void MoveNextWithoutAllocation()
public int Executed { get; private set; }

public void Start()
{
var list = new List<IPostTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
Executed++;
throw new InvalidOperationException("OOPS");
}
}

private class PostLateTickableLoopItemTest

[Test]
public void Start_Throw()
{
[Test]
public void MoveNextWithoutAllocation()
var item = new TestStartable();
var thrownItem = new ThrowStartable();
var exceptionHandler = new EntryPointExceptionHandler(ex => { });
var loopItem = new StartableLoopItem(new IStartable[] { thrownItem, item }, exceptionHandler);

Assert.That(loopItem.MoveNext(), Is.False);
}

[Test]
public void Tick_MoveNextWithoutAllocation()
{
var list = new List<ITickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new TickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
var list = new List<IPostLateTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostLateTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}

private class FixedTickableLoopItemTest

[Test]
public void PostTick_MoveNextWithoutAllocation()
{
[Test]
public void MoveNextWithoutAllocation()
var list = new List<IPostTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
var list = new List<IFixedTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new FixedTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}

private class PostFixedTickableLoopItemTest

[Test]
public void LateTick_MoveNextWithoutAllocation()
{
[Test]
public void MoveNextWithoutAllocation()
var list = new List<IPostLateTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostLateTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
var list = new List<IPostFixedTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostFixedTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}

[Test]
public void FixedTick_MoveNextWithoutAllocation()
{
var list = new List<IFixedTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new FixedTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}

[Test]
public void PostFixedTick_MoveNextWithoutAllocation()
{
var list = new List<IPostFixedTickable> { new Ticker(), new Ticker() };
var exceptionHandler = new EntryPointExceptionHandler(exception => { });
var tickableLoopItem = new PostFixedTickableLoopItem(list, exceptionHandler);

Assert.That(() =>
{
tickableLoopItem.MoveNext();
}, Is.Not.AllocatingGCMemory());
}
}
}
}
11 changes: 7 additions & 4 deletions VContainer/Assets/VContainer/Runtime/Unity/ContainerBuilderUnityExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ public static void EnsureDispatcherRegistered(IContainerBuilder containerBuilder
{
if (containerBuilder.Exists(typeof(EntryPointDispatcher), false)) return;
containerBuilder.Register<EntryPointDispatcher>(Lifetime.Scoped);

containerBuilder.RegisterEntryPointExceptionHandler(UnityEngine.Debug.LogException);

containerBuilder.RegisterBuildCallback(container =>
{
container.Resolve<EntryPointDispatcher>().Dispatch();
Expand Down Expand Up @@ -120,7 +123,7 @@ public static void RegisterEntryPointExceptionHandler(
this IContainerBuilder builder,
Action<Exception> exceptionHandler)
{
builder.RegisterInstance(new EntryPointExceptionHandler(exceptionHandler));
builder.Register(c => new EntryPointExceptionHandler(exceptionHandler), Lifetime.Scoped);
}

public static RegistrationBuilder RegisterComponent<TInterface>(
Expand Down Expand Up @@ -197,7 +200,7 @@ public static ComponentRegistrationBuilder RegisterComponentInNewPrefab<T>(
{
return builder.RegisterComponentInNewPrefab(typeof(T), prefab, lifetime);
}

public static ComponentRegistrationBuilder RegisterComponentInNewPrefab<T>(
this IContainerBuilder builder,
Func<IObjectResolver, T> prefab,
Expand All @@ -206,7 +209,7 @@ public static ComponentRegistrationBuilder RegisterComponentInNewPrefab<T>(
{
return builder.Register(new ComponentRegistrationBuilder(prefab, typeof(T), lifetime));
}

public static ComponentRegistrationBuilder RegisterComponentInNewPrefab<TInterface, TImplement>(
this IContainerBuilder builder,
Func<IObjectResolver, TImplement> prefab,
Expand Down Expand Up @@ -300,7 +303,7 @@ public static RegistrationBuilder RegisterUnmanagedSystemFromWorld<T>(this ICont
Type refType = typeof(UnmanagedSystemReference<>);
Type target = refType.MakeGenericType(typeof(T));
var reference = (UnmanagedSystemReference)Activator.CreateInstance(target, system, world);

return builder.RegisterComponent(reference)
.As(target);
}
Expand Down
2 changes: 1 addition & 1 deletion VContainer/Assets/VContainer/Runtime/Unity/EntryPointDispatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void Dispatch()
{
PlayerLoopHelper.EnsureInitialized();

EntryPointExceptionHandler exceptionHandler = container.ResolveOrDefault<EntryPointExceptionHandler>();
var exceptionHandler = container.ResolveOrDefault<EntryPointExceptionHandler>();

var initializables = container.Resolve<ContainerLocal<IReadOnlyList<IInitializable>>>().Value;
for (var i = 0; i < initializables.Count; i++)
Expand Down
12 changes: 6 additions & 6 deletions VContainer/UserSettings/Layouts/default-2021.dwlt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ MonoBehaviour:
m_MinSize: {x: 300, y: 200}
m_MaxSize: {x: 24288, y: 16192}
vertical: 0
controlID: 83
controlID: 84
--- !u!114 &3
MonoBehaviour:
m_ObjectHideFlags: 52
Expand Down Expand Up @@ -223,7 +223,7 @@ MonoBehaviour:
m_MinSize: {x: 200, y: 200}
m_MaxSize: {x: 16192, y: 16192}
vertical: 1
controlID: 84
controlID: 63
--- !u!114 &10
MonoBehaviour:
m_ObjectHideFlags: 52
Expand All @@ -248,7 +248,7 @@ MonoBehaviour:
m_MinSize: {x: 200, y: 100}
m_MaxSize: {x: 16192, y: 8096}
vertical: 0
controlID: 85
controlID: 64
--- !u!114 &11
MonoBehaviour:
m_ObjectHideFlags: 52
Expand Down Expand Up @@ -333,7 +333,7 @@ MonoBehaviour:
scrollPos: {x: 0, y: 0}
m_SelectedIDs: 84440000
m_LastClickedID: 17540
m_ExpandedIDs: 0000000086440000
m_ExpandedIDs: 000000006e440000
m_RenameOverlay:
m_UserAcceptedRename: 0
m_Name:
Expand Down Expand Up @@ -361,7 +361,7 @@ MonoBehaviour:
scrollPos: {x: 0, y: 0}
m_SelectedIDs:
m_LastClickedID: 0
m_ExpandedIDs: 0000000086440000
m_ExpandedIDs: 000000006e440000
m_RenameOverlay:
m_UserAcceptedRename: 0
m_Name:
Expand Down Expand Up @@ -495,7 +495,7 @@ MonoBehaviour:
scrollPos: {x: 0, y: 0}
m_SelectedIDs:
m_LastClickedID: 0
m_ExpandedIDs: 1efbffff
m_ExpandedIDs: 30fbffff
m_RenameOverlay:
m_UserAcceptedRename: 0
m_Name:
Expand Down
Loading