Skip to content

Commit

Permalink
Merge pull request #30526 from smoogipoo/fix-beatmap-recommender-test
Browse files Browse the repository at this point in the history 
Fix intermittent beatmap recommendations test
  • Loading branch information
@peppy
peppy authored Nov 7, 2024
2 parents 35397d9 + c7d0a7d commit aad9f40
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 52 deletions.
36 changes: 35 additions & 1 deletion osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Extensions.IEnumerableExtensions;
using osu.Framework.Platform;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Database;
using osu.Game.Extensions;
using osu.Game.Online.API;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Catch;
using osu.Game.Rulesets.Mania;
Expand Down Expand Up @@ -191,8 +194,39 @@ private void presentAndConfirm(Func<BeatmapSetInfo> getImport, int expectedDiff)
{
AddStep("present beatmap", () => Game.PresentBeatmap(getImport()));

AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect);
AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect select && select.BeatmapSetsLoaded);
AddUntilStep("recommended beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.MatchesOnlineID(getImport().Beatmaps[expectedDiff - 1]));
}

protected override TestOsuGame CreateTestGame() => new NoBeatmapUpdateGame(LocalStorage, API);

private partial class NoBeatmapUpdateGame : TestOsuGame
{
public NoBeatmapUpdateGame(Storage storage, IAPIProvider api, string[] args = null)
: base(storage, api, args)
{
}

protected override IBeatmapUpdater CreateBeatmapUpdater() => new TestBeatmapUpdater();

private class TestBeatmapUpdater : IBeatmapUpdater
{
public void Queue(Live<BeatmapSetInfo> beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
}

public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
}

public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
}

public void Dispose()
{
}
}
}
}
}
4 changes: 2 additions & 2 deletions osu.Game/Beatmaps/BeatmapOnlineChangeIngest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ namespace osu.Game.Beatmaps
/// </summary>
public partial class BeatmapOnlineChangeIngest : Component
{
private readonly BeatmapUpdater beatmapUpdater;
private readonly IBeatmapUpdater beatmapUpdater;
private readonly RealmAccess realm;
private readonly MetadataClient metadataClient;

public BeatmapOnlineChangeIngest(BeatmapUpdater beatmapUpdater, RealmAccess realm, MetadataClient metadataClient)
public BeatmapOnlineChangeIngest(IBeatmapUpdater beatmapUpdater, RealmAccess realm, MetadataClient metadataClient)
{
this.beatmapUpdater = beatmapUpdater;
this.realm = realm;
Expand Down
84 changes: 38 additions & 46 deletions osu.Game/Beatmaps/BeatmapUpdater.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
Expand All @@ -15,10 +14,7 @@

namespace osu.Game.Beatmaps
{
/// <summary>
/// Handles all processing required to ensure a local beatmap is in a consistent state with any changes.
/// </summary>
public class BeatmapUpdater : IDisposable
public class BeatmapUpdater : IBeatmapUpdater
{
private readonly IWorkingBeatmapCache workingBeatmapCache;

Expand All @@ -38,67 +34,63 @@ public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapDifficult
metadataLookup = new BeatmapUpdaterMetadataLookup(api, storage);
}

/// <summary>
/// Queue a beatmap for background processing.
/// </summary>
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
public void Queue(Live<BeatmapSetInfo> beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
Logger.Log($"Queueing change for local beatmap {beatmapSet}");
Task.Factory.StartNew(() => beatmapSet.PerformRead(b => Process(b, lookupScope)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously,
updateScheduler);
}

/// <summary>
/// Run all processing on a beatmap immediately.
/// </summary>
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapSet.Realm!.Write(_ =>
public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
// Before we use below, we want to invalidate.
workingBeatmapCache.Invalidate(beatmapSet);
beatmapSet.Realm!.Write(_ =>
{
// Before we use below, we want to invalidate.
workingBeatmapCache.Invalidate(beatmapSet);

if (lookupScope != MetadataLookupScope.None)
metadataLookup.Update(beatmapSet, lookupScope == MetadataLookupScope.OnlineFirst);
if (lookupScope != MetadataLookupScope.None)
metadataLookup.Update(beatmapSet, lookupScope == MetadataLookupScope.OnlineFirst);

foreach (var beatmap in beatmapSet.Beatmaps)
{
difficultyCache.Invalidate(beatmap);
foreach (var beatmap in beatmapSet.Beatmaps)
{
difficultyCache.Invalidate(beatmap);

var working = workingBeatmapCache.GetWorkingBeatmap(beatmap);
var ruleset = working.BeatmapInfo.Ruleset.CreateInstance();
var working = workingBeatmapCache.GetWorkingBeatmap(beatmap);
var ruleset = working.BeatmapInfo.Ruleset.CreateInstance();

Debug.Assert(ruleset != null);
Debug.Assert(ruleset != null);

var calculator = ruleset.CreateDifficultyCalculator(working);
var calculator = ruleset.CreateDifficultyCalculator(working);

beatmap.StarRating = calculator.Calculate().StarRating;
beatmap.Length = working.Beatmap.CalculatePlayableLength();
beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength();
beatmap.EndTimeObjectCount = working.Beatmap.HitObjects.Count(h => h is IHasDuration);
beatmap.TotalObjectCount = working.Beatmap.HitObjects.Count;
}
beatmap.StarRating = calculator.Calculate().StarRating;
beatmap.Length = working.Beatmap.CalculatePlayableLength();
beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength();
beatmap.EndTimeObjectCount = working.Beatmap.HitObjects.Count(h => h is IHasDuration);
beatmap.TotalObjectCount = working.Beatmap.HitObjects.Count;
}

// And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required.
workingBeatmapCache.Invalidate(beatmapSet);
});
// And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required.
workingBeatmapCache.Invalidate(beatmapSet);
});
}

public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst) => beatmapInfo.Realm!.Write(_ =>
public void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst)
{
// Before we use below, we want to invalidate.
workingBeatmapCache.Invalidate(beatmapInfo);
beatmapInfo.Realm!.Write(_ =>
{
// Before we use below, we want to invalidate.
workingBeatmapCache.Invalidate(beatmapInfo);

var working = workingBeatmapCache.GetWorkingBeatmap(beatmapInfo);
var beatmap = working.Beatmap;
var working = workingBeatmapCache.GetWorkingBeatmap(beatmapInfo);
var beatmap = working.Beatmap;

beatmapInfo.EndTimeObjectCount = beatmap.HitObjects.Count(h => h is IHasDuration);
beatmapInfo.TotalObjectCount = beatmap.HitObjects.Count;
beatmapInfo.EndTimeObjectCount = beatmap.HitObjects.Count(h => h is IHasDuration);
beatmapInfo.TotalObjectCount = beatmap.HitObjects.Count;

// And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required.
workingBeatmapCache.Invalidate(beatmapInfo);
});
// And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required.
workingBeatmapCache.Invalidate(beatmapInfo);
});
}

