Avoid saving state changes if nothing has changed (via binary comparison)

osu.Game.Tests/Editing/EditorChangeHandlerTest.cs

@@ -2,7 +2,9 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using NUnit.Framework;
+using osu.Framework.Utils;
 using osu.Game.Beatmaps;
+using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Screens.Edit;
 
 namespace osu.Game.Tests.Editing
@@ -13,11 +15,12 @@ public class EditorChangeHandlerTest
         [Test]
         public void TestSaveRestoreState()
         {
-            var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap()));
+            var (handler, beatmap) = createChangeHandler();
 
             Assert.That(handler.CanUndo.Value, Is.False);
             Assert.That(handler.CanRedo.Value, Is.False);
 
+            addArbitraryChange(beatmap);
             handler.SaveState();
 
             Assert.That(handler.CanUndo.Value, Is.True);
@@ -29,15 +32,48 @@ public void TestSaveRestoreState()
             Assert.That(handler.CanRedo.Value, Is.True);
         }
 
+        [Test]
+        public void TestSaveSameStateDoesNotSave()
+        {
+            var (handler, beatmap) = createChangeHandler();
+
+            Assert.That(handler.CanUndo.Value, Is.False);
+            Assert.That(handler.CanRedo.Value, Is.False);
+
+            addArbitraryChange(beatmap);
+            handler.SaveState();
+
+            Assert.That(handler.CanUndo.Value, Is.True);
+            Assert.That(handler.CanRedo.Value, Is.False);
+
+            string hash = handler.CurrentStateHash;
+
+            // save a save without making any changes
+            handler.SaveState();
+
+            Assert.That(hash, Is.EqualTo(handler.CurrentStateHash));
+
+            handler.RestoreState(-1);
+
+            Assert.That(hash, Is.Not.EqualTo(handler.CurrentStateHash));
+
+            // we should only be able to restore once even though we saved twice.
+            Assert.That(handler.CanUndo.Value, Is.False);
+            Assert.That(handler.CanRedo.Value, Is.True);
+        }
+
         [Test]
         public void TestMaxStatesSaved()
         {
-            var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap()));
+            var (handler, beatmap) = createChangeHandler();
 
             Assert.That(handler.CanUndo.Value, Is.False);
 
             for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES; i++)
+            {
+                addArbitraryChange(beatmap);
                 handler.SaveState();
+            }
 
             Assert.That(handler.CanUndo.Value, Is.True);
 
@@ -53,12 +89,15 @@ public void TestMaxStatesSaved()
         [Test]
         public void TestMaxStatesExceeded()
         {
-            var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap()));
+            var (handler, beatmap) = createChangeHandler();
 
             Assert.That(handler.CanUndo.Value, Is.False);
 
             for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES * 2; i++)
+            {
+                addArbitraryChange(beatmap);
                 handler.SaveState();
+            }
 
             Assert.That(handler.CanUndo.Value, Is.True);
 
@@ -70,5 +109,17 @@ public void TestMaxStatesExceeded()
 
             Assert.That(handler.CanUndo.Value, Is.False);
         }
+
+        private (EditorChangeHandler, EditorBeatmap) createChangeHandler()
+        {
+            var beatmap = new EditorBeatmap(new Beatmap());
+
+            return (new EditorChangeHandler(beatmap), beatmap);
+        }
+
+        private void addArbitraryChange(EditorBeatmap beatmap)
+        {
+            beatmap.Add(new HitCircle { StartTime = RNG.Next(0, 100000) });
+        }
     }
 }

osu.Game/Screens/Edit/EditorChangeHandler.cs

@@ -4,9 +4,11 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Text;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions;
+using osu.Framework.Logging;
 using osu.Game.Beatmaps.Formats;
 using osu.Game.Rulesets.Objects;
 
@@ -89,23 +91,27 @@ public void SaveState()
             if (isRestoring)
                 return;
 
-            if (currentState < savedStates.Count - 1)
-                savedStates.RemoveRange(currentState + 1, savedStates.Count - currentState - 1);
-
-            if (savedStates.Count > MAX_SAVED_STATES)
-                savedStates.RemoveAt(0);
-
             using (var stream = new MemoryStream())
             {
                 using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true))
                     new LegacyBeatmapEncoder(editorBeatmap, editorBeatmap.BeatmapSkin).Encode(sw);
 
-                savedStates.Add(stream.ToArray());
+                var newState = stream.ToArray();
+
+                // if the previous state is binary equal we don't need to push a new one, unless this is the initial state.
+                if (savedStates.Count > 0 && newState.SequenceEqual(savedStates.Last())) return;
+
+                if (currentState < savedStates.Count - 1)
+                    savedStates.RemoveRange(currentState + 1, savedStates.Count - currentState - 1);
+
+                if (savedStates.Count > MAX_SAVED_STATES)
+                    savedStates.RemoveAt(0);
+
+                savedStates.Add(newState);
+
+                currentState = savedStates.Count - 1;
+                updateBindables();
             }
-
-            currentState = savedStates.Count - 1;
-
-            updateBindables();
         }
 
         /// <summary>