Merge pull request #16895 from bdach/better-new-difficulty-naming

Name newly created difficulties in a better way

osu.Game.Tests/Utils/NamingUtilsTest.cs

@@ -0,0 +1,132 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using NUnit.Framework;
+using osu.Game.Utils;
+
+namespace osu.Game.Tests.Utils
+{
+    [TestFixture]
+    public class NamingUtilsTest
+    {
+        [Test]
+        public void TestEmptySet()
+        {
+            string nextBestName = NamingUtils.GetNextBestName(Enumerable.Empty<string>(), "New Difficulty");
+
+            Assert.AreEqual("New Difficulty", nextBestName);
+        }
+
+        [Test]
+        public void TestNotTaken()
+        {
+            string[] existingNames =
+            {
+                "Something",
+                "Entirely",
+                "Different"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty", nextBestName);
+        }
+
+        [Test]
+        public void TestNotTakenButClose()
+        {
+            string[] existingNames =
+            {
+                "New Difficulty(1)",
+                "New Difficulty (abcd)",
+                "New Difficulty but not really"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty", nextBestName);
+        }
+
+        [Test]
+        public void TestAlreadyTaken()
+        {
+            string[] existingNames =
+            {
+                "New Difficulty"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty (1)", nextBestName);
+        }
+
+        [Test]
+        public void TestAlreadyTakenWithDifferentCase()
+        {
+            string[] existingNames =
+            {
+                "new difficulty"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty (1)", nextBestName);
+        }
+
+        [Test]
+        public void TestAlreadyTakenWithBrackets()
+        {
+            string[] existingNames =
+            {
+                "new difficulty (copy)"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty (copy)");
+
+            Assert.AreEqual("New Difficulty (copy) (1)", nextBestName);
+        }
+
+        [Test]
+        public void TestMultipleAlreadyTaken()
+        {
+            string[] existingNames =
+            {
+                "New Difficulty",
+                "New difficulty (1)",
+                "new Difficulty (2)",
+                "New DIFFICULTY (3)"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty (4)", nextBestName);
+        }
+
+        [Test]
+        public void TestEvenMoreAlreadyTaken()
+        {
+            string[] existingNames = Enumerable.Range(1, 30).Select(i => $"New Difficulty ({i})").Append("New Difficulty").ToArray();
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty (31)", nextBestName);
+        }
+
+        [Test]
+        public void TestMultipleAlreadyTakenWithGaps()
+        {
+            string[] existingNames =
+            {
+                "New Difficulty",
+                "New Difficulty (1)",
+                "New Difficulty (4)",
+                "New Difficulty (9)"
+            };
+
+            string nextBestName = NamingUtils.GetNextBestName(existingNames, "New Difficulty");
+
+            Assert.AreEqual("New Difficulty (2)", nextBestName);
+        }
+    }
+}

osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs

@@ -269,11 +269,12 @@ public void TestCopyDifficulty()
         }
 
         [Test]
-        public void TestCreateNewBeatmapFailsWithBlankNamedDifficulties()
+        public void TestCreateMultipleNewDifficultiesSucceeds()
         {
             Guid setId = Guid.Empty;
 
             AddStep("retrieve set ID", () => setId = EditorBeatmap.BeatmapInfo.BeatmapSet!.ID);
+            AddStep("set difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = "New Difficulty");
             AddStep("save beatmap", () => Editor.Save());
             AddAssert("new beatmap persisted", () =>
             {
@@ -282,15 +283,24 @@ public void TestCreateNewBeatmapFailsWithBlankNamedDifficulties()
             });
 
             AddStep("try to create new difficulty", () => Editor.CreateNewDifficulty(new OsuRuleset().RulesetInfo));
-            AddAssert("beatmap set unchanged", () =>
+            AddUntilStep("wait for dialog", () => DialogOverlay.CurrentDialog is CreateNewDifficultyDialog);
+            AddStep("confirm creation with no objects", () => DialogOverlay.CurrentDialog.PerformOkAction());
+
+            AddUntilStep("wait for created", () =>
+            {
+                string difficultyName = Editor.ChildrenOfType<EditorBeatmap>().SingleOrDefault()?.BeatmapInfo.DifficultyName;
+                return difficultyName != null && difficultyName != "New Difficulty";
+            });
+            AddAssert("new difficulty has correct name", () => EditorBeatmap.BeatmapInfo.DifficultyName == "New Difficulty (1)");
+            AddAssert("new difficulty persisted", () =>
             {
                 var set = beatmapManager.QueryBeatmapSet(s => s.ID == setId);
-                return set != null && set.PerformRead(s => s.Beatmaps.Count == 1 && s.Files.Count == 1);
+                return set != null && set.PerformRead(s => s.Beatmaps.Count == 2 && s.Files.Count == 2);
             });
         }
 
         [Test]
-        public void TestCreateNewBeatmapFailsWithSameNamedDifficulties([Values] bool sameRuleset)
+        public void TestSavingBeatmapFailsWithSameNamedDifficulties([Values] bool sameRuleset)
         {
             Guid setId = Guid.Empty;
             const string duplicate_difficulty_name = "duplicate";

osu.Game/Beatmaps/BeatmapManager.cs

@@ -22,6 +22,7 @@
 using osu.Game.Rulesets;
 using osu.Game.Skinning;
 using osu.Game.Stores;
+using osu.Game.Utils;
 
 #nullable enable
 
@@ -123,7 +124,10 @@ public virtual WorkingBeatmap CreateNewDifficulty(BeatmapSetInfo targetBeatmapSe
         {
             var playableBeatmap = referenceWorkingBeatmap.GetPlayableBeatmap(rulesetInfo);
 
-            var newBeatmapInfo = new BeatmapInfo(rulesetInfo, new BeatmapDifficulty(), playableBeatmap.Metadata.DeepClone());
+            var newBeatmapInfo = new BeatmapInfo(rulesetInfo, new BeatmapDifficulty(), playableBeatmap.Metadata.DeepClone())
+            {
+                DifficultyName = NamingUtils.GetNextBestName(targetBeatmapSet.Beatmaps.Select(b => b.DifficultyName), "New Difficulty")
+            };
             var newBeatmap = new Beatmap { BeatmapInfo = newBeatmapInfo };
             foreach (var timingPoint in playableBeatmap.ControlPointInfo.TimingPoints)
                 newBeatmap.ControlPointInfo.Add(timingPoint.Time, timingPoint.DeepClone());
@@ -150,8 +154,10 @@ public virtual WorkingBeatmap CopyExistingDifficulty(BeatmapSetInfo targetBeatma
             newBeatmap.BeatmapInfo = newBeatmapInfo = referenceWorkingBeatmap.BeatmapInfo.Clone();
             // assign a new ID to the clone.
             newBeatmapInfo.ID = Guid.NewGuid();
-            // add "(copy)" suffix to difficulty name to avoid clashes on save.
-            newBeatmapInfo.DifficultyName += " (copy)";
+            // add "(copy)" suffix to difficulty name, and additionally ensure that it doesn't conflict with any other potentially pre-existing copies.
+            newBeatmapInfo.DifficultyName = NamingUtils.GetNextBestName(
+                targetBeatmapSet.Beatmaps.Select(b => b.DifficultyName),
+                $"{newBeatmapInfo.DifficultyName} (copy)");
             // clear the hash, as that's what is used to match .osu files with their corresponding realm beatmaps.
             newBeatmapInfo.Hash = string.Empty;
             // clear online properties.

osu.Game/Utils/NamingUtils.cs

@@ -0,0 +1,61 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Collections.Generic;
+using System.Text.RegularExpressions;
+
+namespace osu.Game.Utils
+{
+    public static class NamingUtils
+    {
+        /// <summary>
+        /// Given a set of <paramref name="existingNames"/> and a target <paramref name="desiredName"/>,
+        /// finds a "best" name closest to <paramref name="desiredName"/> that is not in <paramref name="existingNames"/>.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// This helper is most useful in scenarios when creating new objects in a set
+        /// (such as adding new difficulties to a beatmap set, or creating a clone of an existing object that needs a unique name).
+        /// If <paramref name="desiredName"/> is already present in <paramref name="existingNames"/>,
+        /// this method will append the lowest possible number in brackets that doesn't conflict with <paramref name="existingNames"/>
+        /// to <paramref name="desiredName"/> and return that.
+        /// See <c>osu.Game.Tests.Utils.NamingUtilsTest</c> for concrete examples of behaviour.
+        /// </para>
+        /// <para>
+        /// <paramref name="desiredName"/> and <paramref name="existingNames"/> are compared in a case-insensitive manner,
+        /// so this method is safe to use for naming files in a platform-invariant manner.
+        /// </para>
+        /// </remarks>
+        public static string GetNextBestName(IEnumerable<string> existingNames, string desiredName)
+        {
+            string pattern = $@"^(?i){Regex.Escape(desiredName)}(?-i)( \((?<copyNumber>[1-9][0-9]*)\))?$";
+            var regex = new Regex(pattern, RegexOptions.Compiled);
+            var takenNumbers = new HashSet<int>();
+
+            foreach (string name in existingNames)
+            {
+                var match = regex.Match(name);
+                if (!match.Success)
+                    continue;
+
+                string copyNumberString = match.Groups[@"copyNumber"].Value;
+
+                if (string.IsNullOrEmpty(copyNumberString))
+                {
+                    takenNumbers.Add(0);
+                    continue;
+                }
+
+                takenNumbers.Add(int.Parse(copyNumberString));
+            }
+
+            int bestNumber = 0;
+            while (takenNumbers.Contains(bestNumber))
+                bestNumber += 1;
+
+            return bestNumber == 0
+                ? desiredName
+                : $"{desiredName} ({bestNumber})";
+        }
+    }
+}