From e806e5bcd1580f1c85026e53d289d7cf933e4767 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Sun, 31 Jan 2021 23:37:52 +0300
Subject: [PATCH] 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 @@ namespace osu.Game.Overlays.Chat
     {
         public readonly Channel Channel;
         protected FillFlowContainer ChatLineFlow;
-        private OsuScrollContainer scroll;
+        private ChannelScrollContainer scroll;
 
         private bool scrollbarVisible = true;
 
@@ -56,7 +56,7 @@ namespace osu.Game.Overlays.Chat
             {
                 RelativeSizeAxes = Axes.Both,
                 Masking = true,
-                Child = scroll = new OsuScrollContainer
+                Child = scroll = new ChannelScrollContainer
                 {
                     ScrollbarVisible = scrollbarVisible,
                     RelativeSizeAxes = Axes.Both,
@@ -80,12 +80,6 @@ namespace osu.Game.Overlays.Chat
             Channel.PendingMessageResolved += pendingMessageResolved;
         }
 
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-            scrollToEnd();
-        }
-
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
@@ -113,8 +107,6 @@ namespace osu.Game.Overlays.Chat
                 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 @@ namespace osu.Game.Overlays.Chat
                 }
             }
 
-            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 @@ namespace osu.Game.Overlays.Chat
 
         private IEnumerable<ChatLine> chatLines => ChatLineFlow.Children.OfType<ChatLine>();
 
-        private void scrollToEnd() => ScheduleAfterChildren(() => scroll.ScrollToEnd());
-
         public class DaySeparator : Container
         {
             public float TextSize
@@ -243,5 +235,32 @@ namespace osu.Game.Overlays.Chat
                 };
             }
         }
+
+        /// <summary>
+        /// An <see cref="OsuScrollContainer"/> with functionality to automatically scrolls whenever the maximum scrollable distance increases.
+        /// </summary>
+        private class ChannelScrollContainer : OsuScrollContainer
+        {
+            private const float auto_scroll_leniency = 10f;
+
+            private float? lastExtent;
+
+            /// <summary>
+            /// Whether this should automatically scroll to end on the next call to <see cref="UpdateAfterChildren"/>.
+            /// </summary>
+            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;
+            }
+        }
     }
 }