Merge pull request #5 from chickensoft-games/fix/multiple-deps-per-pr…

…ovider

fix: correct dependency resolution for dependents that rely on more than one dependency from the same provider

.vscode/settings.json

@@ -138,7 +138,6 @@
     "**/LICENSE"
   ],
   "omnisharp.enableEditorConfigSupport": true,
-  "omnisharp.enableImportCompletion": true,
   "omnisharp.enableMsBuildLoadProjectsOnDemand": false,
   "omnisharp.enableRoslynAnalyzers": true,
   "omnisharp.maxFindSymbolsItems": 3000,
@@ -162,5 +161,6 @@
       "icon": "terminal-powershell",
       "source": "PowerShell"
     }
-  }
+  },
+  "dotnet.completion.showCompletionItemsFromUnimportedNamespaces": true
 }

Chickensoft.AutoInject.Tests/Chickensoft.AutoInject.Tests.csproj

@@ -9,15 +9,14 @@
     <!-- godotengine/godot/issues/42271#issuecomment-751423827 -->
     <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
     <!-- To show generated files -->
-
     <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
     <CompilerGeneratedFilesOutputPath>.generated</CompilerGeneratedFilesOutputPath>
-
     <DebugType>full</DebugType>
     <DebugSymbols>true</DebugSymbols>
   </PropertyGroup>
   <ItemGroup>
     <PackageReference Include="Chickensoft.GoDotTest" Version="1.1.7" />
+    <PackageReference Include="GodotTestDriver" Version="2.1.0" />
     <!-- Bring your own assertion library for tests! -->
     <!-- We're using Shouldly for this example, but you can use anything. -->
     <PackageReference Include="Shouldly" Version="4.1.0" />

Chickensoft.AutoInject.Tests/icon.svg.import

