From 811874773288186d4e93620e44024331ebb59615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 2 May 2020 01:33:33 +0200 Subject: [PATCH] Make PeriodTracker actually immutable --- osu.Game.Tests/NonVisual/PeriodTrackerTest.cs | 54 +++++++------------ osu.Game/Screens/Play/BreakTracker.cs | 8 +-- osu.Game/Utils/PeriodTracker.cs | 35 +++--------- 3 files changed, 29 insertions(+), 68 deletions(-) diff --git a/osu.Game.Tests/NonVisual/PeriodTrackerTest.cs b/osu.Game.Tests/NonVisual/PeriodTrackerTest.cs index f033672576..62c7732b66 100644 --- a/osu.Game.Tests/NonVisual/PeriodTrackerTest.cs +++ b/osu.Game.Tests/NonVisual/PeriodTrackerTest.cs @@ -12,10 +12,9 @@ namespace osu.Game.Tests.NonVisual [TestFixture] public class PeriodTrackerTest { - private static readonly Period[] test_single_period = { new Period(1.0, 2.0) }; + private static readonly Period[] single_period = { new Period(1.0, 2.0) }; - // this is intended to be unordered to test adding periods in unordered way. - private static readonly Period[] test_periods = + private static readonly Period[] unordered_periods = { new Period(-9.1, -8.3), new Period(-3.4, 2.1), @@ -26,43 +25,40 @@ namespace osu.Game.Tests.NonVisual [Test] public void TestCheckValueInsideSinglePeriod() { - var tracker = new PeriodTracker { Periods = test_single_period }; + var tracker = new PeriodTracker(single_period); - var period = test_single_period.Single(); + var period = single_period.Single(); Assert.IsTrue(tracker.IsInAny(period.Start)); - Assert.IsTrue(tracker.IsInAny(getMidTime(period))); + Assert.IsTrue(tracker.IsInAny(getMidpoint(period))); Assert.IsTrue(tracker.IsInAny(period.End)); } [Test] public void TestCheckValuesInsidePeriods() { - var tracker = new PeriodTracker { Periods = test_periods }; + var tracker = new PeriodTracker(unordered_periods); - foreach (var period in test_periods) - Assert.IsTrue(tracker.IsInAny(getMidTime(period))); + foreach (var period in unordered_periods) + Assert.IsTrue(tracker.IsInAny(getMidpoint(period))); } [Test] public void TestCheckValuesInRandomOrder() { - var tracker = new PeriodTracker { Periods = test_periods }; + var tracker = new PeriodTracker(unordered_periods); - foreach (var period in test_periods.OrderBy(_ => RNG.Next())) - Assert.IsTrue(tracker.IsInAny(getMidTime(period))); + foreach (var period in unordered_periods.OrderBy(_ => RNG.Next())) + Assert.IsTrue(tracker.IsInAny(getMidpoint(period))); } [Test] public void TestCheckValuesOutOfPeriods() { - var tracker = new PeriodTracker + var tracker = new PeriodTracker(new[] { - Periods = new[] - { - new Period(1.0, 2.0), - new Period(3.0, 4.0) - } - }; + new Period(1.0, 2.0), + new Period(3.0, 4.0) + }); Assert.IsFalse(tracker.IsInAny(0.9), "Time before first period is being considered inside"); @@ -72,32 +68,18 @@ namespace osu.Game.Tests.NonVisual Assert.IsFalse(tracker.IsInAny(4.1), "Time after last period is being considered inside"); } - [Test] - public void TestNullRemovesExistingPeriods() - { - var tracker = new PeriodTracker { Periods = test_single_period }; - - var period = test_single_period.Single(); - Assert.IsTrue(tracker.IsInAny(getMidTime(period))); - - tracker.Periods = null; - Assert.IsFalse(tracker.IsInAny(getMidTime(period))); - } - [Test] public void TestReversedPeriodHandling() { - var tracker = new PeriodTracker(); - Assert.Throws(() => { - tracker.Periods = new[] + _ = new PeriodTracker(new[] { new Period(2.0, 1.0) - }; + }); }); } - private double getMidTime(Period period) => period.Start + (period.End - period.Start) / 2; + private double getMidpoint(Period period) => period.Start + (period.End - period.Start) / 2; } } diff --git a/osu.Game/Screens/Play/BreakTracker.cs b/osu.Game/Screens/Play/BreakTracker.cs index 79da548336..51e21656e1 100644 --- a/osu.Game/Screens/Play/BreakTracker.cs +++ b/osu.Game/Screens/Play/BreakTracker.cs @@ -16,7 +16,7 @@ namespace osu.Game.Screens.Play private readonly ScoreProcessor scoreProcessor; private readonly double gameplayStartTime; - private readonly PeriodTracker tracker = new PeriodTracker(); + private PeriodTracker breaks; /// /// Whether the gameplay is currently in a break. @@ -31,8 +31,8 @@ namespace osu.Game.Screens.Play { isBreakTime.Value = false; - tracker.Periods = value?.Where(b => b.HasEffect) - .Select(b => new Period(b.StartTime, b.EndTime - BreakOverlay.BREAK_FADE_DURATION)); + breaks = new PeriodTracker(value.Where(b => b.HasEffect) + .Select(b => new Period(b.StartTime, b.EndTime - BreakOverlay.BREAK_FADE_DURATION))); } } @@ -48,7 +48,7 @@ namespace osu.Game.Screens.Play var time = Clock.CurrentTime; - isBreakTime.Value = tracker.IsInAny(time) + isBreakTime.Value = breaks?.IsInAny(time) == true || time < gameplayStartTime || scoreProcessor?.HasCompleted.Value == true; } diff --git a/osu.Game/Utils/PeriodTracker.cs b/osu.Game/Utils/PeriodTracker.cs index 49b372bb27..ba77702247 100644 --- a/osu.Game/Utils/PeriodTracker.cs +++ b/osu.Game/Utils/PeriodTracker.cs @@ -8,41 +8,22 @@ using System.Linq; namespace osu.Game.Utils { /// - /// Represents a tracking component used for whether a - /// specific time falls into any of the provided periods. + /// Represents a tracking component used for whether a specific time instant falls into any of the provided periods. /// public class PeriodTracker { - private readonly List periods = new List(); + private readonly List periods; private int nearestIndex; - /// - /// The list of periods to add to the tracker for using the required check methods. - /// - public IEnumerable Periods + public PeriodTracker(IEnumerable periods) { - set - { - var sortedValue = value?.ToList(); - sortedValue?.Sort(); - - if (sortedValue != null && periods.SequenceEqual(sortedValue)) - return; - - periods.Clear(); - nearestIndex = 0; - - if (value?.Any() != true) - return; - - periods.AddRange(sortedValue); - } + this.periods = periods.OrderBy(period => period.Start).ToList(); } /// /// Whether the provided time is in any of the added periods. /// - /// The time value to check for. + /// The time value to check. public bool IsInAny(double time) { if (periods.Count == 0) @@ -64,7 +45,7 @@ namespace osu.Game.Utils } } - public readonly struct Period : IComparable + public readonly struct Period { /// /// The start time of this period. @@ -79,12 +60,10 @@ namespace osu.Game.Utils public Period(double start, double end) { if (start >= end) - throw new ArgumentException($"Invalid period provided, {nameof(start)} must be less than {nameof(end)}", nameof(start)); + throw new ArgumentException($"Invalid period provided, {nameof(start)} must be less than {nameof(end)}"); Start = start; End = end; } - - public int CompareTo(Period other) => Start.CompareTo(other.Start); } }