Fix duplicate agent creation when reactivating scene

Runtime/AgentDispatcher.cs

@@ -14,23 +14,16 @@ namespace DeNA.Anjin
     /// <summary>
     /// Agent dispatcher interface
     /// </summary>
-    public interface IAgentDispatcher
+    public interface IAgentDispatcher : IDisposable
     {
-        /// <summary>
-        /// Agent dispatch by next scene
-        /// </summary>
-        /// <param name="current">Current scene</param>
-        /// <param name="next">Next transition scene</param>
-        void DispatchByScene(Scene current, Scene next);
-
         /// <summary>
         /// Agent dispatch by current scene
         /// </summary>
         /// <param name="scene">Current scene</param>
         void DispatchByScene(Scene scene);
     }
 
-    /// <inheritdoc />
+    /// <inheritdoc/>
     public class AgentDispatcher : IAgentDispatcher
     {
         private readonly AutopilotSettings _settings;
@@ -48,22 +41,20 @@ public AgentDispatcher(AutopilotSettings settings, ILogger logger, RandomFactory
             _settings = settings;
             _logger = logger;
             _randomFactory = randomFactory;
+            SceneManager.sceneLoaded += this.DispatchByScene;
         }
 
-        /// <summary>
-        /// Dispatch agent mapped to Scene `next`
-        /// </summary>
-        /// <param name="current"></param>
-        /// <param name="next"></param>
-        public void DispatchByScene(Scene current, Scene next)
+        public void Dispose()
+        {
+            SceneManager.sceneLoaded -= this.DispatchByScene;
+        }
+
+        private void DispatchByScene(Scene next, LoadSceneMode mode)
         {
             DispatchByScene(next);
         }
 
-        /// <summary>
-        /// Dispatch agent mapped to Scene
-        /// </summary>
-        /// <param name="scene"></param>
+        /// <inheritdoc/>
         public void DispatchByScene(Scene scene)
         {
             AbstractAgent agent = null;
@@ -79,17 +70,21 @@ public void DispatchByScene(Scene scene)
 
             if (!agent)
             {
-                if (!_settings.fallbackAgent)
+                if (_settings.fallbackAgent)
+                {
+                    _logger.Log($"Use fallback agent. scene: {scene.path}");
+                    agent = _settings.fallbackAgent;
+                }
+                else
                 {
                     _logger.Log(LogType.Warning, $"Agent not found by scene: {scene.name}");
-                    return;
                 }
-
-                _logger.Log($"Use fallback agent. scene: {scene.path}");
-                agent = _settings.fallbackAgent;
             }
 
-            DispatchAgent(agent);
+            if (agent)
+            {
+                DispatchAgent(agent);
+            }
 
             if (_settings.observerAgent != null)
             {
@@ -107,7 +102,7 @@ private void DispatchAgent(AbstractAgent agent)
 
             agent.Logger = _logger;
             agent.Random = _randomFactory.CreateRandom();
-            
+
             try
             {
                 agent.Run(token).Forget();

Runtime/Autopilot.cs

@@ -5,7 +5,6 @@
 using System.Collections;
 using System.Threading;
 using Cysharp.Threading.Tasks;
-using DeNA.Anjin.Reporters;
 using DeNA.Anjin.Settings;
 using DeNA.Anjin.Utilities;
 using UnityEngine;
@@ -51,7 +50,6 @@ private void Start()
 
             _dispatcher = new AgentDispatcher(_settings, _logger, _randomFactory);
             _dispatcher.DispatchByScene(SceneManager.GetActiveScene());
-            SceneManager.activeSceneChanged += _dispatcher.DispatchByScene;
 
             if (_settings.lifespanSec > 0)
             {
@@ -92,13 +90,14 @@ private IEnumerator Lifespan(int timeoutSec)
         /// <param name="exitCode">Exit code for Unity Editor</param>
         /// <param name="logString">Log message string when terminate by the log message</param>
         /// <param name="stackTrace">Stack trace when terminate by the log message</param>
+        /// <param name="token">Cancellation token</param>
         /// <returns>A task awaits termination get completed</returns>
         public async UniTask TerminateAsync(ExitCode exitCode, string logString = null, string stackTrace = null,
             CancellationToken token = default)
         {
             if (_dispatcher != null)
             {
-                SceneManager.activeSceneChanged -= _dispatcher.DispatchByScene;
+                _dispatcher.Dispose();
             }
 
             if (_logMessageHandler != null)

Tests/Runtime/AgentDispatcherTest.cs

@@ -1,36 +1,45 @@
-// Copyright (c) 2023 DeNA Co., Ltd.
+// Copyright (c) 2023-2024 DeNA Co., Ltd.
 // This software is released under the MIT License.
 
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Threading.Tasks;
+using Cysharp.Threading.Tasks;
 using DeNA.Anjin.Agents;
 using DeNA.Anjin.Settings;
+using DeNA.Anjin.TestDoubles;
 using DeNA.Anjin.Utilities;
 using NUnit.Framework;
-using UnityEditor.SceneManagement;
 using UnityEngine;
 using UnityEngine.SceneManagement;
 using UnityEngine.TestTools;
+#if UNITY_EDITOR
+using UnityEditor.SceneManagement;
+#endif
+
+#pragma warning disable CS0618 // Type or member is obsolete
 
 namespace DeNA.Anjin
 {
     [UnityPlatform(RuntimePlatform.OSXEditor, RuntimePlatform.WindowsEditor, RuntimePlatform.LinuxEditor)]
     [SuppressMessage("ApiDesign", "RS0030")]
     public class AgentDispatcherTest
     {
+        private IAgentDispatcher _dispatcher;
+
         [SetUp]
         public void SetUp()
         {
-            Assume.That(GameObject.Find(nameof(DoNothingAgent)), Is.Null);
+            foreach (var agent in Object.FindObjectsOfType<AbstractAgent>())
+            {
+                Object.Destroy(agent);
+            }
         }
 
         [TearDown]
-        public async Task TearDown()
+        public void TearDown()
         {
-            var testAgentObject = GameObject.Find(nameof(DoNothingAgent));
-            Object.Destroy(testAgentObject);
-            await Task.Delay(100);
+            _dispatcher?.Dispose();
         }
 
         private static AutopilotSettings CreateAutopilotSettings()
@@ -40,87 +49,117 @@ private static AutopilotSettings CreateAutopilotSettings()
             return testSettings;
         }
 
-        private static DoNothingAgent CreateDoNothingAgent(string name = nameof(DoNothingAgent))
+        private static SpyAliveCountAgent CreateSpyAliveCountAgent(string name = nameof(DoNothingAgent))
         {
-            var doNothingAgent = ScriptableObject.CreateInstance<DoNothingAgent>();
-            doNothingAgent.name = name;
-            return doNothingAgent;
+            var agent = ScriptableObject.CreateInstance<SpyAliveCountAgent>();
+            agent.name = name;
+            return agent;
+        }
+
+        private void SetUpDispatcher(AutopilotSettings settings)
+        {
+            var logger = new ConsoleLogger(Debug.unityLogger.logHandler);
+            var randomFactory = new RandomFactory(0);
+
+            _dispatcher = new AgentDispatcher(settings, logger, randomFactory);
         }
 
         private const string TestScenePath = "Packages/com.dena.anjin/Tests/TestScenes/Buttons.unity";
+        private const string TestScenePath2 = "Packages/com.dena.anjin/Tests/TestScenes/Error.unity";
 
-        private static Scene LoadTestScene()
+        private static async UniTask<Scene> LoadTestSceneAsync(string path, LoadSceneMode mode = LoadSceneMode.Single)
         {
-            return EditorSceneManager.LoadSceneInPlayMode(
-                TestScenePath,
-                new LoadSceneParameters(LoadSceneMode.Single));
+            Scene scene = default;
+#if UNITY_EDITOR
+            scene = EditorSceneManager.LoadSceneInPlayMode(path, new LoadSceneParameters(mode));
+            while (!scene.isLoaded)
+            {
+                await Task.Yield();
+            }
+#endif
+            return scene;
         }
 
         [Test]
-        public void DispatchByScene_DispatchAgentBySceneAgentMaps()
+        public async Task DispatchByScene_DispatchAgentBySceneAgentMaps()
         {
-            const string ActualAgentName = nameof(DoNothingAgent);
-
+            const string AgentName = "Mapped Agent";
             var settings = CreateAutopilotSettings();
             settings.sceneAgentMaps.Add(new SceneAgentMap
             {
-                scenePath = TestScenePath, agent = CreateDoNothingAgent()
+                scenePath = TestScenePath, agent = CreateSpyAliveCountAgent(AgentName)
             });
+            SetUpDispatcher(settings);
 
-            var logger = new ConsoleLogger(Debug.unityLogger.logHandler);
-            var randomFactory = new RandomFactory(0);
-            var dispatcher = new AgentDispatcher(settings, logger, randomFactory);
-            dispatcher.DispatchByScene(LoadTestScene());
+            await LoadTestSceneAsync(TestScenePath);
 
-            var gameObject = GameObject.Find(ActualAgentName);
+            var gameObject = GameObject.Find(AgentName);
             Assert.That(gameObject, Is.Not.Null);
+            Assert.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(1));
         }
 
         [Test]
-        public void DispatchByScene_DispatchFallbackAgent()
+        public async Task DispatchByScene_DispatchFallbackAgent()
         {
-            const string ActualAgentName = "Fallback";
-
+            const string AgentName = "Fallback Agent";
             var settings = CreateAutopilotSettings();
-            settings.fallbackAgent = CreateDoNothingAgent(ActualAgentName);
+            settings.fallbackAgent = CreateSpyAliveCountAgent(AgentName);
+            SetUpDispatcher(settings);
 
-            var logger = new ConsoleLogger(Debug.unityLogger.logHandler);
-            var randomFactory = new RandomFactory(0);
-            var dispatcher = new AgentDispatcher(settings, logger, randomFactory);
-            dispatcher.DispatchByScene(LoadTestScene());
+            await LoadTestSceneAsync(TestScenePath);
 
-            var gameObject = GameObject.Find(ActualAgentName);
+            var gameObject = GameObject.Find(AgentName);
             Assert.That(gameObject, Is.Not.Null);
+            Assert.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(1));
         }
 
         [Test]
-        public void DispatchByScene_NoSceneAgentMapsAndFallbackAgent_AgentIsNotDispatch()
+        public async Task DispatchByScene_NoSceneAgentMapsAndFallbackAgent_AgentIsNotDispatch()
         {
             var settings = CreateAutopilotSettings();
-            var logger = new ConsoleLogger(Debug.unityLogger.logHandler);
-            var randomFactory = new RandomFactory(0);
-            var dispatcher = new AgentDispatcher(settings, logger, randomFactory);
-            dispatcher.DispatchByScene(LoadTestScene());
+            SetUpDispatcher(settings);
+
+            await LoadTestSceneAsync(TestScenePath);
 
             LogAssert.Expect(LogType.Warning, "Agent not found by scene: Buttons");
+            Assert.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(0));
         }
 
         [Test]
-        public void DispatchByScene_DispatchObserverAgent()
+        public async Task DispatchByScene_DispatchObserverAgent()
         {
-            const string ActualAgentName = "Observer";
-
+            const string AgentName = "Observer Agent";
             var settings = CreateAutopilotSettings();
-            settings.fallbackAgent = CreateDoNothingAgent();
-            settings.observerAgent = CreateDoNothingAgent(ActualAgentName);
+            settings.observerAgent = CreateSpyAliveCountAgent(AgentName);
+            SetUpDispatcher(settings);
 
-            var logger = new ConsoleLogger(Debug.unityLogger.logHandler);
-            var randomFactory = new RandomFactory(0);
-            var dispatcher = new AgentDispatcher(settings, logger, randomFactory);
-            dispatcher.DispatchByScene(LoadTestScene());
+            await LoadTestSceneAsync(TestScenePath);
 
-            var gameObject = GameObject.Find(ActualAgentName);
+            var gameObject = GameObject.Find(AgentName);
             Assert.That(gameObject, Is.Not.Null);
+            Assert.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(1));
+        }
+
+        [Test]
+        public async Task DispatchByScene_ReActivateScene_NotCreateDuplicateAgents()
+        {
+            const string AgentName = "Mapped Agent";
+            var settings = CreateAutopilotSettings();
+            settings.sceneAgentMaps.Add(new SceneAgentMap
+            {
+                scenePath = TestScenePath, agent = CreateSpyAliveCountAgent(AgentName)
+            });
+            SetUpDispatcher(settings);
+
+            var scene = await LoadTestSceneAsync(TestScenePath);
+            Assume.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(1));
+
+            var additiveScene = await LoadTestSceneAsync(TestScenePath2, LoadSceneMode.Additive);
+            SceneManager.SetActiveScene(additiveScene);
+
+            SceneManager.SetActiveScene(scene); // Re-activate
+
+            Assert.That(SpyAliveCountAgent.AliveInstances, Is.EqualTo(1)); // Not create duplicate agents
         }
     }
 }

Tests/Runtime/TestDoubles/SpyAliveCountAgent.cs

@@ -0,0 +1,33 @@
+// Copyright (c) 2023-2024 DeNA Co., Ltd.
+// This software is released under the MIT License.
+
+using System.Threading;
+using Cysharp.Threading.Tasks;
+using DeNA.Anjin.Agents;
+using UnityEngine;
+
+namespace DeNA.Anjin.TestDoubles
+{
+    /// <summary>
+    /// Spy agent that count the number of alive instances.
+    /// </summary>
+    [AddComponentMenu("")]
+    // ReSharper disable once RequiredBaseTypesIsNotInherited
+    public class SpyAliveCountAgent : AbstractAgent
+    {
+        public static int AliveInstances { get; private set; }
+
+        public override async UniTask Run(CancellationToken token)
+        {
+            AliveInstances++;
+            try
+            {
+                await UniTask.WaitWhile(() => true, cancellationToken: token); // Wait indefinitely
+            }
+            finally
+            {
+                AliveInstances--;
+            }
+        }
+    }
+}