From 27d4016ccb360991c37750bf5fe67e193b8fa038 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 12 Sep 2022 18:32:14 +0900 Subject: [PATCH 01/15] Add `ScoreProcessor.ComputeAccuracy()` --- osu.Game/Rulesets/Scoring/ScoreProcessor.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Scoring/ScoreProcessor.cs b/osu.Game/Rulesets/Scoring/ScoreProcessor.cs index 64d5639133..7456ce06bd 100644 --- a/osu.Game/Rulesets/Scoring/ScoreProcessor.cs +++ b/osu.Game/Rulesets/Scoring/ScoreProcessor.cs @@ -272,7 +272,24 @@ namespace osu.Game.Rulesets.Scoring } /// - /// Computes the total score of a given finalised . This should be used when a score is known to be complete. + /// Computes the accuracy of a given . + /// + /// The to compute the total score of. + /// The score's accuracy. + [Pure] + public double ComputeAccuracy(ScoreInfo scoreInfo) + { + if (!ruleset.RulesetInfo.Equals(scoreInfo.Ruleset)) + throw new ArgumentException($"Unexpected score ruleset. Expected \"{ruleset.RulesetInfo.ShortName}\" but was \"{scoreInfo.Ruleset.ShortName}\"."); + + // We only extract scoring values from the score's statistics. This is because accuracy is always relative to the point of pass or fail rather than relative to the whole beatmap. + extractScoringValues(scoreInfo.Statistics, out var current, out var maximum); + + return maximum.BaseScore > 0 ? current.BaseScore / maximum.BaseScore : 1; + } + + /// + /// Computes the total score of a given . /// /// /// Does not require to have been called before use. From e29f5cb4568019d031c85394aaca8b648e471a78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Sep 2022 18:53:37 +0900 Subject: [PATCH 02/15] Fix new-style legacy skins with animated judgements not adding correct transforms Closes #16173. --- osu.Game/Skinning/LegacyJudgementPieceNew.cs | 2 +- osu.Game/Skinning/LegacyJudgementPieceOld.cs | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/LegacyJudgementPieceNew.cs b/osu.Game/Skinning/LegacyJudgementPieceNew.cs index b87fc9acb5..2cb055d8ba 100644 --- a/osu.Game/Skinning/LegacyJudgementPieceNew.cs +++ b/osu.Game/Skinning/LegacyJudgementPieceNew.cs @@ -56,7 +56,7 @@ namespace osu.Game.Skinning if (result != HitResult.Miss) { //new judgement shows old as a temporary effect - AddInternal(temporaryOldStyle = new LegacyJudgementPieceOld(result, createMainDrawable, 1.05f) + AddInternal(temporaryOldStyle = new LegacyJudgementPieceOld(result, createMainDrawable, 1.05f, true) { Blending = BlendingParameters.Additive, Anchor = Anchor.Centre, diff --git a/osu.Game/Skinning/LegacyJudgementPieceOld.cs b/osu.Game/Skinning/LegacyJudgementPieceOld.cs index c47f46ed2b..7e7f0f027d 100644 --- a/osu.Game/Skinning/LegacyJudgementPieceOld.cs +++ b/osu.Game/Skinning/LegacyJudgementPieceOld.cs @@ -19,10 +19,13 @@ namespace osu.Game.Skinning private readonly float finalScale; - public LegacyJudgementPieceOld(HitResult result, Func createMainDrawable, float finalScale = 1f) + private readonly bool allowTransforms; + + public LegacyJudgementPieceOld(HitResult result, Func createMainDrawable, float finalScale = 1f, bool allowTransforms = false) { this.result = result; this.finalScale = finalScale; + this.allowTransforms = allowTransforms; AutoSizeAxes = Axes.Both; Origin = Anchor.Centre; @@ -43,8 +46,8 @@ namespace osu.Game.Skinning this.FadeInFromZero(fade_in_length); this.Delay(fade_out_delay).FadeOut(fade_out_length); - // legacy judgements don't play any transforms if they are an animation. - if (animation?.FrameCount > 1) + // legacy judgements don't play any transforms if they are an animation.... UNLESS they are the temporary displayed judgement from new piece. + if (!allowTransforms && animation?.FrameCount > 1) return; switch (result) From 837cc760195cd5c0c4aa5c445657a42151b8b7ca Mon Sep 17 00:00:00 2001 From: OliBomby Date: Mon, 12 Sep 2022 12:50:16 +0200 Subject: [PATCH 03/15] Create TestSceneSelectionBlueprintDeselection.cs --- .../TestSceneSelectionBlueprintDeselection.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs new file mode 100644 index 0000000000..fdc05e9456 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs @@ -0,0 +1,35 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Osu.Tests.Editor +{ + public class TestSceneSelectionBlueprintDeselection : TestSceneOsuEditor + { + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); + + [Test] + public void TestSingleDeleteAtSameTime() + { + HitCircle? circle1 = null; + HitCircle? circle2 = null; + + AddStep("add two circles at the same time", () => + { + circle1 = new HitCircle(); + circle2 = new HitCircle(); + EditorClock.Seek(0); + EditorBeatmap.Add(circle1); + EditorBeatmap.Add(circle2); + EditorBeatmap.SelectedHitObjects.Add(circle1); + EditorBeatmap.SelectedHitObjects.Add(circle2); + }); + + AddStep("delete the first circle", () => EditorBeatmap.Remove(circle1)); + } + } +} From 2b4b14ca9990a4626fefcbecf624ca2d97bf4244 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 12 Sep 2022 20:06:52 +0900 Subject: [PATCH 04/15] Fix `SongProgress` invalidating too often This regressed with https://github.com/ppy/osu/pull/19556. Rather than try and figure whether that new container needs to handle size differently, this is a simple solution. Height was taken from a runtime check (maxes out at about 14.5). Closes #20235. --- osu.Game/Screens/Play/HUD/SongProgressInfo.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/HUD/SongProgressInfo.cs b/osu.Game/Screens/Play/HUD/SongProgressInfo.cs index d0eb8f8ca1..b3d5066a9e 100644 --- a/osu.Game/Screens/Play/HUD/SongProgressInfo.cs +++ b/osu.Game/Screens/Play/HUD/SongProgressInfo.cs @@ -47,7 +47,10 @@ namespace osu.Game.Screens.Play.HUD if (clock != null) gameplayClock = clock; - AutoSizeAxes = Axes.Y; + // Lock height so changes in text autosize (if character height changes) + // don't cause parent invalidation. + Height = 14; + Children = new Drawable[] { new Container From 8400de4b2ea1922138fc0d581f0ac8c3a531bd12 Mon Sep 17 00:00:00 2001 From: OliBomby Date: Mon, 12 Sep 2022 17:50:11 +0200 Subject: [PATCH 05/15] invoking hitobject updated before invoking removed --- .../TestSceneSelectionBlueprintDeselection.cs | 35 +++++++++++++-- osu.Game/Screens/Edit/EditorBeatmap.cs | 45 +++++++++++++------ 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs index fdc05e9456..b00582d6f3 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs @@ -1,10 +1,12 @@ // 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 NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Tests.Beatmaps; +using osuTK.Input; namespace osu.Game.Rulesets.Osu.Tests.Editor { @@ -16,13 +18,12 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor public void TestSingleDeleteAtSameTime() { HitCircle? circle1 = null; - HitCircle? circle2 = null; AddStep("add two circles at the same time", () => { - circle1 = new HitCircle(); - circle2 = new HitCircle(); EditorClock.Seek(0); + circle1 = new HitCircle(); + var circle2 = new HitCircle(); EditorBeatmap.Add(circle1); EditorBeatmap.Add(circle2); EditorBeatmap.SelectedHitObjects.Add(circle1); @@ -31,5 +32,33 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddStep("delete the first circle", () => EditorBeatmap.Remove(circle1)); } + + [Test] + public void TestBigStackDeleteAtSameTime() + { + AddStep("add 20 circles at the same time", () => + { + EditorClock.Seek(0); + + for (int i = 0; i < 20; i++) + { + EditorBeatmap.Add(new HitCircle()); + } + }); + + AddStep("select half of the circles", () => + { + foreach (var hitObject in EditorBeatmap.HitObjects.SkipLast(10).Reverse()) + { + EditorBeatmap.SelectedHitObjects.Add(hitObject); + } + }); + + AddStep("delete all selected circles", () => + { + InputManager.PressKey(Key.Delete); + InputManager.ReleaseKey(Key.Delete); + }); + } } } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 8aa754b305..95b848fec0 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -16,6 +16,7 @@ using osu.Game.Beatmaps.Legacy; using osu.Game.Beatmaps.Timing; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; using osu.Game.Skinning; namespace osu.Game.Screens.Edit @@ -79,7 +80,7 @@ namespace osu.Game.Screens.Edit private readonly IBeatmapProcessor beatmapProcessor; - private readonly Dictionary> startTimeBindables = new Dictionary>(); + private readonly Dictionary> hitObjectBindables = new Dictionary>(); public EditorBeatmap(IBeatmap playableBeatmap, ISkin beatmapSkin = null, BeatmapInfo beatmapInfo = null) { @@ -97,7 +98,7 @@ namespace osu.Game.Screens.Edit beatmapProcessor = playableBeatmap.BeatmapInfo.Ruleset.CreateInstance().CreateBeatmapProcessor(PlayableBeatmap); foreach (var obj in HitObjects) - trackStartTime(obj); + trackBindables(obj); } /// @@ -222,7 +223,7 @@ namespace osu.Game.Screens.Edit /// The to insert. public void Insert(int index, HitObject hitObject) { - trackStartTime(hitObject); + trackBindables(hitObject); mutableHitObjects.Insert(index, hitObject); @@ -299,9 +300,9 @@ namespace osu.Game.Screens.Edit mutableHitObjects.RemoveAt(index); - var bindable = startTimeBindables[hitObject]; - bindable.UnbindAll(); - startTimeBindables.Remove(hitObject); + var bindables = hitObjectBindables[hitObject]; + bindables.ForEach(b => b.UnbindAll()); + hitObjectBindables.Remove(hitObject); BeginChange(); batchPendingDeletes.Add(hitObject); @@ -325,25 +326,25 @@ namespace osu.Game.Screens.Edit beatmapProcessor?.PreProcess(); + foreach (var h in batchPendingUpdates) processHitObject(h); foreach (var h in batchPendingDeletes) processHitObject(h); foreach (var h in batchPendingInserts) processHitObject(h); - foreach (var h in batchPendingUpdates) processHitObject(h); beatmapProcessor?.PostProcess(); // callbacks may modify the lists so let's be safe about it + var updates = batchPendingUpdates.ToArray(); + batchPendingUpdates.Clear(); + var deletes = batchPendingDeletes.ToArray(); batchPendingDeletes.Clear(); var inserts = batchPendingInserts.ToArray(); batchPendingInserts.Clear(); - var updates = batchPendingUpdates.ToArray(); - batchPendingUpdates.Clear(); - + foreach (var h in updates) HitObjectUpdated?.Invoke(h); foreach (var h in deletes) HitObjectRemoved?.Invoke(h); foreach (var h in inserts) HitObjectAdded?.Invoke(h); - foreach (var h in updates) HitObjectUpdated?.Invoke(h); updateInProgress.Value = false; } @@ -355,10 +356,12 @@ namespace osu.Game.Screens.Edit private void processHitObject(HitObject hitObject) => hitObject.ApplyDefaults(ControlPointInfo, PlayableBeatmap.Difficulty); - private void trackStartTime(HitObject hitObject) + private void trackBindables(HitObject hitObject) { - startTimeBindables[hitObject] = hitObject.StartTimeBindable.GetBoundCopy(); - startTimeBindables[hitObject].ValueChanged += _ => + var bindables = new List(3); + + var startTimeBindable = hitObject.StartTimeBindable.GetBoundCopy(); + startTimeBindable.ValueChanged += _ => { // For now we'll remove and re-add the hitobject. This is not optimal and can be improved if required. mutableHitObjects.Remove(hitObject); @@ -368,6 +371,20 @@ namespace osu.Game.Screens.Edit Update(hitObject); }; + bindables.Add(startTimeBindable); + + if (hitObject is IHasComboInformation hasCombo) + { + var comboIndexBindable = hasCombo.ComboIndexBindable.GetBoundCopy(); + comboIndexBindable.ValueChanged += _ => Update(hitObject); + bindables.Add(comboIndexBindable); + + var indexInCurrentComboBindable = hasCombo.IndexInCurrentComboBindable.GetBoundCopy(); + indexInCurrentComboBindable.ValueChanged += _ => Update(hitObject); + bindables.Add(indexInCurrentComboBindable); + } + + hitObjectBindables[hitObject] = bindables; } private int findInsertionIndex(IReadOnlyList list, double startTime) From 718f8c4ee206bfeaa52177730fc74cb0c7d404b6 Mon Sep 17 00:00:00 2001 From: OliBomby Date: Tue, 13 Sep 2022 01:09:42 +0200 Subject: [PATCH 06/15] revert the fix --- osu.Game/Screens/Edit/EditorBeatmap.cs | 45 ++++++++------------------ 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 95b848fec0..8aa754b305 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -16,7 +16,6 @@ using osu.Game.Beatmaps.Legacy; using osu.Game.Beatmaps.Timing; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Types; using osu.Game.Skinning; namespace osu.Game.Screens.Edit @@ -80,7 +79,7 @@ namespace osu.Game.Screens.Edit private readonly IBeatmapProcessor beatmapProcessor; - private readonly Dictionary> hitObjectBindables = new Dictionary>(); + private readonly Dictionary> startTimeBindables = new Dictionary>(); public EditorBeatmap(IBeatmap playableBeatmap, ISkin beatmapSkin = null, BeatmapInfo beatmapInfo = null) { @@ -98,7 +97,7 @@ namespace osu.Game.Screens.Edit beatmapProcessor = playableBeatmap.BeatmapInfo.Ruleset.CreateInstance().CreateBeatmapProcessor(PlayableBeatmap); foreach (var obj in HitObjects) - trackBindables(obj); + trackStartTime(obj); } /// @@ -223,7 +222,7 @@ namespace osu.Game.Screens.Edit /// The to insert. public void Insert(int index, HitObject hitObject) { - trackBindables(hitObject); + trackStartTime(hitObject); mutableHitObjects.Insert(index, hitObject); @@ -300,9 +299,9 @@ namespace osu.Game.Screens.Edit mutableHitObjects.RemoveAt(index); - var bindables = hitObjectBindables[hitObject]; - bindables.ForEach(b => b.UnbindAll()); - hitObjectBindables.Remove(hitObject); + var bindable = startTimeBindables[hitObject]; + bindable.UnbindAll(); + startTimeBindables.Remove(hitObject); BeginChange(); batchPendingDeletes.Add(hitObject); @@ -326,25 +325,25 @@ namespace osu.Game.Screens.Edit beatmapProcessor?.PreProcess(); - foreach (var h in batchPendingUpdates) processHitObject(h); foreach (var h in batchPendingDeletes) processHitObject(h); foreach (var h in batchPendingInserts) processHitObject(h); + foreach (var h in batchPendingUpdates) processHitObject(h); beatmapProcessor?.PostProcess(); // callbacks may modify the lists so let's be safe about it - var updates = batchPendingUpdates.ToArray(); - batchPendingUpdates.Clear(); - var deletes = batchPendingDeletes.ToArray(); batchPendingDeletes.Clear(); var inserts = batchPendingInserts.ToArray(); batchPendingInserts.Clear(); - foreach (var h in updates) HitObjectUpdated?.Invoke(h); + var updates = batchPendingUpdates.ToArray(); + batchPendingUpdates.Clear(); + foreach (var h in deletes) HitObjectRemoved?.Invoke(h); foreach (var h in inserts) HitObjectAdded?.Invoke(h); + foreach (var h in updates) HitObjectUpdated?.Invoke(h); updateInProgress.Value = false; } @@ -356,12 +355,10 @@ namespace osu.Game.Screens.Edit private void processHitObject(HitObject hitObject) => hitObject.ApplyDefaults(ControlPointInfo, PlayableBeatmap.Difficulty); - private void trackBindables(HitObject hitObject) + private void trackStartTime(HitObject hitObject) { - var bindables = new List(3); - - var startTimeBindable = hitObject.StartTimeBindable.GetBoundCopy(); - startTimeBindable.ValueChanged += _ => + startTimeBindables[hitObject] = hitObject.StartTimeBindable.GetBoundCopy(); + startTimeBindables[hitObject].ValueChanged += _ => { // For now we'll remove and re-add the hitobject. This is not optimal and can be improved if required. mutableHitObjects.Remove(hitObject); @@ -371,20 +368,6 @@ namespace osu.Game.Screens.Edit Update(hitObject); }; - bindables.Add(startTimeBindable); - - if (hitObject is IHasComboInformation hasCombo) - { - var comboIndexBindable = hasCombo.ComboIndexBindable.GetBoundCopy(); - comboIndexBindable.ValueChanged += _ => Update(hitObject); - bindables.Add(comboIndexBindable); - - var indexInCurrentComboBindable = hasCombo.IndexInCurrentComboBindable.GetBoundCopy(); - indexInCurrentComboBindable.ValueChanged += _ => Update(hitObject); - bindables.Add(indexInCurrentComboBindable); - } - - hitObjectBindables[hitObject] = bindables; } private int findInsertionIndex(IReadOnlyList list, double startTime) From 5fba21d658335f88010cc1748891a9be65f6ea01 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 13 Sep 2022 02:21:01 +0300 Subject: [PATCH 07/15] Rename parameter to clarify purpose --- osu.Game/Skinning/LegacyJudgementPieceOld.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/LegacyJudgementPieceOld.cs b/osu.Game/Skinning/LegacyJudgementPieceOld.cs index 7e7f0f027d..3f4d13c082 100644 --- a/osu.Game/Skinning/LegacyJudgementPieceOld.cs +++ b/osu.Game/Skinning/LegacyJudgementPieceOld.cs @@ -18,14 +18,13 @@ namespace osu.Game.Skinning private readonly HitResult result; private readonly float finalScale; + private readonly bool forceTransforms; - private readonly bool allowTransforms; - - public LegacyJudgementPieceOld(HitResult result, Func createMainDrawable, float finalScale = 1f, bool allowTransforms = false) + public LegacyJudgementPieceOld(HitResult result, Func createMainDrawable, float finalScale = 1f, bool forceTransforms = false) { this.result = result; this.finalScale = finalScale; - this.allowTransforms = allowTransforms; + this.forceTransforms = forceTransforms; AutoSizeAxes = Axes.Both; Origin = Anchor.Centre; @@ -47,7 +46,7 @@ namespace osu.Game.Skinning this.Delay(fade_out_delay).FadeOut(fade_out_length); // legacy judgements don't play any transforms if they are an animation.... UNLESS they are the temporary displayed judgement from new piece. - if (!allowTransforms && animation?.FrameCount > 1) + if (animation?.FrameCount > 1 && !forceTransforms) return; switch (result) From a1f4724685c356272ca11342b0eff4eff3470b46 Mon Sep 17 00:00:00 2001 From: OliBomby Date: Tue, 13 Sep 2022 01:38:29 +0200 Subject: [PATCH 08/15] moved the location of the tests --- .../Editing}/TestSceneSelectionBlueprintDeselection.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) rename {osu.Game.Rulesets.Osu.Tests/Editor => osu.Game.Tests/Visual/Editing}/TestSceneSelectionBlueprintDeselection.cs (88%) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs b/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs similarity index 88% rename from osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs rename to osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs index b00582d6f3..6141e5f31c 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSelectionBlueprintDeselection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs @@ -4,14 +4,18 @@ using System.Linq; using NUnit.Framework; using osu.Game.Beatmaps; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Tests.Beatmaps; using osuTK.Input; -namespace osu.Game.Rulesets.Osu.Tests.Editor +namespace osu.Game.Tests.Visual.Editing { - public class TestSceneSelectionBlueprintDeselection : TestSceneOsuEditor + public class TestSceneSelectionBlueprintDeselection : EditorTestScene { + protected override Ruleset CreateEditorRuleset() => new OsuRuleset(); + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); [Test] From a6f68e4a908dc8a6593445f6ee639b1912577b66 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 13 Sep 2022 03:08:13 +0300 Subject: [PATCH 09/15] Fix NRT inspections --- osu.Game.Tests/Visual/SongSelect/TestSceneAdvancedStats.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneAdvancedStats.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneAdvancedStats.cs index 4510fda11d..1042341337 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneAdvancedStats.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneAdvancedStats.cs @@ -126,7 +126,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("select unchanged Difficulty Adjust mod", () => { var ruleset = advancedStats.BeatmapInfo.Ruleset.CreateInstance().AsNonNull(); - var difficultyAdjustMod = ruleset.CreateMod(); + var difficultyAdjustMod = ruleset.CreateMod().AsNonNull(); difficultyAdjustMod.ReadFromDifficulty(advancedStats.BeatmapInfo.Difficulty); SelectedMods.Value = new[] { difficultyAdjustMod }; }); @@ -145,7 +145,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("select changed Difficulty Adjust mod", () => { var ruleset = advancedStats.BeatmapInfo.Ruleset.CreateInstance().AsNonNull(); - var difficultyAdjustMod = ruleset.CreateMod(); + var difficultyAdjustMod = ruleset.CreateMod().AsNonNull(); var originalDifficulty = advancedStats.BeatmapInfo.Difficulty; difficultyAdjustMod.ReadFromDifficulty(originalDifficulty); From fd48249eef42c8af74fb50373744d58bc80eb3f7 Mon Sep 17 00:00:00 2001 From: OliBomby Date: Tue, 13 Sep 2022 02:20:52 +0200 Subject: [PATCH 10/15] fix with new event --- .../Components/HitObjectOrderedSelectionContainer.cs | 6 ++---- osu.Game/Screens/Edit/EditorBeatmap.cs | 8 ++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs b/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs index d6fd07c998..2d8dca9c2d 100644 --- a/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs @@ -24,11 +24,9 @@ namespace osu.Game.Screens.Edit.Compose.Components { base.LoadComplete(); - editorBeatmap.HitObjectUpdated += hitObjectUpdated; + editorBeatmap.SelectionBlueprintsShouldBeSorted += SortInternal; } - private void hitObjectUpdated(HitObject _) => SortInternal(); - public override void Add(SelectionBlueprint drawable) { SortInternal(); @@ -72,7 +70,7 @@ namespace osu.Game.Screens.Edit.Compose.Components base.Dispose(isDisposing); if (editorBeatmap != null) - editorBeatmap.HitObjectUpdated -= hitObjectUpdated; + editorBeatmap.SelectionBlueprintsShouldBeSorted -= SortInternal; } } } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 8aa754b305..82f2187901 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -48,6 +48,11 @@ namespace osu.Game.Screens.Edit /// public event Action HitObjectUpdated; + /// + /// Invoked after is updated during and blueprints need to be sorted immediately to prevent a crash. + /// + public event Action SelectionBlueprintsShouldBeSorted; + /// /// All currently selected s. /// @@ -331,6 +336,9 @@ namespace osu.Game.Screens.Edit beatmapProcessor?.PostProcess(); + // Signal selection blueprint sorting because it is possible that the beatmap processor changed the order of the selection blueprints + SelectionBlueprintsShouldBeSorted?.Invoke(); + // callbacks may modify the lists so let's be safe about it var deletes = batchPendingDeletes.ToArray(); batchPendingDeletes.Clear(); From 0ca4be5e5bf43db46950b56c900891e1fd1097d4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 13 Sep 2022 03:45:31 +0300 Subject: [PATCH 11/15] Fix one more inspection --- .../UI/DrawableRulesetDependencies.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs index cffbf2c9a1..52387a5740 100644 --- a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs +++ b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs @@ -53,20 +53,17 @@ namespace osu.Game.Rulesets.UI { var resources = ruleset.CreateResourceStore(); - if (resources != null) - { - var host = parent.Get(); + var host = parent.Get(); - TextureStore = new TextureStore(host.Renderer, parent.Get().CreateTextureLoaderStore(new NamespacedResourceStore(resources, @"Textures"))); - CacheAs(TextureStore = new FallbackTextureStore(host.Renderer, TextureStore, parent.Get())); + TextureStore = new TextureStore(host.Renderer, parent.Get().CreateTextureLoaderStore(new NamespacedResourceStore(resources, @"Textures"))); + CacheAs(TextureStore = new FallbackTextureStore(host.Renderer, TextureStore, parent.Get())); - SampleStore = parent.Get().GetSampleStore(new NamespacedResourceStore(resources, @"Samples")); - SampleStore.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - CacheAs(SampleStore = new FallbackSampleStore(SampleStore, parent.Get())); + SampleStore = parent.Get().GetSampleStore(new NamespacedResourceStore(resources, @"Samples")); + SampleStore.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; + CacheAs(SampleStore = new FallbackSampleStore(SampleStore, parent.Get())); - ShaderManager = new ShaderManager(host.Renderer, new NamespacedResourceStore(resources, @"Shaders")); - CacheAs(ShaderManager = new FallbackShaderManager(host.Renderer, ShaderManager, parent.Get())); - } + ShaderManager = new ShaderManager(host.Renderer, new NamespacedResourceStore(resources, @"Shaders")); + CacheAs(ShaderManager = new FallbackShaderManager(host.Renderer, ShaderManager, parent.Get())); RulesetConfigManager = parent.Get().GetConfigFor(ruleset); if (RulesetConfigManager != null) From 6bf6b7e125639aa3a20d972bb87cf2d712975b41 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Sep 2022 13:07:23 +0900 Subject: [PATCH 12/15] Fix null considerations in `DrawableRulesetDependencies` --- .../UI/DrawableRulesetDependencies.cs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs index 52387a5740..ad52b4affc 100644 --- a/osu.Game/Rulesets/UI/DrawableRulesetDependencies.cs +++ b/osu.Game/Rulesets/UI/DrawableRulesetDependencies.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 System.Collections.Generic; using System.IO; @@ -12,6 +10,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics.Rendering; using osu.Framework.Graphics.Shaders; using osu.Framework.Graphics.Textures; @@ -44,9 +43,9 @@ namespace osu.Game.Rulesets.UI public ShaderManager ShaderManager { get; } /// - /// The ruleset config manager. + /// The ruleset config manager. May be null if ruleset does not expose a configuration manager. /// - public IRulesetConfigManager RulesetConfigManager { get; private set; } + public IRulesetConfigManager? RulesetConfigManager { get; } public DrawableRulesetDependencies(Ruleset ruleset, IReadOnlyDependencyContainer parent) : base(parent) @@ -93,10 +92,9 @@ namespace osu.Game.Rulesets.UI isDisposed = true; - SampleStore?.Dispose(); - TextureStore?.Dispose(); - ShaderManager?.Dispose(); - RulesetConfigManager = null; + if (ShaderManager.IsNotNull()) SampleStore.Dispose(); + if (TextureStore.IsNotNull()) TextureStore.Dispose(); + if (ShaderManager.IsNotNull()) ShaderManager.Dispose(); } #endregion @@ -157,7 +155,7 @@ namespace osu.Game.Rulesets.UI public void Dispose() { - primary?.Dispose(); + if (primary.IsNotNull()) primary.Dispose(); } } @@ -182,7 +180,7 @@ namespace osu.Game.Rulesets.UI protected override void Dispose(bool disposing) { base.Dispose(disposing); - primary?.Dispose(); + if (primary.IsNotNull()) primary.Dispose(); } } @@ -198,12 +196,12 @@ namespace osu.Game.Rulesets.UI this.fallback = fallback; } - public override byte[] LoadRaw(string name) => primary.LoadRaw(name) ?? fallback.LoadRaw(name); + public override byte[]? LoadRaw(string name) => primary.LoadRaw(name) ?? fallback.LoadRaw(name); protected override void Dispose(bool disposing) { base.Dispose(disposing); - primary?.Dispose(); + if (primary.IsNotNull()) primary.Dispose(); } } } From ac034bffebe3597716bdaf14395452925881549d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Sep 2022 14:21:54 +0900 Subject: [PATCH 13/15] Fix potential crash if multiplayer spectator load is aborted early --- .../Multiplayer/Spectate/MultiSpectatorPlayer.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorPlayer.cs index 8e79c89685..6e939c3916 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorPlayer.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.Threading; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Game.Beatmaps; @@ -34,8 +35,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Spectate } [BackgroundDependencyLoader] - private void load() + private void load(CancellationToken cancellationToken) { + // HUD overlay may not be loaded if load has been cancelled early. + if (cancellationToken.IsCancellationRequested) + return; + HUDOverlay.PlayerSettingsOverlay.Expire(); HUDOverlay.HoldToQuit.Expire(); } From f53507828c0bfdb7909c895a0d921c6de31ef9ca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Sep 2022 14:59:30 +0900 Subject: [PATCH 14/15] Rename event to be more generic (and add comprehensive xmldoc) --- .../Components/HitObjectOrderedSelectionContainer.cs | 4 ++-- osu.Game/Screens/Edit/EditorBeatmap.cs | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs b/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs index 2d8dca9c2d..18bb6284b8 100644 --- a/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/HitObjectOrderedSelectionContainer.cs @@ -24,7 +24,7 @@ namespace osu.Game.Screens.Edit.Compose.Components { base.LoadComplete(); - editorBeatmap.SelectionBlueprintsShouldBeSorted += SortInternal; + editorBeatmap.BeatmapReprocessed += SortInternal; } public override void Add(SelectionBlueprint drawable) @@ -70,7 +70,7 @@ namespace osu.Game.Screens.Edit.Compose.Components base.Dispose(isDisposing); if (editorBeatmap != null) - editorBeatmap.SelectionBlueprintsShouldBeSorted -= SortInternal; + editorBeatmap.BeatmapReprocessed -= SortInternal; } } } diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index 82f2187901..16c0064e80 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -49,9 +49,13 @@ namespace osu.Game.Screens.Edit public event Action HitObjectUpdated; /// - /// Invoked after is updated during and blueprints need to be sorted immediately to prevent a crash. + /// Invoked after any state changes occurred which triggered a beatmap reprocess via an . /// - public event Action SelectionBlueprintsShouldBeSorted; + /// + /// Beatmap processing may change the order of hitobjects. This event gives external components a chance to handle any changes + /// not covered by the / / events. + /// + public event Action BeatmapReprocessed; /// /// All currently selected s. @@ -336,8 +340,7 @@ namespace osu.Game.Screens.Edit beatmapProcessor?.PostProcess(); - // Signal selection blueprint sorting because it is possible that the beatmap processor changed the order of the selection blueprints - SelectionBlueprintsShouldBeSorted?.Invoke(); + BeatmapReprocessed?.Invoke(); // callbacks may modify the lists so let's be safe about it var deletes = batchPendingDeletes.ToArray(); From 608c893b23f2007e6793de9cff1e72aec92fe70b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 13 Sep 2022 15:03:13 +0900 Subject: [PATCH 15/15] Add basic test guarantees --- .../Editing/TestSceneSelectionBlueprintDeselection.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs b/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs index 6141e5f31c..5c933468be 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneSelectionBlueprintDeselection.cs @@ -28,13 +28,17 @@ namespace osu.Game.Tests.Visual.Editing EditorClock.Seek(0); circle1 = new HitCircle(); var circle2 = new HitCircle(); + EditorBeatmap.Add(circle1); EditorBeatmap.Add(circle2); + EditorBeatmap.SelectedHitObjects.Add(circle1); EditorBeatmap.SelectedHitObjects.Add(circle2); }); AddStep("delete the first circle", () => EditorBeatmap.Remove(circle1)); + AddAssert("one hitobject remains", () => EditorBeatmap.HitObjects.Count == 1); + AddAssert("one hitobject selected", () => EditorBeatmap.SelectedHitObjects.Count == 1); } [Test] @@ -63,6 +67,9 @@ namespace osu.Game.Tests.Visual.Editing InputManager.PressKey(Key.Delete); InputManager.ReleaseKey(Key.Delete); }); + + AddAssert("10 hitobjects remain", () => EditorBeatmap.HitObjects.Count == 10); + AddAssert("no hitobjects selected", () => EditorBeatmap.SelectedHitObjects.Count == 0); } } }