Merge pull request #16754 from bdach/new-difficulty-creation-v3

Add ability to create new difficulties from within the editor

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

@@ -90,5 +90,100 @@ namespace osu.Game.Tests.Visual.Editing
 
             AddAssert("track length changed", () => Beatmap.Value.Track.Length > 60000);
         }
+
+        [Test]
+        public void TestCreateNewDifficulty()
+        {
+            string firstDifficultyName = Guid.NewGuid().ToString();
+            string secondDifficultyName = Guid.NewGuid().ToString();
+
+            AddStep("set unique difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = firstDifficultyName);
+            AddStep("save beatmap", () => Editor.Save());
+            AddAssert("new beatmap persisted", () =>
+            {
+                var beatmap = beatmapManager.QueryBeatmap(b => b.DifficultyName == firstDifficultyName);
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == EditorBeatmap.BeatmapInfo.BeatmapSet.ID);
+
+                return beatmap != null
+                       && beatmap.DifficultyName == firstDifficultyName
+                       && set != null
+                       && set.PerformRead(s => s.Beatmaps.Single().ID == beatmap.ID);
+            });
+            AddAssert("can save again", () => Editor.Save());
+
+            AddStep("create new difficulty", () => Editor.CreateNewDifficulty(new OsuRuleset().RulesetInfo));
+            AddUntilStep("wait for created", () =>
+            {
+                string difficultyName = Editor.ChildrenOfType<EditorBeatmap>().SingleOrDefault()?.BeatmapInfo.DifficultyName;
+                return difficultyName != null && difficultyName != firstDifficultyName;
+            });
+
+            AddStep("set unique difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = secondDifficultyName);
+            AddStep("save beatmap", () => Editor.Save());
+            AddAssert("new beatmap persisted", () =>
+            {
+                var beatmap = beatmapManager.QueryBeatmap(b => b.DifficultyName == secondDifficultyName);
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == EditorBeatmap.BeatmapInfo.BeatmapSet.ID);
+
+                return beatmap != null
+                       && beatmap.DifficultyName == secondDifficultyName
+                       && set != null
+                       && set.PerformRead(s => s.Beatmaps.Count == 2 && s.Beatmaps.Any(b => b.DifficultyName == secondDifficultyName));
+            });
+        }
+
+        [Test]
+        public void TestCreateNewBeatmapFailsWithBlankNamedDifficulties()
+        {
+            Guid setId = Guid.Empty;
+
+            AddStep("retrieve set ID", () => setId = EditorBeatmap.BeatmapInfo.BeatmapSet!.ID);
+            AddStep("save beatmap", () => Editor.Save());
+            AddAssert("new beatmap persisted", () =>
+            {
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == setId);
+                return set != null && set.PerformRead(s => s.Beatmaps.Count == 1 && s.Files.Count == 1);
+            });
+
+            AddStep("try to create new difficulty", () => Editor.CreateNewDifficulty(new OsuRuleset().RulesetInfo));
+            AddAssert("beatmap set unchanged", () =>
+            {
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == setId);
+                return set != null && set.PerformRead(s => s.Beatmaps.Count == 1 && s.Files.Count == 1);
+            });
+        }
+
+        [Test]
+        public void TestCreateNewBeatmapFailsWithSameNamedDifficulties()
+        {
+            Guid setId = Guid.Empty;
+            const string duplicate_difficulty_name = "duplicate";
+
+            AddStep("retrieve set ID", () => setId = EditorBeatmap.BeatmapInfo.BeatmapSet!.ID);
+            AddStep("set difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = duplicate_difficulty_name);
+            AddStep("save beatmap", () => Editor.Save());
+            AddAssert("new beatmap persisted", () =>
+            {
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == setId);
+                return set != null && set.PerformRead(s => s.Beatmaps.Count == 1 && s.Files.Count == 1);
+            });
+
+            AddStep("create new difficulty", () => Editor.CreateNewDifficulty(new OsuRuleset().RulesetInfo));
+            AddUntilStep("wait for created", () =>
+            {
+                string difficultyName = Editor.ChildrenOfType<EditorBeatmap>().SingleOrDefault()?.BeatmapInfo.DifficultyName;
+                return difficultyName != null && difficultyName != duplicate_difficulty_name;
+            });
+
+            AddStep("set difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = duplicate_difficulty_name);
+            AddStep("try to save beatmap", () => Editor.Save());
+            AddAssert("beatmap set not corrupted", () =>
+            {
+                var set = beatmapManager.QueryBeatmapSet(s => s.ID == setId);
+                // the difficulty was already created at the point of the switch.
+                // what we want to check is that both difficulties do not use the same file.
+                return set != null && set.PerformRead(s => s.Beatmaps.Count == 2 && s.Files.Count == 2);
+            });
+        }
     }
 }

