diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs index 3f89bf9e9c..5aa2dd2ebf 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs @@ -13,6 +13,7 @@ using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; +using osu.Game.Collections; using osu.Game.Database; using osu.Game.Overlays.Dialog; using osu.Game.Rulesets; @@ -42,6 +43,9 @@ namespace osu.Game.Tests.Visual.Editing [Resolved] private BeatmapManager beatmapManager { get; set; } = null!; + [Resolved] + private RealmAccess realm { get; set; } = null!; + private Guid currentBeatmapSetID => EditorBeatmap.BeatmapInfo.BeatmapSet?.ID ?? Guid.Empty; public override void SetUpSteps() @@ -224,7 +228,8 @@ namespace osu.Game.Tests.Visual.Editing return beatmap != null && beatmap.DifficultyName == secondDifficultyName && set != null - && set.PerformRead(s => s.Beatmaps.Count == 2 && s.Beatmaps.Any(b => b.DifficultyName == secondDifficultyName) && s.Beatmaps.All(b => s.Status == BeatmapOnlineStatus.LocallyModified)); + && set.PerformRead(s => + s.Beatmaps.Count == 2 && s.Beatmaps.Any(b => b.DifficultyName == secondDifficultyName) && s.Beatmaps.All(b => s.Status == BeatmapOnlineStatus.LocallyModified)); }); } @@ -327,6 +332,56 @@ namespace osu.Game.Tests.Visual.Editing AddAssert("old beatmap file not deleted", () => refetchedBeatmapSet.AsNonNull().PerformRead(s => s.Files.Count == 2)); } + [Test] + public void TestCopyDifficultyDoesNotChangeCollections() + { + string originalDifficultyName = Guid.NewGuid().ToString(); + + AddStep("set unique difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = originalDifficultyName); + AddStep("save beatmap", () => Editor.Save()); + + string originalMd5 = string.Empty; + BeatmapCollection collection = null!; + + AddStep("setup a collection with original beatmap", () => + { + collection = new BeatmapCollection("test copy"); + collection.BeatmapMD5Hashes.Add(originalMd5 = EditorBeatmap.BeatmapInfo.MD5Hash); + + realm.Write(r => + { + r.Add(collection); + }); + }); + + AddAssert("collection contains original beatmap", () => + !string.IsNullOrEmpty(originalMd5) && collection.BeatmapMD5Hashes.Contains(originalMd5)); + + AddStep("create new difficulty", () => Editor.CreateNewDifficulty(new OsuRuleset().RulesetInfo)); + + AddUntilStep("wait for dialog", () => DialogOverlay.CurrentDialog is CreateNewDifficultyDialog); + AddStep("confirm creation as a copy", () => DialogOverlay.CurrentDialog.Buttons.ElementAt(1).TriggerClick()); + + AddUntilStep("wait for created", () => + { + string? difficultyName = Editor.ChildrenOfType().SingleOrDefault()?.BeatmapInfo.DifficultyName; + return difficultyName != null && difficultyName != originalDifficultyName; + }); + + AddStep("save without changes", () => Editor.Save()); + + AddAssert("collection still points to old beatmap", () => !collection.BeatmapMD5Hashes.Contains(EditorBeatmap.BeatmapInfo.MD5Hash) + && collection.BeatmapMD5Hashes.Contains(originalMd5)); + + AddStep("clean up collection", () => + { + realm.Write(r => + { + r.Remove(collection); + }); + }); + } + [Test] public void TestCreateMultipleNewDifficultiesSucceeds() { diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 34637501fa..db9c450bbe 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -186,7 +186,10 @@ namespace osu.Game.Beatmaps targetBeatmapSet.Beatmaps.Add(newBeatmap.BeatmapInfo); newBeatmap.BeatmapInfo.BeatmapSet = targetBeatmapSet; - Save(newBeatmap.BeatmapInfo, newBeatmap, beatmapSkin); + // make sure that collections don't get transferred when adding new difficulties to a set (that function + // was added for the scenario of saving the same difficulty), since this path is invoked from copying + // an existing difficulty as well. + Save(newBeatmap.BeatmapInfo, newBeatmap, beatmapSkin, false); workingBeatmapCache.Invalidate(targetBeatmapSet); return GetWorkingBeatmap(newBeatmap.BeatmapInfo); @@ -285,7 +288,8 @@ namespace osu.Game.Beatmaps /// The to save the content against. The file referenced by will be replaced. /// The content to write. /// The beatmap content to write, null if to be omitted. - public virtual void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null) + /// Whether to transfer the MD5 hashes in collections referencing this beatmap. + public virtual void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null, bool transferCollections = false) { var setInfo = beatmapInfo.BeatmapSet; Debug.Assert(setInfo != null); @@ -337,7 +341,8 @@ namespace osu.Game.Beatmaps setInfo.CopyChangesToRealm(liveBeatmapSet); - beatmapInfo.TransferCollectionReferences(r, oldMd5Hash); + if (transferCollections) + beatmapInfo.TransferCollectionReferences(r, oldMd5Hash); ProcessBeatmap?.Invoke((liveBeatmapSet, false)); }); diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 74ea933255..7161f34df2 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -429,7 +429,7 @@ namespace osu.Game.Screens.Edit try { // save the loaded beatmap's data stream. - beatmapManager.Save(editorBeatmap.BeatmapInfo, editorBeatmap.PlayableBeatmap, editorBeatmap.BeatmapSkin); + beatmapManager.Save(editorBeatmap.BeatmapInfo, editorBeatmap.PlayableBeatmap, editorBeatmap.BeatmapSkin, true); } catch (Exception ex) { diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index 6e2f1e99cd..e9d186fe35 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/osu.Game/Tests/Visual/EditorTestScene.cs @@ -178,7 +178,7 @@ namespace osu.Game.Tests.Visual => testBeatmapManager.TestBeatmap; } - public override void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) + public override void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null, bool transferCollections = false) { // don't actually care about saving for this context. }