From ea536dea23c9b6cfe75693b679304ff416aa7688 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 11 Nov 2021 14:45:40 +0900 Subject: [PATCH 1/5] Gracefully handle missing type rather than triggering `ArgumentNullException` --- osu.Game/Rulesets/RulesetStore.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs index 391bf2c07d..914f33a7ae 100644 --- a/osu.Game/Rulesets/RulesetStore.cs +++ b/osu.Game/Rulesets/RulesetStore.cs @@ -129,13 +129,18 @@ namespace osu.Game.Rulesets { try { - var instanceInfo = ((Ruleset)Activator.CreateInstance(Type.GetType(r.InstantiationInfo).AsNonNull())).RulesetInfo; + var resolvedType = Type.GetType(r.InstantiationInfo); - r.Name = instanceInfo.Name; - r.ShortName = instanceInfo.ShortName; - r.InstantiationInfo = instanceInfo.InstantiationInfo; + if (resolvedType != null) + { + var instanceInfo = ((Ruleset)Activator.CreateInstance(resolvedType)).RulesetInfo; - r.Available = true; + r.Name = instanceInfo.Name; + r.ShortName = instanceInfo.ShortName; + r.InstantiationInfo = instanceInfo.InstantiationInfo; + + r.Available = true; + } } catch { From 57c333b472f724c24aab22e1890ed0b993926755 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 11 Nov 2021 15:29:08 +0900 Subject: [PATCH 2/5] Remove unused using --- osu.Game/Rulesets/RulesetStore.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs index 914f33a7ae..1ddd5396f4 100644 --- a/osu.Game/Rulesets/RulesetStore.cs +++ b/osu.Game/Rulesets/RulesetStore.cs @@ -7,7 +7,6 @@ using System.IO; using System.Linq; using System.Reflection; using osu.Framework; -using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Database; From f38d6ef8db67229dc7a4f440aa63be23d7a6c882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 11 Nov 2021 08:39:59 +0100 Subject: [PATCH 3/5] Add failing test steps --- .../Visual/Multiplayer/TestSceneTeamVersus.cs | 13 +++++++++++-- .../Visual/Multiplayer/TestMultiplayerClient.cs | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index 1efa8d6810..99ff307235 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -10,6 +10,7 @@ using osu.Framework.Platform; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Database; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus; using osu.Game.Online.Rooms; using osu.Game.Rulesets; @@ -97,16 +98,24 @@ namespace osu.Game.Tests.Visual.Multiplayer }); AddAssert("user on team 0", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 0); + AddStep("add another user", () => client.AddUser(new APIUser { Username = "otheruser", Id = 44 })); - AddStep("press button", () => + AddStep("press own button", () => { InputManager.MoveMouseTo(multiplayerScreenStack.ChildrenOfType().First()); InputManager.Click(MouseButton.Left); }); AddAssert("user on team 1", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 1); - AddStep("press button", () => InputManager.Click(MouseButton.Left)); + AddStep("press own button again", () => InputManager.Click(MouseButton.Left)); AddAssert("user on team 0", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 0); + + AddStep("press other user's button", () => + { + InputManager.MoveMouseTo(multiplayerScreenStack.ChildrenOfType().ElementAt(1)); + InputManager.Click(MouseButton.Left); + }); + AddAssert("user still on team 0", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 0); } [Test] diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 0860e45346..f81ca70631 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs @@ -71,6 +71,21 @@ namespace osu.Game.Tests.Visual.Multiplayer // We want the user to be immediately available for testing, so force a scheduler update to run the update-bound continuation. Scheduler.Update(); + + switch (Room?.MatchState) + { + case TeamVersusRoomState teamVersus: + Debug.Assert(Room != null); + + // simulate the server's automatic assignment of users to teams on join. + // the "best" team is the one with the least users on it. + int bestTeam = teamVersus.Teams + .Select(team => (teamID: team.ID, userCount: Room.Users.Count(u => (u.MatchState as TeamVersusUserState)?.TeamID == team.ID))) + .OrderBy(pair => pair.userCount) + .First().teamID; + ((IMultiplayerClient)this).MatchUserStateChanged(user.UserID, new TeamVersusUserState { TeamID = bestTeam }).Wait(); + break; + } } public void RemoveUser(APIUser user) From 9664b9a97ca60957c2137e7c08b59c2ed12792ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 11 Nov 2021 08:40:50 +0100 Subject: [PATCH 4/5] Fix being able to switch own team by clicking other players' team indicators --- .../Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs index 1bf62241f2..73aca0acdc 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/TeamDisplay.cs @@ -51,7 +51,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants Alpha = 0, Scale = new Vector2(0, 1), RelativeSizeAxes = Axes.Y, - Action = changeTeam, Child = box = new Container { RelativeSizeAxes = Axes.Both, From 4bca96d548b72725164ffdc73844ac4644bb26af Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 11 Nov 2021 17:37:43 +0900 Subject: [PATCH 5/5] Throw again to ensure correct available state is set Also standardises handling between `RulesetStore` and `RealmRulesetStore`. --- osu.Game/Rulesets/RulesetLoadException.cs | 15 +++++++++++++++ osu.Game/Rulesets/RulesetStore.cs | 18 ++++++++---------- osu.Game/Stores/RealmRulesetStore.cs | 18 +++++++----------- 3 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 osu.Game/Rulesets/RulesetLoadException.cs diff --git a/osu.Game/Rulesets/RulesetLoadException.cs b/osu.Game/Rulesets/RulesetLoadException.cs new file mode 100644 index 0000000000..7c3a4bb75d --- /dev/null +++ b/osu.Game/Rulesets/RulesetLoadException.cs @@ -0,0 +1,15 @@ +// 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.Rulesets +{ + public class RulesetLoadException : Exception + { + public RulesetLoadException(string message) + : base(@$"Ruleset could not be loaded ({message})") + { + } + } +} diff --git a/osu.Game/Rulesets/RulesetStore.cs b/osu.Game/Rulesets/RulesetStore.cs index 1ddd5396f4..6dd036c0e6 100644 --- a/osu.Game/Rulesets/RulesetStore.cs +++ b/osu.Game/Rulesets/RulesetStore.cs @@ -128,18 +128,16 @@ namespace osu.Game.Rulesets { try { - var resolvedType = Type.GetType(r.InstantiationInfo); + var resolvedType = Type.GetType(r.InstantiationInfo) + ?? throw new RulesetLoadException(@"Type could not be resolved"); - if (resolvedType != null) - { - var instanceInfo = ((Ruleset)Activator.CreateInstance(resolvedType)).RulesetInfo; + var instanceInfo = (Activator.CreateInstance(resolvedType) as Ruleset)?.RulesetInfo + ?? throw new RulesetLoadException(@"Instantiation failure"); - r.Name = instanceInfo.Name; - r.ShortName = instanceInfo.ShortName; - r.InstantiationInfo = instanceInfo.InstantiationInfo; - - r.Available = true; - } + r.Name = instanceInfo.Name; + r.ShortName = instanceInfo.ShortName; + r.InstantiationInfo = instanceInfo.InstantiationInfo; + r.Available = true; } catch { diff --git a/osu.Game/Stores/RealmRulesetStore.cs b/osu.Game/Stores/RealmRulesetStore.cs index 0d2cddf874..cf9ffd112c 100644 --- a/osu.Game/Stores/RealmRulesetStore.cs +++ b/osu.Game/Stores/RealmRulesetStore.cs @@ -147,19 +147,15 @@ namespace osu.Game.Stores { try { - var type = Type.GetType(r.InstantiationInfo); + var resolvedType = Type.GetType(r.InstantiationInfo) + ?? throw new RulesetLoadException(@"Type could not be resolved"); - if (type == null) - throw new InvalidOperationException(@"Type resolution failure."); + var instanceInfo = (Activator.CreateInstance(resolvedType) as Ruleset)?.RulesetInfo + ?? throw new RulesetLoadException(@"Instantiation failure"); - var rInstance = (Activator.CreateInstance(type) as Ruleset)?.RulesetInfo; - - if (rInstance == null) - throw new InvalidOperationException(@"Instantiation failure."); - - r.Name = rInstance.Name; - r.ShortName = rInstance.ShortName; - r.InstantiationInfo = rInstance.InstantiationInfo; + r.Name = instanceInfo.Name; + r.ShortName = instanceInfo.ShortName; + r.InstantiationInfo = instanceInfo.InstantiationInfo; r.Available = true; detachedRulesets.Add(r.Clone());