Merge pull request #99 from nowsprinting/fix/convert_obsolete_fields

Fix obsolete fields convert spec

Runtime/Autopilot.cs

@@ -134,9 +134,9 @@ public async UniTask TerminateAsync(ExitCode exitCode, string message = null, st
 
             _isTerminating = true;
 
-            if (reporting && _state.IsRunning && _settings.reporter != null)
+            if (reporting && _state.IsRunning && _settings.Reporter != null)
             {
-                await _settings.reporter.PostReportAsync(message, stackTrace, exitCode, token);
+                await _settings.Reporter.PostReportAsync(message, stackTrace, exitCode, token);
             }
 
             if (_state.settings != null && !string.IsNullOrEmpty(_state.settings.junitReportPath))

Runtime/Settings/AutopilotSettings.cs

@@ -3,13 +3,18 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using DeNA.Anjin.Agents;
 using DeNA.Anjin.Attributes;
 using DeNA.Anjin.Loggers;
 using DeNA.Anjin.Reporters;
 using UnityEngine;
 using UnityEngine.Assertions;
+using Object = UnityEngine.Object;
+#if UNITY_EDITOR
+using UnityEditor;
+#endif
 
 namespace DeNA.Anjin.Settings
 {
@@ -132,7 +137,7 @@ public class AutopilotSettings : ScriptableObject
         /// <summary>
         /// Logger used for this autopilot settings.
         /// </summary>
-        [Obsolete("Use `loggerAssets` field or `LoggerAsset` property instead")]
+        [Obsolete("Use `LoggerAsset` property instead")]
         public AbstractLoggerAsset loggerAsset;
 
         /// <summary>
@@ -163,7 +168,7 @@ public CompositeLoggerAsset LoggerAsset
         /// <summary>
         /// Reporter that called when some errors occurred
         /// </summary>
-        [Obsolete("Use `reporters` field  or `Reporter` property instead")]
+        [Obsolete("Use `Reporter` property instead")]
         public AbstractReporter reporter;
 
         /// <summary>
@@ -259,6 +264,9 @@ private void CreateDefaultLoggerIfNeeded()
                 // not change field directly.
 
                 this.LoggerAsset.Logger.Log("Create default logger.");
+#if UNITY_EDITOR
+                EditorUtility.SetDirty(this);
+#endif
             }
         }
 
@@ -274,8 +282,7 @@ internal void ConvertLoggersFromObsoleteLogger()
 Please delete the reference using Debug Mode in the Inspector window. And add to the list Loggers.
 This time, temporarily converting.");
 
-            this.LoggerAsset.loggerAssets = new List<AbstractLoggerAsset> { this.loggerAsset };
-            // not change field directly.
+            this.loggerAssets.Add(this.loggerAsset);
         }
 
         [Obsolete("Remove this method when bump major version")]
@@ -290,29 +297,53 @@ internal void ConvertReportersFromObsoleteReporter(ILogger logger)
 Please delete the reference using Debug Mode in the Inspector window. And add to the list Reporters.
 This time, temporarily converting.");
 
-            this.Reporter.reporters = new List<AbstractReporter> { this.reporter };
-            // not change field directly.
+            this.reporters.Add(this.reporter);
+#if UNITY_EDITOR
+            EditorUtility.SetDirty(this);
+#endif
         }
 
         [Obsolete("Remove this method when bump major version")]
         internal void ConvertSlackReporterFromObsoleteSlackSettings(ILogger logger)
         {
             if (string.IsNullOrEmpty(this.slackToken) || string.IsNullOrEmpty(this.slackChannels) ||
-                this.reporters.Any())
+                this.reporters.Any(x => x.GetType() == typeof(SlackReporter)))
             {
                 return;
             }
 
