From 5b3ffb12b7c978f4ec1a4c8de8f346b854a5b9d3 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Fri, 4 Mar 2022 23:23:58 +0300
Subject: [PATCH] Refactor channel scrolling container to handle manual scrolls
 resiliently

---
 .../Online/TestSceneStandAloneChatDisplay.cs  | 27 ++++++++++++++++---
 .../Containers/UserTrackingScrollContainer.cs | 21 ++++++++++-----
 osu.Game/Overlays/Chat/DrawableChannel.cs     | 19 ++++++-------
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs
index 779d72190d..9b9ee0e084 100644
--- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs
+++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs
@@ -207,7 +207,28 @@ namespace osu.Game.Tests.Visual.Online
         }
 
         [Test]
-        public void TestUserScrollOverride()
+        public void TestOverrideChatScrolling()
+        {
+            fillChat();
+
+            sendMessage();
+            checkScrolledToBottom();
+
+            AddStep("Scroll to start", () => chatDisplay.ScrollContainer.ScrollToStart());
+
+            checkNotScrolledToBottom();
+            sendMessage();
+            checkNotScrolledToBottom();
+
+            AddStep("Scroll to bottom", () => chatDisplay.ScrollContainer.ScrollToEnd());
+
+            checkScrolledToBottom();
+            sendMessage();
+            checkScrolledToBottom();
+        }
+
+        [Test]
+        public void TestOverrideChatScrollingByUser()
         {
             fillChat();
 
@@ -314,9 +335,9 @@ namespace osu.Game.Tests.Visual.Online
             {
             }
 
-            protected DrawableChannel DrawableChannel => InternalChildren.OfType<DrawableChannel>().First();
+            public DrawableChannel DrawableChannel => InternalChildren.OfType<DrawableChannel>().First();
 
-            protected UserTrackingScrollContainer ScrollContainer => (UserTrackingScrollContainer)((Container)DrawableChannel.Child).Child;
+            public UserTrackingScrollContainer ScrollContainer => (UserTrackingScrollContainer)((Container)DrawableChannel.Child).Child;
 
             public FillFlowContainer FillFlow => (FillFlowContainer)ScrollContainer.Child;
 
diff --git a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs
index 0561051e35..988695c380 100644
--- a/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs
+++ b/osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs
@@ -25,8 +25,6 @@ namespace osu.Game.Graphics.Containers
         /// </summary>
         public bool UserScrolling { get; private set; }
 
-        public void CancelUserScroll() => UserScrolling = false;
-
         public UserTrackingScrollContainer()
         {
         }
@@ -38,26 +36,37 @@ namespace osu.Game.Graphics.Containers
 
         protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default)
         {
-            UserScrolling = true;
             base.OnUserScroll(value, animated, distanceDecay);
+            OnScrollChange(true);
         }
 
         public new void ScrollIntoView(Drawable target, bool animated = true)
         {
-            UserScrolling = false;
             base.ScrollIntoView(target, animated);
+            OnScrollChange(false);
         }
 
         public new void ScrollTo(float value, bool animated = true, double? distanceDecay = null)
         {
-            UserScrolling = false;
             base.ScrollTo(value, animated, distanceDecay);
+            OnScrollChange(false);
+        }
+
+        public new void ScrollToStart(bool animated = true, bool allowDuringDrag = false)
+        {
+            base.ScrollToStart(animated, allowDuringDrag);
+            OnScrollChange(false);
         }
 
         public new void ScrollToEnd(bool animated = true, bool allowDuringDrag = false)
         {
-            UserScrolling = false;
             base.ScrollToEnd(animated, allowDuringDrag);
+            OnScrollChange(false);
         }
+
+        /// <summary>
+        /// Invoked when any scroll has been performed either automatically or by user.
+        /// </summary>
+        protected virtual void OnScrollChange(bool byUser) => UserScrolling = byUser;
     }
 }
diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs
index 41e70bbfae..ccad55b809 100644
--- a/osu.Game/Overlays/Chat/DrawableChannel.cs
+++ b/osu.Game/Overlays/Chat/DrawableChannel.cs
@@ -249,31 +249,32 @@ namespace osu.Game.Overlays.Chat
             /// </summary>
             private const float auto_scroll_leniency = 10f;
 
+            private bool trackNewContent = true;
             private float? lastExtent;
 
-            protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default)
+            protected override void OnScrollChange(bool byUser)
             {
-                base.OnUserScroll(value, animated, distanceDecay);
-                lastExtent = null;
+                base.OnScrollChange(byUser);
+
+                if (byUser)
+                    lastExtent = null;
+
+                trackNewContent = IsScrolledToEnd(auto_scroll_leniency);
             }
 
             protected override void Update()
             {
                 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))
-                    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));
+                bool requiresScrollUpdate = trackNewContent && (lastExtent == null || Precision.AlmostBigger(ScrollableExtent, lastExtent.Value));
 
                 if (requiresScrollUpdate)
                 {
                     // Schedule required to allow FillFlow to be the correct size.
                     Schedule(() =>
                     {
-                        if (!UserScrolling)
+                        if (trackNewContent)
                         {
                             if (Current < ScrollableExtent)
                                 ScrollToEnd();