Merge pull request #30874 from peppy/chat-order

Sort public chat channels alphabetically, private channels based on recent messages

osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs

@@ -164,10 +164,10 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy
 
         private Drawable getResult(HitResult result)
         {
-            if (!hit_result_mapping.ContainsKey(result))
+            if (!hit_result_mapping.TryGetValue(result, out var value))
                 return null;
 
-            string filename = this.GetManiaSkinConfig<string>(hit_result_mapping[result])?.Value
+            string filename = this.GetManiaSkinConfig<string>(value)?.Value
                               ?? default_hit_result_skin_filenames[result];
 
             var animation = this.GetAnimation(filename, true, true, frameLength: 1000 / 20d);

osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs

@@ -457,6 +457,61 @@ namespace osu.Game.Tests.Visual.Online
             waitForChannel1Visible();
         }
 
+        [Test]
+        public void TestPublicChannelsSortedByName()
+        {
+            // Intentionally join back to front.
+            AddStep("Show overlay with channel 2", () =>
+            {
+                channelManager.CurrentChannel.Value = channelManager.JoinChannel(testChannel2);
+                chatOverlay.Show();
+            });
+            AddUntilStep("second channel is at top of list", () => getFirstVisiblePublicChannel().Channel == testChannel2);
+
+            AddStep("Join channel 1", () => channelManager.JoinChannel(testChannel1));
+            AddUntilStep("first channel is at top of list", () => getFirstVisiblePublicChannel().Channel == testChannel1);
+
+            AddStep("message in channel 2", () =>
+            {
+                testChannel2.AddNewMessages(new Message(1) { Content = "hi!", Sender = new APIUser { Username = "person" } });
+            });
+            AddUntilStep("first channel still at top of list", () => getFirstVisiblePublicChannel().Channel == testChannel1);
+
+            ChannelListItem getFirstVisiblePublicChannel() =>
+                chatOverlay.ChildrenOfType<ChannelList>().Single().PublicChannelGroup.ItemFlow.FlowingChildren.OfType<ChannelListItem>().First(item => item.Channel.Type == ChannelType.Public);
+        }
+
+        [Test]
+        public void TestPrivateChannelsSortedByRecent()
+        {
+            Channel pmChannel1 = createPrivateChannel();
+            Channel pmChannel2 = createPrivateChannel();
+
+            joinChannel(pmChannel1);
+            joinChannel(pmChannel2);
+
+            AddStep("Show overlay", () => chatOverlay.Show());
+
+            AddUntilStep("first channel is at top of list", () => getFirstVisiblePMChannel().Channel == pmChannel1);
+
+            AddStep("message in channel 2", () =>
+            {
+                pmChannel2.AddNewMessages(new Message(1) { Content = "hi!", Sender = new APIUser { Username = "person" } });
+            });
+
+            AddUntilStep("wait for first channel raised to top of list", () => getFirstVisiblePMChannel().Channel == pmChannel2);
+
+            AddStep("message in channel 1", () =>
+            {
+                pmChannel1.AddNewMessages(new Message(1) { Content = "hi!", Sender = new APIUser { Username = "person" } });
+            });
+
+            AddUntilStep("wait for first channel raised to top of list", () => getFirstVisiblePMChannel().Channel == pmChannel1);
+
+            ChannelListItem getFirstVisiblePMChannel() =>
+                chatOverlay.ChildrenOfType<ChannelList>().Single().PrivateChannelGroup.ItemFlow.FlowingChildren.OfType<ChannelListItem>().First(item => item.Channel.Type == ChannelType.PM);
+        }
+
         [Test]
         public void TestKeyboardNewChannel()
         {

osu.Game/Online/Chat/Channel.cs

@@ -161,7 +161,7 @@ namespace osu.Game.Online.Chat
             Messages.AddRange(messages);
 
             long? maxMessageId = messages.Max(m => m.Id);
-            if (maxMessageId > LastMessageId)
+            if (LastMessageId == null || maxMessageId > LastMessageId)
                 LastMessageId = maxMessageId;
 
             purgeOldMessages();

osu.Game/Overlays/Chat/ChannelList/ChannelList.cs

@@ -37,11 +37,13 @@ namespace osu.Game.Overlays.Chat.ChannelList
 
         private readonly Dictionary<Channel, ChannelListItem> channelMap = new Dictionary<Channel, ChannelListItem>();
 
+        public ChannelGroup AnnounceChannelGroup { get; private set; } = null!;
+        public ChannelGroup PublicChannelGroup { get; private set; } = null!;
+        public ChannelGroup PrivateChannelGroup { get; private set; } = null!;
+
         private OsuScrollContainer scroll = null!;
         private SearchContainer groupFlow = null!;
-        private ChannelGroup announceChannelGroup = null!;
+
-        private ChannelGroup publicChannelGroup = null!;
-        private ChannelGroup privateChannelGroup = null!;
         private ChannelListItem selector = null!;
         private TextBox searchTextBox = null!;
 
@@ -77,10 +79,10 @@ namespace osu.Game.Overlays.Chat.ChannelList
                                     RelativeSizeAxes = Axes.X,
                                 }
                             },
