From 3dd2b18ff0d32cb717bddd6c982e42ca3a7a723f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 16:04:58 +0900 Subject: [PATCH 01/10] Make EditorScreen abstract --- osu.Game/Screens/Edit/Editor.cs | 6 +----- osu.Game/Screens/Edit/EditorScreen.cs | 15 +++++---------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 9ebe3bc26a..7f08c2f8b9 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -19,13 +19,13 @@ using osu.Framework.Timing; using osu.Game.Graphics.UserInterface; using osu.Game.Screens.Edit.Components; using osu.Game.Screens.Edit.Components.Menus; -using osu.Game.Screens.Edit.Compose; using osu.Game.Screens.Edit.Design; using osuTK.Input; using System.Collections.Generic; using osu.Framework; using osu.Framework.Input.Bindings; using osu.Game.Input.Bindings; +using osu.Game.Screens.Edit.Compose; using osu.Game.Screens.Edit.Setup; using osu.Game.Screens.Edit.Timing; using osu.Game.Users; @@ -275,10 +275,6 @@ namespace osu.Game.Screens.Edit case EditorScreenMode.Timing: currentScreen = new TimingScreen(); break; - - default: - currentScreen = new EditorScreen(); - break; } LoadComponentAsync(currentScreen, screenContainer.Add); diff --git a/osu.Game/Screens/Edit/EditorScreen.cs b/osu.Game/Screens/Edit/EditorScreen.cs index 045e5a1226..1b57c703ae 100644 --- a/osu.Game/Screens/Edit/EditorScreen.cs +++ b/osu.Game/Screens/Edit/EditorScreen.cs @@ -10,16 +10,17 @@ using osu.Game.Beatmaps; namespace osu.Game.Screens.Edit { /// - /// TODO: eventually make this inherit Screen and add a local scren stack inside the Editor. + /// TODO: eventually make this inherit Screen and add a local screen stack inside the Editor. /// - public class EditorScreen : Container + public abstract class EditorScreen : Container { - protected readonly IBindable Beatmap = new Bindable(); + [Resolved] + protected IBindable Beatmap { get; private set; } protected override Container Content => content; private readonly Container content; - public EditorScreen() + protected EditorScreen() { Anchor = Anchor.Centre; Origin = Anchor.Centre; @@ -28,12 +29,6 @@ namespace osu.Game.Screens.Edit InternalChild = content = new Container { RelativeSizeAxes = Axes.Both }; } - [BackgroundDependencyLoader] - private void load(IBindable beatmap) - { - Beatmap.BindTo(beatmap); - } - protected override void LoadComplete() { base.LoadComplete(); From f2adae8fd167195b667989d30d7bc99b09e12ba3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 16:05:38 +0900 Subject: [PATCH 02/10] Rename test case to better match underlying class --- ...{TestSceneEditorCompose.cs => TestSceneComposeScreen.cs} | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) rename osu.Game.Tests/Visual/Editor/{TestSceneEditorCompose.cs => TestSceneComposeScreen.cs} (72%) diff --git a/osu.Game.Tests/Visual/Editor/TestSceneEditorCompose.cs b/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs similarity index 72% rename from osu.Game.Tests/Visual/Editor/TestSceneEditorCompose.cs rename to osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.cs index 608df1965e..9f16e1d781 100644 --- a/osu.Game.Tests/Visual/Editor/TestSceneEditorCompose.cs +++ b/osu.Game.Tests/Visual/Editor/TestSceneComposeScreen.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. -using System; -using System.Collections.Generic; using NUnit.Framework; using osu.Framework.Allocation; using osu.Game.Rulesets.Osu; @@ -11,10 +9,8 @@ using osu.Game.Screens.Edit.Compose; namespace osu.Game.Tests.Visual.Editor { [TestFixture] - public class TestSceneEditorCompose : EditorClockTestScene + public class TestSceneComposeScreen : EditorClockTestScene { - public override IReadOnlyList RequiredTypes => new[] { typeof(ComposeScreen) }; - [BackgroundDependencyLoader] private void load() { From 2d8e5615e4970fc1f47a94ed2690d6f2c0cb8a99 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 16:05:55 +0900 Subject: [PATCH 03/10] Extract timeline and layout logic from ComposeScreen --- .../Screens/Edit/Compose/ComposeScreen.cs | 111 +----------------- .../Screens/Edit/EditorScreenWithTimeline.cs | 107 +++++++++++++++++ 2 files changed, 112 insertions(+), 106 deletions(-) create mode 100644 osu.Game/Screens/Edit/EditorScreenWithTimeline.cs diff --git a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs index 2a99d81516..1d93c6d4a4 100644 --- a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs +++ b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs @@ -1,112 +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 JetBrains.Annotations; -using osu.Framework.Allocation; -using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Logging; -using osu.Game.Screens.Edit.Compose.Components; -using osu.Game.Screens.Edit.Compose.Components.Timeline; using osu.Game.Skinning; -using osuTK.Graphics; namespace osu.Game.Screens.Edit.Compose { - public class ComposeScreen : EditorScreen + public class ComposeScreen : EditorScreenWithTimeline { - private const float vertical_margins = 10; - private const float horizontal_margins = 20; - - private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); - - [BackgroundDependencyLoader(true)] - private void load([CanBeNull] BindableBeatDivisor beatDivisor) + protected override Drawable CreateMainContent() { - if (beatDivisor != null) - this.beatDivisor.BindTo(beatDivisor); - - Container composerContainer; - - Children = new Drawable[] - { - new GridContainer - { - RelativeSizeAxes = Axes.Both, - Content = new[] - { - new Drawable[] - { - new Container - { - Name = "Timeline", - RelativeSizeAxes = Axes.Both, - Children = new Drawable[] - { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = Color4.Black.Opacity(0.5f) - }, - new Container - { - Name = "Timeline content", - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Horizontal = horizontal_margins, Vertical = vertical_margins }, - Child = new GridContainer - { - RelativeSizeAxes = Axes.Both, - Content = new[] - { - new Drawable[] - { - new Container - { - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Right = 5 }, - Child = new TimelineArea { RelativeSizeAxes = Axes.Both } - }, - new BeatDivisorControl(beatDivisor) { RelativeSizeAxes = Axes.Both } - }, - }, - ColumnDimensions = new[] - { - new Dimension(), - new Dimension(GridSizeMode.Absolute, 90), - } - }, - } - } - } - }, - new Drawable[] - { - composerContainer = new Container - { - Name = "Composer content", - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Horizontal = horizontal_margins, Vertical = vertical_margins }, - } - } - }, - RowDimensions = new[] { new Dimension(GridSizeMode.Absolute, 110) } - }, - }; - var ruleset = Beatmap.Value.BeatmapInfo.Ruleset?.CreateInstance(); - if (ruleset == null) - { - Logger.Log("Beatmap doesn't have a ruleset assigned."); - // ExitRequested?.Invoke(); - return; - } - - var composer = ruleset.CreateHitObjectComposer(); - - Drawable content; + var composer = ruleset?.CreateHitObjectComposer(); if (composer != null) { @@ -118,18 +25,10 @@ namespace osu.Game.Screens.Edit.Compose // load the skinning hierarchy first. // this is intentionally done in two stages to ensure things are in a loaded state before exposing the ruleset to skin sources. - content = beatmapSkinProvider.WithChild(rulesetSkinProvider.WithChild(ruleset.CreateHitObjectComposer())); - } - else - { - content = new ScreenWhiteBox.UnderConstructionMessage($"{ruleset.Description}'s composer"); + return beatmapSkinProvider.WithChild(rulesetSkinProvider.WithChild(ruleset.CreateHitObjectComposer())); } - LoadComponentAsync(content, _ => - { - composerContainer.Add(content); - content.FadeInFromZero(300, Easing.OutQuint); - }); + return new ScreenWhiteBox.UnderConstructionMessage($"{ruleset.Description}'s composer"); } } } diff --git a/osu.Game/Screens/Edit/EditorScreenWithTimeline.cs b/osu.Game/Screens/Edit/EditorScreenWithTimeline.cs new file mode 100644 index 0000000000..752356e8c4 --- /dev/null +++ b/osu.Game/Screens/Edit/EditorScreenWithTimeline.cs @@ -0,0 +1,107 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using JetBrains.Annotations; +using osu.Framework.Allocation; +using osu.Framework.Extensions.Color4Extensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Game.Screens.Edit.Compose.Components; +using osu.Game.Screens.Edit.Compose.Components.Timeline; +using osuTK.Graphics; + +namespace osu.Game.Screens.Edit +{ + public abstract class EditorScreenWithTimeline : EditorScreen + { + private const float vertical_margins = 10; + private const float horizontal_margins = 20; + + private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); + + [BackgroundDependencyLoader(true)] + private void load([CanBeNull] BindableBeatDivisor beatDivisor) + { + if (beatDivisor != null) + this.beatDivisor.BindTo(beatDivisor); + + Container mainContent; + + Children = new Drawable[] + { + new GridContainer + { + RelativeSizeAxes = Axes.Both, + Content = new[] + { + new Drawable[] + { + new Container + { + Name = "Timeline", + RelativeSizeAxes = Axes.Both, + Children = new Drawable[] + { + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = Color4.Black.Opacity(0.5f) + }, + new Container + { + Name = "Timeline content", + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Horizontal = horizontal_margins, Vertical = vertical_margins }, + Child = new GridContainer + { + RelativeSizeAxes = Axes.Both, + Content = new[] + { + new Drawable[] + { + new Container + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Right = 5 }, + Child = CreateTimeline() + }, + new BeatDivisorControl(beatDivisor) { RelativeSizeAxes = Axes.Both } + }, + }, + ColumnDimensions = new[] + { + new Dimension(), + new Dimension(GridSizeMode.Absolute, 90), + } + }, + } + } + } + }, + new Drawable[] + { + mainContent = new Container + { + Name = "Main content", + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Horizontal = horizontal_margins, Vertical = vertical_margins }, + } + } + }, + RowDimensions = new[] { new Dimension(GridSizeMode.Absolute, 110) } + }, + }; + + LoadComponentAsync(CreateMainContent(), content => + { + mainContent.Add(content); + content.FadeInFromZero(300, Easing.OutQuint); + }); + } + + protected abstract Drawable CreateMainContent(); + + protected virtual Drawable CreateTimeline() => new TimelineArea { RelativeSizeAxes = Axes.Both }; + } +} From 0681bb9a2b27f063ae0153ca204b25a895334db1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 16:49:10 +0900 Subject: [PATCH 04/10] Fix potential nullref --- osu.Game/Screens/Edit/Compose/ComposeScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs index 1d93c6d4a4..2e9094ebe6 100644 --- a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs +++ b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs @@ -28,7 +28,7 @@ namespace osu.Game.Screens.Edit.Compose return beatmapSkinProvider.WithChild(rulesetSkinProvider.WithChild(ruleset.CreateHitObjectComposer())); } - return new ScreenWhiteBox.UnderConstructionMessage($"{ruleset.Description}'s composer"); + return new ScreenWhiteBox.UnderConstructionMessage(ruleset == null ? "This beatmap" : $"{ruleset.Description}'s composer"); } } } From 93d2c3d7a1c43cb9871f1bcb779b51cddcc086e9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:03:20 +0900 Subject: [PATCH 05/10] Warn on incorrect null usage --- osu.sln.DotSettings | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.sln.DotSettings b/osu.sln.DotSettings index ed162eed6e..ae08a1666f 100644 --- a/osu.sln.DotSettings +++ b/osu.sln.DotSettings @@ -17,7 +17,8 @@ WARNING WARNING HINT - HINT + + True HINT HINT WARNING @@ -738,6 +739,7 @@ See the LICENCE file in the repository root for full licence text. <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /> + True True True From 51bf600ea7a830d736a748342bd987e6cc4dc878 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:08:31 +0900 Subject: [PATCH 06/10] Use empty hitwindows instead of null --- .../Objects/CatchHitObject.cs | 2 +- osu.Game.Rulesets.Mania/Objects/HoldNote.cs | 2 +- .../Objects/HoldNoteTick.cs | 2 +- osu.Game.Rulesets.Osu/Objects/RepeatPoint.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Slider.cs | 2 +- .../Objects/SliderTailCircle.cs | 2 +- osu.Game.Rulesets.Osu/Objects/SliderTick.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Spinner.cs | 2 +- osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs | 2 +- .../Objects/DrumRollTick.cs | 2 +- .../Objects/StrongHitObject.cs | 2 +- osu.Game.Rulesets.Taiko/Objects/Swell.cs | 2 +- osu.Game.Rulesets.Taiko/Objects/SwellTick.cs | 2 +- osu.Game/Rulesets/Objects/HitObject.cs | 6 +--- .../Objects/Legacy/Mania/ConvertHit.cs | 2 +- .../Objects/Legacy/Mania/ConvertHold.cs | 2 +- .../Objects/Legacy/Mania/ConvertSlider.cs | 2 +- .../Objects/Legacy/Mania/ConvertSpinner.cs | 2 +- .../Rulesets/Objects/Legacy/Osu/ConvertHit.cs | 2 +- .../Objects/Legacy/Osu/ConvertSlider.cs | 2 +- .../Objects/Legacy/Osu/ConvertSpinner.cs | 2 +- .../Objects/Legacy/Taiko/ConvertHit.cs | 2 +- .../Objects/Legacy/Taiko/ConvertSlider.cs | 2 +- .../Objects/Legacy/Taiko/ConvertSpinner.cs | 2 +- osu.Game/Rulesets/Scoring/HitWindows.cs | 35 +++++++++++++++++++ osu.Game/Rulesets/UI/DrawableRuleset.cs | 4 +-- osu.Game/Screens/Play/HUD/HitErrorDisplay.cs | 2 +- 27 files changed, 62 insertions(+), 31 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs index 77d7de989a..e4ad49ea50 100644 --- a/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs +++ b/osu.Game.Rulesets.Catch/Objects/CatchHitObject.cs @@ -93,7 +93,7 @@ namespace osu.Game.Rulesets.Catch.Objects Scale = 1.0f - 0.7f * (difficulty.CircleSize - 5) / 5; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } public enum FruitVisualRepresentation diff --git a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs index 0c82cf7bbc..bdba813eed 100644 --- a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs @@ -101,6 +101,6 @@ namespace osu.Game.Rulesets.Mania.Objects public override Judgement CreateJudgement() => new HoldNoteJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Mania/Objects/HoldNoteTick.cs b/osu.Game.Rulesets.Mania/Objects/HoldNoteTick.cs index d0125f8793..ac6697a6dc 100644 --- a/osu.Game.Rulesets.Mania/Objects/HoldNoteTick.cs +++ b/osu.Game.Rulesets.Mania/Objects/HoldNoteTick.cs @@ -14,6 +14,6 @@ namespace osu.Game.Rulesets.Mania.Objects { public override Judgement CreateJudgement() => new HoldNoteTickJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Osu/Objects/RepeatPoint.cs b/osu.Game.Rulesets.Osu/Objects/RepeatPoint.cs index a794e57c9e..a277517f9f 100644 --- a/osu.Game.Rulesets.Osu/Objects/RepeatPoint.cs +++ b/osu.Game.Rulesets.Osu/Objects/RepeatPoint.cs @@ -30,6 +30,6 @@ namespace osu.Game.Rulesets.Osu.Objects public override Judgement CreateJudgement() => new OsuJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 3ed1f2cdde..9bed123465 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -205,6 +205,6 @@ namespace osu.Game.Rulesets.Osu.Objects public override Judgement CreateJudgement() => new OsuJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs b/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs index 7e540a577b..14c3369967 100644 --- a/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs +++ b/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs @@ -25,6 +25,6 @@ namespace osu.Game.Rulesets.Osu.Objects public override Judgement CreateJudgement() => new OsuSliderTailJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Osu/Objects/SliderTick.cs b/osu.Game.Rulesets.Osu/Objects/SliderTick.cs index af7cf5b144..a49f4cef8b 100644 --- a/osu.Game.Rulesets.Osu/Objects/SliderTick.cs +++ b/osu.Game.Rulesets.Osu/Objects/SliderTick.cs @@ -32,6 +32,6 @@ namespace osu.Game.Rulesets.Osu.Objects public override Judgement CreateJudgement() => new OsuJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Osu/Objects/Spinner.cs b/osu.Game.Rulesets.Osu/Objects/Spinner.cs index 2e7b763966..2441a1449d 100644 --- a/osu.Game.Rulesets.Osu/Objects/Spinner.cs +++ b/osu.Game.Rulesets.Osu/Objects/Spinner.cs @@ -33,6 +33,6 @@ namespace osu.Game.Rulesets.Osu.Objects public override Judgement CreateJudgement() => new OsuJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs index 4e02c76a8b..8956ca9c19 100644 --- a/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs +++ b/osu.Game.Rulesets.Taiko/Objects/DrumRoll.cs @@ -88,6 +88,6 @@ namespace osu.Game.Rulesets.Taiko.Objects public override Judgement CreateJudgement() => new TaikoDrumRollJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Taiko/Objects/DrumRollTick.cs b/osu.Game.Rulesets.Taiko/Objects/DrumRollTick.cs index c466ca7c8a..8a8be3e38d 100644 --- a/osu.Game.Rulesets.Taiko/Objects/DrumRollTick.cs +++ b/osu.Game.Rulesets.Taiko/Objects/DrumRollTick.cs @@ -27,6 +27,6 @@ namespace osu.Game.Rulesets.Taiko.Objects public override Judgement CreateJudgement() => new TaikoDrumRollTickJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Taiko/Objects/StrongHitObject.cs b/osu.Game.Rulesets.Taiko/Objects/StrongHitObject.cs index d660149528..72a04698be 100644 --- a/osu.Game.Rulesets.Taiko/Objects/StrongHitObject.cs +++ b/osu.Game.Rulesets.Taiko/Objects/StrongHitObject.cs @@ -11,6 +11,6 @@ namespace osu.Game.Rulesets.Taiko.Objects { public override Judgement CreateJudgement() => new TaikoStrongJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Taiko/Objects/Swell.cs b/osu.Game.Rulesets.Taiko/Objects/Swell.cs index f96c033dce..e60984596d 100644 --- a/osu.Game.Rulesets.Taiko/Objects/Swell.cs +++ b/osu.Game.Rulesets.Taiko/Objects/Swell.cs @@ -35,6 +35,6 @@ namespace osu.Game.Rulesets.Taiko.Objects public override Judgement CreateJudgement() => new TaikoSwellJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game.Rulesets.Taiko/Objects/SwellTick.cs b/osu.Game.Rulesets.Taiko/Objects/SwellTick.cs index 68212e8f12..91f4d3dbe7 100644 --- a/osu.Game.Rulesets.Taiko/Objects/SwellTick.cs +++ b/osu.Game.Rulesets.Taiko/Objects/SwellTick.cs @@ -11,6 +11,6 @@ namespace osu.Game.Rulesets.Taiko.Objects { public override Judgement CreateJudgement() => new TaikoSwellTickJudgement(); - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/HitObject.cs b/osu.Game/Rulesets/Objects/HitObject.cs index a99fac09cc..eb8652443f 100644 --- a/osu.Game/Rulesets/Objects/HitObject.cs +++ b/osu.Game/Rulesets/Objects/HitObject.cs @@ -66,7 +66,6 @@ namespace osu.Game.Rulesets.Objects /// /// The hit windows for this . /// - [CanBeNull] public HitWindows HitWindows { get; set; } private readonly List nestedHitObjects = new List(); @@ -113,10 +112,7 @@ namespace osu.Game.Rulesets.Objects nestedHitObjects.Sort((h1, h2) => h1.StartTime.CompareTo(h2.StartTime)); foreach (var h in nestedHitObjects) - { - h.HitWindows = HitWindows; h.ApplyDefaults(controlPointInfo, difficulty); - } } protected virtual void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, BeatmapDifficulty difficulty) @@ -147,7 +143,7 @@ namespace osu.Game.Rulesets.Objects /// This will only be invoked if hasn't been set externally (e.g. from a . /// /// - [CanBeNull] + [NotNull] protected virtual HitWindows CreateHitWindows() => new HitWindows(); } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHit.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHit.cs index 609bdd571a..883ef55df0 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHit.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHit.cs @@ -13,6 +13,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Mania { public float X { get; set; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHold.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHold.cs index 350ee3185d..69e6f8379d 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHold.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHold.cs @@ -14,6 +14,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Mania public double Duration => EndTime - StartTime; - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSlider.cs index e372fbd273..4486c5d906 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSlider.cs @@ -13,6 +13,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Mania { public float X { get; set; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSpinner.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSpinner.cs index 067377d300..c6d1f1922c 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSpinner.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertSpinner.cs @@ -17,6 +17,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Mania public float X { get; set; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs index c9851a0074..e40b5b4505 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHit.cs @@ -22,6 +22,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu public int ComboOffset { get; set; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs index 1c1180702b..a163329d47 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSlider.cs @@ -22,6 +22,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu public int ComboOffset { get; set; } - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs index bc94ea1803..5d96a61633 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertSpinner.cs @@ -22,7 +22,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu public float Y => Position.Y; - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; public bool NewCombo { get; set; } diff --git a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHit.cs b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHit.cs index 709345170f..efb9810927 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHit.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHit.cs @@ -10,6 +10,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Taiko /// internal sealed class ConvertHit : HitObject { - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSlider.cs index c173b3e11a..b365fd34ae 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSlider.cs @@ -10,6 +10,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Taiko /// internal sealed class ConvertSlider : Legacy.ConvertSlider { - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSpinner.cs b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSpinner.cs index 9a35ad2776..840ba51ac2 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSpinner.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertSpinner.cs @@ -15,6 +15,6 @@ namespace osu.Game.Rulesets.Objects.Legacy.Taiko public double Duration => EndTime - StartTime; - protected override HitWindows CreateHitWindows() => null; + protected override HitWindows CreateHitWindows() => HitWindows.Empty; } } diff --git a/osu.Game/Rulesets/Scoring/HitWindows.cs b/osu.Game/Rulesets/Scoring/HitWindows.cs index efc4cd9f5c..9e7e594104 100644 --- a/osu.Game/Rulesets/Scoring/HitWindows.cs +++ b/osu.Game/Rulesets/Scoring/HitWindows.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects; @@ -30,6 +31,17 @@ namespace osu.Game.Rulesets.Scoring private double meh; private double miss; + /// + /// An empty with only and . + /// No time values are provided (meaning instantaneous hit or miss). + /// + public static HitWindows Empty => new EmptyHitWindows(); + + public HitWindows() + { + Debug.Assert(GetRanges().Length >= 1, $"{nameof(HitWindows)}"); + } + /// /// Retrieves the with the largest hit window that produces a successful hit. /// @@ -168,6 +180,29 @@ namespace osu.Game.Rulesets.Scoring /// Defaults are provided but can be overridden to customise for a ruleset. /// protected virtual DifficultyRange[] GetRanges() => base_ranges; + + public class EmptyHitWindows : HitWindows + { + private static readonly DifficultyRange[] ranges = + { + new DifficultyRange(HitResult.Perfect, 0, 0, 0), + new DifficultyRange(HitResult.Miss, 0, 0, 0), + }; + + public override bool IsHitResultAllowed(HitResult result) + { + switch (result) + { + case HitResult.Great: + case HitResult.Miss: + return true; + } + + return false; + } + + protected override DifficultyRange[] GetRanges() => ranges; + } } public struct DifficultyRange diff --git a/osu.Game/Rulesets/UI/DrawableRuleset.cs b/osu.Game/Rulesets/UI/DrawableRuleset.cs index d68b0e94c5..d5b3df27df 100644 --- a/osu.Game/Rulesets/UI/DrawableRuleset.cs +++ b/osu.Game/Rulesets/UI/DrawableRuleset.cs @@ -426,11 +426,11 @@ namespace osu.Game.Rulesets.UI { foreach (var h in Objects) { - if (h.HitWindows != null) + if (h.HitWindows.WindowFor(HitResult.Miss) > 0) return h.HitWindows; foreach (var n in h.NestedHitObjects) - if (n.HitWindows != null) + if (h.HitWindows.WindowFor(HitResult.Miss) > 0) return n.HitWindows; } diff --git a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs index 920d11c910..54556f8648 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs @@ -48,7 +48,7 @@ namespace osu.Game.Screens.Play.HUD private void onNewJudgement(JudgementResult result) { - if (result.HitObject.HitWindows == null) + if (result.HitObject.HitWindows.WindowFor(HitResult.Miss) == 0) return; foreach (var c in Children) From ad6b8d3e04eccee5eaabf496b7b3df2d77c79659 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:08:55 +0900 Subject: [PATCH 07/10] Add result offset bounding to result itself, rather than just transforms --- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index f8bc74b2a6..7f3bfd3b5c 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -173,7 +173,7 @@ namespace osu.Game.Rulesets.Objects.Drawables { UpdateInitialTransforms(); - var judgementOffset = Math.Min(HitObject.HitWindows?.WindowFor(HitResult.Miss) ?? double.MaxValue, Result?.TimeOffset ?? 0); + var judgementOffset = Result?.TimeOffset ?? 0; using (BeginDelayedSequence(InitialLifetimeOffset + judgementOffset, true)) { @@ -379,7 +379,8 @@ namespace osu.Game.Rulesets.Objects.Drawables // Ensure that the judgement is given a valid time offset, because this may not get set by the caller var endTime = (HitObject as IHasEndTime)?.EndTime ?? HitObject.StartTime; - Result.TimeOffset = Time.Current - endTime; + + Result.TimeOffset = Math.Min(HitObject.HitWindows.WindowFor(HitResult.Miss), Time.Current - endTime); switch (Result.Type) { From 9f2a64843291fbe2e8dae6f86f8004559b75991b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:23:37 +0900 Subject: [PATCH 08/10] Add full asserts --- osu.Game/Rulesets/Scoring/HitWindows.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Scoring/HitWindows.cs b/osu.Game/Rulesets/Scoring/HitWindows.cs index 9e7e594104..93e4dfc5fa 100644 --- a/osu.Game/Rulesets/Scoring/HitWindows.cs +++ b/osu.Game/Rulesets/Scoring/HitWindows.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects; @@ -39,7 +40,8 @@ namespace osu.Game.Rulesets.Scoring public HitWindows() { - Debug.Assert(GetRanges().Length >= 1, $"{nameof(HitWindows)}"); + Debug.Assert(GetRanges().Any(r => r.Result == HitResult.Miss), $"{nameof(GetRanges)} should always contain {nameof(HitResult.Miss)}"); + Debug.Assert(GetRanges().Any(r => r.Result != HitResult.Miss), $"{nameof(GetRanges)} should always contain at least one result type other than {nameof(HitResult.Miss)}."); } /// From e6f857d0d8db175f6cae7b70df070227fb474387 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:03:20 +0900 Subject: [PATCH 09/10] Revert "Warn on incorrect null usage" This reverts commit 93d2c3d7a1c43cb9871f1bcb779b51cddcc086e9. --- osu.sln.DotSettings | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.sln.DotSettings b/osu.sln.DotSettings index ae08a1666f..ed162eed6e 100644 --- a/osu.sln.DotSettings +++ b/osu.sln.DotSettings @@ -17,8 +17,7 @@ WARNING WARNING HINT - - True + HINT HINT HINT WARNING @@ -739,7 +738,6 @@ See the LICENCE file in the repository root for full licence text. <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /> - True True True From 4e273fc628d2bea152dc93f65e9cdbf144c9c610 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 9 Oct 2019 19:50:05 +0900 Subject: [PATCH 10/10] Return correct allowed value for Perfect Co-Authored-By: Salman Ahmed --- osu.Game/Rulesets/Scoring/HitWindows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Scoring/HitWindows.cs b/osu.Game/Rulesets/Scoring/HitWindows.cs index 93e4dfc5fa..39d67f1071 100644 --- a/osu.Game/Rulesets/Scoring/HitWindows.cs +++ b/osu.Game/Rulesets/Scoring/HitWindows.cs @@ -195,7 +195,7 @@ namespace osu.Game.Rulesets.Scoring { switch (result) { - case HitResult.Great: + case HitResult.Perfect: case HitResult.Miss: return true; }