mirror of
https://github.com/ppy/osu.git
synced 2024-11-11 12:17:26 +08:00
Merge pull request #22318 from naoey/fix-copy-difficulty-moving-collections
Fix creating a copy of a difficulty in the editor removing the original beatmap from user collections
This commit is contained in:
commit
4674956ccf
@ -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<EditorBeatmap>().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()
|
||||
{
|
||||
|
@ -186,7 +186,7 @@ namespace osu.Game.Beatmaps
|
||||
targetBeatmapSet.Beatmaps.Add(newBeatmap.BeatmapInfo);
|
||||
newBeatmap.BeatmapInfo.BeatmapSet = targetBeatmapSet;
|
||||
|
||||
Save(newBeatmap.BeatmapInfo, newBeatmap, beatmapSkin);
|
||||
save(newBeatmap.BeatmapInfo, newBeatmap, beatmapSkin, transferCollections: false);
|
||||
|
||||
workingBeatmapCache.Invalidate(targetBeatmapSet);
|
||||
return GetWorkingBeatmap(newBeatmap.BeatmapInfo);
|
||||
@ -280,77 +280,16 @@ namespace osu.Game.Beatmaps
|
||||
public IWorkingBeatmap DefaultBeatmap => workingBeatmapCache.DefaultBeatmap;
|
||||
|
||||
/// <summary>
|
||||
/// Saves an <see cref="IBeatmap"/> file against a given <see cref="BeatmapInfo"/>.
|
||||
/// Saves an existing <see cref="IBeatmap"/> file against a given <see cref="BeatmapInfo"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// This method will also update any user beatmap collection hash references to the new post-saved hash.
|
||||
/// </remarks>
|
||||
/// <param name="beatmapInfo">The <see cref="BeatmapInfo"/> to save the content against. The file referenced by <see cref="BeatmapInfo.Path"/> will be replaced.</param>
|
||||
/// <param name="beatmapContent">The <see cref="IBeatmap"/> content to write.</param>
|
||||
/// <param name="beatmapSkin">The beatmap <see cref="ISkin"/> content to write, null if to be omitted.</param>
|
||||
public virtual void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null)
|
||||
{
|
||||
var setInfo = beatmapInfo.BeatmapSet;
|
||||
Debug.Assert(setInfo != null);
|
||||
|
||||
// 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.
|
||||
|
||||
// Warning: The directionality here is important. Changes have to be copied *from* beatmapContent (which comes from editor and is being saved)
|
||||
// *to* the beatmapInfo (which is a database model and needs to receive values without the taiko slider velocity multiplier for correct operation).
|
||||
// CopyTo() will undo such adjustments, while CopyFrom() will not.
|
||||
beatmapContent.Difficulty.CopyTo(beatmapInfo.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))
|
||||
new LegacyBeatmapEncoder(beatmapContent, beatmapSkin).Encode(sw);
|
||||
|
||||
stream.Seek(0, SeekOrigin.Begin);
|
||||
|
||||
// AddFile generally handles updating/replacing files, but this is a case where the filename may have also changed so let's delete for simplicity.
|
||||
var existingFileInfo = beatmapInfo.Path != null ? setInfo.GetFile(beatmapInfo.Path) : null;
|
||||
string targetFilename = createBeatmapFilenameFromMetadata(beatmapInfo);
|
||||
|
||||
// ensure that two difficulties from the set don't point at the same beatmap file.
|
||||
if (setInfo.Beatmaps.Any(b => b.ID != beatmapInfo.ID && string.Equals(b.Path, targetFilename, StringComparison.OrdinalIgnoreCase)))
|
||||
throw new InvalidOperationException($"{setInfo.GetDisplayString()} already has a difficulty with the name of '{beatmapInfo.DifficultyName}'.");
|
||||
|
||||
if (existingFileInfo != null)
|
||||
DeleteFile(setInfo, existingFileInfo);
|
||||
|
||||
string oldMd5Hash = beatmapInfo.MD5Hash;
|
||||
|
||||
beatmapInfo.MD5Hash = stream.ComputeMD5Hash();
|
||||
beatmapInfo.Hash = stream.ComputeSHA2Hash();
|
||||
|
||||
beatmapInfo.LastLocalUpdate = DateTimeOffset.Now;
|
||||
beatmapInfo.Status = BeatmapOnlineStatus.LocallyModified;
|
||||
|
||||
AddFile(setInfo, stream, createBeatmapFilenameFromMetadata(beatmapInfo));
|
||||
|
||||
updateHashAndMarkDirty(setInfo);
|
||||
|
||||
Realm.Write(r =>
|
||||
{
|
||||
var liveBeatmapSet = r.Find<BeatmapSetInfo>(setInfo.ID);
|
||||
|
||||
setInfo.CopyChangesToRealm(liveBeatmapSet);
|
||||
|
||||
beatmapInfo.TransferCollectionReferences(r, oldMd5Hash);
|
||||
|
||||
ProcessBeatmap?.Invoke((liveBeatmapSet, false));
|
||||
});
|
||||
}
|
||||
|
||||
Debug.Assert(beatmapInfo.BeatmapSet != null);
|
||||
|
||||
static string createBeatmapFilenameFromMetadata(BeatmapInfo beatmapInfo)
|
||||
{
|
||||
var metadata = beatmapInfo.Metadata;
|
||||
return $"{metadata.Artist} - {metadata.Title} ({metadata.Author.Username}) [{beatmapInfo.DifficultyName}].osu".GetValidFilename();
|
||||
}
|
||||
}
|
||||
public virtual void Save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin = null) =>
|
||||
save(beatmapInfo, beatmapContent, beatmapSkin, transferCollections: true);
|
||||
|
||||
public void DeleteAllVideos()
|
||||
{
|
||||
@ -460,6 +399,74 @@ namespace osu.Game.Beatmaps
|
||||
setInfo.Status = BeatmapOnlineStatus.LocallyModified;
|
||||
}
|
||||
|
||||
private void save(BeatmapInfo beatmapInfo, IBeatmap beatmapContent, ISkin? beatmapSkin, bool transferCollections)
|
||||
{
|
||||
var setInfo = beatmapInfo.BeatmapSet;
|
||||
Debug.Assert(setInfo != null);
|
||||
|
||||
// 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.
|
||||
|
||||
// Warning: The directionality here is important. Changes have to be copied *from* beatmapContent (which comes from editor and is being saved)
|
||||
// *to* the beatmapInfo (which is a database model and needs to receive values without the taiko slider velocity multiplier for correct operation).
|
||||
// CopyTo() will undo such adjustments, while CopyFrom() will not.
|
||||
beatmapContent.Difficulty.CopyTo(beatmapInfo.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))
|
||||
new LegacyBeatmapEncoder(beatmapContent, beatmapSkin).Encode(sw);
|
||||
|
||||
stream.Seek(0, SeekOrigin.Begin);
|
||||
|
||||
// AddFile generally handles updating/replacing files, but this is a case where the filename may have also changed so let's delete for simplicity.
|
||||
var existingFileInfo = beatmapInfo.Path != null ? setInfo.GetFile(beatmapInfo.Path) : null;
|
||||
string targetFilename = createBeatmapFilenameFromMetadata(beatmapInfo);
|
||||
|
||||
// ensure that two difficulties from the set don't point at the same beatmap file.
|
||||
if (setInfo.Beatmaps.Any(b => b.ID != beatmapInfo.ID && string.Equals(b.Path, targetFilename, StringComparison.OrdinalIgnoreCase)))
|
||||
throw new InvalidOperationException($"{setInfo.GetDisplayString()} already has a difficulty with the name of '{beatmapInfo.DifficultyName}'.");
|
||||
|
||||
if (existingFileInfo != null)
|
||||
DeleteFile(setInfo, existingFileInfo);
|
||||
|
||||
string oldMd5Hash = beatmapInfo.MD5Hash;
|
||||
|
||||
beatmapInfo.MD5Hash = stream.ComputeMD5Hash();
|
||||
beatmapInfo.Hash = stream.ComputeSHA2Hash();
|
||||
|
||||
beatmapInfo.LastLocalUpdate = DateTimeOffset.Now;
|
||||
beatmapInfo.Status = BeatmapOnlineStatus.LocallyModified;
|
||||
|
||||
AddFile(setInfo, stream, createBeatmapFilenameFromMetadata(beatmapInfo));
|
||||
|
||||
updateHashAndMarkDirty(setInfo);
|
||||
|
||||
Realm.Write(r =>
|
||||
{
|
||||
var liveBeatmapSet = r.Find<BeatmapSetInfo>(setInfo.ID);
|
||||
|
||||
setInfo.CopyChangesToRealm(liveBeatmapSet);
|
||||
|
||||
if (transferCollections)
|
||||
beatmapInfo.TransferCollectionReferences(r, oldMd5Hash);
|
||||
|
||||
ProcessBeatmap?.Invoke((liveBeatmapSet, false));
|
||||
});
|
||||
}
|
||||
|
||||
Debug.Assert(beatmapInfo.BeatmapSet != null);
|
||||
|
||||
static string createBeatmapFilenameFromMetadata(BeatmapInfo beatmapInfo)
|
||||
{
|
||||
var metadata = beatmapInfo.Metadata;
|
||||
return $"{metadata.Artist} - {metadata.Title} ({metadata.Author.Username}) [{beatmapInfo.DifficultyName}].osu".GetValidFilename();
|
||||
}
|
||||
}
|
||||
|
||||
#region Implementation of ICanAcceptFiles
|
||||
|
||||
public Task Import(params string[] paths) => beatmapImporter.Import(paths);
|
||||
|
Loading…
Reference in New Issue
Block a user