diff --git a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs index 7d2ac90939..ebd5e12acb 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneStandAloneChatDisplay.cs @@ -73,6 +73,11 @@ public void SetUp() => Schedule(() => messageIdSequence = 0; channelManager.CurrentChannel.Value = testChannel = new Channel(); + reinitialiseDrawableDisplay(); + }); + + private void reinitialiseDrawableDisplay() + { Children = new[] { chatDisplay = new TestStandAloneChatDisplay @@ -92,13 +97,14 @@ public void SetUp() => Schedule(() => Channel = { Value = testChannel }, } }; - }); + } [Test] public void TestSystemMessageOrdering() { var standardMessage = new Message(messageIdSequence++) { + Timestamp = DateTimeOffset.Now, Sender = admin, Content = "I am a wang!" }; @@ -106,14 +112,45 @@ public void TestSystemMessageOrdering() var infoMessage1 = new InfoMessage($"the system is calling {messageIdSequence++}"); var infoMessage2 = new InfoMessage($"the system is calling {messageIdSequence++}"); + var standardMessage2 = new Message(messageIdSequence++) + { + Timestamp = DateTimeOffset.Now, + Sender = admin, + Content = "I am a wang!" + }; + AddStep("message from admin", () => testChannel.AddNewMessages(standardMessage)); AddStep("message from system", () => testChannel.AddNewMessages(infoMessage1)); AddStep("message from system", () => testChannel.AddNewMessages(infoMessage2)); + AddStep("message from admin", () => testChannel.AddNewMessages(standardMessage2)); - AddAssert("message order is correct", () => testChannel.Messages.Count == 3 - && testChannel.Messages[0] == standardMessage - && testChannel.Messages[1] == infoMessage1 - && testChannel.Messages[2] == infoMessage2); + AddAssert("count is correct", () => testChannel.Messages.Count, () => Is.EqualTo(4)); + + AddAssert("message order is correct", () => testChannel.Messages, () => Is.EqualTo(new[] + { + standardMessage, + infoMessage1, + infoMessage2, + standardMessage2 + })); + + AddAssert("displayed order is correct", () => chatDisplay.DrawableChannel.ChildrenOfType().Select(c => c.Message), () => Is.EqualTo(new[] + { + standardMessage, + infoMessage1, + infoMessage2, + standardMessage2 + })); + + AddStep("reinit drawable channel", reinitialiseDrawableDisplay); + + AddAssert("displayed order is still correct", () => chatDisplay.DrawableChannel.ChildrenOfType().Select(c => c.Message), () => Is.EqualTo(new[] + { + standardMessage, + infoMessage1, + infoMessage2, + standardMessage2 + })); } [Test] diff --git a/osu.Game/Online/Chat/InfoMessage.cs b/osu.Game/Online/Chat/InfoMessage.cs index d98c67de34..2ade99dcb2 100644 --- a/osu.Game/Online/Chat/InfoMessage.cs +++ b/osu.Game/Online/Chat/InfoMessage.cs @@ -1,9 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - -using System; using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Online.Chat @@ -13,7 +10,6 @@ public class InfoMessage : LocalMessage public InfoMessage(string message) : base(null) { - Timestamp = DateTimeOffset.Now; Content = message; Sender = APIUser.SYSTEM_USER; diff --git a/osu.Game/Online/Chat/LocalEchoMessage.cs b/osu.Game/Online/Chat/LocalEchoMessage.cs index b226fe6cad..8a39515575 100644 --- a/osu.Game/Online/Chat/LocalEchoMessage.cs +++ b/osu.Game/Online/Chat/LocalEchoMessage.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - namespace osu.Game.Online.Chat { public class LocalEchoMessage : LocalMessage diff --git a/osu.Game/Online/Chat/LocalMessage.cs b/osu.Game/Online/Chat/LocalMessage.cs index 5736f5cabf..57caca2287 100644 --- a/osu.Game/Online/Chat/LocalMessage.cs +++ b/osu.Game/Online/Chat/LocalMessage.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable +using System; namespace osu.Game.Online.Chat { @@ -13,6 +13,7 @@ public class LocalMessage : Message protected LocalMessage(long? id) : base(id) { + Timestamp = DateTimeOffset.Now; } } } diff --git a/osu.Game/Online/Chat/Message.cs b/osu.Game/Online/Chat/Message.cs index 9f6f9c8d6b..8ea3ca0fc7 100644 --- a/osu.Game/Online/Chat/Message.cs +++ b/osu.Game/Online/Chat/Message.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Threading; using Newtonsoft.Json; using osu.Game.Online.API.Requests.Responses; @@ -59,19 +60,28 @@ public Message() /// The s' and s are according to public List Links; + private static long constructionOrderStatic; + private readonly long constructionOrder; + public Message(long? id) { Id = id; + + constructionOrder = Interlocked.Increment(ref constructionOrderStatic); } public int CompareTo(Message other) { - if (!Id.HasValue) - return other.Id.HasValue ? 1 : Timestamp.CompareTo(other.Timestamp); - if (!other.Id.HasValue) - return -1; + if (Id.HasValue && other.Id.HasValue) + return Id.Value.CompareTo(other.Id.Value); - return Id.Value.CompareTo(other.Id.Value); + int timestampComparison = Timestamp.CompareTo(other.Timestamp); + + if (timestampComparison != 0) + return timestampComparison; + + // Timestamp might not be accurate enough to make a stable sorting decision. + return constructionOrder.CompareTo(other.constructionOrder); } public virtual bool Equals(Message other) @@ -85,6 +95,6 @@ public virtual bool Equals(Message other) // ReSharper disable once ImpureMethodCallOnReadonlyValueField public override int GetHashCode() => Id.GetHashCode(); - public override string ToString() => $"[{ChannelId}] ({Id}) {Sender}: {Content}"; + public override string ToString() => $"({(Id?.ToString() ?? "null")}) {Timestamp} {Sender}: {Content}"; } }