From 53e61c5041954c570fb1b5f08a73dbc43da5ed8a Mon Sep 17 00:00:00 2001 From: andy840119 Date: Sun, 3 Jul 2022 22:42:41 +0800 Subject: [PATCH 01/46] Remove the nullable annotation in the catch ruleset. --- osu.Game.Rulesets.Catch/Mods/CatchModAutoplay.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModCinema.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModClassic.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModDaycore.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModDifficultyAdjust.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModDoubleTime.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModEasy.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModHalfTime.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModHardRock.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModMirror.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModMuted.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModNightcore.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModNoFail.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModNoScope.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModPerfect.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs | 2 -- osu.Game.Rulesets.Catch/Mods/CatchModSuddenDeath.cs | 2 -- 20 files changed, 40 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModAutoplay.cs b/osu.Game.Rulesets.Catch/Mods/CatchModAutoplay.cs index c5ca595fd6..50e48101d3 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModAutoplay.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModAutoplay.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using osu.Game.Beatmaps; using osu.Game.Rulesets.Catch.Replays; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModCinema.cs b/osu.Game.Rulesets.Catch/Mods/CatchModCinema.cs index 10a0809e05..7eda6b37d3 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModCinema.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModCinema.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using osu.Game.Beatmaps; using osu.Game.Rulesets.Catch.Objects; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModClassic.cs b/osu.Game.Rulesets.Catch/Mods/CatchModClassic.cs index 904656993e..9624e84018 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModClassic.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModClassic.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModDaycore.cs b/osu.Game.Rulesets.Catch/Mods/CatchModDaycore.cs index 8d4b57c244..cae19e9468 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModDaycore.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModDaycore.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModDifficultyAdjust.cs b/osu.Game.Rulesets.Catch/Mods/CatchModDifficultyAdjust.cs index 6927d7953f..e59a0a0431 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModDifficultyAdjust.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModDifficultyAdjust.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Linq; using osu.Framework.Bindables; using osu.Game.Beatmaps; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModDoubleTime.cs b/osu.Game.Rulesets.Catch/Mods/CatchModDoubleTime.cs index c58ce9b07d..57c06e1cd1 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModDoubleTime.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModDoubleTime.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModEasy.cs b/osu.Game.Rulesets.Catch/Mods/CatchModEasy.cs index bea9b094fa..16ef56d845 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModEasy.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModEasy.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs index d166646eaf..7f7c04fb52 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Game.Configuration; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs b/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs index 1fe892c9b5..63203dd57c 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Game.Rulesets.Catch.Objects; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModHalfTime.cs b/osu.Game.Rulesets.Catch/Mods/CatchModHalfTime.cs index 0c7886be10..ce06b841aa 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModHalfTime.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModHalfTime.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModHardRock.cs b/osu.Game.Rulesets.Catch/Mods/CatchModHardRock.cs index 39b992b3f5..93eadcc13e 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModHardRock.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModHardRock.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; using osu.Game.Beatmaps; using osu.Game.Rulesets.Catch.Beatmaps; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs b/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs index b4fbc9d566..51516edacd 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModHidden.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Linq; using osu.Framework.Graphics; using osu.Game.Rulesets.Catch.Objects; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModMirror.cs b/osu.Game.Rulesets.Catch/Mods/CatchModMirror.cs index 89fc40356d..a97e940a64 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModMirror.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModMirror.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Linq; using osu.Game.Beatmaps; using osu.Game.Rulesets.Catch.Beatmaps; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModMuted.cs b/osu.Game.Rulesets.Catch/Mods/CatchModMuted.cs index 6b28d1a127..6d2565440a 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModMuted.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModMuted.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Mods; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModNightcore.cs b/osu.Game.Rulesets.Catch/Mods/CatchModNightcore.cs index 1fd2227eb7..9e38913be7 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModNightcore.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModNightcore.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Mods; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModNoFail.cs b/osu.Game.Rulesets.Catch/Mods/CatchModNoFail.cs index 89e7e4bcd6..3c02646e99 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModNoFail.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModNoFail.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModNoScope.cs b/osu.Game.Rulesets.Catch/Mods/CatchModNoScope.cs index 385d4c50c0..a24a6227fe 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModNoScope.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModNoScope.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using osu.Framework.Bindables; using osu.Game.Rulesets.Mods; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModPerfect.cs b/osu.Game.Rulesets.Catch/Mods/CatchModPerfect.cs index 0a74ee4fbb..fb92399102 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModPerfect.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModPerfect.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs index f4d6fb9ab3..d0a94767d1 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Graphics; using osu.Framework.Input; using osu.Framework.Input.Bindings; diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModSuddenDeath.cs b/osu.Game.Rulesets.Catch/Mods/CatchModSuddenDeath.cs index d98829137c..68e01391ce 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModSuddenDeath.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModSuddenDeath.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Catch.Mods From 91bc7b9381093ca30fb0e501be9a7b6dad4440d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=82=BA=E4=BB=80=E9=BA=BC?= Date: Sun, 10 Jul 2022 23:18:27 +0800 Subject: [PATCH 02/46] Mark the class as non-nullable. Not the safe way but got no better idea. --- osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs | 2 +- osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs index 7f7c04fb52..abe391ba4e 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Catch.Mods protected override Flashlight CreateFlashlight() => new CatchFlashlight(this, playfield); - private CatchPlayfield playfield; + private CatchPlayfield playfield = null!; public override void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs index d0a94767d1..60f1614d98 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs @@ -18,7 +18,7 @@ namespace osu.Game.Rulesets.Catch.Mods { public override string Description => @"Use the mouse to control the catcher."; - private DrawableRuleset drawableRuleset; + private DrawableRuleset drawableRuleset = null!; public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { From 6a096cf11fad6feb5847266a7f66bb113573b7c6 Mon Sep 17 00:00:00 2001 From: andy840119 Date: Wed, 20 Jul 2022 20:30:04 +0800 Subject: [PATCH 03/46] Remove nullable disable annotation in the Catch test case. --- osu.Game.Rulesets.Catch.Tests/Mods/CatchModMirrorTest.cs | 2 -- osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModNoScope.cs | 2 -- osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModPerfect.cs | 2 -- osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModRelax.cs | 2 -- 4 files changed, 8 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/Mods/CatchModMirrorTest.cs b/osu.Game.Rulesets.Catch.Tests/Mods/CatchModMirrorTest.cs index 19321a48b9..fbbfee6b60 100644 --- a/osu.Game.Rulesets.Catch.Tests/Mods/CatchModMirrorTest.cs +++ b/osu.Game.Rulesets.Catch.Tests/Mods/CatchModMirrorTest.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using NUnit.Framework; using osu.Framework.Utils; diff --git a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModNoScope.cs b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModNoScope.cs index ffc5734f01..bbe543e73e 100644 --- a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModNoScope.cs +++ b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModNoScope.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using NUnit.Framework; using osu.Framework.Utils; diff --git a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModPerfect.cs b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModPerfect.cs index 886822f9a5..3e06e78dba 100644 --- a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModPerfect.cs +++ b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModPerfect.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using NUnit.Framework; using osu.Game.Rulesets.Catch.Mods; using osu.Game.Rulesets.Catch.Objects; diff --git a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModRelax.cs b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModRelax.cs index 3209be12d5..c01aff0aa0 100644 --- a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModRelax.cs +++ b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModRelax.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using System.Linq; using NUnit.Framework; From 9c2f6103c524edf3e5cbfc118da7bbbcdf35fa4e Mon Sep 17 00:00:00 2001 From: andy840119 Date: Thu, 21 Jul 2022 19:30:04 +0800 Subject: [PATCH 04/46] Following the suggestion to mark the property as nullable. --- osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs | 5 +++-- osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs index abe391ba4e..4824106c55 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModFlashlight.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Game.Configuration; using osu.Game.Rulesets.Catch.Objects; @@ -35,9 +36,9 @@ namespace osu.Game.Rulesets.Catch.Mods public override float DefaultFlashlightSize => 350; - protected override Flashlight CreateFlashlight() => new CatchFlashlight(this, playfield); + protected override Flashlight CreateFlashlight() => new CatchFlashlight(this, playfield.AsNonNull()); - private CatchPlayfield playfield = null!; + private CatchPlayfield? playfield; public override void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs index 60f1614d98..0ab6da0363 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModRelax.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Diagnostics; using osu.Framework.Graphics; using osu.Framework.Input; using osu.Framework.Input.Bindings; @@ -18,7 +19,7 @@ namespace osu.Game.Rulesets.Catch.Mods { public override string Description => @"Use the mouse to control the catcher."; - private DrawableRuleset drawableRuleset = null!; + private DrawableRuleset? drawableRuleset; public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { @@ -27,6 +28,8 @@ namespace osu.Game.Rulesets.Catch.Mods public void ApplyToPlayer(Player player) { + Debug.Assert(drawableRuleset != null); + if (!drawableRuleset.HasReplayLoaded.Value) drawableRuleset.Cursor.Add(new MouseInputHelper((CatchPlayfield)drawableRuleset.Playfield)); } From fcf767e28bc4d68d97a0137358016261a7255cee Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Mon, 25 Jul 2022 04:07:33 -0400 Subject: [PATCH 05/46] Add contextmenu to beatmap external link --- .../UserInterface/ExternalLinkButton.cs | 17 +- .../BeatmapSet/BeatmapSetHeaderContent.cs | 184 +++++++++--------- 2 files changed, 110 insertions(+), 91 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs index ae286f5092..4c54e45a50 100644 --- a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs +++ b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs @@ -3,10 +3,12 @@ #nullable disable +using System.Collections.Generic; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.UserInterface; using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Events; using osu.Framework.Localisation; @@ -16,7 +18,7 @@ using osuTK.Graphics; namespace osu.Game.Graphics.UserInterface { - public class ExternalLinkButton : CompositeDrawable, IHasTooltip + public class ExternalLinkButton : CompositeDrawable, IHasTooltip, IHasContextMenu { public string Link { get; set; } @@ -41,6 +43,19 @@ namespace osu.Game.Graphics.UserInterface new HoverClickSounds() }; } + + public MenuItem[] ContextMenuItems + { + get + { + List items = new List + { + new OsuMenuItem("Copy URL", MenuItemType.Standard, () => host.GetClipboard()?.SetText(Link)) + }; + + return items.ToArray(); + } + } [BackgroundDependencyLoader] private void load(OsuColour colours) diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index c9e97d5f2f..85b98a92e2 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -9,6 +9,7 @@ using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; +using osu.Game.Graphics.Cursor; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Localisation; @@ -90,113 +91,116 @@ namespace osu.Game.Overlays.BeatmapSet }, }, }, - new Container + new OsuContextMenuContainer { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Padding = new MarginPadding + RelativeSizeAxes = Axes.Both, + Child = new Container { - Vertical = BeatmapSetOverlay.Y_PADDING, - Left = BeatmapSetOverlay.X_PADDING, - Right = BeatmapSetOverlay.X_PADDING + BeatmapSetOverlay.RIGHT_WIDTH, - }, - Children = new Drawable[] - { - fadeContent = new FillFlowContainer + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Padding = new MarginPadding { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Direction = FillDirection.Vertical, - Children = new Drawable[] + Vertical = BeatmapSetOverlay.Y_PADDING, + Left = BeatmapSetOverlay.X_PADDING, + Right = BeatmapSetOverlay.X_PADDING + BeatmapSetOverlay.RIGHT_WIDTH, + }, + Children = new Drawable[] + { + fadeContent = new FillFlowContainer { - new Container + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Vertical, + Children = new Drawable[] { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Child = Picker = new BeatmapPicker(), - }, - new FillFlowContainer - { - Direction = FillDirection.Horizontal, - AutoSizeAxes = Axes.Both, - Margin = new MarginPadding { Top = 15 }, - Children = new Drawable[] + new Container { - title = new OsuSpriteText + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Child = Picker = new BeatmapPicker(), + }, + new FillFlowContainer + { + Direction = FillDirection.Horizontal, + AutoSizeAxes = Axes.Both, + Margin = new MarginPadding { Top = 15 }, + Children = new Drawable[] { - Font = OsuFont.GetFont(size: 30, weight: FontWeight.SemiBold, italics: true) - }, - externalLink = new ExternalLinkButton - { - Anchor = Anchor.BottomLeft, - Origin = Anchor.BottomLeft, - Margin = new MarginPadding { Left = 5, Bottom = 4 }, // To better lineup with the font - }, - explicitContent = new ExplicitContentBeatmapBadge - { - Alpha = 0f, - Anchor = Anchor.BottomLeft, - Origin = Anchor.BottomLeft, - Margin = new MarginPadding { Left = 10, Bottom = 4 }, - }, - spotlight = new SpotlightBeatmapBadge - { - Alpha = 0f, - Anchor = Anchor.BottomLeft, - Origin = Anchor.BottomLeft, - Margin = new MarginPadding { Left = 10, Bottom = 4 }, + title = new OsuSpriteText + { + Font = OsuFont.GetFont(size: 30, weight: FontWeight.SemiBold, italics: true) + }, + externalLink = new ExternalLinkButton { + Anchor = Anchor.BottomLeft, + Origin = Anchor.BottomLeft, + Margin = new MarginPadding { Left = 5, Bottom = 4 }, // To better lineup with the font + }, + explicitContent = new ExplicitContentBeatmapBadge + { + Alpha = 0f, + Anchor = Anchor.BottomLeft, + Origin = Anchor.BottomLeft, + Margin = new MarginPadding { Left = 10, Bottom = 4 }, + }, + spotlight = new SpotlightBeatmapBadge + { + Alpha = 0f, + Anchor = Anchor.BottomLeft, + Origin = Anchor.BottomLeft, + Margin = new MarginPadding { Left = 10, Bottom = 4 }, + } } - } - }, - new FillFlowContainer - { - Direction = FillDirection.Horizontal, - AutoSizeAxes = Axes.Both, - Margin = new MarginPadding { Bottom = 20 }, - Children = new Drawable[] + }, + new FillFlowContainer { - artist = new OsuSpriteText + Direction = FillDirection.Horizontal, + AutoSizeAxes = Axes.Both, + Margin = new MarginPadding { Bottom = 20 }, + Children = new Drawable[] { - Font = OsuFont.GetFont(size: 20, weight: FontWeight.Medium, italics: true), - }, - featuredArtist = new FeaturedArtistBeatmapBadge - { - Alpha = 0f, - Anchor = Anchor.BottomLeft, - Origin = Anchor.BottomLeft, - Margin = new MarginPadding { Left = 10 } + artist = new OsuSpriteText + { + Font = OsuFont.GetFont(size: 20, weight: FontWeight.Medium, italics: true), + }, + featuredArtist = new FeaturedArtistBeatmapBadge + { + Alpha = 0f, + Anchor = Anchor.BottomLeft, + Origin = Anchor.BottomLeft, + Margin = new MarginPadding { Left = 10 } + } } - } - }, - new Container - { - RelativeSizeAxes = Axes.X, - AutoSizeAxes = Axes.Y, - Child = author = new AuthorInfo(), - }, - beatmapAvailability = new BeatmapAvailability(), - new Container - { - RelativeSizeAxes = Axes.X, - Height = buttons_height, - Margin = new MarginPadding { Top = 10 }, - Children = new Drawable[] + }, + new Container { - favouriteButton = new FavouriteButton + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Child = author = new AuthorInfo(), + }, + beatmapAvailability = new BeatmapAvailability(), + new Container + { + RelativeSizeAxes = Axes.X, + Height = buttons_height, + Margin = new MarginPadding { Top = 10 }, + Children = new Drawable[] { - BeatmapSet = { BindTarget = BeatmapSet } - }, - downloadButtonsContainer = new FillFlowContainer - { - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Left = buttons_height + buttons_spacing }, - Spacing = new Vector2(buttons_spacing), + favouriteButton = new FavouriteButton + { + BeatmapSet = { BindTarget = BeatmapSet } + }, + downloadButtonsContainer = new FillFlowContainer + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Left = buttons_height + buttons_spacing }, + Spacing = new Vector2(buttons_spacing), + }, }, }, }, }, - }, - } + } + }, }, loading = new LoadingSpinner { From f1534da683623e65f74019d62983d28bd2391a32 Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Mon, 25 Jul 2022 04:13:05 -0400 Subject: [PATCH 06/46] Formatting issues --- osu.Game/Graphics/UserInterface/ExternalLinkButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs index 4c54e45a50..e813e2d8e8 100644 --- a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs +++ b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs @@ -43,7 +43,7 @@ namespace osu.Game.Graphics.UserInterface new HoverClickSounds() }; } - + public MenuItem[] ContextMenuItems { get From 93175eaf6efda8c782557c8624b4cc23f2075c66 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 25 Jul 2022 11:39:23 +0300 Subject: [PATCH 07/46] Re-enable timeline zoom test and remove flaky attribute --- .../Visual/Editing/TestSceneTimelineZoom.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs index 09d753ba41..11ac102814 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs @@ -8,22 +8,11 @@ using osu.Framework.Graphics; namespace osu.Game.Tests.Visual.Editing { - [Ignore("Timeline initialisation is kinda broken.")] // Initial work to rectify this was done in https://github.com/ppy/osu/pull/19297, but needs more massaging to work. public class TestSceneTimelineZoom : TimelineTestScene { public override Drawable CreateTestComponent() => Empty(); [Test] - [FlakyTest] - /* - * Fail rate around 0.3% - * - * TearDown : osu.Framework.Testing.Drawables.Steps.AssertButton+TracedException : range halved - * --TearDown - * at osu.Framework.Threading.ScheduledDelegate.RunTaskInternal() - * at osu.Framework.Threading.Scheduler.Update() - * at osu.Framework.Graphics.Drawable.UpdateSubTree() - */ public void TestVisibleRangeUpdatesOnZoomChange() { double initialVisibleRange = 0; From 123930306bc0146f52273f66e26f5c2c5e48f6e2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 25 Jul 2022 11:54:10 +0300 Subject: [PATCH 08/46] Refactor `ZoomableScrollContainer` to allow setting up zoom range and initial zoom after load --- .../TestSceneZoomableScrollContainer.cs | 17 +--- .../Timeline/ZoomableScrollContainer.cs | 99 ++++++++++--------- 2 files changed, 56 insertions(+), 60 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs index 9dc403814b..ce418f33f0 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs @@ -46,7 +46,7 @@ namespace osu.Game.Tests.Visual.Editing RelativeSizeAxes = Axes.Both, Colour = OsuColour.Gray(30) }, - scrollContainer = new ZoomableScrollContainer + scrollContainer = new ZoomableScrollContainer(1, 60, 1) { Anchor = Anchor.Centre, Origin = Anchor.Centre, @@ -80,21 +80,6 @@ namespace osu.Game.Tests.Visual.Editing AddAssert("Inner container width matches scroll container", () => innerBox.DrawWidth == scrollContainer.DrawWidth); } - [Test] - public void TestZoomRangeUpdate() - { - AddStep("set zoom to 2", () => scrollContainer.Zoom = 2); - AddStep("set min zoom to 5", () => scrollContainer.MinZoom = 5); - AddAssert("zoom = 5", () => scrollContainer.Zoom == 5); - - AddStep("set max zoom to 10", () => scrollContainer.MaxZoom = 10); - AddAssert("zoom = 5", () => scrollContainer.Zoom == 5); - - AddStep("set min zoom to 20", () => scrollContainer.MinZoom = 20); - AddStep("set max zoom to 40", () => scrollContainer.MaxZoom = 40); - AddAssert("zoom = 20", () => scrollContainer.Zoom == 20); - } - [Test] public void TestZoom0() { diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs index 83fd1dea2b..fb2297e88c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs @@ -32,19 +32,27 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline private readonly Container zoomedContent; protected override Container Content => zoomedContent; - private float currentZoom = 1; /// - /// The current zoom level of . - /// It may differ from during transitions. + /// The current zoom level of . + /// It may differ from during transitions. /// - public float CurrentZoom => currentZoom; + public float CurrentZoom { get; private set; } = 1; + + private bool isZoomSetUp; [Resolved(canBeNull: true)] private IFrameBasedClock editorClock { get; set; } private readonly LayoutValue zoomedContentWidthCache = new LayoutValue(Invalidation.DrawSize); + private float minZoom; + private float maxZoom; + + /// + /// Creates a with no zoom range. + /// Functionality will be disabled until zoom is set up via . + /// public ZoomableScrollContainer() : base(Direction.Horizontal) { @@ -53,46 +61,36 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline AddLayout(zoomedContentWidthCache); } - private float minZoom = 1; - /// - /// The minimum zoom level allowed. + /// Creates a with a defined zoom range. /// - public float MinZoom + public ZoomableScrollContainer(float minimum, float maximum, float initial) + : this() { - get => minZoom; - set - { - if (value < 1) - throw new ArgumentException($"{nameof(MinZoom)} must be >= 1.", nameof(value)); - - minZoom = value; - - // ensure zoom range is in valid state before updating zoom. - if (MinZoom < MaxZoom) - updateZoom(); - } + SetupZoom(initial, minimum, maximum); } - private float maxZoom = 60; - /// - /// The maximum zoom level allowed. + /// Sets up the minimum and maximum range of this zoomable scroll container, along with the initial zoom value. /// - public float MaxZoom + /// The initial zoom value, applied immediately. + /// The minimum zoom value. + /// The maximum zoom value. + public void SetupZoom(float initial, float minimum, float maximum) { - get => maxZoom; - set - { - if (value < 1) - throw new ArgumentException($"{nameof(MaxZoom)} must be >= 1.", nameof(value)); + if (minimum < 1) + throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be >= 1.", nameof(maximum)); - maxZoom = value; + if (maximum < 1) + throw new ArgumentException($"{nameof(maximum)} ({maximum}) must be >= 1.", nameof(maximum)); - // ensure zoom range is in valid state before updating zoom. - if (MaxZoom > MinZoom) - updateZoom(); - } + if (minimum > maximum) + throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be less than {nameof(maximum)} ({maximum})"); + + minZoom = minimum; + maxZoom = maximum; + CurrentZoom = zoomTarget = initial; + isZoomSetUp = true; } /// @@ -104,14 +102,17 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline set => updateZoom(value); } - private void updateZoom(float? value = null) + private void updateZoom(float value) { - float newZoom = Math.Clamp(value ?? Zoom, MinZoom, MaxZoom); + if (!isZoomSetUp) + return; + + float newZoom = Math.Clamp(value, minZoom, maxZoom); if (IsLoaded) setZoomTarget(newZoom, ToSpaceOfOtherDrawable(new Vector2(DrawWidth / 2, 0), zoomedContent).X); else - currentZoom = zoomTarget = newZoom; + CurrentZoom = zoomTarget = newZoom; } protected override void Update() @@ -141,22 +142,32 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline private void updateZoomedContentWidth() { - zoomedContent.Width = DrawWidth * currentZoom; + zoomedContent.Width = DrawWidth * CurrentZoom; zoomedContentWidthCache.Validate(); } public void AdjustZoomRelatively(float change, float? focusPoint = null) { + if (!isZoomSetUp) + return; + const float zoom_change_sensitivity = 0.02f; - setZoomTarget(zoomTarget + change * (MaxZoom - minZoom) * zoom_change_sensitivity, focusPoint); + setZoomTarget(zoomTarget + change * (maxZoom - minZoom) * zoom_change_sensitivity, focusPoint); + } + + protected void SetZoomImmediately(float value, float min, float max) + { + maxZoom = max; + minZoom = min; + CurrentZoom = zoomTarget = value; } private float zoomTarget = 1; private void setZoomTarget(float newZoom, float? focusPoint = null) { - zoomTarget = Math.Clamp(newZoom, MinZoom, MaxZoom); + zoomTarget = Math.Clamp(newZoom, minZoom, maxZoom); focusPoint ??= zoomedContent.ToLocalSpace(ToScreenSpace(new Vector2(DrawWidth / 2, 0))).X; transformZoomTo(zoomTarget, focusPoint.Value, ZoomDuration, ZoomEasing); @@ -192,7 +203,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline private readonly float scrollOffset; /// - /// Transforms to a new value. + /// Transforms to a new value. /// /// The focus point in absolute coordinates local to the content. /// The size of the content. @@ -204,7 +215,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline this.scrollOffset = scrollOffset; } - public override string TargetMember => nameof(currentZoom); + public override string TargetMember => nameof(CurrentZoom); private float valueAt(double time) { @@ -222,7 +233,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline float expectedWidth = d.DrawWidth * newZoom; float targetOffset = expectedWidth * (focusPoint / contentSize) - focusOffset; - d.currentZoom = newZoom; + d.CurrentZoom = newZoom; d.updateZoomedContentWidth(); // Temporarily here to make sure ScrollTo gets the correct DrawSize for scrollable area. @@ -231,7 +242,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline d.ScrollTo(targetOffset, false); } - protected override void ReadIntoStartValue(ZoomableScrollContainer d) => StartValue = d.currentZoom; + protected override void ReadIntoStartValue(ZoomableScrollContainer d) => StartValue = d.CurrentZoom; } } } From 07c6b4486491848b358f1a2e8c8de56523cc62e3 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 25 Jul 2022 11:54:47 +0300 Subject: [PATCH 09/46] Fix `Timeline` attempting to setup zoom with unloaded track --- .../Compose/Components/Timeline/Timeline.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs index bbe011a2e0..54f2d13707 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs @@ -146,13 +146,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline waveform.Waveform = b.NewValue.Waveform; track = b.NewValue.Track; - // todo: i don't think this is safe, the track may not be loaded yet. - if (track.Length > 0) - { - MaxZoom = getZoomLevelForVisibleMilliseconds(500); - MinZoom = getZoomLevelForVisibleMilliseconds(10000); - defaultTimelineZoom = getZoomLevelForVisibleMilliseconds(6000); - } + setupTimelineZoom(); }, true); Zoom = (float)(defaultTimelineZoom * editorBeatmap.BeatmapInfo.TimelineZoom); @@ -205,6 +199,20 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline scrollToTrackTime(); } + private void setupTimelineZoom() + { + if (!track.IsLoaded) + { + Scheduler.AddOnce(setupTimelineZoom); + return; + } + + defaultTimelineZoom = getZoomLevelForVisibleMilliseconds(6000); + + float initialZoom = (float)(defaultTimelineZoom * editorBeatmap.BeatmapInfo.TimelineZoom); + SetupZoom(initialZoom, getZoomLevelForVisibleMilliseconds(10000), getZoomLevelForVisibleMilliseconds(500)); + } + protected override bool OnScroll(ScrollEvent e) { // if this is not a precision scroll event, let the editor handle the seek itself (for snapping support) From bc2b629ee7c89ec71dfbac7d11bd36adfdc4bb8a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 25 Jul 2022 11:43:34 +0300 Subject: [PATCH 10/46] Let tests wait until track load before testing zoom --- osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs index 11ac102814..630d048867 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs @@ -17,6 +17,8 @@ namespace osu.Game.Tests.Visual.Editing { double initialVisibleRange = 0; + AddUntilStep("wait for load", () => MusicController.TrackLoaded); + AddStep("reset zoom", () => TimelineArea.Timeline.Zoom = 100); AddStep("get initial range", () => initialVisibleRange = TimelineArea.Timeline.VisibleRange); @@ -34,6 +36,8 @@ namespace osu.Game.Tests.Visual.Editing { double initialVisibleRange = 0; + AddUntilStep("wait for load", () => MusicController.TrackLoaded); + AddStep("reset timeline size", () => TimelineArea.Timeline.Width = 1); AddStep("get initial range", () => initialVisibleRange = TimelineArea.Timeline.VisibleRange); From 48bcf57066f1b5f67e0849a3cac9a9c8e4feb8bc Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 25 Jul 2022 12:06:54 +0300 Subject: [PATCH 11/46] Mark `SetupZoom` and parameterless `ZoomableScrollContainer` ctor as protected --- .../Compose/Components/Timeline/ZoomableScrollContainer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs index fb2297e88c..725f94e7db 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline /// Creates a with no zoom range. /// Functionality will be disabled until zoom is set up via . /// - public ZoomableScrollContainer() + protected ZoomableScrollContainer() : base(Direction.Horizontal) { base.Content.Add(zoomedContent = new Container { RelativeSizeAxes = Axes.Y }); @@ -76,7 +76,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline /// The initial zoom value, applied immediately. /// The minimum zoom value. /// The maximum zoom value. - public void SetupZoom(float initial, float minimum, float maximum) + protected void SetupZoom(float initial, float minimum, float maximum) { if (minimum < 1) throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be >= 1.", nameof(maximum)); From ec477a3ebfd74b2cdfa0444c23644d710c422df8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 17:18:28 +0900 Subject: [PATCH 12/46] Add basic coverage of current behaviour of beatmap reimport --- .../Database/BeatmapImporterTests.cs | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 9ee88c0670..076be8339a 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -670,6 +670,61 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestImportThenReimportWithNewDifficulty() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var store = new RealmRulesetStore(realm, storage); + + string? pathOriginal = TestResources.GetTestBeatmapForImport(); + + string pathMissingOneBeatmap = pathOriginal.Replace(".osz", "_missing_difficulty.osz"); + + string extractedFolder = $"{pathOriginal}_extracted"; + Directory.CreateDirectory(extractedFolder); + + try + { + using (var zip = ZipArchive.Open(pathOriginal)) + zip.WriteToDirectory(extractedFolder); + + // remove one difficulty before first import + new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).Delete(); + + using (var zip = ZipArchive.Create()) + { + zip.AddAllFromDirectory(extractedFolder); + zip.SaveTo(pathMissingOneBeatmap, new ZipWriterOptions(CompressionType.Deflate)); + } + + var firstImport = await importer.Import(new ImportTask(pathMissingOneBeatmap)); + Assert.That(firstImport, Is.Not.Null); + + Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); + Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); + + // Second import matches first but contains one extra .osu file. + var secondImport = await importer.Import(new ImportTask(pathOriginal)); + Assert.That(secondImport, Is.Not.Null); + + Assert.That(realm.Realm.All(), Has.Count.EqualTo(23)); + Assert.That(realm.Realm.All(), Has.Count.EqualTo(2)); + + Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); + Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(12)); + + // check the newly "imported" beatmap is not the original. + Assert.That(firstImport?.ID, Is.Not.EqualTo(secondImport?.ID)); + } + finally + { + Directory.Delete(extractedFolder, true); + } + }); + } + [Test] public void TestImportThenReimportAfterMissingFiles() { From 6a3e8e31de7587b2d6859c7bbb9964531424cfaf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 18:51:19 +0900 Subject: [PATCH 13/46] Centralise calls to reset online info of a `BeatmapInfo` --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 2 +- .../Visual/Gameplay/TestScenePlayerScoreSubmission.cs | 2 +- osu.Game/Beatmaps/BeatmapImporter.cs | 4 ++-- osu.Game/Beatmaps/BeatmapInfo.cs | 11 +++++++++++ osu.Game/Beatmaps/BeatmapManager.cs | 3 +-- osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs | 4 ++-- 6 files changed, 18 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 076be8339a..66384e8cd1 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -797,7 +797,7 @@ namespace osu.Game.Tests.Database await realm.Realm.WriteAsync(() => { foreach (var b in imported.Beatmaps) - b.OnlineID = -1; + b.ResetOnlineInfo(); }); deleteBeatmapSet(imported, realm.Realm); diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs index 96efca6b65..d1bdfb1dfa 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs @@ -239,7 +239,7 @@ namespace osu.Game.Tests.Visual.Gameplay createPlayerTest(false, r => { var beatmap = createTestBeatmap(r); - beatmap.BeatmapInfo.OnlineID = -1; + beatmap.BeatmapInfo.ResetOnlineInfo(); return beatmap; }); diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 3e4d01a9a3..2fdaeb9eed 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -87,7 +87,7 @@ namespace osu.Game.Beatmaps existingSetWithSameOnlineID.OnlineID = -1; foreach (var b in existingSetWithSameOnlineID.Beatmaps) - b.OnlineID = -1; + b.ResetOnlineInfo(); LogForModel(beatmapSet, $"Found existing beatmap set with same OnlineID ({beatmapSet.OnlineID}). It will be disassociated and marked for deletion."); } @@ -133,7 +133,7 @@ namespace osu.Game.Beatmaps } } - void resetIds() => beatmapSet.Beatmaps.ForEach(b => b.OnlineID = -1); + void resetIds() => beatmapSet.Beatmaps.ForEach(b => b.ResetOnlineInfo()); } protected override bool CanSkipImport(BeatmapSetInfo existing, BeatmapSetInfo import) diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 466448d771..66c1995c8b 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -109,6 +109,17 @@ namespace osu.Game.Beatmaps [JsonIgnore] public bool Hidden { get; set; } + /// + /// Reset any fetched online linking information (and history). + /// + public void ResetOnlineInfo() + { + OnlineID = -1; + LastOnlineUpdate = null; + OnlineMD5Hash = string.Empty; + Status = BeatmapOnlineStatus.None; + } + #region Properties we may not want persisted (but also maybe no harm?) public double AudioLeadIn { get; set; } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 7717c9cc87..62edd6e8da 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -164,8 +164,7 @@ namespace osu.Game.Beatmaps // clear the hash, as that's what is used to match .osu files with their corresponding realm beatmaps. newBeatmapInfo.Hash = string.Empty; // clear online properties. - newBeatmapInfo.OnlineID = -1; - newBeatmapInfo.Status = BeatmapOnlineStatus.None; + newBeatmapInfo.ResetOnlineInfo(); return addDifficultyToSet(targetBeatmapSet, newBeatmap, referenceWorkingBeatmap.Skin); } diff --git a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs b/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs index d3be240d4c..6a3383cc92 100644 --- a/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs +++ b/osu.Game/Beatmaps/BeatmapOnlineLookupQueue.cs @@ -88,7 +88,7 @@ namespace osu.Game.Beatmaps if (req.CompletionState == APIRequestCompletionState.Failed) { logForModel(set, $"Online retrieval failed for {beatmapInfo}"); - beatmapInfo.OnlineID = -1; + beatmapInfo.ResetOnlineInfo(); return; } @@ -118,7 +118,7 @@ namespace osu.Game.Beatmaps catch (Exception e) { logForModel(set, $"Online retrieval failed for {beatmapInfo} ({e.Message})"); - beatmapInfo.OnlineID = -1; + beatmapInfo.ResetOnlineInfo(); } } From b7f6413bcee7c4381fbbe30f9725bf144248eda8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 18:52:53 +0900 Subject: [PATCH 14/46] Fix old version of beatmap potentially not being deleted during update flow This can happen if the online IDs are not present in the `.osu` files. Previously this was only working due to the early logic in the import process (that relies on matching all online IDs perfectly). --- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 20 +++++++++++++++++++ osu.Game/Database/ModelDownloader.cs | 8 ++++++-- .../Select/Carousel/UpdateBeatmapSetButton.cs | 2 +- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index 4295def5c3..8c037b6382 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Logging; using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -19,5 +20,24 @@ namespace osu.Game.Beatmaps : base(beatmapImporter, api) { } + + public bool Update(BeatmapSetInfo model) + { + return Download(model, false, onSuccess); + + void onSuccess(Live imported) + { + imported.PerformWrite(updated => + { + Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); + + var original = updated.Realm.Find(model.ID); + + // Generally the import process will do this for us if the OnlineIDs match, + // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). + original.DeletePending = true; + }); + } + } } } diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 02bcb342e4..8fa1c12d88 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -42,7 +42,9 @@ namespace osu.Game.Database /// The request object. protected abstract ArchiveDownloadRequest CreateDownloadRequest(T model, bool minimiseDownloadSize); - public bool Download(T model, bool minimiseDownloadSize = false) + public bool Download(T model, bool minimiseDownloadSize = false) => Download(model, minimiseDownloadSize, null); + + protected bool Download(T model, bool minimiseDownloadSize, Action>? onSuccess) { if (!canDownload(model)) return false; @@ -67,7 +69,9 @@ namespace osu.Game.Database var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); // for now a failed import will be marked as a failed download for simplicity. - if (!imported.Any()) + if (imported.Any()) + onSuccess?.Invoke(imported.Single()); + else DownloadFailed?.Invoke(request); CurrentDownloads.Remove(request); diff --git a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs index b80eb40018..3746c1975c 100644 --- a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs +++ b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs @@ -90,7 +90,7 @@ namespace osu.Game.Screens.Select.Carousel Action = () => { - beatmapDownloader.Download(beatmapSetInfo); + beatmapDownloader.Update(beatmapSetInfo); attachExistingDownload(); }; } From 912218e1231a18ebf7fc17e749f60963c90a4bf8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 18:54:33 +0900 Subject: [PATCH 15/46] Ensure scores are transferred after beatmap update if difficulty hash didn't change --- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index 8c037b6382..0110ce0c50 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using osu.Framework.Logging; using osu.Game.Database; using osu.Game.Online.API; @@ -36,6 +37,23 @@ namespace osu.Game.Beatmaps // Generally the import process will do this for us if the OnlineIDs match, // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). original.DeletePending = true; + + foreach (var beatmap in original.Beatmaps.ToArray()) + { + var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); + + if (updatedBeatmap != null) + { + // If the updated beatmap matches an existing one, transfer any user data across.. + if (beatmap.Scores.Any()) + { + Logger.Log($"Transferring {beatmap.Scores.Count()} scores for unchanged difficulty \"{beatmap}\"", LoggingTarget.Database); + + foreach (var score in beatmap.Scores) + score.BeatmapInfo = updatedBeatmap; + } + } + } }); } } From e5ad07454ca37dbe6bb57b03d3dfcb48da1504cc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 18:54:55 +0900 Subject: [PATCH 16/46] Ensure previous version prior to update loses online info after marked pending delete --- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index 0110ce0c50..7c48abbe66 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -52,6 +52,19 @@ namespace osu.Game.Beatmaps foreach (var score in beatmap.Scores) score.BeatmapInfo = updatedBeatmap; } + + // ..then nuke the old beatmap completely. + // this is done instead of a soft deletion to avoid a user potentially creating weird + // interactions, like restoring the outdated beatmap then updating a second time + // (causing user data to be wiped). + original.Beatmaps.Remove(beatmap); + } + else + { + // If the beatmap differs in the original, leave it in a soft-deleted state but reset online info. + // This caters to the case where a user has made modifications they potentially want to restore, + // but after restoring we want to ensure it can't be used to trigger an update of the beatmap. + beatmap.ResetOnlineInfo(); } } }); From 2363a3fb7b1d5b6a8cafbce199fde6e96636e942 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 18:55:08 +0900 Subject: [PATCH 17/46] Persist `DateAdded` over beatmap updates --- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index 7c48abbe66..a566aee49e 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -38,6 +38,9 @@ namespace osu.Game.Beatmaps // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). original.DeletePending = true; + // Transfer local values which should be persisted across a beatmap update. + updated.DateAdded = original.DateAdded; + foreach (var beatmap in original.Beatmaps.ToArray()) { var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); From 2e14d8730c397f86cf5ad75cc6b7bd24ef73ffd7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 19:31:46 +0900 Subject: [PATCH 18/46] Move implementation of updating a beatmap to `BeatmapImporter` --- ...eneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Beatmaps/BeatmapImporter.cs | 56 ++++++++++++++++ osu.Game/Beatmaps/BeatmapManager.cs | 3 + osu.Game/Beatmaps/BeatmapModelDownloader.cs | 64 ++++--------------- .../Drawables/BundledBeatmapDownloader.cs | 3 +- osu.Game/Database/ModelDownloader.cs | 20 ++++-- 6 files changed, 89 insertions(+), 59 deletions(-) diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 536322805b..0a980efbae 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -237,7 +237,7 @@ namespace osu.Game.Tests.Online internal class TestBeatmapModelDownloader : BeatmapModelDownloader { - public TestBeatmapModelDownloader(IModelImporter importer, IAPIProvider apiProvider) + public TestBeatmapModelDownloader(BeatmapManager importer, IAPIProvider apiProvider) : base(importer, apiProvider) { } diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 2fdaeb9eed..3cfeb0df67 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -3,9 +3,11 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; +using System.Threading.Tasks; using osu.Framework.Extensions; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Logging; @@ -16,6 +18,7 @@ using osu.Game.Database; using osu.Game.Extensions; using osu.Game.IO; using osu.Game.IO.Archives; +using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; using Realms; @@ -38,6 +41,59 @@ namespace osu.Game.Beatmaps { } + public async Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) + { + var imported = await Import(notification, importTask); + + Debug.Assert(imported.Count() == 1); + + imported.First().PerformWrite(updated => + { + Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); + + original = updated.Realm.Find(original.ID); + + // Generally the import process will do this for us if the OnlineIDs match, + // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). + original.DeletePending = true; + + // Transfer local values which should be persisted across a beatmap update. + updated.DateAdded = original.DateAdded; + + foreach (var beatmap in original.Beatmaps.ToArray()) + { + var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); + + if (updatedBeatmap != null) + { + // If the updated beatmap matches an existing one, transfer any user data across.. + if (beatmap.Scores.Any()) + { + Logger.Log($"Transferring {beatmap.Scores.Count()} scores for unchanged difficulty \"{beatmap}\"", LoggingTarget.Database); + + foreach (var score in beatmap.Scores) + score.BeatmapInfo = updatedBeatmap; + } + + // ..then nuke the old beatmap completely. + // this is done instead of a soft deletion to avoid a user potentially creating weird + // interactions, like restoring the outdated beatmap then updating a second time + // (causing user data to be wiped). + original.Beatmaps.Remove(beatmap); + } + else + { + // If the beatmap differs in the original, leave it in a soft-deleted state but reset online info. + // This caters to the case where a user has made modifications they potentially want to restore, + // but after restoring we want to ensure it can't be used to trigger an update of the beatmap. + beatmap.ResetOnlineInfo(); + } + } + }); + + return imported; + } + protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz"; protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 62edd6e8da..1b31d29c2b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -408,6 +408,9 @@ namespace osu.Game.Beatmaps Realm.Run(r => Undelete(r.All().Where(s => s.DeletePending).ToList())); } + public Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => + beatmapImporter.ImportAsUpdate(notification, importTask, original); + #region Implementation of ICanAcceptFiles public Task Import(params string[] paths) => beatmapImporter.Import(paths); diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index a566aee49e..e70168f1b3 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,77 +1,39 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Linq; -using osu.Framework.Logging; +using System.Collections.Generic; +using System.Threading.Tasks; using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using osu.Game.Overlays.Notifications; namespace osu.Game.Beatmaps { public class BeatmapModelDownloader : ModelDownloader { + private readonly BeatmapManager beatmapManager; + protected override ArchiveDownloadRequest CreateDownloadRequest(IBeatmapSetInfo set, bool minimiseDownloadSize) => new DownloadBeatmapSetRequest(set, minimiseDownloadSize); public override ArchiveDownloadRequest? GetExistingDownload(IBeatmapSetInfo model) => CurrentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); - public BeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) + public BeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { + beatmapManager = beatmapImporter; } - public bool Update(BeatmapSetInfo model) + protected override Task>> Import(ProgressNotification notification, string filename, BeatmapSetInfo? originalModel) { - return Download(model, false, onSuccess); + if (originalModel != null) + return beatmapManager.ImportAsUpdate(notification, new ImportTask(filename), originalModel); - void onSuccess(Live imported) - { - imported.PerformWrite(updated => - { - Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); - - var original = updated.Realm.Find(model.ID); - - // Generally the import process will do this for us if the OnlineIDs match, - // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). - original.DeletePending = true; - - // Transfer local values which should be persisted across a beatmap update. - updated.DateAdded = original.DateAdded; - - foreach (var beatmap in original.Beatmaps.ToArray()) - { - var updatedBeatmap = updated.Beatmaps.FirstOrDefault(b => b.Hash == beatmap.Hash); - - if (updatedBeatmap != null) - { - // If the updated beatmap matches an existing one, transfer any user data across.. - if (beatmap.Scores.Any()) - { - Logger.Log($"Transferring {beatmap.Scores.Count()} scores for unchanged difficulty \"{beatmap}\"", LoggingTarget.Database); - - foreach (var score in beatmap.Scores) - score.BeatmapInfo = updatedBeatmap; - } - - // ..then nuke the old beatmap completely. - // this is done instead of a soft deletion to avoid a user potentially creating weird - // interactions, like restoring the outdated beatmap then updating a second time - // (causing user data to be wiped). - original.Beatmaps.Remove(beatmap); - } - else - { - // If the beatmap differs in the original, leave it in a soft-deleted state but reset online info. - // This caters to the case where a user has made modifications they potentially want to restore, - // but after restoring we want to ensure it can't be used to trigger an update of the beatmap. - beatmap.ResetOnlineInfo(); - } - } - }); - } + return base.Import(notification, filename, null); } + + public bool Update(BeatmapSetInfo model) => Download(model, false, model); } } diff --git a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs index 80af4108c7..bcbe7084d5 100644 --- a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs +++ b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs @@ -11,7 +11,6 @@ using System.Text.RegularExpressions; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Game.Database; using osu.Game.Online; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -109,7 +108,7 @@ namespace osu.Game.Beatmaps.Drawables private class BundledBeatmapModelDownloader : BeatmapModelDownloader { - public BundledBeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) + public BundledBeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { } diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 8fa1c12d88..5fd124cafc 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -44,7 +44,7 @@ namespace osu.Game.Database public bool Download(T model, bool minimiseDownloadSize = false) => Download(model, minimiseDownloadSize, null); - protected bool Download(T model, bool minimiseDownloadSize, Action>? onSuccess) + protected bool Download(T model, bool minimiseDownloadSize, TModel? originalModel) { if (!canDownload(model)) return false; @@ -66,12 +66,10 @@ namespace osu.Game.Database Task.Factory.StartNew(async () => { // This gets scheduled back to the update thread, but we want the import to run in the background. - var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); + var imported = await Import(notification, filename, originalModel).ConfigureAwait(false); // for now a failed import will be marked as a failed download for simplicity. - if (imported.Any()) - onSuccess?.Invoke(imported.Single()); - else + if (!imported.Any()) DownloadFailed?.Invoke(request); CurrentDownloads.Remove(request); @@ -107,6 +105,18 @@ namespace osu.Game.Database } } + /// + /// Run the post-download import for the model. + /// + /// The notification to update. + /// The path of the temporary downloaded file. + /// An optional model for update scenarios, to be used as a reference. + /// The imported model. + protected virtual Task>> Import(ProgressNotification notification, string filename, TModel? originalModel) + { + return importer.Import(notification, new ImportTask(filename)); + } + public abstract ArchiveDownloadRequest? GetExistingDownload(T model); private bool canDownload(T model) => GetExistingDownload(model) == null && api != null; From 92dd1bcddb6dc3cfd669e721fc0f459a6b6d1e21 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 19:50:58 +0900 Subject: [PATCH 19/46] Add test coverage of actual update flow --- .../Database/BeatmapImporterTests.cs | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 66384e8cd1..784613ce3e 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -725,6 +725,60 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestImportThenReimportWithNewDifficultyAsUpdate() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var store = new RealmRulesetStore(realm, storage); + + string? pathOriginal = TestResources.GetTestBeatmapForImport(); + + string pathMissingOneBeatmap = pathOriginal.Replace(".osz", "_missing_difficulty.osz"); + + string extractedFolder = $"{pathOriginal}_extracted"; + Directory.CreateDirectory(extractedFolder); + + try + { + using (var zip = ZipArchive.Open(pathOriginal)) + zip.WriteToDirectory(extractedFolder); + + // remove one difficulty before first import + new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).Delete(); + + using (var zip = ZipArchive.Create()) + { + zip.AddAllFromDirectory(extractedFolder); + zip.SaveTo(pathMissingOneBeatmap, new ZipWriterOptions(CompressionType.Deflate)); + } + + var firstImport = await importer.Import(new ImportTask(pathMissingOneBeatmap)); + Assert.That(firstImport, Is.Not.Null); + Debug.Assert(firstImport != null); + + Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); + Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); + + // Second import matches first but contains one extra .osu file. + var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); + Assert.That(secondImport, Is.Not.Null); + + Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); + Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); + Assert.That(realm.Realm.All(), Has.Count.EqualTo(1)); + + // check the newly "imported" beatmap is not the original. + Assert.That(firstImport?.ID, Is.Not.EqualTo(secondImport?.ID)); + } + finally + { + Directory.Delete(extractedFolder, true); + } + }); + } + [Test] public void TestImportThenReimportAfterMissingFiles() { From 8a0c8f5fd874cbca96918c1f396ff31b2fb27b6e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 19:51:10 +0900 Subject: [PATCH 20/46] Fix some realm pieces not being cleaned up --- osu.Game/Beatmaps/BeatmapImporter.cs | 13 +++++++++++-- osu.Game/Database/RealmAccess.cs | 1 - 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 3cfeb0df67..104a36c788 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -49,9 +49,11 @@ namespace osu.Game.Beatmaps imported.First().PerformWrite(updated => { - Logger.Log($"Beatmap \"{updated}\"update completed successfully", LoggingTarget.Database); + var realm = updated.Realm; - original = updated.Realm.Find(original.ID); + Logger.Log($"Beatmap \"{updated}\" update completed successfully", LoggingTarget.Database); + + original = realm.Find(original.ID); // Generally the import process will do this for us if the OnlineIDs match, // but that isn't a guarantee (ie. if the .osu file doesn't have OnlineIDs populated). @@ -80,6 +82,9 @@ namespace osu.Game.Beatmaps // interactions, like restoring the outdated beatmap then updating a second time // (causing user data to be wiped). original.Beatmaps.Remove(beatmap); + + realm.Remove(beatmap.Metadata); + realm.Remove(beatmap); } else { @@ -89,6 +94,10 @@ namespace osu.Game.Beatmaps beatmap.ResetOnlineInfo(); } } + + // If the original has no beatmaps left, delete the set as well. + if (!original.Beatmaps.Any()) + realm.Remove(original); }); return imported; diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index 826341a5b9..a93fdea35b 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -278,7 +278,6 @@ namespace osu.Game.Database realm.Remove(score); realm.Remove(beatmap.Metadata); - realm.Remove(beatmap); } From 9c411c2250175f7cf4ecb34983bb356fd9deae93 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 20:06:36 +0900 Subject: [PATCH 21/46] Fix test nullability assertions --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 784613ce3e..3b1f82375c 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -764,13 +764,14 @@ namespace osu.Game.Tests.Database // Second import matches first but contains one extra .osu file. var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); Assert.That(secondImport, Is.Not.Null); + Debug.Assert(secondImport != null); Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); Assert.That(realm.Realm.All(), Has.Count.EqualTo(1)); // check the newly "imported" beatmap is not the original. - Assert.That(firstImport?.ID, Is.Not.EqualTo(secondImport?.ID)); + Assert.That(firstImport.ID, Is.Not.EqualTo(secondImport.ID)); } finally { From d41ac36a69c60c41b24ce20ece66a1a67ee2304e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Jul 2022 23:59:25 +0900 Subject: [PATCH 22/46] Fix scenario where import is expected to be empty --- osu.Game/Beatmaps/BeatmapImporter.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 104a36c788..2a6121b541 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -45,6 +45,9 @@ namespace osu.Game.Beatmaps { var imported = await Import(notification, importTask); + if (!imported.Any()) + return imported; + Debug.Assert(imported.Count() == 1); imported.First().PerformWrite(updated => From 46c4e784777b031a71e3c60374904d5fb15d1931 Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Mon, 25 Jul 2022 15:40:20 -0400 Subject: [PATCH 23/46] Add notification and another menuitem --- .../UserInterface/ExternalLinkButton.cs | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs index e813e2d8e8..c5e1e51e07 100644 --- a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs +++ b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs @@ -13,6 +13,8 @@ using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Framework.Platform; +using osu.Game.Overlays; +using osu.Game.Overlays.Notifications; using osuTK; using osuTK.Graphics; @@ -22,6 +24,9 @@ namespace osu.Game.Graphics.UserInterface { public string Link { get; set; } + [Resolved] + private INotificationOverlay notificationOverlay { get; set; } + private Color4 hoverColour; [Resolved] @@ -44,14 +49,26 @@ namespace osu.Game.Graphics.UserInterface }; } + private void copyUrl() + { + host.GetClipboard()?.SetText(Link); + notificationOverlay.Post(new SimpleNotification + { + Text = "Copied URL!" + }); + } + public MenuItem[] ContextMenuItems { get { - List items = new List + List items = new List(); + + if (Link != null) { - new OsuMenuItem("Copy URL", MenuItemType.Standard, () => host.GetClipboard()?.SetText(Link)) - }; + items.Add(new OsuMenuItem("Open", MenuItemType.Standard, () => host.OpenUrlExternally(Link))); + items.Add(new OsuMenuItem("Copy URL", MenuItemType.Standard, () => copyUrl())); + } return items.ToArray(); } From a8e315abf033fb82a148c507b8be1d12613d91bf Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Mon, 25 Jul 2022 17:16:33 -0400 Subject: [PATCH 24/46] Refactor --- osu.Game/Graphics/UserInterface/ExternalLinkButton.cs | 2 +- osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs index c5e1e51e07..166546bb8a 100644 --- a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs +++ b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs @@ -8,8 +8,8 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; -using osu.Framework.Graphics.UserInterface; using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Framework.Platform; diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index 85b98a92e2..c02c42786b 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -130,7 +130,8 @@ namespace osu.Game.Overlays.BeatmapSet { Font = OsuFont.GetFont(size: 30, weight: FontWeight.SemiBold, italics: true) }, - externalLink = new ExternalLinkButton { + externalLink = new ExternalLinkButton + { Anchor = Anchor.BottomLeft, Origin = Anchor.BottomLeft, Margin = new MarginPadding { Left = 5, Bottom = 4 }, // To better lineup with the font From ee0c67e114826da6d497bec8b78f6ddd5efd5630 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 14:07:33 +0900 Subject: [PATCH 25/46] Add ability to make cursor show even during touch input I completely disagree with this from a UX perspective, but it's come up so often that I figure we should just let users bone themselves. --- osu.Game/Configuration/OsuConfigManager.cs | 2 ++ osu.Game/Graphics/Cursor/MenuCursorContainer.cs | 15 +++++++++++++-- osu.Game/Localisation/SkinSettingsStrings.cs | 5 +++++ .../Settings/Sections/Gameplay/InputSettings.cs | 5 +++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index e816fd50f3..fb585e9cbd 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -91,6 +91,7 @@ namespace osu.Game.Configuration // Input SetDefault(OsuSetting.MenuCursorSize, 1.0f, 0.5f, 2f, 0.01f); SetDefault(OsuSetting.GameplayCursorSize, 1.0f, 0.1f, 2f, 0.01f); + SetDefault(OsuSetting.GameplayCursorDuringTouch, false); SetDefault(OsuSetting.AutoCursorSize, false); SetDefault(OsuSetting.MouseDisableButtons, false); @@ -292,6 +293,7 @@ namespace osu.Game.Configuration MenuCursorSize, GameplayCursorSize, AutoCursorSize, + GameplayCursorDuringTouch, DimLevel, BlurLevel, LightenDuringBreaks, diff --git a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs index 746f1b5d5e..e03dc4df6f 100644 --- a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs +++ b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs @@ -3,16 +3,20 @@ #nullable disable +using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; using osu.Framework.Input; using osu.Framework.Input.StateChanges; +using osu.Game.Configuration; namespace osu.Game.Graphics.Cursor { /// - /// A container which provides a which can be overridden by hovered s. + /// A container which provides a . + /// It also handles cases where a more localised cursor is provided by another component (via ). /// public class MenuCursorContainer : Container, IProvideCursor { @@ -36,12 +40,19 @@ namespace osu.Game.Graphics.Cursor }); } + private Bindable showDuringTouch; + private InputManager inputManager; + [Resolved] + private OsuConfigManager config { get; set; } + protected override void LoadComplete() { base.LoadComplete(); inputManager = GetContainingInputManager(); + + showDuringTouch = config.GetBindable(OsuSetting.GameplayCursorDuringTouch); } private IProvideCursor currentTarget; @@ -51,7 +62,7 @@ namespace osu.Game.Graphics.Cursor base.Update(); var lastMouseSource = inputManager.CurrentState.Mouse.LastSource; - bool hasValidInput = lastMouseSource != null && !(lastMouseSource is ISourcedFromTouch); + bool hasValidInput = lastMouseSource != null && (showDuringTouch.Value || lastMouseSource is not ISourcedFromTouch); if (!hasValidInput || !CanShowCursor) { diff --git a/osu.Game/Localisation/SkinSettingsStrings.cs b/osu.Game/Localisation/SkinSettingsStrings.cs index 81035c5a5e..4b6b0ce1d6 100644 --- a/osu.Game/Localisation/SkinSettingsStrings.cs +++ b/osu.Game/Localisation/SkinSettingsStrings.cs @@ -34,6 +34,11 @@ namespace osu.Game.Localisation /// public static LocalisableString AutoCursorSize => new TranslatableString(getKey(@"auto_cursor_size"), @"Adjust gameplay cursor size based on current beatmap"); + /// + /// "Show gameplay cursor during touch input" + /// + public static LocalisableString GameplayCursorDuringTouch => new TranslatableString(getKey(@"gameplay_cursor_during_touch"), @"Show gameplay cursor during touch input"); + /// /// "Beatmap skins" /// diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/InputSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/InputSettings.cs index b5315d5268..ac59a6c0ed 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/InputSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/InputSettings.cs @@ -32,6 +32,11 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay LabelText = SkinSettingsStrings.AutoCursorSize, Current = config.GetBindable(OsuSetting.AutoCursorSize) }, + new SettingsCheckbox + { + LabelText = SkinSettingsStrings.GameplayCursorDuringTouch, + Current = config.GetBindable(OsuSetting.GameplayCursorDuringTouch) + }, }; if (RuntimeInfo.OS == RuntimeInfo.Platform.Windows) From ef10145d6f9f77117e6e148cf82f76f6b89ffdb4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 14:11:52 +0900 Subject: [PATCH 26/46] Rename `MenuCursorContainer` and clean up code --- .../Visual/Gameplay/TestScenePause.cs | 2 +- .../Visual/UserInterface/TestSceneCursors.cs | 62 +++++++++--------- osu.Game.Tournament/TournamentGameBase.cs | 4 +- ...sorContainer.cs => GlobalCursorDisplay.cs} | 64 +++++++++---------- osu.Game/Graphics/Cursor/IProvideCursor.cs | 4 +- osu.Game/OsuGame.cs | 4 +- osu.Game/OsuGameBase.cs | 6 +- osu.Game/Rulesets/UI/DrawableRuleset.cs | 2 +- osu.Game/Screens/Utility/LatencyArea.cs | 8 +-- .../Visual/OsuManualInputManagerTestScene.cs | 6 +- 10 files changed, 80 insertions(+), 82 deletions(-) rename osu.Game/Graphics/Cursor/{MenuCursorContainer.cs => GlobalCursorDisplay.cs} (52%) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 5bd0a29308..71cc1f7b23 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -31,7 +31,7 @@ namespace osu.Game.Tests.Visual.Gameplay public TestScenePause() { - base.Content.Add(content = new MenuCursorContainer { RelativeSizeAxes = Axes.Both }); + base.Content.Add(content = new GlobalCursorDisplay { RelativeSizeAxes = Axes.Both }); } [SetUpSteps] diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs index 0fa7c5303c..bcbe146456 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs @@ -21,12 +21,12 @@ namespace osu.Game.Tests.Visual.UserInterface [TestFixture] public class TestSceneCursors : OsuManualInputManagerTestScene { - private readonly MenuCursorContainer menuCursorContainer; + private readonly GlobalCursorDisplay globalCursorDisplay; private readonly CustomCursorBox[] cursorBoxes = new CustomCursorBox[6]; public TestSceneCursors() { - Child = menuCursorContainer = new MenuCursorContainer + Child = globalCursorDisplay = new GlobalCursorDisplay { RelativeSizeAxes = Axes.Both, Children = new[] @@ -96,11 +96,11 @@ namespace osu.Game.Tests.Visual.UserInterface private void testUserCursor() { AddStep("Move to green area", () => InputManager.MoveMouseTo(cursorBoxes[0])); - AddAssert("Check green cursor visible", () => checkVisible(cursorBoxes[0].Cursor)); - AddAssert("Check green cursor at mouse", () => checkAtMouse(cursorBoxes[0].Cursor)); + AddAssert("Check green cursor visible", () => checkVisible(cursorBoxes[0].MenuCursor)); + AddAssert("Check green cursor at mouse", () => checkAtMouse(cursorBoxes[0].MenuCursor)); AddStep("Move out", moveOut); - AddAssert("Check green cursor invisible", () => !checkVisible(cursorBoxes[0].Cursor)); - AddAssert("Check global cursor visible", () => checkVisible(menuCursorContainer.Cursor)); + AddAssert("Check green cursor invisible", () => !checkVisible(cursorBoxes[0].MenuCursor)); + AddAssert("Check global cursor visible", () => checkVisible(globalCursorDisplay.MenuCursor)); } /// @@ -111,13 +111,13 @@ namespace osu.Game.Tests.Visual.UserInterface private void testLocalCursor() { AddStep("Move to purple area", () => InputManager.MoveMouseTo(cursorBoxes[3])); - AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].Cursor)); - AddAssert("Check purple cursor at mouse", () => checkAtMouse(cursorBoxes[3].Cursor)); - AddAssert("Check global cursor visible", () => checkVisible(menuCursorContainer.Cursor)); - AddAssert("Check global cursor at mouse", () => checkAtMouse(menuCursorContainer.Cursor)); + AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].MenuCursor)); + AddAssert("Check purple cursor at mouse", () => checkAtMouse(cursorBoxes[3].MenuCursor)); + AddAssert("Check global cursor visible", () => checkVisible(globalCursorDisplay.MenuCursor)); + AddAssert("Check global cursor at mouse", () => checkAtMouse(globalCursorDisplay.MenuCursor)); AddStep("Move out", moveOut); - AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].Cursor)); - AddAssert("Check global cursor visible", () => checkVisible(menuCursorContainer.Cursor)); + AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].MenuCursor)); + AddAssert("Check global cursor visible", () => checkVisible(globalCursorDisplay.MenuCursor)); } /// @@ -128,12 +128,12 @@ namespace osu.Game.Tests.Visual.UserInterface private void testUserCursorOverride() { AddStep("Move to blue-green boundary", () => InputManager.MoveMouseTo(cursorBoxes[1].ScreenSpaceDrawQuad.BottomRight - new Vector2(10))); - AddAssert("Check blue cursor visible", () => checkVisible(cursorBoxes[1].Cursor)); - AddAssert("Check green cursor invisible", () => !checkVisible(cursorBoxes[0].Cursor)); - AddAssert("Check blue cursor at mouse", () => checkAtMouse(cursorBoxes[1].Cursor)); + AddAssert("Check blue cursor visible", () => checkVisible(cursorBoxes[1].MenuCursor)); + AddAssert("Check green cursor invisible", () => !checkVisible(cursorBoxes[0].MenuCursor)); + AddAssert("Check blue cursor at mouse", () => checkAtMouse(cursorBoxes[1].MenuCursor)); AddStep("Move out", moveOut); - AddAssert("Check blue cursor not visible", () => !checkVisible(cursorBoxes[1].Cursor)); - AddAssert("Check green cursor not visible", () => !checkVisible(cursorBoxes[0].Cursor)); + AddAssert("Check blue cursor not visible", () => !checkVisible(cursorBoxes[1].MenuCursor)); + AddAssert("Check green cursor not visible", () => !checkVisible(cursorBoxes[0].MenuCursor)); } /// @@ -143,13 +143,13 @@ namespace osu.Game.Tests.Visual.UserInterface private void testMultipleLocalCursors() { AddStep("Move to yellow-purple boundary", () => InputManager.MoveMouseTo(cursorBoxes[5].ScreenSpaceDrawQuad.BottomRight - new Vector2(10))); - AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].Cursor)); - AddAssert("Check purple cursor at mouse", () => checkAtMouse(cursorBoxes[3].Cursor)); - AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].Cursor)); - AddAssert("Check yellow cursor at mouse", () => checkAtMouse(cursorBoxes[5].Cursor)); + AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].MenuCursor)); + AddAssert("Check purple cursor at mouse", () => checkAtMouse(cursorBoxes[3].MenuCursor)); + AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].MenuCursor)); + AddAssert("Check yellow cursor at mouse", () => checkAtMouse(cursorBoxes[5].MenuCursor)); AddStep("Move out", moveOut); - AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].Cursor)); - AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].Cursor)); + AddAssert("Check purple cursor visible", () => checkVisible(cursorBoxes[3].MenuCursor)); + AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].MenuCursor)); } /// @@ -159,13 +159,13 @@ namespace osu.Game.Tests.Visual.UserInterface private void testUserOverrideWithLocal() { AddStep("Move to yellow-blue boundary", () => InputManager.MoveMouseTo(cursorBoxes[5].ScreenSpaceDrawQuad.TopRight - new Vector2(10))); - AddAssert("Check blue cursor visible", () => checkVisible(cursorBoxes[1].Cursor)); - AddAssert("Check blue cursor at mouse", () => checkAtMouse(cursorBoxes[1].Cursor)); - AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].Cursor)); - AddAssert("Check yellow cursor at mouse", () => checkAtMouse(cursorBoxes[5].Cursor)); + AddAssert("Check blue cursor visible", () => checkVisible(cursorBoxes[1].MenuCursor)); + AddAssert("Check blue cursor at mouse", () => checkAtMouse(cursorBoxes[1].MenuCursor)); + AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].MenuCursor)); + AddAssert("Check yellow cursor at mouse", () => checkAtMouse(cursorBoxes[5].MenuCursor)); AddStep("Move out", moveOut); - AddAssert("Check blue cursor invisible", () => !checkVisible(cursorBoxes[1].Cursor)); - AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].Cursor)); + AddAssert("Check blue cursor invisible", () => !checkVisible(cursorBoxes[1].MenuCursor)); + AddAssert("Check yellow cursor visible", () => checkVisible(cursorBoxes[5].MenuCursor)); } /// @@ -191,7 +191,7 @@ namespace osu.Game.Tests.Visual.UserInterface { public bool SmoothTransition; - public CursorContainer Cursor { get; } + public CursorContainer MenuCursor { get; } public bool ProvidingUserCursor { get; } public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || (SmoothTransition && !ProvidingUserCursor); @@ -218,7 +218,7 @@ namespace osu.Game.Tests.Visual.UserInterface Origin = Anchor.Centre, Text = providesUserCursor ? "User cursor" : "Local cursor" }, - Cursor = new TestCursorContainer + MenuCursor = new TestCursorContainer { State = { Value = providesUserCursor ? Visibility.Hidden : Visibility.Visible }, } diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 063b62bf08..1861e39c60 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -70,10 +70,10 @@ namespace osu.Game.Tournament protected override void LoadComplete() { - MenuCursorContainer.Cursor.AlwaysPresent = true; // required for tooltip display + GlobalCursorDisplay.MenuCursor.AlwaysPresent = true; // required for tooltip display // we don't want to show the menu cursor as it would appear on stream output. - MenuCursorContainer.Cursor.Alpha = 0; + GlobalCursorDisplay.MenuCursor.Alpha = 0; base.LoadComplete(); diff --git a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs b/osu.Game/Graphics/Cursor/GlobalCursorDisplay.cs similarity index 52% rename from osu.Game/Graphics/Cursor/MenuCursorContainer.cs rename to osu.Game/Graphics/Cursor/GlobalCursorDisplay.cs index e03dc4df6f..6613e18cbe 100644 --- a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs +++ b/osu.Game/Graphics/Cursor/GlobalCursorDisplay.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -15,48 +13,48 @@ using osu.Game.Configuration; namespace osu.Game.Graphics.Cursor { /// - /// A container which provides a . - /// It also handles cases where a more localised cursor is provided by another component (via ). + /// A container which provides the main . + /// Also handles cases where a more localised cursor is provided by another component (via ). /// - public class MenuCursorContainer : Container, IProvideCursor + public class GlobalCursorDisplay : Container, IProvideCursor { - protected override Container Content => content; - private readonly Container content; - /// - /// Whether any cursors can be displayed. + /// Control whether any cursor should be displayed. /// - internal bool CanShowCursor = true; + internal bool ShowCursor = true; + + public CursorContainer MenuCursor { get; } - public CursorContainer Cursor { get; } public bool ProvidingUserCursor => true; - public MenuCursorContainer() + protected override Container Content { get; } = new Container { RelativeSizeAxes = Axes.Both }; + + private Bindable showDuringTouch = null!; + + private InputManager inputManager = null!; + + private IProvideCursor? currentOverrideProvider; + + [Resolved] + private OsuConfigManager config { get; set; } = null!; + + public GlobalCursorDisplay() { AddRangeInternal(new Drawable[] { - Cursor = new MenuCursor { State = { Value = Visibility.Hidden } }, - content = new Container { RelativeSizeAxes = Axes.Both } + MenuCursor = new MenuCursor { State = { Value = Visibility.Hidden } }, + Content = new Container { RelativeSizeAxes = Axes.Both } }); } - private Bindable showDuringTouch; - - private InputManager inputManager; - - [Resolved] - private OsuConfigManager config { get; set; } - protected override void LoadComplete() { base.LoadComplete(); - inputManager = GetContainingInputManager(); + inputManager = GetContainingInputManager(); showDuringTouch = config.GetBindable(OsuSetting.GameplayCursorDuringTouch); } - private IProvideCursor currentTarget; - protected override void Update() { base.Update(); @@ -64,31 +62,31 @@ namespace osu.Game.Graphics.Cursor var lastMouseSource = inputManager.CurrentState.Mouse.LastSource; bool hasValidInput = lastMouseSource != null && (showDuringTouch.Value || lastMouseSource is not ISourcedFromTouch); - if (!hasValidInput || !CanShowCursor) + if (!hasValidInput || !ShowCursor) { - currentTarget?.Cursor?.Hide(); - currentTarget = null; + currentOverrideProvider?.MenuCursor?.Hide(); + currentOverrideProvider = null; return; } - IProvideCursor newTarget = this; + IProvideCursor newOverrideProvider = this; foreach (var d in inputManager.HoveredDrawables) { if (d is IProvideCursor p && p.ProvidingUserCursor) { - newTarget = p; + newOverrideProvider = p; break; } } - if (currentTarget == newTarget) + if (currentOverrideProvider == newOverrideProvider) return; - currentTarget?.Cursor?.Hide(); - newTarget.Cursor?.Show(); + currentOverrideProvider?.MenuCursor?.Hide(); + newOverrideProvider.MenuCursor?.Show(); - currentTarget = newTarget; + currentOverrideProvider = newOverrideProvider; } } } diff --git a/osu.Game/Graphics/Cursor/IProvideCursor.cs b/osu.Game/Graphics/Cursor/IProvideCursor.cs index 9f01e5da6d..f7f7b75bc8 100644 --- a/osu.Game/Graphics/Cursor/IProvideCursor.cs +++ b/osu.Game/Graphics/Cursor/IProvideCursor.cs @@ -17,10 +17,10 @@ namespace osu.Game.Graphics.Cursor /// The cursor provided by this . /// May be null if no cursor should be visible. /// - CursorContainer Cursor { get; } + CursorContainer MenuCursor { get; } /// - /// Whether should be displayed as the singular user cursor. This will temporarily hide any other user cursor. + /// Whether should be displayed as the singular user cursor. This will temporarily hide any other user cursor. /// This value is checked every frame and may be used to control whether multiple cursors are displayed (e.g. watching replays). /// bool ProvidingUserCursor { get; } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 4f08511783..1ee53e2848 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -716,7 +716,7 @@ namespace osu.Game // The next time this is updated is in UpdateAfterChildren, which occurs too late and results // in the cursor being shown for a few frames during the intro. // This prevents the cursor from showing until we have a screen with CursorVisible = true - MenuCursorContainer.CanShowCursor = menuScreen?.CursorVisible ?? false; + GlobalCursorDisplay.ShowCursor = menuScreen?.CursorVisible ?? false; // todo: all archive managers should be able to be looped here. SkinManager.PostNotification = n => Notifications.Post(n); @@ -1231,7 +1231,7 @@ namespace osu.Game ScreenOffsetContainer.X = horizontalOffset; overlayContent.X = horizontalOffset * 1.2f; - MenuCursorContainer.CanShowCursor = (ScreenStack.CurrentScreen as IOsuScreen)?.CursorVisible ?? false; + GlobalCursorDisplay.ShowCursor = (ScreenStack.CurrentScreen as IOsuScreen)?.CursorVisible ?? false; } private void screenChanged(IScreen current, IScreen newScreen) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 3b81b5c8cd..7b6cda17a2 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -138,7 +138,7 @@ namespace osu.Game protected RealmKeyBindingStore KeyBindingStore { get; private set; } - protected MenuCursorContainer MenuCursorContainer { get; private set; } + protected GlobalCursorDisplay GlobalCursorDisplay { get; private set; } protected MusicController MusicController { get; private set; } @@ -340,10 +340,10 @@ namespace osu.Game RelativeSizeAxes = Axes.Both, Child = CreateScalingContainer().WithChildren(new Drawable[] { - (MenuCursorContainer = new MenuCursorContainer + (GlobalCursorDisplay = new GlobalCursorDisplay { RelativeSizeAxes = Axes.Both - }).WithChild(content = new OsuTooltipContainer(MenuCursorContainer.Cursor) + }).WithChild(content = new OsuTooltipContainer(GlobalCursorDisplay.MenuCursor) { RelativeSizeAxes = Axes.Both }), diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index 95cb02c477..f7f62d2af0 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -380,7 +380,7 @@ namespace osu.Game.Rulesets.UI // only show the cursor when within the playfield, by default. public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => Playfield.ReceivePositionalInputAt(screenSpacePos); - CursorContainer IProvideCursor.Cursor => Playfield.Cursor; + CursorContainer IProvideCursor.MenuCursor => Playfield.Cursor; public override GameplayCursorContainer Cursor => Playfield.Cursor; diff --git a/osu.Game/Screens/Utility/LatencyArea.cs b/osu.Game/Screens/Utility/LatencyArea.cs index b7d45ba642..c8e0bf93a2 100644 --- a/osu.Game/Screens/Utility/LatencyArea.cs +++ b/osu.Game/Screens/Utility/LatencyArea.cs @@ -36,7 +36,7 @@ namespace osu.Game.Screens.Utility public readonly Bindable VisualMode = new Bindable(); - public CursorContainer? Cursor { get; private set; } + public CursorContainer? MenuCursor { get; private set; } public bool ProvidingUserCursor => IsActiveArea.Value; @@ -91,7 +91,7 @@ namespace osu.Game.Screens.Utility { RelativeSizeAxes = Axes.Both, }, - Cursor = new LatencyCursorContainer + MenuCursor = new LatencyCursorContainer { RelativeSizeAxes = Axes.Both, }, @@ -105,7 +105,7 @@ namespace osu.Game.Screens.Utility { RelativeSizeAxes = Axes.Both, }, - Cursor = new LatencyCursorContainer + MenuCursor = new LatencyCursorContainer { RelativeSizeAxes = Axes.Both, }, @@ -119,7 +119,7 @@ namespace osu.Game.Screens.Utility { RelativeSizeAxes = Axes.Both, }, - Cursor = new LatencyCursorContainer + MenuCursor = new LatencyCursorContainer { RelativeSizeAxes = Axes.Both, }, diff --git a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs index eccd2efa96..9082ca9c58 100644 --- a/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs +++ b/osu.Game/Tests/Visual/OsuManualInputManagerTestScene.cs @@ -38,11 +38,11 @@ namespace osu.Game.Tests.Visual protected OsuManualInputManagerTestScene() { - MenuCursorContainer cursorContainer; + GlobalCursorDisplay cursorDisplay; - CompositeDrawable mainContent = cursorContainer = new MenuCursorContainer { RelativeSizeAxes = Axes.Both }; + CompositeDrawable mainContent = cursorDisplay = new GlobalCursorDisplay { RelativeSizeAxes = Axes.Both }; - cursorContainer.Child = content = new OsuTooltipContainer(cursorContainer.Cursor) + cursorDisplay.Child = content = new OsuTooltipContainer(cursorDisplay.MenuCursor) { RelativeSizeAxes = Axes.Both }; From 1b2158ff048da3e37c13f908eac533e4cefe2aca Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 26 Jul 2022 14:15:59 +0900 Subject: [PATCH 27/46] Remove unused method --- .../Compose/Components/Timeline/ZoomableScrollContainer.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs index 725f94e7db..7d51284f46 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs @@ -156,13 +156,6 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline setZoomTarget(zoomTarget + change * (maxZoom - minZoom) * zoom_change_sensitivity, focusPoint); } - protected void SetZoomImmediately(float value, float min, float max) - { - maxZoom = max; - minZoom = min; - CurrentZoom = zoomTarget = value; - } - private float zoomTarget = 1; private void setZoomTarget(float newZoom, float? focusPoint = null) From 7d8a78ef015b10dcb77edf9b4c14c890b22fee88 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 14:53:20 +0900 Subject: [PATCH 28/46] Move tests to own class --- .../Database/BeatmapImporterTests.cs | 55 ------------- .../Database/BeatmapImporterUpdateTests.cs | 81 +++++++++++++++++++ 2 files changed, 81 insertions(+), 55 deletions(-) create mode 100644 osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 3b1f82375c..66384e8cd1 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -725,61 +725,6 @@ namespace osu.Game.Tests.Database }); } - [Test] - public void TestImportThenReimportWithNewDifficultyAsUpdate() - { - RunTestWithRealmAsync(async (realm, storage) => - { - var importer = new BeatmapImporter(storage, realm); - using var store = new RealmRulesetStore(realm, storage); - - string? pathOriginal = TestResources.GetTestBeatmapForImport(); - - string pathMissingOneBeatmap = pathOriginal.Replace(".osz", "_missing_difficulty.osz"); - - string extractedFolder = $"{pathOriginal}_extracted"; - Directory.CreateDirectory(extractedFolder); - - try - { - using (var zip = ZipArchive.Open(pathOriginal)) - zip.WriteToDirectory(extractedFolder); - - // remove one difficulty before first import - new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).Delete(); - - using (var zip = ZipArchive.Create()) - { - zip.AddAllFromDirectory(extractedFolder); - zip.SaveTo(pathMissingOneBeatmap, new ZipWriterOptions(CompressionType.Deflate)); - } - - var firstImport = await importer.Import(new ImportTask(pathMissingOneBeatmap)); - Assert.That(firstImport, Is.Not.Null); - Debug.Assert(firstImport != null); - - Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); - Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); - - // Second import matches first but contains one extra .osu file. - var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); - Assert.That(secondImport, Is.Not.Null); - Debug.Assert(secondImport != null); - - Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); - Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); - Assert.That(realm.Realm.All(), Has.Count.EqualTo(1)); - - // check the newly "imported" beatmap is not the original. - Assert.That(firstImport.ID, Is.Not.EqualTo(secondImport.ID)); - } - finally - { - Directory.Delete(extractedFolder, true); - } - }); - } - [Test] public void TestImportThenReimportAfterMissingFiles() { diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs new file mode 100644 index 0000000000..713a73d64e --- /dev/null +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -0,0 +1,81 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Diagnostics; +using System.IO; +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Database; +using osu.Game.Overlays.Notifications; +using osu.Game.Rulesets; +using osu.Game.Tests.Resources; +using SharpCompress.Archives; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Writers.Zip; + +namespace osu.Game.Tests.Database +{ + /// + /// Tests the flow where a beatmap is already loaded and an update is applied. + /// + [TestFixture] + public class BeatmapImporterUpdateTests : RealmTest + { + [Test] + public void TestImportThenUpdateWithNewDifficulty() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var store = new RealmRulesetStore(realm, storage); + + string? pathOriginal = TestResources.GetTestBeatmapForImport(); + + string pathMissingOneBeatmap = pathOriginal.Replace(".osz", "_missing_difficulty.osz"); + + string extractedFolder = $"{pathOriginal}_extracted"; + Directory.CreateDirectory(extractedFolder); + + try + { + using (var zip = ZipArchive.Open(pathOriginal)) + zip.WriteToDirectory(extractedFolder); + + // remove one difficulty before first import + new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).Delete(); + + using (var zip = ZipArchive.Create()) + { + zip.AddAllFromDirectory(extractedFolder); + zip.SaveTo(pathMissingOneBeatmap, new ZipWriterOptions(CompressionType.Deflate)); + } + + var firstImport = await importer.Import(new ImportTask(pathMissingOneBeatmap)); + Assert.That(firstImport, Is.Not.Null); + Debug.Assert(firstImport != null); + + Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); + Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); + + // Second import matches first but contains one extra .osu file. + var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); + Assert.That(secondImport, Is.Not.Null); + Debug.Assert(secondImport != null); + + Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); + Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); + Assert.That(realm.Realm.All(), Has.Count.EqualTo(1)); + + // check the newly "imported" beatmap is not the original. + Assert.That(firstImport.ID, Is.Not.EqualTo(secondImport.ID)); + } + finally + { + Directory.Delete(extractedFolder, true); + } + }); + } + } +} From 59d3cc52c445694f2874ff91d4f8bcb9538b6430 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 14:56:41 +0900 Subject: [PATCH 29/46] Avoid leaving left-over files after test run completes --- .../Database/BeatmapImporterTests.cs | 1 - osu.Game.Tests/Database/RealmLiveTests.cs | 30 ++++++++----------- osu.Game.Tests/Database/RealmTest.cs | 23 +++++--------- .../NonVisual/DataLoadTest.cs | 4 +-- 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 9ee88c0670..2e49aa7bdc 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -142,7 +142,6 @@ namespace osu.Game.Tests.Database { Task.Run(async () => { - // ReSharper disable once AccessToDisposedClosure var beatmapSet = await importer.Import(new ImportTask(TestResources.GetTestBeatmapStream(), "renatus.osz")); Assert.NotNull(beatmapSet); diff --git a/osu.Game.Tests/Database/RealmLiveTests.cs b/osu.Game.Tests/Database/RealmLiveTests.cs index 3615cebe6a..d853e75db0 100644 --- a/osu.Game.Tests/Database/RealmLiveTests.cs +++ b/osu.Game.Tests/Database/RealmLiveTests.cs @@ -32,31 +32,29 @@ namespace osu.Game.Tests.Database [Test] public void TestAccessAfterStorageMigrate() { - RunTestWithRealm((realm, storage) => + using (var migratedStorage = new TemporaryNativeStorage("realm-test-migration-target")) { - var beatmap = new BeatmapInfo(CreateRuleset(), new BeatmapDifficulty(), new BeatmapMetadata()); - - Live? liveBeatmap = null; - - realm.Run(r => + RunTestWithRealm((realm, storage) => { - r.Write(_ => r.Add(beatmap)); + var beatmap = new BeatmapInfo(CreateRuleset(), new BeatmapDifficulty(), new BeatmapMetadata()); - liveBeatmap = beatmap.ToLive(realm); - }); + Live? liveBeatmap = null; + + realm.Run(r => + { + r.Write(_ => r.Add(beatmap)); + + liveBeatmap = beatmap.ToLive(realm); + }); - using (var migratedStorage = new TemporaryNativeStorage("realm-test-migration-target")) - { migratedStorage.DeleteDirectory(string.Empty); using (realm.BlockAllOperations("testing")) - { storage.Migrate(migratedStorage); - } Assert.IsFalse(liveBeatmap?.PerformRead(l => l.Hidden)); - } - }); + }); + } } [Test] @@ -341,14 +339,12 @@ namespace osu.Game.Tests.Database liveBeatmap.PerformRead(resolved => { // retrieval causes an implicit refresh. even changes that aren't related to the retrieval are fired at this point. - // ReSharper disable once AccessToDisposedClosure Assert.AreEqual(2, outerRealm.All().Count()); Assert.AreEqual(1, changesTriggered); // can access properties without a crash. Assert.IsFalse(resolved.Hidden); - // ReSharper disable once AccessToDisposedClosure outerRealm.Write(r => { // can use with the main context. diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs index d6b3c1ff44..1b1878942b 100644 --- a/osu.Game.Tests/Database/RealmTest.cs +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -4,11 +4,11 @@ using System; using System.Runtime.CompilerServices; using System.Threading.Tasks; +using JetBrains.Annotations; using NUnit.Framework; using osu.Framework.Extensions; using osu.Framework.Logging; using osu.Framework.Platform; -using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.IO; @@ -20,22 +20,15 @@ namespace osu.Game.Tests.Database [TestFixture] public abstract class RealmTest { - private static readonly TemporaryNativeStorage storage; - - static RealmTest() - { - storage = new TemporaryNativeStorage("realm-test"); - storage.DeleteDirectory(string.Empty); - } - - protected void RunTestWithRealm(Action testAction, [CallerMemberName] string caller = "") + protected void RunTestWithRealm([InstantHandle] Action testAction, [CallerMemberName] string caller = "") { using (HeadlessGameHost host = new CleanRunHeadlessGameHost(callingMethodName: caller)) { host.Run(new RealmTestGame(() => { - // ReSharper disable once AccessToDisposedClosure - var testStorage = new OsuStorage(host, storage.GetStorageForDirectory(caller)); + var defaultStorage = host.Storage; + + var testStorage = new OsuStorage(host, defaultStorage); using (var realm = new RealmAccess(testStorage, OsuGameBase.CLIENT_DATABASE_FILENAME)) { @@ -58,7 +51,7 @@ namespace osu.Game.Tests.Database { host.Run(new RealmTestGame(async () => { - var testStorage = storage.GetStorageForDirectory(caller); + var testStorage = host.Storage; using (var realm = new RealmAccess(testStorage, OsuGameBase.CLIENT_DATABASE_FILENAME)) { @@ -116,7 +109,7 @@ namespace osu.Game.Tests.Database private class RealmTestGame : Framework.Game { - public RealmTestGame(Func work) + public RealmTestGame([InstantHandle] Func work) { // ReSharper disable once AsyncVoidLambda Scheduler.Add(async () => @@ -126,7 +119,7 @@ namespace osu.Game.Tests.Database }); } - public RealmTestGame(Action work) + public RealmTestGame([InstantHandle] Action work) { Scheduler.Add(() => { diff --git a/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs b/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs index ad776622be..df77b31191 100644 --- a/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs +++ b/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs @@ -6,6 +6,7 @@ using System; using System.IO; using System.Threading.Tasks; +using JetBrains.Annotations; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -27,7 +28,6 @@ namespace osu.Game.Tournament.Tests.NonVisual { var osu = new TestTournament(runOnLoadComplete: () => { - // ReSharper disable once AccessToDisposedClosure var storage = host.Storage.GetStorageForDirectory(Path.Combine("tournaments", "default")); using (var stream = storage.CreateFileSafely("bracket.json")) @@ -85,7 +85,7 @@ namespace osu.Game.Tournament.Tests.NonVisual public new Task BracketLoadTask => base.BracketLoadTask; - public TestTournament(bool resetRuleset = false, Action runOnLoadComplete = null) + public TestTournament(bool resetRuleset = false, [InstantHandle] Action runOnLoadComplete = null) { this.resetRuleset = resetRuleset; this.runOnLoadComplete = runOnLoadComplete; From ac553d22ea076a161743a0fefdc67099b37b0d4b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 15:05:01 +0900 Subject: [PATCH 30/46] Fix left over resource file which isn't actually imported --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 2e49aa7bdc..af285f82b1 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -310,6 +310,7 @@ namespace osu.Game.Tests.Database } finally { + File.Delete(temp); Directory.Delete(extractedFolder, true); } }); From 99bdf417175c8fa856fca0e9dba78babbf7005c9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 15:12:22 +0900 Subject: [PATCH 31/46] Avoid potential realm fetch after disposal in `StatisticsPanel` As seen at https://github.com/ppy/osu/runs/7513799859?check_suite_focus=true. --- .../Screens/Ranking/Statistics/StatisticsPanel.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Ranking/Statistics/StatisticsPanel.cs b/osu.Game/Screens/Ranking/Statistics/StatisticsPanel.cs index 435162e057..79d7b99e51 100644 --- a/osu.Game/Screens/Ranking/Statistics/StatisticsPanel.cs +++ b/osu.Game/Screens/Ranking/Statistics/StatisticsPanel.cs @@ -11,6 +11,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; +using osu.Framework.Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Events; @@ -90,18 +91,16 @@ namespace osu.Game.Screens.Ranking.Statistics spinner.Show(); var localCancellationSource = loadCancellation = new CancellationTokenSource(); - IBeatmap playableBeatmap = null; + + var workingBeatmap = beatmapManager.GetWorkingBeatmap(newScore.BeatmapInfo); // Todo: The placement of this is temporary. Eventually we'll both generate the playable beatmap _and_ run through it in a background task to generate the hit events. - Task.Run(() => - { - playableBeatmap = beatmapManager.GetWorkingBeatmap(newScore.BeatmapInfo).GetPlayableBeatmap(newScore.Ruleset, newScore.Mods); - }, loadCancellation.Token).ContinueWith(_ => Schedule(() => + Task.Run(() => workingBeatmap.GetPlayableBeatmap(newScore.Ruleset, newScore.Mods), loadCancellation.Token).ContinueWith(task => Schedule(() => { bool hitEventsAvailable = newScore.HitEvents.Count != 0; Container container; - var statisticRows = newScore.Ruleset.CreateInstance().CreateStatisticsForScore(newScore, playableBeatmap); + var statisticRows = newScore.Ruleset.CreateInstance().CreateStatisticsForScore(newScore, task.GetResultSafely()); if (!hitEventsAvailable && statisticRows.SelectMany(r => r.Columns).All(c => c.RequiresHitEvents)) { From d7ef4170be894807edb5880d282d7f01844a5dfb Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 26 Jul 2022 09:36:25 +0300 Subject: [PATCH 32/46] Maintain sort stability by using carousel item ID as fallback --- osu.Game/Screens/Select/Carousel/CarouselGroup.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index e430ff3d3a..07c3c24018 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -24,6 +24,7 @@ namespace osu.Game.Screens.Select.Carousel private ulong currentChildID; private Comparer? criteriaComparer; + private Comparer? itemIDComparer; private FilterCriteria? lastCriteria; @@ -90,7 +91,8 @@ namespace osu.Game.Screens.Select.Carousel // IEnumerable.OrderBy() is used instead of List.Sort() to ensure sorting stability criteriaComparer = Comparer.Create((x, y) => x.CompareTo(criteria, y)); - items = items.OrderBy(c => c, criteriaComparer).ToList(); + itemIDComparer = Comparer.Create((x, y) => x.ChildID.CompareTo(y.ChildID)); + items = items.OrderBy(c => c, criteriaComparer).ThenBy(c => c, itemIDComparer).ToList(); lastCriteria = criteria; } From 693ac8750c214d43a28eb44fd76f12829d1c6177 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 26 Jul 2022 09:39:30 +0300 Subject: [PATCH 33/46] Remove remaining uses of "child" terminology in non-drawable components --- osu.Game/Screens/Select/Carousel/CarouselGroup.cs | 12 ++++++------ .../Select/Carousel/CarouselGroupEagerSelect.cs | 14 +++++++------- osu.Game/Screens/Select/Carousel/CarouselItem.cs | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 07c3c24018..7fcf53e68c 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -7,7 +7,7 @@ using System.Linq; namespace osu.Game.Screens.Select.Carousel { /// - /// A group which ensures only one child is selected. + /// A group which ensures only one item is selected. /// public class CarouselGroup : CarouselItem { @@ -18,10 +18,10 @@ namespace osu.Game.Screens.Select.Carousel private List items = new List(); /// - /// Used to assign a monotonically increasing ID to children as they are added. This member is - /// incremented whenever a child is added. + /// Used to assign a monotonically increasing ID to items as they are added. This member is + /// incremented whenever an item is added. /// - private ulong currentChildID; + private ulong currentItemID; private Comparer? criteriaComparer; private Comparer? itemIDComparer; @@ -42,7 +42,7 @@ namespace osu.Game.Screens.Select.Carousel public virtual void AddItem(CarouselItem i) { i.State.ValueChanged += state => ChildItemStateChanged(i, state.NewValue); - i.ChildID = ++currentChildID; + i.ItemID = ++currentItemID; if (lastCriteria != null) { @@ -91,7 +91,7 @@ namespace osu.Game.Screens.Select.Carousel // IEnumerable.OrderBy() is used instead of List.Sort() to ensure sorting stability criteriaComparer = Comparer.Create((x, y) => x.CompareTo(criteria, y)); - itemIDComparer = Comparer.Create((x, y) => x.ChildID.CompareTo(y.ChildID)); + itemIDComparer = Comparer.Create((x, y) => x.ItemID.CompareTo(y.ItemID)); items = items.OrderBy(c => c, criteriaComparer).ThenBy(c => c, itemIDComparer).ToList(); lastCriteria = criteria; diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 613b3db5d5..61109829f3 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -10,7 +10,7 @@ using System.Linq; namespace osu.Game.Screens.Select.Carousel { /// - /// A group which ensures at least one child is selected (if the group itself is selected). + /// A group which ensures at least one item is selected (if the group itself is selected). /// public class CarouselGroupEagerSelect : CarouselGroup { @@ -35,16 +35,16 @@ namespace osu.Game.Screens.Select.Carousel /// /// To avoid overhead during filter operations, we don't attempt any selections until after all - /// children have been filtered. This bool will be true during the base + /// items have been filtered. This bool will be true during the base /// operation. /// - private bool filteringChildren; + private bool filteringItems; public override void Filter(FilterCriteria criteria) { - filteringChildren = true; + filteringItems = true; base.Filter(criteria); - filteringChildren = false; + filteringItems = false; attemptSelection(); } @@ -97,12 +97,12 @@ namespace osu.Game.Screens.Select.Carousel private void attemptSelection() { - if (filteringChildren) return; + if (filteringItems) return; // we only perform eager selection if we are a currently selected group. if (State.Value != CarouselItemState.Selected) return; - // we only perform eager selection if none of our children are in a selected state already. + // we only perform eager selection if none of our items are in a selected state already. if (Items.Any(i => i.State.Value == CarouselItemState.Selected)) return; PerformSelection(); diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index a901f72dad..cbf079eb4b 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -38,7 +38,7 @@ namespace osu.Game.Screens.Select.Carousel /// /// Used as a default sort method for s of differing types. /// - internal ulong ChildID; + internal ulong ItemID; /// /// Create a fresh drawable version of this item. @@ -49,7 +49,7 @@ namespace osu.Game.Screens.Select.Carousel { } - public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => ChildID.CompareTo(other.ChildID); + public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => ItemID.CompareTo(other.ItemID); public int CompareTo(CarouselItem other) => CarouselYPosition.CompareTo(other.CarouselYPosition); } From 8370ca976557116af528330389f7fe18583d0896 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 15:46:29 +0900 Subject: [PATCH 34/46] Add `ImportAsUpdate` method to `IModelImporter` to avoid otehr changes --- .../Database/BeatmapImporterUpdateTests.cs | 2 +- ...ceneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Beatmaps/BeatmapImporter.cs | 10 ++++++---- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- osu.Game/Beatmaps/BeatmapModelDownloader.cs | 18 +----------------- osu.Game/Database/IModelImporter.cs | 10 ++++++++++ osu.Game/Database/ModelDownloader.cs | 16 +++------------- osu.Game/Database/RealmArchiveModelImporter.cs | 2 ++ osu.Game/Scoring/ScoreManager.cs | 2 ++ .../Select/Carousel/UpdateBeatmapSetButton.cs | 2 +- osu.Game/Skinning/SkinManager.cs | 2 ++ 11 files changed, 30 insertions(+), 38 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 713a73d64e..cf4245ae83 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -60,7 +60,7 @@ namespace osu.Game.Tests.Database Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); // Second import matches first but contains one extra .osu file. - var secondImport = (await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value)).FirstOrDefault(); + var secondImport = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value); Assert.That(secondImport, Is.Not.Null); Debug.Assert(secondImport != null); diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 0a980efbae..536322805b 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -237,7 +237,7 @@ namespace osu.Game.Tests.Online internal class TestBeatmapModelDownloader : BeatmapModelDownloader { - public TestBeatmapModelDownloader(BeatmapManager importer, IAPIProvider apiProvider) + public TestBeatmapModelDownloader(IModelImporter importer, IAPIProvider apiProvider) : base(importer, apiProvider) { } diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 2a6121b541..1e90e14e68 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -41,16 +41,18 @@ namespace osu.Game.Beatmaps { } - public async Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) + public override async Task?> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) { var imported = await Import(notification, importTask); if (!imported.Any()) - return imported; + return null; Debug.Assert(imported.Count() == 1); - imported.First().PerformWrite(updated => + var first = imported.First(); + + first.PerformWrite(updated => { var realm = updated.Realm; @@ -103,7 +105,7 @@ namespace osu.Game.Beatmaps realm.Remove(original); }); - return imported; + return first; } protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == ".osz"; diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 1b31d29c2b..debe4c6829 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -408,7 +408,7 @@ namespace osu.Game.Beatmaps Realm.Run(r => Undelete(r.All().Where(s => s.DeletePending).ToList())); } - public Task>> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => + public Task?> ImportAsUpdate(ProgressNotification notification, ImportTask importTask, BeatmapSetInfo original) => beatmapImporter.ImportAsUpdate(notification, importTask, original); #region Implementation of ICanAcceptFiles diff --git a/osu.Game/Beatmaps/BeatmapModelDownloader.cs b/osu.Game/Beatmaps/BeatmapModelDownloader.cs index e70168f1b3..4295def5c3 100644 --- a/osu.Game/Beatmaps/BeatmapModelDownloader.cs +++ b/osu.Game/Beatmaps/BeatmapModelDownloader.cs @@ -1,39 +1,23 @@ // 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.Threading.Tasks; using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.API.Requests; -using osu.Game.Overlays.Notifications; namespace osu.Game.Beatmaps { public class BeatmapModelDownloader : ModelDownloader { - private readonly BeatmapManager beatmapManager; - protected override ArchiveDownloadRequest CreateDownloadRequest(IBeatmapSetInfo set, bool minimiseDownloadSize) => new DownloadBeatmapSetRequest(set, minimiseDownloadSize); public override ArchiveDownloadRequest? GetExistingDownload(IBeatmapSetInfo model) => CurrentDownloads.Find(r => r.Model.OnlineID == model.OnlineID); - public BeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) + public BeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { - beatmapManager = beatmapImporter; } - - protected override Task>> Import(ProgressNotification notification, string filename, BeatmapSetInfo? originalModel) - { - if (originalModel != null) - return beatmapManager.ImportAsUpdate(notification, new ImportTask(filename), originalModel); - - return base.Import(notification, filename, null); - } - - public bool Update(BeatmapSetInfo model) => Download(model, false, model); } } diff --git a/osu.Game/Database/IModelImporter.cs b/osu.Game/Database/IModelImporter.cs index ebb8be39ef..4085f122d0 100644 --- a/osu.Game/Database/IModelImporter.cs +++ b/osu.Game/Database/IModelImporter.cs @@ -23,6 +23,16 @@ namespace osu.Game.Database /// The imported models. Task>> Import(ProgressNotification notification, params ImportTask[] tasks); + /// + /// Process a single import as an update for an existing model. + /// This will still run a full import, but perform any post-processing required to make it feel like an update to the user. + /// + /// The notification to update. + /// The import task. + /// The original model which is being updated. + /// The imported model. + Task?> ImportAsUpdate(ProgressNotification notification, ImportTask task, TModel original); + /// /// A user displayable name for the model type associated with this manager. /// diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 5fd124cafc..7ce0fa46b7 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -44,6 +44,8 @@ namespace osu.Game.Database public bool Download(T model, bool minimiseDownloadSize = false) => Download(model, minimiseDownloadSize, null); + public void DownloadAsUpdate(TModel originalModel) => Download(originalModel, false, originalModel); + protected bool Download(T model, bool minimiseDownloadSize, TModel? originalModel) { if (!canDownload(model)) return false; @@ -66,7 +68,7 @@ namespace osu.Game.Database Task.Factory.StartNew(async () => { // This gets scheduled back to the update thread, but we want the import to run in the background. - var imported = await Import(notification, filename, originalModel).ConfigureAwait(false); + var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); // for now a failed import will be marked as a failed download for simplicity. if (!imported.Any()) @@ -105,18 +107,6 @@ namespace osu.Game.Database } } - /// - /// Run the post-download import for the model. - /// - /// The notification to update. - /// The path of the temporary downloaded file. - /// An optional model for update scenarios, to be used as a reference. - /// The imported model. - protected virtual Task>> Import(ProgressNotification notification, string filename, TModel? originalModel) - { - return importer.Import(notification, new ImportTask(filename)); - } - public abstract ArchiveDownloadRequest? GetExistingDownload(T model); private bool canDownload(T model) => GetExistingDownload(model) == null && api != null; diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index aa7fac07a8..a0cf98b978 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -174,6 +174,8 @@ namespace osu.Game.Database return imported; } + public virtual Task?> ImportAsUpdate(ProgressNotification notification, ImportTask task, TModel original) => throw new NotImplementedException(); + /// /// Import one from the filesystem and delete the file on success. /// Note that this bypasses the UI flow and should only be used for special cases or testing. diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 7cfc55580b..7367a1ef77 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -268,6 +268,8 @@ namespace osu.Game.Scoring public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => scoreImporter.Import(notification, tasks); + public Task> ImportAsUpdate(ProgressNotification notification, ImportTask task, ScoreInfo original) => scoreImporter.ImportAsUpdate(notification, task, original); + public Live Import(ScoreInfo item, ArchiveReader archive = null, bool batchImport = false, CancellationToken cancellationToken = default) => scoreImporter.ImportModel(item, archive, batchImport, cancellationToken); diff --git a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs index 3746c1975c..73e7d23df0 100644 --- a/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs +++ b/osu.Game/Screens/Select/Carousel/UpdateBeatmapSetButton.cs @@ -90,7 +90,7 @@ namespace osu.Game.Screens.Select.Carousel Action = () => { - beatmapDownloader.Update(beatmapSetInfo); + beatmapDownloader.DownloadAsUpdate(beatmapSetInfo); attachExistingDownload(); }; } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index dc0197e613..dd35f83434 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -272,6 +272,8 @@ namespace osu.Game.Skinning public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => skinImporter.Import(notification, tasks); + public Task> ImportAsUpdate(ProgressNotification notification, ImportTask task, SkinInfo original) => skinImporter.ImportAsUpdate(notification, task, original); + public Task> Import(ImportTask task, bool batchImport = false, CancellationToken cancellationToken = default) => skinImporter.Import(task, batchImport, cancellationToken); #endregion From 9939866f7d3e65796fa61c39880c36a38b3258aa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 15:54:10 +0900 Subject: [PATCH 35/46] Revert one more missed change --- osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs index bcbe7084d5..80af4108c7 100644 --- a/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs +++ b/osu.Game/Beatmaps/Drawables/BundledBeatmapDownloader.cs @@ -11,6 +11,7 @@ using System.Text.RegularExpressions; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Game.Database; using osu.Game.Online; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -108,7 +109,7 @@ namespace osu.Game.Beatmaps.Drawables private class BundledBeatmapModelDownloader : BeatmapModelDownloader { - public BundledBeatmapModelDownloader(BeatmapManager beatmapImporter, IAPIProvider api) + public BundledBeatmapModelDownloader(IModelImporter beatmapImporter, IAPIProvider api) : base(beatmapImporter, api) { } From a4f6f2b9eb8ad8a5e0f0ca35d6c641a3fc411360 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 26 Jul 2022 09:55:37 +0300 Subject: [PATCH 36/46] Make item ID comparer static --- osu.Game/Screens/Select/Carousel/CarouselGroup.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 7fcf53e68c..8d141b6f72 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -24,7 +24,8 @@ namespace osu.Game.Screens.Select.Carousel private ulong currentItemID; private Comparer? criteriaComparer; - private Comparer? itemIDComparer; + + private static readonly Comparer item_id_comparer = Comparer.Create((x, y) => x.ItemID.CompareTo(y.ItemID)); private FilterCriteria? lastCriteria; @@ -91,8 +92,7 @@ namespace osu.Game.Screens.Select.Carousel // IEnumerable.OrderBy() is used instead of List.Sort() to ensure sorting stability criteriaComparer = Comparer.Create((x, y) => x.CompareTo(criteria, y)); - itemIDComparer = Comparer.Create((x, y) => x.ItemID.CompareTo(y.ItemID)); - items = items.OrderBy(c => c, criteriaComparer).ThenBy(c => c, itemIDComparer).ToList(); + items = items.OrderBy(c => c, criteriaComparer).ThenBy(c => c, item_id_comparer).ToList(); lastCriteria = criteria; } From 91ffa7007f85f56cbe96f1c23111624a16e13172 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 26 Jul 2022 10:24:16 +0300 Subject: [PATCH 37/46] Improve existing test coverage to cover order changes from other sort modes --- .../Visual/SongSelect/TestSceneBeatmapCarousel.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index e574ee30fb..59932f8781 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -522,15 +522,15 @@ namespace osu.Game.Tests.Visual.SongSelect { var sets = new List(); - for (int i = 0; i < 20; i++) + for (int i = 0; i < 10; i++) { var set = TestResources.CreateTestBeatmapSetInfo(); // only need to set the first as they are a shared reference. var beatmap = set.Beatmaps.First(); - beatmap.Metadata.Artist = "same artist"; - beatmap.Metadata.Title = "same title"; + beatmap.Metadata.Artist = $"artist {i / 2}"; + beatmap.Metadata.Title = $"title {9 - i}"; sets.Add(set); } @@ -540,10 +540,13 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); - AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == index + idOffset).All(b => b)); + AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b)); AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); - AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == index + idOffset).All(b => b)); + AddAssert("Items are in reverse order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + sets.Count - index - 1).All(b => b)); + + AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddAssert("Items reset to original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b)); } [Test] From 846291d20387ecf0e23669edb100ec288a3675b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 16:30:44 +0900 Subject: [PATCH 38/46] Refactor new tests to not suck as much as the old importer tests --- .../Database/BeatmapImporterUpdateTests.cs | 107 +++++++++++------- 1 file changed, 69 insertions(+), 38 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index cf4245ae83..38c5172227 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -1,15 +1,19 @@ // 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.Diagnostics; using System.IO; using System.Linq; +using System.Linq.Expressions; using NUnit.Framework; +using osu.Framework.Allocation; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; using osu.Game.Tests.Resources; +using Realms; using SharpCompress.Archives; using SharpCompress.Archives.Zip; using SharpCompress.Common; @@ -24,58 +28,85 @@ namespace osu.Game.Tests.Database public class BeatmapImporterUpdateTests : RealmTest { [Test] - public void TestImportThenUpdateWithNewDifficulty() + public void TestNewDifficultyAdded() { RunTestWithRealmAsync(async (realm, storage) => { var importer = new BeatmapImporter(storage, realm); - using var store = new RealmRulesetStore(realm, storage); + using var rulesets = new RealmRulesetStore(realm, storage); - string? pathOriginal = TestResources.GetTestBeatmapForImport(); - - string pathMissingOneBeatmap = pathOriginal.Replace(".osz", "_missing_difficulty.osz"); - - string extractedFolder = $"{pathOriginal}_extracted"; - Directory.CreateDirectory(extractedFolder); - - try + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory => { - using (var zip = ZipArchive.Open(pathOriginal)) - zip.WriteToDirectory(extractedFolder); - // remove one difficulty before first import - new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).Delete(); + directory.GetFiles("*.osu").First().Delete(); + }); - using (var zip = ZipArchive.Create()) - { - zip.AddAllFromDirectory(extractedFolder); - zip.SaveTo(pathMissingOneBeatmap, new ZipWriterOptions(CompressionType.Deflate)); - } + var importBeforeUpdate = await importer.Import(new ImportTask(pathMissingOneBeatmap)); - var firstImport = await importer.Import(new ImportTask(pathMissingOneBeatmap)); - Assert.That(firstImport, Is.Not.Null); - Debug.Assert(firstImport != null); + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); - Assert.That(realm.Realm.All().Where(s => !s.DeletePending), Has.Count.EqualTo(1)); - Assert.That(realm.Realm.All().First(s => !s.DeletePending).Beatmaps, Has.Count.EqualTo(11)); + checkCount(realm, 1, s => !s.DeletePending); + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(11)); - // Second import matches first but contains one extra .osu file. - var secondImport = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), firstImport.Value); - Assert.That(secondImport, Is.Not.Null); - Debug.Assert(secondImport != null); + // Second import matches first but contains one extra .osu file. + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), importBeforeUpdate.Value); - Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); - Assert.That(realm.Realm.All(), Has.Count.EqualTo(12)); - Assert.That(realm.Realm.All(), Has.Count.EqualTo(1)); + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); - // check the newly "imported" beatmap is not the original. - Assert.That(firstImport.ID, Is.Not.EqualTo(secondImport.ID)); - } - finally - { - Directory.Delete(extractedFolder, true); - } + checkCount(realm, 12); + checkCount(realm, 12); + checkCount(realm, 1); + + // check the newly "imported" beatmap is not the original. + Assert.That(importBeforeUpdate.ID, Is.Not.EqualTo(importAfterUpdate.ID)); }); } + + private static void checkCount(RealmAccess realm, int expected, Expression>? condition = null) where T : RealmObject + { + var query = realm.Realm.All(); + + if (condition != null) + query = query.Where(condition); + + Assert.That(query, Has.Count.EqualTo(expected)); + } + + private static IDisposable getBeatmapArchiveWithModifications(out string path, Action applyModifications) + { + var cleanup = getBeatmapArchive(out path); + + string extractedFolder = $"{path}_extracted"; + Directory.CreateDirectory(extractedFolder); + + using (var zip = ZipArchive.Open(path)) + zip.WriteToDirectory(extractedFolder); + + applyModifications(new DirectoryInfo(extractedFolder)); + + File.Delete(path); + + using (var zip = ZipArchive.Create()) + { + zip.AddAllFromDirectory(extractedFolder); + zip.SaveTo(path, new ZipWriterOptions(CompressionType.Deflate)); + } + + Directory.Delete(extractedFolder, true); + + return cleanup; + } + + private static IDisposable getBeatmapArchive(out string path, bool quick = true) + { + string beatmapPath = TestResources.GetTestBeatmapForImport(quick); + + path = beatmapPath; + + return new InvokeOnDisposal(() => File.Delete(beatmapPath)); + } } } From 4c22b55ce345a7e0b05aef98f165e1969fd42332 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 17:00:28 +0900 Subject: [PATCH 39/46] Fix incorrect handling if an update is processed with no changes --- osu.Game/Beatmaps/BeatmapImporter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 1e90e14e68..ef0e76234a 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -52,6 +52,10 @@ namespace osu.Game.Beatmaps var first = imported.First(); + // If there were no changes, ensure we don't accidentally nuke ourselves. + if (first.ID == original.ID) + return first; + first.PerformWrite(updated => { var realm = updated.Realm; From aaf6ec05bbef6d30b9be0e5e8f9775e882c5c3c5 Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Tue, 26 Jul 2022 04:19:54 -0400 Subject: [PATCH 40/46] Remove notification upon copy --- .../Graphics/UserInterface/ExternalLinkButton.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs index 166546bb8a..acabeca66e 100644 --- a/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs +++ b/osu.Game/Graphics/UserInterface/ExternalLinkButton.cs @@ -13,8 +13,6 @@ using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Framework.Platform; -using osu.Game.Overlays; -using osu.Game.Overlays.Notifications; using osuTK; using osuTK.Graphics; @@ -24,9 +22,6 @@ namespace osu.Game.Graphics.UserInterface { public string Link { get; set; } - [Resolved] - private INotificationOverlay notificationOverlay { get; set; } - private Color4 hoverColour; [Resolved] @@ -49,15 +44,6 @@ namespace osu.Game.Graphics.UserInterface }; } - private void copyUrl() - { - host.GetClipboard()?.SetText(Link); - notificationOverlay.Post(new SimpleNotification - { - Text = "Copied URL!" - }); - } - public MenuItem[] ContextMenuItems { get @@ -67,7 +53,7 @@ namespace osu.Game.Graphics.UserInterface if (Link != null) { items.Add(new OsuMenuItem("Open", MenuItemType.Standard, () => host.OpenUrlExternally(Link))); - items.Add(new OsuMenuItem("Copy URL", MenuItemType.Standard, () => copyUrl())); + items.Add(new OsuMenuItem("Copy URL", MenuItemType.Standard, () => host.GetClipboard()?.SetText(Link))); } return items.ToArray(); From 1221cb1a42a68265c0f80151514b6952442e9fdf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 17:22:52 +0900 Subject: [PATCH 41/46] Add comprehensive test coverage of update scenarios --- .../Database/BeatmapImporterUpdateTests.cs | 289 +++++++++++++++++- 1 file changed, 286 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 38c5172227..515bed5c35 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -10,8 +10,10 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Game.Beatmaps; using osu.Game.Database; +using osu.Game.Models; using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; +using osu.Game.Scoring; using osu.Game.Tests.Resources; using Realms; using SharpCompress.Archives; @@ -27,6 +29,8 @@ namespace osu.Game.Tests.Database [TestFixture] public class BeatmapImporterUpdateTests : RealmTest { + private const int count_beatmaps = 12; + [Test] public void TestNewDifficultyAdded() { @@ -48,7 +52,7 @@ namespace osu.Game.Tests.Database Debug.Assert(importBeforeUpdate != null); checkCount(realm, 1, s => !s.DeletePending); - Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(11)); + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps - 1)); // Second import matches first but contains one extra .osu file. var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), importBeforeUpdate.Value); @@ -56,12 +60,291 @@ namespace osu.Game.Tests.Database Assert.That(importAfterUpdate, Is.Not.Null); Debug.Assert(importAfterUpdate != null); - checkCount(realm, 12); - checkCount(realm, 12); + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); checkCount(realm, 1); // check the newly "imported" beatmap is not the original. Assert.That(importBeforeUpdate.ID, Is.Not.EqualTo(importAfterUpdate.ID)); + + // Previous beatmap set has no beatmaps so will be completely purged on the spot. + Assert.That(importBeforeUpdate.Value.IsValid, Is.False); + }); + } + + [Test] + public void TestExistingDifficultyModified() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathModified, directory => + { + // Modify one .osu file with different content. + var firstOsuFile = directory.GetFiles("*.osu").First(); + + string existingContent = File.ReadAllText(firstOsuFile.FullName); + + File.WriteAllText(firstOsuFile.FullName, existingContent + "\n# I am new content"); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + checkCount(realm, 1, s => !s.DeletePending); + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps)); + + // Second import matches first but contains one extra .osu file. + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathModified), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + // should only contain the modified beatmap (others purged). + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(1)); + Assert.That(importAfterUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps)); + + checkCount(realm, count_beatmaps + 1); + checkCount(realm, count_beatmaps + 1); + + checkCount(realm, 1, s => !s.DeletePending); + checkCount(realm, 1, s => s.DeletePending); + }); + } + + [Test] + public void TestExistingDifficultyRemoved() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory => + { + // remove one difficulty before first import + directory.GetFiles("*.osu").First().Delete(); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps)); + Assert.That(importBeforeUpdate.Value.Beatmaps.First().OnlineID, Is.GreaterThan(-1)); + + // Second import matches first but contains one extra .osu file. + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathMissingOneBeatmap), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); + checkCount(realm, 2); + + // previous set should contain the removed beatmap still. + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(1)); + Assert.That(importBeforeUpdate.Value.Beatmaps.First().OnlineID, Is.EqualTo(-1)); + + // Previous beatmap set has no beatmaps so will be completely purged on the spot. + Assert.That(importAfterUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps - 1)); + }); + } + + [Test] + public void TestUpdatedImportContainsNothing() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathEmpty, directory => + { + foreach (var file in directory.GetFiles()) + file.Delete(); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathEmpty), importBeforeUpdate.Value); + Assert.That(importAfterUpdate, Is.Null); + + checkCount(realm, 1); + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); + + Assert.That(importBeforeUpdate.Value.IsValid, Is.True); + }); + } + + [Test] + public void TestNoChanges() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchive(out string pathOriginalSecond); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginalSecond), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + checkCount(realm, 1); + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); + + Assert.That(importBeforeUpdate.Value.Beatmaps.First().OnlineID, Is.GreaterThan(-1)); + Assert.That(importBeforeUpdate.ID, Is.EqualTo(importAfterUpdate.ID)); + }); + } + + [Test] + public void TestScoreTransferredOnUnchanged() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory => + { + // arbitrary beatmap removal + directory.GetFiles("*.osu").First().Delete(); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + string scoreTargetBeatmapHash = string.Empty; + + importBeforeUpdate.PerformWrite(s => + { + var beatmapInfo = s.Beatmaps.Last(); + scoreTargetBeatmapHash = beatmapInfo.Hash; + s.Realm.Add(new ScoreInfo(beatmapInfo, s.Realm.All().First(), new RealmUser())); + }); + + checkCount(realm, 1); + + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathMissingOneBeatmap), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); + checkCount(realm, 2); + + // score is transferred across to the new set + checkCount(realm, 1); + Assert.That(importAfterUpdate.Value.Beatmaps.First(b => b.Hash == scoreTargetBeatmapHash).Scores, Has.Count.EqualTo(1)); + }); + } + + [Test] + public void TestScoreLostOnModification() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + string? scoreTargetFilename = string.Empty; + + importBeforeUpdate.PerformWrite(s => + { + var beatmapInfo = s.Beatmaps.Last(); + scoreTargetFilename = beatmapInfo.File?.Filename; + s.Realm.Add(new ScoreInfo(beatmapInfo, s.Realm.All().First(), new RealmUser())); + }); + + checkCount(realm, 1); + + using var _ = getBeatmapArchiveWithModifications(out string pathModified, directory => + { + // Modify one .osu file with different content. + var firstOsuFile = directory.GetFiles(scoreTargetFilename).First(); + + string existingContent = File.ReadAllText(firstOsuFile.FullName); + + File.WriteAllText(firstOsuFile.FullName, existingContent + "\n# I am new content"); + }); + + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathModified), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + checkCount(realm, count_beatmaps + 1); + checkCount(realm, count_beatmaps + 1); + checkCount(realm, 2); + + // score is not transferred due to modifications. + checkCount(realm, 1); + Assert.That(importBeforeUpdate.Value.Beatmaps.AsEnumerable().First(b => b.File?.Filename == scoreTargetFilename).Scores, Has.Count.EqualTo(1)); + Assert.That(importAfterUpdate.Value.Beatmaps.AsEnumerable().First(b => b.File?.Filename == scoreTargetFilename).Scores, Has.Count.EqualTo(0)); + }); + } + + [Test] + public void TestMetadataTransferred() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory => + { + // arbitrary beatmap removal + directory.GetFiles("*.osu").First().Delete(); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathOriginal)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathMissingOneBeatmap), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + Assert.That(importBeforeUpdate.ID, Is.Not.EqualTo(importAfterUpdate.ID)); + Assert.That(importBeforeUpdate.Value.DateAdded, Is.EqualTo(importAfterUpdate.Value.DateAdded)); }); } From 003aec86ae8b2304804d129c3c90cf2776aa627e Mon Sep 17 00:00:00 2001 From: Andrew Hong Date: Tue, 26 Jul 2022 04:27:22 -0400 Subject: [PATCH 42/46] Rearrange sizeaxes --- osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index c02c42786b..9e14122ae4 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -93,7 +93,8 @@ namespace osu.Game.Overlays.BeatmapSet }, new OsuContextMenuContainer { - RelativeSizeAxes = Axes.Both, + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, Child = new Container { RelativeSizeAxes = Axes.X, From ee694c12576369bd0b04cf58a4b4607e22812375 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 17:27:23 +0900 Subject: [PATCH 43/46] Add test coverage of no online ID scenario --- .../Database/BeatmapImporterUpdateTests.cs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 515bed5c35..a82386fd51 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -72,6 +72,58 @@ namespace osu.Game.Tests.Database }); } + /// + /// Regression test covering https://github.com/ppy/osu/issues/19369 (import potentially duplicating if original has no ). + /// + [Test] + public void TestNewDifficultyAddedNoOnlineID() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var rulesets = new RealmRulesetStore(realm, storage); + + using var __ = getBeatmapArchive(out string pathOriginal); + using var _ = getBeatmapArchiveWithModifications(out string pathMissingOneBeatmap, directory => + { + // remove one difficulty before first import + directory.GetFiles("*.osu").First().Delete(); + }); + + var importBeforeUpdate = await importer.Import(new ImportTask(pathMissingOneBeatmap)); + + Assert.That(importBeforeUpdate, Is.Not.Null); + Debug.Assert(importBeforeUpdate != null); + + // This test is the same as TestNewDifficultyAdded except for this block. + importBeforeUpdate.PerformWrite(s => + { + s.OnlineID = -1; + foreach (var beatmap in s.Beatmaps) + beatmap.ResetOnlineInfo(); + }); + + checkCount(realm, 1, s => !s.DeletePending); + Assert.That(importBeforeUpdate.Value.Beatmaps, Has.Count.EqualTo(count_beatmaps - 1)); + + // Second import matches first but contains one extra .osu file. + var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOriginal), importBeforeUpdate.Value); + + Assert.That(importAfterUpdate, Is.Not.Null); + Debug.Assert(importAfterUpdate != null); + + checkCount(realm, count_beatmaps); + checkCount(realm, count_beatmaps); + checkCount(realm, 1); + + // check the newly "imported" beatmap is not the original. + Assert.That(importBeforeUpdate.ID, Is.Not.EqualTo(importAfterUpdate.ID)); + + // Previous beatmap set has no beatmaps so will be completely purged on the spot. + Assert.That(importBeforeUpdate.Value.IsValid, Is.False); + }); + } + [Test] public void TestExistingDifficultyModified() { From 1539fa704bb1b8c1b64a297862007ba615d6bc78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 17:46:23 +0900 Subject: [PATCH 44/46] Always allow selecting the top-most button using the select binding --- osu.Game/Overlays/DialogOverlay.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/DialogOverlay.cs b/osu.Game/Overlays/DialogOverlay.cs index 493cd66258..ba8083e535 100644 --- a/osu.Game/Overlays/DialogOverlay.cs +++ b/osu.Game/Overlays/DialogOverlay.cs @@ -121,7 +121,11 @@ namespace osu.Game.Overlays switch (e.Action) { case GlobalAction.Select: - CurrentDialog?.Buttons.OfType().FirstOrDefault()?.TriggerClick(); + var clickableButton = + CurrentDialog?.Buttons.OfType().FirstOrDefault() ?? + CurrentDialog?.Buttons.First(); + + clickableButton?.TriggerClick(); return true; } From 91732719005dde9e7aa6e031f8a76067fc4c873a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 17:58:09 +0900 Subject: [PATCH 45/46] Fix new update pathway not actually being used --- osu.Game/Database/ModelDownloader.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/ModelDownloader.cs b/osu.Game/Database/ModelDownloader.cs index 7ce0fa46b7..edb8563c65 100644 --- a/osu.Game/Database/ModelDownloader.cs +++ b/osu.Game/Database/ModelDownloader.cs @@ -67,11 +67,15 @@ namespace osu.Game.Database { Task.Factory.StartNew(async () => { - // This gets scheduled back to the update thread, but we want the import to run in the background. - var imported = await importer.Import(notification, new ImportTask(filename)).ConfigureAwait(false); + bool importSuccessful; + + if (originalModel != null) + importSuccessful = (await importer.ImportAsUpdate(notification, new ImportTask(filename), originalModel)) != null; + else + importSuccessful = (await importer.Import(notification, new ImportTask(filename))).Any(); // for now a failed import will be marked as a failed download for simplicity. - if (!imported.Any()) + if (!importSuccessful) DownloadFailed?.Invoke(request); CurrentDownloads.Remove(request); From e782590b3cc3f58a4a126041dc40a702edea6c66 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Jul 2022 18:20:43 +0900 Subject: [PATCH 46/46] Don't show audio playback issue notification if debugger is attached I've hit this countless times recently when debugging during the startup procedure. --- osu.Game/Screens/Menu/IntroScreen.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Menu/IntroScreen.cs b/osu.Game/Screens/Menu/IntroScreen.cs index c1621ce78f..a2ecd7eacb 100644 --- a/osu.Game/Screens/Menu/IntroScreen.cs +++ b/osu.Game/Screens/Menu/IntroScreen.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.Diagnostics; using System.Linq; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -195,10 +196,14 @@ namespace osu.Game.Screens.Menu PrepareMenuLoad(); LoadMenu(); - notifications.Post(new SimpleErrorNotification + + if (!Debugger.IsAttached) { - Text = "osu! doesn't seem to be able to play audio correctly.\n\nPlease try changing your audio device to a working setting." - }); + notifications.Post(new SimpleErrorNotification + { + Text = "osu! doesn't seem to be able to play audio correctly.\n\nPlease try changing your audio device to a working setting." + }); + } }, 5000); }