Merge pull request #15075 from peppy/fix-editor-difficulty-name-update

Use actual `BeatmapInfo` rather than `PlayableBeatmap.BeatmapInfo` for editor writes
This commit is contained in:
Dan Balasescu 2021-10-15 18:12:12 +09:00 committed by GitHub
commit 8d11f4ce48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 78 additions and 27 deletions

View File

@ -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<MetadataSection>().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<HitObjectComposer>().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");
}
}
}

View File

@ -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;

View File

@ -189,11 +189,14 @@ namespace osu.Game.Beatmaps
/// </summary>
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<IBeatmap> loadBeatmapAsync() => beatmapLoadTask ??= Task.Factory.StartNew(() =>
private readonly object beatmapFetchLock = new object();
private Task<IBeatmap> 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();

View File

@ -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;

View File

@ -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<IReadOnlyList<Mod>> mods)

View File

@ -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<IEditorChangeHandler>(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);
}

View File

@ -44,6 +44,7 @@ namespace osu.Game.Screens.Edit
/// </summary>
public readonly Bindable<HitObject> PlacementObject = new Bindable<HitObject>();
private readonly BeatmapInfo beatmapInfo;
public readonly IBeatmap PlayableBeatmap;
/// <summary>
@ -66,9 +67,12 @@ namespace osu.Game.Screens.Edit
private readonly Dictionary<HitObject, Bindable<double>> startTimeBindables = new Dictionary<HitObject, Bindable<double>>();
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
{

View File

@ -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;

View File

@ -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))
{