diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs index f0aa3e2350..ab2bc4649a 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs @@ -8,6 +8,7 @@ using osu.Framework.Testing; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit; +using osu.Game.Screens.Edit.Setup; using osu.Game.Screens.Menu; using osu.Game.Screens.Select; using osuTK.Input; @@ -30,23 +31,35 @@ namespace osu.Game.Tests.Visual.Editing PushAndConfirm(() => new EditorLoader()); - AddUntilStep("wait for editor load", () => editor != null); + AddUntilStep("wait for editor load", () => editor?.IsLoaded == true); - AddStep("Set overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty = 7); + AddUntilStep("wait for metadata screen load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); - AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); + // 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. AddStep("Enter compose mode", () => InputManager.Key(Key.F1)); AddUntilStep("Wait for compose mode load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); + AddStep("Set overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty = 7); + AddStep("Set artist and title", () => + { + editorBeatmap.BeatmapInfo.Metadata.Artist = "artist"; + editorBeatmap.BeatmapInfo.Metadata.Title = "title"; + }); + AddStep("Set difficulty name", () => editorBeatmap.BeatmapInfo.Version = "difficulty"); + + AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); + AddStep("Change to placement mode", () => InputManager.Key(Key.Number2)); AddStep("Move to playfield", () => InputManager.MoveMouseTo(Game.ScreenSpaceDrawQuad.Centre)); AddStep("Place single hitcircle", () => InputManager.Click(MouseButton.Left)); - AddAssert("Beatmap contains single hitcircle", () => editorBeatmap.HitObjects.Count == 1); + checkMutations(); AddStep("Save", () => InputManager.Keys(PlatformAction.Save)); + checkMutations(); + AddStep("Exit", () => InputManager.Key(Key.Escape)); AddUntilStep("Wait for main menu", () => Game.ScreenStack.CurrentScreen is MainMenu); @@ -58,8 +71,16 @@ namespace osu.Game.Tests.Visual.Editing AddStep("Enter editor", () => InputManager.Key(Key.Number5)); AddUntilStep("Wait for editor load", () => editor != null); + + checkMutations(); + } + + private void checkMutations() + { AddAssert("Beatmap contains single hitcircle", () => editorBeatmap.HitObjects.Count == 1); AddAssert("Beatmap has correct overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty == 7); + AddAssert("Beatmap has correct metadata", () => editorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && editorBeatmap.BeatmapInfo.Metadata.Title == "title"); + AddAssert("Beatmap has correct difficulty name", () => editorBeatmap.BeatmapInfo.Version == "difficulty"); } } } diff --git a/osu.Game/Beatmaps/BeatmapModelManager.cs b/osu.Game/Beatmaps/BeatmapModelManager.cs index 9c0fc5ef8a..76019a15ae 100644 --- a/osu.Game/Beatmaps/BeatmapModelManager.cs +++ b/osu.Game/Beatmaps/BeatmapModelManager.cs @@ -192,6 +192,13 @@ namespace osu.Game.Beatmaps { var setInfo = beatmapInfo.BeatmapSet; + // Difficulty settings must be copied first due to the clone in `Beatmap<>.BeatmapInfo_Set`. + // This should hopefully be temporary, assuming said clone is eventually removed. + beatmapInfo.BaseDifficulty.CopyFrom(beatmapContent.Difficulty); + + // All changes to metadata are made in the provided beatmapInfo, so this should be copied to the `IBeatmap` before encoding. + beatmapContent.BeatmapInfo = beatmapInfo; + using (var stream = new MemoryStream()) { using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) @@ -202,7 +209,6 @@ namespace osu.Game.Beatmaps using (ContextFactory.GetForWrite()) { beatmapInfo = setInfo.Beatmaps.Single(b => b.ID == beatmapInfo.ID); - beatmapInfo.BaseDifficulty.CopyFrom(beatmapContent.Difficulty); var metadata = beatmapInfo.Metadata ?? setInfo.Metadata; diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 18adecb7aa..d2c0f7de0f 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -189,11 +189,14 @@ namespace osu.Game.Beatmaps /// public void CancelAsyncLoad() { - loadCancellation?.Cancel(); - loadCancellation = new CancellationTokenSource(); + lock (beatmapFetchLock) + { + loadCancellation?.Cancel(); + loadCancellation = new CancellationTokenSource(); - if (beatmapLoadTask?.IsCompleted != true) - beatmapLoadTask = null; + if (beatmapLoadTask?.IsCompleted != true) + beatmapLoadTask = null; + } } private CancellationTokenSource createCancellationTokenSource(TimeSpan? timeout) @@ -205,19 +208,27 @@ namespace osu.Game.Beatmaps return new CancellationTokenSource(timeout ?? TimeSpan.FromSeconds(10)); } - private Task loadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() => + private readonly object beatmapFetchLock = new object(); + + private Task loadBeatmapAsync() { - // Todo: Handle cancellation during beatmap parsing - var b = GetBeatmap() ?? new Beatmap(); + lock (beatmapFetchLock) + { + return beatmapLoadTask ??= Task.Factory.StartNew(() => + { + // Todo: Handle cancellation during beatmap parsing + var b = GetBeatmap() ?? new Beatmap(); - // The original beatmap version needs to be preserved as the database doesn't contain it - BeatmapInfo.BeatmapVersion = b.BeatmapInfo.BeatmapVersion; + // The original beatmap version needs to be preserved as the database doesn't contain it + BeatmapInfo.BeatmapVersion = b.BeatmapInfo.BeatmapVersion; - // Use the database-backed info for more up-to-date values (beatmap id, ranked status, etc) - b.BeatmapInfo = BeatmapInfo; + // Use the database-backed info for more up-to-date values (beatmap id, ranked status, etc) + b.BeatmapInfo = BeatmapInfo; - return b; - }, loadCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); + return b; + }, loadCancellation.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); + } + } public override string ToString() => BeatmapInfo.ToString(); diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index ad3e890b3a..cf83345e2a 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -66,8 +66,12 @@ namespace osu.Game.Beatmaps lock (workingCache) { var working = workingCache.FirstOrDefault(w => w.BeatmapInfo?.ID == info.ID); + if (working != null) + { + Logger.Log($"Invalidating working beatmap cache for {info}"); workingCache.Remove(working); + } } } @@ -86,6 +90,7 @@ namespace osu.Game.Beatmaps lock (workingCache) { var working = workingCache.FirstOrDefault(w => w.BeatmapInfo?.ID == beatmapInfo.ID); + if (working != null) return working; diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 6d2131d85b..820597488b 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -554,6 +554,7 @@ namespace osu.Game { beatmap.OldValue?.CancelAsyncLoad(); beatmap.NewValue?.BeginAsyncLoad(); + Logger.Log($"Game-wide working beatmap updated to {beatmap.NewValue}"); } private void modsChanged(ValueChangedEvent> mods) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 1170658abb..512226413b 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -162,7 +162,7 @@ namespace osu.Game.Screens.Edit // todo: remove caching of this and consume via editorBeatmap? dependencies.Cache(beatDivisor); - AddInternal(editorBeatmap = new EditorBeatmap(playableBeatmap, loadableBeatmap.GetSkin())); + AddInternal(editorBeatmap = new EditorBeatmap(playableBeatmap, loadableBeatmap.GetSkin(), loadableBeatmap.BeatmapInfo)); dependencies.CacheAs(editorBeatmap); changeHandler = new EditorChangeHandler(editorBeatmap); dependencies.CacheAs(changeHandler); @@ -333,10 +333,10 @@ namespace osu.Game.Screens.Edit isNewBeatmap = false; // apply any set-level metadata changes. - beatmapManager.Update(playableBeatmap.BeatmapInfo.BeatmapSet); + beatmapManager.Update(editorBeatmap.BeatmapInfo.BeatmapSet); // save the loaded beatmap's data stream. - beatmapManager.Save(playableBeatmap.BeatmapInfo, editorBeatmap, editorBeatmap.BeatmapSkin); + beatmapManager.Save(editorBeatmap.BeatmapInfo, editorBeatmap.PlayableBeatmap, editorBeatmap.BeatmapSkin); updateLastSavedHash(); } @@ -523,7 +523,10 @@ namespace osu.Game.Screens.Edit var refetchedBeatmap = beatmapManager.GetWorkingBeatmap(Beatmap.Value.BeatmapInfo); if (!(refetchedBeatmap is DummyWorkingBeatmap)) + { + Logger.Log("Editor providing re-fetched beatmap post edit session"); Beatmap.Value = refetchedBeatmap; + } return base.OnExiting(next); } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 64eb6225fa..c96c17326d 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -44,6 +44,7 @@ namespace osu.Game.Screens.Edit /// public readonly Bindable PlacementObject = new Bindable(); + private readonly BeatmapInfo beatmapInfo; public readonly IBeatmap PlayableBeatmap; /// @@ -66,9 +67,12 @@ namespace osu.Game.Screens.Edit private readonly Dictionary> startTimeBindables = new Dictionary>(); - public EditorBeatmap(IBeatmap playableBeatmap, ISkin beatmapSkin = null) + public EditorBeatmap(IBeatmap playableBeatmap, ISkin beatmapSkin = null, BeatmapInfo beatmapInfo = null) { PlayableBeatmap = playableBeatmap; + + this.beatmapInfo = beatmapInfo ?? playableBeatmap.BeatmapInfo; + if (beatmapSkin is Skin skin) BeatmapSkin = new EditorBeatmapSkin(skin); @@ -80,11 +84,11 @@ namespace osu.Game.Screens.Edit public BeatmapInfo BeatmapInfo { - get => PlayableBeatmap.BeatmapInfo; - set => PlayableBeatmap.BeatmapInfo = value; + get => beatmapInfo; + set => throw new InvalidOperationException(); } - public BeatmapMetadata Metadata => PlayableBeatmap.Metadata; + public BeatmapMetadata Metadata => beatmapInfo.Metadata; public BeatmapDifficulty Difficulty { diff --git a/osu.Game/Screens/Edit/Setup/MetadataSection.cs b/osu.Game/Screens/Edit/Setup/MetadataSection.cs index 9e93b0b038..5bb40c09a5 100644 --- a/osu.Game/Screens/Edit/Setup/MetadataSection.cs +++ b/osu.Game/Screens/Edit/Setup/MetadataSection.cs @@ -10,7 +10,7 @@ using osu.Game.Graphics.UserInterfaceV2; namespace osu.Game.Screens.Edit.Setup { - internal class MetadataSection : SetupSection + public class MetadataSection : SetupSection { protected LabelledTextBox ArtistTextBox; protected LabelledTextBox RomanisedArtistTextBox; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 6cafcb9d16..a2dea355ac 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -410,7 +410,7 @@ namespace osu.Game.Screens.Select { if (e.NewValue is DummyWorkingBeatmap || !this.IsCurrentScreen()) return; - Logger.Log($"working beatmap updated to {e.NewValue}"); + Logger.Log($"Song select working beatmap updated to {e.NewValue}"); if (!Carousel.SelectBeatmap(e.NewValue.BeatmapInfo, false)) {