osu.Game/Beatmaps/BeatmapManager.cs

@@ -73,7 +73,9 @@ namespace osu.Game.Beatmaps
             new BeatmapModelManager(realm, storage, onlineLookupQueue);
 
         /// <summary>
-        /// Create a new <see cref="WorkingBeatmap"/>.
+        /// Create a new beatmap set, backed by a <see cref="BeatmapSetInfo"/> model,
+        /// with a single difficulty which is backed by a <see cref="BeatmapInfo"/> model
+        /// and represented by the returned usable <see cref="WorkingBeatmap"/>.
         /// </summary>
         public WorkingBeatmap CreateNew(RulesetInfo ruleset, APIUser user)
         {
@@ -105,6 +107,40 @@ namespace osu.Game.Beatmaps
             return imported.PerformRead(s => GetWorkingBeatmap(s.Beatmaps.First()));
         }
 
+        /// <summary>
+        /// Add a new difficulty to the beatmap set represented by the provided <see cref="BeatmapSetInfo"/>.
+        /// The new difficulty will be backed by a <see cref="BeatmapInfo"/> model
+        /// and represented by the returned <see cref="WorkingBeatmap"/>.
+        /// </summary>
+        public virtual WorkingBeatmap CreateNewBlankDifficulty(BeatmapSetInfo beatmapSetInfo, RulesetInfo rulesetInfo)
+        {
+            // fetch one of the existing difficulties to copy timing points and metadata from,
+            // so that the user doesn't have to fill all of that out again.
+            // this silently assumes that all difficulties have the same timing points and metadata,
+            // but cases where this isn't true seem rather rare / pathological.
+            var referenceBeatmap = GetWorkingBeatmap(beatmapSetInfo.Beatmaps.First());
+
+            var newBeatmapInfo = new BeatmapInfo(rulesetInfo, new BeatmapDifficulty(), referenceBeatmap.Metadata.DeepClone());
+
+            // populate circular beatmap set info <-> beatmap info references manually.
+            // several places like `BeatmapModelManager.Save()` or `GetWorkingBeatmap()`
+            // rely on them being freely traversable in both directions for correct operation.
+            beatmapSetInfo.Beatmaps.Add(newBeatmapInfo);
+            newBeatmapInfo.BeatmapSet = beatmapSetInfo;
+
+            var newBeatmap = new Beatmap { BeatmapInfo = newBeatmapInfo };
+            foreach (var timingPoint in referenceBeatmap.Beatmap.ControlPointInfo.TimingPoints)
+                newBeatmap.ControlPointInfo.Add(timingPoint.Time, timingPoint.DeepClone());
+
+            beatmapModelManager.Save(newBeatmapInfo, newBeatmap);
+
+            workingBeatmapCache.Invalidate(beatmapSetInfo);
+            return GetWorkingBeatmap(newBeatmap.BeatmapInfo);
+        }
+
+        // TODO: add back support for making a copy of another difficulty
+        // (likely via a separate `CopyDifficulty()` method).
+
         /// <summary>
         /// Delete a beatmap difficulty.
         /// </summary>

osu.Game/Beatmaps/BeatmapMetadata.cs

@@ -7,6 +7,7 @@ using Newtonsoft.Json;
 using osu.Framework.Testing;
 using osu.Game.Models;
 using osu.Game.Users;
+using osu.Game.Utils;
 using Realms;
 
 #nullable enable
@@ -16,7 +17,7 @@ namespace osu.Game.Beatmaps
     [ExcludeFromDynamicCompile]
     [Serializable]
     [MapTo("BeatmapMetadata")]
-    public class BeatmapMetadata : RealmObject, IBeatmapMetadataInfo
+    public class BeatmapMetadata : RealmObject, IBeatmapMetadataInfo, IDeepCloneable<BeatmapMetadata>
     {
         public string Title { get; set; } = string.Empty;
 
@@ -57,5 +58,18 @@ namespace osu.Game.Beatmaps
         IUser IBeatmapMetadataInfo.Author => Author;
 
         public override string ToString() => this.GetDisplayTitle();
+
+        public BeatmapMetadata DeepClone() => new BeatmapMetadata(Author.DeepClone())
+        {
+            Title = Title,
+            TitleUnicode = TitleUnicode,
+            Artist = Artist,
+            ArtistUnicode = ArtistUnicode,
+            Source = Source,
+            Tags = Tags,
+            PreviewTime = PreviewTime,
+            AudioFile = AudioFile,
+            BackgroundFile = BackgroundFile
+        };
     }
 }

