Merge pull request #21349 from peppy/fix-missing-day-separator

Fix chat day separator not being added on pending message resolution

osu.Game.Tests/Visual/Online/TestSceneDrawableChannel.cs

@@ -0,0 +1,87 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Graphics;
+using osu.Framework.Testing;
+using osu.Game.Online.API.Requests.Responses;
+using osu.Game.Online.Chat;
+using osu.Game.Overlays.Chat;
+
+namespace osu.Game.Tests.Visual.Online
+{
+    [TestFixture]
+    public class TestSceneDrawableChannel : OsuTestScene
+    {
+        private Channel channel = null!;
+        private DrawableChannel drawableChannel = null!;
+
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("create channel", () => channel = new Channel
+            {
+                Id = 1,
+                Name = "Test channel"
+            });
+            AddStep("create drawable channel", () => Child = drawableChannel = new DrawableChannel(channel)
+            {
+                RelativeSizeAxes = Axes.Both
+            });
+        }
+
+        [Test]
+        public void TestDaySeparators()
+        {
+            var localUser = new APIUser
+            {
+                Id = 3,
+                Username = "LocalUser"
+            };
+            string uuid = Guid.NewGuid().ToString();
+            AddStep("add local echo message", () => channel.AddLocalEcho(new LocalEchoMessage
+            {
+                Sender = localUser,
+                Content = "Hi there all!",
+                Timestamp = new DateTimeOffset(2022, 11, 21, 20, 11, 13, TimeSpan.Zero),
+                Uuid = uuid
+            }));
+            AddUntilStep("one day separator present", () => drawableChannel.ChildrenOfType<DaySeparator>().Count() == 1);
+
+            AddStep("add two prior messages to channel", () => channel.AddNewMessages(
+                new Message(1)
+                {
+                    Sender = new APIUser
+                    {
+                        Id = 1,
+                        Username = "TestUser"
+                    },
+                    Content = "This is a message",
+                    Timestamp = new DateTimeOffset(2021, 10, 10, 13, 33, 23, TimeSpan.Zero),
+                },
+                new Message(2)
+                {
+                    Sender = new APIUser
+                    {
+                        Id = 2,
+                        Username = "TestUser2"
+                    },
+                    Content = "This is another message",
+                    Timestamp = new DateTimeOffset(2021, 10, 11, 13, 33, 23, TimeSpan.Zero)
+                }));
+            AddUntilStep("three day separators present", () => drawableChannel.ChildrenOfType<DaySeparator>().Count() == 3);
+
+            AddStep("resolve pending message", () => channel.ReplaceMessage(channel.Messages.OfType<LocalEchoMessage>().Single(), new Message(3)
+            {
+                Sender = localUser,
+                Content = "Hi there all!",
+                Timestamp = new DateTimeOffset(2022, 11, 22, 20, 11, 16, TimeSpan.Zero),
+                Uuid = uuid
+            }));
+            AddUntilStep("three day separators present", () => drawableChannel.ChildrenOfType<DaySeparator>().Count() == 3);
+            AddAssert("last day separator is from correct day", () => drawableChannel.ChildrenOfType<DaySeparator>().Last().Date.Date == new DateTime(2022, 11, 22));
+        }
+    }
+}

osu.Game/Online/Chat/StandAloneChatDisplay.cs

@@ -177,8 +177,8 @@ namespace osu.Game.Online.Chat
             protected override float Spacing => 5;
             protected override float DateAlign => 125;
 
-            public StandAloneDaySeparator(DateTimeOffset time)
+            public StandAloneDaySeparator(DateTimeOffset date)
-                : base(time)
+                : base(date)
             {
             }
 

osu.Game/Overlays/Chat/DaySeparator.cs

@@ -22,14 +22,14 @@ namespace osu.Game.Overlays.Chat
 
         protected virtual float Spacing => 15;
 
-        private readonly DateTimeOffset time;
+        public readonly DateTimeOffset Date;
 
         [Resolved(CanBeNull = true)]
         private OverlayColourProvider? colourProvider { get; set; }
 
-        public DaySeparator(DateTimeOffset time)
+        public DaySeparator(DateTimeOffset date)
         {
-            this.time = time;
+            Date = date;
             Height = 40;
         }
 