#region Implementation of IDisposable

Expand Down
35 changes: 35 additions & 0 deletions osu.Game/Beatmaps/IBeatmapUpdater.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using osu.Game.Database;

namespace osu.Game.Beatmaps
{
/// <summary>
/// Handles all processing required to ensure a local beatmap is in a consistent state with any changes.
/// </summary>
public interface IBeatmapUpdater : IDisposable
{
/// <summary>
/// Queue a beatmap for background processing.
/// </summary>
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
void Queue(Live<BeatmapSetInfo> beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst);

/// <summary>
/// Run all processing on a beatmap immediately.
/// </summary>
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst);

/// <summary>
/// Runs a subset of processing focused on updating any cached beatmap object counts.
/// </summary>
/// <param name="beatmapInfo">The managed beatmap to update. A transaction will be opened to apply changes.</param>
/// <param name="lookupScope">The preferred scope to use for metadata lookup.</param>
void ProcessObjectCounts(BeatmapInfo beatmapInfo, MetadataLookupScope lookupScope = MetadataLookupScope.LocalCacheFirst);
}
}
2 changes: 1 addition & 1 deletion osu.Game/Database/BackgroundDataStoreProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public partial class BackgroundDataStoreProcessor : Component
private RealmAccess realmAccess { get; set; } = null!;

[Resolved]
private BeatmapUpdater beatmapUpdater { get; set; } = null!;
private IBeatmapUpdater beatmapUpdater { get; set; } = null!;

[Resolved]
private IBindable<WorkingBeatmap> gameBeatmap { get; set; } = null!;
Expand Down
6 changes: 4 additions & 2 deletions osu.Game/OsuGameBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public virtual string Version
public readonly Bindable<Dictionary<ModType, IReadOnlyList<Mod>>> AvailableMods = new Bindable<Dictionary<ModType, IReadOnlyList<Mod>>>(new Dictionary<ModType, IReadOnlyList<Mod>>());

private BeatmapDifficultyCache difficultyCache;
private BeatmapUpdater beatmapUpdater;
private IBeatmapUpdater beatmapUpdater;

private UserLookupCache userCache;
private BeatmapLookupCache beatmapCache;
Expand Down Expand Up @@ -324,7 +324,7 @@ private void load(ReadableKeyCombinationProvider keyCombinationProvider, Framewo
base.Content.Add(difficultyCache);

// TODO: OsuGame or OsuGameBase?
dependencies.CacheAs(beatmapUpdater = new BeatmapUpdater(BeatmapManager, difficultyCache, API, Storage));
dependencies.CacheAs(beatmapUpdater = CreateBeatmapUpdater());
dependencies.CacheAs(SpectatorClient = new OnlineSpectatorClient(endpoints));
dependencies.CacheAs(MultiplayerClient = new OnlineMultiplayerClient(endpoints));
dependencies.CacheAs(metadataClient = new OnlineMetadataClient(endpoints));
Expand Down Expand Up @@ -563,6 +563,8 @@ public bool Migrate(string path)
}
}

protected virtual IBeatmapUpdater CreateBeatmapUpdater() => new BeatmapUpdater(BeatmapManager, difficultyCache, API, Storage);

protected override UserInputManager CreateUserInputManager() => new OsuUserInputManager();

protected virtual BatteryInfo CreateBatteryInfo() => null;
Expand Down

0 comments on commit aad9f40

Please sign in to comment.