-                            announceChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitleANNOUNCE.ToUpper()),
+                            AnnounceChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitleANNOUNCE.ToUpper(), false),
-                            publicChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitlePUBLIC.ToUpper()),
+                            PublicChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitlePUBLIC.ToUpper(), false),
                             selector = new ChannelListItem(ChannelListingChannel),
-                            privateChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitlePM.ToUpper()),
+                            PrivateChannelGroup = new ChannelGroup(ChatStrings.ChannelsListTitlePM.ToUpper(), true),
                         },
                     },
                 },
@@ -111,69 +113,70 @@ namespace osu.Game.Overlays.Chat.ChannelList
             item.OnRequestSelect += chan => OnRequestSelect?.Invoke(chan);
             item.OnRequestLeave += chan => OnRequestLeave?.Invoke(chan);
 
-            FillFlowContainer<ChannelListItem> flow = getFlowForChannel(channel);
+            ChannelGroup group = getGroupFromChannel(channel);
             channelMap.Add(channel, item);
-            flow.Add(item);
+            group.AddChannel(item);
 
             updateVisibility();
         }
 
         public void RemoveChannel(Channel channel)
         {
-            if (!channelMap.ContainsKey(channel))
+            if (!channelMap.TryGetValue(channel, out var item))
                 return;
 
-            ChannelListItem item = channelMap[channel];
+            ChannelGroup group = getGroupFromChannel(channel);
-            FillFlowContainer<ChannelListItem> flow = getFlowForChannel(channel);
 
             channelMap.Remove(channel);
-            flow.Remove(item, true);
+            group.RemoveChannel(item);
 
             updateVisibility();
         }
 
         public ChannelListItem GetItem(Channel channel)
         {
-            if (!channelMap.ContainsKey(channel))
+            if (!channelMap.TryGetValue(channel, out var item))
                 throw new ArgumentOutOfRangeException();
 
-            return channelMap[channel];
+            return item;
         }
 
         public void ScrollChannelIntoView(Channel channel) => scroll.ScrollIntoView(GetItem(channel));
 
-        private FillFlowContainer<ChannelListItem> getFlowForChannel(Channel channel)
+        private ChannelGroup getGroupFromChannel(Channel channel)
         {
             switch (channel.Type)
             {
                 case ChannelType.Public:
-                    return publicChannelGroup.ItemFlow;
+                    return PublicChannelGroup;
 
                 case ChannelType.PM:
-                    return privateChannelGroup.ItemFlow;
+                    return PrivateChannelGroup;
 
                 case ChannelType.Announce:
-                    return announceChannelGroup.ItemFlow;
+                    return AnnounceChannelGroup;
 
                 default:
-                    return publicChannelGroup.ItemFlow;
+                    return PublicChannelGroup;
             }
         }
 
         private void updateVisibility()
         {
-            if (announceChannelGroup.ItemFlow.Children.Count == 0)
+            if (AnnounceChannelGroup.ItemFlow.Children.Count == 0)
-                announceChannelGroup.Hide();
+                AnnounceChannelGroup.Hide();
             else
-                announceChannelGroup.Show();
+                AnnounceChannelGroup.Show();
         }
 
