1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-11 09:27:29 +08:00

Fix UpdateBeatmapSetButton intermittent test failure

Carousel would only expire items when off-screen. This meant that for a
case (like a test) where items are generally always on-screen,
`UpdateBeatmapSet` calls would result in panels remaining hidden but not
cleaned up.
This commit is contained in:
Dean Herbert 2022-07-21 16:24:46 +09:00
parent 3cfe624af1
commit fc0c9f76bd
2 changed files with 22 additions and 7 deletions

View File

@ -71,6 +71,7 @@ namespace osu.Game.Tests.Visual.SongSelect
carousel.UpdateBeatmapSet(testBeatmapSetInfo);
});
AddUntilStep("only one set visible", () => carousel.ChildrenOfType<DrawableCarouselBeatmapSet>().Count() == 1);
AddUntilStep("update button visible", () => getUpdateButton() != null);
AddStep("click button", () => getUpdateButton()?.TriggerClick());
@ -120,6 +121,7 @@ namespace osu.Game.Tests.Visual.SongSelect
carousel.UpdateBeatmapSet(testBeatmapSetInfo);
});
AddUntilStep("only one set visible", () => carousel.ChildrenOfType<DrawableCarouselBeatmapSet>().Count() == 1);
AddUntilStep("update button visible", () => getUpdateButton() != null);
AddStep("click button", () => getUpdateButton()?.TriggerClick());

View File

@ -316,8 +316,13 @@ namespace osu.Game.Screens.Select
previouslySelectedID = selectedBeatmap?.BeatmapInfo.ID;
var newSet = createCarouselSet(beatmapSet);
var removedSet = root.RemoveChild(beatmapSet.ID);
root.RemoveChild(beatmapSet.ID);
// If we don't remove this here, it may remain in a hidden state until scrolled off screen.
// Doesn't really affect anything during actual user interaction, but makes testing annoying.
var removedDrawable = Scroll.FirstOrDefault(c => c.Item == removedSet);
if (removedDrawable != null)
expirePanelImmediately(removedDrawable);
if (newSet != null)
{
@ -696,11 +701,7 @@ namespace osu.Game.Screens.Select
// panel loaded as drawable but not required by visible range.
// remove but only if too far off-screen
if (panel.Y + panel.DrawHeight < visibleUpperBound - distance_offscreen_before_unload || panel.Y > visibleBottomBound + distance_offscreen_before_unload)
{
// may want a fade effect here (could be seen if a huge change happens, like a set with 20 difficulties becomes selected).
panel.ClearTransforms();
panel.Expire();
}
expirePanelImmediately(panel);
}
// Add those items within the previously found index range that should be displayed.
@ -730,6 +731,13 @@ namespace osu.Game.Screens.Select
}
}
private static void expirePanelImmediately(DrawableCarouselItem panel)
{
// may want a fade effect here (could be seen if a huge change happens, like a set with 20 difficulties becomes selected).
panel.ClearTransforms();
panel.Expire();
}
private readonly CarouselBoundsItem carouselBoundsItem = new CarouselBoundsItem();
private (int firstIndex, int lastIndex) getDisplayRange()
@ -972,10 +980,15 @@ namespace osu.Game.Screens.Select
base.AddItem(i);
}
public void RemoveChild(Guid beatmapSetID)
public CarouselBeatmapSet RemoveChild(Guid beatmapSetID)
{
if (BeatmapSetsByID.TryGetValue(beatmapSetID, out var carouselBeatmapSet))
{
RemoveItem(carouselBeatmapSet);
return carouselBeatmapSet;
}
return null;
}
public override void RemoveItem(CarouselItem i)