WIP changes for code review

osu.Game/Online/Chat/Channel.cs

@@ -44,7 +44,7 @@ namespace osu.Game.Online.Chat
         /// <summary>
         /// An event that fires when new messages arrived.
         /// </summary>
-        public event Action<IEnumerable<Message>> NewMessagesArrived;
+        public event Action<IEnumerable<Message>, bool> NewMessagesArrived;
 
         /// <summary>
         /// An event that fires when a pending message gets resolved.
@@ -58,6 +58,11 @@ namespace osu.Game.Online.Chat
 
         public bool ReadOnly => false; //todo not yet used.
 
+        /// <summary>
+        /// Determines if the channel's previous messages have been loaded.
+        /// </summary>
+        public bool Populated { get; set; } = false;
+
         public override string ToString() => Name;
 
         [JsonProperty(@"name")]
@@ -105,7 +110,7 @@ namespace osu.Game.Online.Chat
             pendingMessages.Add(message);
             Messages.Add(message);
 
-            NewMessagesArrived?.Invoke(new[] { message });
+            NewMessagesArrived?.Invoke(new[] { message }, Populated);
         }
 
         public bool MessagesLoaded;
@@ -118,17 +123,21 @@ namespace osu.Game.Online.Chat
         {
             messages = messages.Except(Messages).ToArray();
 
-            if (messages.Length == 0) return;
+            if (messages.Length != 0)
+            {
+                Messages.AddRange(messages);
 
-            Messages.AddRange(messages);
+                var maxMessageId = messages.Max(m => m.Id);
+                if (maxMessageId > LastMessageId)
+                    LastMessageId = maxMessageId;
 
-            var maxMessageId = messages.Max(m => m.Id);
+                purgeOldMessages();
-            if (maxMessageId > LastMessageId)
-                LastMessageId = maxMessageId;
 
-            purgeOldMessages();
+                NewMessagesArrived?.Invoke(messages, Populated);
+            }
 
-            NewMessagesArrived?.Invoke(messages);
+            if (!Populated)
+                Populated = true;
         }
 
         /// <summary>

osu.Game/Online/Chat/ChannelManager.cs

@@ -49,10 +49,6 @@ namespace osu.Game.Online.Chat
         public IBindableList<Channel> AvailableChannels => availableChannels;
 
         private IAPIProvider api;
-
-        [Resolved]
-        private MessageNotifier messageNotifier { get; set; }
-
         public readonly BindableBool HighPollRate = new BindableBool();
 
         public ChannelManager()
@@ -258,7 +254,6 @@ namespace osu.Game.Online.Chat
 
                 var groupArray = group.ToArray();
                 channel.AddNewMessages(groupArray);
-                messageNotifier.HandleMessages(channel, groupArray);
             }
         }
 

osu.Game/Online/Chat/MessageNotifier.cs

@@ -8,6 +8,7 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Logging;
 using osu.Game.Configuration;
 using osu.Game.Graphics;
 using osu.Game.Online.API;
@@ -31,9 +32,6 @@ namespace osu.Game.Online.Chat
         [Resolved(CanBeNull = true)]
         private ChannelManager channelManager { get; set; }
 
-        [Resolved]
-        private OsuColour colours { get; set; }
-
         private Bindable<bool> notifyOnMention;
         private Bindable<bool> notifyOnChat;
         private Bindable<string> highlightWords;
@@ -45,12 +43,53 @@ namespace osu.Game.Online.Chat
         public bool IsActive => chatOverlay?.IsPresent == true;
 
         [BackgroundDependencyLoader]
