Core - Fall back to the default value passed to GetSetting when a stored setting can't be read

src/Artemis.Core.Tests/Services/SettingsServiceTests.cs

@@ -75,15 +75,13 @@ public class SettingsServiceTests : IDisposable
     }
 
     [Fact]
-    public void GetSetting_AStoredValueThatDoesNotFitTheType_UsesTheDefaultOfTheType()
+    public void GetSetting_AStoredValueThatDoesNotFitTheType_UsesTheDefaultValuePassedIn()
     {
         Settings.GetSetting("Tests.Value", "not a number");
 
-        // Pinned as-is: when the stored value can't be read as the requested type the setting falls back to default(T)
-        // rather than to the default value passed in, so the caller's default of 7 is ignored here
         PluginSetting<int> setting = CreateNewSettingsService().GetSetting("Tests.Value", 7);
 
-        setting.Value.ShouldBe(0);
+        setting.Value.ShouldBe(7);
         setting.HasChanged.ShouldBeTrue();
     }
 

src/Artemis.Core/Plugins/Settings/PluginSetting.cs

@@ -15,7 +15,7 @@ public class PluginSetting<T> : CorePropertyChanged, IPluginSetting
     private readonly PluginSettingEntity _pluginSettingEntity;
     private T _value;
 
-    internal PluginSetting(IPluginRepository pluginRepository, PluginSettingEntity pluginSettingEntity)
+    internal PluginSetting(IPluginRepository pluginRepository, PluginSettingEntity pluginSettingEntity, T? defaultValue = default)
     {
         _pluginRepository = pluginRepository;
         _pluginSettingEntity = pluginSettingEntity;
@@ -27,7 +27,7 @@ public class PluginSetting<T> : CorePropertyChanged, IPluginSetting
         }
         catch (JsonException)
         {
-            _value = default!;
+            _value = defaultValue!;
         }
     }
 

src/Artemis.Core/Plugins/Settings/PluginSettings.cs

@@ -32,7 +32,7 @@ public class PluginSettings
     /// </summary>
     /// <typeparam name="T">The type of the setting, can be any serializable type</typeparam>
     /// <param name="name">The name of the setting, may not be longer than 128 characters</param>
-    /// <param name="defaultValue">The default value to use if the setting does not exist yet</param>
+    /// <param name="defaultValue">The default value to use if the setting does not exist yet, or can't be read as <typeparamref name="T" /></param>
     public PluginSetting<T> GetSetting<T>(string name, T? defaultValue = default)
     {
         if (name.Length > 128)
@@ -57,7 +57,7 @@ public class PluginSettings
                 _pluginRepository.SaveSetting(settingEntity);
             }
 
-            PluginSetting<T> pluginSetting = new(_pluginRepository, settingEntity);
+            PluginSetting<T> pluginSetting = new(_pluginRepository, settingEntity, defaultValue);
 
             // This overrides null with the default value, I'm not sure if that's desirable because you
             // might expect something to go null and you might not