Skip to content

Fix local beatmap metadata not being cleared when previously selected beatmap is online #36632

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

Merged
peppy merged 3 commits into from
Feb 9, 2026
Merged

Fix local beatmap metadata not being cleared when previously selected beatmap is online #36632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
540db68
Add failing asserts and make bindable value flow more in line with game
Joehuu Feb 8, 2026
b79e2ba
Fix local beatmap metadata not being cleared when previously selected…
Joehuu Feb 8, 2026
2e116b7
Only call `updateDisplay()` once per beatmap and online lookup
Joehuu Feb 8, 2026
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
38 changes: 24 additions & 14 deletions osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapMetadataWedge.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics.Containers;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Extensions;
using osu.Game.Graphics.Sprites;
using osu.Game.Models;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Screens.SelectV2;
Expand Down Expand Up @@ -53,8 +55,8 @@ public void TestVariousMetrics()

working.Metadata.Source = string.Empty;

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
AddStep("no success rate", () =>
{
Expand All @@ -63,26 +65,26 @@ public void TestVariousMetrics()
online.Result!.Beatmaps.Single().PlayCount = 0;
online.Result!.Beatmaps.Single().PassCount = 0;

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
AddStep("no user ratings", () =>
{
var (working, online) = createTestBeatmap();

online.Result!.Ratings = Array.Empty<int>();

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
AddStep("no fail times", () =>
{
var (working, online) = createTestBeatmap();

online.Result!.Beatmaps.Single().FailTimes = null;

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
AddStep("no metrics", () =>
{
Expand All @@ -91,17 +93,17 @@ public void TestVariousMetrics()
online.Result!.Ratings = Array.Empty<int>();
online.Result!.Beatmaps.Single().FailTimes = null;

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
AddStep("local beatmap", () =>
{
var (working, _) = createTestBeatmap();

working.BeatmapInfo.OnlineID = 0;

onlineLookupResult.Value = null;
Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.Completed(null);
});
}

Expand All @@ -119,8 +121,8 @@ public void TestTruncation()
online.Result!.Language = new BeatmapSetOnlineLanguage { Id = 12, Name = "Verrrrryyyy llooonngggggg language" };
online.Result!.Beatmaps.Single().TopTags = Enumerable.Repeat(online.Result!.Beatmaps.Single().TopTags, 3).SelectMany(t => t!).ToArray();

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
}

Expand All @@ -137,20 +139,28 @@ public void TestOnlineAvailability()

working.BeatmapInfo.ResetOnlineInfo();

onlineLookupResult.Value = lookupResult;
Beatmap.Value = working;
onlineLookupResult.Value = lookupResult;
});
AddUntilStep("rating wedge hidden", () => !wedge.RatingsVisible);
AddUntilStep("fail time wedge hidden", () => !wedge.FailRetryVisible);

// just check for text everywhere on the wedge as the classes are private and generic
Copy link
Copy Markdown
Member

@peppy peppy Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bit dodge but worst case it'll yell at us so 🤷

AddAssert("genre is still visible", () => wedge.ChildrenOfType<OsuSpriteText>().Any(t => t.Text == "Pop"));
AddAssert("language is still visible", () => wedge.ChildrenOfType<OsuSpriteText>().Any(t => t.Text == "English"));

AddStep("local beatmap", () =>
{
var (working, _) = createTestBeatmap();

onlineLookupResult.Value = null;
Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.Completed(null);
});
AddAssert("rating wedge still hidden", () => !wedge.RatingsVisible);
AddAssert("fail time wedge still hidden", () => !wedge.FailRetryVisible);

AddAssert("genre is cleared", () => wedge.ChildrenOfType<OsuSpriteText>().All(t => t.Text != "Pop"));
AddAssert("language is cleared", () => wedge.ChildrenOfType<OsuSpriteText>().All(t => t.Text != "English"));
}

[Test]
Expand All @@ -166,8 +176,8 @@ public void TestUserTags()
online.Result!.RelatedTags = null;
working.BeatmapSetInfo.Beatmaps.Single().Metadata.UserTags.Clear();

onlineLookupResult.Value = online;
Beatmap.Value = working;
onlineLookupResult.Value = online;
});
}

Expand All @@ -178,9 +188,9 @@ public void TestLoading()
{
var (working, online) = createTestBeatmap();

Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.InProgress();
Scheduler.AddDelayed(() => onlineLookupResult.Value = online, 500);
Beatmap.Value = working;
});
AddWaitStep("wait", 5);

Expand All @@ -192,9 +202,9 @@ public void TestLoading()
online.Result!.RelatedTags[1].Name = "another/tag";
online.Result!.RelatedTags[2].Name = "some/tag";

Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.InProgress();
Scheduler.AddDelayed(() => onlineLookupResult.Value = online, 500);
Beatmap.Value = working;
});
AddWaitStep("wait", 5);

Expand All @@ -206,9 +216,9 @@ public void TestLoading()
online.Result!.RelatedTags = null;
working.BeatmapSetInfo.Beatmaps.Single().Metadata.UserTags.Clear();

Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.InProgress();
Scheduler.AddDelayed(() => onlineLookupResult.Value = online, 500);
Beatmap.Value = working;
});
AddWaitStep("wait", 5);

Expand All @@ -220,9 +230,9 @@ public void TestLoading()
online.Result!.RelatedTags = null;
working.BeatmapSetInfo.Beatmaps.Single().Metadata.UserTags.Clear();

Beatmap.Value = working;
onlineLookupResult.Value = Screens.SelectV2.SongSelect.BeatmapSetLookupResult.InProgress();
Scheduler.AddDelayed(() => onlineLookupResult.Value = online, 500);
Beatmap.Value = working;
});
AddWaitStep("wait", 5);
}
Expand Down
4 changes: 2 additions & 2 deletions osu.Game/Screens/SelectV2/BeatmapMetadataWedge.MetadataDisplay.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,13 @@ private void setText(LocalisableString text)
contentText.Text = text;
}

private void setLink(LocalisableString text, Action action) => Schedule(() =>
private void setLink(LocalisableString text, Action action)
{
clear();

contentLinkText.Text = text;
contentLink.Action = action;
});
}

private void setDate(DateTimeOffset date)
{
Expand Down
4 changes: 2 additions & 2 deletions osu.Game/Screens/SelectV2/BeatmapMetadataWedge.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ private void load(AudioManager audio)
protected override void LoadComplete()
{
base.LoadComplete();
beatmap.BindValueChanged(_ => updateDisplay());
onlineLookupResult.BindValueChanged(_ => updateDisplay());
beatmap.BindValueChanged(_ => Scheduler.AddOnce(updateDisplay));
onlineLookupResult.BindValueChanged(_ => Scheduler.AddOnce(updateDisplay));

apiState = api.State.GetBoundCopy();
apiState.BindValueChanged(_ => Scheduler.AddOnce(updateDisplay), true);
Expand Down
Loading