@@ -16,9 +16,9 @@ dest_files=["res://.godot/imported/icon.svg-218a8f2b3041327d8a5756f3a245f83b.cte
 [params]
 
 compress/mode=0
+compress/high_quality=false
 compress/lossy_quality=0.7
 compress/hdr_compression=1
-compress/bptc_ldr=0
 compress/normal_map=0
 compress/channel_pack=0
 mipmaps/generate=false

Chickensoft.AutoInject.Tests/src/Dependent.cs

@@ -162,12 +162,17 @@ public class DependencyState {
   /// their provided values. We use this in the rare event that we have to
   /// clean up subscriptions before providers ever finished initializing.
   /// </summary>
-  public Dictionary<IProvider, DependencyPending> Pending { get; }
+  public Dictionary<IProvider, PendingProvider> Pending { get; }
     = new();
 }
 
-public record DependencyPending(IProvider Provider, Action<IProvider> Success) {
-  public void Cancel() => Provider.ProviderState.OnInitialized -= Success;
+public record PendingProvider(
+  IProvider Provider,
+  Action<IProvider> Success
+) {
+  public void Unsubscribe() {
+    Provider.ProviderState.OnInitialized -= Success;
+  }
 }
 
 /// <summary>
@@ -249,7 +254,7 @@ ImmutableDictionary<string, ScriptPropertyOrField> allDependencies
     if (what == Node.NotificationExitTree) {
       dependent.DependentState.ShouldResolveDependencies = true;
       foreach (var pending in dependent.DependentState.Pending.Values) {
-        pending.Cancel();
+        pending.Unsubscribe();
       }
       dependent.DependentState.Pending.Clear();
     }
@@ -340,7 +345,7 @@ void onProviderInitialized(IProvider provider) {
       providersInitializing--;
 
       lock (dependent.DependentState.Pending) {
-        dependent.DependentState.Pending[provider].Cancel();
+        dependent.DependentState.Pending[provider].Unsubscribe();
         dependent.DependentState.Pending.Remove(provider);
       }
 
@@ -378,11 +383,12 @@ void onProviderInitialized(IProvider provider) {
             // If the provider is not yet initialized, subscribe to its
             // initialization event and add it to the list of pending
             // subscriptions.
-            if (!provider.ProviderState.IsInitialized) {
-              state.Pending.Add(
-                key: provider,
-                value: new DependencyPending(provider, onProviderInitialized)
-              );
+            if (
+              !provider.ProviderState.IsInitialized &&
+              !state.Pending.ContainsKey(provider)
+            ) {
+              state.Pending[provider] =
+                new PendingProvider(provider, onProviderInitialized);
               provider.ProviderState.OnInitialized += onProviderInitialized;
               providersInitializing++;
             }

Chickensoft.AutoInject.Tests/test/fixtures/MultiProvider.cs

@@ -0,0 +1,29 @@
+namespace Chickensoft.AutoInject.Tests.Fixtures;
+
+using Chickensoft.AutoInject;
+using Chickensoft.AutoInject.Tests.Subjects;
+using Godot;
+using SuperNodes.Types;
+
+[SuperNode(typeof(Provider))]
+public partial class MultiProvider : Node2D, IProvide<int>, IProvide<string> {
+  public override partial void _Notification(int what);
+
+  int IProvide<int>.Value() => IntValue;
+  string IProvide<string>.Value() => StringValue;
+
+  public MultiDependent Child { get; private set; } = default!;
+
+  public override void _Ready() {
+    Child = new MultiDependent();
+    AddChild(Child);
+
+    Provide();
+  }
+
+  public bool OnProvidedCalled { get; private set; }
+  public int IntValue { get; set; }
+  public string StringValue { get; set; } = "";
+
+  public void OnProvided() => OnProvidedCalled = true;
+}

Chickensoft.AutoInject.Tests/test/fixtures/MultiProvider.tscn

@@ -0,0 +1,6 @@
+[gd_scene load_steps=2 format=3 uid="uid://bby7an0ngn7l6"]
+
+[ext_resource type="Script" path="res://test/fixtures/MultiProvider.cs" id="1_gn2k1"]
+
+[node name="MultiProvider" type="Node2D"]
+script = ExtResource("1_gn2k1")

Chickensoft.AutoInject.Tests/test/src/MiscTest.cs

@@ -46,9 +46,9 @@ public void DependencyPendingCancels() {
 
     provider.ProviderState.OnInitialized += onInitialized;
 
-    var pending = new DependencyPending(provider, onInitialized);
+    var pending = new PendingProvider(provider, onInitialized);
 
-    pending.Cancel();
+    pending.Unsubscribe();
 
     provider.ProviderState.Announce(provider);
 

Chickensoft.AutoInject.Tests/test/src/MultiResolutionTest.cs

@@ -0,0 +1,36 @@
+namespace Chickensoft.GodotGame;
+
+using System.Threading.Tasks;
+using Chickensoft.AutoInject.Tests.Fixtures;
+using Chickensoft.GoDotTest;
+using Godot;
+using GodotTestDriver;
+using GodotTestDriver.Util;
+using Shouldly;
+
+#pragma warning disable CA1001
+public class MultiResolutionTest : TestClass {
+  private Fixture _fixture = default!;
+  private MultiProvider _provider = default!;
+
+  public MultiResolutionTest(Node testScene) : base(testScene) { }
+
+  [Setup]
+  public async Task Setup() {
+    _fixture = new Fixture(TestScene.GetTree());
+    _provider = await _fixture.LoadAndAddScene<MultiProvider>(
+      "res://test/fixtures/MultiProvider.tscn"
+    );
+  }
+
+  [Cleanup]
+  public void Cleanup() => _fixture.Cleanup();
+
+  [Test]
+  public async Task MultiDependentSubscribesToMultiProviderCorrectly() {
+    _provider.Child.ReadyCalled.ShouldBeTrue();
+    await TestScene.ProcessFrame(2);
+    _provider.Child.OnResolvedCalled.ShouldBeTrue();
+  }
+}
+#pragma warning restore CA1001

Chickensoft.AutoInject.Tests/test/src/subjects/Dependents.cs

@@ -57,3 +57,26 @@ public void OnResolved() {
     ResolvedValue = MyDependency;
   }
 }
+
+[SuperNode(typeof(Dependent))]
+public partial class MultiDependent : Node {
+  public override partial void _Notification(int what);
+
+  [Dependency]
+  public int IntDependency => DependOn<int>();
+
+  [Dependency]
+  public string StringDependency => DependOn<string>();
+
+  public bool OnResolvedCalled { get; private set; }
+  public int IntResolvedValue { get; set; }
+  public string StringResolvedValue { get; set; } = null!;
+  public bool ReadyCalled { get; set; }
+  public void OnReady() => ReadyCalled = true;
+
+  public void OnResolved() {
+    OnResolvedCalled = true;
+    IntResolvedValue = IntDependency;
+    StringResolvedValue = StringDependency;
+  }
+}

global.json

@@ -4,6 +4,6 @@
     "rollForward": "latestMinor"
   },
   "msbuild-sdks": {
-    "Godot.NET.Sdk": "4.0.2"
+    "Godot.NET.Sdk": "4.0.4"
   }
 }