osu.Game/Beatmaps/BeatmapModelManager.cs

@@ -46,10 +46,9 @@ namespace osu.Game.Beatmaps
         /// <param name="beatmapInfo">The <see cref="BeatmapInfo"/> to save the content against. The file referenced by <see cref="BeatmapInfo.Path"/> will be replaced.</param>
         /// <param name="beatmapContent">The <see cref="IBeatmap"/> content to write.</param>
         /// <param name="beatmapSkin">The beatmap <see cref="ISkin"/> content to write, null if to be omitted.</param>
-        public virtual void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null)
+        public void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null)
         {
             var setInfo = beatmapInfo.BeatmapSet;
-
             Debug.Assert(setInfo != null);
 
             // Difficulty settings must be copied first due to the clone in `Beatmap<>.BeatmapInfo_Set`.
@@ -72,6 +71,12 @@ namespace osu.Game.Beatmaps
 
                 // AddFile generally handles updating/replacing files, but this is a case where the filename may have also changed so let's delete for simplicity.
                 var existingFileInfo = setInfo.Files.SingleOrDefault(f => string.Equals(f.Filename, beatmapInfo.Path, StringComparison.OrdinalIgnoreCase));
+                string targetFilename = getFilename(beatmapInfo);
+
+                // ensure that two difficulties from the set don't point at the same beatmap file.
+                if (setInfo.Beatmaps.Any(b => b.ID != beatmapInfo.ID && string.Equals(b.Path, targetFilename, StringComparison.OrdinalIgnoreCase)))
+                    throw new InvalidOperationException($"{setInfo.GetDisplayString()} already has a difficulty with the name of '{beatmapInfo.DifficultyName}'.");
+
                 if (existingFileInfo != null)
                     DeleteFile(setInfo, existingFileInfo);
 
@@ -103,9 +108,9 @@ namespace osu.Game.Beatmaps
 
         public void Update(BeatmapSetInfo item)
         {
-            Realm.Write(realm =>
+            Realm.Write(r =>
             {
-                var existing = realm.Find<BeatmapSetInfo>(item.ID);
+                var existing = r.Find<BeatmapSetInfo>(item.ID);
                 item.CopyChangesToRealm(existing);
             });
         }

osu.Game/Database/RealmObjectExtensions.cs

@@ -58,7 +58,16 @@ namespace osu.Game.Database
                      if (existing != null)
                          copyChangesToRealm(beatmap, existing);
                      else
-                         d.Beatmaps.Add(beatmap);
+                     {
+                         var newBeatmap = new BeatmapInfo
+                         {
+                             ID = beatmap.ID,
+                             BeatmapSet = d,
+                             Ruleset = d.Realm.Find<RulesetInfo>(beatmap.Ruleset.ShortName)
+                         };
+                         d.Beatmaps.Add(newBeatmap);
+                         copyChangesToRealm(beatmap, newBeatmap);
+                     }
                  }
              });
 

osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs

@@ -117,6 +117,7 @@ namespace osu.Game.Graphics.UserInterface
                 {
                     NormalText = new OsuSpriteText
                     {
+                        AlwaysPresent = true, // ensures that the menu item does not change width when switching between normal and bold text.
                         Anchor = Anchor.CentreLeft,
                         Origin = Anchor.CentreLeft,
                         Font = OsuFont.GetFont(size: text_size),
@@ -124,7 +125,7 @@ namespace osu.Game.Graphics.UserInterface
                     },
                     BoldText = new OsuSpriteText
                     {
-                        AlwaysPresent = true,
+                        AlwaysPresent = true, // ensures that the menu item does not change width when switching between normal and bold text.
                         Alpha = 0,
                         Anchor = Anchor.CentreLeft,
                         Origin = Anchor.CentreLeft,

osu.Game/Models/RealmUser.cs

@@ -2,12 +2,14 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using osu.Game.Database;
 using osu.Game.Users;
+using osu.Game.Utils;
 using Realms;
 
 namespace osu.Game.Models
 {
-    public class RealmUser : EmbeddedObject, IUser, IEquatable<RealmUser>
+    public class RealmUser : EmbeddedObject, IUser, IEquatable<RealmUser>, IDeepCloneable<RealmUser>
     {
         public int OnlineID { get; set; } = 1;
 
@@ -22,5 +24,7 @@ namespace osu.Game.Models
 
             return OnlineID == other.OnlineID && Username == other.Username;
         }
+
+        public RealmUser DeepClone() => (RealmUser)this.Detach().MemberwiseClone();
     }
 }

osu.Game/Screens/Edit/Editor.cs

@@ -77,6 +77,9 @@ namespace osu.Game.Screens.Edit
         [Resolved]
         private BeatmapManager beatmapManager { get; set; }
 
+        [Resolved]
+        private RulesetStore rulesets { get; set; }
+
         [Resolved]
         private Storage storage { get; set; }
 
@@ -375,21 +378,34 @@ namespace osu.Game.Screens.Edit
             Clipboard.Content.Value = state.ClipboardContent;
         });
 
