From b06e70e546ca0a2d82ecfbce85f977a2a200fb28 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 29 Oct 2019 15:27:08 +0900 Subject: [PATCH 1/2] Add failing test showing issue with day separator logic --- .../Online/TestSceneStandAloneChatDisplay.cs | 20 ++++++++++++++++++- osu.Game/Overlays/Chat/DrawableChannel.cs | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index d973799405..01400bf1d9 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -9,6 +9,7 @@ using osuTK; using System; using System.Linq; using osu.Framework.Graphics.Containers; +using osu.Game.Graphics.Containers; using osu.Game.Overlays.Chat; namespace osu.Game.Tests.Visual.Online @@ -137,6 +138,17 @@ namespace osu.Game.Tests.Visual.Online }); }, Channel.MAX_HISTORY / messages_per_call + 5); + AddAssert("Ensure no adjacent day separators", () => + { + var indices = chatDisplay.FillFlow.OfType().Select(ds => chatDisplay.FillFlow.IndexOf(ds)); + + foreach (var i in indices) + if (i < chatDisplay.FillFlow.Count && chatDisplay.FillFlow[i + 1] is DrawableChannel.DaySeparator) + return false; + + return true; + }); + AddUntilStep("ensure still scrolled to bottom", () => chatDisplay.ScrolledToBottom); } @@ -147,7 +159,13 @@ namespace osu.Game.Tests.Visual.Online { } - public bool ScrolledToBottom => ((ScrollContainer)((Container)InternalChildren.OfType().First().Child).Child).IsScrolledToEnd(1); + protected DrawableChannel DrawableChannel => InternalChildren.OfType().First(); + + protected OsuScrollContainer ScrollContainer => (OsuScrollContainer)((Container)DrawableChannel.Child).Child; + + public FillFlowContainer FillFlow => (FillFlowContainer)ScrollContainer.Child; + + public bool ScrolledToBottom => ScrollContainer.IsScrolledToEnd(1); } } } diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 9301daa9ff..636fafb5f2 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -142,7 +142,7 @@ namespace osu.Game.Overlays.Chat private void scrollToEnd() => ScheduleAfterChildren(() => scroll.ScrollToEnd()); - protected class DaySeparator : Container + public class DaySeparator : Container { public float TextSize { From 54befb6f8fdbdfe36e9f299b3df54782ff08cf7f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 29 Oct 2019 15:45:41 +0900 Subject: [PATCH 2/2] Remove adjacent day separators --- osu.Game/Overlays/Chat/DrawableChannel.cs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/Chat/DrawableChannel.cs b/osu.Game/Overlays/Chat/DrawableChannel.cs index 636fafb5f2..427bd8dcde 100644 --- a/osu.Game/Overlays/Chat/DrawableChannel.cs +++ b/osu.Game/Overlays/Chat/DrawableChannel.cs @@ -108,11 +108,25 @@ namespace osu.Game.Overlays.Chat var staleMessages = chatLines.Where(c => c.LifetimeEnd == double.MaxValue).ToArray(); int count = staleMessages.Length - Channel.MAX_HISTORY; - for (int i = 0; i < count; i++) + if (count > 0) { - var d = staleMessages[i]; - scroll.OffsetScrollPosition(-d.DrawHeight); - d.Expire(); + 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 + 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]); + } } if (shouldScrollToEnd)