-            logger.Log(LogType.Warning, @"Slack settings in AutopilotSettings has been obsolete.
+            const string AutoConvertingMessage = @"Slack settings in AutopilotSettings has been obsolete.
 Please delete the value using Debug Mode in the Inspector window. And create a SlackReporter asset file.
-This time, temporarily generate and use SlackReporter instance.");
+This time, temporarily generate and use SlackReporter instance.";
+            logger.Log(LogType.Warning, AutoConvertingMessage);
 
             var convertedReporter = CreateInstance<SlackReporter>();
             convertedReporter.slackToken = this.slackToken;
             convertedReporter.slackChannels = this.slackChannels;
             convertedReporter.mentionSubTeamIDs = this.mentionSubTeamIDs;
             convertedReporter.addHereInSlackMessage = this.addHereInSlackMessage;
+#if UNITY_EDITOR
+            convertedReporter.description = AutoConvertingMessage;
+            SaveConvertedObject(convertedReporter);
+#endif
             this.reporters.Add(convertedReporter);
+#if UNITY_EDITOR
+            EditorUtility.SetDirty(this);
+#endif
+        }
+
+        private void SaveConvertedObject(Object obj)
+        {
+#if UNITY_EDITOR
+            var settingsPath = AssetDatabase.GetAssetPath(this);
+            if (string.IsNullOrEmpty(settingsPath))
+            {
+                return;
+            }
+
+            var dir = Path.GetDirectoryName(settingsPath) ?? "Assets";
+            AssetDatabase.CreateAsset(obj, Path.Combine(dir, $"New {obj.GetType().Name}.asset"));
+#endif
         }
     }
 }

Tests/Runtime/Settings/AutopilotSettingsTest.cs

@@ -6,7 +6,6 @@
 using DeNA.Anjin.TestDoubles;
 using NUnit.Framework;
 using UnityEngine;
-using UnityEngine.TestTools;
 
 namespace DeNA.Anjin.Settings
 {
@@ -98,9 +97,8 @@ public void ConvertLoggersFromObsoleteLogger_HasLogger_IncludeToLoggers()
             settings.loggerAsset = legacyLogger; // already exists
 
             settings.ConvertLoggersFromObsoleteLogger();
-            Assert.That(settings.loggerAssets.Count, Is.EqualTo(0)); // Not added directly to the field
-            Assert.That(settings.LoggerAsset.loggerAssets.Count, Is.EqualTo(1));
-            Assert.That(settings.LoggerAsset.loggerAssets, Has.Member(legacyLogger));
+            Assert.That(settings.loggerAssets.Count, Is.EqualTo(1));
+            Assert.That(settings.loggerAssets, Has.Member(legacyLogger));
 
             Assert.That(legacyLogger.Logs, Has.Member((LogType.Warning,
                 @"Single Logger setting in AutopilotSettings has been obsolete.
@@ -139,9 +137,8 @@ public void ConvertReportersFromObsoleteReporter_HasReporter_IncludeToReporters(
 
             var spyLogger = ScriptableObject.CreateInstance<SpyLoggerAsset>();
             settings.ConvertReportersFromObsoleteReporter(spyLogger.Logger);
-            Assert.That(settings.reporters.Count, Is.EqualTo(0)); // Not added directly to the field
-            Assert.That(settings.Reporter.reporters.Count, Is.EqualTo(1));
-            Assert.That(settings.Reporter.reporters, Has.Member(legacyReporter));
+            Assert.That(settings.reporters.Count, Is.EqualTo(1));
+            Assert.That(settings.reporters, Has.Member(legacyReporter));
 
             Assert.That(spyLogger.Logs, Has.Member((LogType.Warning,
                 @"Single Reporter setting in AutopilotSettings has been obsolete.
@@ -222,14 +219,15 @@ public void ConvertSlackReporterFromObsoleteSlackSettings_HasNotSlackChannels_No
         [Test]
         public void ConvertSlackReporterFromObsoleteSlackSettings_ExistReporter_NotGenerateSlackReporter()
         {
+            var existReporter = ScriptableObject.CreateInstance<SlackReporter>();
             var settings = ScriptableObject.CreateInstance<AutopilotSettings>();
-            settings.reporters.Add(ScriptableObject.CreateInstance<CompositeReporter>()); // already exists
+            settings.reporters.Add(existReporter); // already exists
             settings.slackToken = "token";
             settings.slackChannels = "channels";
 
             settings.ConvertSlackReporterFromObsoleteSlackSettings(Debug.unityLogger);
             Assert.That(settings.reporters.Count, Is.EqualTo(1));
-            Assert.That(settings.reporters, Has.No.InstanceOf<SlackReporter>());
+            Assert.That(settings.reporters, Does.Contain(existReporter));
         }
     }
 }