diff --git a/osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs b/osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs index 958877cc8f..1bdd79c10c 100644 --- a/osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs +++ b/osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs @@ -56,14 +56,14 @@ namespace osu.Game.Tests.Visual.RankedPlay }); AddStep("single selection mode", () => handOfCards.SelectionMode = HandSelectionMode.Single); - AddStep("click first card", () => handOfCards.Cards.First().TriggerClick()); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.First().Item])); + AddStep("click first card", () => handOfCards.GetCardsInDisplayOrder()[0].TriggerClick()); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[0].Item])); - AddStep("click second card", () => handOfCards.Cards.ElementAt(1).TriggerClick()); - AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(1).Item])); + AddStep("click second card", () => handOfCards.GetCardsInDisplayOrder()[1].TriggerClick()); + AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[1].Item])); - AddStep("click second card again", () => handOfCards.Cards.ElementAt(1).TriggerClick()); - AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(1).Item])); + AddStep("click second card again", () => handOfCards.GetCardsInDisplayOrder()[1].TriggerClick()); + AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[1].Item])); } [Test] @@ -76,14 +76,14 @@ namespace osu.Game.Tests.Visual.RankedPlay }); AddStep("multi selection mode", () => handOfCards.SelectionMode = HandSelectionMode.Multiple); - AddStep("click first card", () => handOfCards.Cards.First().TriggerClick()); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.First().Item])); + AddStep("click first card", () => handOfCards.GetCardsInDisplayOrder().First().TriggerClick()); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder().First().Item])); - AddStep("click second card", () => handOfCards.Cards.ElementAt(1).TriggerClick()); - AddAssert("both cards selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(0).Item, handOfCards.Cards.ElementAt(1).Item])); + AddStep("click second card", () => handOfCards.GetCardsInDisplayOrder()[1].TriggerClick()); + AddAssert("both cards selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[0].Item, handOfCards.GetCardsInDisplayOrder()[1].Item])); - AddStep("click second card again", () => handOfCards.Cards.ElementAt(1).TriggerClick()); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(0).Item])); + AddStep("click second card again", () => handOfCards.GetCardsInDisplayOrder()[1].TriggerClick()); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[0].Item])); } [Test] @@ -131,20 +131,20 @@ namespace osu.Game.Tests.Visual.RankedPlay Key key = Key.Number1 + i; AddStep($"key {i + 1}", () => InputManager.Key(key)); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(i1).Item])); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[i1].Item])); } AddStep("right arrow", () => InputManager.Key(Key.Right)); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(0).Item])); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[0].Item])); AddStep("right arrow", () => InputManager.Key(Key.Right)); - AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(1).Item])); + AddAssert("second card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[1].Item])); AddStep("left arrow", () => InputManager.Key(Key.Left)); - AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(0).Item])); + AddAssert("first card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[0].Item])); AddStep("left arrow", () => InputManager.Key(Key.Left)); - AddAssert("last card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.Cards.ElementAt(^1).Item])); + AddAssert("last card selected", () => handOfCards.Selection.SequenceEqual([handOfCards.GetCardsInDisplayOrder()[^1].Item])); AddStep("space", () => InputManager.Key(Key.Space)); AddAssert("play action triggered", () => playActionTriggered); @@ -166,11 +166,11 @@ namespace osu.Game.Tests.Visual.RankedPlay Key key = Key.Number1 + i; AddStep($"key {i + 1}", () => InputManager.Key(key)); - AddAssert("card hovered", () => handOfCards.Cards.ElementAt(i1).CardHovered); + AddAssert("card hovered", () => handOfCards.GetCardsInDisplayOrder()[i1].CardHovered); - AddAssert("card not selected", () => !handOfCards.Selection.Contains(handOfCards.Cards.ElementAt(i1).Card.Item)); + AddAssert("card not selected", () => !handOfCards.Selection.Contains(handOfCards.GetCardsInDisplayOrder()[i1].Card.Item)); AddStep("space", () => InputManager.Key(Key.Space)); - AddAssert("card selected", () => handOfCards.Selection.Contains(handOfCards.Cards.ElementAt(i1).Card.Item)); + AddAssert("card selected", () => handOfCards.Selection.Contains(handOfCards.GetCardsInDisplayOrder()[i1].Card.Item)); } } @@ -201,9 +201,9 @@ namespace osu.Game.Tests.Visual.RankedPlay for (int i = 0; i < 5; i++) handOfCards.AddCard(new RankedPlayCardWithPlaylistItem(new RankedPlayCardItem())); }); - AddStep("hover card", () => InputManager.MoveMouseTo(handOfCards.Cards.First())); + AddStep("hover card", () => InputManager.MoveMouseTo(handOfCards.GetCardsInDisplayOrder()[0])); AddStep("start drag", () => InputManager.PressButton(MouseButton.Left)); - AddStep("move card", () => InputManager.MoveMouseTo(handOfCards.Cards[3])); + AddStep("move card", () => InputManager.MoveMouseTo(handOfCards.GetCardsInDisplayOrder()[3])); AddStep("remove cards", () => { foreach (var card in handOfCards.Cards.ToArray()) diff --git a/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs b/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs index 0ad621fbc9..27103371c0 100644 --- a/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs +++ b/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs @@ -7,7 +7,6 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; -using osu.Framework.Caching; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Primitives; @@ -29,7 +28,18 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand private const float card_spacing = -15; - public IReadOnlyList Cards => cardContainer.Children; + /// + /// Cards currently present in this + /// + /// + /// Entries are not sorted by display order + /// + public IEnumerable Cards => cardLookup.Values; + + /// + /// Returns a list of the cards present in this ordered by the cards' + /// + public List GetCardsInDisplayOrder() => Cards.OrderBy(static c => c.Order).ToList(); /// /// How far a card slides upwards when hovered. @@ -66,12 +76,6 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand { base.Update(); - if (!drawOrderBacking.IsValid) - { - cardContainer.Sort(); - drawOrderBacking.Validate(); - } - if (!layoutBacking.IsValid) { updateLayout(); @@ -126,23 +130,25 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand drawable.Order = cardContainer.Max(c => c.Order) + 1; cardContainer.Add(drawable); - InvalidateLayout(drawOrder: true); + cardContainer.Sort(); + InvalidateLayout(); setupAction?.Invoke(drawable); } - public void Clear() => cardContainer.Clear(); + public void Clear() + { + foreach (var card in Cards.ToArray()) + RemoveCard(card.Item); + } public bool RemoveCard(RankedPlayCardWithPlaylistItem item) { 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); + InvalidateLayout(); return true; } @@ -165,11 +171,8 @@ 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); + InvalidateLayout(); return true; } @@ -178,7 +181,9 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand protected virtual void OnCardStateChanged(HandCard card, ValueChangedEvent evt) { - InvalidateLayout(drawOrder: affectsDrawOrder(evt)); + InvalidateLayout(); + if (affectsDrawOrder(evt)) + cardContainer.Sort(); // hovered state can be caused by keyboard focus, in which case we have to clean up after the other cards manually if (evt.NewValue.Hovered) @@ -198,18 +203,11 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand #region Layout private readonly LayoutValue layoutBacking = new LayoutValue(Invalidation.DrawSize | Invalidation.MiscGeometry); - private readonly Cached drawOrderBacking = new Cached(); /// /// Invalidates the layout of the hand of cards, causing a relayout to occur. /// - /// If set to true, also invalidates the draw order of the cards. - protected void InvalidateLayout(bool drawOrder = false) - { - layoutBacking.Invalidate(); - if (drawOrder) - drawOrderBacking.Invalidate(); - } + protected void InvalidateLayout() => layoutBacking.Invalidate(); private void updateLayout() { @@ -217,11 +215,11 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand return; // card container draws dragged card on top so we need to sort those separately - var cards = cardContainer.Children.OrderBy(static c => c.State.Order).ToArray(); + var cards = GetCardsInDisplayOrder(); int activeCardIndex = GetActiveCardIndex(cards); - for (int i = 0; i < cards.Length; i++) + for (int i = 0; i < cards.Count; i++) { var card = cards[i]; diff --git a/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/PlayerHandOfCards.cs b/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/PlayerHandOfCards.cs index 3d1ba32ed9..1e2474ab87 100644 --- a/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/PlayerHandOfCards.cs +++ b/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/PlayerHandOfCards.cs @@ -161,10 +161,15 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand switch (e.Key) { case >= Key.Number1 and <= Key.Number9: - focusCard(e.Key - Key.Number1); + { + int index = e.Key - Key.Number1; + if (GetCardsInDisplayOrder().ElementAtOrDefault(index) is HandCard card) + focusCard(card); return true; + } case Key.Space: + { if (selectionMode == HandSelectionMode.Disabled) return false; @@ -177,6 +182,7 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand card.TriggerClick(); return true; + } case Key.Left: moveCardFocus(-1); @@ -192,30 +198,27 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand private void moveCardFocus(int direction) { - int currentIndex = Cards.ToList().FindIndex(c => c.HasFocus); + var cards = GetCardsInDisplayOrder(); + + int currentIndex = cards.FindIndex(c => c.HasFocus); // default behaviour is to start from either end of the cards if no card is focused currently // in single-selection mode we can however use the current selection as a fallback index if there's no focus if (selectionMode == HandSelectionMode.Single && currentIndex == -1) - currentIndex = Cards.ToList().FindIndex(c => c.Selected); + currentIndex = cards.FindIndex(c => c.Selected); int newIndex = currentIndex + direction; if (newIndex < 0) - newIndex = Cards.Count - 1; - else if (newIndex >= Cards.Count) + newIndex = cards.Count - 1; + else if (newIndex >= cards.Count) newIndex = 0; - focusCard(newIndex); + focusCard(cards[newIndex]); } - private void focusCard(int index) + private void focusCard(HandCard card) { - var card = Cards.ElementAtOrDefault(index); - - if (card == null) - return; - GetContainingFocusManager()?.ChangeFocus(card); if (SelectionMode == HandSelectionMode.Single && !card.Selected) @@ -224,7 +227,7 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand private void cardDragged(PlayerHandCard card, Vector2 screenSpacePosition) { - var cards = Cards.OrderBy(static c => c.Order).ToArray(); + var cards = GetCardsInDisplayOrder(); int newIndex = cardIndexInLayout(cards, card.ScreenSpaceDrawQuad.Centre); @@ -247,9 +250,9 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand c.Item.DisplayOrder = c.Order; } - private int cardIndexInLayout(HandCard[] cards, Vector2 screenSpacePosition) + private int cardIndexInLayout(IReadOnlyList cards, Vector2 screenSpacePosition) { - Debug.Assert(cards.Length > 0); + Debug.Assert(cards.Count > 0); var position = ToLocalSpace(screenSpacePosition) - DrawSize / 2; @@ -258,7 +261,7 @@ namespace osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand int minIndex = 0; float minDistance = float.MaxValue; - for (int i = 0; i < cards.Length; i++) + for (int i = 0; i < cards.Count; i++) { float distance = MathF.Abs(GetCardX(i, activeIndex) - position.X);