Merge pull request #23362 from bdach/fix-saving-online-beatmap-not-resetting-id

Reset online ID on locally modifying beatmap

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

@@ -0,0 +1,37 @@
+// 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.Framework.Extensions;
+using osu.Game.Database;
+using osu.Game.Tests.Resources;
+
+namespace osu.Game.Tests.Visual.Editing
+{
+    public partial class TestSceneLocallyModifyingOnlineBeatmaps : EditorSavingTestScene
+    {
+        public override void SetUpSteps()
+        {
+            CreateInitialBeatmap = () =>
+            {
+                var importedSet = Game.BeatmapManager.Import(new ImportTask(TestResources.GetTestBeatmapForImport())).GetResultSafely();
+                return Game.BeatmapManager.GetWorkingBeatmap(importedSet!.Value.Beatmaps.First());
+            };
+
+            base.SetUpSteps();
+        }
+
+        [Test]
+        public void TestLocallyModifyingOnlineBeatmap()
+        {
+            AddAssert("editor beatmap has online ID", () => EditorBeatmap.BeatmapInfo.OnlineID, () => Is.GreaterThan(0));
+
+            AddStep("delete first hitobject", () => EditorBeatmap.RemoveAt(0));
+            SaveEditor();
+
+            ReloadEditorToSameBeatmap();
+            AddAssert("editor beatmap online ID reset", () => EditorBeatmap.BeatmapInfo.OnlineID, () => Is.EqualTo(-1));
+        }
+    }
+}

osu.Game/Beatmaps/BeatmapManager.cs

@@ -415,6 +415,13 @@ namespace osu.Game.Beatmaps
             // All changes to metadata are made in the provided beatmapInfo, so this should be copied to the `IBeatmap` before encoding.
             beatmapContent.BeatmapInfo = beatmapInfo;
 
+            // Since now this is a locally-modified beatmap, we also set all relevant flags to indicate this.
+            // Importantly, the `ResetOnlineInfo()` call must happen before encoding, as online ID is encoded into the `.osu` file,
+            // which influences the beatmap checksums.
+            beatmapInfo.LastLocalUpdate = DateTimeOffset.Now;
+            beatmapInfo.Status = BeatmapOnlineStatus.LocallyModified;
+            beatmapInfo.ResetOnlineInfo();
+
             using (var stream = new MemoryStream())
             {
                 using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true))
@@ -438,9 +445,6 @@ namespace osu.Game.Beatmaps
                 beatmapInfo.MD5Hash = stream.ComputeMD5Hash();
                 beatmapInfo.Hash = stream.ComputeSHA2Hash();
 
-                beatmapInfo.LastLocalUpdate = DateTimeOffset.Now;
-                beatmapInfo.Status = BeatmapOnlineStatus.LocallyModified;
-
                 AddFile(setInfo, stream, createBeatmapFilenameFromMetadata(beatmapInfo));
 
                 updateHashAndMarkDirty(setInfo);

osu.Game/Tests/Visual/EditorSavingTestScene.cs

@@ -3,9 +3,12 @@
 
 #nullable disable
 
+using System;
 using System.Linq;
+using JetBrains.Annotations;
 using osu.Framework.Input;
 using osu.Framework.Testing;
+using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Edit;
 using osu.Game.Screens.Edit;
 using osu.Game.Screens.Edit.Setup;
@@ -24,17 +27,26 @@ namespace osu.Game.Tests.Visual
 
         protected EditorBeatmap EditorBeatmap => (EditorBeatmap)Editor.Dependencies.Get(typeof(EditorBeatmap));
 
+        [CanBeNull]
+        protected Func<WorkingBeatmap> CreateInitialBeatmap { get; set; }
+
         [SetUpSteps]
         public override void SetUpSteps()
         {
             base.SetUpSteps();
 
+            if (CreateInitialBeatmap == null)
                 AddStep("set default beatmap", () => Game.Beatmap.SetDefault());
+            else
+            {
+                AddStep("set test beatmap", () => Game.Beatmap.Value = CreateInitialBeatmap?.Invoke());
+            }
 
             PushAndConfirm(() => new EditorLoader());
 
             AddUntilStep("wait for editor load", () => Editor?.IsLoaded == true);
 
+            if (CreateInitialBeatmap == null)
                 AddUntilStep("wait for metadata screen load", () => Editor.ChildrenOfType<MetadataSection>().FirstOrDefault()?.IsLoaded == true);
 
             // We intentionally switch away from the metadata screen, else there is a feedback loop with the textbox handling which causes metadata changes below to get overwritten.
@@ -50,6 +62,14 @@ namespace osu.Game.Tests.Visual
 
         protected void ReloadEditorToSameBeatmap()
         {
+            Guid beatmapSetGuid = Guid.Empty;
+            Guid beatmapGuid = Guid.Empty;
+
+            AddStep("Store beatmap GUIDs", () =>
+            {
+                beatmapSetGuid = EditorBeatmap.BeatmapInfo.BeatmapSet!.ID;
+                beatmapGuid = EditorBeatmap.BeatmapInfo.ID;
+            });
             AddStep("Exit", () => InputManager.Key(Key.Escape));
 
             AddUntilStep("Wait for main menu", () => Game.ScreenStack.CurrentScreen is MainMenu);
@@ -59,7 +79,8 @@ namespace osu.Game.Tests.Visual
             PushAndConfirm(() => songSelect = new PlaySongSelect());
             AddUntilStep("wait for carousel load", () => songSelect.BeatmapSetsLoaded);
 
-            AddUntilStep("Wait for beatmap selected", () => !Game.Beatmap.IsDefault);
+            AddStep("Present same beatmap", () => Game.PresentBeatmap(Game.BeatmapManager.QueryBeatmapSet(set => set.ID == beatmapSetGuid)!.Value, beatmap => beatmap.ID == beatmapGuid));
+            AddUntilStep("Wait for beatmap selected", () => Game.Beatmap.Value.BeatmapInfo.ID == beatmapGuid);
             AddStep("Open options", () => InputManager.Key(Key.F3));
             AddStep("Enter editor", () => InputManager.Key(Key.Number5));