@@ -80,7 +80,7 @@ namespace osu.Game.Overlays.Chat
                                     {
                                         Anchor = Anchor.CentreRight,
                                         Origin = Anchor.CentreRight,
-                                        Text = time.ToLocalTime().ToLocalisableString(@"dd MMMM yyyy").ToUpper(),
+                                        Text = Date.ToLocalTime().ToLocalisableString(@"dd MMMM yyyy").ToUpper(),
                                         Font = OsuFont.Torus.With(size: TextSize, weight: FontWeight.SemiBold),
                                         Colour = colourProvider?.Content1 ?? Colour4.White,
                                     },

osu.Game/Overlays/Chat/DrawableChannel.cs

@@ -134,35 +134,22 @@ namespace osu.Game.Overlays.Chat
 
             foreach (var message in displayMessages)
             {
-                if (lastMessage == null || lastMessage.Timestamp.ToLocalTime().Date != message.Timestamp.ToLocalTime().Date)
+                addDaySeparatorIfRequired(lastMessage, message);
-                    ChatLineFlow.Add(CreateDaySeparator(message.Timestamp));
 
                 ChatLineFlow.Add(CreateChatLine(message));
                 lastMessage = message;
             }
 
             var staleMessages = chatLines.Where(c => c.LifetimeEnd == double.MaxValue).ToArray();
+
             int count = staleMessages.Length - Channel.MAX_HISTORY;
 
             if (count > 0)
             {
-                void expireAndAdjustScroll(Drawable d)
-                {
-                    scroll.OffsetScrollPosition(-d.DrawHeight);
-                    d.Expire();
-                }
-
                 for (int i = 0; i < count; i++)
                     expireAndAdjustScroll(staleMessages[i]);
 
-                // remove all adjacent day separators after stale message removal
+                removeAdjacentDaySeparators();
-                for (int i = 0; i < ChatLineFlow.Count - 1; i++)
-                {
-                    if (!(ChatLineFlow[i] is DaySeparator)) break;
-                    if (!(ChatLineFlow[i + 1] is DaySeparator)) break;
-
-                    expireAndAdjustScroll(ChatLineFlow[i]);
-                }
             }
 
             // due to the scroll adjusts from old messages removal above, a scroll-to-end must be enforced,
@@ -183,10 +170,46 @@ namespace osu.Game.Overlays.Chat
 
                 ChatLineFlow.Remove(found, false);
                 found.Message = updated;
+
+                addDaySeparatorIfRequired(chatLines.LastOrDefault()?.Message, updated);
                 ChatLineFlow.Add(found);
             }
         });
 
+        private void addDaySeparatorIfRequired(Message lastMessage, Message message)
+        {
+            if (lastMessage == null || lastMessage.Timestamp.ToLocalTime().Date != message.Timestamp.ToLocalTime().Date)
+            {
+                // A day separator is displayed even if no messages are in the channel.
+                // If there are no messages after it, the simplest way to ensure it is fresh is to remove it
+                // and add a new one instead.
+                if (ChatLineFlow.LastOrDefault() is DaySeparator ds)
+                    ChatLineFlow.Remove(ds, true);
+
+                ChatLineFlow.Add(CreateDaySeparator(message.Timestamp));
+
+                removeAdjacentDaySeparators();
+            }
+        }
+
+        private void removeAdjacentDaySeparators()
+        {
+            // remove all adjacent day separators after stale message removal
+            for (int i = 0; i < ChatLineFlow.Count - 1; i++)
+            {
+                if (!(ChatLineFlow[i] is DaySeparator)) break;
+                if (!(ChatLineFlow[i + 1] is DaySeparator)) break;
+
+                expireAndAdjustScroll(ChatLineFlow[i]);
+            }
+        }
+
+        private void expireAndAdjustScroll(Drawable d)
+        {
+            scroll.OffsetScrollPosition(-d.DrawHeight);
+            d.Expire();
+        }
+
         private void messageRemoved(Message removed) => Schedule(() =>
         {
             chatLines.FirstOrDefault(c => c.Message == removed)?.FadeColour(Color4.Red, 400).FadeOut(600).Expire();