Fixed bugs and added tests

osu.Game.Tests/Visual/TestCaseChatLink.cs

@@ -4,12 +4,9 @@
 using OpenTK.Graphics;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Colour;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Graphics;
-using osu.Game.Graphics.Containers;
 using osu.Game.Online.Chat;
-using osu.Game.Overlays;
 using osu.Game.Overlays.Chat;
 using osu.Game.Users;
 using System;
@@ -20,20 +17,11 @@ namespace osu.Game.Tests.Visual
 {
     public class TestCaseChatLink : OsuTestCase
     {
-        private readonly BeatmapSetOverlay beatmapSetOverlay;
-        private readonly ChatOverlay chat;
-
-        private DependencyContainer dependencies;
-
         private readonly TestChatLineContainer textContainer;
-
+        private Color4 linkColour;
-        protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent);
 
         public TestCaseChatLink()
         {
-            chat = new ChatOverlay();
-            Add(beatmapSetOverlay = new BeatmapSetOverlay { Depth = float.MinValue });
-
             Add(textContainer = new TestChatLineContainer
             {
                 Padding = new MarginPadding { Left = 20, Right = 20 },
@@ -43,37 +31,50 @@ namespace osu.Game.Tests.Visual
             });
 
             testLinksGeneral();
-            testAddingLinks();
             testEcho();
         }
 
         private void clear() => AddStep("clear messages", textContainer.Clear);
 
-        private void addMessageWithChecks(string text, int linkAmount = 0, bool isAction = false, bool isImportant = false)
+        private void addMessageWithChecks(string text, int linkAmount = 0, bool isAction = false, bool isImportant = false, params LinkAction[] expectedActions)
         {
             var newLine = new ChatLine(new DummyMessage(text, isAction, isImportant));
             textContainer.Add(newLine);
 
             AddAssert($"msg #{textContainer.Count} has {linkAmount} link(s)", () => newLine.Message.Links.Count == linkAmount);
-            AddAssert($"msg #{textContainer.Count} is " + (isAction ? "italic" : "not italic"), () => newLine.ContentFlow.Any() && isAction == isItalic(newLine.ContentFlow));
+            AddAssert($"msg #{textContainer.Count} has the right action", hasExpectedActions);
-            AddAssert($"msg #{textContainer.Count} shows link(s)", isShowingLinks);
+            AddAssert($"msg #{textContainer.Count} is " + (isAction ? "italic" : "not italic"), () => newLine.ContentFlow.Any() && isAction == isItalic());
+            AddAssert($"msg #{textContainer.Count} shows {linkAmount} link(s)", isShowingLinks);
 
-            bool isItalic(LinkFlowContainer c) => c.Cast<OsuSpriteText>().All(sprite => sprite.Font == @"Exo2.0-MediumItalic");
+            bool hasExpectedActions()
+            {
+                var expectedActionsList = expectedActions.ToList();
+
+                if (expectedActionsList.Count != newLine.Message.Links.Count)
+                    return false;
+
+                for (int i = 0; i < newLine.Message.Links.Count; i++)
+                {
+                    var action = newLine.Message.Links[i].Action;
+                    if (action != expectedActions[i]) return false;
+                }
+
+                return true;
+            }
+
+            bool isItalic() => newLine.ContentFlow.Where(d => d is OsuSpriteText).Cast<OsuSpriteText>().All(sprite => sprite.Font == "Exo2.0-MediumItalic");
 
             bool isShowingLinks()
             {
-                SRGBColour textColour = Color4.White;
                 bool hasBackground = !string.IsNullOrEmpty(newLine.Message.Sender.Colour);
 
-                if (isAction && hasBackground)
+                Color4 textColour = isAction && hasBackground ? OsuColour.FromHex(newLine.Message.Sender.Colour) : Color4.White;
-                    textColour = OsuColour.FromHex(newLine.Message.Sender.Colour);
 
-                return newLine.ContentFlow
+                var linkCompilers = newLine.ContentFlow.Where(d => d is DrawableLinkCompiler).ToList();
-                    .Cast<OsuSpriteText>()
+                var linkSprites = linkCompilers.SelectMany(comp => ((DrawableLinkCompiler)comp).Parts);
-                    .All(sprite => sprite.HandleInput && !sprite.Colour.Equals(textColour)
+
-                                || !sprite.HandleInput && sprite.Colour.Equals(textColour)
+                return linkSprites.All(d => d.Colour == linkColour)
-                                // if someone with a background uses /me with a link, the usual link colour is overridden
+                    && newLine.ContentFlow.Except(linkSprites.Concat(linkCompilers)).All(d => d.Colour == textColour);
-                                || isAction && hasBackground && sprite.HandleInput && !sprite.Colour.Equals((ColourInfo)Color4.White));
             }
         }
 
@@ -81,30 +82,20 @@ namespace osu.Game.Tests.Visual
         {
             addMessageWithChecks("test!");
             addMessageWithChecks("osu.ppy.sh!");
-            addMessageWithChecks("https://osu.ppy.sh!", 1);
+            addMessageWithChecks("https://osu.ppy.sh!", 1, expectedActions: LinkAction.External);
-            addMessageWithChecks("00:12:345 (1,2) - Test?", 1);
+            addMessageWithChecks("00:12:345 (1,2) - Test?", 1, expectedActions: LinkAction.OpenEditorTimestamp);
-            addMessageWithChecks("Wiki link for tasty [[Performance Points]]", 1);
+            addMessageWithChecks("Wiki link for tasty [[Performance Points]]", 1, expectedActions: LinkAction.External);
-            addMessageWithChecks("(osu forums)[https://osu.ppy.sh/forum] (old link format)", 1);
+            addMessageWithChecks("(osu forums)[https://osu.ppy.sh/forum] (old link format)", 1, expectedActions: LinkAction.External);
-            addMessageWithChecks("[https://osu.ppy.sh/home New site] (new link format)", 1);
+            addMessageWithChecks("[https://osu.ppy.sh/home New site] (new link format)", 1, expectedActions: LinkAction.External);
-            addMessageWithChecks("[https://osu.ppy.sh/home This is only a link to the new osu webpage but this is supposed to test word wrap.]", 1);
+            addMessageWithChecks("[https://osu.ppy.sh/home This is only a link to the new osu webpage but this is supposed to test word wrap.]", 1, expectedActions: LinkAction.External);
-            addMessageWithChecks("is now listening to [https://osu.ppy.sh/s/93523 IMAGE -MATERIAL- <Version 0>]", 1, true);
+            addMessageWithChecks("is now listening to [https://osu.ppy.sh/s/93523 IMAGE -MATERIAL- <Version 0>]", 1, true, expectedActions: LinkAction.OpenBeatmapSet);
-            addMessageWithChecks("is now playing [https://osu.ppy.sh/b/252238 IMAGE -MATERIAL- <Version 0>]", 1, true);
+            addMessageWithChecks("is now playing [https://osu.ppy.sh/b/252238 IMAGE -MATERIAL- <Version 0>]", 1, true, expectedActions: LinkAction.OpenBeatmap);
-            addMessageWithChecks("Let's (try)[https://osu.ppy.sh/home] [https://osu.ppy.sh/home multiple links] https://osu.ppy.sh/home", 3);
+            addMessageWithChecks("Let's (try)[https://osu.ppy.sh/home] [https://osu.ppy.sh/b/252238 multiple links] https://osu.ppy.sh/home", 3, expectedActions: new[] { LinkAction.External, LinkAction.OpenBeatmap, LinkAction.External });
             // note that there's 0 links here (they get removed if a channel is not found)
             addMessageWithChecks("#lobby or #osu would be blue (and work) in the ChatDisplay test (when a proper ChatOverlay is present).");
             addMessageWithChecks("I am important!", 0, false, true);
             addMessageWithChecks("feels important", 0, true, true);
-            addMessageWithChecks("likes to post this [https://osu.ppy.sh/home link].", 1, true, true);
+            addMessageWithChecks("likes to post this [https://osu.ppy.sh/home link].", 1, true, true, expectedActions: LinkAction.External);
-        }
-
-        private void testAddingLinks()
-        {
-            const int count = 5;
-
-            for (int i = 1; i <= count; i++)
-                AddStep($"add long msg #{i}", () => textContainer.Add(new ChatLine(new DummyMessage("alright let's just put a really long text here to see if it loads in correctly rather than adding the text sprites individually after the chat line appearing!"))));
-
-            clear();
         }
 
         private void testEcho()
@@ -131,10 +122,9 @@ namespace osu.Game.Tests.Visual
         }
 
         [BackgroundDependencyLoader]