-        protected void Save()
+        /// <summary>
+        /// Saves the currently edited beatmap.
+        /// </summary>
+        /// <returns>Whether the save was successful.</returns>
+        protected bool Save()
         {
             if (!canSave)
             {
                 notifications?.Post(new SimpleErrorNotification { Text = "Saving is not supported for this ruleset yet, sorry!" });
-                return;
+                return false;
+            }
+
+            try
+            {
+                // save the loaded beatmap's data stream.
+                beatmapManager.Save(editorBeatmap.BeatmapInfo, editorBeatmap.PlayableBeatmap, editorBeatmap.BeatmapSkin);
+            }
+            catch (Exception ex)
+            {
+                // can fail e.g. due to duplicated difficulty names.
+                Logger.Error(ex, ex.Message);
+                return false;
             }
 
             // no longer new after first user-triggered save.
             isNewBeatmap = false;
-
-            // save the loaded beatmap's data stream.
-            beatmapManager.Save(editorBeatmap.BeatmapInfo, editorBeatmap.PlayableBeatmap, editorBeatmap.BeatmapSkin);
-
             updateLastSavedHash();
+            return true;
         }
 
         protected override void Update()
@@ -798,7 +814,7 @@ namespace osu.Game.Screens.Edit
         {
             var fileMenuItems = new List<MenuItem>
             {
-                new EditorMenuItem("Save", MenuItemType.Standard, Save)
+                new EditorMenuItem("Save", MenuItemType.Standard, () => Save())
             };
 
             if (RuntimeInfo.IsDesktop)
@@ -806,6 +822,29 @@ namespace osu.Game.Screens.Edit
 
             fileMenuItems.Add(new EditorMenuItemSpacer());
 
+            fileMenuItems.Add(createDifficultyCreationMenu());
+            fileMenuItems.Add(createDifficultySwitchMenu());
+
+            fileMenuItems.Add(new EditorMenuItemSpacer());
+            fileMenuItems.Add(new EditorMenuItem("Exit", MenuItemType.Standard, this.Exit));
+            return fileMenuItems;
+        }
+
+        private EditorMenuItem createDifficultyCreationMenu()
+        {
+            var rulesetItems = new List<MenuItem>();
+
+            foreach (var ruleset in rulesets.AvailableRulesets)
+                rulesetItems.Add(new EditorMenuItem(ruleset.Name, MenuItemType.Standard, () => CreateNewDifficulty(ruleset)));
+
+            return new EditorMenuItem("Create new difficulty") { Items = rulesetItems };
+        }
+
+        protected void CreateNewDifficulty(RulesetInfo rulesetInfo)
+            => loader?.ScheduleSwitchToNewDifficulty(editorBeatmap.BeatmapInfo.BeatmapSet, rulesetInfo, GetState());
+
+        private EditorMenuItem createDifficultySwitchMenu()
+        {
             var beatmapSet = playableBeatmap.BeatmapInfo.BeatmapSet;
 
             Debug.Assert(beatmapSet != null);
@@ -818,23 +857,16 @@ namespace osu.Game.Screens.Edit
                     difficultyItems.Add(new EditorMenuItemSpacer());
 
                 foreach (var beatmap in rulesetBeatmaps.OrderBy(b => b.StarRating))
-                    difficultyItems.Add(createDifficultyMenuItem(beatmap));
+                {
+                    bool isCurrentDifficulty = playableBeatmap.BeatmapInfo.Equals(beatmap);
+                    difficultyItems.Add(new DifficultyMenuItem(beatmap, isCurrentDifficulty, SwitchToDifficulty));
+                }
             }
 