-        private partial class ChannelGroup : FillFlowContainer
+        public partial class ChannelGroup : FillFlowContainer
         {
-            public readonly FillFlowContainer<ChannelListItem> ItemFlow;
+            private readonly bool sortByRecent;
+            public readonly ChannelListItemFlow ItemFlow;
 
-            public ChannelGroup(LocalisableString label)
+            public ChannelGroup(LocalisableString label, bool sortByRecent)
             {
+                this.sortByRecent = sortByRecent;
                 Direction = FillDirection.Vertical;
                 RelativeSizeAxes = Axes.X;
                 AutoSizeAxes = Axes.Y;
@@ -187,7 +190,7 @@ namespace osu.Game.Overlays.Chat.ChannelList
                         Margin = new MarginPadding { Left = 18, Bottom = 5 },
                         Font = OsuFont.Torus.With(size: 12, weight: FontWeight.SemiBold),
                     },
-                    ItemFlow = new FillFlowContainer<ChannelListItem>
+                    ItemFlow = new ChannelListItemFlow(sortByRecent)
                     {
                         Direction = FillDirection.Vertical,
                         RelativeSizeAxes = Axes.X,
@@ -195,6 +198,60 @@ namespace osu.Game.Overlays.Chat.ChannelList
                     },
                 };
             }
+
+            public partial class ChannelListItemFlow : FillFlowContainer<ChannelListItem>
+            {
+                private readonly bool sortByRecent;
+
+                public ChannelListItemFlow(bool sortByRecent)
+                {
+                    this.sortByRecent = sortByRecent;
+                }
+
+                public void Reflow() => InvalidateLayout();
+
+                public override IEnumerable<Drawable> FlowingChildren => sortByRecent
+                    ? base.FlowingChildren.OfType<ChannelListItem>().OrderByDescending(i => i.Channel.LastMessageId ?? long.MinValue)
+                    : base.FlowingChildren.OfType<ChannelListItem>().OrderBy(i => i.Channel.Name);
+            }
+
+            public void AddChannel(ChannelListItem item)
+            {
+                ItemFlow.Add(item);
+
+                if (sortByRecent)
+                {
+                    item.Channel.NewMessagesArrived += newMessagesArrived;
+                    item.Channel.PendingMessageResolved += pendingMessageResolved;
+                }
+
+                ItemFlow.Reflow();
+            }
+
+            public void RemoveChannel(ChannelListItem item)
+            {
+                if (sortByRecent)
+                {
+                    item.Channel.NewMessagesArrived -= newMessagesArrived;
+                    item.Channel.PendingMessageResolved -= pendingMessageResolved;
+                }
+
+                ItemFlow.Remove(item, true);
+            }
+
+            private void pendingMessageResolved(LocalEchoMessage _, Message __) => ItemFlow.Reflow();
+            private void newMessagesArrived(IEnumerable<Message> _) => ItemFlow.Reflow();
+
+            protected override void Dispose(bool isDisposing)
+            {
+                base.Dispose(isDisposing);
+
+                foreach (var item in ItemFlow)
+                {
+                    item.Channel.NewMessagesArrived -= newMessagesArrived;
+                    item.Channel.PendingMessageResolved -= pendingMessageResolved;
+                }
+            }
         }
 
         private partial class ChannelSearchTextBox : BasicSearchTextBox

osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs

@@ -78,12 +78,12 @@ namespace osu.Game.Tests.Visual.Spectator
         /// <param name="state">The spectator state to end play with.</param>
         public void SendEndPlay(int userId, SpectatedUserState state = SpectatedUserState.Quit)
         {
-            if (!userBeatmapDictionary.ContainsKey(userId))
+            if (!userBeatmapDictionary.TryGetValue(userId, out int value))
                 return;
 
             ((ISpectatorClient)this).UserFinishedPlaying(userId, new SpectatorState
             {
-                BeatmapID = userBeatmapDictionary[userId],
+                BeatmapID = value,
                 RulesetID = 0,
                 Mods = userModsDictionary[userId],
                 State = state