From 71c0e3b94722c8572747fdcfa420ac80ef393c58 Mon Sep 17 00:00:00 2001
From: Dan Korostelev <nadako@gmail.com>
Date: Mon, 12 Jun 2023 17:43:40 +0200
Subject: [PATCH] Swap between two List for the UnityBatchScheduler queues
 instead of allocating a new one each time (closes #19)

Also add tests for the case of one binding scheduling another
---
 .../Runtime/UnityBatchScheduler.cs            | 11 +++--
 .../Tests/TestUnityBatchScheduler.cs          | 42 ++++++++++++++++++-
 .../Tests/TestTriggerBindingFromBinding.cs    | 38 +++++++++++++++++
 .../TestTriggerBindingFromBinding.cs.meta     |  3 ++
 4 files changed, 90 insertions(+), 4 deletions(-)
 create mode 100644 src/TinkState/Tests/TestTriggerBindingFromBinding.cs
 create mode 100644 src/TinkState/Tests/TestTriggerBindingFromBinding.cs.meta

diff --git a/src/TinkState-Unity/Runtime/UnityBatchScheduler.cs b/src/TinkState-Unity/Runtime/UnityBatchScheduler.cs
index 1cc5f2f..a3009fe 100644
--- a/src/TinkState-Unity/Runtime/UnityBatchScheduler.cs
+++ b/src/TinkState-Unity/Runtime/UnityBatchScheduler.cs
@@ -30,6 +30,7 @@ static void OnUpdate()
         static readonly UnityBatchScheduler Instance = new UnityBatchScheduler();
 
         List<Schedulable> queue = new List<Schedulable>();
+        List<Schedulable> nextQueue = new List<Schedulable>();
         bool scheduled;
 
         UnityBatchScheduler() { }
@@ -45,9 +46,13 @@ void Progress(float maxSeconds)
             var end = GetTimeStamp() + maxSeconds;
             do
             {
-                var old = queue;
-                queue = new List<Schedulable>();
-                foreach (var o in old) o.Run();
+                // to handle the unfortunate case where a binding invocation schedules another one
+                // we have two queues and swap between them to avoid allocating a new list every time
+                var currentQueue = queue;
+                queue = nextQueue;
+                foreach (var o in currentQueue) o.Run();
+                currentQueue.Clear();
+                nextQueue = currentQueue;
             }
             while (queue.Count > 0 && GetTimeStamp() < end);
 
diff --git a/src/TinkState-Unity/Tests/TestUnityBatchScheduler.cs b/src/TinkState-Unity/Tests/TestUnityBatchScheduler.cs
index 0ee994c..7ea1bbf 100644
--- a/src/TinkState-Unity/Tests/TestUnityBatchScheduler.cs
+++ b/src/TinkState-Unity/Tests/TestUnityBatchScheduler.cs
@@ -2,6 +2,7 @@
 using NUnit.Framework;
 using UnityEngine.TestTools;
 using TinkState;
+using UnityEngine;
 
 public class TestUnityBatchScheduler
 {
@@ -64,4 +65,43 @@ public IEnumerator TestBindingNotInvokedIfFinalValueIsSame()
         yield return null;
         Assert.That(bindingCalls, Is.EqualTo(1));
     }
-}
+
+    // NOTE: this is not an intended usage for bindings, as one shouldn't
+    // trigger bindings from other bindings (e.g. by changing a State) and
+    // instead make use of auto-Observables and such, but we still test
+    // that this works as expected and doesn't hang
+    [UnityTest]
+    public IEnumerator TestTriggerBindingFromBinding()
+    {
+	    var s = Observable.State(2);
+	    var s2 = Observable.State(2);
+
+	    var bindingCalls = 0;
+	    var expectedValue = 2;
+	    void BindCallback(int value)
+	    {
+		    bindingCalls++;
+		    Assert.That(value, Is.EqualTo(expectedValue));
+	    }
+
+	    s2.Bind(BindCallback);
+	    Assert.That(bindingCalls, Is.EqualTo(1));
+
+	    s.Bind(v => s2.Value = v * 2);
+
+	    // first binding was still not called because it's scheduled!
+	    Assert.That(bindingCalls, Is.EqualTo(1));
+
+	    // it will be called as scheduled though
+	    expectedValue = 4;
+	    yield return null;
+	    Assert.That(bindingCalls, Is.EqualTo(2));
+
+	    // changing the value will invoke the binding for `s` which will in its turn schedule the binding for `s2`,
+	    // which should be processed right after, let's check that
+	    s.Value = 3;
+	    expectedValue = 6;
+	    yield return null;
+	    Assert.That(bindingCalls, Is.EqualTo(3));
+    }
+}
\ No newline at end of file
diff --git a/src/TinkState/Tests/TestTriggerBindingFromBinding.cs b/src/TinkState/Tests/TestTriggerBindingFromBinding.cs
new file mode 100644
index 0000000..3a5bc00
--- /dev/null
+++ b/src/TinkState/Tests/TestTriggerBindingFromBinding.cs
@@ -0,0 +1,38 @@
+using NUnit.Framework;
+using TinkState;
+
+namespace Test
+{
+	// NOTE: this is not an intended usage for bindings, as one shouldn't
+	// trigger bindings from other bindings (e.g. by changing a State) and
+	// instead make use of auto-Observables and such, but we still test
+	// that this works as expected and doesn't hang
+	class TestTriggerBindingFromBinding : BaseTest
+	{
+		[Test]
+		public void Test()
+		{
+			var s = Observable.State(2);
+			var s2 = Observable.State(2);
+
+			var bindingCalls = 0;
+			var expectedValue = 2;
+			void BindCallback(int value)
+			{
+				bindingCalls++;
+				Assert.That(value, Is.EqualTo(expectedValue));
+			}
+
+			s2.Bind(BindCallback);
+			Assert.That(bindingCalls, Is.EqualTo(1));
+
+			expectedValue = 4;
+			s.Bind(v => s2.Value = v * 2);
+			Assert.That(bindingCalls, Is.EqualTo(2));
+
+			expectedValue = 6;
+			s.Value = 3;
+			Assert.That(bindingCalls, Is.EqualTo(3));
+		}
+	}
+}
\ No newline at end of file
diff --git a/src/TinkState/Tests/TestTriggerBindingFromBinding.cs.meta b/src/TinkState/Tests/TestTriggerBindingFromBinding.cs.meta
new file mode 100644
index 0000000..18cf1bd
--- /dev/null
+++ b/src/TinkState/Tests/TestTriggerBindingFromBinding.cs.meta
@@ -0,0 +1,3 @@
+fileFormatVersion: 2
+guid: ac392cec5dc84d1b91730fe3f0ec4baa
+timeCreated: 1686583493
\ No newline at end of file