From 92cab12eb7425179fb2277d3428cf99fc3617332 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Fri, 11 Dec 2020 21:52:36 -0800 Subject: [PATCH 01/50] Added ctrl + w and ctrl + t keyboard shortcuts --- osu.Game/Overlays/ChatOverlay.cs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 8bc7e21047..5162b6aab0 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -25,6 +25,9 @@ using osuTK.Input; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; +using System; +using osu.Game.Input.Bindings; + namespace osu.Game.Overlays { public class ChatOverlay : OsuFocusedOverlayContainer, INamedOverlayComponent @@ -333,6 +336,16 @@ namespace osu.Game.Overlays ChannelTabControl.Current.Value = channel; } + private void selectChannelSelectorTab() + { + int i = 0; + var channel = ChannelTabControl.Items + .Where(tab => (tab is ChannelSelectorTabItem.ChannelSelectorTabChannel)) + .ElementAtOrDefault(i); + if (channel != null) + ChannelTabControl.Current.Value = channel; + } + protected override bool OnKeyDown(KeyDownEvent e) { if (e.AltPressed) @@ -356,6 +369,18 @@ namespace osu.Game.Overlays return true; } } + if (e.ControlPressed) + { + switch (e.Key) + { + case Key.W: + channelManager.LeaveChannel(channelManager.CurrentChannel.Value); + return true; + case Key.T: + selectChannelSelectorTab(); + return true; + } + } return base.OnKeyDown(e); } @@ -392,6 +417,7 @@ namespace osu.Game.Overlays private void joinedChannelsChanged(object sender, NotifyCollectionChangedEventArgs args) { + switch (args.Action) { case NotifyCollectionChangedAction.Add: From 3dd6589d6e2d516ec5790ce06c24bb5d98447b31 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Sat, 12 Dec 2020 15:53:08 -0800 Subject: [PATCH 02/50] Fixed style changes and cleaned up ctrl + t implementation --- .../Overlays/Chat/Tabs/ChannelTabControl.cs | 5 +++++ osu.Game/Overlays/ChatOverlay.cs | 19 +++---------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs index 19c6f437b6..bc68a9f32a 100644 --- a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs +++ b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs @@ -96,6 +96,11 @@ namespace osu.Game.Overlays.Chat.Tabs selectorTab.Active.Value = false; } + public void SelectChannelSelectorTab() + { + SelectTab(selectorTab); + } + protected override TabFillFlowContainer CreateTabFlow() => new ChannelTabFillFlowContainer { Direction = FillDirection.Full, diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 5162b6aab0..7ff63595c7 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -25,9 +25,6 @@ using osuTK.Input; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; -using System; -using osu.Game.Input.Bindings; - namespace osu.Game.Overlays { public class ChatOverlay : OsuFocusedOverlayContainer, INamedOverlayComponent @@ -335,17 +332,6 @@ namespace osu.Game.Overlays if (channel != null) ChannelTabControl.Current.Value = channel; } - - private void selectChannelSelectorTab() - { - int i = 0; - var channel = ChannelTabControl.Items - .Where(tab => (tab is ChannelSelectorTabItem.ChannelSelectorTabChannel)) - .ElementAtOrDefault(i); - if (channel != null) - ChannelTabControl.Current.Value = channel; - } - protected override bool OnKeyDown(KeyDownEvent e) { if (e.AltPressed) @@ -369,6 +355,7 @@ namespace osu.Game.Overlays return true; } } + if (e.ControlPressed) { switch (e.Key) @@ -376,8 +363,9 @@ namespace osu.Game.Overlays case Key.W: channelManager.LeaveChannel(channelManager.CurrentChannel.Value); return true; + case Key.T: - selectChannelSelectorTab(); + ChannelTabControl.SelectChannelSelectorTab(); return true; } } @@ -417,7 +405,6 @@ namespace osu.Game.Overlays private void joinedChannelsChanged(object sender, NotifyCollectionChangedEventArgs args) { - switch (args.Action) { case NotifyCollectionChangedAction.Add: From 4c1e75f1017b3b5a5aa01f4f07f9ebcd44bfed5e Mon Sep 17 00:00:00 2001 From: Angela Zhang <43687061+angelaz1@users.noreply.github.com> Date: Sat, 12 Dec 2020 18:02:08 -0600 Subject: [PATCH 03/50] Small whitespace fix --- osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs index bc68a9f32a..c314e10aff 100644 --- a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs +++ b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs @@ -96,7 +96,7 @@ namespace osu.Game.Overlays.Chat.Tabs selectorTab.Active.Value = false; } - public void SelectChannelSelectorTab() + public void SelectChannelSelectorTab() { SelectTab(selectorTab); } From b9f687d7f9605f97d8494e9e6a3e6545aa5a9c24 Mon Sep 17 00:00:00 2001 From: Angela Zhang <43687061+angelaz1@users.noreply.github.com> Date: Sat, 12 Dec 2020 18:55:17 -0600 Subject: [PATCH 04/50] Adding in extra blank line --- osu.Game/Overlays/ChatOverlay.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 7ff63595c7..1b1b7ec6e8 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -332,6 +332,7 @@ namespace osu.Game.Overlays if (channel != null) ChannelTabControl.Current.Value = channel; } + protected override bool OnKeyDown(KeyDownEvent e) { if (e.AltPressed) From 084e4ce50bb4265c5fd020c47e9a41a08fc9dc0f Mon Sep 17 00:00:00 2001 From: Angela Zhang <43687061+angelaz1@users.noreply.github.com> Date: Sat, 12 Dec 2020 20:11:57 -0600 Subject: [PATCH 05/50] Removing whitespace --- osu.Game/Overlays/ChatOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 1b1b7ec6e8..b310e3ff4c 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -332,7 +332,7 @@ namespace osu.Game.Overlays if (channel != null) ChannelTabControl.Current.Value = channel; } - + protected override bool OnKeyDown(KeyDownEvent e) { if (e.AltPressed) From 2d98da0d61374a2abedb3c1daae108df0e77d905 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 13:21:50 -0500 Subject: [PATCH 06/50] Untested Ctrl+Shift+T shortcut prototype Added a list to the ChannelManager class that tracks which tabs I closed. Works like a stack, where it adds to the end every time I close a tab. Then added a function that uses this list to open the last closed channel, and added a shortcut inside of ChatOverlay, similar to how jmeng implemented shortcuts. Code is currently untested. --- osu.Game/Online/Chat/ChannelManager.cs | 30 ++++++++++++++++++++++++++ osu.Game/Overlays/ChatOverlay.cs | 26 ++++++++++++++++------ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 16f46581c5..9c47c94677 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,6 +33,19 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); + // Prototype for keeping a list of closed channels in an + // order so we can figure out the reverse order of how channels + // were close + // Bindable list supports an insert at indexc function and a + // remove function. If the list re-indexes after each remove (I can + // check the behaviour of the C# List System.Collections.Generic library to confirm this, since that + // library appears to be what is used underneath), then I can just always add at the end + // of the list and always remove index 0 (if size > 0) + // A stack exchange post indicates that List remove will decrement all the + // indeces after the node we removed + + private readonly BindableList closedChannels = new BindableList(); + /// /// The currently opened channel /// @@ -48,6 +61,11 @@ namespace osu.Game.Online.Chat /// public IBindableList AvailableChannels => availableChannels; + /// + /// The channels available for the player to join + /// + public IBindableList ClosedChannels => ClosedChannels; + [Resolved] private IAPIProvider api { get; set; } @@ -407,6 +425,8 @@ namespace osu.Game.Online.Chat CurrentChannel.Value = null; joinedChannels.Remove(channel); + // insert at the end of the list + closedChannels.Insert(closedChannels.Count, channel); if (channel.Joined.Value) { @@ -415,6 +435,16 @@ namespace osu.Game.Online.Chat } } + + public void JoinLastClosedChannel() + { + if(joinedChannels.Count == 0) + return; + Channel lastClosedChannel = joinedChannels[joinedChannels.Count - 1]; + JoinChannel(lastClosedChannel); + joinedChannels.Remove(lastClosedChannel); + } + private long lastMessageId; private bool channelsInitialised; diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index b310e3ff4c..0adf3cb2c5 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -359,15 +359,27 @@ namespace osu.Game.Overlays if (e.ControlPressed) { - switch (e.Key) + if(e.ShiftPressed) { - case Key.W: - channelManager.LeaveChannel(channelManager.CurrentChannel.Value); - return true; + switch(e.Key) + { + case Key.T: + channelManager.JoinLastClosedChannel(); + return true; + } + } + else + { + switch (e.Key) + { + case Key.W: + channelManager.LeaveChannel(channelManager.CurrentChannel.Value); + return true; - case Key.T: - ChannelTabControl.SelectChannelSelectorTab(); - return true; + case Key.T: + ChannelTabControl.SelectChannelSelectorTab(); + return true; + } } } From 5481ba43c7875e6bb8df292813c8de23dabe7738 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 14:14:57 -0500 Subject: [PATCH 07/50] Fixed a bug where Ctrl+Shift+t shortcut was using the wrong list The JoinLastClosedChannel code was using the joinedChannels list instead of the closedChannels list. Fixing this bug made the Ctrl+Shift+t shortuct work as expected. --- osu.Game/Online/Chat/ChannelManager.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 9c47c94677..b472f521d7 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -435,14 +435,13 @@ namespace osu.Game.Online.Chat } } - public void JoinLastClosedChannel() { - if(joinedChannels.Count == 0) + if(closedChannels.Count == 0) return; - Channel lastClosedChannel = joinedChannels[joinedChannels.Count - 1]; + Channel lastClosedChannel = closedChannels[closedChannels.Count - 1]; JoinChannel(lastClosedChannel); - joinedChannels.Remove(lastClosedChannel); + closedChannels.Remove(lastClosedChannel); } private long lastMessageId; From 2863187b16013c2db36c04d8512355d128cc525d Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 20:46:02 -0500 Subject: [PATCH 08/50] Changing behvaior for channels that have already been reopened Ctrl+Shift+t will now skip trying to reopen any channels that are already open, and will instead attempt to open the next still-closed channel. --- osu.Game/Online/Chat/ChannelManager.cs | 54 +++++++++++++++++--------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index b472f521d7..cf7f5604d9 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,19 +33,15 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); - // Prototype for keeping a list of closed channels in an - // order so we can figure out the reverse order of how channels - // were close - // Bindable list supports an insert at indexc function and a - // remove function. If the list re-indexes after each remove (I can - // check the behaviour of the C# List System.Collections.Generic library to confirm this, since that - // library appears to be what is used underneath), then I can just always add at the end - // of the list and always remove index 0 (if size > 0) - // A stack exchange post indicates that List remove will decrement all the - // indeces after the node we removed + // Keeps a list of closed channels. More recently closed channels appear at higher indeces private readonly BindableList closedChannels = new BindableList(); + // 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. + // Must be a positive number. + private const int closedChannelsMaxSize = 50; + /// /// The currently opened channel /// @@ -61,11 +57,6 @@ namespace osu.Game.Online.Chat /// public IBindableList AvailableChannels => availableChannels; - /// - /// The channels available for the player to join - /// - public IBindableList ClosedChannels => ClosedChannels; - [Resolved] private IAPIProvider api { get; set; } @@ -425,7 +416,15 @@ namespace osu.Game.Online.Chat CurrentChannel.Value = null; joinedChannels.Remove(channel); - // insert at the end of the list + + // Prevent the closedChannel list from exceeding the max size + // by removing the oldest element + if(closedChannels.Count >= closedChannelsMaxSize) + { + closedChannels.Remove(closedChannels[0]); + } + + // insert at the end of the closedChannels list closedChannels.Insert(closedChannels.Count, channel); if (channel.Joined.Value) @@ -435,13 +434,32 @@ namespace osu.Game.Online.Chat } } + + + /// + /// Opens the most recently closed channel that has not + /// already been reopened + /// Works similarly to reopening last closed tab on a web browser. + /// public void JoinLastClosedChannel() { - if(closedChannels.Count == 0) + if(closedChannels.Count <= 0) + { return; + } + Channel lastClosedChannel = closedChannels[closedChannels.Count - 1]; - JoinChannel(lastClosedChannel); closedChannels.Remove(lastClosedChannel); + // If the user already joined the channel, try the next + // channel in the list + if(joinedChannels.IndexOf(lastClosedChannel) >= 0) + { + JoinLastClosedChannel(); + } + else + { + JoinChannel(lastClosedChannel); + } } private long lastMessageId; From 08a2cdaf8d43b7efe9a44d4c9833242abd261c60 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 21:02:35 -0500 Subject: [PATCH 09/50] Minor formatting changes --- osu.Game/Online/Chat/ChannelManager.cs | 10 +++++----- osu.Game/Overlays/ChatOverlay.cs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index cf7f5604d9..e1f4d82e52 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -419,7 +419,7 @@ namespace osu.Game.Online.Chat // Prevent the closedChannel list from exceeding the max size // by removing the oldest element - if(closedChannels.Count >= closedChannelsMaxSize) + if (closedChannels.Count >= closedChannelsMaxSize) { closedChannels.Remove(closedChannels[0]); } @@ -435,15 +435,15 @@ namespace osu.Game.Online.Chat } - + /// /// Opens the most recently closed channel that has not /// already been reopened - /// Works similarly to reopening last closed tab on a web browser. + /// Works similarly to reopening the last closed tab on a web browser. /// public void JoinLastClosedChannel() { - if(closedChannels.Count <= 0) + if (closedChannels.Count <= 0) { return; } @@ -452,7 +452,7 @@ namespace osu.Game.Online.Chat closedChannels.Remove(lastClosedChannel); // If the user already joined the channel, try the next // channel in the list - if(joinedChannels.IndexOf(lastClosedChannel) >= 0) + if (joinedChannels.IndexOf(lastClosedChannel) >= 0) { JoinLastClosedChannel(); } diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 0adf3cb2c5..12bd7e895e 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -359,9 +359,9 @@ namespace osu.Game.Overlays if (e.ControlPressed) { - if(e.ShiftPressed) + if (e.ShiftPressed) { - switch(e.Key) + switch (e.Key) { case Key.T: channelManager.JoinLastClosedChannel(); From 337309ec1332766fd314c8bf29f3d2790933dd28 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 22:06:59 -0500 Subject: [PATCH 10/50] Added Tests for the Ctrl+Shift+T shortcut Tests have not actually been run yet. --- .../Visual/Online/TestSceneChatOverlay.cs | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index fca642ad6c..1770c34d2f 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -208,6 +208,84 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } + // Control-Shift-T should reopen the most recently closed tab that has not + // already been reopened + private void pressControlShiftT() + { + InputManager.PressKey(Key.ControlLeft); + InputManager.PressKey(Key.ShiftLeft); + InputManager.Key(Key.T); + InputManager.ReleaseKey(Key.ControlLeft); + InputManager.ReleaseKey(Key.ShiftLeft); + } + + public void TestCtrlShiftTShortcut1() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Close channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + AddStep("Press Control-Shift-T", () => pressControlShiftT()); + + // Channel 2 should now be open again + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } + + public void TestCtrlShiftTShortcut2() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Close channels 1 and channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + + AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); + AddStep("Click channel 1 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); + + AddAssert("channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + AddStep("Press Control-Shift-T", () => pressControlShiftT()); + // Channel 1 should now be open again. Channel 2 should not + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + AddStep("Press Control-Shift-T", () => pressControlShiftT()); + + // Both Channel 1 and Channel 2 should be open + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + + // Processing this again should have not effect + AddStep("Press Control-Shift-T", () => pressControlShiftT()); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + + // Close channel 2 again + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + + // Both channel 1 and channel 2 should be open + AddStep("Press Control-Shift-T", () => pressControlShiftT()); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } + + + private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From 3301f532eed41ef457b3be009e9c9ca2c44773ca Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Sun, 13 Dec 2020 23:27:48 -0500 Subject: [PATCH 11/50] Additional formatting Ran the dotnet format as well as the .\InspectCode.ps1 tools, and fixed the style issues they found. --- .../Visual/Online/TestSceneChatOverlay.cs | 22 +++++++++---------- osu.Game/Online/Chat/ChannelManager.cs | 12 +++++----- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 1770c34d2f..fddaaa8215 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -212,11 +212,11 @@ namespace osu.Game.Tests.Visual.Online // already been reopened private void pressControlShiftT() { - InputManager.PressKey(Key.ControlLeft); - InputManager.PressKey(Key.ShiftLeft); - InputManager.Key(Key.T); - InputManager.ReleaseKey(Key.ControlLeft); - InputManager.ReleaseKey(Key.ShiftLeft); + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); + AddStep("Press T", () => InputManager.Key(Key.T)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); } public void TestCtrlShiftTShortcut1() @@ -233,7 +233,7 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - AddStep("Press Control-Shift-T", () => pressControlShiftT()); + pressControlShiftT(); // Channel 2 should now be open again AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); @@ -258,19 +258,19 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - AddStep("Press Control-Shift-T", () => pressControlShiftT()); + pressControlShiftT(); // Channel 1 should now be open again. Channel 2 should not AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - AddStep("Press Control-Shift-T", () => pressControlShiftT()); + pressControlShiftT(); // Both Channel 1 and Channel 2 should be open AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); // Processing this again should have not effect - AddStep("Press Control-Shift-T", () => pressControlShiftT()); + pressControlShiftT(); AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); @@ -279,13 +279,11 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); // Both channel 1 and channel 2 should be open - AddStep("Press Control-Shift-T", () => pressControlShiftT()); + pressControlShiftT(); AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); } - - private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index e1f4d82e52..63eea0b190 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,14 +33,13 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); - // Keeps a list of closed channels. More recently closed channels appear at higher indeces private readonly BindableList closedChannels = new BindableList(); // 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. // Must be a positive number. - private const int closedChannelsMaxSize = 50; + private const int closed_channels_max_size = 50; /// /// The currently opened channel @@ -419,7 +418,7 @@ namespace osu.Game.Online.Chat // Prevent the closedChannel list from exceeding the max size // by removing the oldest element - if (closedChannels.Count >= closedChannelsMaxSize) + if (closedChannels.Count >= closed_channels_max_size) { closedChannels.Remove(closedChannels[0]); } @@ -434,8 +433,6 @@ namespace osu.Game.Online.Chat } } - - /// /// Opens the most recently closed channel that has not /// already been reopened @@ -448,8 +445,11 @@ namespace osu.Game.Online.Chat return; } - Channel lastClosedChannel = closedChannels[closedChannels.Count - 1]; + // equivalent to Channel lastClosedChannel = closedChannels[closedChannels.Count - 1]; + Channel lastClosedChannel = closedChannels[^1]; + closedChannels.Remove(lastClosedChannel); + // If the user already joined the channel, try the next // channel in the list if (joinedChannels.IndexOf(lastClosedChannel) >= 0) From 35f403dacd6b8999a7d51c90d9d176eb2a4fdc88 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Sun, 13 Dec 2020 22:05:54 -0800 Subject: [PATCH 12/50] Added tests for ctrl + w and ctrl + t --- .../Visual/Online/TestSceneChatOverlay.cs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index fca642ad6c..e80bab407e 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -207,6 +207,62 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } + + private void pressControlW() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press W", () => InputManager.Key(Key.W)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + } + + + public void TestCtrlWShortcut() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Want to close channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + + pressControlW(); + + // Channel 2 should be closed + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + // Want to close channel 1 + AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); + + pressControlW(); + + // Channel 1 and channel 2 should be closed + AddAssert("channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + } + + private void pressControlT() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press T", () => InputManager.Key(Key.T)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + } + + public void TestCtrlTShortcut() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Want to join another channel + pressControlT(); + // Selector should be visible + AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); + } private void pressChannelHotkey(int number) { From ccbf6db810be69444c87c9ffa91d139730e2bea3 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Sun, 13 Dec 2020 22:11:52 -0800 Subject: [PATCH 13/50] Fixed some formatting and style --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index e80bab407e..44901992e7 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -214,8 +214,8 @@ namespace osu.Game.Tests.Visual.Online AddStep("Press W", () => InputManager.Key(Key.W)); AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); } - + [Test] public void TestCtrlWShortcut() { AddStep("Join 2 channels", () => @@ -250,6 +250,7 @@ namespace osu.Game.Tests.Visual.Online AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); } + [Test] public void TestCtrlTShortcut() { AddStep("Join 2 channels", () => From 5903569386a7bd35f6abb075c5bf5331965c5414 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 13:00:02 -0500 Subject: [PATCH 14/50] Changing the Tests --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index fddaaa8215..d4cc60ba97 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -255,8 +255,7 @@ namespace osu.Game.Tests.Visual.Online AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); AddStep("Click channel 1 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); - AddAssert("channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any(); pressControlShiftT(); // Channel 1 should now be open again. Channel 2 should not From b26946ba8e38f50ec64cc10943d505d087e879cd Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 13:09:34 -0500 Subject: [PATCH 15/50] Temporarily removed test cases I did this to try to figure out what was failing on pull requests --- .../Visual/Online/TestSceneChatOverlay.cs | 64 ------------------- 1 file changed, 64 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index d4cc60ba97..d444c2e9f1 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -219,70 +219,6 @@ namespace osu.Game.Tests.Visual.Online AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); } - public void TestCtrlShiftTShortcut1() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); - - // Close channel 2 - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - - pressControlShiftT(); - - // Channel 2 should now be open again - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } - - public void TestCtrlShiftTShortcut2() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); - - // Close channels 1 and channel 2 - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); - - AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - AddStep("Click channel 1 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); - - AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any(); - - pressControlShiftT(); - // Channel 1 should now be open again. Channel 2 should not - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - - pressControlShiftT(); - - // Both Channel 1 and Channel 2 should be open - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - - // Processing this again should have not effect - pressControlShiftT(); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - - // Close channel 2 again - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); - - // Both channel 1 and channel 2 should be open - pressControlShiftT(); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } - private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From bd2765ecc4fe8900ccf60a693b1b30e33635cdb4 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 13:23:43 -0500 Subject: [PATCH 16/50] Fixed style concerns --- osu.Game/Online/Chat/ChannelManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 63eea0b190..607c9a1223 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -420,7 +420,7 @@ namespace osu.Game.Online.Chat // by removing the oldest element if (closedChannels.Count >= closed_channels_max_size) { - closedChannels.Remove(closedChannels[0]); + closedChannels.RemoveAt(0); } // insert at the end of the closedChannels list @@ -445,8 +445,7 @@ namespace osu.Game.Online.Chat return; } - // equivalent to Channel lastClosedChannel = closedChannels[closedChannels.Count - 1]; - Channel lastClosedChannel = closedChannels[^1]; + Channel lastClosedChannel = closedChannels.Last(); closedChannels.Remove(lastClosedChannel); From 9cb0ed3b8f42716db57540e78c11f5326faabfc3 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Mon, 14 Dec 2020 12:38:59 -0800 Subject: [PATCH 17/50] Fixed formatting for tests --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 44901992e7..53577f41ee 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -207,18 +207,16 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - private void pressControlW() { AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); AddStep("Press W", () => InputManager.Key(Key.W)); AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); } - [Test] public void TestCtrlWShortcut() { - AddStep("Join 2 channels", () => + AddStep("Join 2 channels", () => { channelManager.JoinChannel(channel1); channelManager.JoinChannel(channel2); @@ -228,7 +226,6 @@ namespace osu.Game.Tests.Visual.Online AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); pressControlW(); - // Channel 2 should be closed AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); @@ -237,10 +234,8 @@ namespace osu.Game.Tests.Visual.Online AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); pressControlW(); - // Channel 1 and channel 2 should be closed - AddAssert("channel 1 closed", () => !channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } private void pressControlT() @@ -253,7 +248,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestCtrlTShortcut() { - AddStep("Join 2 channels", () => + AddStep("Join 2 channels", () => { channelManager.JoinChannel(channel1); channelManager.JoinChannel(channel2); From 61fe6ca84e91ddccfc80b5d20baf4b6f233cd6a9 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 15:47:48 -0500 Subject: [PATCH 18/50] Added back CI tests --- .../Visual/Online/TestSceneChatOverlay.cs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index d444c2e9f1..adeeae8652 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -219,6 +219,70 @@ namespace osu.Game.Tests.Visual.Online AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); } + public void TestCtrlShiftTShortcut1() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Close channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + pressControlShiftT(); + + // Channel 2 should now be open again + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } + + public void TestCtrlShiftTShortcut2() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Close channels 1 and channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + + AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); + AddStep("Click channel 1 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); + + AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); + + pressControlShiftT(); + // Channel 1 should now be open again. Channel 2 should not + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + pressControlShiftT(); + + // Both Channel 1 and Channel 2 should be open + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + + // Processing this again should have not effect + pressControlShiftT(); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + + // Close channel 2 again + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + + // Both channel 1 and channel 2 should be open + pressControlShiftT(); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } + private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From 1af0f79ddf4b1af252539164198fd1947982d072 Mon Sep 17 00:00:00 2001 From: Angela Zhang <43687061+angelaz1@users.noreply.github.com> Date: Mon, 14 Dec 2020 16:58:43 -0600 Subject: [PATCH 19/50] Adding newline style fixes --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 53577f41ee..d39f0678c8 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -207,12 +207,14 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } + private void pressControlW() { AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); AddStep("Press W", () => InputManager.Key(Key.W)); AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); } + [Test] public void TestCtrlWShortcut() { From 206312d4c2feafe29b1a8c0bdf927126f40bbd2e Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 18:26:57 -0500 Subject: [PATCH 20/50] Changed the CI tests I'm trying to make changes because something is going wrong with theGitHub CI tests. --- .../Visual/Online/TestSceneChatOverlay.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index adeeae8652..556b42390e 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -228,8 +228,7 @@ namespace osu.Game.Tests.Visual.Online }); // Close channel 2 - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); @@ -249,12 +248,8 @@ namespace osu.Game.Tests.Visual.Online }); // Close channels 1 and channel 2 - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); - - AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - AddStep("Click channel 1 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); - + AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); + AddStep("Leave channel 1", () => channelManager.LeaveChannel(channel1)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); pressControlShiftT(); @@ -274,8 +269,7 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); // Close channel 2 again - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Click channel 2 close button", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); // Both channel 1 and channel 2 should be open pressControlShiftT(); From 017e00eb268eeaa176d18b128f20a9f53a39ee9c Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Mon, 14 Dec 2020 15:27:57 -0800 Subject: [PATCH 21/50] More style fixes --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index d39f0678c8..3dfa5d09be 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -207,14 +207,12 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - private void pressControlW() { AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); AddStep("Press W", () => InputManager.Key(Key.W)); AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - } - + } [Test] public void TestCtrlWShortcut() { From 37a601088786d3dcdccc85adc04195acf4f74edd Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 18:33:51 -0500 Subject: [PATCH 22/50] Changed test formatting Tests should be prefixed with [Test]. Previously my tests do not have this. --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 556b42390e..3eaba131ad 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -219,6 +219,7 @@ namespace osu.Game.Tests.Visual.Online AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); } + [Test] public void TestCtrlShiftTShortcut1() { AddStep("Join 2 channels", () => @@ -239,6 +240,7 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); } + [Test] public void TestCtrlShiftTShortcut2() { AddStep("Join 2 channels", () => From 7fe7c24ce15cd1ccd0c19020b9d38d26960e4ca8 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Mon, 14 Dec 2020 15:44:40 -0800 Subject: [PATCH 23/50] Style fixes for whitespaces --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 3dfa5d09be..68fc8ac8c0 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -207,12 +207,14 @@ namespace osu.Game.Tests.Visual.Online AddStep("Click normal close button", () => clickDrawable(((TestChannelTabItem)chatOverlay.TabMap[channel1]).CloseButton.Child)); AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } + private void pressControlW() { AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); AddStep("Press W", () => InputManager.Key(Key.W)); AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - } + } + [Test] public void TestCtrlWShortcut() { From bd2e42e7867153fb88851e7defb5fe7ec3828665 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 19:10:26 -0500 Subject: [PATCH 24/50] Toggling out CI tests I'm still trying to track donwn why GitHub won't accept my CI. I'm toggling tests to see what is the problem. --- .../Visual/Online/TestSceneChatOverlay.cs | 63 +++++++++---------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 3eaba131ad..fd83f761a6 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -240,45 +240,44 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); } - [Test] - public void TestCtrlShiftTShortcut2() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); + // [Test] + // public void TestCtrlShiftTShortcut2() + // { + // AddStep("Join 2 channels", () => + // { + // channelManager.JoinChannel(channel1); + // channelManager.JoinChannel(channel2); + // }); - // Close channels 1 and channel 2 - AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - AddStep("Leave channel 1", () => channelManager.LeaveChannel(channel1)); - AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); + // // Close channels 1 and channel 2 + // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); + // AddStep("Leave channel 1", () => channelManager.LeaveChannel(channel1)); + // AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); - pressControlShiftT(); - // Channel 1 should now be open again. Channel 2 should not - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + // pressControlShiftT(); + // // Channel 1 should now be open again. Channel 2 should not + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - pressControlShiftT(); + // pressControlShiftT(); - // Both Channel 1 and Channel 2 should be open - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + // // Both Channel 1 and Channel 2 should be open + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // Processing this again should have not effect - pressControlShiftT(); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + // // Processing this again should have not effect + // pressControlShiftT(); + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // Close channel 2 again - AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - - // Both channel 1 and channel 2 should be open - pressControlShiftT(); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } + // // Close channel 2 again + // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); + // // Both channel 1 and channel 2 should be open + // pressControlShiftT(); + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + // } private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From f17df1fdf59c71ad3d3e32f2175bbd5e8d3bb575 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 19:27:04 -0500 Subject: [PATCH 25/50] Commented out all CI tests Part of my efforts to narrow down GitHub CI issues --- .../Visual/Online/TestSceneChatOverlay.cs | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index fd83f761a6..43ff5dc78c 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -210,36 +210,35 @@ namespace osu.Game.Tests.Visual.Online // Control-Shift-T should reopen the most recently closed tab that has not // already been reopened - private void pressControlShiftT() - { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); - AddStep("Press T", () => InputManager.Key(Key.T)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); - } + // private void pressControlShiftT() + // { + // AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + // AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); + // AddStep("Press T", () => InputManager.Key(Key.T)); + // AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + // AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + // } - [Test] - public void TestCtrlShiftTShortcut1() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); + // [Test] + // public void TestCtrlShiftTShortcut1() + // { + // AddStep("Join 2 channels", () => + // { + // channelManager.JoinChannel(channel1); + // channelManager.JoinChannel(channel2); + // }); - // Close channel 2 - AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + // // Close channel 2 + // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - pressControlShiftT(); - - // Channel 2 should now be open again - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } + // pressControlShiftT(); + // // Channel 2 should now be open again + // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + // } // [Test] // public void TestCtrlShiftTShortcut2() // { From 9af448d559f465b50921f1f5ed5b5d8cc0abd8de Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 22:02:34 -0500 Subject: [PATCH 26/50] Testing first CI test --- .../Visual/Online/TestSceneChatOverlay.cs | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 43ff5dc78c..ce590839c2 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -210,35 +210,36 @@ namespace osu.Game.Tests.Visual.Online // Control-Shift-T should reopen the most recently closed tab that has not // already been reopened - // private void pressControlShiftT() - // { - // AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - // AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); - // AddStep("Press T", () => InputManager.Key(Key.T)); - // AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - // AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); - // } + private void pressControlShiftT() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); + AddStep("Press T", () => InputManager.Key(Key.T)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + } - // [Test] - // public void TestCtrlShiftTShortcut1() - // { - // AddStep("Join 2 channels", () => - // { - // channelManager.JoinChannel(channel1); - // channelManager.JoinChannel(channel2); - // }); + [Test] + public void TestCtrlShiftTShortcut1() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); - // // Close channel 2 - // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + // Close channel 2 + AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - // pressControlShiftT(); + pressControlShiftT(); + + // Channel 2 should now be open again + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } - // // Channel 2 should now be open again - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // } // [Test] // public void TestCtrlShiftTShortcut2() // { @@ -247,31 +248,30 @@ namespace osu.Game.Tests.Visual.Online // channelManager.JoinChannel(channel1); // channelManager.JoinChannel(channel2); // }); - + // // // Close channels 1 and channel 2 // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); // AddStep("Leave channel 1", () => channelManager.LeaveChannel(channel1)); // AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); - + // // pressControlShiftT(); // // Channel 1 should now be open again. Channel 2 should not // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); // AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - + // // pressControlShiftT(); - + // // // Both Channel 1 and Channel 2 should be open // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - + // // // Processing this again should have not effect // pressControlShiftT(); // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - + // // // Close channel 2 again // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - // // Both channel 1 and channel 2 should be open // pressControlShiftT(); // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); From 5dc917c1081c52a7bf2978d69eb11fecd0022604 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 22:30:16 -0500 Subject: [PATCH 27/50] Modified test case I believe the tests are failing because the shortcut only works when you select a channel on the channel dropdown. So I updated the testcases to do this. --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index ce590839c2..2db6bf6220 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -233,6 +233,8 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); + pressControlShiftT(); // Channel 2 should now be open again From c000d6d622512eead83e310ae9e0a116a4f9b288 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Mon, 14 Dec 2020 23:30:57 -0500 Subject: [PATCH 28/50] Minor formatting change Still trying to fix the CI problems --- .../Visual/Online/TestSceneChatOverlay.cs | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 2db6bf6220..15fb12df6a 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -242,43 +242,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); } - // [Test] - // public void TestCtrlShiftTShortcut2() - // { - // AddStep("Join 2 channels", () => - // { - // channelManager.JoinChannel(channel1); - // channelManager.JoinChannel(channel2); - // }); - // - // // Close channels 1 and channel 2 - // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - // AddStep("Leave channel 1", () => channelManager.LeaveChannel(channel1)); - // AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); - // - // pressControlShiftT(); - // // Channel 1 should now be open again. Channel 2 should not - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - // - // pressControlShiftT(); - // - // // Both Channel 1 and Channel 2 should be open - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // - // // Processing this again should have not effect - // pressControlShiftT(); - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // - // // Close channel 2 again - // AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - // // Both channel 1 and channel 2 should be open - // pressControlShiftT(); - // AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - // AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - // } private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From 16a103952282b574cf2893cf9c028f31c1ee8e55 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Tue, 15 Dec 2020 00:00:43 -0500 Subject: [PATCH 29/50] Completely removed all CI tests THe GitHub CI is still failing for unknown reasons. I tried to remove them all to see what the problem really is --- .../Visual/Online/TestSceneChatOverlay.cs | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 15fb12df6a..fca642ad6c 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -208,40 +208,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - // Control-Shift-T should reopen the most recently closed tab that has not - // already been reopened - private void pressControlShiftT() - { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); - AddStep("Press T", () => InputManager.Key(Key.T)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); - } - - [Test] - public void TestCtrlShiftTShortcut1() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); - - // Close channel 2 - AddStep("Leave channel 2", () => channelManager.LeaveChannel(channel2)); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - - AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - - pressControlShiftT(); - - // Channel 2 should now be open again - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } - private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From bd32dbdc2f6a9491ba931356140799d687ad3b8a Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Tue, 15 Dec 2020 00:10:11 -0500 Subject: [PATCH 30/50] Added back a modified test case Added a simple test for Ctrl+Shift+T. --- .../Visual/Online/TestSceneChatOverlay.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index fca642ad6c..8795e50645 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -208,6 +208,41 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } + // Control-Shift-T should reopen the most recently closed tab that has not + // already been reopened + private void pressControlShiftT() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); + AddStep("Press T", () => InputManager.Key(Key.T)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); + } + + [Test] + public void TestCtrlShiftTShortcut1() + { + AddStep("Join 2 channels", () => + { + channelManager.JoinChannel(channel1); + channelManager.JoinChannel(channel2); + }); + + // Close channel 2 + AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Close channel 2", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + + AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); + + pressControlShiftT(); + + // Channel 2 should now be open again + AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); + } + private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From 1b702b2b1f0341f1336351f8ebded3874caa1030 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Tue, 15 Dec 2020 11:57:51 -0500 Subject: [PATCH 31/50] Removed all CI tests Just trying to get a commit that passes all CI tests --- .../Visual/Online/TestSceneChatOverlay.cs | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 8795e50645..fca642ad6c 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -208,41 +208,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - // Control-Shift-T should reopen the most recently closed tab that has not - // already been reopened - private void pressControlShiftT() - { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press and hold Shift", () => InputManager.PressKey(Key.ShiftLeft)); - AddStep("Press T", () => InputManager.Key(Key.T)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - AddStep("Release Shift", () => InputManager.ReleaseKey(Key.ShiftLeft)); - } - - [Test] - public void TestCtrlShiftTShortcut1() - { - AddStep("Join 2 channels", () => - { - channelManager.JoinChannel(channel1); - channelManager.JoinChannel(channel2); - }); - - // Close channel 2 - AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Close channel 2", () => clickDrawable(((TestPrivateChannelTabItem)chatOverlay.TabMap[channel2]).CloseButton.Child)); - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); - - AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - - pressControlShiftT(); - - // Channel 2 should now be open again - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 open", () => channelManager.JoinedChannels.Contains(channel2)); - } - private void pressChannelHotkey(int number) { var channelKey = Key.Number0 + number; From 8bd787dce976fb4564d5ebb26f283f1cbfa67e86 Mon Sep 17 00:00:00 2001 From: Jess Meng Date: Tue, 15 Dec 2020 20:50:23 -0800 Subject: [PATCH 32/50] Moved private methods beneath public ones --- .../Visual/Online/TestSceneChatOverlay.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 68fc8ac8c0..9d0be85a3a 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -208,13 +208,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - private void pressControlW() - { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press W", () => InputManager.Key(Key.W)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - } - [Test] public void TestCtrlWShortcut() { @@ -240,13 +233,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } - private void pressControlT() - { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press T", () => InputManager.Key(Key.T)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); - } - [Test] public void TestCtrlTShortcut() { @@ -270,6 +256,20 @@ namespace osu.Game.Tests.Visual.Online InputManager.ReleaseKey(Key.AltLeft); } + private void pressControlW() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press W", () => InputManager.Key(Key.W)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + } + + private void pressControlT() + { + AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); + AddStep("Press T", () => InputManager.Key(Key.T)); + AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + } + private void clickDrawable(Drawable d) { InputManager.MoveMouseTo(d); From 7b169c4f62428c69fbaee874eb22ca278bb72f78 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Wed, 16 Dec 2020 10:13:50 -0500 Subject: [PATCH 33/50] Changed internal closedChannel list to store channels names instead of channel objects This is for efficiency purposes; it's more efficient to just store the names than store the whole object. --- osu.Game/Online/Chat/ChannelManager.cs | 27 ++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 607c9a1223..303696c294 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,8 +33,8 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); - // Keeps a list of closed channels. More recently closed channels appear at higher indeces - private readonly BindableList closedChannels = new BindableList(); + // Keeps a list of closed channel identifiers + private readonly BindableList closedChannels = new BindableList(); // 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. @@ -424,7 +424,7 @@ namespace osu.Game.Online.Chat } // insert at the end of the closedChannels list - closedChannels.Insert(closedChannels.Count, channel); + closedChannels.Insert(closedChannels.Count, channel.Name); if (channel.Joined.Value) { @@ -445,13 +445,24 @@ namespace osu.Game.Online.Chat return; } - Channel lastClosedChannel = closedChannels.Last(); + string lastClosedChannelName = closedChannels.Last(); + closedChannels.RemoveAt(closedChannels.Count - 1); - closedChannels.Remove(lastClosedChannel); + // types did not work with existing enumerable interfaces funcitons like Contains + for (int i = 0; i < joinedChannels.Count; i++) + { + // If the user already joined the channel, try the next + // channel in the list + if (joinedChannels[i].Name == lastClosedChannelName) + { + JoinLastClosedChannel(); + return; + } + } - // If the user already joined the channel, try the next - // channel in the list - if (joinedChannels.IndexOf(lastClosedChannel) >= 0) + Channel lastClosedChannel = AvailableChannels.FirstOrDefault(c => c.Name == lastClosedChannelName); + + if (lastClosedChannel == null) { JoinLastClosedChannel(); } From 2f8a085adfb89ce696f6eb1212a63d9287da46fc Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Wed, 16 Dec 2020 12:04:07 -0500 Subject: [PATCH 34/50] Reworked reopening last tab to no longer use recursion A reviewer of the pull request was concerned about recursion. I changed the code to be iterative. --- osu.Game/Online/Chat/ChannelManager.cs | 56 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 303696c294..1bbd2c3471 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -445,33 +445,47 @@ namespace osu.Game.Online.Chat return; } - string lastClosedChannelName = closedChannels.Last(); - closedChannels.RemoveAt(closedChannels.Count - 1); - - // types did not work with existing enumerable interfaces funcitons like Contains - for (int i = 0; i < joinedChannels.Count; i++) + // 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 + // can only occur if the user actually decides to use this feature. + for (int i = closedChannels.Count - 1; i >= 0; i--) { - // If the user already joined the channel, try the next - // channel in the list - if (joinedChannels[i].Name == lastClosedChannelName) + string lastClosedChannelName = closedChannels.Last(); + closedChannels.RemoveAt(closedChannels.Count - 1); + bool alreadyJoined = false; + // If the user already joined the channel, do not + // try to join it + for (int j = 0; j < joinedChannels.Count; j++) { - JoinLastClosedChannel(); - return; + if (joinedChannels[j].Name == lastClosedChannelName) + { + alreadyJoined = true; + break; + } + } + + if (alreadyJoined == false) + { + Channel lastClosedChannel = AvailableChannels.FirstOrDefault(c => c.Name == lastClosedChannelName); + + if (lastClosedChannel == null) + { + continue; + } + else + { + JoinChannel(lastClosedChannel); + return; + } } } - - Channel lastClosedChannel = AvailableChannels.FirstOrDefault(c => c.Name == lastClosedChannelName); - - if (lastClosedChannel == null) - { - JoinLastClosedChannel(); - } - else - { - JoinChannel(lastClosedChannel); - } } + + private long lastMessageId; private bool channelsInitialised; From b37a983fbf6397d2acc8a244dd408f62b8305f18 Mon Sep 17 00:00:00 2001 From: Joseph-Ramos-CMU Date: Wed, 16 Dec 2020 12:56:36 -0500 Subject: [PATCH 35/50] Formatting fixes Fixed some formatting issues to comply with InspectCode. --- osu.Game/Online/Chat/ChannelManager.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 1bbd2c3471..51bbe6ca45 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -447,7 +447,7 @@ namespace osu.Game.Online.Chat // 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 + // 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 // can only occur if the user actually decides to use this feature. @@ -456,6 +456,7 @@ namespace osu.Game.Online.Chat string lastClosedChannelName = closedChannels.Last(); closedChannels.RemoveAt(closedChannels.Count - 1); bool alreadyJoined = false; + // If the user already joined the channel, do not // try to join it for (int j = 0; j < joinedChannels.Count; j++) @@ -471,11 +472,7 @@ namespace osu.Game.Online.Chat { Channel lastClosedChannel = AvailableChannels.FirstOrDefault(c => c.Name == lastClosedChannelName); - if (lastClosedChannel == null) - { - continue; - } - else + if (lastClosedChannel != null) { JoinChannel(lastClosedChannel); return; @@ -484,8 +481,6 @@ namespace osu.Game.Online.Chat } } - - private long lastMessageId; private bool channelsInitialised; From 71a082110abe74015dfbaf62b7e6b1dca7b2a726 Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Thu, 17 Dec 2020 16:56:34 -0600 Subject: [PATCH 36/50] Making style changes + supports reopening PM chats --- osu.Game/Online/Chat/ChannelManager.cs | 54 ++++++++++++++------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 51bbe6ca45..54ef3c6c30 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,8 +33,12 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); - // Keeps a list of closed channel identifiers - private readonly BindableList closedChannels = new BindableList(); + + /// + /// Keeps a list of closed channel identifiers. Stores the channel ID for normal + /// channels, or the user ID for PM channels + /// + private readonly BindableList closedChannelIds = new BindableList(); // 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. @@ -418,13 +422,13 @@ namespace osu.Game.Online.Chat // Prevent the closedChannel list from exceeding the max size // by removing the oldest element - if (closedChannels.Count >= closed_channels_max_size) + if (closedChannelIds.Count >= closed_channels_max_size) { - closedChannels.RemoveAt(0); + closedChannelIds.RemoveAt(0); } - // insert at the end of the closedChannels list - closedChannels.Insert(closedChannels.Count, channel.Name); + // 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); if (channel.Joined.Value) { @@ -440,7 +444,7 @@ namespace osu.Game.Online.Chat /// public void JoinLastClosedChannel() { - if (closedChannels.Count <= 0) + if (closedChannelIds.Count <= 0) { return; } @@ -451,32 +455,32 @@ namespace osu.Game.Online.Chat // every time the user joins a channel, which would make joining a channel // slower. I wanted to centralize all major slowdowns so they // can only occur if the user actually decides to use this feature. - for (int i = closedChannels.Count - 1; i >= 0; i--) + while (closedChannelIds.Count != 0) { - string lastClosedChannelName = closedChannels.Last(); - closedChannels.RemoveAt(closedChannels.Count - 1); - bool alreadyJoined = false; + long lastClosedChannelId = closedChannelIds.Last(); + closedChannelIds.RemoveAt(closedChannelIds.Count - 1); - // If the user already joined the channel, do not - // try to join it - for (int j = 0; j < joinedChannels.Count; j++) - { - if (joinedChannels[j].Name == lastClosedChannelName) - { - alreadyJoined = true; - break; - } - } + bool lookupCondition(Channel ch) => + ch.Type == ChannelType.PM ? ch.Users.Any(u => u.Id == lastClosedChannelId) + : ch.Id == lastClosedChannelId; - if (alreadyJoined == false) + // If the user hasn't already joined the channel, try to join it + if (joinedChannels.FirstOrDefault(lookupCondition) == null) { - Channel lastClosedChannel = AvailableChannels.FirstOrDefault(c => c.Name == lastClosedChannelName); + Channel lastClosedChannel = AvailableChannels.FirstOrDefault(lookupCondition); if (lastClosedChannel != null) { - JoinChannel(lastClosedChannel); - return; + CurrentChannel.Value = JoinChannel(lastClosedChannel); } + else + { + // Try to get User to open PM chat + var req = new GetUserRequest(lastClosedChannelId); + req.Success += user => CurrentChannel.Value = JoinChannel(new Channel(user)); + api.Queue(req); + } + return; } } } From cbfa292c05d0d67e57c60066cc3181a692c4d6ef Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Thu, 17 Dec 2020 16:59:19 -0600 Subject: [PATCH 37/50] Minor test style fixes --- .../Visual/Online/TestSceneChatOverlay.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 9d0be85a3a..12a24963c4 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -220,15 +220,15 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 2 AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - pressControlW(); + AddStep("Press Ctrl + W", () => pressControlW()); // Channel 2 should be closed - AddAssert("channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); - AddAssert("channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); + AddAssert("Channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); + AddAssert("Channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); // Want to close channel 1 AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - pressControlW(); + AddStep("Press Ctrl + W", () => pressControlW()); // Channel 1 and channel 2 should be closed AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } @@ -243,7 +243,7 @@ namespace osu.Game.Tests.Visual.Online }); // Want to join another channel - pressControlT(); + AddStep("Press Ctrl + T", () => pressControlT()); // Selector should be visible AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); } @@ -258,16 +258,16 @@ namespace osu.Game.Tests.Visual.Online private void pressControlW() { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press W", () => InputManager.Key(Key.W)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + InputManager.PressKey(Key.ControlLeft); + InputManager.Key(Key.W); + InputManager.ReleaseKey(Key.ControlLeft); } private void pressControlT() { - AddStep("Press and hold Control", () => InputManager.PressKey(Key.ControlLeft)); - AddStep("Press T", () => InputManager.Key(Key.T)); - AddStep("Release Control", () => InputManager.ReleaseKey(Key.ControlLeft)); + InputManager.PressKey(Key.ControlLeft); + InputManager.Key(Key.T); + InputManager.ReleaseKey(Key.ControlLeft); } private void clickDrawable(Drawable d) From 454c7538c0dcc38fc823ae1d59d43475d9ccd8c9 Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Thu, 17 Dec 2020 17:59:36 -0600 Subject: [PATCH 38/50] CI Style Fixes --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 7 ++++--- osu.Game/Online/Chat/ChannelManager.cs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 12a24963c4..52dafbcfb2 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -219,8 +219,8 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 2 AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); + AddStep("Press Ctrl + W", pressControlW); - AddStep("Press Ctrl + W", () => pressControlW()); // Channel 2 should be closed AddAssert("Channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); AddAssert("Channel 2 closed", () => !channelManager.JoinedChannels.Contains(channel2)); @@ -228,7 +228,7 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 1 AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - AddStep("Press Ctrl + W", () => pressControlW()); + AddStep("Press Ctrl + W", pressControlW); // Channel 1 and channel 2 should be closed AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } @@ -243,7 +243,8 @@ namespace osu.Game.Tests.Visual.Online }); // Want to join another channel - AddStep("Press Ctrl + T", () => pressControlT()); + AddStep("Press Ctrl + T", pressControlT); + // Selector should be visible AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); } diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 54ef3c6c30..5936c4e580 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -33,7 +33,6 @@ namespace osu.Game.Online.Chat private readonly BindableList availableChannels = new BindableList(); private readonly BindableList joinedChannels = new BindableList(); - /// /// Keeps a list of closed channel identifiers. Stores the channel ID for normal /// channels, or the user ID for PM channels @@ -461,8 +460,9 @@ namespace osu.Game.Online.Chat closedChannelIds.RemoveAt(closedChannelIds.Count - 1); bool lookupCondition(Channel ch) => - ch.Type == ChannelType.PM ? ch.Users.Any(u => u.Id == lastClosedChannelId) - : ch.Id == lastClosedChannelId; + 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) @@ -480,6 +480,7 @@ namespace osu.Game.Online.Chat req.Success += user => CurrentChannel.Value = JoinChannel(new Channel(user)); api.Queue(req); } + return; } } From 45482e8709d50527aa9a292cb0c0599390bff50d Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Thu, 17 Dec 2020 18:43:39 -0600 Subject: [PATCH 39/50] Whitespace fixes --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 2 +- osu.Game/Online/Chat/ChannelManager.cs | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 52dafbcfb2..8cb2d3df95 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -244,7 +244,7 @@ namespace osu.Game.Tests.Visual.Online // Want to join another channel AddStep("Press Ctrl + T", pressControlT); - + // Selector should be visible AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); } diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 5936c4e580..164330e78b 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -459,10 +459,9 @@ namespace osu.Game.Online.Chat long lastClosedChannelId = closedChannelIds.Last(); closedChannelIds.RemoveAt(closedChannelIds.Count - 1); - bool lookupCondition(Channel ch) => - ch.Type == ChannelType.PM - ? ch.Users.Any(u => u.Id == lastClosedChannelId) - : ch.Id == lastClosedChannelId; + 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) @@ -480,7 +479,7 @@ namespace osu.Game.Online.Chat req.Success += user => CurrentChannel.Value = JoinChannel(new Channel(user)); api.Queue(req); } - + return; } } From 7d326c7f2462d72f1d87d7e1d4e6ddf90fc0cafc Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Sun, 20 Dec 2020 13:18:00 -0600 Subject: [PATCH 40/50] Review changes + added tests --- .../Visual/Online/TestSceneChatOverlay.cs | 58 ++++++++++++- osu.Game/Online/Chat/ChannelManager.cs | 81 ++++++++++++------- 2 files changed, 110 insertions(+), 29 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 8cb2d3df95..beb069d2a2 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/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); diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 164330e78b..ecf5c6e245 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/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 joinedChannels = new BindableList(); /// - /// Keeps a list of closed channel identifiers. Stores the channel ID for normal - /// channels, or the user ID for PM channels + /// Keeps a stack of recently closed channels /// - private readonly BindableList closedChannelIds = new BindableList(); + private readonly BindableList closedChannels = new BindableList(); // 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 /// public void JoinLastClosedChannel() { - if (closedChannelIds.Count <= 0) - { - return; - } - - // This nested loop could be eliminated if a check was added so that + // This 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(); - closedChannelIds.RemoveAt(closedChannelIds.Count - 1); - - bool lookupCondition(Channel ch) => ch.Type == ChannelType.PM - ? ch.Users.Any(u => u.Id == lastClosedChannelId) - : ch.Id == lastClosedChannelId; + ClosedChannel lastClosedChannel = closedChannels.Last(); + closedChannels.RemoveAt(closedChannels.Count - 1); // 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); - req.Success += user => CurrentChannel.Value = JoinChannel(new Channel(user)); - api.Queue(req); + users.GetUserAsync((int)lastClosedChannel.Id).ContinueWith(u => + { + if (u.Result == null) return; + + Schedule(() => CurrentChannel.Value = JoinChannel(new Channel(u.Result))); + }); } return; @@ -569,4 +570,28 @@ namespace osu.Game.Online.Chat { } } + + /// + /// Class that stores information about a closed channel + /// + 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; + } + } } From 74bd2f41e60b35553f2bad451c1c225a224eebea Mon Sep 17 00:00:00 2001 From: Angela Zhang Date: Sun, 20 Dec 2020 13:51:39 -0600 Subject: [PATCH 41/50] Style fixes --- osu.Game/Online/Chat/ChannelManager.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index ecf5c6e245..f4d1aeae84 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -62,7 +62,6 @@ namespace osu.Game.Online.Chat [Resolved] private IAPIProvider api { get; set; } - [Resolved] private UserLookupCache users { get; set; } @@ -587,11 +586,11 @@ namespace osu.Game.Online.Chat public bool IsEqual(Channel channel) { - if (channel.Type != this.Type) return false; + if (channel.Type != Type) return false; - return this.Type == ChannelType.PM - ? channel.Users.Single().Id == this.Id - : channel.Id == this.Id; + return Type == ChannelType.PM + ? channel.Users.Single().Id == Id + : channel.Id == Id; } } } From a6cc37eb3b7a84de7159ea1e4f270f3f157419a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Jun 2021 14:56:21 +0900 Subject: [PATCH 42/50] Mark fields `readonly` --- osu.Game/Online/Chat/ChannelManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index e5e38e425f..f6804fdbf7 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -587,8 +587,8 @@ namespace osu.Game.Online.Chat /// public class ClosedChannel { - public ChannelType Type; - public long Id; + public readonly ChannelType Type; + public readonly long Id; public ClosedChannel(ChannelType type, long id) { From a91015302e5de78319fcddad63ad6888def53076 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Jun 2021 14:56:29 +0900 Subject: [PATCH 43/50] Replace second usage of new function --- osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs index c314e10aff..c0de093425 100644 --- a/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs +++ b/osu.Game/Overlays/Chat/Tabs/ChannelTabControl.cs @@ -81,9 +81,11 @@ namespace osu.Game.Overlays.Chat.Tabs RemoveItem(channel); if (SelectedTab == null) - SelectTab(selectorTab); + SelectChannelSelectorTab(); } + public void SelectChannelSelectorTab() => SelectTab(selectorTab); + protected override void SelectTab(TabItem tab) { if (tab is ChannelSelectorTabItem) @@ -96,11 +98,6 @@ namespace osu.Game.Overlays.Chat.Tabs selectorTab.Active.Value = false; } - public void SelectChannelSelectorTab() - { - SelectTab(selectorTab); - } - protected override TabFillFlowContainer CreateTabFlow() => new ChannelTabFillFlowContainer { Direction = FillDirection.Full, From e74fe68c9656947476ed88e6ef859b7a75de8c60 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Jun 2021 15:04:07 +0900 Subject: [PATCH 44/50] Use new platform actions instead of hardcoded keys --- osu.Game/Overlays/ChatOverlay.cs | 50 ++++++++++++++++---------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 84653226cc..0aa6108815 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -24,13 +24,15 @@ using osu.Game.Overlays.Chat.Tabs; using osuTK.Input; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; +using osu.Framework.Input; +using osu.Framework.Input.Bindings; using osu.Framework.Localisation; using osu.Game.Localisation; using osu.Game.Online; namespace osu.Game.Overlays { - public class ChatOverlay : OsuFocusedOverlayContainer, INamedOverlayComponent + public class ChatOverlay : OsuFocusedOverlayContainer, INamedOverlayComponent, IKeyBindingHandler { public string IconTexture => "Icons/Hexacons/messaging"; public LocalisableString Title => ChatStrings.HeaderTitle; @@ -367,33 +369,31 @@ namespace osu.Game.Overlays } } - if (e.ControlPressed) - { - if (e.ShiftPressed) - { - switch (e.Key) - { - case Key.T: - channelManager.JoinLastClosedChannel(); - return true; - } - } - else - { - switch (e.Key) - { - case Key.W: - channelManager.LeaveChannel(channelManager.CurrentChannel.Value); - return true; + return base.OnKeyDown(e); + } - case Key.T: - ChannelTabControl.SelectChannelSelectorTab(); - return true; - } - } + public bool OnPressed(PlatformAction action) + { + switch (action.ActionType) + { + case PlatformActionType.TabNew: + ChannelTabControl.SelectChannelSelectorTab(); + return true; + + case PlatformActionType.TabRestore: + channelManager.JoinLastClosedChannel(); + return true; + + case PlatformActionType.DocumentClose: + channelManager.LeaveChannel(channelManager.CurrentChannel.Value); + return true; } - return base.OnKeyDown(e); + return false; + } + + public void OnReleased(PlatformAction action) + { } public override bool AcceptsFocus => true; From b917d6d80c2edf191a6548762aca01e288e2562a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 3 Jun 2021 15:34:06 +0900 Subject: [PATCH 45/50] Update tests to check for correct platform action keys --- .../Visual/Online/TestSceneChatOverlay.cs | 55 +++++++++---------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 8dbccde3d6..c40b60c35d 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -9,6 +9,8 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.UserInterface; +using osu.Framework.Input; +using osu.Framework.Platform; using osu.Framework.Testing; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; @@ -38,6 +40,9 @@ namespace osu.Game.Tests.Visual.Online private Channel channel2 => channels[1]; private Channel channel3 => channels[2]; + [Resolved] + private GameHost host { get; set; } + public TestSceneChatOverlay() { channels = Enumerable.Range(1, 10) @@ -231,7 +236,7 @@ namespace osu.Game.Tests.Visual.Online } [Test] - public void TestCtrlWShortcut() + public void TestCloseTabShortcut() { AddStep("Join 2 channels", () => { @@ -241,7 +246,7 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 2 AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Press Ctrl + W", pressControlW); + AddStep("Press keys", pressCloseDocumentKeys); // Channel 2 should be closed AddAssert("Channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); @@ -250,13 +255,13 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 1 AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - AddStep("Press Ctrl + W", pressControlW); + AddStep("Press keys", pressCloseDocumentKeys); // Channel 1 and channel 2 should be closed AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } [Test] - public void TestCtrlTShortcut() + public void TestNewTabShortcut() { AddStep("Join 2 channels", () => { @@ -265,14 +270,14 @@ namespace osu.Game.Tests.Visual.Online }); // Want to join another channel - AddStep("Press Ctrl + T", pressControlT); + AddStep("Press keys", pressNewTabKeys); // Selector should be visible AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); } [Test] - public void TestCtrlShiftTShortcut() + public void TestRestoreTabShortcut() { AddStep("Join 3 channels", () => { @@ -282,7 +287,7 @@ namespace osu.Game.Tests.Visual.Online }); // Should do nothing - AddStep("Press Ctrl + Shift + T", pressControlShiftT); + AddStep("Press keys", pressRestoreTabKeys); AddAssert("All channels still open", () => channelManager.JoinedChannels.Count == 3); // Close channel 1 @@ -292,7 +297,7 @@ namespace osu.Game.Tests.Visual.Online AddAssert("Other channels still open", () => channelManager.JoinedChannels.Count == 2); // Reopen channel 1 - AddStep("Press Ctrl + Shift + T", pressControlShiftT); + AddStep("Press keys", pressRestoreTabKeys); AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3); AddAssert("Current channel is channel 1", () => currentChannel == channel1); @@ -305,13 +310,13 @@ namespace osu.Game.Tests.Visual.Online AddAssert("Current channel is channel 3", () => currentChannel == channel3); // Should first re-open channel 2 - AddStep("Press Ctrl + Shift + T", pressControlShiftT); + AddStep("Press keys", pressRestoreTabKeys); 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); + AddStep("Press keys", pressRestoreTabKeys); AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3); AddAssert("Current channel is channel 1", () => currentChannel == channel1); } @@ -324,27 +329,21 @@ namespace osu.Game.Tests.Visual.Online InputManager.ReleaseKey(Key.AltLeft); } - private void pressControlW() - { - InputManager.PressKey(Key.ControlLeft); - InputManager.Key(Key.W); - InputManager.ReleaseKey(Key.ControlLeft); - } + private void pressCloseDocumentKeys() => pressKeysFor(PlatformActionType.DocumentClose); - private void pressControlT() - { - InputManager.PressKey(Key.ControlLeft); - InputManager.Key(Key.T); - InputManager.ReleaseKey(Key.ControlLeft); - } + private void pressNewTabKeys() => pressKeysFor(PlatformActionType.TabNew); - private void pressControlShiftT() + private void pressRestoreTabKeys() => pressKeysFor(PlatformActionType.TabRestore); + + private void pressKeysFor(PlatformActionType type) { - InputManager.PressKey(Key.ControlLeft); - InputManager.PressKey(Key.ShiftLeft); - InputManager.Key(Key.T); - InputManager.ReleaseKey(Key.ShiftLeft); - InputManager.ReleaseKey(Key.ControlLeft); + var binding = host.PlatformKeyBindings.First(b => ((PlatformAction)b.Action).ActionType == type); + + foreach (var k in binding.KeyCombination.Keys) + InputManager.PressKey((Key)k); + + foreach (var k in binding.KeyCombination.Keys) + InputManager.ReleaseKey((Key)k); } private void clickDrawable(Drawable d) From 8193691cbcaed3f84383e595b9428dd395e3348e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 3 Jun 2021 13:58:19 +0200 Subject: [PATCH 46/50] Invert condition to reduce nesting --- osu.Game/Online/Chat/ChannelManager.cs | 42 +++++++++++++------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index f6804fdbf7..cf0fd2cbe6 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -471,29 +471,29 @@ namespace osu.Game.Online.Chat ClosedChannel lastClosedChannel = closedChannels.Last(); closedChannels.RemoveAt(closedChannels.Count - 1); - // If the user hasn't already joined the channel, try to join it - if (joinedChannels.FirstOrDefault(lastClosedChannel.IsEqual) == null) + // If the user has already joined the channel, try the next one + if (joinedChannels.FirstOrDefault(lastClosedChannel.IsEqual) != null) + continue; + + Channel lastChannel = AvailableChannels.FirstOrDefault(lastClosedChannel.IsEqual); + + if (lastChannel != null) { - Channel lastChannel = AvailableChannels.FirstOrDefault(lastClosedChannel.IsEqual); - - if (lastChannel != null) - { - // Channel exists as an availaable channel, directly join it - CurrentChannel.Value = JoinChannel(lastChannel); - } - else if (lastClosedChannel.Type == ChannelType.PM) - { - // Try to get User to open PM chat - users.GetUserAsync((int)lastClosedChannel.Id).ContinueWith(u => - { - if (u.Result == null) return; - - Schedule(() => CurrentChannel.Value = JoinChannel(new Channel(u.Result))); - }); - } - - return; + // Channel exists as an availaable channel, directly join it + CurrentChannel.Value = JoinChannel(lastChannel); } + else if (lastClosedChannel.Type == ChannelType.PM) + { + // Try to get User to open PM chat + users.GetUserAsync((int)lastClosedChannel.Id).ContinueWith(u => + { + if (u.Result == null) return; + + Schedule(() => CurrentChannel.Value = JoinChannel(new Channel(u.Result))); + }); + } + + return; } } From e8c2483f19c17197552f67135773888870d0e294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 3 Jun 2021 13:59:25 +0200 Subject: [PATCH 47/50] Use standard list instead of bindable list No reason to use a bindable list there, as `CollectionChanged` was never subscribed to. --- osu.Game/Online/Chat/ChannelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index cf0fd2cbe6..617471f75f 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -37,7 +37,7 @@ namespace osu.Game.Online.Chat /// /// Keeps a stack of recently closed channels /// - private readonly BindableList closedChannels = new BindableList(); + private readonly List closedChannels = new List(); // 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. From b2cc2a51ec112ef2834b2f566d75375b3b4781ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 3 Jun 2021 14:13:01 +0200 Subject: [PATCH 48/50] Rename method to be less misleading Would rather avoid variations of Equals/Equal/IsEqual. There's not really much equality involved as the types are different. --- osu.Game/Online/Chat/ChannelManager.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index 617471f75f..bdce366c06 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -472,10 +472,10 @@ namespace osu.Game.Online.Chat closedChannels.RemoveAt(closedChannels.Count - 1); // If the user has already joined the channel, try the next one - if (joinedChannels.FirstOrDefault(lastClosedChannel.IsEqual) != null) + if (joinedChannels.FirstOrDefault(lastClosedChannel.Matches) != null) continue; - Channel lastChannel = AvailableChannels.FirstOrDefault(lastClosedChannel.IsEqual); + Channel lastChannel = AvailableChannels.FirstOrDefault(lastClosedChannel.Matches); if (lastChannel != null) { @@ -596,7 +596,7 @@ namespace osu.Game.Online.Chat Id = id; } - public bool IsEqual(Channel channel) + public bool Matches(Channel channel) { if (channel.Type != Type) return false; From 5f5f3a8c5c17a3a634ff49e9ce0bfb086ec4e3c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 3 Jun 2021 14:20:52 +0200 Subject: [PATCH 49/50] General comment cleanups --- osu.Game/Online/Chat/ChannelManager.cs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/osu.Game/Online/Chat/ChannelManager.cs b/osu.Game/Online/Chat/ChannelManager.cs index bdce366c06..8507887357 100644 --- a/osu.Game/Online/Chat/ChannelManager.cs +++ b/osu.Game/Online/Chat/ChannelManager.cs @@ -441,7 +441,7 @@ namespace osu.Game.Online.Chat closedChannels.RemoveAt(0); } - // For PM channels, we store the user ID; else, we store the channel id + // For PM channels, we store the user ID; else, we store the channel ID closedChannels.Add(channel.Type == ChannelType.PM ? new ClosedChannel(ChannelType.PM, channel.Users.Single().Id) : new ClosedChannel(channel.Type, channel.Id)); @@ -454,18 +454,14 @@ namespace osu.Game.Online.Chat }); /// - /// Opens the most recently closed channel that has not - /// already been reopened + /// Opens the most recently closed channel that has not already been reopened, /// Works similarly to reopening the last closed tab on a web browser. /// public void JoinLastClosedChannel() { - // This 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. We wanted to centralize all major slowdowns so they - // can only occur if the user actually decides to use this feature. + // This loop could be eliminated if the join channel operation ensured that every channel joined + // is removed from the closedChannels list, but it'd require a linear scan of closed channels on every join. + // To keep the overhead of joining channels low, just lazily scan the list of closed channels locally. while (closedChannels.Count > 0) { ClosedChannel lastClosedChannel = closedChannels.Last(); @@ -479,12 +475,12 @@ namespace osu.Game.Online.Chat if (lastChannel != null) { - // Channel exists as an availaable channel, directly join it + // Channel exists as an available channel, directly join it CurrentChannel.Value = JoinChannel(lastChannel); } else if (lastClosedChannel.Type == ChannelType.PM) { - // Try to get User to open PM chat + // Try to get user in order to open PM chat users.GetUserAsync((int)lastClosedChannel.Id).ContinueWith(u => { if (u.Result == null) return; @@ -583,7 +579,7 @@ namespace osu.Game.Online.Chat } /// - /// Class that stores information about a closed channel + /// Stores information about a closed channel /// public class ClosedChannel { From e117f98bfaf099bde18bc72a66007d43348deee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 3 Jun 2021 14:31:13 +0200 Subject: [PATCH 50/50] Rename test steps --- .../Visual/Online/TestSceneChatOverlay.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index c40b60c35d..3971146ff8 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -246,7 +246,7 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 2 AddStep("Select channel 2", () => clickDrawable(chatOverlay.TabMap[channel2])); - AddStep("Press keys", pressCloseDocumentKeys); + AddStep("Close tab via shortcut", pressCloseDocumentKeys); // Channel 2 should be closed AddAssert("Channel 1 open", () => channelManager.JoinedChannels.Contains(channel1)); @@ -255,7 +255,7 @@ namespace osu.Game.Tests.Visual.Online // Want to close channel 1 AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); - AddStep("Press keys", pressCloseDocumentKeys); + AddStep("Close tab via shortcut", pressCloseDocumentKeys); // Channel 1 and channel 2 should be closed AddAssert("All channels closed", () => !channelManager.JoinedChannels.Any()); } @@ -270,7 +270,7 @@ namespace osu.Game.Tests.Visual.Online }); // Want to join another channel - AddStep("Press keys", pressNewTabKeys); + AddStep("Press new tab shortcut", pressNewTabKeys); // Selector should be visible AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); @@ -287,7 +287,7 @@ namespace osu.Game.Tests.Visual.Online }); // Should do nothing - AddStep("Press keys", pressRestoreTabKeys); + AddStep("Restore tab via shortcut", pressRestoreTabKeys); AddAssert("All channels still open", () => channelManager.JoinedChannels.Count == 3); // Close channel 1 @@ -297,7 +297,7 @@ namespace osu.Game.Tests.Visual.Online AddAssert("Other channels still open", () => channelManager.JoinedChannels.Count == 2); // Reopen channel 1 - AddStep("Press keys", pressRestoreTabKeys); + AddStep("Restore tab via shortcut", pressRestoreTabKeys); AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3); AddAssert("Current channel is channel 1", () => currentChannel == channel1); @@ -310,13 +310,13 @@ namespace osu.Game.Tests.Visual.Online AddAssert("Current channel is channel 3", () => currentChannel == channel3); // Should first re-open channel 2 - AddStep("Press keys", pressRestoreTabKeys); + AddStep("Restore tab via shortcut", pressRestoreTabKeys); 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 keys", pressRestoreTabKeys); + AddStep("Restore tab via shortcut", pressRestoreTabKeys); AddAssert("All channels now open", () => channelManager.JoinedChannels.Count == 3); AddAssert("Current channel is channel 1", () => currentChannel == channel1); }