-        private void load(OsuColour colours, OsuConfigManager config, IAPIProvider api)
+        private void load(OsuConfigManager config, IAPIProvider api)
         {
             notifyOnMention = config.GetBindable<bool>(OsuSetting.ChatHighlightName);
             notifyOnChat = config.GetBindable<bool>(OsuSetting.ChatMessageNotification);
             highlightWords = config.GetBindable<string>(OsuSetting.HighlightWords);
             localUser = api.LocalUser;
+
+            // Listen for new messages
+
+            channelManager.JoinedChannels.ItemsAdded += (joinedChannels) =>
+            {
+                foreach (var channel in joinedChannels)
+                    channel.NewMessagesArrived += channel_NewMessagesArrived;
+            };
+
+            channelManager.JoinedChannels.ItemsRemoved += (leftChannels) =>
+            {
+                foreach (var channel in leftChannels)
+                    channel.NewMessagesArrived -= channel_NewMessagesArrived;
+            };
+        }
+
+        private void channel_NewMessagesArrived(IEnumerable<Message> messages, bool populated)
+        {
+            if (messages == null || !messages.Any())
+                return;
+
+            if (!populated)
+                return;
+
+            HandleMessages(messages.First().ChannelId, messages);
+        }
+
+        /// <remarks>
+        /// Resolves the channel id
+        /// </remarks>
+        public void HandleMessages(long channelId, IEnumerable<Message> messages)
+        {
+            var channel = channelManager.JoinedChannels.FirstOrDefault(c => c.Id == channelId);
+
+            if (channel == null)
+            {
+                Logger.Log($"Couldn't resolve channel id {channelId}", LoggingTarget.Information);
+                return;
+            }
+
+            HandleMessages(channel, messages);
         }
 
         public void HandleMessages(Channel channel, IEnumerable<Message> messages)
@@ -59,10 +98,6 @@ namespace osu.Game.Online.Chat
             if (IsActive && channelManager.CurrentChannel.Value == channel)
                 return;
 
-            var channelDrawable = chatOverlay.GetChannelDrawable(channel);
-            if (channelDrawable == null)
-                return;
-
             foreach (var message in messages)
             {
                 var words = getWords(message.Content);
@@ -73,20 +108,17 @@ namespace osu.Game.Online.Chat
 
                 void onClick()
                 {
-                    if (channelManager != null)
+                    chatOverlay.ScrollToAndHighlightMessage(channel, message);
-                        channelManager.CurrentChannel.Value = channel;
+                    chatOverlay.Show();
-
-                    channelDrawable.ScrollToAndHighlightMessage(message);
                 }
 
                 if (notifyOnChat.Value && channel.Type == ChannelType.PM)
                 {
-                    var username = message.Sender.Username;
+                    var existingNotification = notificationOverlay.Notifications.OfType<PrivateMessageNotification>().FirstOrDefault(n => n.Username == message.Sender.Username);
-                    var existingNotification = notificationOverlay.Notifications.OfType<PrivateMessageNotification>().FirstOrDefault(n => n.Username == username);
 
                     if (existingNotification == null)
                     {
-                        var notification = new PrivateMessageNotification(username, onClick);
+                        var notification = new PrivateMessageNotification(message.Sender.Username, onClick);
                         notificationOverlay?.Post(notification);
                     }
                     else
@@ -139,13 +171,12 @@ namespace osu.Game.Online.Chat
             public override bool IsImportant => false;
 
             [BackgroundDependencyLoader]
-            private void load(OsuColour colours, NotificationOverlay notificationOverlay, ChatOverlay chatOverlay)
+            private void load(OsuColour colours, NotificationOverlay notificationOverlay)
             {
                 IconBackgound.Colour = colours.PurpleDark;
                 Activated = delegate
                 {
                     notificationOverlay.Hide();
-                    chatOverlay.Show();
                     onClick?.Invoke();
 
                     return true;

osu.Game/Overlays/Chat/DrawableChannel.cs

@@ -67,7 +67,7 @@ namespace osu.Game.Overlays.Chat
                 },
             };
 
-            newMessagesArrived(Channel.Messages);
+            newMessagesArrived(Channel.Messages, Channel.Populated);
 
             Channel.NewMessagesArrived += newMessagesArrived;
             Channel.MessageRemoved += messageRemoved;
@@ -97,7 +97,7 @@ namespace osu.Game.Overlays.Chat
             Colour = colours.ChatBlue.Lighten(0.7f),
         };
 
-        private void newMessagesArrived(IEnumerable<Message> newMessages)
+        private void newMessagesArrived(IEnumerable<Message> newMessages, bool populated)
         {
             bool shouldScrollToEnd = scroll.IsScrolledToEnd(10) || !chatLines.Any() || newMessages.Any(m => m is LocalMessage);
 
@@ -164,7 +164,7 @@ namespace osu.Game.Overlays.Chat
 
             var chatLine = findChatLine(message);
             scroll.ScrollTo(chatLine);
-            chatLine.FlashColour(HighlightColour, 5000, Easing.InExpo);
+            chatLine.FlashColour(HighlightColour, 7500, Easing.InExpo);
         }
 
         private void messageRemoved(Message removed)

osu.Game/Overlays/ChatOverlay.cs

@@ -22,6 +22,7 @@ using osu.Game.Overlays.Chat.Selection;
 using osu.Game.Overlays.Chat.Tabs;
 using osuTK.Input;
 using osu.Framework.Graphics.Sprites;
+using System;
 
 namespace osu.Game.Overlays
 {
@@ -60,6 +61,8 @@ namespace osu.Game.Overlays
         private Container channelSelectionContainer;
         protected ChannelSelectionOverlay ChannelSelectionOverlay;
 
+        private Message highlightingMessage { get; set; }
+
         public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos)
                                                                  || (ChannelSelectionOverlay.State.Value == Visibility.Visible && ChannelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos));
 
