From 46e66e66e477507c3c7564930b2c615ed81f47da Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 17 Mar 2022 08:19:38 +0300 Subject: [PATCH 1/2] Make opening chat overlay opt-in to add coverage for unloaded chat overlays Causes `TestHighlightWhileChatNeverOpen` to fail as expected. --- .../Visual/Online/TestSceneChatOverlay.cs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index 00ff6a9576..80a6698761 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -122,6 +122,8 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestHideOverlay() { + AddStep("Open chat overlay", () => chatOverlay.Show()); + AddAssert("Chat overlay is visible", () => chatOverlay.State.Value == Visibility.Visible); AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); @@ -134,6 +136,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestChannelSelection() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddAssert("Selector is visible", () => chatOverlay.SelectionOverlayState == Visibility.Visible); AddStep("Setup get message response", () => onGetMessages = channel => { @@ -169,6 +172,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestSearchInSelector() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Search for 'no. 2'", () => chatOverlay.ChildrenOfType().First().Text = "no. 2"); AddUntilStep("Only channel 2 visible", () => { @@ -180,6 +184,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestChannelShortcutKeys() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join channels", () => channels.ForEach(channel => channelManager.JoinChannel(channel))); AddStep("Close channel selector", () => InputManager.Key(Key.Escape)); AddUntilStep("Wait for close", () => chatOverlay.SelectionOverlayState == Visibility.Hidden); @@ -199,6 +204,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestCloseChannelBehaviour() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddUntilStep("Join until dropdown has channels", () => { if (visibleChannels.Count() < joinedChannels.Count()) @@ -269,6 +275,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestChannelCloseButton() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join 2 channels", () => { channelManager.JoinChannel(channel1); @@ -289,6 +296,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestCloseTabShortcut() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join 2 channels", () => { channelManager.JoinChannel(channel1); @@ -314,6 +322,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestNewTabShortcut() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join 2 channels", () => { channelManager.JoinChannel(channel1); @@ -330,6 +339,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestRestoreTabShortcut() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join 3 channels", () => { channelManager.JoinChannel(channel1); @@ -375,6 +385,7 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestChatCommand() { + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join channel 1", () => channelManager.JoinChannel(channel1)); AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); @@ -398,6 +409,8 @@ namespace osu.Game.Tests.Visual.Online { Channel multiplayerChannel = null; + AddStep("open chat overlay", () => chatOverlay.Show()); + AddStep("join multiplayer channel", () => channelManager.JoinChannel(multiplayerChannel = new Channel(new APIUser()) { Name = "#mp_1", @@ -417,6 +430,7 @@ namespace osu.Game.Tests.Visual.Online { Message message = null; + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join channel 1", () => channelManager.JoinChannel(channel1)); AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); @@ -443,6 +457,7 @@ namespace osu.Game.Tests.Visual.Online { Message message = null; + AddStep("Open chat overlay", () => chatOverlay.Show()); AddStep("Join channel 1", () => channelManager.JoinChannel(channel1)); AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); @@ -471,6 +486,8 @@ namespace osu.Game.Tests.Visual.Online { Message message = null; + AddStep("Open chat overlay", () => chatOverlay.Show()); + AddStep("Join channel 1", () => channelManager.JoinChannel(channel1)); AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); @@ -496,14 +513,11 @@ namespace osu.Game.Tests.Visual.Online } [Test] - public void TestHighlightWhileChatHidden() + public void TestHighlightWhileChatNeverOpen() { Message message = null; - AddStep("hide chat", () => chatOverlay.Hide()); - AddStep("Join channel 1", () => channelManager.JoinChannel(channel1)); - AddStep("Select channel 1", () => clickDrawable(chatOverlay.TabMap[channel1])); AddStep("Send message in channel 1", () => { @@ -520,7 +534,7 @@ namespace osu.Game.Tests.Visual.Online }); }); - AddStep("Highlight message and show chat", () => + AddStep("Highlight message and open chat", () => { chatOverlay.HighlightMessage(message, channel1); chatOverlay.Show(); @@ -571,8 +585,6 @@ namespace osu.Game.Tests.Visual.Online ChannelManager, ChatOverlay = new TestChatOverlay { RelativeSizeAxes = Axes.Both, }, }; - - ChatOverlay.Show(); } } From 7aae9bbd1b33943fab4301934db6d3d20322660e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 17 Mar 2022 08:31:38 +0300 Subject: [PATCH 2/2] Improve channel bindable logic in `ChatOverlay` to avoid potential nullrefs --- osu.Game/Overlays/ChatOverlay.cs | 38 ++++++++++++-------------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index 3764ac42fc..3d39c7ce3a 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -73,6 +73,10 @@ namespace osu.Game.Overlays private Container channelSelectionContainer; protected ChannelSelectionOverlay ChannelSelectionOverlay; + private readonly IBindableList availableChannels = new BindableList(); + private readonly IBindableList joinedChannels = new BindableList(); + private readonly Bindable currentChannel = new Bindable(); + public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos) || (ChannelSelectionOverlay.State.Value == Visibility.Visible && ChannelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos)); @@ -198,9 +202,13 @@ namespace osu.Game.Overlays }, }; + availableChannels.BindTo(channelManager.AvailableChannels); + joinedChannels.BindTo(channelManager.JoinedChannels); + currentChannel.BindTo(channelManager.CurrentChannel); + textBox.OnCommit += postMessage; - ChannelTabControl.Current.ValueChanged += current => channelManager.CurrentChannel.Value = current.NewValue; + ChannelTabControl.Current.ValueChanged += current => currentChannel.Value = current.NewValue; ChannelTabControl.ChannelSelectorActive.ValueChanged += active => ChannelSelectionOverlay.State.Value = active.NewValue ? Visibility.Visible : Visibility.Hidden; ChannelSelectionOverlay.State.ValueChanged += state => { @@ -238,18 +246,12 @@ namespace osu.Game.Overlays Schedule(() => { // TODO: consider scheduling bindable callbacks to not perform when overlay is not present. - channelManager.JoinedChannels.BindCollectionChanged(joinedChannelsChanged, true); - - channelManager.AvailableChannels.CollectionChanged += availableChannelsChanged; - availableChannelsChanged(null, null); - - currentChannel = channelManager.CurrentChannel.GetBoundCopy(); + joinedChannels.BindCollectionChanged(joinedChannelsChanged, true); + availableChannels.BindCollectionChanged(availableChannelsChanged, true); currentChannel.BindValueChanged(currentChannelChanged, true); }); } - private Bindable currentChannel; - private void currentChannelChanged(ValueChangedEvent e) { if (e.NewValue == null) @@ -318,7 +320,7 @@ namespace osu.Game.Overlays if (!channel.Joined.Value) channel = channelManager.JoinChannel(channel); - channelManager.CurrentChannel.Value = channel; + currentChannel.Value = channel; } channel.HighlightedMessage.Value = message; @@ -407,7 +409,7 @@ namespace osu.Game.Overlays return true; case PlatformAction.DocumentClose: - channelManager.LeaveChannel(channelManager.CurrentChannel.Value); + channelManager.LeaveChannel(currentChannel.Value); return true; } @@ -487,19 +489,7 @@ namespace osu.Game.Overlays private void availableChannelsChanged(object sender, NotifyCollectionChangedEventArgs args) { - ChannelSelectionOverlay.UpdateAvailableChannels(channelManager.AvailableChannels); - } - - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - - if (channelManager != null) - { - channelManager.CurrentChannel.ValueChanged -= currentChannelChanged; - channelManager.JoinedChannels.CollectionChanged -= joinedChannelsChanged; - channelManager.AvailableChannels.CollectionChanged -= availableChannelsChanged; - } + ChannelSelectionOverlay.UpdateAvailableChannels(availableChannels); } private void postMessage(TextBox textBox, bool newText)