Review changes + added tests

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

@@ -34,6 +34,7 @@ namespace osu.Game.Tests.Visual.Online
         private Channel previousChannel => joinedChannels.ElementAt(joinedChannels.ToList().IndexOf(currentChannel) - 1);
         private Channel channel1 => channels[0];
         private Channel channel2 => channels[1];
+        private Channel channel3 => channels[2];
 
         public TestSceneChatOverlay()
         {
@@ -42,7 +43,8 @@ namespace osu.Game.Tests.Visual.Online
                                  {
                                      Name = $"Channel no. {index}",
                                      Topic = index == 3 ? null : $"We talk about the number {index} here",
-                                     Type = index % 2 == 0 ? ChannelType.PM : ChannelType.Temporary
+                                     Type = index % 2 == 0 ? ChannelType.PM : ChannelType.Temporary,
+                                     Id = index
                                  })
                                  .ToList();
         }
@@ -249,6 +251,51 @@ namespace osu.Game.Tests.Visual.Online
             AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible);
         }
 
+        [Test]
+        public void TestCtrlShiftTShortcut()
+        {
+            AddStep("Join 3 channels", () =>
+            {
+                channelManager.JoinChannel(channel1);
+                channelManager.JoinChannel(channel2);
+                channelManager.JoinChannel(channel3);
+            });
+
+            // Should do nothing
+            AddStep("Press Ctrl + Shift + T", pressControlShiftT);
+            AddAssert("All channels still open", () => channelManager.JoinedChannels.Count == 3);
+
+            // Close channel 1
+            AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1]));
+            AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child));
+            AddAssert("Channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1));
+            AddAssert("Other channels still open", () => channelManager.JoinedChannels.Count == 2);
+
+            // Reopen channel 1
+            AddStep("Press Ctrl + Shift + T", pressControlShiftT);
+            AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3);
+            AddAssert("Current channel is channel 1", () => currentChannel == channel1);
+
+            // Close two channels
+            AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1]));
+            AddStep("Close channel 1", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child));
+            AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2]));
+            AddStep("Close channel 2", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child));
+            AddAssert("Only one channel open", () => channelManager.JoinedChannels.Count == 1);
+            AddAssert("Current channel is channel 3", () => currentChannel == channel3);
+
+            // Should first re-open channel 2
+            AddStep("Press Ctrl + Shift + T", pressControlShiftT);
+            AddAssert("Channel 1 still closed", () => !channelManager.JoinedChannels.Contains(channel1));
+            AddAssert("Channel 2 now open", () => channelManager.JoinedChannels.Contains(channel2));
+            AddAssert("Current channel is channel 2", () => currentChannel == channel2);
+
+            // Should then re-open channel 1
+            AddStep("Press Ctrl + Shift + T", pressControlShiftT);
+            AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3);
+            AddAssert("Current channel is channel 1", () => currentChannel == channel1);
+        }
+
         private void pressChannelHotkey(int number)
         {
             var channelKey = Key.Number0 + number;
@@ -271,6 +318,15 @@ namespace osu.Game.Tests.Visual.Online
             InputManager.ReleaseKey(Key.ControlLeft);
         }
 