@@ -252,15 +255,14 @@ namespace osu.Game.Overlays
             if (ChannelTabControl.Current.Value != e.NewValue)
                 Scheduler.Add(() => ChannelTabControl.Current.Value = e.NewValue);
 
-            var loaded = loadedChannels.Find(d => d.Channel == e.NewValue);
+            var loaded = GetChannelDrawable(e.NewValue);
 
             if (loaded == null)
             {
                 currentChannelContainer.FadeOut(500, Easing.OutQuint);
                 loading.Show();
 
-                loaded = new DrawableChannel(e.NewValue);
+                loaded = loadChannelDrawable(e.NewValue);
-                loadedChannels.Add(loaded);
                 LoadComponentAsync(loaded, l =>
                 {
                     if (currentChannel.Value != e.NewValue)
@@ -271,6 +273,12 @@ namespace osu.Game.Overlays
                     currentChannelContainer.Clear(false);
                     currentChannelContainer.Add(loaded);
                     currentChannelContainer.FadeIn(500, Easing.OutQuint);
+
+                    if (highlightingMessage != null && highlightingMessage.ChannelId == e.NewValue.Id)
+                    {
+                        loaded.ScrollToAndHighlightMessage(highlightingMessage);
+                        highlightingMessage = null;
+                    }
                 });
             }
             else
@@ -439,9 +447,32 @@ namespace osu.Game.Overlays
         }
 
         /// <summary>
-        /// Returns the loaded drawable for a channel. Returns null if not found.
+        /// Returns the loaded drawable for a channel. Creates new instance if <paramref name="createIfUnloaded"/> is true. Otherwise returns null if not found.
         /// </summary>
-        public DrawableChannel GetChannelDrawable(Channel channel) => loadedChannels.Find(drawable => drawable.Channel == channel);
+        public DrawableChannel GetChannelDrawable(Channel channel, bool createIfUnloaded = false)
+        {
+            var result = loadedChannels.Find(drawable => drawable.Channel == channel);
+
+            if (createIfUnloaded && result == null)
+            {
+                result = loadChannelDrawable(channel);
+            }
+
+            return result;
+        }
+
+        private DrawableChannel loadChannelDrawable(Channel channel)
+        {
+            var loaded = new DrawableChannel(channel);
+            loadedChannels.Add(loaded);
+            return loaded;
+        }
+
+        public void ScrollToAndHighlightMessage(Channel channel, Message message)
+        {
+            highlightingMessage = message;
+            channelManager.CurrentChannel.Value = channel;
+        }
 
         private class TabsArea : Container
         {