From d66fa093205320b3a09a20ded2632b68dbf3fe21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 6 Dec 2023 18:21:44 +0100 Subject: [PATCH] Simplify `UserStatus` to be an enumeration type There were absolutely no gains from having it be a reference type / class, only complications, especially when coming from the serialisation angle. --- osu.Desktop/DiscordRichPresence.cs | 6 +-- .../Online/TestSceneUserClickableAvatar.cs | 2 +- .../Visual/Online/TestSceneUserPanel.cs | 18 ++++---- osu.Game/Online/API/APIAccess.cs | 2 +- .../Online/API/Requests/Responses/APIUser.cs | 2 +- osu.Game/Overlays/Login/LoginPanel.cs | 6 +-- osu.Game/Users/ExtendedUserPanel.cs | 17 +++---- osu.Game/Users/UserStatus.cs | 46 +++++++++++-------- 8 files changed, 53 insertions(+), 46 deletions(-) diff --git a/osu.Desktop/DiscordRichPresence.cs b/osu.Desktop/DiscordRichPresence.cs index c66725e3e3..f990fd55fc 100644 --- a/osu.Desktop/DiscordRichPresence.cs +++ b/osu.Desktop/DiscordRichPresence.cs @@ -33,7 +33,7 @@ namespace osu.Desktop [Resolved] private IAPIProvider api { get; set; } = null!; - private readonly IBindable status = new Bindable(); + private readonly IBindable status = new Bindable(); private readonly IBindable activity = new Bindable(); private readonly Bindable privacyMode = new Bindable(); @@ -86,13 +86,13 @@ namespace osu.Desktop if (!client.IsInitialized) return; - if (status.Value is UserStatusOffline || privacyMode.Value == DiscordRichPresenceMode.Off) + if (status.Value == UserStatus.Offline || privacyMode.Value == DiscordRichPresenceMode.Off) { client.ClearPresence(); return; } - if (status.Value is UserStatusOnline && activity.Value != null) + if (status.Value == UserStatus.Online && activity.Value != null) { bool hideIdentifiableInformation = privacyMode.Value == DiscordRichPresenceMode.Limited; presence.State = truncate(activity.Value.GetStatus(hideIdentifiableInformation)); diff --git a/osu.Game.Tests/Visual/Online/TestSceneUserClickableAvatar.cs b/osu.Game.Tests/Visual/Online/TestSceneUserClickableAvatar.cs index 9edaa841b2..4539eae25f 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneUserClickableAvatar.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneUserClickableAvatar.cs @@ -64,7 +64,7 @@ namespace osu.Game.Tests.Visual.Online Colour = color ?? "000000", Status = { - Value = new UserStatusOnline() + Value = UserStatus.Online }, }; diff --git a/osu.Game.Tests/Visual/Online/TestSceneUserPanel.cs b/osu.Game.Tests/Visual/Online/TestSceneUserPanel.cs index c61b572d8c..b3b8fd78d3 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneUserPanel.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneUserPanel.cs @@ -24,7 +24,7 @@ namespace osu.Game.Tests.Visual.Online public partial class TestSceneUserPanel : OsuTestScene { private readonly Bindable activity = new Bindable(); - private readonly Bindable status = new Bindable(); + private readonly Bindable status = new Bindable(); private UserGridPanel boundPanel1; private TestUserListPanel boundPanel2; @@ -66,7 +66,7 @@ namespace osu.Game.Tests.Visual.Online Id = 3103765, CountryCode = CountryCode.JP, CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c6.jpg", - Status = { Value = new UserStatusOnline() } + Status = { Value = UserStatus.Online } }) { Width = 300 }, boundPanel1 = new UserGridPanel(new APIUser { @@ -99,16 +99,16 @@ namespace osu.Game.Tests.Visual.Online [Test] public void TestUserStatus() { - AddStep("online", () => status.Value = new UserStatusOnline()); - AddStep("do not disturb", () => status.Value = new UserStatusDoNotDisturb()); - AddStep("offline", () => status.Value = new UserStatusOffline()); + AddStep("online", () => status.Value = UserStatus.Online); + AddStep("do not disturb", () => status.Value = UserStatus.DoNotDisturb); + AddStep("offline", () => status.Value = UserStatus.Offline); AddStep("null status", () => status.Value = null); } [Test] public void TestUserActivity() { - AddStep("set online status", () => status.Value = new UserStatusOnline()); + AddStep("set online status", () => status.Value = UserStatus.Online); AddStep("idle", () => activity.Value = null); AddStep("watching replay", () => activity.Value = new UserActivity.WatchingReplay(createScore(@"nats"))); @@ -127,12 +127,12 @@ namespace osu.Game.Tests.Visual.Online public void TestUserActivityChange() { AddAssert("visit message is visible", () => boundPanel2.LastVisitMessage.IsPresent); - AddStep("set online status", () => status.Value = new UserStatusOnline()); + AddStep("set online status", () => status.Value = UserStatus.Online); AddAssert("visit message is not visible", () => !boundPanel2.LastVisitMessage.IsPresent); AddStep("set choosing activity", () => activity.Value = new UserActivity.ChoosingBeatmap()); - AddStep("set offline status", () => status.Value = new UserStatusOffline()); + AddStep("set offline status", () => status.Value = UserStatus.Offline); AddAssert("visit message is visible", () => boundPanel2.LastVisitMessage.IsPresent); - AddStep("set online status", () => status.Value = new UserStatusOnline()); + AddStep("set online status", () => status.Value = UserStatus.Online); AddAssert("visit message is not visible", () => !boundPanel2.LastVisitMessage.IsPresent); } diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index 4f586c8fff..21107d61fc 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -247,7 +247,7 @@ namespace osu.Game.Online.API userReq.Success += user => { // todo: save/pull from settings - user.Status.Value = new UserStatusOnline(); + user.Status.Value = UserStatus.Online; setLocalUser(user); diff --git a/osu.Game/Online/API/Requests/Responses/APIUser.cs b/osu.Game/Online/API/Requests/Responses/APIUser.cs index 2ee66453cf..56eec19fa1 100644 --- a/osu.Game/Online/API/Requests/Responses/APIUser.cs +++ b/osu.Game/Online/API/Requests/Responses/APIUser.cs @@ -43,7 +43,7 @@ namespace osu.Game.Online.API.Requests.Responses set => countryCodeString = value.ToString(); } - public readonly Bindable Status = new Bindable(); + public readonly Bindable Status = new Bindable(); public readonly Bindable Activity = new Bindable(); diff --git a/osu.Game/Overlays/Login/LoginPanel.cs b/osu.Game/Overlays/Login/LoginPanel.cs index 71ecf2e75a..19af95459f 100644 --- a/osu.Game/Overlays/Login/LoginPanel.cs +++ b/osu.Game/Overlays/Login/LoginPanel.cs @@ -148,17 +148,17 @@ namespace osu.Game.Overlays.Login switch (action.NewValue) { case UserAction.Online: - api.LocalUser.Value.Status.Value = new UserStatusOnline(); + api.LocalUser.Value.Status.Value = UserStatus.Online; dropdown.StatusColour = colours.Green; break; case UserAction.DoNotDisturb: - api.LocalUser.Value.Status.Value = new UserStatusDoNotDisturb(); + api.LocalUser.Value.Status.Value = UserStatus.DoNotDisturb; dropdown.StatusColour = colours.Red; break; case UserAction.AppearOffline: - api.LocalUser.Value.Status.Value = new UserStatusOffline(); + api.LocalUser.Value.Status.Value = UserStatus.Offline; dropdown.StatusColour = colours.Gray7; break; diff --git a/osu.Game/Users/ExtendedUserPanel.cs b/osu.Game/Users/ExtendedUserPanel.cs index 3c1b68f9ef..18fe852556 100644 --- a/osu.Game/Users/ExtendedUserPanel.cs +++ b/osu.Game/Users/ExtendedUserPanel.cs @@ -6,6 +6,7 @@ using osuTK; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Graphics; @@ -18,7 +19,7 @@ namespace osu.Game.Users { public abstract partial class ExtendedUserPanel : UserPanel { - public readonly Bindable Status = new Bindable(); + public readonly Bindable Status = new Bindable(); public readonly IBindable Activity = new Bindable(); @@ -97,14 +98,14 @@ namespace osu.Game.Users return statusContainer; } - private void displayStatus(UserStatus status, UserActivity activity = null) + private void displayStatus(UserStatus? status, UserActivity activity = null) { if (status != null) { - LastVisitMessage.FadeTo(status is UserStatusOffline && User.LastVisit.HasValue ? 1 : 0); + LastVisitMessage.FadeTo(status == UserStatus.Offline && User.LastVisit.HasValue ? 1 : 0); // Set status message based on activity (if we have one) and status is not offline - if (activity != null && !(status is UserStatusOffline)) + if (activity != null && status != UserStatus.Offline) { statusMessage.Text = activity.GetStatus(); statusIcon.FadeColour(activity.GetAppropriateColour(Colours), 500, Easing.OutQuint); @@ -112,8 +113,8 @@ namespace osu.Game.Users } // Otherwise use only status - statusMessage.Text = status.Message; - statusIcon.FadeColour(status.GetAppropriateColour(Colours), 500, Easing.OutQuint); + statusMessage.Text = status.GetLocalisableDescription(); + statusIcon.FadeColour(status.Value.GetAppropriateColour(Colours), 500, Easing.OutQuint); return; } @@ -121,11 +122,11 @@ namespace osu.Game.Users // Fallback to web status if local one is null if (User.IsOnline) { - Status.Value = new UserStatusOnline(); + Status.Value = UserStatus.Online; return; } - Status.Value = new UserStatusOffline(); + Status.Value = UserStatus.Offline; } protected override bool OnHover(HoverEvent e) diff --git a/osu.Game/Users/UserStatus.cs b/osu.Game/Users/UserStatus.cs index ffd86b78c7..cd25add4d1 100644 --- a/osu.Game/Users/UserStatus.cs +++ b/osu.Game/Users/UserStatus.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; +using System.ComponentModel; using osu.Framework.Localisation; using osuTK.Graphics; using osu.Game.Graphics; @@ -8,32 +10,36 @@ using osu.Game.Resources.Localisation.Web; namespace osu.Game.Users { - public abstract class UserStatus + public enum UserStatus { - public abstract LocalisableString Message { get; } - public abstract Color4 GetAppropriateColour(OsuColour colours); + [LocalisableDescription(typeof(UsersStrings), nameof(UsersStrings.StatusOffline))] + Offline, + + [Description("Do not disturb")] + DoNotDisturb, + + [LocalisableDescription(typeof(UsersStrings), nameof(UsersStrings.StatusOnline))] + Online, } - public class UserStatusOnline : UserStatus + public static class UserStatusExtensions { - public override LocalisableString Message => UsersStrings.StatusOnline; - public override Color4 GetAppropriateColour(OsuColour colours) => colours.GreenLight; - } + public static Color4 GetAppropriateColour(this UserStatus userStatus, OsuColour colours) + { + switch (userStatus) + { + case UserStatus.Offline: + return Color4.Black; - public abstract class UserStatusBusy : UserStatusOnline - { - public override Color4 GetAppropriateColour(OsuColour colours) => colours.YellowDark; - } + case UserStatus.DoNotDisturb: + return colours.RedDark; - public class UserStatusOffline : UserStatus - { - public override LocalisableString Message => UsersStrings.StatusOffline; - public override Color4 GetAppropriateColour(OsuColour colours) => Color4.Black; - } + case UserStatus.Online: + return colours.GreenDark; - public class UserStatusDoNotDisturb : UserStatus - { - public override LocalisableString Message => "Do not disturb"; - public override Color4 GetAppropriateColour(OsuColour colours) => colours.RedDark; + default: + throw new ArgumentOutOfRangeException(nameof(userStatus), userStatus, "Unsupported user status"); + } + } } }