+        private void pressControlShiftT()
+        {
+            InputManager.PressKey(Key.ControlLeft);
+            InputManager.PressKey(Key.ShiftLeft);
+            InputManager.Key(Key.T);
+            InputManager.ReleaseKey(Key.ShiftLeft);
+            InputManager.ReleaseKey(Key.ControlLeft);
+        }
+
         private void clickDrawable(Drawable d)
         {
             InputManager.MoveMouseTo(d);

osu.Game/Online/Chat/ChannelManager.cs

@@ -8,6 +8,7 @@ using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Logging;
+using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests;
 using osu.Game.Overlays.Chat.Tabs;
@@ -34,10 +35,9 @@ namespace osu.Game.Online.Chat
         private readonly BindableList<Channel> joinedChannels = new BindableList<Channel>();
 
         /// <summary>
-        /// Keeps a list of closed channel identifiers. Stores the channel ID for normal
+        /// Keeps a stack of recently closed channels
-        /// channels, or the user ID for PM channels
         /// </summary>
-        private readonly BindableList<long> closedChannelIds = new BindableList<long>();
+        private readonly BindableList<ClosedChannel> closedChannels = new BindableList<ClosedChannel>();
 
         // For efficiency purposes, this constant bounds the number of closed channels we store.
         // This number is somewhat arbitrary; future developers are free to modify it.
@@ -62,6 +62,10 @@ namespace osu.Game.Online.Chat
         [Resolved]
         private IAPIProvider api { get; set; }
 
+
+        [Resolved]
+        private UserLookupCache users { get; set; }
+
         public readonly BindableBool HighPollRate = new BindableBool();
 
         public ChannelManager()
@@ -421,13 +425,15 @@ namespace osu.Game.Online.Chat
 
             // Prevent the closedChannel list from exceeding the max size
             // by removing the oldest element
-            if (closedChannelIds.Count >= closed_channels_max_size)
+            if (closedChannels.Count >= closed_channels_max_size)
             {
-                closedChannelIds.RemoveAt(0);
+                closedChannels.RemoveAt(0);
             }
 
             // For PM channels, we store the user ID; else, we store the channel id
-            closedChannelIds.Add(channel.Type == ChannelType.PM ? channel.Users.First().Id : channel.Id);
+            closedChannels.Add(channel.Type == ChannelType.PM
+                ? new ClosedChannel(ChannelType.PM, channel.Users.Single().Id)
+                : new ClosedChannel(channel.Type, channel.Id));
 
             if (channel.Joined.Value)
             {
@@ -443,41 +449,36 @@ namespace osu.Game.Online.Chat
         /// </summary>
         public void JoinLastClosedChannel()
         {
-            if (closedChannelIds.Count <= 0)
+            // This loop could be eliminated if a check was added so that
-            {
-                return;
-            }
-
-            // This nested loop could be eliminated if a check was added so that
             // when the code opens a channel it removes from the closedChannel list.
             // However, this would require adding an O(|closeChannels|) work operation
             // every time the user joins a channel, which would make joining a channel
-            // slower. I wanted to centralize all major slowdowns so they
+            // slower. We wanted to centralize all major slowdowns so they
             // can only occur if the user actually decides to use this feature.
-            while (closedChannelIds.Count != 0)
+            while (closedChannels.Count > 0)
             {
-                long lastClosedChannelId = closedChannelIds.Last();
+                ClosedChannel lastClosedChannel = closedChannels.Last();
-                closedChannelIds.RemoveAt(closedChannelIds.Count - 1);
+                closedChannels.RemoveAt(closedChannels.Count - 1);
-
-                bool lookupCondition(Channel ch) => ch.Type == ChannelType.PM
-                    ? ch.Users.Any(u => u.Id == lastClosedChannelId)
-                    : ch.Id == lastClosedChannelId;
 
                 // If the user hasn't already joined the channel, try to join it
-                if (joinedChannels.FirstOrDefault(lookupCondition) == null)
+                if (joinedChannels.FirstOrDefault(lastClosedChannel.IsEqual) == null)
                 {
-                    Channel lastClosedChannel = AvailableChannels.FirstOrDefault(lookupCondition);
+                    Channel lastChannel = AvailableChannels.FirstOrDefault(lastClosedChannel.IsEqual);
 
-                    if (lastClosedChannel != null)
+                    if (lastChannel != null)
                     {
-                        CurrentChannel.Value = JoinChannel(lastClosedChannel);
+                        // Channel exists as an availaable channel, directly join it
+                        CurrentChannel.Value = JoinChannel(lastChannel);
                     }
-                    else
+                    else if (lastClosedChannel.Type == ChannelType.PM)
                     {
                         // Try to get User to open PM chat
-                        var req = new GetUserRequest(lastClosedChannelId);
+                        users.GetUserAsync((int)lastClosedChannel.Id).ContinueWith(u =>
-                        req.Success += user => CurrentChannel.Value = JoinChannel(new Channel(user));
+                        {
-                        api.Queue(req);
+                            if (u.Result == null) return;
+
+                            Schedule(() => CurrentChannel.Value = JoinChannel(new Channel(u.Result)));
+                        });
                     }
 
                     return;
@@ -569,4 +570,28 @@ namespace osu.Game.Online.Chat
         {
         }
     }
+
+    /// <summary>
+    /// Class that stores information about a closed channel
+    /// </summary>
+    public class ClosedChannel
+    {
+        public ChannelType Type;
+        public long Id;
+
+        public ClosedChannel(ChannelType type, long id)
+        {
+            Type = type;
+            Id = id;
+        }
+
+        public bool IsEqual(Channel channel)
+        {
+            if (channel.Type != this.Type) return false;
+
+            return this.Type == ChannelType.PM
+                ? channel.Users.Single().Id == this.Id
+                : channel.Id == this.Id;
+        }
+    }
 }