h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2026-05-13 20:33:35 +08:00
Code Issues Releases Wiki Activity

Fix crash when dragging card as discard phase ends (#37419)

Browse Source 
Closes: #37402

Issue was `HandOfCards.CardContainer.Compare` working under the
assumption that the it would never be called with the same child for
both entries, which can happen when doing a `BinarySearch` (called in
`RemoveInternal`).
This lead to `IndexOf` returning a negative value despite the card being
present in the container, and the drawable getting disposed but not
actually removed.

I also included a precautionary `cardContainer.Sort()` call before the
removal as well, seemed to work without that from testing but better not
rely on that.

TLDR: card container child sorting was unstable due to poor assumptions
This commit is contained in:
maarvin
2026-04-20 10:58:27 +02:00
committed by GitHub
Unverified
parent 48434dd683
commit 11f01113a1
2 changed files with 29 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs
+19
View File
@@ -192,5 +192,24 @@ namespace osu.Game.Tests.Visual.RankedPlay
.All(card => !card.ScreenSpaceDrawQuad.AABBFloat.IntersectsWith(handOfCards.ScreenSpaceDrawQuad.AABBFloat))
);
}
[Test]
public void TestRemoveCardsWhileDragging()
{
AddStep("add cards", () =>
{
for (int i = 0; i < 5; i++)
handOfCards.AddCard(new RankedPlayCardWithPlaylistItem(new RankedPlayCardItem()));
});
AddStep("hover card", () => InputManager.MoveMouseTo(handOfCards.Cards.First()));
AddStep("start drag", () => InputManager.PressButton(MouseButton.Left));
AddStep("move card", () => InputManager.MoveMouseTo(handOfCards.Cards[3]));
AddStep("remove cards", () =>
{
foreach (var card in handOfCards.Cards.ToArray())
handOfCards.RemoveCard(card.Item);
});
AddStep("release mouse", () => InputManager.ReleaseButton(MouseButton.Left));
}
}
}
osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs
+10 -6
View File
@@ -138,6 +138,9 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand
if (!cardLookup.Remove(item.Card, out var drawable))
return false;
// child order is only updated once per frame so ordering can change between that and the card getting removed
// which can mess when doing a binary-search for the child during removal
cardContainer.Sort();
cardContainer.Remove(drawable, true);
InvalidateLayout(drawOrder: true);
return true;
@@ -162,6 +165,9 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand
screenSpaceDrawQuad = drawable.ScreenSpaceDrawQuad;
card = drawable.Detach();
// child order is only updated once per frame so ordering can change between that and the card getting removed
// which can mess when doing a binary-search for the child during removal
cardContainer.Sort();
cardContainer.Remove(drawable, true);
InvalidateLayout(drawOrder: true);
@@ -352,13 +358,11 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand
if (x is HandCard c1 && y is HandCard c2)
{
// dragged cards should always be drawn on top
if (c1.CardDragged)
return 1;
int result = c1.CardDragged.CompareTo(c2.CardDragged);
if (result != 0)
return result;
if (c2.CardDragged)
return -1;
int result = c1.Order.CompareTo(c2.Order);
result = c1.Order.CompareTo(c2.Order);
if (result != 0)
return result;
}