From f3fad287926ff5038aaec8602cf1497cb4556479 Mon Sep 17 00:00:00 2001 From: Joanna May Date: Sat, 6 Jul 2024 12:58:54 -0500 Subject: [PATCH] fix: deterministic lifecycle --- .../src/auto_connect/IAutoConnect.cs | 2 +- .../dependent/DependencyResolver.cs | 19 ++++++++++++--- .../auto_inject/dependent/DependentState.cs | 5 ++++ .../src/auto_inject/dependent/IDependent.cs | 20 +++++++++++++++- .../src/auto_on/IAutoOn.cs | 6 +++++ .../src/misc/IReadyAware.cs | 12 ++++++++++ .../test/src/AutoConnectInvalidCastTest.cs | 24 ++++++++++++++++++- .../test/src/ResolutionTest.cs | 20 +++++++++------- README.md | 18 ++++++++++++++ 9 files changed, 111 insertions(+), 15 deletions(-) create mode 100644 Chickensoft.AutoInject.Tests/src/misc/IReadyAware.cs diff --git a/Chickensoft.AutoInject.Tests/src/auto_connect/IAutoConnect.cs b/Chickensoft.AutoInject.Tests/src/auto_connect/IAutoConnect.cs index 0d86112..0b9474f 100644 --- a/Chickensoft.AutoInject.Tests/src/auto_connect/IAutoConnect.cs +++ b/Chickensoft.AutoInject.Tests/src/auto_connect/IAutoConnect.cs @@ -31,7 +31,7 @@ public interface IAutoConnect : IMixin, IFakeNodeTreeEnabled { void IMixin.Handler() { var what = MixinState.Get().Notification; - if (what == Node.NotificationSceneInstantiated) { + if (what == Node.NotificationEnterTree) { AutoConnector.ConnectNodes(Types.Graph.GetProperties(GetType()), this); } } diff --git a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependencyResolver.cs b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependencyResolver.cs index 778403b..c097774 100644 --- a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependencyResolver.cs +++ b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependencyResolver.cs @@ -181,15 +181,28 @@ IEnumerable properties GetDependenciesToResolve(properties) ); - var node = ((Node)dependent).GetParent(); + var self = (Node)dependent; + var node = self.GetParent(); var foundDependencies = new HashSet(); var providersInitializing = 0; void resolve() { + if (self.IsNodeReady()) { + // Godot node is already ready. + if (!dependent.IsTesting) { + dependent.Setup(); + } + dependent.OnResolved(); + return; + } + + // Godot node is not ready yet, so we will wait for OnReady before + // calling Setup() and OnResolved(). + if (!dependent.IsTesting) { - dependent.Setup(); + state.PleaseCallSetup = true; } - dependent.OnResolved(); + state.PleaseCallOnResolved = true; } void onProviderInitialized(IBaseProvider provider) { diff --git a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependentState.cs b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependentState.cs index 6e286f4..2d746dc 100644 --- a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependentState.cs +++ b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/DependentState.cs @@ -33,6 +33,11 @@ public class DependentState { /// public bool ShouldResolveDependencies { get; set; } = true; + /// Set internally when Setup() should be called. + public bool PleaseCallSetup { get; set; } + /// Set internally when OnResolved() should be called. + public bool PleaseCallOnResolved { get; set; } + /// /// Dictionary of providers we are listening to that are still initializing /// their provided values. We use this in the rare event that we have to diff --git a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/IDependent.cs b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/IDependent.cs index ad9a724..f34c19f 100644 --- a/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/IDependent.cs +++ b/Chickensoft.AutoInject.Tests/src/auto_inject/dependent/IDependent.cs @@ -6,6 +6,8 @@ namespace Chickensoft.AutoInject; using Chickensoft.AutoInject; using Chickensoft.Introspection; using System.Globalization; +using System; +using System.Runtime.CompilerServices; /// @@ -13,7 +15,7 @@ namespace Chickensoft.AutoInject; /// resolve dependencies marked with the [Dependency] attribute. /// [Mixin] -public interface IDependent : IMixin, IAutoInit { +public interface IDependent : IMixin, IAutoInit, IReadyAware { DependentState DependentState { get { AddStateIfNeeded(); @@ -36,6 +38,22 @@ void Setup() { } /// void OnResolved() { } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + void IReadyAware.OnBeforeReady() { } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + void IReadyAware.OnAfterReady() { + if (DependentState.PleaseCallSetup) { + Setup(); + DependentState.PleaseCallSetup = false; + } + if (DependentState.PleaseCallOnResolved) { + OnResolved(); + DependentState.PleaseCallOnResolved = false; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void AddStateIfNeeded() { if (MixinState.Has()) { return; } diff --git a/Chickensoft.AutoInject.Tests/src/auto_on/IAutoOn.cs b/Chickensoft.AutoInject.Tests/src/auto_on/IAutoOn.cs index aa085e1..ebcb736 100644 --- a/Chickensoft.AutoInject.Tests/src/auto_on/IAutoOn.cs +++ b/Chickensoft.AutoInject.Tests/src/auto_on/IAutoOn.cs @@ -102,6 +102,12 @@ public static void InvokeNotificationMethods(object? obj, int what) { autoNode.OnChildOrderChanged(); break; case (int)Node.NotificationReady: + if (node is IReadyAware readyAware) { + readyAware.OnBeforeReady(); + autoNode.OnReady(); + readyAware.OnAfterReady(); + break; + } autoNode.OnReady(); break; case (int)Node.NotificationEditorPostSave: diff --git a/Chickensoft.AutoInject.Tests/src/misc/IReadyAware.cs b/Chickensoft.AutoInject.Tests/src/misc/IReadyAware.cs new file mode 100644 index 0000000..773c48e --- /dev/null +++ b/Chickensoft.AutoInject.Tests/src/misc/IReadyAware.cs @@ -0,0 +1,12 @@ +namespace Chickensoft.AutoInject; + +/// +/// Types that want to be informed of ready can implement this interface. +/// +public interface IReadyAware { + /// Called right before the node is ready. + void OnBeforeReady(); + + /// Called right after the node is readied. + void OnAfterReady(); +} diff --git a/Chickensoft.AutoInject.Tests/test/src/AutoConnectInvalidCastTest.cs b/Chickensoft.AutoInject.Tests/test/src/AutoConnectInvalidCastTest.cs index 81c4a2b..fc0a2bf 100644 --- a/Chickensoft.AutoInject.Tests/test/src/AutoConnectInvalidCastTest.cs +++ b/Chickensoft.AutoInject.Tests/test/src/AutoConnectInvalidCastTest.cs @@ -27,7 +27,29 @@ public void ThrowsIfFakedChildNodeIsWrongType() { scene.FakeNodeTree(new() { ["Node3D"] = new Mock().Object }); Should.Throw( - () => scene._Notification((int)Node.NotificationSceneInstantiated) + () => scene._Notification((int)Node.NotificationEnterTree) + ); + } + + [Test] + public void ThrowsIfNoNode() { + var scene = new AutoConnectInvalidCastTestScene(); + Should.Throw( + () => scene._Notification((int)Node.NotificationEnterTree) + ); + } + + [Test] + public void ThrowsIfTypeIsWrong() { + var scene = new AutoConnectInvalidCastTestScene(); + + var node = new Control { + Name = "Node3D" + }; + scene.AddChild(node); + + Should.Throw( + () => scene._Notification((int)Node.NotificationEnterTree) ); } } diff --git a/Chickensoft.AutoInject.Tests/test/src/ResolutionTest.cs b/Chickensoft.AutoInject.Tests/test/src/ResolutionTest.cs index b4e1dcc..b5e825f 100644 --- a/Chickensoft.AutoInject.Tests/test/src/ResolutionTest.cs +++ b/Chickensoft.AutoInject.Tests/test/src/ResolutionTest.cs @@ -1,7 +1,9 @@ namespace Chickensoft.AutoInject.Tests; +using System.Threading.Tasks; using Chickensoft.AutoInject.Tests.Subjects; using Chickensoft.GoDotTest; +using Chickensoft.GodotTestDriver; using Godot; using Shouldly; @@ -59,19 +61,18 @@ public void ResolvesDependencyWhenProviderIsAlreadyInitialized() { } [Test] - public void ResolvesDependencyAfterProviderIsResolved() { + public async Task ResolvesDependencyAfterProviderIsResolved() { var value = "Hello, world!"; var obj = new StringProvider() { Value = value }; var provider = obj as IBaseProvider; var dependent = new StringDependent(); - + var fixture = new Fixture(TestScene.GetTree()); obj.AddChild(dependent); - ((IProvide)provider).Value().ShouldBe(value); + await fixture.AddToRoot(obj); - dependent._Notification((int)Node.NotificationReady); + ((IProvide)provider).Value().ShouldBe(value); - obj._Notification((int)Node.NotificationReady); provider.ProviderState.IsInitialized.ShouldBeTrue(); obj.OnProvidedCalled.ShouldBeTrue(); @@ -79,13 +80,15 @@ public void ResolvesDependencyAfterProviderIsResolved() { dependent.ResolvedValue.ShouldBe(value); ((IDependent)dependent).DependentState.Pending.ShouldBeEmpty(); + await fixture.Cleanup(); + obj.RemoveChild(dependent); dependent.QueueFree(); obj.QueueFree(); } [Test] - public void FindsDependenciesAcrossAncestors() { + public async Task FindsDependenciesAcrossAncestors() { var value = "Hello, world!"; var objA = new StringProvider() { Value = value }; @@ -94,17 +97,16 @@ public void FindsDependenciesAcrossAncestors() { var providerB = objB as IBaseProvider; var depObj = new StringDependent(); var dependent = depObj as IDependent; + var fixture = new Fixture(TestScene.GetTree()); objA.AddChild(objB); objA.AddChild(depObj); - depObj._Notification((int)Node.NotificationReady); + await fixture.AddToRoot(objA); - objA._Notification((int)Node.NotificationReady); providerA.ProviderState.IsInitialized.ShouldBeTrue(); objA.OnProvidedCalled.ShouldBeTrue(); - objB._Notification((int)Node.NotificationReady); providerB.ProviderState.IsInitialized.ShouldBeTrue(); objB.OnProvidedCalled.ShouldBeTrue(); diff --git a/README.md b/README.md index 8335b0d..a25e1bc 100644 --- a/README.md +++ b/README.md @@ -416,6 +416,24 @@ var myNode = new MyNode() { For example tests, please see the [Game Demo] project. +## 🌱 Enhanced Lifecycle + +AutoInject enhances the typical Godot node lifecycle by adding additional hooks that allow you to handle dependencies and initialization in a more controlled manner (primarily for making testing easier). + +This is the lifecycle of a dependent node in the game environment: + +```text +Initialize() -> OnReady() -> Setup() -> OnResolved() +``` + +Note that this lifecycle is preserved regardless of how the node is added to the scene tree. + +And this is the lifecycle of a dependent node in a test environment: + +```text +OnReady() -> OnResolved() +``` + ## 🔋 IAutoOn The `IAutoOn` mixin allows node scripts to implement .NET-style handler methods for Godot notifications, prefixed with `On`.