From 47a732ef604ac2f675ef71683d69eb10930d4785 Mon Sep 17 00:00:00 2001
From: Shivam <s.r.v.ramkisoen@umail.leidenuniv.nl>
Date: Wed, 24 Jun 2020 23:01:56 +0200
Subject: [PATCH] Address review comments

Now asserting instead of an if-statement, change cast from OsuStorage to MigratableStorage and make internal virtual properties protected.
---
 .../NonVisual/CustomDataDirectoryTest.cs      | 25 +++++++++----------
 osu.Game/IO/MigratableStorage.cs              |  4 +--
 osu.Game/IO/OsuStorage.cs                     |  4 +--
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs
index 4149e3d3ef..43c1c77786 100644
--- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs
+++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs
@@ -126,7 +126,7 @@ namespace osu.Game.Tests.NonVisual
                 {
                     var osu = loadOsu(host);
                     var storage = osu.Dependencies.Get<Storage>();
-                    var osuStorage = storage as OsuStorage;
+                    var osuStorage = storage as MigratableStorage;
 
                     // ensure we perform a save
                     host.Dependencies.Get<FrameworkConfigManager>().Save();
@@ -152,19 +152,18 @@ namespace osu.Game.Tests.NonVisual
                     Assert.That(!host.Storage.ExistsDirectory(Path.Combine("test-nested", "cache")));
                     Assert.That(storage.ExistsDirectory(Path.Combine("test-nested", "cache")));
 
-                    if (osuStorage != null)
-                    {
-                        foreach (var file in osuStorage.IgnoreFiles)
-                        {
-                            Assert.That(host.Storage.Exists(file), Is.True);
-                            Assert.That(storage.Exists(file), Is.False);
-                        }
+                    Assert.That(osuStorage, Is.Not.Null);
 
-                        foreach (var dir in osuStorage.IgnoreDirectories)
-                        {
-                            Assert.That(host.Storage.ExistsDirectory(dir), Is.True);
-                            Assert.That(storage.ExistsDirectory(dir), Is.False);
-                        }
+                    foreach (var file in osuStorage.IgnoreFiles)
+                    {
+                        Assert.That(host.Storage.Exists(file), Is.True);
+                        Assert.That(storage.Exists(file), Is.False);
+                    }
+
+                    foreach (var dir in osuStorage.IgnoreDirectories)
+                    {
+                        Assert.That(host.Storage.ExistsDirectory(dir), Is.True);
+                        Assert.That(storage.ExistsDirectory(dir), Is.False);
                     }
 
                     Assert.That(new StreamReader(host.Storage.GetStream("storage.ini")).ReadToEnd().Contains($"FullPath = {customPath}"));
diff --git a/osu.Game/IO/MigratableStorage.cs b/osu.Game/IO/MigratableStorage.cs
index ec85e0bac9..faa39d2ef8 100644
--- a/osu.Game/IO/MigratableStorage.cs
+++ b/osu.Game/IO/MigratableStorage.cs
@@ -14,8 +14,8 @@ namespace osu.Game.IO
     /// </summary>
     public abstract class MigratableStorage : WrappedStorage
     {
-        internal virtual string[] IgnoreDirectories => Array.Empty<string>();
-        internal virtual string[] IgnoreFiles => Array.Empty<string>();
+        public virtual string[] IgnoreDirectories => Array.Empty<string>();
+        public virtual string[] IgnoreFiles => Array.Empty<string>();
 
         protected MigratableStorage(Storage storage, string subPath = null)
             : base(storage, subPath)
diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs
index 514f172f74..31ee802141 100644
--- a/osu.Game/IO/OsuStorage.cs
+++ b/osu.Game/IO/OsuStorage.cs
@@ -14,9 +14,9 @@ namespace osu.Game.IO
         private readonly GameHost host;
         private readonly StorageConfigManager storageConfig;
 
-        internal override string[] IgnoreDirectories => new[] { "cache" };
+        public override string[] IgnoreDirectories => new[] { "cache" };
 
-        internal override string[] IgnoreFiles => new[]
+        public override string[] IgnoreFiles => new[]
         {
             "framework.ini",
             "storage.ini"