From 3b600f0a7b2dda8fa17ba85ee1563526bad44ece Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 10:45:19 +0300 Subject: [PATCH 01/40] Target net5.0 instead of netcoreapp3 --- global.json | 2 +- osu.Desktop/osu.Desktop.csproj | 2 +- osu.Game.Benchmarks/osu.Game.Benchmarks.csproj | 2 +- .../osu.Game.Rulesets.Catch.Tests.csproj | 2 +- .../osu.Game.Rulesets.Mania.Tests.csproj | 2 +- osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj | 2 +- .../osu.Game.Rulesets.Taiko.Tests.csproj | 2 +- osu.Game.Tests/osu.Game.Tests.csproj | 2 +- osu.Game.Tournament.Tests/osu.Game.Tournament.Tests.csproj | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/global.json b/global.json index 10b61047ac..2cb4c02970 100644 --- a/global.json +++ b/global.json @@ -2,7 +2,7 @@ "sdk": { "allowPrerelease": false, "rollForward": "minor", - "version": "3.1.100" + "version": "5.0.100" }, "msbuild-sdks": { "Microsoft.Build.Traversal": "2.2.3" diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 62e8f7c518..7f5154f456 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -1,6 +1,6 @@  - netcoreapp3.1 + net5.0 WinExe true A free-to-win rhythm game. Rhythm is just a *click* away! diff --git a/osu.Game.Benchmarks/osu.Game.Benchmarks.csproj b/osu.Game.Benchmarks/osu.Game.Benchmarks.csproj index ff26f4afaa..7805bfcefc 100644 --- a/osu.Game.Benchmarks/osu.Game.Benchmarks.csproj +++ b/osu.Game.Benchmarks/osu.Game.Benchmarks.csproj @@ -1,7 +1,7 @@ - netcoreapp3.1 + net5.0 Exe false diff --git a/osu.Game.Rulesets.Catch.Tests/osu.Game.Rulesets.Catch.Tests.csproj b/osu.Game.Rulesets.Catch.Tests/osu.Game.Rulesets.Catch.Tests.csproj index dfe3bf8af4..a51b9830be 100644 --- a/osu.Game.Rulesets.Catch.Tests/osu.Game.Rulesets.Catch.Tests.csproj +++ b/osu.Game.Rulesets.Catch.Tests/osu.Game.Rulesets.Catch.Tests.csproj @@ -9,7 +9,7 @@ WinExe - netcoreapp3.1 + net5.0 diff --git a/osu.Game.Rulesets.Mania.Tests/osu.Game.Rulesets.Mania.Tests.csproj b/osu.Game.Rulesets.Mania.Tests/osu.Game.Rulesets.Mania.Tests.csproj index 892f27d27f..d314671bce 100644 --- a/osu.Game.Rulesets.Mania.Tests/osu.Game.Rulesets.Mania.Tests.csproj +++ b/osu.Game.Rulesets.Mania.Tests/osu.Game.Rulesets.Mania.Tests.csproj @@ -9,7 +9,7 @@ WinExe - netcoreapp3.1 + net5.0 diff --git a/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj b/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj index 3639c3616f..b0799bd3f5 100644 --- a/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj +++ b/osu.Game.Rulesets.Osu.Tests/osu.Game.Rulesets.Osu.Tests.csproj @@ -9,7 +9,7 @@ WinExe - netcoreapp3.1 + net5.0 diff --git a/osu.Game.Rulesets.Taiko.Tests/osu.Game.Rulesets.Taiko.Tests.csproj b/osu.Game.Rulesets.Taiko.Tests/osu.Game.Rulesets.Taiko.Tests.csproj index b59f3a4344..d3dbba4bfc 100644 --- a/osu.Game.Rulesets.Taiko.Tests/osu.Game.Rulesets.Taiko.Tests.csproj +++ b/osu.Game.Rulesets.Taiko.Tests/osu.Game.Rulesets.Taiko.Tests.csproj @@ -9,7 +9,7 @@ WinExe - netcoreapp3.1 + net5.0 diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index c692bcd5e4..34de54411b 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -10,7 +10,7 @@ WinExe - netcoreapp3.1 + net5.0 diff --git a/osu.Game.Tournament.Tests/osu.Game.Tournament.Tests.csproj b/osu.Game.Tournament.Tests/osu.Game.Tournament.Tests.csproj index 5d55196dcf..d820794980 100644 --- a/osu.Game.Tournament.Tests/osu.Game.Tournament.Tests.csproj +++ b/osu.Game.Tournament.Tests/osu.Game.Tournament.Tests.csproj @@ -11,7 +11,7 @@ WinExe - netcoreapp3.1 + net5.0 From ca0e1c8cee1ffd03b2bb5fe773741012b90bb654 Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 10:47:57 +0300 Subject: [PATCH 02/40] Update NuGet packages --- osu.Desktop/osu.Desktop.csproj | 4 ++-- osu.Game.Tournament/osu.Game.Tournament.csproj | 2 +- osu.Game/osu.Game.csproj | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 7f5154f456..2052c4bc25 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -24,11 +24,11 @@ - + - + diff --git a/osu.Game.Tournament/osu.Game.Tournament.csproj b/osu.Game.Tournament/osu.Game.Tournament.csproj index 9cce40c9d3..b049542bb0 100644 --- a/osu.Game.Tournament/osu.Game.Tournament.csproj +++ b/osu.Game.Tournament/osu.Game.Tournament.csproj @@ -9,6 +9,6 @@ - + \ No newline at end of file diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 54f3fcede6..1670bf5b11 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -21,8 +21,8 @@ - - + + @@ -31,6 +31,6 @@ - + From f562a7ea0df68baee20551108bb4bd9fd13d5747 Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 11:52:17 +0300 Subject: [PATCH 03/40] Fix FileNotFoundException on startup --- osu.Desktop/osu.Desktop.csproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 2052c4bc25..53b9cdcf92 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -24,6 +24,8 @@ + + From 044622036cbc94adae124f68ec5fbff18befb2b4 Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 11:53:17 +0300 Subject: [PATCH 04/40] Disable CA1416 --- osu.Desktop/OsuGameDesktop.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index 0feab9a717..e2a06d7877 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -58,8 +58,10 @@ namespace osu.Desktop try { +#pragma warning disable CA1416 // Validate platform compatibility using (RegistryKey key = Registry.ClassesRoot.OpenSubKey("osu")) stableInstallPath = key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty).ToString()?.Split('"')[1].Replace("osu!.exe", ""); +#pragma warning restore CA1416 // Validate platform compatibility if (checkExists(stableInstallPath)) return stableInstallPath; From 1a676ef0d82c4d4cfd0d7fbac2338da3a763272f Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 12:06:08 +0300 Subject: [PATCH 05/40] Resolve CA1416 properly using new API --- osu.Desktop/OsuGameDesktop.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index e2a06d7877..f9c932b260 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -5,6 +5,7 @@ using System; using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.Versioning; using System.Threading.Tasks; using Microsoft.Win32; using osu.Desktop.Overlays; @@ -56,19 +57,12 @@ namespace osu.Desktop string stableInstallPath; - try + if (OperatingSystem.IsWindows()) { -#pragma warning disable CA1416 // Validate platform compatibility - using (RegistryKey key = Registry.ClassesRoot.OpenSubKey("osu")) - stableInstallPath = key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty).ToString()?.Split('"')[1].Replace("osu!.exe", ""); -#pragma warning restore CA1416 // Validate platform compatibility - + stableInstallPath = getStableInstallPathFromRegistry(); if (checkExists(stableInstallPath)) return stableInstallPath; } - catch - { - } stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @"osu!"); if (checkExists(stableInstallPath)) @@ -81,6 +75,13 @@ namespace osu.Desktop return null; } + [SupportedOSPlatform("windows")] + private string getStableInstallPathFromRegistry() + { + using (RegistryKey key = Registry.ClassesRoot.OpenSubKey("osu")) + return key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty).ToString()?.Split('"')[1].Replace("osu!.exe", ""); + } + protected override UpdateManager CreateUpdateManager() { switch (RuntimeInfo.OS) From 743541649706d9b9165197421860dfd0b3cf264d Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Fri, 20 Nov 2020 14:13:16 +0300 Subject: [PATCH 06/40] Workaround FileNotFoundException in a test projects --- osu.Desktop/osu.Desktop.csproj | 2 -- osu.Game/osu.Game.csproj | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Desktop/osu.Desktop.csproj b/osu.Desktop/osu.Desktop.csproj index 53b9cdcf92..2052c4bc25 100644 --- a/osu.Desktop/osu.Desktop.csproj +++ b/osu.Desktop/osu.Desktop.csproj @@ -24,8 +24,6 @@ - - diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 1670bf5b11..1c6139b519 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -18,6 +18,8 @@ + + From 1feda1152da0b15835af3ce83bed31faf5fbed65 Mon Sep 17 00:00:00 2001 From: Roman Kapustin Date: Sat, 21 Nov 2020 02:06:20 +0300 Subject: [PATCH 07/40] Fix InspectCode warnings --- osu.Desktop/OsuGameDesktop.cs | 2 +- osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs | 5 +++++ osu.Game.Tournament/IPC/FileBasedIPC.cs | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index f9c932b260..dbbf6d048b 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -79,7 +79,7 @@ namespace osu.Desktop private string getStableInstallPathFromRegistry() { using (RegistryKey key = Registry.ClassesRoot.OpenSubKey("osu")) - return key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty).ToString()?.Split('"')[1].Replace("osu!.exe", ""); + return key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty)?.ToString()?.Split('"')[1].Replace("osu!.exe", ""); } protected override UpdateManager CreateUpdateManager() diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs index 35473ee76c..58992366ff 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; +using System.Diagnostics; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -73,6 +74,8 @@ namespace osu.Game.Tests.Visual.Gameplay switch (args.Action) { case NotifyCollectionChangedAction.Add: + Debug.Assert(args.NewItems != null, "args.NewItems != null"); + foreach (int user in args.NewItems) { if (user == api.LocalUser.Value.Id) @@ -82,6 +85,8 @@ namespace osu.Game.Tests.Visual.Gameplay break; case NotifyCollectionChangedAction.Remove: + Debug.Assert(args.OldItems != null, "args.OldItems != null"); + foreach (int user in args.OldItems) { if (user == api.LocalUser.Value.Id) diff --git a/osu.Game.Tournament/IPC/FileBasedIPC.cs b/osu.Game.Tournament/IPC/FileBasedIPC.cs index 999ce61ac8..99147951b2 100644 --- a/osu.Game.Tournament/IPC/FileBasedIPC.cs +++ b/osu.Game.Tournament/IPC/FileBasedIPC.cs @@ -243,7 +243,7 @@ namespace osu.Game.Tournament.IPC string stableInstallPath; using (RegistryKey key = Registry.ClassesRoot.OpenSubKey("osu")) - stableInstallPath = key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty).ToString().Split('"')[1].Replace("osu!.exe", ""); + stableInstallPath = key?.OpenSubKey(@"shell\open\command")?.GetValue(string.Empty)?.ToString().Split('"')[1].Replace("osu!.exe", ""); if (ipcFileExistsInDirectory(stableInstallPath)) return stableInstallPath; From 8bb84570df904c9c55cc781e18cc13bfb13e834e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 04:21:50 +0300 Subject: [PATCH 08/40] Introduce beatmap availability structure --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 56 ++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 osu.Game/Online/Rooms/BeatmapAvailability.cs diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs new file mode 100644 index 0000000000..ca53cb2295 --- /dev/null +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -0,0 +1,56 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; + +namespace osu.Game.Online.Rooms +{ + /// + /// The local availability information about a certain beatmap for the client. + /// + public readonly struct BeatmapAvailability : IEquatable + { + /// + /// The beatmap's availability state. + /// + public readonly DownloadState State; + + /// + /// The beatmap's downloading progress, null when not in state. + /// + public readonly double? DownloadProgress; + + /// + /// Constructs a new non- beatmap availability state. + /// + /// The beatmap availability state. + /// Throws if was specified in this constructor, as it has its own constructor (see . + public BeatmapAvailability(DownloadState state) + { + if (state == DownloadState.Downloading) + throw new ArgumentException($"{nameof(DownloadState.Downloading)} state has its own constructor, use it instead."); + + State = state; + DownloadProgress = null; + } + + /// + /// Constructs a new -specific beatmap availability state. + /// + /// The beatmap availability state (always ). + /// The beatmap's downloading current progress. + /// Throws if non- was specified in this constructor, as they have their own constructor (see . + public BeatmapAvailability(DownloadState state, double downloadProgress) + { + if (state != DownloadState.Downloading) + throw new ArgumentException($"This is a constructor specific for {DownloadState.Downloading} state, use the regular one instead."); + + State = DownloadState.Downloading; + DownloadProgress = downloadProgress; + } + + public bool Equals(BeatmapAvailability other) => State == other.State && DownloadProgress == other.DownloadProgress; + + public override string ToString() => $"{string.Join(", ", State, $"{DownloadProgress:0.00%}")}"; + } +} From 09e5e2629ab2ffa0ad052683bcb91000e18a87fe Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 04:25:06 +0300 Subject: [PATCH 09/40] Add user beatmap availability property --- osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs index 99624dc3e7..92c92f438d 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs @@ -5,6 +5,7 @@ using System; using Newtonsoft.Json; +using osu.Game.Online.Rooms; using osu.Game.Users; namespace osu.Game.Online.Multiplayer @@ -16,6 +17,11 @@ namespace osu.Game.Online.Multiplayer public MultiplayerUserState State { get; set; } = MultiplayerUserState.Idle; + /// + /// The availability state of the beatmap, set to by default. + /// + public BeatmapAvailability BeatmapAvailability { get; set; } = new BeatmapAvailability(DownloadState.LocallyAvailable); + public User? User { get; set; } [JsonConstructor] From dfa8be9173839c3f5dd7aaabb14f0a60692580f1 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 04:32:50 +0300 Subject: [PATCH 10/40] Add beatmap availability change state & event methods --- .../Online/Multiplayer/IMultiplayerClient.cs | 8 ++++++++ .../Multiplayer/IMultiplayerRoomServer.cs | 8 ++++++++ .../Online/Multiplayer/MultiplayerClient.cs | 9 +++++++++ .../Multiplayer/StatefulMultiplayerClient.cs | 20 +++++++++++++++++++ .../Multiplayer/TestMultiplayerClient.cs | 15 ++++++++++++++ 5 files changed, 60 insertions(+) diff --git a/osu.Game/Online/Multiplayer/IMultiplayerClient.cs b/osu.Game/Online/Multiplayer/IMultiplayerClient.cs index b97fcc9ae7..5410fbc030 100644 --- a/osu.Game/Online/Multiplayer/IMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/IMultiplayerClient.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Threading.Tasks; +using osu.Game.Online.Rooms; namespace osu.Game.Online.Multiplayer { @@ -47,6 +48,13 @@ namespace osu.Game.Online.Multiplayer /// The new state of the user. Task UserStateChanged(int userId, MultiplayerUserState state); + /// + /// Signals that a user in this room has their beatmap availability state changed. + /// + /// The ID of the user whose beatmap availability state has changed. + /// The new beatmap availability state of the user. + Task UserBeatmapAvailabilityChanged(int userId, BeatmapAvailability beatmapAvailability); + /// /// Signals that a match is to be started. This will *only* be sent to clients which are to begin loading at this point. /// diff --git a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs index 481e3fb1de..7fda526faf 100644 --- a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs +++ b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Threading.Tasks; +using osu.Game.Online.Rooms; namespace osu.Game.Online.Multiplayer { @@ -40,6 +41,13 @@ namespace osu.Game.Online.Multiplayer /// If the user is not in a room. Task ChangeState(MultiplayerUserState newState); + /// + /// Change the user's local availability state of the beatmap set in joined room. + /// This will also force user state back to . + /// + /// The proposed new beatmap availability state. + Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability); + /// /// As the host of a room, start the match. /// diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 7cd1ef78f7..50dc8f661c 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -14,6 +14,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Game.Online.API; +using osu.Game.Online.Rooms; namespace osu.Game.Online.Multiplayer { @@ -173,6 +174,14 @@ namespace osu.Game.Online.Multiplayer return connection.InvokeAsync(nameof(IMultiplayerServer.ChangeState), newState); } + public override Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability) + { + if (!isConnected.Value) + return Task.CompletedTask; + + return connection.InvokeAsync(nameof(IMultiplayerServer.ChangeBeatmapAvailability), newBeatmapAvailability); + } + public override Task StartMatch() { if (!isConnected.Value) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index dc80488d39..c1818de87a 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -184,6 +184,8 @@ namespace osu.Game.Online.Multiplayer public abstract Task ChangeState(MultiplayerUserState newState); + public abstract Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability); + public abstract Task StartMatch(); Task IMultiplayerClient.RoomStateChanged(MultiplayerRoomState state) @@ -311,6 +313,24 @@ namespace osu.Game.Online.Multiplayer return Task.CompletedTask; } + Task IMultiplayerClient.UserBeatmapAvailabilityChanged(int userId, BeatmapAvailability beatmapAvailability) + { + if (Room == null) + return Task.CompletedTask; + + Scheduler.Add(() => + { + if (Room == null) + return; + + Room.Users.Single(u => u.UserID == userId).BeatmapAvailability = beatmapAvailability; + + RoomUpdated?.Invoke(); + }, false); + + return Task.CompletedTask; + } + Task IMultiplayerClient.LoadRequested() { if (Room == null) diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 2ce5211757..c155447f8c 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -10,6 +10,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Game.Online.API; using osu.Game.Online.Multiplayer; +using osu.Game.Online.Rooms; using osu.Game.Users; namespace osu.Game.Tests.Visual.Multiplayer @@ -77,6 +78,14 @@ namespace osu.Game.Tests.Visual.Multiplayer }); } + public void ChangeUserBeatmapAvailability(int userId, BeatmapAvailability newBeatmapAvailability) + { + Debug.Assert(Room != null); + + ((IMultiplayerClient)this).UserBeatmapAvailabilityChanged(userId, newBeatmapAvailability); + ChangeUserState(userId, MultiplayerUserState.Idle); + } + protected override Task JoinRoom(long roomId) { var user = new MultiplayerRoomUser(api.LocalUser.Value.Id) { User = api.LocalUser.Value }; @@ -108,6 +117,12 @@ namespace osu.Game.Tests.Visual.Multiplayer return Task.CompletedTask; } + public override Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability) + { + ChangeUserBeatmapAvailability(api.LocalUser.Value.Id, newBeatmapAvailability); + return Task.CompletedTask; + } + public override Task StartMatch() { Debug.Assert(Room != null); From 152e9ecccf0f6150236d70a0d90f6d3a721e87aa Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 18:34:11 +0300 Subject: [PATCH 11/40] Make `BeatmapAvailability` class in-line with other online data structures --- osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs | 2 +- osu.Game/Online/Rooms/BeatmapAvailability.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs index 92c92f438d..f515b574df 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs @@ -20,7 +20,7 @@ namespace osu.Game.Online.Multiplayer /// /// The availability state of the beatmap, set to by default. /// - public BeatmapAvailability BeatmapAvailability { get; set; } = new BeatmapAvailability(DownloadState.LocallyAvailable); + public BeatmapAvailability BeatmapAvailability { get; set; } = BeatmapAvailability.LocallyAvailable(); public User? User { get; set; } diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index ca53cb2295..04dcd2a84a 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -8,7 +8,7 @@ namespace osu.Game.Online.Rooms /// /// The local availability information about a certain beatmap for the client. /// - public readonly struct BeatmapAvailability : IEquatable + public class BeatmapAvailability : IEquatable { /// /// The beatmap's availability state. @@ -49,7 +49,7 @@ namespace osu.Game.Online.Rooms DownloadProgress = downloadProgress; } - public bool Equals(BeatmapAvailability other) => State == other.State && DownloadProgress == other.DownloadProgress; + public bool Equals(BeatmapAvailability other) => other != null && State == other.State && DownloadProgress == other.DownloadProgress; public override string ToString() => $"{string.Join(", ", State, $"{DownloadProgress:0.00%}")}"; } From c8423d1c4600d89bf4979715e7da540ca118633c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 18:34:58 +0300 Subject: [PATCH 12/40] Make constructors design more pleasent to eyes --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 30 ++++---------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index 04dcd2a84a..1fd099fcc7 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -20,35 +20,17 @@ namespace osu.Game.Online.Rooms /// public readonly double? DownloadProgress; - /// - /// Constructs a new non- beatmap availability state. - /// - /// The beatmap availability state. - /// Throws if was specified in this constructor, as it has its own constructor (see . - public BeatmapAvailability(DownloadState state) + private BeatmapAvailability(DownloadState state, double? downloadProgress = null) { - if (state == DownloadState.Downloading) - throw new ArgumentException($"{nameof(DownloadState.Downloading)} state has its own constructor, use it instead."); - State = state; - DownloadProgress = null; - } - - /// - /// Constructs a new -specific beatmap availability state. - /// - /// The beatmap availability state (always ). - /// The beatmap's downloading current progress. - /// Throws if non- was specified in this constructor, as they have their own constructor (see . - public BeatmapAvailability(DownloadState state, double downloadProgress) - { - if (state != DownloadState.Downloading) - throw new ArgumentException($"This is a constructor specific for {DownloadState.Downloading} state, use the regular one instead."); - - State = DownloadState.Downloading; DownloadProgress = downloadProgress; } + public static BeatmapAvailability NotDownload() => new BeatmapAvailability(DownloadState.NotDownloaded); + public static BeatmapAvailability Downloading(double progress) => new BeatmapAvailability(DownloadState.Downloading, progress); + public static BeatmapAvailability Downloaded() => new BeatmapAvailability(DownloadState.Downloaded); + public static BeatmapAvailability LocallyAvailable() => new BeatmapAvailability(DownloadState.LocallyAvailable); + public bool Equals(BeatmapAvailability other) => other != null && State == other.State && DownloadProgress == other.DownloadProgress; public override string ToString() => $"{string.Join(", ", State, $"{DownloadProgress:0.00%}")}"; From 839f5a75705f92cb77dfb75392ecf748a7f955d9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sun, 3 Jan 2021 18:36:37 +0300 Subject: [PATCH 13/40] Ensure clients don't blow up when given user isn't in room --- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index c1818de87a..799b66020c 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -320,10 +320,13 @@ namespace osu.Game.Online.Multiplayer Scheduler.Add(() => { - if (Room == null) + var user = Room?.Users.SingleOrDefault(u => u.UserID == userId); + + // we don't care whether the room doesn't exist or user isn't in joined room, just return in that point. + if (user == null) return; - Room.Users.Single(u => u.UserID == userId).BeatmapAvailability = beatmapAvailability; + user.BeatmapAvailability = beatmapAvailability; RoomUpdated?.Invoke(); }, false); From e99310b59cfddbad40882e187a79668cdd865bb4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 11 Jan 2021 08:02:57 +0300 Subject: [PATCH 14/40] Add JsonConstructor attribute --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index 1fd099fcc7..4796e26d14 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using Newtonsoft.Json; namespace osu.Game.Online.Rooms { @@ -20,6 +21,7 @@ namespace osu.Game.Online.Rooms /// public readonly double? DownloadProgress; + [JsonConstructor] private BeatmapAvailability(DownloadState state, double? downloadProgress = null) { State = state; From a8dfa5e2a9b09e07893c83fb410b0c669e6868a5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 11 Jan 2021 08:04:00 +0300 Subject: [PATCH 15/40] Rename typo'd method --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index 4796e26d14..794e79ce55 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -28,7 +28,7 @@ namespace osu.Game.Online.Rooms DownloadProgress = downloadProgress; } - public static BeatmapAvailability NotDownload() => new BeatmapAvailability(DownloadState.NotDownloaded); + public static BeatmapAvailability NotDownloaded() => new BeatmapAvailability(DownloadState.NotDownloaded); public static BeatmapAvailability Downloading(double progress) => new BeatmapAvailability(DownloadState.Downloading, progress); public static BeatmapAvailability Downloaded() => new BeatmapAvailability(DownloadState.Downloaded); public static BeatmapAvailability LocallyAvailable() => new BeatmapAvailability(DownloadState.LocallyAvailable); From 2286e3679f75adb10b9ba7f32316db4e116cea71 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 11 Jan 2021 08:21:07 +0300 Subject: [PATCH 16/40] Downloaded -> Importing --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index 794e79ce55..b6b9c632fe 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -30,7 +30,7 @@ namespace osu.Game.Online.Rooms public static BeatmapAvailability NotDownloaded() => new BeatmapAvailability(DownloadState.NotDownloaded); public static BeatmapAvailability Downloading(double progress) => new BeatmapAvailability(DownloadState.Downloading, progress); - public static BeatmapAvailability Downloaded() => new BeatmapAvailability(DownloadState.Downloaded); + public static BeatmapAvailability Importing() => new BeatmapAvailability(DownloadState.Downloaded); public static BeatmapAvailability LocallyAvailable() => new BeatmapAvailability(DownloadState.LocallyAvailable); public bool Equals(BeatmapAvailability other) => other != null && State == other.State && DownloadProgress == other.DownloadProgress; From 90fb67b377e2022f3bfa60dfca4f31a7d3670228 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 11 Jan 2021 20:52:02 +0300 Subject: [PATCH 17/40] Update code in-line with decided direction --- osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs | 1 - osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs index 7fda526faf..f324a1a216 100644 --- a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs +++ b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs @@ -43,7 +43,6 @@ namespace osu.Game.Online.Multiplayer /// /// Change the user's local availability state of the beatmap set in joined room. - /// This will also force user state back to . /// /// The proposed new beatmap availability state. Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability); diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index c155447f8c..7fbc770351 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -83,7 +83,6 @@ namespace osu.Game.Tests.Visual.Multiplayer Debug.Assert(Room != null); ((IMultiplayerClient)this).UserBeatmapAvailabilityChanged(userId, newBeatmapAvailability); - ChangeUserState(userId, MultiplayerUserState.Idle); } protected override Task JoinRoom(long roomId) From 0d5fbb15ac06c321a0f43d9a2e6ff5d9a61e8cc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 11 Jan 2021 20:28:24 +0100 Subject: [PATCH 18/40] Fix up code comments Default value restated in xmldoc was snipped because it's made redundant by the initialiser and possibly bound to be outdated at some point. --- osu.Game/Online/Multiplayer/IMultiplayerClient.cs | 2 +- osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs | 2 +- osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs | 2 +- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/Multiplayer/IMultiplayerClient.cs b/osu.Game/Online/Multiplayer/IMultiplayerClient.cs index 5410fbc030..19dd473230 100644 --- a/osu.Game/Online/Multiplayer/IMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/IMultiplayerClient.cs @@ -49,7 +49,7 @@ namespace osu.Game.Online.Multiplayer Task UserStateChanged(int userId, MultiplayerUserState state); /// - /// Signals that a user in this room has their beatmap availability state changed. + /// Signals that a user in this room changed their beatmap availability state. /// /// The ID of the user whose beatmap availability state has changed. /// The new beatmap availability state of the user. diff --git a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs index f324a1a216..09816974a7 100644 --- a/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs +++ b/osu.Game/Online/Multiplayer/IMultiplayerRoomServer.cs @@ -42,7 +42,7 @@ namespace osu.Game.Online.Multiplayer Task ChangeState(MultiplayerUserState newState); /// - /// Change the user's local availability state of the beatmap set in joined room. + /// Change the local user's availability state of the current beatmap set in joined room. /// /// The proposed new beatmap availability state. Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability); diff --git a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs index f515b574df..2590acbc81 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerRoomUser.cs @@ -18,7 +18,7 @@ namespace osu.Game.Online.Multiplayer public MultiplayerUserState State { get; set; } = MultiplayerUserState.Idle; /// - /// The availability state of the beatmap, set to by default. + /// The availability state of the current beatmap. /// public BeatmapAvailability BeatmapAvailability { get; set; } = BeatmapAvailability.LocallyAvailable(); diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 799b66020c..99aa8fe015 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -322,7 +322,7 @@ namespace osu.Game.Online.Multiplayer { var user = Room?.Users.SingleOrDefault(u => u.UserID == userId); - // we don't care whether the room doesn't exist or user isn't in joined room, just return in that point. + // errors here are not critical - beatmap availability state is mostly for display. if (user == null) return; From 422260797b68cc6e3710d327c4e3accc66d4797c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Jan 2021 16:03:12 +0900 Subject: [PATCH 19/40] Revert polling changes to fix participant list display It turns out this polling was necessary to get extra data that isn't included in the main listing request. It was removed deemed useless, and in order to fix the order of rooms changing when selecting a room. Weirdly, I can't reproduce this happening any more, and on close inspection of the code can't see how it could happen in the first place. For now, let's revert this change and iterate from there, if/when the same issue arises again. I've discussed avoiding this second poll by potentially including more data (just `user_id`s?) in the main listing request, but not 100% sure on this - even if the returned data is minimal it's an extra join server-side, which could cause performance issues for large numbers of rooms. --- .../TestSceneMultiplayerRoomManager.cs | 1 + .../OnlinePlay/Multiplayer/Multiplayer.cs | 5 +++- .../Multiplayer/MultiplayerRoomManager.cs | 28 ++++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs index 80d1acd145..7a3845cbf3 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs @@ -143,6 +143,7 @@ namespace osu.Game.Tests.Visual.Multiplayer RoomManager = { TimeBetweenListingPolls = { Value = 1 }, + TimeBetweenSelectionPolls = { Value = 1 } } }; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs index 310617a0bc..76f5c74433 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs @@ -33,6 +33,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer if (!this.IsCurrentScreen()) { multiplayerRoomManager.TimeBetweenListingPolls.Value = 0; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = 0; } else { @@ -40,16 +41,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { case LoungeSubScreen _: multiplayerRoomManager.TimeBetweenListingPolls.Value = isIdle ? 120000 : 15000; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = isIdle ? 120000 : 15000; break; // Don't poll inside the match or anywhere else. default: multiplayerRoomManager.TimeBetweenListingPolls.Value = 0; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = 0; break; } } - Logger.Log($"Polling adjusted (listing: {multiplayerRoomManager.TimeBetweenListingPolls.Value})"); + Logger.Log($"Polling adjusted (listing: {multiplayerRoomManager.TimeBetweenListingPolls.Value}, selection: {multiplayerRoomManager.TimeBetweenSelectionPolls.Value})"); } protected override Room CreateNewRoom() diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs index 5c327266a3..3cb263298f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private StatefulMultiplayerClient multiplayerClient { get; set; } public readonly Bindable TimeBetweenListingPolls = new Bindable(); - + public readonly Bindable TimeBetweenSelectionPolls = new Bindable(); private readonly IBindable isConnected = new Bindable(); private readonly Bindable allowPolling = new Bindable(); @@ -119,6 +119,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer TimeBetweenPolls = { BindTarget = TimeBetweenListingPolls }, AllowPolling = { BindTarget = allowPolling } }, + new MultiplayerSelectionPollingComponent + { + TimeBetweenPolls = { BindTarget = TimeBetweenSelectionPolls }, + AllowPolling = { BindTarget = allowPolling } + } }; private class MultiplayerListingPollingComponent : ListingPollingComponent @@ -141,5 +146,26 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer protected override Task Poll() => !AllowPolling.Value ? Task.CompletedTask : base.Poll(); } + + private class MultiplayerSelectionPollingComponent : SelectionPollingComponent + { + public readonly IBindable AllowPolling = new Bindable(); + + protected override void LoadComplete() + { + base.LoadComplete(); + + AllowPolling.BindValueChanged(allowPolling => + { + if (!allowPolling.NewValue) + return; + + if (IsLoaded) + PollImmediately(); + }); + } + + protected override Task Poll() => !AllowPolling.Value ? Task.CompletedTask : base.Poll(); + } } } From b51b07c3a90c0dc1635984f9346175746c5e3da7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Jan 2021 18:05:29 +0900 Subject: [PATCH 20/40] Fix unstable multiplayer room ordering when selection is made --- .../OnlinePlay/Components/SelectionPollingComponent.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs index 0eec155060..dcf3c94b76 100644 --- a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs +++ b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -47,9 +48,11 @@ namespace osu.Game.Screens.OnlinePlay.Components pollReq.Success += result => { - var rooms = new List(roomManager.Rooms); + // existing rooms need to be ordered by their position because the received of NotifyRoomsReceives expects to be able to sort them based on this order. + var rooms = new List(roomManager.Rooms.OrderBy(r => r.Position.Value)); int index = rooms.FindIndex(r => r.RoomID.Value == result.RoomID.Value); + if (index < 0) return; From 7298adc9d9b8ef878f4be5b8799be222aa0dd90b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Jan 2021 19:04:16 +0900 Subject: [PATCH 21/40] Fix non-threadsafe usage of MultiplayerClient.IsConnected --- osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs | 1 + .../OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs | 4 ++-- .../Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 4 ++-- .../OnlinePlay/Multiplayer/MultiplayerRoomManager.cs | 6 ++---- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs index 34cba09e8c..770039d79d 100644 --- a/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/StatefulMultiplayerClient.cs @@ -52,6 +52,7 @@ namespace osu.Game.Online.Multiplayer /// /// Whether the is currently connected. + /// This is NOT thread safe and usage should be scheduled. /// public abstract IBindable IsConnected { get; } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs index 87b0e49b5b..a13d2cf540 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/CreateMultiplayerMatchButton.cs @@ -34,8 +34,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { base.LoadComplete(); - isConnected.BindValueChanged(_ => updateState()); - operationInProgress.BindValueChanged(_ => updateState(), true); + isConnected.BindValueChanged(_ => Scheduler.AddOnce(updateState)); + operationInProgress.BindValueChanged(_ => Scheduler.AddOnce(updateState), true); } private void updateState() => Enabled.Value = isConnected.Value && !operationInProgress.Value; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index 4bee502e2e..04d9e0a72a 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -77,14 +77,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer }); isConnected = client.IsConnected.GetBoundCopy(); - isConnected.BindValueChanged(connected => + isConnected.BindValueChanged(connected => Schedule(() => { if (!connected.NewValue) { // messaging to the user about this disconnect will be provided by the MultiplayerMatchSubScreen. failAndBail(); } - }, true); + }), true); Debug.Assert(client.Room != null); } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs index 5c327266a3..bcae2e5cbb 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs @@ -34,10 +34,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer base.LoadComplete(); isConnected.BindTo(multiplayerClient.IsConnected); - isConnected.BindValueChanged(_ => Schedule(updatePolling)); - JoinedRoom.BindValueChanged(_ => updatePolling()); - - updatePolling(); + isConnected.BindValueChanged(_ => Scheduler.AddOnce(updatePolling)); + JoinedRoom.BindValueChanged(_ => Scheduler.AddOnce(updatePolling), true); } public override void CreateRoom(Room room, Action onSuccess = null, Action onError = null) From 95acc457aad5f08df2b7a7a4bc37af4a241ffc2d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 13 Jan 2021 22:34:48 +0300 Subject: [PATCH 22/40] Fix stupid mistake fuck. --- osu.Game/Online/Rooms/BeatmapAvailability.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/Rooms/BeatmapAvailability.cs b/osu.Game/Online/Rooms/BeatmapAvailability.cs index b6b9c632fe..e7dbc5f436 100644 --- a/osu.Game/Online/Rooms/BeatmapAvailability.cs +++ b/osu.Game/Online/Rooms/BeatmapAvailability.cs @@ -30,7 +30,7 @@ namespace osu.Game.Online.Rooms public static BeatmapAvailability NotDownloaded() => new BeatmapAvailability(DownloadState.NotDownloaded); public static BeatmapAvailability Downloading(double progress) => new BeatmapAvailability(DownloadState.Downloading, progress); - public static BeatmapAvailability Importing() => new BeatmapAvailability(DownloadState.Downloaded); + public static BeatmapAvailability Importing() => new BeatmapAvailability(DownloadState.Importing); public static BeatmapAvailability LocallyAvailable() => new BeatmapAvailability(DownloadState.LocallyAvailable); public bool Equals(BeatmapAvailability other) => other != null && State == other.State && DownloadProgress == other.DownloadProgress; From 8a0b975d71d08f6a845b10b46aec50d714d00c68 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Jan 2021 18:25:32 +0900 Subject: [PATCH 23/40] Fix deadlock scenario when calculating fallback difficulty The previous code would run a calcaulation for the beatmap's own ruleset if the current one failed. While this does make sense, with the current way we use this component (and the implementation flow) it is quite unsafe. The to the call on `.Result` in the `catch` block, this would 100% deadlock due to the thread concurrency of the `ThreadedTaskScheduler` being 1. Even if the nested run could be run inline (it should be), the task scheduler won't even get to the point of checking whether this is feasible due to it being saturated by the already running task. I'm not sure if we still need this fallback lookup logic. After removing it, it's feasible that 0 stars will be returned during the scenario that previously caused a deadlock, but I don't necessarily think this is incorrect. There may be another reason for this needing to exist which I'm not aware of (diffcalc?) but if that's the case we may want to move the try-catch handling to the point of usage. To reproduce the deadlock scenario with 100% success (the repro instructions in the linked issue aren't that simple and require some patience and good timing), the main portion of the lookup can be changed to randomly trigger a nested lookup: ``` if (RNG.NextSingle() > 0.5f) return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result; else return new StarDifficulty(attributes); ``` After switching beatmap once or twice, pausing debug and viewing the state of threads should show exactly what is going on. --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 3b58062add..37d262abe5 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -260,17 +260,10 @@ namespace osu.Game.Beatmaps } catch (BeatmapInvalidForRulesetException e) { - // Conversion has failed for the given ruleset, so return the difficulty in the beatmap's default ruleset. - - // Ensure the beatmap's default ruleset isn't the one already being converted to. - // This shouldn't happen as it means something went seriously wrong, but if it does an endless loop should be avoided. if (rulesetInfo.Equals(beatmapInfo.Ruleset)) - { Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineBeatmapID} to the beatmap's default ruleset ({beatmapInfo.Ruleset})."); - return new StarDifficulty(); - } - return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result; + return new StarDifficulty(); } catch { From 4cccde9007cdb1646f041da71d77184c51f490d2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 13:20:46 +0900 Subject: [PATCH 24/40] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 492c88c7e4..919d83f8db 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index ff016199d3..ac014f2964 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -28,7 +28,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 93be3645ee..799042626b 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -88,7 +88,7 @@ - + From 0a65ae8f1ef72f883e9f1e324bf3cd622bd321d0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 14:07:24 +0900 Subject: [PATCH 25/40] Fix the beatmap carousel playing the difficulty change sample on beatmap change --- osu.Game/Screens/Select/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 40db04ae71..6c0bd3a228 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -500,7 +500,7 @@ namespace osu.Game.Screens.Select if (beatmap != null) { - if (beatmap.BeatmapSetInfoID == beatmapNoDebounce?.BeatmapSetInfoID) + if (beatmap.BeatmapSetInfoID == previous?.BeatmapInfo.BeatmapSetInfoID) sampleChangeDifficulty.Play(); else sampleChangeBeatmap.Play(); From ebbc32adfa9c4652cadff32dc5c429af9867ff09 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 14:51:26 +0900 Subject: [PATCH 26/40] Change conditional used to decide legacy judgement animation to match stable In stable, the type of legacy judgement to show is based on the presence of particle textures in the skin. We were using the skin version instead, which turns out to be incorrect and not what some user skins expect. Closes #11078. --- osu.Game/Skinning/LegacySkin.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index e4e5bf2f75..7397e3d08b 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -378,8 +378,12 @@ namespace osu.Game.Skinning // kind of wasteful that we throw this away, but should do for now. if (createDrawable() != null) { - if (Configuration.LegacyVersion > 1) - return new LegacyJudgementPieceNew(resultComponent.Component, createDrawable, getParticleTexture(resultComponent.Component)); + var particle = getParticleTexture(resultComponent.Component); + + if (particle != null) + { + return new LegacyJudgementPieceNew(resultComponent.Component, createDrawable, particle); + } else return new LegacyJudgementPieceOld(resultComponent.Component, createDrawable); } From 86f66727de47175b8045523c56f719a8051041a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 13:29:37 +0900 Subject: [PATCH 27/40] Update KeyBinding usages in line with interface changes --- osu.Game/Database/OsuDbContext.cs | 2 ++ osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs | 2 +- osu.Game/Input/Bindings/GlobalActionContainer.cs | 2 +- osu.Game/Input/KeyBindingStore.cs | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/OsuDbContext.cs b/osu.Game/Database/OsuDbContext.cs index 2ae07b3cf8..2aae62edea 100644 --- a/osu.Game/Database/OsuDbContext.cs +++ b/osu.Game/Database/OsuDbContext.cs @@ -135,6 +135,8 @@ namespace osu.Game.Database modelBuilder.Entity().HasIndex(b => new { b.RulesetID, b.Variant }); modelBuilder.Entity().HasIndex(b => b.IntAction); + modelBuilder.Entity().Ignore(b => b.KeyCombination); + modelBuilder.Entity().Ignore(b => b.Action); modelBuilder.Entity().HasIndex(b => new { b.RulesetID, b.Variant }); diff --git a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs index 94edc33099..d12eaa10f6 100644 --- a/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs +++ b/osu.Game/Input/Bindings/DatabasedKeyBindingContainer.cs @@ -23,7 +23,7 @@ namespace osu.Game.Input.Bindings private KeyBindingStore store; - public override IEnumerable DefaultKeyBindings => ruleset.CreateInstance().GetDefaultKeyBindings(variant ?? 0); + public override IEnumerable DefaultKeyBindings => ruleset.CreateInstance().GetDefaultKeyBindings(variant ?? 0); /// /// Create a new instance. diff --git a/osu.Game/Input/Bindings/GlobalActionContainer.cs b/osu.Game/Input/Bindings/GlobalActionContainer.cs index b8c2fa201f..8ccdb9249e 100644 --- a/osu.Game/Input/Bindings/GlobalActionContainer.cs +++ b/osu.Game/Input/Bindings/GlobalActionContainer.cs @@ -21,7 +21,7 @@ namespace osu.Game.Input.Bindings handler = game; } - public override IEnumerable DefaultKeyBindings => GlobalKeyBindings.Concat(InGameKeyBindings).Concat(AudioControlKeyBindings).Concat(EditorKeyBindings); + public override IEnumerable DefaultKeyBindings => GlobalKeyBindings.Concat(InGameKeyBindings).Concat(AudioControlKeyBindings).Concat(EditorKeyBindings); public IEnumerable GlobalKeyBindings => new[] { diff --git a/osu.Game/Input/KeyBindingStore.cs b/osu.Game/Input/KeyBindingStore.cs index bc73d74d74..b25b00eb84 100644 --- a/osu.Game/Input/KeyBindingStore.cs +++ b/osu.Game/Input/KeyBindingStore.cs @@ -49,7 +49,7 @@ namespace osu.Game.Input } } - private void insertDefaults(IEnumerable defaults, int? rulesetId = null, int? variant = null) + private void insertDefaults(IEnumerable defaults, int? rulesetId = null, int? variant = null) { using (var usage = ContextFactory.GetForWrite()) { From 51255033e24c2afbbac85573ef5e7edf610eb920 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 13:41:35 +0900 Subject: [PATCH 28/40] Update some missed usages of KeyBindingContainer in tests --- osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecorder.cs | 2 +- osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs | 2 +- osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecorder.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecorder.cs index b2ad7ca5b4..802dbf2021 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecorder.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecorder.cs @@ -244,7 +244,7 @@ namespace osu.Game.Tests.Visual.Gameplay internal class TestKeyBindingContainer : KeyBindingContainer { - public override IEnumerable DefaultKeyBindings => new[] + public override IEnumerable DefaultKeyBindings => new[] { new KeyBinding(InputKey.MouseLeft, TestAction.Down), }; diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs index 40c4214749..6e338b7202 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayRecording.cs @@ -179,7 +179,7 @@ namespace osu.Game.Tests.Visual.Gameplay internal class TestKeyBindingContainer : KeyBindingContainer { - public override IEnumerable DefaultKeyBindings => new[] + public override IEnumerable DefaultKeyBindings => new[] { new KeyBinding(InputKey.MouseLeft, TestAction.Down), }; diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs index b9ff95cb29..8278ff9adf 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs @@ -303,7 +303,7 @@ namespace osu.Game.Tests.Visual.Gameplay internal class TestKeyBindingContainer : KeyBindingContainer { - public override IEnumerable DefaultKeyBindings => new[] + public override IEnumerable DefaultKeyBindings => new[] { new KeyBinding(InputKey.MouseLeft, TestAction.Down), }; From f42a6270bbd750e2539ddad3ff97c0adb29ee521 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 14:53:55 +0900 Subject: [PATCH 29/40] Update framework (again) for native libs fix --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 919d83f8db..db5c933c41 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index ac014f2964..5e9e90c78f 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -28,7 +28,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 799042626b..225cf981f2 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -88,7 +88,7 @@ - + From e0a4a666c8e5fa8224fae065d3597b49fa747103 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 15:01:16 +0900 Subject: [PATCH 30/40] Remove unnecessary workaround (mentioned package is pinned by SignalR to a working version) --- osu.Game/osu.Game.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 5e9e90c78f..301ee39a61 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -18,8 +18,6 @@ - - From 7c612ec5561225b2a618bf11915aa693e20e7cf6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 15:11:03 +0900 Subject: [PATCH 31/40] Remove global.json --- global.json | 10 ---------- osu.sln | 1 - 2 files changed, 11 deletions(-) delete mode 100644 global.json diff --git a/global.json b/global.json deleted file mode 100644 index f5aaffcd3d..0000000000 --- a/global.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "sdk": { - "allowPrerelease": false, - "rollForward": "minor", - "version": "5.0.100" - }, - "msbuild-sdks": { - "Microsoft.Build.Traversal": "3.0.2" - } -} \ No newline at end of file diff --git a/osu.sln b/osu.sln index 1d64f6ff10..c9453359b1 100644 --- a/osu.sln +++ b/osu.sln @@ -57,7 +57,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution ProjectSection(SolutionItems) = preProject .editorconfig = .editorconfig Directory.Build.props = Directory.Build.props - global.json = global.json osu.Android.props = osu.Android.props osu.iOS.props = osu.iOS.props CodeAnalysis\osu.ruleset = CodeAnalysis\osu.ruleset From 3f8834030416aeb80f5746a37999a83c16064d86 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 15:17:38 +0900 Subject: [PATCH 32/40] Restore previous exception handling flow for stable path lookup --- osu.Desktop/OsuGameDesktop.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index 55e42b160e..d1515acafa 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -59,10 +59,14 @@ namespace osu.Desktop if (OperatingSystem.IsWindows()) { - stableInstallPath = getStableInstallPathFromRegistry(); + try + { + stableInstallPath = getStableInstallPathFromRegistry(); - if (checkExists(stableInstallPath)) - return stableInstallPath; + if (!string.IsNullOrEmpty(stableInstallPath) && checkExists(stableInstallPath)) + return stableInstallPath; + } + catch { } } stableInstallPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @"osu!"); From d023ad8ad1a03b500f13b90ca5d1d24a75e182ea Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 15:18:29 +0900 Subject: [PATCH 33/40] Remove assert messages --- osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs index 8278ff9adf..35b3bfc1f8 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSpectatorPlayback.cs @@ -75,7 +75,7 @@ namespace osu.Game.Tests.Visual.Gameplay switch (args.Action) { case NotifyCollectionChangedAction.Add: - Debug.Assert(args.NewItems != null, "args.NewItems != null"); + Debug.Assert(args.NewItems != null); foreach (int user in args.NewItems) { @@ -86,7 +86,7 @@ namespace osu.Game.Tests.Visual.Gameplay break; case NotifyCollectionChangedAction.Remove: - Debug.Assert(args.OldItems != null, "args.OldItems != null"); + Debug.Assert(args.OldItems != null); foreach (int user in args.OldItems) { From b8c85ef017184b8672adb80401375bd3c8c8e224 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Jan 2021 16:03:12 +0900 Subject: [PATCH 34/40] Revert polling changes to fix participant list display It turns out this polling was necessary to get extra data that isn't included in the main listing request. It was removed deemed useless, and in order to fix the order of rooms changing when selecting a room. Weirdly, I can't reproduce this happening any more, and on close inspection of the code can't see how it could happen in the first place. For now, let's revert this change and iterate from there, if/when the same issue arises again. I've discussed avoiding this second poll by potentially including more data (just `user_id`s?) in the main listing request, but not 100% sure on this - even if the returned data is minimal it's an extra join server-side, which could cause performance issues for large numbers of rooms. --- .../TestSceneMultiplayerRoomManager.cs | 1 + .../OnlinePlay/Multiplayer/Multiplayer.cs | 5 +++- .../Multiplayer/MultiplayerRoomManager.cs | 28 ++++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs index 80d1acd145..7a3845cbf3 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerRoomManager.cs @@ -143,6 +143,7 @@ namespace osu.Game.Tests.Visual.Multiplayer RoomManager = { TimeBetweenListingPolls = { Value = 1 }, + TimeBetweenSelectionPolls = { Value = 1 } } }; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs index 310617a0bc..76f5c74433 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Multiplayer.cs @@ -33,6 +33,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer if (!this.IsCurrentScreen()) { multiplayerRoomManager.TimeBetweenListingPolls.Value = 0; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = 0; } else { @@ -40,16 +41,18 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { case LoungeSubScreen _: multiplayerRoomManager.TimeBetweenListingPolls.Value = isIdle ? 120000 : 15000; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = isIdle ? 120000 : 15000; break; // Don't poll inside the match or anywhere else. default: multiplayerRoomManager.TimeBetweenListingPolls.Value = 0; + multiplayerRoomManager.TimeBetweenSelectionPolls.Value = 0; break; } } - Logger.Log($"Polling adjusted (listing: {multiplayerRoomManager.TimeBetweenListingPolls.Value})"); + Logger.Log($"Polling adjusted (listing: {multiplayerRoomManager.TimeBetweenListingPolls.Value}, selection: {multiplayerRoomManager.TimeBetweenSelectionPolls.Value})"); } protected override Room CreateNewRoom() diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs index 5c327266a3..3cb263298f 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerRoomManager.cs @@ -23,7 +23,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private StatefulMultiplayerClient multiplayerClient { get; set; } public readonly Bindable TimeBetweenListingPolls = new Bindable(); - + public readonly Bindable TimeBetweenSelectionPolls = new Bindable(); private readonly IBindable isConnected = new Bindable(); private readonly Bindable allowPolling = new Bindable(); @@ -119,6 +119,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer TimeBetweenPolls = { BindTarget = TimeBetweenListingPolls }, AllowPolling = { BindTarget = allowPolling } }, + new MultiplayerSelectionPollingComponent + { + TimeBetweenPolls = { BindTarget = TimeBetweenSelectionPolls }, + AllowPolling = { BindTarget = allowPolling } + } }; private class MultiplayerListingPollingComponent : ListingPollingComponent @@ -141,5 +146,26 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer protected override Task Poll() => !AllowPolling.Value ? Task.CompletedTask : base.Poll(); } + + private class MultiplayerSelectionPollingComponent : SelectionPollingComponent + { + public readonly IBindable AllowPolling = new Bindable(); + + protected override void LoadComplete() + { + base.LoadComplete(); + + AllowPolling.BindValueChanged(allowPolling => + { + if (!allowPolling.NewValue) + return; + + if (IsLoaded) + PollImmediately(); + }); + } + + protected override Task Poll() => !AllowPolling.Value ? Task.CompletedTask : base.Poll(); + } } } From ede5abdba4eb0f88806c38790c7a622546f70af9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Jan 2021 18:05:29 +0900 Subject: [PATCH 35/40] Fix unstable multiplayer room ordering when selection is made --- .../OnlinePlay/Components/SelectionPollingComponent.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs index 0eec155060..dcf3c94b76 100644 --- a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs +++ b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -47,9 +48,11 @@ namespace osu.Game.Screens.OnlinePlay.Components pollReq.Success += result => { - var rooms = new List(roomManager.Rooms); + // existing rooms need to be ordered by their position because the received of NotifyRoomsReceives expects to be able to sort them based on this order. + var rooms = new List(roomManager.Rooms.OrderBy(r => r.Position.Value)); int index = rooms.FindIndex(r => r.RoomID.Value == result.RoomID.Value); + if (index < 0) return; From 2b578e97e532fc0bb215566b8bd34df25d1d4625 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Jan 2021 18:25:32 +0900 Subject: [PATCH 36/40] Fix deadlock scenario when calculating fallback difficulty The previous code would run a calcaulation for the beatmap's own ruleset if the current one failed. While this does make sense, with the current way we use this component (and the implementation flow) it is quite unsafe. The to the call on `.Result` in the `catch` block, this would 100% deadlock due to the thread concurrency of the `ThreadedTaskScheduler` being 1. Even if the nested run could be run inline (it should be), the task scheduler won't even get to the point of checking whether this is feasible due to it being saturated by the already running task. I'm not sure if we still need this fallback lookup logic. After removing it, it's feasible that 0 stars will be returned during the scenario that previously caused a deadlock, but I don't necessarily think this is incorrect. There may be another reason for this needing to exist which I'm not aware of (diffcalc?) but if that's the case we may want to move the try-catch handling to the point of usage. To reproduce the deadlock scenario with 100% success (the repro instructions in the linked issue aren't that simple and require some patience and good timing), the main portion of the lookup can be changed to randomly trigger a nested lookup: ``` if (RNG.NextSingle() > 0.5f) return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result; else return new StarDifficulty(attributes); ``` After switching beatmap once or twice, pausing debug and viewing the state of threads should show exactly what is going on. --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 3b58062add..37d262abe5 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -260,17 +260,10 @@ namespace osu.Game.Beatmaps } catch (BeatmapInvalidForRulesetException e) { - // Conversion has failed for the given ruleset, so return the difficulty in the beatmap's default ruleset. - - // Ensure the beatmap's default ruleset isn't the one already being converted to. - // This shouldn't happen as it means something went seriously wrong, but if it does an endless loop should be avoided. if (rulesetInfo.Equals(beatmapInfo.Ruleset)) - { Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineBeatmapID} to the beatmap's default ruleset ({beatmapInfo.Ruleset})."); - return new StarDifficulty(); - } - return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result; + return new StarDifficulty(); } catch { From ed78be825f9a988df3920fdbda730120bebfe125 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 15:47:41 +0900 Subject: [PATCH 37/40] Fix editor timeline not snapping on non-precise wheel scroll For wheel input with precision, we still prefer exact tracking for now. May change this in the future based on feedback from mappers, but it makes little sense to do non-snapped scrolling when input is coming from a non-precise source. --- .../Screens/Edit/Compose/Components/Timeline/Timeline.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs index 20836c0e68..12f7625bf9 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs @@ -138,6 +138,15 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline scrollToTrackTime(); } + protected override bool OnScroll(ScrollEvent e) + { + // if this is not a precision scroll event, let the editor handle the seek itself (for snapping support) + if (!e.AltPressed && !e.IsPrecise) + return false; + + return base.OnScroll(e); + } + protected override void UpdateAfterChildren() { base.UpdateAfterChildren(); From 88a27124c095f5a3cff7eaddaaab92cfe1832d55 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 17:13:27 +0900 Subject: [PATCH 38/40] Make long spinner test longer and fix step name --- osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs index 496b1b3559..c22b1dc407 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs @@ -34,7 +34,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(true)] public void TestLongSpinner(bool autoplay) { - AddStep("Very short spinner", () => SetContents(() => testSingle(5, autoplay, 2000))); + AddStep("Very long spinner", () => SetContents(() => testSingle(5, autoplay, 4000))); AddUntilStep("Wait for completion", () => drawableSpinner.Result.HasResult); AddUntilStep("Check correct progress", () => drawableSpinner.Progress == (autoplay ? 1 : 0)); } From 6adb6b6700b4b19336955b397d5939679919d70b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Jan 2021 17:13:52 +0900 Subject: [PATCH 39/40] Fix spinner tests not playing spinning sound due to empty hitsamples --- osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs index c22b1dc407..f697a77d94 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs @@ -1,9 +1,11 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Graphics; +using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Rulesets.Mods; @@ -55,7 +57,11 @@ namespace osu.Game.Rulesets.Osu.Tests var spinner = new Spinner { StartTime = Time.Current + delay, - EndTime = Time.Current + delay + length + EndTime = Time.Current + delay + length, + Samples = new List + { + new HitSampleInfo("hitnormal") + } }; spinner.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = circleSize }); From 3c1a86d11deefc694a00ebad3369c563a07168de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 15 Jan 2021 22:04:45 +0100 Subject: [PATCH 40/40] Trim braces for consistency --- osu.Game/Skinning/LegacySkin.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 7397e3d08b..090ffaebd7 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -381,9 +381,7 @@ namespace osu.Game.Skinning var particle = getParticleTexture(resultComponent.Component); if (particle != null) - { return new LegacyJudgementPieceNew(resultComponent.Component, createDrawable, particle); - } else return new LegacyJudgementPieceOld(resultComponent.Component, createDrawable); }