From a9c8f9bd4aebe9929007a89bdb5a938aa1541077 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 17:47:38 +0900 Subject: [PATCH 01/24] Fix a potential crash when exiting the editor before a new beatmap is added to the database --- osu.Game/Screens/Edit/Editor.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index b7ebf0c0a4..66fe1a9507 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Threading; using osu.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -109,7 +110,16 @@ private void load(OsuColour colours, GameHost host, OsuConfigManager config) if (Beatmap.Value is DummyWorkingBeatmap) { isNewBeatmap = true; - Beatmap.Value = beatmapManager.CreateNew(Ruleset.Value, api.LocalUser.Value); + + var newBeatmap = beatmapManager.CreateNew(Ruleset.Value, api.LocalUser.Value); + + // this is a bit haphazard, but guards against setting the lease Beatmap bindable if + // the editor has already been exited. + if (!ValidForPush) + return; + + // this probably shouldn't be set in the asynchronous load method, but everything following relies on it. + Beatmap.Value = newBeatmap; } beatDivisor.Value = Beatmap.Value.BeatmapInfo.BeatDivisor; From b44bd8c4eea20cb3338c3f85e639a7472b6f5262 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Jan 2021 18:03:33 +0900 Subject: [PATCH 02/24] Remove unused using statement --- osu.Game/Screens/Edit/Editor.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 66fe1a9507..a49de3b887 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Threading; using osu.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; From 39d46d21e6e2453121e513a2025cd6c5a2bd9ba9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 31 Jan 2021 23:44:27 +0300 Subject: [PATCH 03/24] Add failing test case --- .../Online/TestSceneStandAloneChatDisplay.cs | 116 ++++++++++++------ 1 file changed, 80 insertions(+), 36 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index 492abdd88d..02ef024128 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -8,6 +8,7 @@ using osuTK; using System; using System.Linq; +using NUnit.Framework; using osu.Framework.Graphics.Containers; using osu.Game.Graphics.Containers; using osu.Game.Overlays.Chat; @@ -16,8 +17,6 @@ namespace osu.Game.Tests.Visual.Online { public class TestSceneStandAloneChatDisplay : OsuTestScene { - private readonly Channel testChannel = new Channel(); - private readonly User admin = new User { Username = "HappyStick", @@ -46,78 +45,84 @@ public class TestSceneStandAloneChatDisplay : OsuTestScene [Cached] private ChannelManager channelManager = new ChannelManager(); - private readonly TestStandAloneChatDisplay chatDisplay; - private readonly TestStandAloneChatDisplay chatDisplay2; + private TestStandAloneChatDisplay chatDisplay; + private TestStandAloneChatDisplay chatDisplay2; + private int messageIdSequence; + + private Channel testChannel; public TestSceneStandAloneChatDisplay() { Add(channelManager); - - Add(chatDisplay = new TestStandAloneChatDisplay - { - Anchor = Anchor.CentreLeft, - Origin = Anchor.CentreLeft, - Margin = new MarginPadding(20), - Size = new Vector2(400, 80) - }); - - Add(chatDisplay2 = new TestStandAloneChatDisplay(true) - { - Anchor = Anchor.CentreRight, - Origin = Anchor.CentreRight, - Margin = new MarginPadding(20), - Size = new Vector2(400, 150) - }); } - protected override void LoadComplete() + [SetUp] + public void SetUp() => Schedule(() => { - base.LoadComplete(); + messageIdSequence = 0; + channelManager.CurrentChannel.Value = testChannel = new Channel(); - channelManager.CurrentChannel.Value = testChannel; + Children = new[] + { + chatDisplay = new TestStandAloneChatDisplay + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + Margin = new MarginPadding(20), + Size = new Vector2(400, 80), + Channel = { Value = testChannel }, + }, + chatDisplay2 = new TestStandAloneChatDisplay(true) + { + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + Margin = new MarginPadding(20), + Size = new Vector2(400, 150), + Channel = { Value = testChannel }, + } + }; + }); - chatDisplay.Channel.Value = testChannel; - chatDisplay2.Channel.Value = testChannel; - - int sequence = 0; - - AddStep("message from admin", () => testChannel.AddNewMessages(new Message(sequence++) + [Test] + public void TestManyMessages() + { + AddStep("message from admin", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = admin, Content = "I am a wang!" })); - AddStep("message from team red", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message from team red", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = redUser, Content = "I am team red." })); - AddStep("message from team red", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message from team red", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = redUser, Content = "I plan to win!" })); - AddStep("message from team blue", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message from team blue", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = blueUser, Content = "Not on my watch. Prepare to eat saaaaaaaaaand. Lots and lots of saaaaaaand." })); - AddStep("message from admin", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message from admin", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = admin, Content = "Okay okay, calm down guys. Let's do this!" })); - AddStep("message from long username", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message from long username", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = longUsernameUser, Content = "Hi guys, my new username is lit!" })); - AddStep("message with new date", () => testChannel.AddNewMessages(new Message(sequence++) + AddStep("message with new date", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = longUsernameUser, Content = "Message from the future!", @@ -131,7 +136,7 @@ protected override void LoadComplete() { for (int i = 0; i < messages_per_call; i++) { - testChannel.AddNewMessages(new Message(sequence++) + testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = longUsernameUser, Content = "Many messages! " + Guid.NewGuid(), @@ -156,6 +161,45 @@ protected override void LoadComplete() AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); } + /// + /// Tests that when a message gets wrapped by the chat display getting contracted while scrolled to bottom, the chat will still keep scrolling down. + /// + [Test] + public void TestMessageWrappingKeepsAutoScrolling() + { + AddStep("fill chat", () => + { + for (int i = 0; i < 10; i++) + { + testChannel.AddNewMessages(new Message(messageIdSequence++) + { + Sender = longUsernameUser, + Content = $"some stuff {Guid.NewGuid()}", + }); + } + }); + + AddAssert("ensure scrolled to bottom", () => chatDisplay.ScrolledToBottom); + + // send message with short words for text wrapping to occur when contracting chat. + AddStep("send lorem ipsum", () => testChannel.AddNewMessages(new Message(messageIdSequence++) + { + Sender = longUsernameUser, + Content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce et bibendum velit.", + })); + + AddStep("contract chat", () => chatDisplay.Width -= 100); + AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); + + AddStep("send another message", () => testChannel.AddNewMessages(new Message(messageIdSequence++) + { + Sender = admin, + Content = "As we were saying...", + })); + + AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); + } + private class TestStandAloneChatDisplay : StandAloneChatDisplay { public TestStandAloneChatDisplay(bool textbox = false) From e806e5bcd1580f1c85026e53d289d7cf933e4767 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 31 Jan 2021 23:37:52 +0300 Subject: [PATCH 04/24] Improve robustness of chat auto-scrolling logic Fix auto-scrolling state changing by old messages removal logic --- osu.Game/Overlays/Chat/DrawableChannel.cs | 47 ++++++++++++++++------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 5926d11c03..f1aa387c17 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -24,7 +24,7 @@ public class DrawableChannel : Container { public readonly Channel Channel; protected FillFlowContainer ChatLineFlow; - private OsuScrollContainer scroll; + private ChannelScrollContainer scroll; private bool scrollbarVisible = true; @@ -56,7 +56,7 @@ private void load() { RelativeSizeAxes = Axes.Both, Masking = true, - Child = scroll = new OsuScrollContainer + Child = scroll = new ChannelScrollContainer { ScrollbarVisible = scrollbarVisible, RelativeSizeAxes = Axes.Both, @@ -80,12 +80,6 @@ private void load() Channel.PendingMessageResolved += pendingMessageResolved; } - protected override void LoadComplete() - { - base.LoadComplete(); - scrollToEnd(); - } - protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); @@ -113,8 +107,6 @@ private void newMessagesArrived(IEnumerable newMessages) => Schedule(() ChatLineFlow.Clear(); } - bool shouldScrollToEnd = scroll.IsScrolledToEnd(10) || !chatLines.Any() || newMessages.Any(m => m is LocalMessage); - // Add up to last Channel.MAX_HISTORY messages var displayMessages = newMessages.Skip(Math.Max(0, newMessages.Count() - Channel.MAX_HISTORY)); @@ -153,8 +145,10 @@ void expireAndAdjustScroll(Drawable d) } } - if (shouldScrollToEnd) - scrollToEnd(); + // due to the scroll adjusts from old messages removal above, a scroll-to-end must be enforced, + // to avoid making the container think the user has scrolled back up and unwantedly disable auto-scrolling. + if (scroll.ShouldAutoScroll || newMessages.Any(m => m is LocalMessage)) + ScheduleAfterChildren(() => scroll.ScrollToEnd()); }); private void pendingMessageResolved(Message existing, Message updated) => Schedule(() => @@ -178,8 +172,6 @@ private void messageRemoved(Message removed) => Schedule(() => private IEnumerable chatLines => ChatLineFlow.Children.OfType(); - private void scrollToEnd() => ScheduleAfterChildren(() => scroll.ScrollToEnd()); - public class DaySeparator : Container { public float TextSize @@ -243,5 +235,32 @@ public DaySeparator(DateTimeOffset time) }; } } + + /// + /// An with functionality to automatically scrolls whenever the maximum scrollable distance increases. + /// + private class ChannelScrollContainer : OsuScrollContainer + { + private const float auto_scroll_leniency = 10f; + + private float? lastExtent; + + /// + /// Whether this should automatically scroll to end on the next call to . + /// + public bool ShouldAutoScroll { get; private set; } = true; + + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + + if ((lastExtent == null || ScrollableExtent > lastExtent) && ShouldAutoScroll) + ScrollToEnd(); + else + ShouldAutoScroll = IsScrolledToEnd(auto_scroll_leniency); + + lastExtent = ScrollableExtent; + } + } } } From 49e62c3a4b7118c1f3c81c6af357534bd541d8ee Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 1 Feb 2021 11:02:08 +0300 Subject: [PATCH 05/24] Apply documentation changes Co-authored-by: Dean Herbert --- osu.Game/Overlays/Chat/DrawableChannel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index f1aa387c17..4c8513b1d5 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -237,7 +237,7 @@ public DaySeparator(DateTimeOffset time) } /// - /// An with functionality to automatically scrolls whenever the maximum scrollable distance increases. + /// An with functionality to automatically scroll whenever the maximum scrollable distance increases. /// private class ChannelScrollContainer : OsuScrollContainer { @@ -246,7 +246,7 @@ private class ChannelScrollContainer : OsuScrollContainer private float? lastExtent; /// - /// Whether this should automatically scroll to end on the next call to . + /// Whether this container should automatically scroll to end on the next call to . /// public bool ShouldAutoScroll { get; private set; } = true; From 286726feb0cb93c4f3d54e60d690979bba35c005 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Feb 2021 17:44:30 +0000 Subject: [PATCH 06/24] Bump SharpCompress from 0.26.0 to 0.27.1 Bumps [SharpCompress](https://github.com/adamhathcock/sharpcompress) from 0.26.0 to 0.27.1. - [Release notes](https://github.com/adamhathcock/sharpcompress/releases) - [Commits](https://github.com/adamhathcock/sharpcompress/compare/0.26...0.27.1) Signed-off-by: dependabot-preview[bot] --- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 1552dff17d..d0aeb10c9c 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -30,7 +30,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 48dc01f5de..b40e6a3346 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -89,7 +89,7 @@ - + From 0560676236e2b73eb25726b10a07927912b2b201 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Feb 2021 18:09:05 +0000 Subject: [PATCH 07/24] Bump ppy.osu.Framework.NativeLibs from 2020.923.0 to 2021.115.0 Bumps [ppy.osu.Framework.NativeLibs](https://github.com/ppy/osu-framework) from 2020.923.0 to 2021.115.0. - [Release notes](https://github.com/ppy/osu-framework/releases) - [Commits](https://github.com/ppy/osu-framework/compare/2020.923.0...2021.115.0) Signed-off-by: dependabot-preview[bot] --- osu.iOS.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.iOS.props b/osu.iOS.props index b40e6a3346..dc3527c687 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -93,6 +93,6 @@ - + From 57213e630806a6e25973fa19588009217a0ad9ea Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Feb 2021 18:09:07 +0000 Subject: [PATCH 08/24] Bump DiscordRichPresence from 1.0.169 to 1.0.175 Bumps [DiscordRichPresence](https://github.com/Lachee/discord-rpc-csharp) from 1.0.169 to 1.0.175. - [Release notes](https://github.com/Lachee/discord-rpc-csharp/releases) - [Commits](https://github.com/Lachee/discord-rpc-csharp/commits) Signed-off-by: dependabot-preview[bot] --- osu.Desktop/osu.Desktop.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index cce7907c6c..3e0f0cb7f6 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -30,7 +30,7 @@ - + From 15fcabb1282c8c36c4f005e6acba36a7d239fb11 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 1 Feb 2021 22:04:44 +0300 Subject: [PATCH 09/24] Add documentation to auto-scroll leniency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Overlays/Chat/DrawableChannel.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 4c8513b1d5..0cd5ecc05a 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -241,6 +241,11 @@ public DaySeparator(DateTimeOffset time) /// private class ChannelScrollContainer : OsuScrollContainer { + /// + /// The chat will be automatically scrolled to end if and only if + /// the distance between the current scroll position and the end of the scroll + /// is less than this value. + /// private const float auto_scroll_leniency = 10f; private float? lastExtent; From 5c28c030c86a270cd6435955ac82de4daa8a196e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 1 Feb 2021 22:08:55 +0300 Subject: [PATCH 10/24] Unconditionally set "autoscroll" state --- osu.Game/Overlays/Chat/DrawableChannel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 0cd5ecc05a..db6a27bf8c 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -261,8 +261,8 @@ protected override void UpdateAfterChildren() if ((lastExtent == null || ScrollableExtent > lastExtent) && ShouldAutoScroll) ScrollToEnd(); - else - ShouldAutoScroll = IsScrolledToEnd(auto_scroll_leniency); + + ShouldAutoScroll = IsScrolledToEnd(auto_scroll_leniency); lastExtent = ScrollableExtent; } From 216b0d89a729cff6ad311ebb440f32dcb1e174e6 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Feb 2021 19:16:51 +0000 Subject: [PATCH 11/24] Bump Sentry from 2.1.8 to 3.0.1 Bumps [Sentry](https://github.com/getsentry/sentry-dotnet) from 2.1.8 to 3.0.1. - [Release notes](https://github.com/getsentry/sentry-dotnet/releases) - [Changelog](https://github.com/getsentry/sentry-dotnet/blob/main/CHANGELOG.md) - [Commits](https://github.com/getsentry/sentry-dotnet/compare/2.1.8...3.0.1) Signed-off-by: dependabot-preview[bot] --- osu.Game/osu.Game.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index d0aeb10c9c..e2b506e187 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -29,7 +29,7 @@ - + From dcb1626e4d66649304d9307cdc63329f523192f9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 1 Feb 2021 22:38:42 +0300 Subject: [PATCH 12/24] Remove no longer necessary field --- osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index 02ef024128..8ea05784e9 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -46,7 +46,6 @@ public class TestSceneStandAloneChatDisplay : OsuTestScene private ChannelManager channelManager = new ChannelManager(); private TestStandAloneChatDisplay chatDisplay; - private TestStandAloneChatDisplay chatDisplay2; private int messageIdSequence; private Channel testChannel; @@ -72,7 +71,7 @@ public void SetUp() => Schedule(() => Size = new Vector2(400, 80), Channel = { Value = testChannel }, }, - chatDisplay2 = new TestStandAloneChatDisplay(true) + new TestStandAloneChatDisplay(true) { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, From 0d5353008c1d3e9520d7d12bf50d6f23b85d9712 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 13:34:34 +0900 Subject: [PATCH 13/24] Update sentry sdk usage --- osu.Game/Utils/SentryLogger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Utils/SentryLogger.cs b/osu.Game/Utils/SentryLogger.cs index e8e41cdbbe..be9d01cde6 100644 --- a/osu.Game/Utils/SentryLogger.cs +++ b/osu.Game/Utils/SentryLogger.cs @@ -23,7 +23,7 @@ public SentryLogger(OsuGame game) var options = new SentryOptions { - Dsn = new Dsn("https://5e342cd55f294edebdc9ad604d28bbd3@sentry.io/1255255"), + Dsn = "https://5e342cd55f294edebdc9ad604d28bbd3@sentry.io/1255255", Release = game.Version }; From ed63b571d2185d4b3e86d77a08c0f9ce656f819c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:16:26 +0900 Subject: [PATCH 14/24] Add "new" override for ScrollToEnd To UserTrackingScrollContainer --- osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs index b8ce34b204..be33c231c9 100644 --- a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs +++ b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs @@ -45,5 +45,11 @@ protected override void OnUserScroll(float value, bool animated = true, double? UserScrolling = false; base.ScrollTo(value, animated, distanceDecay); } + + public new void ScrollToEnd(bool animated = true, bool allowDuringDrag = false) + { + UserScrolling = false; + base.ScrollToEnd(animated, allowDuringDrag); + } } } From 398ab9c2c2283a8bad2dd2bf513103a606868e89 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:16:10 +0900 Subject: [PATCH 15/24] Use UserTrackingScrollContainer instead --- .../Online/TestSceneStandAloneChatDisplay.cs | 2 +- osu.Game/Overlays/Chat/DrawableChannel.cs | 32 ++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index 8ea05784e9..e7669262fe 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -208,7 +208,7 @@ public TestStandAloneChatDisplay(bool textbox = false) protected DrawableChannel DrawableChannel => InternalChildren.OfType().First(); - protected OsuScrollContainer ScrollContainer => (OsuScrollContainer)((Container)DrawableChannel.Child).Child; + protected UserTrackingScrollContainer ScrollContainer => (UserTrackingScrollContainer)((Container)DrawableChannel.Child).Child; public FillFlowContainer FillFlow => (FillFlowContainer)ScrollContainer.Child; diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index db6a27bf8c..1d021b331a 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -16,6 +16,7 @@ using osu.Game.Graphics; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics.Sprites; +using osu.Framework.Utils; using osu.Game.Graphics.Sprites; namespace osu.Game.Overlays.Chat @@ -147,8 +148,8 @@ void expireAndAdjustScroll(Drawable d) // due to the scroll adjusts from old messages removal above, a scroll-to-end must be enforced, // to avoid making the container think the user has scrolled back up and unwantedly disable auto-scrolling. - if (scroll.ShouldAutoScroll || newMessages.Any(m => m is LocalMessage)) - ScheduleAfterChildren(() => scroll.ScrollToEnd()); + if (newMessages.Any(m => m is LocalMessage)) + scroll.ScrollToEnd(); }); private void pendingMessageResolved(Message existing, Message updated) => Schedule(() => @@ -239,7 +240,7 @@ public DaySeparator(DateTimeOffset time) /// /// An with functionality to automatically scroll whenever the maximum scrollable distance increases. /// - private class ChannelScrollContainer : OsuScrollContainer + private class ChannelScrollContainer : UserTrackingScrollContainer { /// /// The chat will be automatically scrolled to end if and only if @@ -250,21 +251,30 @@ private class ChannelScrollContainer : OsuScrollContainer private float? lastExtent; - /// - /// Whether this container should automatically scroll to end on the next call to . - /// - public bool ShouldAutoScroll { get; private set; } = true; + protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default) + { + base.OnUserScroll(value, animated, distanceDecay); + lastExtent = null; + } protected override void UpdateAfterChildren() { base.UpdateAfterChildren(); - if ((lastExtent == null || ScrollableExtent > lastExtent) && ShouldAutoScroll) - ScrollToEnd(); + // If the user has scrolled to the bottom of the container, we should resume tracking new content. + bool cancelUserScroll = UserScrolling && IsScrolledToEnd(auto_scroll_leniency); - ShouldAutoScroll = IsScrolledToEnd(auto_scroll_leniency); + // If the user hasn't overridden our behaviour and there has been new content added to the container, we should update our scroll position to track it. + bool requiresScrollUpdate = !UserScrolling && (lastExtent == null || Precision.AlmostBigger(ScrollableExtent, lastExtent.Value)); - lastExtent = ScrollableExtent; + if (cancelUserScroll || requiresScrollUpdate) + { + ScheduleAfterChildren(() => + { + ScrollToEnd(); + lastExtent = ScrollableExtent; + }); + } } } } From bb0753f68d79860b0631938dbbea7b9cd9ab2210 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:44:03 +0900 Subject: [PATCH 16/24] Use a better method of cancelling user scroll --- .../Containers/UserTrackingScrollContainer.cs | 2 ++ osu.Game/Overlays/Chat/DrawableChannel.cs | 12 ++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs index be33c231c9..17506ce0f5 100644 --- a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs +++ b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs @@ -25,6 +25,8 @@ public class UserTrackingScrollContainer : OsuScrollContainer /// public bool UserScrolling { get; private set; } + public void CancelUserScroll() => UserScrolling = false; + public UserTrackingScrollContainer() { } diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 1d021b331a..de3057e9dc 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -262,17 +262,21 @@ protected override void UpdateAfterChildren() base.UpdateAfterChildren(); // If the user has scrolled to the bottom of the container, we should resume tracking new content. - bool cancelUserScroll = UserScrolling && IsScrolledToEnd(auto_scroll_leniency); + if (UserScrolling && IsScrolledToEnd(auto_scroll_leniency)) + CancelUserScroll(); // If the user hasn't overridden our behaviour and there has been new content added to the container, we should update our scroll position to track it. bool requiresScrollUpdate = !UserScrolling && (lastExtent == null || Precision.AlmostBigger(ScrollableExtent, lastExtent.Value)); - if (cancelUserScroll || requiresScrollUpdate) + if (requiresScrollUpdate) { ScheduleAfterChildren(() => { - ScrollToEnd(); - lastExtent = ScrollableExtent; + if (!UserScrolling) + { + ScrollToEnd(); + lastExtent = ScrollableExtent; + } }); } } From 3670bd40c2c8dee50cdbc4ab3a61cf412c603b15 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:44:11 +0900 Subject: [PATCH 17/24] Add test coverage of user scroll overriding --- .../Online/TestSceneStandAloneChatDisplay.cs | 82 +++++++++++++++---- 1 file changed, 66 insertions(+), 16 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index e7669262fe..0e1c90f88e 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -12,10 +12,11 @@ using osu.Framework.Graphics.Containers; using osu.Game.Graphics.Containers; using osu.Game.Overlays.Chat; +using osuTK.Input; namespace osu.Game.Tests.Visual.Online { - public class TestSceneStandAloneChatDisplay : OsuTestScene + public class TestSceneStandAloneChatDisplay : OsuManualInputManagerTestScene { private readonly User admin = new User { @@ -128,7 +129,7 @@ public void TestManyMessages() Timestamp = DateTimeOffset.Now })); - AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); + checkScrolledToBottom(); const int messages_per_call = 10; AddRepeatStep("add many messages", () => @@ -157,7 +158,7 @@ public void TestManyMessages() return true; }); - AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); + checkScrolledToBottom(); } /// @@ -165,6 +166,58 @@ public void TestManyMessages() /// [Test] public void TestMessageWrappingKeepsAutoScrolling() + { + fillChat(); + + // send message with short words for text wrapping to occur when contracting chat. + sendMessage(); + + AddStep("contract chat", () => chatDisplay.Width -= 100); + checkScrolledToBottom(); + + AddStep("send another message", () => testChannel.AddNewMessages(new Message(messageIdSequence++) + { + Sender = admin, + Content = "As we were saying...", + })); + + checkScrolledToBottom(); + } + + [Test] + public void TestUserScrollOverride() + { + fillChat(); + + sendMessage(); + checkScrolledToBottom(); + + AddStep("User scroll up", () => + { + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre); + InputManager.PressButton(MouseButton.Left); + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre + new Vector2(0, chatDisplay.ScreenSpaceDrawQuad.Height)); + InputManager.ReleaseButton(MouseButton.Left); + }); + + checkNotScrolledToBottom(); + sendMessage(); + checkNotScrolledToBottom(); + + AddRepeatStep("User scroll to bottom", () => + { + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre); + InputManager.PressButton(MouseButton.Left); + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre - new Vector2(0, chatDisplay.ScreenSpaceDrawQuad.Height)); + InputManager.ReleaseButton(MouseButton.Left); + }, 5); + + checkScrolledToBottom(); + sendMessage(); + checkScrolledToBottom(); + } + + private void fillChat() { AddStep("fill chat", () => { @@ -178,27 +231,24 @@ public void TestMessageWrappingKeepsAutoScrolling() } }); - AddAssert("ensure scrolled to bottom", () => chatDisplay.ScrolledToBottom); + checkScrolledToBottom(); + } - // send message with short words for text wrapping to occur when contracting chat. + private void sendMessage() + { AddStep("send lorem ipsum", () => testChannel.AddNewMessages(new Message(messageIdSequence++) { Sender = longUsernameUser, Content = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce et bibendum velit.", })); - - AddStep("contract chat", () => chatDisplay.Width -= 100); - AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); - - AddStep("send another message", () => testChannel.AddNewMessages(new Message(messageIdSequence++) - { - Sender = admin, - Content = "As we were saying...", - })); - - AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); } + private void checkScrolledToBottom() => + AddUntilStep("is scrolled to bottom", () => chatDisplay.ScrolledToBottom); + + private void checkNotScrolledToBottom() => + AddUntilStep("not scrolled to bottom", () => !chatDisplay.ScrolledToBottom); + private class TestStandAloneChatDisplay : StandAloneChatDisplay { public TestStandAloneChatDisplay(bool textbox = false) From b3105fb2920ac3f3c24e144550cbc598bbcf0cf3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:46:26 +0900 Subject: [PATCH 18/24] Add coverage of local echo messages performing automatic scrolling --- .../Online/TestSceneStandAloneChatDisplay.cs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index 0e1c90f88e..ee01eb5f3a 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -217,6 +217,33 @@ public void TestUserScrollOverride() checkScrolledToBottom(); } + [Test] + public void TestLocalEchoMessageResetsScroll() + { + fillChat(); + + sendMessage(); + checkScrolledToBottom(); + + AddStep("User scroll up", () => + { + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre); + InputManager.PressButton(MouseButton.Left); + InputManager.MoveMouseTo(chatDisplay.ScreenSpaceDrawQuad.Centre + new Vector2(0, chatDisplay.ScreenSpaceDrawQuad.Height)); + InputManager.ReleaseButton(MouseButton.Left); + }); + + checkNotScrolledToBottom(); + sendMessage(); + checkNotScrolledToBottom(); + + sendLocalMessage(); + checkScrolledToBottom(); + + sendMessage(); + checkScrolledToBottom(); + } + private void fillChat() { AddStep("fill chat", () => @@ -243,6 +270,15 @@ private void sendMessage() })); } + private void sendLocalMessage() + { + AddStep("send local echo", () => testChannel.AddLocalEcho(new LocalEchoMessage() + { + Sender = longUsernameUser, + Content = "This is a local echo message.", + })); + } + private void checkScrolledToBottom() => AddUntilStep("is scrolled to bottom", () => chatDisplay.ScrolledToBottom); From a76314a8760f98474067decfa23c8ed980def836 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 15:57:17 +0900 Subject: [PATCH 19/24] Use Update instead of UpdateAfterChildren (no need for the latter) --- osu.Game/Overlays/Chat/DrawableChannel.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index de3057e9dc..86ce724390 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -257,9 +257,9 @@ protected override void OnUserScroll(float value, bool animated = true, double? lastExtent = null; } - protected override void UpdateAfterChildren() + protected override void Update() { - base.UpdateAfterChildren(); + base.Update(); // If the user has scrolled to the bottom of the container, we should resume tracking new content. if (UserScrolling && IsScrolledToEnd(auto_scroll_leniency)) @@ -270,7 +270,8 @@ protected override void UpdateAfterChildren() if (requiresScrollUpdate) { - ScheduleAfterChildren(() => + // Schedule required to allow FillFlow to be the correct size. + Schedule(() => { if (!UserScrolling) { From 54c0bdf7d3174ec1b6ee5ba61abf096492d7fa2c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 16:04:42 +0900 Subject: [PATCH 20/24] Fix PlaylistLoungeTestScene appearing very narrow --- .../Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs index 008c862cc3..730bbbb397 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs @@ -28,12 +28,7 @@ public override void SetUpSteps() { base.SetUpSteps(); - AddStep("push screen", () => LoadScreen(loungeScreen = new PlaylistsLoungeSubScreen - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Width = 0.5f, - })); + AddStep("push screen", () => LoadScreen(loungeScreen = new PlaylistsLoungeSubScreen())); AddUntilStep("wait for present", () => loungeScreen.IsCurrentScreen()); } From 3002fef05eebf7c7102ff2e07838347b232d4749 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 16:11:13 +0900 Subject: [PATCH 21/24] Remove empty parenthesis --- osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index ee01eb5f3a..01e67b1681 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -272,7 +272,7 @@ private void sendMessage() private void sendLocalMessage() { - AddStep("send local echo", () => testChannel.AddLocalEcho(new LocalEchoMessage() + AddStep("send local echo", () => testChannel.AddLocalEcho(new LocalEchoMessage { Sender = longUsernameUser, Content = "This is a local echo message.", From 43052991f8120b51b6cce28f68f07f9250b09b47 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 16:18:55 +0900 Subject: [PATCH 22/24] Remove unused using statement --- .../Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs index 730bbbb397..618447eae2 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Graphics; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Graphics.Containers; From bdc05af4b7d3fcf43d919c594991f679410ae18d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 2 Feb 2021 16:30:45 +0900 Subject: [PATCH 23/24] Make playlist settings area taller to better match screen aspect ratio --- .../OnlinePlay/Playlists/PlaylistsMatchSettingsOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsMatchSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsMatchSettingsOverlay.cs index 01f9920609..ced6d1c5db 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsMatchSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsMatchSettingsOverlay.cs @@ -200,7 +200,7 @@ private void load(OsuColour colours) Child = new GridContainer { RelativeSizeAxes = Axes.X, - Height = 300, + Height = 500, Content = new[] { new Drawable[] From 180af3c7f8311d0a4477ecb79e7f7403da9dc85a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 2 Feb 2021 19:02:09 +0900 Subject: [PATCH 24/24] Add codeanalysis attribute --- osu.Game/Utils/ModUtils.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 9336add465..8ac5bde65a 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using osu.Game.Rulesets.Mods; @@ -45,7 +46,7 @@ public static bool CheckCompatibleSet(IEnumerable combination) /// The combination to check. /// Any invalid mods in the set. /// Whether all s in the combination are compatible with each-other. - public static bool CheckCompatibleSet(IEnumerable combination, out List? invalidMods) + public static bool CheckCompatibleSet(IEnumerable combination, [NotNullWhen(false)] out List? invalidMods) { combination = FlattenMods(combination).ToArray(); invalidMods = null;