-            fileMenuItems.Add(new EditorMenuItem("Change difficulty") { Items = difficultyItems });
+            return new EditorMenuItem("Change difficulty") { Items = difficultyItems };
-
-            fileMenuItems.Add(new EditorMenuItemSpacer());
-            fileMenuItems.Add(new EditorMenuItem("Exit", MenuItemType.Standard, this.Exit));
-            return fileMenuItems;
         }
 
-        private DifficultyMenuItem createDifficultyMenuItem(BeatmapInfo beatmapInfo)
+        protected void SwitchToDifficulty(BeatmapInfo nextBeatmap) => loader?.ScheduleSwitchToExistingDifficulty(nextBeatmap, GetState(nextBeatmap));
-        {
-            bool isCurrentDifficulty = playableBeatmap.BeatmapInfo.Equals(beatmapInfo);
-            return new DifficultyMenuItem(beatmapInfo, isCurrentDifficulty, SwitchToDifficulty);
-        }
-
-        protected void SwitchToDifficulty(BeatmapInfo nextBeatmap) => loader?.ScheduleDifficultySwitch(nextBeatmap, GetState(nextBeatmap));
 
         private void cancelExit()
         {

osu.Game/Screens/Edit/EditorLoader.cs

@@ -6,10 +6,12 @@ using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Logging;
 using osu.Framework.Screens;
 using osu.Framework.Threading;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.Menu;
 using osu.Game.Screens.Play;
@@ -78,7 +80,26 @@ namespace osu.Game.Screens.Edit
             }
         }
 
-        public void ScheduleDifficultySwitch(BeatmapInfo nextBeatmap, EditorState editorState)
+        public void ScheduleSwitchToNewDifficulty(BeatmapSetInfo beatmapSetInfo, RulesetInfo rulesetInfo, EditorState editorState)
+            => scheduleDifficultySwitch(() =>
+            {
+                try
+                {
+                    return beatmapManager.CreateNewBlankDifficulty(beatmapSetInfo, rulesetInfo);
+                }
+                catch (Exception ex)
+                {
+                    // if the beatmap creation fails (e.g. due to duplicated difficulty names),
+                    // bring the user back to the previous beatmap as a best-effort.
+                    Logger.Error(ex, ex.Message);
+                    return Beatmap.Value;
+                }
+            }, editorState);
+
+        public void ScheduleSwitchToExistingDifficulty(BeatmapInfo beatmapInfo, EditorState editorState)
+            => scheduleDifficultySwitch(() => beatmapManager.GetWorkingBeatmap(beatmapInfo), editorState);
+
+        private void scheduleDifficultySwitch(Func<WorkingBeatmap> nextBeatmap, EditorState editorState)
         {
             scheduledDifficultySwitch?.Cancel();
             ValidForResume = true;
@@ -87,7 +108,7 @@ namespace osu.Game.Screens.Edit
 
             scheduledDifficultySwitch = Schedule(() =>
             {
-                Beatmap.Value = beatmapManager.GetWorkingBeatmap(nextBeatmap);
+                Beatmap.Value = nextBeatmap.Invoke();
                 state = editorState;
 
                 // This screen is a weird exception to the rule that nothing after song select changes the global beatmap.

osu.Game/Tests/Visual/EditorTestScene.cs

@@ -97,7 +97,7 @@ namespace osu.Game.Tests.Visual
 
             public new void Redo() => base.Redo();
 
-            public new void Save() => base.Save();
+            public new bool Save() => base.Save();
 
             public new void Cut() => base.Cut();
 
@@ -107,6 +107,8 @@ namespace osu.Game.Tests.Visual
 
             public new void SwitchToDifficulty(BeatmapInfo beatmapInfo) => base.SwitchToDifficulty(beatmapInfo);
 
+            public new void CreateNewDifficulty(RulesetInfo rulesetInfo) => base.CreateNewDifficulty(rulesetInfo);
+
             public new bool HasUnsavedChanges => base.HasUnsavedChanges;
 
             public TestEditor(EditorLoader loader = null)
@@ -134,6 +136,12 @@ namespace osu.Game.Tests.Visual
                 return new TestWorkingBeatmapCache(this, audioManager, resources, storage, defaultBeatmap, host);
             }
 
+            public override WorkingBeatmap CreateNewBlankDifficulty(BeatmapSetInfo beatmapSetInfo, RulesetInfo rulesetInfo)
+            {
+                // don't actually care about properly creating a difficulty for this context.
+                return TestBeatmap;
+            }
+
             private class TestWorkingBeatmapCache : WorkingBeatmapCache
             {
                 private readonly TestBeatmapManager testBeatmapManager;