Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split out beatmap update tasks to BeatmapUpdater and invoke from editor save flow #18835

Merged
merged 17 commits into from
Jul 1, 2022
Merged

Split out beatmap update tasks to BeatmapUpdater and invoke from editor save flow #18835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
06d59b7
Move beatmap processing tasks to new `BeatmapUpdater` class
peppy Jun 20, 2022
fe570d8
Queue beatmaps for update after editing
peppy Jun 20, 2022
021b16f
Ensure `WorkingBeatmap` is invalidated after update
peppy Jun 20, 2022
6999933
Split updater process into realm transaction and non-transaction
peppy Jun 24, 2022
66a01d1
Allow song select to refresh the global `WorkingBeatmap` after an ext…
peppy Jun 20, 2022
30b3973
Difficulty cache invalidation flow
peppy Jun 24, 2022
0c3d890
Fix reprocessing not working on import due to realm threading woes
peppy Jun 24, 2022
7692bac
Simplify refetch (and ensure to invalidate after processing)
peppy Jun 24, 2022
2c5e5fe
Add test coverage of star rating and difficulty updating on editor save
peppy Jun 24, 2022
9893882
Merge branch 'master' into beatmap-update-flow
peppy Jun 30, 2022
82134ad
Allow `null` parameter to `GetWorkingBeatmap` for now
peppy Jun 30, 2022
e34c2f0
Remove unnecessary `nullable-enable`
peppy Jun 30, 2022
ef42c6e
Add missing xmldoc
peppy Jun 30, 2022
0698471
Move `BeatmapOnlineLookupQueue` to inside `BeatmapUpdater`
peppy Jun 30, 2022
aab4dce
Remove unnecessary invalidation handling flow
peppy Jun 30, 2022
2a73210
Add xmldoc and update parameter naming for `MemoryCachingComponent.In…
peppy Jun 30, 2022
0e1f08e
Fix regressing `BeatmapRecommendations` tests due to diffcalc running at
peppy Jul 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 31 additions & 27 deletions osu.Game.Tests/Database/BeatmapImporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public void TestDetachBeatmapSet()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using (var importer = new BeatmapImporter(storage, realm))
var importer = new BeatmapImporter(storage, realm);

using (new RealmRulesetStore(realm, storage))
{
var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
Expand Down Expand Up @@ -76,7 +77,8 @@ public void TestUpdateDetachedBeatmapSet()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using (var importer = new BeatmapImporter(storage, realm))
var importer = new BeatmapImporter(storage, realm);

using (new RealmRulesetStore(realm, storage))
{
var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
Expand Down Expand Up @@ -134,7 +136,8 @@ public void TestAddFileToAsyncImportedBeatmap()

var manager = new ModelManager<BeatmapSetInfo>(storage, realm);

using (var importer = new BeatmapImporter(storage, realm))
var importer = new BeatmapImporter(storage, realm);

using (new RealmRulesetStore(realm, storage))
{
Task.Run(async () =>
Expand All @@ -160,7 +163,8 @@ public void TestImportBeatmapThenCleanup()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using (var importer = new BeatmapImporter(storage, realm))
var importer = new BeatmapImporter(storage, realm);

using (new RealmRulesetStore(realm, storage))
{
var imported = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz"));
Expand All @@ -187,7 +191,7 @@ public void TestImportWhenClosed()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -199,7 +203,7 @@ public void TestAccessFileAfterImport()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -217,7 +221,7 @@ public void TestImportThenDelete()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -231,7 +235,7 @@ public void TestImportThenDeleteFromStream()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? tempPath = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -261,7 +265,7 @@ public void TestImportThenImport()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -281,7 +285,7 @@ public void TestImportDirectoryWithEmptyOsuFiles()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -317,7 +321,7 @@ public void TestImportThenImportWithReZip()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -366,7 +370,7 @@ public void TestImportThenImportWithChangedHashedFile()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -417,7 +421,7 @@ public void TestImportThenImportWithChangedFile()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -465,7 +469,7 @@ public void TestImportThenImportWithDifferentFilename()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -513,7 +517,7 @@ public void TestImportCorruptThenImport()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand Down Expand Up @@ -548,7 +552,7 @@ public void TestModelCreationFailureDoesntReturn()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var progressNotification = new ImportProgressNotification();
Expand Down Expand Up @@ -586,7 +590,7 @@ public void TestRollbackOnFailure()
Interlocked.Increment(ref loggedExceptionCount);
};

using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand Down Expand Up @@ -644,7 +648,7 @@ public void TestImportThenDeleteThenImportOptimisedPath()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm, batchImport: true);
Expand All @@ -671,7 +675,7 @@ public void TestImportThenReimportAfterMissingFiles()
{
RunTestWithRealmAsync(async (realmFactory, storage) =>
{
using var importer = new BeatmapImporter(storage, realmFactory);
var importer = new BeatmapImporter(storage, realmFactory);
using var store = new RealmRulesetStore(realmFactory, storage);

var imported = await LoadOszIntoStore(importer, realmFactory.Realm);
Expand Down Expand Up @@ -703,7 +707,7 @@ public void TestImportThenDeleteThenImportNonOptimisedPath()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -730,7 +734,7 @@ public void TestImportThenDeleteThenImportWithOnlineIDsMissing()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var imported = await LoadOszIntoStore(importer, realm.Realm);
Expand All @@ -756,7 +760,7 @@ public void TestImportWithDuplicateBeatmapIDs()
{
RunTestWithRealm((realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

var metadata = new BeatmapMetadata
Expand Down Expand Up @@ -804,7 +808,7 @@ public void TestImportWhenFileOpen()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand All @@ -821,7 +825,7 @@ public void TestImportWithDuplicateHashes()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -857,7 +861,7 @@ public void TestImportNestedStructure()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -899,7 +903,7 @@ public void TestImportWithIgnoredDirectoryInArchive()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down Expand Up @@ -950,7 +954,7 @@ public void TestUpdateBeatmapInfo()
{
RunTestWithRealmAsync(async (realm, storage) =>
{
using var importer = new BeatmapImporter(storage, realm);
var importer = new BeatmapImporter(storage, realm);
using var store = new RealmRulesetStore(realm, storage);

string? temp = TestResources.GetTestBeatmapForImport();
Expand Down
8 changes: 4 additions & 4 deletions osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,17 +212,17 @@ public TestBeatmapManager(Storage storage, RealmAccess realm, RulesetStore rules
{
}

protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue onlineLookupQueue)
protected override BeatmapImporter CreateBeatmapImporter(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapUpdater beatmapUpdater)
{
return new TestBeatmapImporter(this, storage, realm, onlineLookupQueue);
return new TestBeatmapImporter(this, storage, realm, beatmapUpdater);
}

internal class TestBeatmapImporter : BeatmapImporter
{
private readonly TestBeatmapManager testBeatmapManager;

public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapOnlineLookupQueue beatmapOnlineLookupQueue)
: base(storage, databaseAccess, beatmapOnlineLookupQueue)
public TestBeatmapImporter(TestBeatmapManager testBeatmapManager, Storage storage, RealmAccess databaseAccess, BeatmapUpdater beatmapUpdater)
: base(storage, databaseAccess, beatmapUpdater)
{
this.testBeatmapManager = testBeatmapManager;
}
Expand Down
49 changes: 49 additions & 0 deletions osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using osu.Framework.Screens;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Screens.Edit;
using osu.Game.Screens.Edit.Compose.Components.Timeline;
Expand Down Expand Up @@ -130,6 +131,54 @@ public void TestHitObjectPlacement()
!ReferenceEquals(EditorBeatmap.HitObjects[0].DifficultyControlPoint, DifficultyControlPoint.DEFAULT));
}

[Test]
public void TestLengthAndStarRatingUpdated()
{
WorkingBeatmap working = null;
double lastStarRating = 0;
double lastLength = 0;

AddStep("Add timing point", () => EditorBeatmap.ControlPointInfo.Add(500, 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("One hitobject placed", () => EditorBeatmap.HitObjects.Count == 1);

SaveEditor();
AddStep("Get working beatmap", () => working = Game.BeatmapManager.GetWorkingBeatmap(EditorBeatmap.BeatmapInfo, true));

AddAssert("Beatmap length is zero", () => working.BeatmapInfo.Length == 0);
checkDifficultyIncreased();

AddStep("Move forward", () => InputManager.Key(Key.Right));
AddStep("Place another hitcircle", () => InputManager.Click(MouseButton.Left));
AddAssert("Two hitobjects placed", () => EditorBeatmap.HitObjects.Count == 2);

SaveEditor();
AddStep("Get working beatmap", () => working = Game.BeatmapManager.GetWorkingBeatmap(EditorBeatmap.BeatmapInfo, true));

checkDifficultyIncreased();
checkLengthIncreased();

void checkLengthIncreased()
{
AddStep("Beatmap length increased", () =>
{
Assert.That(working.BeatmapInfo.Length, Is.GreaterThan(lastLength));
lastLength = working.BeatmapInfo.Length;
});
}

void checkDifficultyIncreased()
{
AddStep("Beatmap difficulty increased", () =>
{
Assert.That(working.BeatmapInfo.StarRating, Is.GreaterThan(lastStarRating));
lastStarRating = working.BeatmapInfo.StarRating;
});
}
}

[Test]
public void TestExitWithoutSaveFromExistingBeatmap()
{
Expand Down
20 changes: 14 additions & 6 deletions osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Allocation;
Expand Down Expand Up @@ -166,15 +167,22 @@ private BeatmapSetInfo importBeatmapSet(IEnumerable<RulesetInfo> difficultyRules

var beatmapSet = TestResources.CreateTestBeatmapSetInfo(rulesets.Length, rulesets);

for (int i = 0; i < rulesets.Length; i++)
var importedBeatmapSet = Game.BeatmapManager.Import(beatmapSet);

Debug.Assert(importedBeatmapSet != null);

importedBeatmapSet.PerformWrite(s =>
{
var beatmap = beatmapSet.Beatmaps[i];
for (int i = 0; i < rulesets.Length; i++)
{
var beatmap = s.Beatmaps[i];

beatmap.StarRating = i + 1;
beatmap.DifficultyName = $"SR{i + 1}";
}
beatmap.StarRating = i + 1;
beatmap.DifficultyName = $"SR{i + 1}";
}
});

return Game.BeatmapManager.Import(beatmapSet)?.Value;
return importedBeatmapSet.Value;
}

private bool ensureAllBeatmapSetsImported(IEnumerable<BeatmapSetInfo> beatmapSets) => beatmapSets.All(set => set != null);
Expand Down
5 changes: 5 additions & 0 deletions osu.Game/Beatmaps/BeatmapDifficultyCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ protected override void LoadComplete()
}, true);
}

public void Invalidate(IBeatmapInfo beatmap)
{
base.Invalidate(lookup => lookup.BeatmapInfo.Equals(beatmap));
}

/// <summary>
/// Retrieves a bindable containing the star difficulty of a <see cref="BeatmapInfo"/> that follows the currently-selected ruleset and mods.
/// </summary>
Expand Down
Loading