From 7b1ff38df7652b3eee6c159f50125f7809cefb4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 9 Sep 2019 23:41:51 +0200 Subject: [PATCH 01/20] Implement line-buffered reader Add a line-buffered reader decorator operating on StreamReader instances. The decorator has two main operations - PeekLine(), which allows to see the next line in the stream without consuming it, ReadLine(), which consumes and returns the next line in the stream, and ReadToEnd() which reads all the remaining text in the stream (including the unconsumed peeked line). Peeking line-per-line uses an internal queue of lines that have been read ahead from the underlying stream. The addition of the line-buffered reader is a workaround solution to a problem with decoding. At current selecting a decoder works by irreversibly reading the first line from the stream and looking for a magic string that indicates the type of decoder to use. It might however be possible for a file to be valid in format, just missing a header. In such a case a lack of a line-buffered reader makes it impossible to reparse the content of that first line. Introducing it will however allow to peek the first line for magic first. - If magic is found in the first line, GetDecoder() will peek it and use it to return the correct Decoder instance. Note that in the case of JsonBeatmapDecoder the magic is the opening JSON object brace, and therefore must not be consumed. - If magic is not found, the fallback decoder will be able to consume it using ReadLine() in Decode(). This commit additionally contains basic unit tests for the reader. Suggested-by: Aergwyn --- .../Beatmaps/IO/LineBufferedReaderTest.cs | 133 ++++++++++++++++++ osu.Game/IO/LineBufferedReader.cs | 70 +++++++++ 2 files changed, 203 insertions(+) create mode 100644 osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs create mode 100644 osu.Game/IO/LineBufferedReader.cs diff --git a/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs new file mode 100644 index 0000000000..b582ca0a6f --- /dev/null +++ b/osu.Game.Tests/Beatmaps/IO/LineBufferedReaderTest.cs @@ -0,0 +1,133 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.IO; +using System.Text; +using NUnit.Framework; +using osu.Game.IO; + +namespace osu.Game.Tests.Beatmaps.IO +{ + [TestFixture] + public class LineBufferedReaderTest + { + [Test] + public void TestReadLineByLine() + { + const string contents = @"line 1 +line 2 +line 3"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual("line 1", bufferedReader.ReadLine()); + Assert.AreEqual("line 2", bufferedReader.ReadLine()); + Assert.AreEqual("line 3", bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.ReadLine()); + } + } + + [Test] + public void TestPeekLineOnce() + { + const string contents = @"line 1 +peek this +line 3"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual("line 1", bufferedReader.ReadLine()); + Assert.AreEqual("peek this", bufferedReader.PeekLine()); + Assert.AreEqual("peek this", bufferedReader.ReadLine()); + Assert.AreEqual("line 3", bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.ReadLine()); + } + } + + [Test] + public void TestPeekLineMultipleTimes() + { + const string contents = @"peek this once +line 2 +peek this a lot"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual("peek this once", bufferedReader.PeekLine()); + Assert.AreEqual("peek this once", bufferedReader.ReadLine()); + Assert.AreEqual("line 2", bufferedReader.ReadLine()); + Assert.AreEqual("peek this a lot", bufferedReader.PeekLine()); + Assert.AreEqual("peek this a lot", bufferedReader.PeekLine()); + Assert.AreEqual("peek this a lot", bufferedReader.PeekLine()); + Assert.AreEqual("peek this a lot", bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.ReadLine()); + } + } + + [Test] + public void TestPeekLineAtEndOfStream() + { + const string contents = @"first line +second line"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual("first line", bufferedReader.ReadLine()); + Assert.AreEqual("second line", bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.PeekLine()); + Assert.IsNull(bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.PeekLine()); + } + } + + [Test] + public void TestPeekReadLineOnEmptyStream() + { + using (var stream = new MemoryStream()) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.IsNull(bufferedReader.PeekLine()); + Assert.IsNull(bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.ReadLine()); + Assert.IsNull(bufferedReader.PeekLine()); + } + } + + [Test] + public void TestReadToEndNoPeeks() + { + const string contents = @"first line +second line"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual(contents, bufferedReader.ReadToEnd()); + } + } + + [Test] + public void TestReadToEndAfterReadsAndPeeks() + { + const string contents = @"this line is gone +this one shouldn't be +these ones +definitely not"; + + using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(contents))) + using (var bufferedReader = new LineBufferedReader(stream)) + { + Assert.AreEqual("this line is gone", bufferedReader.ReadLine()); + Assert.AreEqual("this one shouldn't be", bufferedReader.PeekLine()); + const string ending = @"this one shouldn't be +these ones +definitely not"; + Assert.AreEqual(ending, bufferedReader.ReadToEnd()); + } + } + } +} diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs new file mode 100644 index 0000000000..66dee1181f --- /dev/null +++ b/osu.Game/IO/LineBufferedReader.cs @@ -0,0 +1,70 @@ +// 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 System.IO; +using System.Text; + +namespace osu.Game.IO +{ + /// + /// A -like decorator (with more limited API) for s + /// that allows lines to be peeked without consuming. + /// + public class LineBufferedReader : IDisposable + { + private readonly StreamReader streamReader; + private readonly Queue lineBuffer; + + public LineBufferedReader(Stream stream) + { + streamReader = new StreamReader(stream); + lineBuffer = new Queue(); + } + + /// + /// Reads the next line from the stream without consuming it. + /// + public string PeekLine() + { + if (lineBuffer.Count > 0) + return lineBuffer.Peek(); + + var line = streamReader.ReadLine(); + if (line != null) + lineBuffer.Enqueue(line); + return line; + } + + /// + /// Reads the next line from the stream and consumes it. + /// + public string ReadLine() => lineBuffer.Count > 0 ? lineBuffer.Dequeue() : streamReader.ReadLine(); + + /// + /// Reads the stream to its end and returns the text read. + /// This includes any peeked but unconsumed lines. + /// + public string ReadToEnd() + { + var remainingText = streamReader.ReadToEnd(); + if (lineBuffer.Count == 0) + return remainingText; + + var builder = new StringBuilder(); + + // this might not be completely correct due to varying platform line endings + while (lineBuffer.Count > 0) + builder.AppendLine(lineBuffer.Dequeue()); + builder.Append(remainingText); + + return builder.ToString(); + } + + public void Dispose() + { + streamReader?.Dispose(); + } + } +} From 11eda44d3451c889469af4e8e30d522b04d2dd15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 10 Sep 2019 00:43:30 +0200 Subject: [PATCH 02/20] Migrate decoding to line-buffered reader Migrate all usages of StreamReader in the context of decoding beatmaps, storyboards or skins to the new LineBufferedReader. --- osu.Game.Rulesets.Osu.Tests/StackingTest.cs | 3 +- .../Formats/LegacyBeatmapDecoderTest.cs | 42 +++++++++---------- .../Beatmaps/Formats/LegacyDecoderTest.cs | 4 +- .../Formats/LegacyStoryboardDecoderTest.cs | 6 +-- .../Beatmaps/Formats/OsuJsonDecoderTest.cs | 5 ++- .../Beatmaps/IO/OszArchiveReaderTest.cs | 3 +- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 6 +-- osu.Game.Tests/WaveformTestBeatmap.cs | 3 +- osu.Game/Beatmaps/BeatmapManager.cs | 5 ++- .../Beatmaps/BeatmapManager_WorkingBeatmap.cs | 8 ++-- osu.Game/Beatmaps/Formats/Decoder.cs | 9 ++-- .../Beatmaps/Formats/JsonBeatmapDecoder.cs | 7 +--- .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 3 +- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 4 +- .../Formats/LegacyStoryboardDecoder.cs | 3 +- osu.Game/Skinning/LegacySkin.cs | 3 +- .../Tests/Beatmaps/BeatmapConversionTest.cs | 3 +- .../Beatmaps/DifficultyCalculatorTest.cs | 3 +- osu.Game/Tests/Beatmaps/TestBeatmap.cs | 3 +- 19 files changed, 64 insertions(+), 59 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/StackingTest.cs b/osu.Game.Rulesets.Osu.Tests/StackingTest.cs index e8b99e86f9..871afdb09d 100644 --- a/osu.Game.Rulesets.Osu.Tests/StackingTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/StackingTest.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Text; using NUnit.Framework; using osu.Game.Beatmaps; +using osu.Game.IO; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Tests.Beatmaps; @@ -21,7 +22,7 @@ namespace osu.Game.Rulesets.Osu.Tests public void TestStacking() { using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(beatmap_data))) - using (var reader = new StreamReader(stream)) + using (var reader = new LineBufferedReader(stream)) { var beatmap = Decoder.GetDecoder(reader).Decode(reader); var converted = new TestWorkingBeatmap(beatmap).GetPlayableBeatmap(new OsuRuleset().RulesetInfo, Array.Empty()); diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 535320530d..6a77c6ca96 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.IO; using NUnit.Framework; using osuTK; using osuTK.Graphics; @@ -13,6 +12,7 @@ using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects.Types; using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.Timing; +using osu.Game.IO; using osu.Game.Rulesets.Catch.Beatmaps; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; @@ -30,13 +30,9 @@ namespace osu.Game.Tests.Beatmaps.Formats public void TestDecodeBeatmapVersion() { using (var resStream = TestResources.OpenResource("beatmap-version.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var decoder = Decoder.GetDecoder(stream); - - stream.BaseStream.Position = 0; - stream.DiscardBufferedData(); - var working = new TestWorkingBeatmap(decoder.Decode(stream)); Assert.AreEqual(6, working.BeatmapInfo.BeatmapVersion); @@ -51,7 +47,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); var beatmapInfo = beatmap.BeatmapInfo; @@ -75,7 +71,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder(); using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmapInfo = decoder.Decode(stream).BeatmapInfo; @@ -101,7 +97,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder(); using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); var beatmapInfo = beatmap.BeatmapInfo; @@ -126,7 +122,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder(); using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var difficulty = decoder.Decode(stream).BeatmapInfo.BaseDifficulty; @@ -145,7 +141,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); var metadata = beatmap.Metadata; @@ -164,7 +160,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); var controlPoints = beatmap.ControlPointInfo; @@ -239,7 +235,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("overlapping-control-points.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var controlPoints = decoder.Decode(stream).ControlPointInfo; @@ -271,7 +267,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacySkinDecoder(); using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var comboColors = decoder.Decode(stream).ComboColours; @@ -297,7 +293,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder(); using (var resStream = TestResources.OpenResource("hitobject-combo-offset.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); @@ -320,7 +316,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder(); using (var resStream = TestResources.OpenResource("hitobject-combo-offset.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var beatmap = decoder.Decode(stream); @@ -343,7 +339,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("Soleily - Renatus (Gamu) [Insane].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -371,7 +367,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("controlpoint-custom-samplebank.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -393,7 +389,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("hitobject-custom-samplebank.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -411,7 +407,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("hitobject-file-samples.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -431,7 +427,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("slider-samples.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -475,7 +471,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var resStream = TestResources.OpenResource("hitobject-no-addition-bank.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var hitObjects = decoder.Decode(stream).HitObjects; @@ -489,7 +485,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; using (var badResStream = TestResources.OpenResource("invalid-events.osu")) - using (var badStream = new StreamReader(badResStream)) + using (var badStream = new LineBufferedReader(badResStream)) { Assert.DoesNotThrow(() => decoder.Decode(badStream)); } diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyDecoderTest.cs index b4d219456c..335a6aeeb0 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyDecoderTest.cs @@ -2,9 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.IO; using NUnit.Framework; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.Tests.Resources; namespace osu.Game.Tests.Beatmaps.Formats @@ -18,7 +18,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LineLoggingDecoder(14); using (var resStream = TestResources.OpenResource("comments.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { decoder.Decode(stream); diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs index 953763c95d..66d53d7e7b 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs @@ -1,12 +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.IO; using System.Linq; using NUnit.Framework; using osuTK; using osu.Framework.Graphics; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.Storyboards; using osu.Game.Tests.Resources; @@ -21,7 +21,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyStoryboardDecoder(); using (var resStream = TestResources.OpenResource("Himeringo - Yotsuya-san ni Yoroshiku (RLC) [Winber1's Extreme].osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var storyboard = decoder.Decode(stream); @@ -94,7 +94,7 @@ namespace osu.Game.Tests.Beatmaps.Formats var decoder = new LegacyStoryboardDecoder(); using (var resStream = TestResources.OpenResource("variable-with-suffix.osb")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var storyboard = decoder.Decode(stream); diff --git a/osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs index 4859abbb8e..63346b8c9d 100644 --- a/osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs @@ -8,6 +8,7 @@ using NUnit.Framework; using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.IO.Serialization; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; @@ -148,13 +149,13 @@ namespace osu.Game.Tests.Beatmaps.Formats private Beatmap decode(string filename, out Beatmap jsonDecoded) { using (var stream = TestResources.OpenResource(filename)) - using (var sr = new StreamReader(stream)) + using (var sr = new LineBufferedReader(stream)) { var legacyDecoded = new LegacyBeatmapDecoder { ApplyOffsets = false }.Decode(sr); using (var ms = new MemoryStream()) using (var sw = new StreamWriter(ms)) - using (var sr2 = new StreamReader(ms)) + using (var sr2 = new LineBufferedReader(ms)) { sw.Write(legacyDecoded.Serialize()); sw.Flush(); diff --git a/osu.Game.Tests/Beatmaps/IO/OszArchiveReaderTest.cs b/osu.Game.Tests/Beatmaps/IO/OszArchiveReaderTest.cs index 37e0565df0..022b2c1a59 100644 --- a/osu.Game.Tests/Beatmaps/IO/OszArchiveReaderTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/OszArchiveReaderTest.cs @@ -7,6 +7,7 @@ using NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Tests.Resources; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.IO.Archives; namespace osu.Game.Tests.Beatmaps.IO @@ -50,7 +51,7 @@ namespace osu.Game.Tests.Beatmaps.IO Beatmap beatmap; - using (var stream = new StreamReader(reader.GetStream("Soleily - Renatus (Deif) [Platter].osu"))) + using (var stream = new LineBufferedReader(reader.GetStream("Soleily - Renatus (Deif) [Platter].osu"))) beatmap = Decoder.GetDecoder(stream).Decode(stream); var meta = beatmap.Metadata; diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index 8bd846518b..0d96dd08da 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs @@ -2,8 +2,8 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.IO; using NUnit.Framework; +using osu.Game.IO; using osu.Game.Skinning; using osu.Game.Tests.Resources; using osuTK.Graphics; @@ -20,7 +20,7 @@ namespace osu.Game.Tests.Skins var decoder = new LegacySkinDecoder(); using (var resStream = TestResources.OpenResource(hasColours ? "skin.ini" : "skin-empty.ini")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var comboColors = decoder.Decode(stream).ComboColours; @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Skins var decoder = new LegacySkinDecoder(); using (var resStream = TestResources.OpenResource("skin.ini")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var config = decoder.Decode(stream); diff --git a/osu.Game.Tests/WaveformTestBeatmap.cs b/osu.Game.Tests/WaveformTestBeatmap.cs index db9576b5fa..0d16a78f75 100644 --- a/osu.Game.Tests/WaveformTestBeatmap.cs +++ b/osu.Game.Tests/WaveformTestBeatmap.cs @@ -9,6 +9,7 @@ using osu.Framework.Graphics.Textures; using osu.Framework.Graphics.Video; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Tests.Resources; @@ -56,7 +57,7 @@ namespace osu.Game.Tests private Beatmap createTestBeatmap() { using (var beatmapStream = getBeatmapStream()) - using (var beatmapReader = new StreamReader(beatmapStream)) + using (var beatmapReader = new LineBufferedReader(beatmapStream)) return Decoder.GetDecoder(beatmapReader).Decode(beatmapReader); } } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index b9ed3664ef..372eec50fd 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -20,6 +20,7 @@ using osu.Framework.Platform; using osu.Framework.Threading; using osu.Game.Beatmaps.Formats; using osu.Game.Database; +using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -264,7 +265,7 @@ namespace osu.Game.Beatmaps } Beatmap beatmap; - using (var stream = new StreamReader(reader.GetStream(mapName))) + using (var stream = new LineBufferedReader(reader.GetStream(mapName))) beatmap = Decoder.GetDecoder(stream).Decode(stream); return new BeatmapSetInfo @@ -287,7 +288,7 @@ namespace osu.Game.Beatmaps { using (var raw = reader.GetStream(name)) using (var ms = new MemoryStream()) //we need a memory stream so we can seek - using (var sr = new StreamReader(ms)) + using (var sr = new LineBufferedReader(ms)) { raw.CopyTo(ms); ms.Position = 0; diff --git a/osu.Game/Beatmaps/BeatmapManager_WorkingBeatmap.cs b/osu.Game/Beatmaps/BeatmapManager_WorkingBeatmap.cs index 1d00c94ef2..b879b92f01 100644 --- a/osu.Game/Beatmaps/BeatmapManager_WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/BeatmapManager_WorkingBeatmap.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.IO; using System.Linq; using osu.Framework.Audio; using osu.Framework.Audio.Track; @@ -11,6 +10,7 @@ using osu.Framework.Graphics.Video; using osu.Framework.IO.Stores; using osu.Framework.Logging; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.Skinning; using osu.Game.Storyboards; @@ -33,7 +33,7 @@ namespace osu.Game.Beatmaps { try { - using (var stream = new StreamReader(store.GetStream(getPathForFile(BeatmapInfo.Path)))) + using (var stream = new LineBufferedReader(store.GetStream(getPathForFile(BeatmapInfo.Path)))) return Decoder.GetDecoder(stream).Decode(stream); } catch @@ -127,7 +127,7 @@ namespace osu.Game.Beatmaps try { - using (var stream = new StreamReader(store.GetStream(getPathForFile(BeatmapInfo.Path)))) + using (var stream = new LineBufferedReader(store.GetStream(getPathForFile(BeatmapInfo.Path)))) { var decoder = Decoder.GetDecoder(stream); @@ -136,7 +136,7 @@ namespace osu.Game.Beatmaps storyboard = decoder.Decode(stream); else { - using (var secondaryStream = new StreamReader(store.GetStream(getPathForFile(BeatmapSetInfo.StoryboardFile)))) + using (var secondaryStream = new LineBufferedReader(store.GetStream(getPathForFile(BeatmapSetInfo.StoryboardFile)))) storyboard = decoder.Decode(stream, secondaryStream); } } diff --git a/osu.Game/Beatmaps/Formats/Decoder.cs b/osu.Game/Beatmaps/Formats/Decoder.cs index 953e50eadc..0f1b617809 100644 --- a/osu.Game/Beatmaps/Formats/Decoder.cs +++ b/osu.Game/Beatmaps/Formats/Decoder.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using osu.Game.IO; namespace osu.Game.Beatmaps.Formats { @@ -13,15 +14,15 @@ namespace osu.Game.Beatmaps.Formats { protected virtual TOutput CreateTemplateObject() => new TOutput(); - public TOutput Decode(StreamReader primaryStream, params StreamReader[] otherStreams) + public TOutput Decode(LineBufferedReader primaryStream, params LineBufferedReader[] otherStreams) { var output = CreateTemplateObject(); - foreach (StreamReader stream in otherStreams.Prepend(primaryStream)) + foreach (LineBufferedReader stream in otherStreams.Prepend(primaryStream)) ParseStreamInto(stream, output); return output; } - protected abstract void ParseStreamInto(StreamReader stream, TOutput output); + protected abstract void ParseStreamInto(LineBufferedReader stream, TOutput output); } public abstract class Decoder @@ -39,7 +40,7 @@ namespace osu.Game.Beatmaps.Formats /// Retrieves a to parse a . /// /// A stream pointing to the . - public static Decoder GetDecoder(StreamReader stream) + public static Decoder GetDecoder(LineBufferedReader stream) where T : new() { if (stream == null) diff --git a/osu.Game/Beatmaps/Formats/JsonBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/JsonBeatmapDecoder.cs index d8482b200f..988968fa42 100644 --- a/osu.Game/Beatmaps/Formats/JsonBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/JsonBeatmapDecoder.cs @@ -1,7 +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.IO; +using osu.Game.IO; using osu.Game.IO.Serialization; namespace osu.Game.Beatmaps.Formats @@ -13,11 +13,8 @@ namespace osu.Game.Beatmaps.Formats AddDecoder("{", m => new JsonBeatmapDecoder()); } - protected override void ParseStreamInto(StreamReader stream, Beatmap output) + protected override void ParseStreamInto(LineBufferedReader stream, Beatmap output) { - stream.BaseStream.Position = 0; - stream.DiscardBufferedData(); - stream.ReadToEnd().DeserializeInto(output); foreach (var hitObject in output.HitObjects) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 0532790f0a..6e34fd8e90 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -8,6 +8,7 @@ using osu.Framework.IO.File; using osu.Game.Beatmaps.Timing; using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Beatmaps.ControlPoints; +using osu.Game.IO; namespace osu.Game.Beatmaps.Formats { @@ -41,7 +42,7 @@ namespace osu.Game.Beatmaps.Formats offset = FormatVersion < 5 ? 24 : 0; } - protected override void ParseStreamInto(StreamReader stream, Beatmap beatmap) + protected override void ParseStreamInto(LineBufferedReader stream, Beatmap beatmap) { this.beatmap = beatmap; this.beatmap.BeatmapInfo.BeatmapVersion = FormatVersion; diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 9a8197ad82..83d20da458 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -3,10 +3,10 @@ using System; using System.Collections.Generic; -using System.IO; using osu.Framework.Logging; using osu.Game.Audio; using osu.Game.Beatmaps.ControlPoints; +using osu.Game.IO; using osuTK.Graphics; namespace osu.Game.Beatmaps.Formats @@ -21,7 +21,7 @@ namespace osu.Game.Beatmaps.Formats FormatVersion = version; } - protected override void ParseStreamInto(StreamReader stream, T output) + protected override void ParseStreamInto(LineBufferedReader stream, T output) { Section section = Section.None; diff --git a/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs index 14c6ea5c8e..a8f8dbf69d 100644 --- a/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs @@ -10,6 +10,7 @@ using osuTK; using osuTK.Graphics; using osu.Framework.Graphics; using osu.Framework.IO.File; +using osu.Game.IO; using osu.Game.Storyboards; namespace osu.Game.Beatmaps.Formats @@ -35,7 +36,7 @@ namespace osu.Game.Beatmaps.Formats AddDecoder(@"[Events]", m => new LegacyStoryboardDecoder()); } - protected override void ParseStreamInto(StreamReader stream, Storyboard storyboard) + protected override void ParseStreamInto(LineBufferedReader stream, Storyboard storyboard) { this.storyboard = storyboard; base.ParseStreamInto(stream, storyboard); diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 0b1076be01..fea15458e4 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -12,6 +12,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; using osu.Game.Audio; +using osu.Game.IO; using osu.Game.Rulesets.Scoring; using osuTK.Graphics; @@ -35,7 +36,7 @@ namespace osu.Game.Skinning { Stream stream = storage?.GetStream(filename); if (stream != null) - using (StreamReader reader = new StreamReader(stream)) + using (LineBufferedReader reader = new LineBufferedReader(stream)) Configuration = new LegacySkinDecoder().Decode(reader); else Configuration = new DefaultSkinConfiguration(); diff --git a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs index 3fc9662b17..e99b5fc5fb 100644 --- a/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs +++ b/osu.Game/Tests/Beatmaps/BeatmapConversionTest.cs @@ -13,6 +13,7 @@ using osu.Framework.Graphics.Textures; using osu.Framework.Graphics.Video; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; @@ -142,7 +143,7 @@ namespace osu.Game.Tests.Beatmaps private IBeatmap getBeatmap(string name) { using (var resStream = openResource($"{resource_namespace}.{name}.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var decoder = Decoder.GetDecoder(stream); ((LegacyBeatmapDecoder)decoder).ApplyOffsets = false; diff --git a/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs b/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs index 108fa8ff71..748a52d1c5 100644 --- a/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs +++ b/osu.Game/Tests/Beatmaps/DifficultyCalculatorTest.cs @@ -7,6 +7,7 @@ using System.Reflection; using NUnit.Framework; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.IO; using osu.Game.Rulesets; using osu.Game.Rulesets.Difficulty; using osu.Game.Rulesets.Mods; @@ -26,7 +27,7 @@ namespace osu.Game.Tests.Beatmaps private WorkingBeatmap getBeatmap(string name) { using (var resStream = openResource($"{resource_namespace}.{name}.osu")) - using (var stream = new StreamReader(resStream)) + using (var stream = new LineBufferedReader(resStream)) { var decoder = Decoder.GetDecoder(stream); ((LegacyBeatmapDecoder)decoder).ApplyOffsets = false; diff --git a/osu.Game/Tests/Beatmaps/TestBeatmap.cs b/osu.Game/Tests/Beatmaps/TestBeatmap.cs index b77a8508ad..d6f92ba086 100644 --- a/osu.Game/Tests/Beatmaps/TestBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestBeatmap.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Text; using osu.Game.Beatmaps; +using osu.Game.IO; using osu.Game.Rulesets; using Decoder = osu.Game.Beatmaps.Formats.Decoder; @@ -39,7 +40,7 @@ namespace osu.Game.Tests.Beatmaps private static Beatmap createTestBeatmap() { using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(test_beatmap_data))) - using (var reader = new StreamReader(stream)) + using (var reader = new LineBufferedReader(stream)) return Decoder.GetDecoder(reader).Decode(reader); } From 86588778b170b363b2e7156a9ffa08ba32e15f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 10 Sep 2019 22:06:10 +0200 Subject: [PATCH 03/20] Implement fallback decoder registration After the preparatory introduction of LineBufferedReader, it is now possible to introduce registration of fallback decoders that won't drop input supplied in the first line of the file. A fallback decoder is used when the magic in the first line of the file does not match any of the other known decoders. In such a case, the fallback decoder is constructed and provided a LineBufferedReader instance. The process of matching magic only peeks the first non-empty line, so it is available for re-reading in Decode() using ReadLine(). There can be only one fallback decoder per type; a second attempt of registering a fallback will result in an exception to avoid bugs. To address the issue of parsing failing on badly or non-headered files, set the legacy decoders for Beatmaps and Storyboards as the fallbacks. Due to non-trivial logic, several new, passing unit tests with possible edge cases also included. --- .../Formats/LegacyBeatmapDecoderTest.cs | 101 ++++++++++++++++++ .../Beatmaps/IO/ImportBeatmapTest.cs | 2 +- osu.Game.Tests/Resources/corrupted-header.osu | 5 + .../empty-line-instead-of-header.osu | 5 + .../Resources/empty-lines-at-start.osu | 8 ++ osu.Game.Tests/Resources/missing-header.osu | 4 + .../Resources/no-empty-line-after-header.osu | 4 + osu.Game.Tests/osu.Game.Tests.csproj | 7 ++ osu.Game/Beatmaps/Formats/Decoder.cs | 41 +++++-- .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 1 + .../Formats/LegacyStoryboardDecoder.cs | 1 + osu.Game/IO/LineBufferedReader.cs | 2 + 12 files changed, 172 insertions(+), 9 deletions(-) create mode 100644 osu.Game.Tests/Resources/corrupted-header.osu create mode 100644 osu.Game.Tests/Resources/empty-line-instead-of-header.osu create mode 100644 osu.Game.Tests/Resources/empty-lines-at-start.osu create mode 100644 osu.Game.Tests/Resources/missing-header.osu create mode 100644 osu.Game.Tests/Resources/no-empty-line-after-header.osu diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 6a77c6ca96..f6c0dbbecf 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.IO; using NUnit.Framework; using osuTK; using osuTK.Graphics; @@ -490,5 +491,105 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.DoesNotThrow(() => decoder.Decode(badStream)); } } + + [Test] + public void TestFallbackDecoderForCorruptedHeader() + { + Decoder decoder = null; + Beatmap beatmap = null; + + using (var resStream = TestResources.OpenResource("corrupted-header.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.DoesNotThrow(() => decoder = Decoder.GetDecoder(stream)); + Assert.IsInstanceOf(decoder); + Assert.DoesNotThrow(() => beatmap = decoder.Decode(stream)); + Assert.IsNotNull(beatmap); + Assert.AreEqual("Beatmap with corrupted header", beatmap.Metadata.Title); + Assert.AreEqual("Evil Hacker", beatmap.Metadata.AuthorString); + } + } + + [Test] + public void TestFallbackDecoderForMissingHeader() + { + Decoder decoder = null; + Beatmap beatmap = null; + + using (var resStream = TestResources.OpenResource("missing-header.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.DoesNotThrow(() => decoder = Decoder.GetDecoder(stream)); + Assert.IsInstanceOf(decoder); + Assert.DoesNotThrow(() => beatmap = decoder.Decode(stream)); + Assert.IsNotNull(beatmap); + Assert.AreEqual("Beatmap with no header", beatmap.Metadata.Title); + Assert.AreEqual("Incredibly Evil Hacker", beatmap.Metadata.AuthorString); + } + } + + [Test] + public void TestDecodeFileWithEmptyLinesAtStart() + { + Decoder decoder = null; + Beatmap beatmap = null; + + using (var resStream = TestResources.OpenResource("empty-lines-at-start.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.DoesNotThrow(() => decoder = Decoder.GetDecoder(stream)); + Assert.IsInstanceOf(decoder); + Assert.DoesNotThrow(() => beatmap = decoder.Decode(stream)); + Assert.IsNotNull(beatmap); + Assert.AreEqual("Empty lines at start", beatmap.Metadata.Title); + Assert.AreEqual("Edge Case Hunter", beatmap.Metadata.AuthorString); + } + } + + [Test] + public void TestDecodeFileWithEmptyLinesAndNoHeader() + { + Decoder decoder = null; + Beatmap beatmap = null; + + using (var resStream = TestResources.OpenResource("empty-line-instead-of-header.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.DoesNotThrow(() => decoder = Decoder.GetDecoder(stream)); + Assert.IsInstanceOf(decoder); + Assert.DoesNotThrow(() => beatmap = decoder.Decode(stream)); + Assert.IsNotNull(beatmap); + Assert.AreEqual("The dog ate the file header", beatmap.Metadata.Title); + Assert.AreEqual("Why does this keep happening", beatmap.Metadata.AuthorString); + } + } + + [Test] + public void TestDecodeFileWithContentImmediatelyAfterHeader() + { + Decoder decoder = null; + Beatmap beatmap = null; + + using (var resStream = TestResources.OpenResource("no-empty-line-after-header.osu")) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.DoesNotThrow(() => decoder = Decoder.GetDecoder(stream)); + Assert.IsInstanceOf(decoder); + Assert.DoesNotThrow(() => beatmap = decoder.Decode(stream)); + Assert.IsNotNull(beatmap); + Assert.AreEqual("No empty line delimiting header from contents", beatmap.Metadata.Title); + Assert.AreEqual("Edge Case Hunter", beatmap.Metadata.AuthorString); + } + } + + [Test] + public void TestDecodeEmptyFile() + { + using (var resStream = new MemoryStream()) + using (var stream = new LineBufferedReader(resStream)) + { + Assert.Throws(() => Decoder.GetDecoder(stream)); + } + } } } diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index ad0ed00989..0686073292 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -126,7 +126,7 @@ namespace osu.Game.Tests.Beatmaps.IO var breakTemp = TestResources.GetTestBeatmapForImport(); - MemoryStream brokenOsu = new MemoryStream(new byte[] { 1, 3, 3, 7 }); + MemoryStream brokenOsu = new MemoryStream(); MemoryStream brokenOsz = new MemoryStream(File.ReadAllBytes(breakTemp)); File.Delete(breakTemp); diff --git a/osu.Game.Tests/Resources/corrupted-header.osu b/osu.Game.Tests/Resources/corrupted-header.osu new file mode 100644 index 0000000000..92701a4a7d --- /dev/null +++ b/osu.Game.Tests/Resources/corrupted-header.osu @@ -0,0 +1,5 @@ +ow computerosu file format v14 + +[Metadata] +Title: Beatmap with corrupted header +Creator: Evil Hacker diff --git a/osu.Game.Tests/Resources/empty-line-instead-of-header.osu b/osu.Game.Tests/Resources/empty-line-instead-of-header.osu new file mode 100644 index 0000000000..91ecf8d84a --- /dev/null +++ b/osu.Game.Tests/Resources/empty-line-instead-of-header.osu @@ -0,0 +1,5 @@ + + +[Metadata] +Title: The dog ate the file header +Creator: Why does this keep happening \ No newline at end of file diff --git a/osu.Game.Tests/Resources/empty-lines-at-start.osu b/osu.Game.Tests/Resources/empty-lines-at-start.osu new file mode 100644 index 0000000000..cb3b1761a2 --- /dev/null +++ b/osu.Game.Tests/Resources/empty-lines-at-start.osu @@ -0,0 +1,8 @@ + + + +osu file format v14 + +[Metadata] +Title: Empty lines at start +Creator: Edge Case Hunter \ No newline at end of file diff --git a/osu.Game.Tests/Resources/missing-header.osu b/osu.Game.Tests/Resources/missing-header.osu new file mode 100644 index 0000000000..95fac0d79b --- /dev/null +++ b/osu.Game.Tests/Resources/missing-header.osu @@ -0,0 +1,4 @@ +[Metadata] + +Title: Beatmap with no header +Creator: Incredibly Evil Hacker diff --git a/osu.Game.Tests/Resources/no-empty-line-after-header.osu b/osu.Game.Tests/Resources/no-empty-line-after-header.osu new file mode 100644 index 0000000000..9db2b7c01c --- /dev/null +++ b/osu.Game.Tests/Resources/no-empty-line-after-header.osu @@ -0,0 +1,4 @@ +osu file format v14 +[Metadata] +Title: No empty line delimiting header from contents +Creator: Edge Case Hunter \ No newline at end of file diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 84f67c9319..9dc3e85e1b 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -1,5 +1,12 @@  + + + + + + + diff --git a/osu.Game/Beatmaps/Formats/Decoder.cs b/osu.Game/Beatmaps/Formats/Decoder.cs index 0f1b617809..4a539cc33f 100644 --- a/osu.Game/Beatmaps/Formats/Decoder.cs +++ b/osu.Game/Beatmaps/Formats/Decoder.cs @@ -28,6 +28,7 @@ namespace osu.Game.Beatmaps.Formats public abstract class Decoder { private static readonly Dictionary>> decoders = new Dictionary>>(); + private static readonly Dictionary> fallback_decoders = new Dictionary>(); static Decoder() { @@ -49,21 +50,31 @@ namespace osu.Game.Beatmaps.Formats if (!decoders.TryGetValue(typeof(T), out var typedDecoders)) throw new IOException(@"Unknown decoder type"); - string line; + // start off with the first line of the file + string line = stream.PeekLine()?.Trim(); - do + while (line != null && line.Length == 0) { - line = stream.ReadLine()?.Trim(); - } while (line != null && line.Length == 0); + // consume the previously peeked empty line and advance to the next one + stream.ReadLine(); + line = stream.PeekLine()?.Trim(); + } if (line == null) - throw new IOException(@"Unknown file format (null)"); + throw new IOException("Unknown file format (null)"); var decoder = typedDecoders.Select(d => line.StartsWith(d.Key, StringComparison.InvariantCulture) ? d.Value : null).FirstOrDefault(); - if (decoder == null) - throw new IOException($@"Unknown file format ({line})"); - return (Decoder)decoder.Invoke(line); + // it's important the magic does NOT get consumed here, since sometimes it's part of the structure + // (see JsonBeatmapDecoder - the magic string is the opening brace) + // decoder implementations should therefore not die on receiving their own magic + if (decoder != null) + return (Decoder)decoder.Invoke(line); + + if (!fallback_decoders.TryGetValue(typeof(T), out var fallbackDecoder)) + throw new IOException($"Unknown file format ({line})"); + + return (Decoder)fallbackDecoder.Invoke(); } /// @@ -78,5 +89,19 @@ namespace osu.Game.Beatmaps.Formats typedDecoders[magic] = constructor; } + + /// + /// Registers a fallback decoder instantiation function. + /// The fallback will be returned if the first line of the decoded stream does not match any known magic. + /// + /// Type of object being decoded. + /// A function that constructs the fallback. + protected static void SetFallbackDecoder(Func constructor) + { + if (fallback_decoders.ContainsKey(typeof(T))) + throw new InvalidOperationException($"A fallback decoder was already added for type {typeof(T)}."); + + fallback_decoders[typeof(T)] = constructor; + } } } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 6e34fd8e90..786b7611b5 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -26,6 +26,7 @@ namespace osu.Game.Beatmaps.Formats public static void Register() { AddDecoder(@"osu file format v", m => new LegacyBeatmapDecoder(Parsing.ParseInt(m.Split('v').Last()))); + SetFallbackDecoder(() => new LegacyBeatmapDecoder()); } /// diff --git a/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs index a8f8dbf69d..5dbd67d304 100644 --- a/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyStoryboardDecoder.cs @@ -34,6 +34,7 @@ namespace osu.Game.Beatmaps.Formats // note that this isn't completely correct AddDecoder(@"osu file format v", m => new LegacyStoryboardDecoder()); AddDecoder(@"[Events]", m => new LegacyStoryboardDecoder()); + SetFallbackDecoder(() => new LegacyStoryboardDecoder()); } protected override void ParseStreamInto(LineBufferedReader stream, Storyboard storyboard) diff --git a/osu.Game/IO/LineBufferedReader.cs b/osu.Game/IO/LineBufferedReader.cs index 66dee1181f..aab761afd8 100644 --- a/osu.Game/IO/LineBufferedReader.cs +++ b/osu.Game/IO/LineBufferedReader.cs @@ -25,6 +25,7 @@ namespace osu.Game.IO /// /// Reads the next line from the stream without consuming it. + /// Subsequent calls to without a will return the same string. /// public string PeekLine() { @@ -39,6 +40,7 @@ namespace osu.Game.IO /// /// Reads the next line from the stream and consumes it. + /// If a line was peeked, that same line will then be consumed and returned. /// public string ReadLine() => lineBuffer.Count > 0 ? lineBuffer.Dequeue() : streamReader.ReadLine(); From 29fcab65f9a2e553dd9c9cd550173612f1b74303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 10 Sep 2019 22:40:52 +0200 Subject: [PATCH 04/20] Remove superfluous csproj entries --- osu.Game.Tests/osu.Game.Tests.csproj | 7 ------- 1 file changed, 7 deletions(-) diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 9dc3e85e1b..84f67c9319 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -1,12 +1,5 @@  - - - - - - - From dd9f620c2364e35c1f1185f2f4b5a43893b5874f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 10 Sep 2019 22:45:34 +0200 Subject: [PATCH 05/20] Fix misleading xmldoc --- osu.Game/Beatmaps/Formats/Decoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Formats/Decoder.cs b/osu.Game/Beatmaps/Formats/Decoder.cs index 4a539cc33f..045eb2d14d 100644 --- a/osu.Game/Beatmaps/Formats/Decoder.cs +++ b/osu.Game/Beatmaps/Formats/Decoder.cs @@ -92,7 +92,7 @@ namespace osu.Game.Beatmaps.Formats /// /// Registers a fallback decoder instantiation function. - /// The fallback will be returned if the first line of the decoded stream does not match any known magic. + /// The fallback will be returned if the first non-empty line of the decoded stream does not match any known magic. /// /// Type of object being decoded. /// A function that constructs the fallback. From 028c958431728deb42811a92a3993f2382ee49a6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 20 Sep 2019 19:19:43 +0900 Subject: [PATCH 06/20] Initial implementation of a switch button --- .../UserInterface/TestSceneSwitchButton.cs | 20 +++ .../Graphics/UserInterface/SwitchButton.cs | 117 ++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs create mode 100644 osu.Game/Graphics/UserInterface/SwitchButton.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs new file mode 100644 index 0000000000..8fe381f141 --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs @@ -0,0 +1,20 @@ +// 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.Graphics; +using osu.Game.Graphics.UserInterface; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public class TestSceneSwitchButton : OsuTestScene + { + public TestSceneSwitchButton() + { + Child = new SwitchButton + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + }; + } + } +} diff --git a/osu.Game/Graphics/UserInterface/SwitchButton.cs b/osu.Game/Graphics/UserInterface/SwitchButton.cs new file mode 100644 index 0000000000..21712051ef --- /dev/null +++ b/osu.Game/Graphics/UserInterface/SwitchButton.cs @@ -0,0 +1,117 @@ +// 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.Allocation; +using osu.Framework.Extensions.Color4Extensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Colour; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.UserInterface; +using osu.Framework.Input.Events; +using osuTK; +using osuTK.Graphics; + +namespace osu.Game.Graphics.UserInterface +{ + public class SwitchButton : Checkbox + { + private const float border_thickness = 4.5f; + private const float padding = 1.25f; + + private readonly Box fill; + private readonly Container switchContainer; + private readonly Drawable switchCircle; + private readonly CircularBorderContainer circularContainer; + + private Color4 enabledColour; + private Color4 disabledColour; + + public SwitchButton() + { + Size = new Vector2(45, 20); + + InternalChild = circularContainer = new CircularBorderContainer + { + RelativeSizeAxes = Axes.Both, + BorderColour = Color4.White, + BorderThickness = border_thickness, + Masking = true, + Children = new Drawable[] + { + fill = new Box + { + RelativeSizeAxes = Axes.Both, + AlwaysPresent = true, + Alpha = 0 + }, + new Container + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding(border_thickness + padding), + Child = switchContainer = new Container + { + RelativeSizeAxes = Axes.Both, + Child = switchCircle = new CircularContainer + { + RelativeSizeAxes = Axes.Both, + FillMode = FillMode.Fit, + Masking = true, + Child = new Box { RelativeSizeAxes = Axes.Both } + } + } + } + } + }; + } + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + enabledColour = colours.BlueDark; + disabledColour = colours.Gray3; + + switchContainer.Colour = enabledColour; + fill.Colour = disabledColour; + + updateBorder(); + } + + protected override void OnUserChange(bool value) + { + base.OnUserChange(value); + + if (value) + switchCircle.MoveToX(switchContainer.DrawWidth - switchCircle.DrawWidth, 200, Easing.OutQuint); + else + switchCircle.MoveToX(0, 200, Easing.OutQuint); + + fill.FadeTo(value ? 1 : 0, 250, Easing.OutQuint); + + updateBorder(); + } + + protected override bool OnHover(HoverEvent e) + { + updateBorder(); + return base.OnHover(e); + } + + protected override void OnHoverLost(HoverLostEvent e) + { + updateBorder(); + base.OnHoverLost(e); + } + + private void updateBorder() + { + circularContainer.TransformBorderTo((Current.Value ? enabledColour : disabledColour).Lighten(IsHovered ? 0.3f : 0)); + } + + private class CircularBorderContainer : CircularContainer + { + public void TransformBorderTo(SRGBColour colour) + => this.TransformTo(nameof(BorderColour), colour, 250, Easing.OutQuint); + } + } +} From 261ba5c80ac4b82d5d73ce02c9609fe8bc829783 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 25 Sep 2019 17:42:27 +0900 Subject: [PATCH 07/20] Fix button not transforming correctly in some cases --- .../UserInterface/TestSceneSwitchButton.cs | 30 +++++++++++++++++-- .../Graphics/UserInterface/SwitchButton.cs | 19 ++++++------ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs index 8fe381f141..bf9071b812 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs @@ -1,20 +1,44 @@ // 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.Framework.Bindables; using osu.Framework.Graphics; using osu.Game.Graphics.UserInterface; +using osuTK.Input; namespace osu.Game.Tests.Visual.UserInterface { - public class TestSceneSwitchButton : OsuTestScene + public class TestSceneSwitchButton : ManualInputManagerTestScene { - public TestSceneSwitchButton() + private SwitchButton switchButton; + + [SetUp] + public void Setup() => Schedule(() => { - Child = new SwitchButton + Child = switchButton = new SwitchButton { Anchor = Anchor.Centre, Origin = Anchor.Centre, }; + }); + + [Test] + public void TestChangeThroughInput() + { + AddStep("move to switch button", () => InputManager.MoveMouseTo(switchButton)); + AddStep("click on", () => InputManager.Click(MouseButton.Left)); + AddStep("click off", () => InputManager.Click(MouseButton.Left)); + } + + [Test] + public void TestChangeThroughBindable() + { + BindableBool bindable = null; + + AddStep("bind bindable", () => switchButton.Current.BindTo(bindable = new BindableBool())); + AddStep("toggle bindable", () => bindable.Toggle()); + AddStep("toggle bindable", () => bindable.Toggle()); } } } diff --git a/osu.Game/Graphics/UserInterface/SwitchButton.cs b/osu.Game/Graphics/UserInterface/SwitchButton.cs index 21712051ef..9964af91d5 100644 --- a/osu.Game/Graphics/UserInterface/SwitchButton.cs +++ b/osu.Game/Graphics/UserInterface/SwitchButton.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; @@ -73,20 +74,20 @@ namespace osu.Game.Graphics.UserInterface switchContainer.Colour = enabledColour; fill.Colour = disabledColour; - - updateBorder(); } - protected override void OnUserChange(bool value) + protected override void LoadComplete() { - base.OnUserChange(value); + base.LoadComplete(); - if (value) - switchCircle.MoveToX(switchContainer.DrawWidth - switchCircle.DrawWidth, 200, Easing.OutQuint); - else - switchCircle.MoveToX(0, 200, Easing.OutQuint); + Current.BindValueChanged(updateState, true); + FinishTransforms(true); + } - fill.FadeTo(value ? 1 : 0, 250, Easing.OutQuint); + private void updateState(ValueChangedEvent state) + { + switchCircle.MoveToX(state.NewValue ? switchContainer.DrawWidth - switchCircle.DrawWidth : 0, 200, Easing.OutQuint); + fill.FadeTo(state.NewValue ? 1 : 0, 250, Easing.OutQuint); updateBorder(); } From c9e39c124e5817bce9909acc8e057606701ea0ce Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 25 Sep 2019 17:42:35 +0900 Subject: [PATCH 08/20] Add a labelled switch button --- .../TestSceneLabelledSwitchButton.cs | 50 +++++++++++++++++++ .../LabelledSwitchButton.cs | 17 +++++++ 2 files changed, 67 insertions(+) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs create mode 100644 osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledSwitchButton.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs new file mode 100644 index 0000000000..dbce08c898 --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs @@ -0,0 +1,50 @@ +// 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.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Graphics.UserInterface; +using osu.Game.Screens.Edit.Setup.Components.LabelledComponents; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public class TestSceneLabelledSwitchButton : OsuTestScene + { + public override IReadOnlyList RequiredTypes => new[] + { + typeof(LabelledSwitchButton), + typeof(SwitchButton) + }; + + [TestCase(false)] + [TestCase(true)] + public void TestSwitchButton(bool hasDescription) => createSwitchButton(hasDescription); + + private void createSwitchButton(bool hasDescription = false) + { + AddStep("create component", () => + { + LabelledSwitchButton component; + + Child = new Container + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Width = 500, + AutoSizeAxes = Axes.Y, + Child = component = new LabelledSwitchButton + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + } + }; + + component.Label = "a sample component"; + component.Description = hasDescription ? "this text describes the component" : string.Empty; + }); + } + } +} diff --git a/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledSwitchButton.cs b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledSwitchButton.cs new file mode 100644 index 0000000000..54f6184578 --- /dev/null +++ b/osu.Game/Screens/Edit/Setup/Components/LabelledComponents/LabelledSwitchButton.cs @@ -0,0 +1,17 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Graphics.UserInterface; + +namespace osu.Game.Screens.Edit.Setup.Components.LabelledComponents +{ + public class LabelledSwitchButton : LabelledComponent + { + public LabelledSwitchButton() + : base(true) + { + } + + protected override SwitchButton CreateComponent() => new SwitchButton(); + } +} From 9f77a1ef35c081db6812bbd40e38185101ae4fcc Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 25 Sep 2019 17:53:08 +0900 Subject: [PATCH 09/20] Adjust namespaces --- .../Visual/UserInterface/TestSceneLabelledSwitchButton.cs | 3 +-- osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs | 2 +- osu.Game/Graphics/UserInterfaceV2/LabelledSwitchButton.cs | 4 +--- .../{UserInterface => UserInterfaceV2}/SwitchButton.cs | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) rename osu.Game/Graphics/{UserInterface => UserInterfaceV2}/SwitchButton.cs (98%) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs index dbce08c898..6ca4d9fa4c 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneLabelledSwitchButton.cs @@ -6,8 +6,7 @@ using System.Collections.Generic; using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Game.Graphics.UserInterface; -using osu.Game.Screens.Edit.Setup.Components.LabelledComponents; +using osu.Game.Graphics.UserInterfaceV2; namespace osu.Game.Tests.Visual.UserInterface { diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs index bf9071b812..4a104b4a41 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSwitchButton.cs @@ -4,7 +4,7 @@ using NUnit.Framework; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Game.Graphics.UserInterface; +using osu.Game.Graphics.UserInterfaceV2; using osuTK.Input; namespace osu.Game.Tests.Visual.UserInterface diff --git a/osu.Game/Graphics/UserInterfaceV2/LabelledSwitchButton.cs b/osu.Game/Graphics/UserInterfaceV2/LabelledSwitchButton.cs index 54f6184578..c973f1d13e 100644 --- a/osu.Game/Graphics/UserInterfaceV2/LabelledSwitchButton.cs +++ b/osu.Game/Graphics/UserInterfaceV2/LabelledSwitchButton.cs @@ -1,9 +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.Game.Graphics.UserInterface; - -namespace osu.Game.Screens.Edit.Setup.Components.LabelledComponents +namespace osu.Game.Graphics.UserInterfaceV2 { public class LabelledSwitchButton : LabelledComponent { diff --git a/osu.Game/Graphics/UserInterface/SwitchButton.cs b/osu.Game/Graphics/UserInterfaceV2/SwitchButton.cs similarity index 98% rename from osu.Game/Graphics/UserInterface/SwitchButton.cs rename to osu.Game/Graphics/UserInterfaceV2/SwitchButton.cs index 9964af91d5..a7fd25b554 100644 --- a/osu.Game/Graphics/UserInterface/SwitchButton.cs +++ b/osu.Game/Graphics/UserInterfaceV2/SwitchButton.cs @@ -13,7 +13,7 @@ using osu.Framework.Input.Events; using osuTK; using osuTK.Graphics; -namespace osu.Game.Graphics.UserInterface +namespace osu.Game.Graphics.UserInterfaceV2 { public class SwitchButton : Checkbox { From a69b9f1148bbee373e6243452291cbab0bab0e16 Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 11:16:31 -0700 Subject: [PATCH 10/20] Fix alt-f4 being blocked during gameplay --- osu.Game/Screens/Play/Player.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 44be73b089..91d34c8056 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -299,7 +299,10 @@ namespace osu.Game.Screens.Play { if (!this.IsCurrentScreen()) return; - this.Exit(); + if (canPause) + Pause(); + else + this.Exit(); } public void Restart() @@ -508,12 +511,6 @@ namespace osu.Game.Screens.Play return true; } - if (canPause) - { - Pause(); - return true; - } - // ValidForResume is false when restarting if (ValidForResume) { From 752dd26a4f5a73823a9acbcbc5b518f9a9b6f4bf Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 11:17:43 -0700 Subject: [PATCH 11/20] Fix alt-f4 being blocked in interface --- osu.Game/Screens/Menu/MainMenu.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index c195ed6cb6..df4803b2b6 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -81,8 +81,8 @@ namespace osu.Game.Screens.Menu { if (holdDelay.Value > 0) confirmAndExit(); - else - this.Exit(); + else if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) + dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); } }); } @@ -253,12 +253,6 @@ namespace osu.Game.Screens.Menu public override bool OnExiting(IScreen next) { - if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) - { - dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); - return true; - } - buttons.State = ButtonSystemState.Exit; this.FadeOut(3000); return base.OnExiting(next); From ff56453f1a259ed80d8ab6338ba4d8c9b93a27d3 Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 12:07:07 -0700 Subject: [PATCH 12/20] Fix test regressions --- osu.Game.Tests/Visual/Gameplay/TestScenePause.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 50583e43c4..c67001c3d8 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -132,9 +132,7 @@ namespace osu.Game.Tests.Visual.Gameplay { AddStep("exit", () => Player.Exit()); - confirmPaused(); - - exitAndConfirm(); + confirmExited(); } [Test] From 38fe519c91f9836f46b6a9347e4ceb97a96f0d67 Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 12:28:48 -0700 Subject: [PATCH 13/20] Remove unnecessary exitConfirmed condition check --- osu.Game/Screens/Menu/MainMenu.cs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index df4803b2b6..6deb29c2f2 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -80,9 +80,9 @@ namespace osu.Game.Screens.Menu Action = () => { if (holdDelay.Value > 0) - confirmAndExit(); - else if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) - dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); + this.Exit(); + else if (dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) + dialogOverlay.Push(new ConfirmExitDialog(this.Exit, () => exitConfirmOverlay.Abort())); } }); } @@ -100,7 +100,7 @@ namespace osu.Game.Screens.Menu OnEdit = delegate { this.Push(new Editor()); }, OnSolo = onSolo, OnMulti = delegate { this.Push(new Multiplayer()); }, - OnExit = confirmAndExit, + OnExit = this.Exit, } } }, @@ -129,12 +129,6 @@ namespace osu.Game.Screens.Menu preloadSongSelect(); } - private void confirmAndExit() - { - exitConfirmed = true; - this.Exit(); - } - private void preloadSongSelect() { if (songSelect == null) @@ -172,8 +166,6 @@ namespace osu.Game.Screens.Menu Beatmap.ValueChanged += beatmap_ValueChanged; } - private bool exitConfirmed; - protected override void LogoArriving(OsuLogo logo, bool resuming) { base.LogoArriving(logo, resuming); From 148089f160240fc408f5a68b72caa2b84dc8731e Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 16:08:19 -0700 Subject: [PATCH 14/20] Revert "Remove unnecessary exitConfirmed condition check" This reverts commit 38fe519c91f9836f46b6a9347e4ceb97a96f0d67. --- osu.Game/Screens/Menu/MainMenu.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index 6deb29c2f2..df4803b2b6 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -80,9 +80,9 @@ namespace osu.Game.Screens.Menu Action = () => { if (holdDelay.Value > 0) - this.Exit(); - else if (dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) - dialogOverlay.Push(new ConfirmExitDialog(this.Exit, () => exitConfirmOverlay.Abort())); + confirmAndExit(); + else if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) + dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); } }); } @@ -100,7 +100,7 @@ namespace osu.Game.Screens.Menu OnEdit = delegate { this.Push(new Editor()); }, OnSolo = onSolo, OnMulti = delegate { this.Push(new Multiplayer()); }, - OnExit = this.Exit, + OnExit = confirmAndExit, } } }, @@ -129,6 +129,12 @@ namespace osu.Game.Screens.Menu preloadSongSelect(); } + private void confirmAndExit() + { + exitConfirmed = true; + this.Exit(); + } + private void preloadSongSelect() { if (songSelect == null) @@ -166,6 +172,8 @@ namespace osu.Game.Screens.Menu Beatmap.ValueChanged += beatmap_ValueChanged; } + private bool exitConfirmed; + protected override void LogoArriving(OsuLogo logo, bool resuming) { base.LogoArriving(logo, resuming); From 80177885213d56d52f2386dc885784c0549a0db6 Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 2 Oct 2019 16:08:24 -0700 Subject: [PATCH 15/20] Revert "Fix alt-f4 being blocked in interface" This reverts commit 752dd26a4f5a73823a9acbcbc5b518f9a9b6f4bf. --- osu.Game/Screens/Menu/MainMenu.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index df4803b2b6..c195ed6cb6 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -81,8 +81,8 @@ namespace osu.Game.Screens.Menu { if (holdDelay.Value > 0) confirmAndExit(); - else if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) - dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); + else + this.Exit(); } }); } @@ -253,6 +253,12 @@ namespace osu.Game.Screens.Menu public override bool OnExiting(IScreen next) { + if (!exitConfirmed && dialogOverlay != null && !(dialogOverlay.CurrentDialog is ConfirmExitDialog)) + { + dialogOverlay.Push(new ConfirmExitDialog(confirmAndExit, () => exitConfirmOverlay.Abort())); + return true; + } + buttons.State = ButtonSystemState.Exit; this.FadeOut(3000); return base.OnExiting(next); From 71985c7ef13913af2d3c6acf07c17ac4d11986c5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Oct 2019 11:23:42 +0800 Subject: [PATCH 16/20] Update fail logic to match --- osu.Game/Screens/Play/Player.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 91d34c8056..5560baa7d1 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -299,6 +299,12 @@ namespace osu.Game.Screens.Play { if (!this.IsCurrentScreen()) return; + if (HasFailed && !FailOverlay.IsPresent) + { + failAnimation.FinishTransforms(true); + return; + } + if (canPause) Pause(); else @@ -517,12 +523,6 @@ namespace osu.Game.Screens.Play if (pauseCooldownActive && !GameplayClockContainer.IsPaused.Value) // still want to block if we are within the cooldown period and not already paused. return true; - - if (HasFailed && !FailOverlay.IsPresent) - { - failAnimation.FinishTransforms(true); - return true; - } } GameplayClockContainer.ResetLocalAdjustments(); From e646b2677ca0b07267690783cf8894676e437dc4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Oct 2019 11:25:23 +0800 Subject: [PATCH 17/20] Add test coverage --- osu.Game.Tests/Visual/Gameplay/TestScenePause.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index c67001c3d8..48159aa5e0 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -127,6 +127,15 @@ namespace osu.Game.Tests.Visual.Gameplay exitAndConfirm(); } + [Test] + public void TestExitFromFailedGameplay() + { + AddUntilStep("wait for fail", () => Player.HasFailed); + AddStep("exit", () => Player.Exit()); + + confirmExited(); + } + [Test] public void TestExitFromGameplay() { From 47c1f36f9d91f54eb68b824badad248b69c169be Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Oct 2019 11:41:53 +0800 Subject: [PATCH 18/20] Add ValidForResume check --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 5560baa7d1..0b363eac4d 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -299,7 +299,7 @@ namespace osu.Game.Screens.Play { if (!this.IsCurrentScreen()) return; - if (HasFailed && !FailOverlay.IsPresent) + if (ValidForResume && HasFailed && !FailOverlay.IsPresent) { failAnimation.FinishTransforms(true); return; From 626f7388c8fdc8dd34935d2a8cf86cfaf702ac4a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Oct 2019 12:23:01 +0800 Subject: [PATCH 19/20] Add tests for quick retry and quick exit scenarios --- .../Visual/Gameplay/TestScenePause.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 48159aa5e0..2df22df659 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -136,6 +136,24 @@ namespace osu.Game.Tests.Visual.Gameplay confirmExited(); } + [Test] + public void TestQuickRetryFromFailedGameplay() + { + AddUntilStep("wait for fail", () => Player.HasFailed); + AddStep("quick retry", () => Player.GameplayClockContainer.OfType().First().Action?.Invoke()); + + confirmExited(); + } + + [Test] + public void TestQuickExitFromFailedGameplay() + { + AddUntilStep("wait for fail", () => Player.HasFailed); + AddStep("quick exit", () => Player.GameplayClockContainer.OfType().First().Action?.Invoke()); + + confirmExited(); + } + [Test] public void TestExitFromGameplay() { @@ -144,6 +162,14 @@ namespace osu.Game.Tests.Visual.Gameplay confirmExited(); } + [Test] + public void TestQuickExitFromGameplay() + { + AddStep("quick exit", () => Player.GameplayClockContainer.OfType().First().Action?.Invoke()); + + confirmExited(); + } + [Test] public void TestExitViaHoldToExit() { From ffde389641a15f0b0faceb8e1e900fa186509bda Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Fri, 4 Oct 2019 13:28:32 +0900 Subject: [PATCH 20/20] Add difficulty calculator beatmap decoder fallback --- .../Beatmaps/Formats/LegacyDifficultyCalculatorBeatmapDecoder.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Beatmaps/Formats/LegacyDifficultyCalculatorBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDifficultyCalculatorBeatmapDecoder.cs index 2c493254e0..238187bf8f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDifficultyCalculatorBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDifficultyCalculatorBeatmapDecoder.cs @@ -24,6 +24,7 @@ namespace osu.Game.Beatmaps.Formats public new static void Register() { AddDecoder(@"osu file format v", m => new LegacyDifficultyCalculatorBeatmapDecoder(int.Parse(m.Split('v').Last()))); + SetFallbackDecoder(() => new LegacyDifficultyCalculatorBeatmapDecoder()); } protected override TimingControlPoint CreateTimingControlPoint()