-        private void load()
+        private void load(OsuColour colours)
         {
-            dependencies.Cache(chat);
+            linkColour = colours.Blue;
-            dependencies.Cache(beatmapSetOverlay);
         }
 
         private class DummyEchoMessage : LocalEchoMessage

osu.Game/Graphics/Containers/LinkFlowContainer.cs

@@ -7,7 +7,7 @@ using System.Diagnostics;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics.Sprites;
-using osu.Game.Overlays;
+using System.Collections.Generic;
 
 namespace osu.Game.Graphics.Containers
 {
@@ -20,19 +20,36 @@ namespace osu.Game.Graphics.Containers
 
         public override bool HandleInput => true;
 
-        private BeatmapSetOverlay beatmapSetOverlay;
-        private ChatOverlay chat;
         private OsuGame game;
 
         [BackgroundDependencyLoader(true)]
-        private void load(BeatmapSetOverlay beatmapSetOverlay, ChatOverlay chat, OsuGame game)
+        private void load(OsuGame game)
         {
-            this.beatmapSetOverlay = beatmapSetOverlay;
+            // will be null in tests
-            this.chat = chat;
-            // this will be null in tests
             this.game = game;
         }
 
+        public void AddLinks(string text, List<Link> links)
+        {
+            if (string.IsNullOrEmpty(text) || links == null)
+                return;
+
+            if (links.Count == 0)
+            {
+                AddText(text);
+                return;
+            }
+
+            int previousLinkEnd = 0;
+            foreach (var link in links)
+            {
+                AddText(text.Substring(previousLinkEnd, link.Index - previousLinkEnd));
+
+                AddLink(text.Substring(link.Index, link.Length), link.Url, link.Action, link.Argument);
+                previousLinkEnd = link.Index + link.Length;
+            }
+        }
+
         public void AddLink(string text, string url, LinkAction linkType = LinkAction.External, string linkArgument = null, string tooltipText = null)
         {
             AddInternal(new DrawableLinkCompiler(AddText(text).ToList())
@@ -47,10 +64,10 @@ namespace osu.Game.Graphics.Containers
                             break;
                         case LinkAction.OpenBeatmapSet:
                             if (int.TryParse(linkArgument, out int setId))
-                                beatmapSetOverlay.ShowBeatmapSet(setId);
+                                game?.ShowBeatmapSet(setId);
                             break;
                         case LinkAction.OpenChannel:
-                            chat.OpenChannel(chat.AvailableChannels.Find(c => c.Name == linkArgument));
+                            game?.OpenChannel(linkArgument);
                             break;
                         case LinkAction.OpenEditorTimestamp:
                             game?.LoadEditorTimestamp();

osu.Game/Online/Chat/Message.cs

@@ -41,6 +41,15 @@ namespace osu.Game.Online.Chat
         {
         }
 
+        /// <summary>
+        /// The text that is displayed in chat.
+        /// </summary>
+        public string DisplayContent { get; set; }
+
+        /// <summary>
+        /// The links found in this message.
+        /// </summary>
+        /// <remarks>The links' positions are according to <see cref="DisplayContent"/></remarks>
         public List<Link> Links;
 
         public Message(long? id)

osu.Game/Online/Chat/MessageFormatter.cs

@@ -66,7 +66,7 @@ namespace osu.Game.Online.Chat
                     //since we just changed the line display text, offset any already processed links.
                     result.Links.ForEach(l => l.Index -= l.Index > index ? m.Length - displayText.Length : 0);
 
-                    var details = getLinkDetails(link);
+                    var details = getLinkDetails(linkText);
                     result.Links.Add(new Link(linkText, index, displayText.Length, linkActionOverride ?? details.linkType, details.linkArgument));
 
                     //adjust the offset for processing the current matches group.
@@ -119,7 +119,7 @@ namespace osu.Game.Online.Chat
                             case "s":
                             case "beatmapsets":
                             case "d":
-                                return (LinkAction.External, args[3]);
+                                return (LinkAction.OpenBeatmapSet, args[3]);
                         }
                     }
 
@@ -196,7 +196,7 @@ namespace osu.Game.Online.Chat
         {
             var result = format(inputMessage.Content);
 
-            inputMessage.Content = result.Text;
+            inputMessage.DisplayContent = result.Text;
 
             // Sometimes, regex matches are not in order
             result.Links.Sort();

osu.Game/OsuGame.cs

@@ -119,7 +119,12 @@ namespace osu.Game
 
         private ScheduledDelegate scoreLoad;
 
-        // TODO: Implement this properly as soon as the Editor is done
+        #region chat link actions
+
+        internal void OpenChannel(string channelName) => chat.OpenChannel(chat.AvailableChannels.Find(c => c.Name == channelName));
+
+        internal void ShowBeatmapSet(int setId) => beatmapSetOverlay.ShowBeatmapSet(setId);
+
         internal void LoadEditorTimestamp()
         {
             notifications.Post(new SimpleNotification
@@ -147,6 +152,8 @@ namespace osu.Game
             });
         }
 
+        #endregion
+
         protected void LoadScore(Score s)
         {
             scoreLoad?.Cancel();

osu.Game/Overlays/Chat/ChatLine.cs

@@ -225,42 +225,17 @@ namespace osu.Game.Overlays.Chat
             timestamp.Text = $@"{message.Timestamp.LocalDateTime:HH:mm:ss}";
             username.Text = $@"{message.Sender.Username}" + (senderHasBackground || message.IsAction ? "" : ":");
 
+            // remove any non-existent channels from the link list
+            var linksToRemove = new List<Link>();
+            foreach (var link in message.Links)
+                if (link.Action == LinkAction.OpenChannel && chat?.AvailableChannels.TrueForAll(c => c.Name != link.Argument) != false)
+                    linksToRemove.Add(link);
+
+            foreach (var link in linksToRemove)
+                message.Links.Remove(link);
+
             contentFlow.Clear();
-
+            contentFlow.AddLinks(message.DisplayContent, message.Links);
-            if (message.Links == null || message.Links.Count == 0)
-                contentFlow.AddText(message.Content);
-            else
-            {
-                int lastLinkEndIndex = 0;
-                List<Link> linksToRemove = new List<Link>();
-
-                foreach (var link in message.Links)
-                {
-                    contentFlow.AddText(message.Content.Substring(lastLinkEndIndex, link.Index - lastLinkEndIndex));
-                    lastLinkEndIndex = link.Index + link.Length;
-
-                    const string channel_link_prefix = "osu://chan/";
-                    // If a channel doesn't exist, add it as normal text instead
-                    if (link.Url.StartsWith(channel_link_prefix))
-                    {
-                        var channelName = link.Url.Substring(channel_link_prefix.Length).Split('/')[0];
-                        if (chat?.AvailableChannels.TrueForAll(c => c.Name != channelName) != false)
-                        {
-                            linksToRemove.Add(link);
-                            contentFlow.AddText(message.Content.Substring(link.Index, link.Length));
-                            continue;
-                        }
-                    }
-
-                    contentFlow.AddLink(message.Content.Substring(link.Index, link.Length), link.Url);
-                }
-
-                var lastLink = message.Links[message.Links.Count - 1];
-                contentFlow.AddText(message.Content.Substring(lastLink.Index + lastLink.Length));
-
-                foreach (var link in linksToRemove)
-                    message.Links.Remove(link);
-            }
         }
 
         private class MessageSender : OsuClickableContainer, IHasContextMenu