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.

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

Fix beatmap carousel not preloading panels when off-screen #26385

Merged
merged 4 commits into from
Jan 8, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 4, 2024

Something I noticed in passing that felt like it deserved 45 minutes of attention.

Before:

CleanShot.2024-01-04.at.10.16.09.mp4

After:

CleanShot.2024-01-04.at.10.22.56.mp4

The preload numbers are still on the conservative side. We can probably adjust these based on the system the game is running on in the future. Or across the board. But this should improve the overall responsiveness of carousel loading, something that's been bugging me for quite a while now (and not just me)

For testing purposes (to see when loads are triggered):

diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
index 119818242e..328678cd2b 100644
--- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
+++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
@@ -13,6 +13,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.Primitives;
+using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Utils;
 using osu.Game.Beatmaps;
@@ -20,6 +21,7 @@
 using osu.Game.Database;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Overlays;
+using osuTK.Graphics;
 
 namespace osu.Game.Screens.Select.Carousel
 {
@@ -203,6 +205,14 @@ private void loadContentIfRequired()
             {
                 loadCancellation = new CancellationTokenSource();
 
+                var drawable = new Box
+                {
+                    Colour = Color4.Red,
+                    RelativeSizeAxes = Axes.Both,
+                };
+
+                Header.Add(drawable);
+
                 LoadComponentsAsync(new CompositeDrawable[]
                 {
                     new SetPanelBackground(manager.GetWorkingBeatmap(beatmapSet.Beatmaps.MinBy(b => b.OnlineID)))
@@ -218,6 +228,7 @@ private void loadContentIfRequired()
                 {
                     Header.AddRange(drawables);
                     drawables.ForEach(d => d.FadeInFromZero(150));
+                    drawable.Colour = Color4.White;
                 }, loadCancellation.Token);
             }
         }

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

don't get it

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

i worry about the performance implications of this but we won't see how it goes without trying it i guess

@bdach bdach merged commit 70ba5dd into ppy:master Jan 8, 2024
@peppy peppy deleted the carousel-thing branch January 12, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants