From cee4b005e6a5ceb3e4af6380f73321205b9d9403 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 13 Apr 2020 20:00:06 +0900 Subject: [PATCH 01/21] Fix custom sample set 0 not falling back to default samples --- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 2 +- .../Objects/Legacy/ConvertHitObjectParser.cs | 2 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 561707f9ef..90a5d0dcba 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -168,7 +168,7 @@ namespace osu.Game.Beatmaps.Formats { var baseInfo = base.ApplyTo(hitSampleInfo); - if (string.IsNullOrEmpty(baseInfo.Suffix) && CustomSampleBank > 1) + if (string.IsNullOrEmpty(baseInfo.Suffix) && CustomSampleBank > 0) baseInfo.Suffix = CustomSampleBank.ToString(); return baseInfo; diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 8d3ad5984f..8580cdede3 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -415,7 +415,7 @@ namespace osu.Game.Rulesets.Objects.Legacy { set { - if (value > 1) + if (value > 0) Suffix = value.ToString(); } } diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 1190a330fe..c4636f46f5 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -2,8 +2,10 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Audio; +using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.IO.Stores; +using osu.Game.Audio; using osu.Game.Beatmaps; namespace osu.Game.Skinning @@ -33,6 +35,17 @@ namespace osu.Game.Skinning return base.GetConfig(lookup); } + public override SampleChannel GetSample(ISampleInfo sampleInfo) + { + if (sampleInfo is HitSampleInfo hsi && string.IsNullOrEmpty(hsi.Suffix)) + { + // When no custom sample set is provided, always fall-back to the default samples. + return null; + } + + return base.GetSample(sampleInfo); + } + private static SkinInfo createSkinInfo(BeatmapInfo beatmap) => new SkinInfo { Name = beatmap.ToString(), Creator = beatmap.Metadata.Author.ToString() }; } From 58a7313091812597a622ff0f616fcd23a271febd Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 13 Apr 2020 20:09:17 +0900 Subject: [PATCH 02/21] Fix fallback for file hit samples --- osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 8580cdede3..1dca4a5c02 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -425,6 +425,12 @@ namespace osu.Game.Rulesets.Objects.Legacy { public string Filename; + public FileHitSampleInfo() + { + // Has no effect since LookupNames is overridden, however prompts LegacyBeatmapSkin to not fallback. + Suffix = "0"; + } + public override IEnumerable LookupNames => new[] { Filename, From 7f95418262d84096ea2afb38896d674170f9942e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 14 Apr 2020 16:52:17 +0900 Subject: [PATCH 03/21] Fix osu!mania replays actuating incorrect keys when multiple stages are involved --- .../ManiaLegacyReplayTest.cs | 51 ++++++++++++ .../Replays/ManiaReplayFrame.cs | 83 ++++++++++++++++--- 2 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 osu.Game.Rulesets.Mania.Tests/ManiaLegacyReplayTest.cs diff --git a/osu.Game.Rulesets.Mania.Tests/ManiaLegacyReplayTest.cs b/osu.Game.Rulesets.Mania.Tests/ManiaLegacyReplayTest.cs new file mode 100644 index 0000000000..40bb83aece --- /dev/null +++ b/osu.Game.Rulesets.Mania.Tests/ManiaLegacyReplayTest.cs @@ -0,0 +1,51 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Game.Rulesets.Mania.Beatmaps; +using osu.Game.Rulesets.Mania.Replays; + +namespace osu.Game.Rulesets.Mania.Tests +{ + [TestFixture] + public class ManiaLegacyReplayTest + { + [TestCase(ManiaAction.Key1)] + [TestCase(ManiaAction.Key1, ManiaAction.Key2)] + [TestCase(ManiaAction.Special1)] + [TestCase(ManiaAction.Key8)] + public void TestEncodeDecodeSingleStage(params ManiaAction[] actions) + { + var beatmap = new ManiaBeatmap(new StageDefinition { Columns = 9 }); + + var frame = new ManiaReplayFrame(0, actions); + var legacyFrame = frame.ToLegacy(beatmap); + + var decodedFrame = new ManiaReplayFrame(); + decodedFrame.FromLegacy(legacyFrame, beatmap); + + Assert.That(decodedFrame.Actions, Is.EquivalentTo(frame.Actions)); + } + + [TestCase(ManiaAction.Key1)] + [TestCase(ManiaAction.Key1, ManiaAction.Key2)] + [TestCase(ManiaAction.Special1)] + [TestCase(ManiaAction.Special2)] + [TestCase(ManiaAction.Special1, ManiaAction.Special2)] + [TestCase(ManiaAction.Special1, ManiaAction.Key5)] + [TestCase(ManiaAction.Key8)] + public void TestEncodeDecodeDualStage(params ManiaAction[] actions) + { + var beatmap = new ManiaBeatmap(new StageDefinition { Columns = 5 }); + beatmap.Stages.Add(new StageDefinition { Columns = 5 }); + + var frame = new ManiaReplayFrame(0, actions); + var legacyFrame = frame.ToLegacy(beatmap); + + var decodedFrame = new ManiaReplayFrame(); + decodedFrame.FromLegacy(legacyFrame, beatmap); + + Assert.That(decodedFrame.Actions, Is.EquivalentTo(frame.Actions)); + } + } +} diff --git a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs index 8c73c36e99..0059a78a44 100644 --- a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs +++ b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs @@ -1,8 +1,8 @@ // 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.Linq; using osu.Game.Beatmaps; using osu.Game.Replays.Legacy; using osu.Game.Rulesets.Mania.Beatmaps; @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Mania.Replays while (activeColumns > 0) { - var isSpecial = maniaBeatmap.Stages.First().IsSpecialColumn(counter); + bool isSpecial = isColumnAtIndexSpecial(maniaBeatmap, counter); if ((activeColumns & 1) > 0) Actions.Add(isSpecial ? specialAction : normalAction); @@ -58,33 +58,90 @@ namespace osu.Game.Rulesets.Mania.Replays int keys = 0; - var specialColumns = new List(); - - for (int i = 0; i < maniaBeatmap.TotalColumns; i++) - { - if (maniaBeatmap.Stages.First().IsSpecialColumn(i)) - specialColumns.Add(i); - } - foreach (var action in Actions) { switch (action) { case ManiaAction.Special1: - keys |= 1 << specialColumns[0]; + keys |= 1 << getSpecialColumnIndex(maniaBeatmap, 0); break; case ManiaAction.Special2: - keys |= 1 << specialColumns[1]; + keys |= 1 << getSpecialColumnIndex(maniaBeatmap, 1); break; default: - keys |= 1 << (action - ManiaAction.Key1); + // the index in lazer, which doesn't include special keys. + int nonSpecialKeyIndex = action - ManiaAction.Key1; + + int overallIndex = 0; + + // iterate to find the index including special keys. + while (true) + { + if (!isColumnAtIndexSpecial(maniaBeatmap, overallIndex)) + { + // found a non-special column we could use. + if (nonSpecialKeyIndex == 0) + break; + + // found a non-special column but not ours. + nonSpecialKeyIndex--; + } + + overallIndex++; + } + + keys |= 1 << overallIndex; break; } } return new LegacyReplayFrame(Time, keys, null, ReplayButtonState.None); } + + /// + /// Find the overall index (across all stages) for a specified special key. + /// + /// The beatmap. + /// The special key offset (0 is S1). + /// The overall index for the special column. + private int getSpecialColumnIndex(ManiaBeatmap maniaBeatmap, int specialOffset) + { + for (int i = 0; i < maniaBeatmap.TotalColumns; i++) + { + if (isColumnAtIndexSpecial(maniaBeatmap, i)) + { + if (specialOffset == 0) + return i; + + specialOffset--; + } + } + + throw new InvalidOperationException("Special key index too high"); + } + + /// + /// Check whether the column at an overall index (across all stages) is a special column. + /// + /// The beatmap. + /// The overall index to check. + /// + private bool isColumnAtIndexSpecial(ManiaBeatmap beatmap, int index) + { + foreach (var stage in beatmap.Stages) + { + for (int stageIndex = 0; stageIndex < stage.Columns; stageIndex++) + { + if (index == 0) + return stage.IsSpecialColumn(stageIndex); + + index--; + } + } + + return false; + } } } From 69352214637b9b5ef6f7e9d56da30da98455584a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 21:05:07 +0900 Subject: [PATCH 04/21] Improve logic for CSB transfer --- osu.Game/Beatmaps/Formats/LegacyDecoder.cs | 8 ++++-- .../Objects/Legacy/ConvertHitObjectParser.cs | 28 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs index 90a5d0dcba..5b2b213322 100644 --- a/osu.Game/Beatmaps/Formats/LegacyDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyDecoder.cs @@ -8,6 +8,7 @@ using osu.Framework.Logging; using osu.Game.Audio; using osu.Game.Beatmaps.ControlPoints; using osu.Game.IO; +using osu.Game.Rulesets.Objects.Legacy; using osuTK.Graphics; namespace osu.Game.Beatmaps.Formats @@ -168,8 +169,11 @@ namespace osu.Game.Beatmaps.Formats { var baseInfo = base.ApplyTo(hitSampleInfo); - if (string.IsNullOrEmpty(baseInfo.Suffix) && CustomSampleBank > 0) - baseInfo.Suffix = CustomSampleBank.ToString(); + if (baseInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacy + && legacy.CustomSampleBank == 0) + { + legacy.CustomSampleBank = CustomSampleBank; + } return baseInfo; } diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 1dca4a5c02..95cbf3ab40 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -409,26 +409,46 @@ namespace osu.Game.Rulesets.Objects.Legacy public SampleBankInfo Clone() => (SampleBankInfo)MemberwiseClone(); } - private class LegacyHitSampleInfo : HitSampleInfo + internal class LegacyHitSampleInfo : HitSampleInfo { + private int customSampleBank; + public int CustomSampleBank { + get => customSampleBank; set { + customSampleBank = value; + + // A 0 custom sample bank should cause LegacyBeatmapSkin to always fall back to the user skin. This is done by giving a null suffix. if (value > 0) Suffix = value.ToString(); } } + + public override IEnumerable LookupNames + { + get + { + // The lookup should only contain the suffix for custom sample bank 2 and beyond. + // For custom sample bank 1 and 0, the lookup should not contain the suffix as only the lookup source (beatmap or user skin) is changed. + if (CustomSampleBank >= 2) + yield return $"{Bank}-{Name}{Suffix}"; + + yield return $"{Bank}-{Name}"; + } + } } - private class FileHitSampleInfo : HitSampleInfo + private class FileHitSampleInfo : LegacyHitSampleInfo { public string Filename; public FileHitSampleInfo() { - // Has no effect since LookupNames is overridden, however prompts LegacyBeatmapSkin to not fallback. - Suffix = "0"; + // Make sure that the LegacyBeatmapSkin does not fall back to the user skin. + // Note that this does not change the lookup names, as they are overridden locally. + CustomSampleBank = 1; } public override IEnumerable LookupNames => new[] From b29957798f38c28f3940649ecc13be0ca1ee5b8e Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 21:05:42 +0900 Subject: [PATCH 05/21] Fix no audiomanager in test scene working beatmap --- osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs | 6 ++++-- osu.Game/Tests/Visual/OsuTestScene.cs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs index 6db34af20c..8f8afb87d4 100644 --- a/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestWorkingBeatmap.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Audio; using osu.Framework.Audio.Track; using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; @@ -18,8 +19,9 @@ namespace osu.Game.Tests.Beatmaps /// /// The beatmap. /// An optional storyboard. - public TestWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) - : base(beatmap.BeatmapInfo, null) + /// The . + public TestWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null, AudioManager audioManager = null) + : base(beatmap.BeatmapInfo, audioManager) { this.beatmap = beatmap; this.storyboard = storyboard; diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index d1d8059cb1..5dc8714c07 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -179,7 +179,7 @@ namespace osu.Game.Tests.Visual /// Audio manager. Required if a reference clock isn't provided. /// The length of the returned virtual track. public ClockBackedTestWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock referenceClock, AudioManager audio, double length = 60000) - : base(beatmap, storyboard) + : base(beatmap, storyboard, audio) { if (referenceClock != null) { From 00d564d29cafc4b4319e7165de2d5a5210c9d083 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 21:05:54 +0900 Subject: [PATCH 06/21] Add tests --- .../Gameplay/TestSceneHitObjectSamples.cs | 344 ++++++++++++++++++ .../controlpoint-beatmap-custom-sample.osu | 7 + .../controlpoint-beatmap-sample.osu | 7 + .../controlpoint-skin-sample.osu | 7 + .../SampleLookups/file-beatmap-sample.osu | 4 + ...tobject-beatmap-custom-sample-override.osu | 7 + .../hitobject-beatmap-custom-sample.osu | 4 + .../hitobject-beatmap-sample.osu | 4 + .../SampleLookups/hitobject-skin-sample.osu | 4 + 9 files changed, 388 insertions(+) create mode 100644 osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs create mode 100644 osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-custom-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/controlpoint-skin-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/file-beatmap-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample-override.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-sample.osu create mode 100644 osu.Game.Tests/Resources/SampleLookups/hitobject-skin-sample.osu diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs new file mode 100644 index 0000000000..f80ea3ae88 --- /dev/null +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -0,0 +1,344 @@ +// 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.Linq; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.IO.Stores; +using osu.Framework.Timing; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Formats; +using osu.Game.IO; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; +using osu.Game.Skinning; +using osu.Game.Storyboards; +using osu.Game.Tests.Resources; +using osu.Game.Tests.Visual; +using osu.Game.Users; + +namespace osu.Game.Tests.Gameplay +{ + public class TestSceneHitObjectSamples : PlayerTestScene + { + private readonly SkinInfo userSkinInfo = new SkinInfo(); + + private readonly BeatmapInfo beatmapInfo = new BeatmapInfo + { + BeatmapSet = new BeatmapSetInfo(), + Metadata = new BeatmapMetadata + { + Author = User.SYSTEM_USER + } + }; + + private readonly TestResourceStore userSkinResourceStore = new TestResourceStore(); + private readonly TestResourceStore beatmapSkinResourceStore = new TestResourceStore(); + + protected override bool HasCustomSteps => true; + + public TestSceneHitObjectSamples() + : base(new OsuRuleset()) + { + } + + private SkinSourceDependencyContainer dependencies; + + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) + => new DependencyContainer(dependencies = new SkinSourceDependencyContainer(base.CreateChildDependencies(parent))); + + /// + /// Tests that a hitobject which provides no custom sample set retrieves samples from the user skin. + /// + [Test] + public void TestDefaultSampleFromUserSkin() + { + const string expected_sample = "normal-hitnormal"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("hitobject-skin-sample.osu"); + + assertUserLookup(expected_sample); + } + + /// + /// Tests that a hitobject which provides a sample set of 1 retrieves samples from the beatmap skin. + /// + [Test] + public void TestDefaultSampleFromBeatmap() + { + const string expected_sample = "normal-hitnormal"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("hitobject-beatmap-sample.osu"); + + assertBeatmapLookup(expected_sample); + } + + /// + /// Tests that a hitobject which provides a sample set of 1 retrieves samples from the user skin when the beatmap does not contain the sample. + /// + [Test] + public void TestDefaultSampleFromUserSkinFallback() + { + const string expected_sample = "normal-hitnormal"; + + setupSkins(null, expected_sample); + + createTestWithBeatmap("hitobject-beatmap-sample.osu"); + + assertUserLookup(expected_sample); + } + + /// + /// Tests that a hitobject which provides a custom sample set of 2 retrieves the following samples from the beatmap skin: + /// normal-hitnormal2 + /// normal-hitnormal + /// + [TestCase("normal-hitnormal2")] + [TestCase("normal-hitnormal")] + public void TestDefaultCustomSampleFromBeatmap(string expectedSample) + { + setupSkins(expectedSample, expectedSample); + + createTestWithBeatmap("hitobject-beatmap-custom-sample.osu"); + + assertBeatmapLookup(expectedSample); + } + + /// + /// Tests that a hitobject which provides a custom sample set of 2 retrieves the following samples from the user skin when the beatmap does not contain the sample: + /// normal-hitnormal2 + /// normal-hitnormal + /// + [TestCase("normal-hitnormal2")] + [TestCase("normal-hitnormal")] + public void TestDefaultCustomSampleFromUserSkinFallback(string expectedSample) + { + setupSkins(string.Empty, expectedSample); + + createTestWithBeatmap("hitobject-beatmap-custom-sample.osu"); + + assertUserLookup(expectedSample); + } + + /// + /// Tests that a hitobject which provides a sample file retrieves the sample file from the beatmap skin. + /// + [Test] + public void TestFileSampleFromBeatmap() + { + const string expected_sample = "hit_1.wav"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("file-beatmap-sample.osu"); + + assertBeatmapLookup(expected_sample); + } + + /// + /// Tests that a default hitobject and control point causes . + /// + [Test] + public void TestControlPointSampleFromSkin() + { + const string expected_sample = "normal-hitnormal"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("controlpoint-skin-sample.osu"); + + assertUserLookup(expected_sample); + } + + /// + /// Tests that a control point that provides a custom sample set of 1 causes . + /// + [Test] + public void TestControlPointSampleFromBeatmap() + { + const string expected_sample = "normal-hitnormal"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("controlpoint-beatmap-sample.osu"); + + assertBeatmapLookup(expected_sample); + } + + /// + /// Tests that a control point that provides a custom sample of 2 causes . + /// + [TestCase("normal-hitnormal2")] + [TestCase("normal-hitnormal")] + public void TestControlPointCustomSampleFromBeatmap(string sampleName) + { + setupSkins(sampleName, sampleName); + + createTestWithBeatmap("controlpoint-beatmap-custom-sample.osu"); + + assertBeatmapLookup(sampleName); + } + + /// + /// Tests that a hitobject's custom sample overrides the control point's. + /// + [Test] + public void TestHitObjectCustomSampleOverride() + { + const string expected_sample = "normal-hitnormal3"; + + setupSkins(expected_sample, expected_sample); + + createTestWithBeatmap("hitobject-beatmap-custom-sample-override.osu"); + + assertBeatmapLookup(expected_sample); + } + + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => currentTestBeatmap; + + protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard = null) + => new TestWorkingBeatmap(beatmapInfo, beatmapSkinResourceStore, beatmap, storyboard, Clock, Audio); + + private IBeatmap currentTestBeatmap; + + private void createTestWithBeatmap(string filename) + { + CreateTest(() => + { + AddStep("clear performed lookups", () => + { + userSkinResourceStore.PerformedLookups.Clear(); + beatmapSkinResourceStore.PerformedLookups.Clear(); + }); + + AddStep($"load {filename}", () => + { + using (var reader = new LineBufferedReader(TestResources.OpenResource($"SampleLookups/{filename}"))) + currentTestBeatmap = Decoder.GetDecoder(reader).Decode(reader); + }); + }); + } + + private void setupSkins(string beatmapFile, string userFile) + { + AddStep("setup skins", () => + { + userSkinInfo.Files = new List + { + new SkinFileInfo + { + Filename = userFile, + FileInfo = new IO.FileInfo { Hash = userFile } + } + }; + + beatmapInfo.BeatmapSet.Files = new List + { + new BeatmapSetFileInfo + { + Filename = beatmapFile, + FileInfo = new IO.FileInfo { Hash = beatmapFile } + } + }; + + // Need to refresh the cached skin source to refresh the skin resource store. + dependencies.SkinSource = new SkinProvidingContainer(new LegacySkin(userSkinInfo, userSkinResourceStore, dependencies.Get())); + }); + } + + private void assertBeatmapLookup(string name) => AddAssert($"\"{name}\" looked up from beatmap skin", + () => !userSkinResourceStore.PerformedLookups.Contains(name) && beatmapSkinResourceStore.PerformedLookups.Contains(name)); + + private void assertUserLookup(string name) => AddAssert($"\"{name}\" looked up from user skin", + () => !beatmapSkinResourceStore.PerformedLookups.Contains(name) && userSkinResourceStore.PerformedLookups.Contains(name)); + + private class SkinSourceDependencyContainer : IReadOnlyDependencyContainer + { + public ISkinSource SkinSource; + + private readonly IReadOnlyDependencyContainer fallback; + + public SkinSourceDependencyContainer(IReadOnlyDependencyContainer fallback) + { + this.fallback = fallback; + } + + public object Get(Type type) + { + if (type == typeof(ISkinSource)) + return SkinSource; + + return fallback.Get(type); + } + + public object Get(Type type, CacheInfo info) + { + if (type == typeof(ISkinSource)) + return SkinSource; + + return fallback.Get(type); + } + + public void Inject(T instance) where T : class + { + // Never used directly + } + } + + private class TestResourceStore : IResourceStore + { + public readonly List PerformedLookups = new List(); + + public byte[] Get(string name) + { + markLookup(name); + return Array.Empty(); + } + + public Task GetAsync(string name) + { + markLookup(name); + return Task.FromResult(Array.Empty()); + } + + public Stream GetStream(string name) + { + markLookup(name); + return new MemoryStream(); + } + + private void markLookup(string name) => PerformedLookups.Add(name.Substring(name.LastIndexOf('/') + 1)); + + public IEnumerable GetAvailableResources() => Enumerable.Empty(); + + public void Dispose() + { + } + } + + private class TestWorkingBeatmap : ClockBackedTestWorkingBeatmap + { + private readonly BeatmapInfo skinBeatmapInfo; + private readonly IResourceStore resourceStore; + + public TestWorkingBeatmap(BeatmapInfo skinBeatmapInfo, IResourceStore resourceStore, IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock referenceClock, AudioManager audio, + double length = 60000) + : base(beatmap, storyboard, referenceClock, audio, length) + { + this.skinBeatmapInfo = skinBeatmapInfo; + this.resourceStore = resourceStore; + } + + protected override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resourceStore, AudioManager); + } + } +} diff --git a/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-custom-sample.osu b/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-custom-sample.osu new file mode 100644 index 0000000000..91dbc6a60e --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-custom-sample.osu @@ -0,0 +1,7 @@ +osu file format v14 + +[TimingPoints] +0,300,4,0,2,100,1,0 + +[HitObjects] +444,320,1000,5,0,0:0:0:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-sample.osu b/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-sample.osu new file mode 100644 index 0000000000..3274820100 --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/controlpoint-beatmap-sample.osu @@ -0,0 +1,7 @@ +osu file format v14 + +[TimingPoints] +0,300,4,0,1,100,1,0 + +[HitObjects] +444,320,1000,5,0,0:0:0:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/controlpoint-skin-sample.osu b/osu.Game.Tests/Resources/SampleLookups/controlpoint-skin-sample.osu new file mode 100644 index 0000000000..c53ec465fb --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/controlpoint-skin-sample.osu @@ -0,0 +1,7 @@ +osu file format v14 + +[TimingPoints] +0,300,4,0,0,100,1,0 + +[HitObjects] +444,320,1000,5,0,0:0:0:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/file-beatmap-sample.osu b/osu.Game.Tests/Resources/SampleLookups/file-beatmap-sample.osu new file mode 100644 index 0000000000..65b5ea8707 --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/file-beatmap-sample.osu @@ -0,0 +1,4 @@ +osu file format v14 + +[HitObjects] +255,193,2170,1,0,0:0:0:0:hit_1.wav diff --git a/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample-override.osu b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample-override.osu new file mode 100644 index 0000000000..13dc2faab1 --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample-override.osu @@ -0,0 +1,7 @@ +osu file format v14 + +[TimingPoints] +0,300,4,0,2,100,1,0 + +[HitObjects] +444,320,1000,5,0,0:0:3:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample.osu b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample.osu new file mode 100644 index 0000000000..4ab672dbb0 --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample.osu @@ -0,0 +1,4 @@ +osu file format v14 + +[HitObjects] +444,320,1000,5,0,0:0:2:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-sample.osu b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-sample.osu new file mode 100644 index 0000000000..33bc34949a --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-sample.osu @@ -0,0 +1,4 @@ +osu file format v14 + +[HitObjects] +444,320,1000,5,0,0:0:1:0: \ No newline at end of file diff --git a/osu.Game.Tests/Resources/SampleLookups/hitobject-skin-sample.osu b/osu.Game.Tests/Resources/SampleLookups/hitobject-skin-sample.osu new file mode 100644 index 0000000000..47f5b44c90 --- /dev/null +++ b/osu.Game.Tests/Resources/SampleLookups/hitobject-skin-sample.osu @@ -0,0 +1,4 @@ +osu file format v14 + +[HitObjects] +444,320,1000,5,0,0:0:0:0: \ No newline at end of file From 44981431c5470c457d1708ad9227e4ec1dd60566 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 21:33:32 +0900 Subject: [PATCH 07/21] Remove suffix hackery --- .../Objects/Legacy/ConvertHitObjectParser.cs | 16 +--------------- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 +++-- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 95cbf3ab40..9a60a0a75c 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -420,24 +420,10 @@ namespace osu.Game.Rulesets.Objects.Legacy { customSampleBank = value; - // A 0 custom sample bank should cause LegacyBeatmapSkin to always fall back to the user skin. This is done by giving a null suffix. - if (value > 0) + if (value >= 2) Suffix = value.ToString(); } } - - public override IEnumerable LookupNames - { - get - { - // The lookup should only contain the suffix for custom sample bank 2 and beyond. - // For custom sample bank 1 and 0, the lookup should not contain the suffix as only the lookup source (beatmap or user skin) is changed. - if (CustomSampleBank >= 2) - yield return $"{Bank}-{Name}{Suffix}"; - - yield return $"{Bank}-{Name}"; - } - } } private class FileHitSampleInfo : LegacyHitSampleInfo diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index c4636f46f5..21533e58cd 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -7,6 +7,7 @@ using osu.Framework.Bindables; using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps; +using osu.Game.Rulesets.Objects.Legacy; namespace osu.Game.Skinning { @@ -37,9 +38,9 @@ namespace osu.Game.Skinning public override SampleChannel GetSample(ISampleInfo sampleInfo) { - if (sampleInfo is HitSampleInfo hsi && string.IsNullOrEmpty(hsi.Suffix)) + if (sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacy && legacy.CustomSampleBank == 0) { - // When no custom sample set is provided, always fall-back to the default samples. + // When no custom sample bank is provided, always fall-back to the default samples. return null; } From 64d44dedcd643cef2e971a968154e0fd17e2a6b2 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 22:39:51 +0900 Subject: [PATCH 08/21] Make testscene headless --- osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index f80ea3ae88..a8bd902117 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -10,6 +10,7 @@ using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.IO.Stores; +using osu.Framework.Testing; using osu.Framework.Timing; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; @@ -24,6 +25,7 @@ using osu.Game.Users; namespace osu.Game.Tests.Gameplay { + [HeadlessTest] public class TestSceneHitObjectSamples : PlayerTestScene { private readonly SkinInfo userSkinInfo = new SkinInfo(); From 10486a0ad2b2aa1809d1d4f18d532cf545ebcb38 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 14 Apr 2020 23:10:14 +0900 Subject: [PATCH 09/21] Fix potential dependency-related issues --- osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index a8bd902117..366437a771 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -253,7 +253,7 @@ namespace osu.Game.Tests.Gameplay }; // Need to refresh the cached skin source to refresh the skin resource store. - dependencies.SkinSource = new SkinProvidingContainer(new LegacySkin(userSkinInfo, userSkinResourceStore, dependencies.Get())); + dependencies.SkinSource = new SkinProvidingContainer(new LegacySkin(userSkinInfo, userSkinResourceStore, Audio)); }); } @@ -287,7 +287,7 @@ namespace osu.Game.Tests.Gameplay if (type == typeof(ISkinSource)) return SkinSource; - return fallback.Get(type); + return fallback.Get(type, info); } public void Inject(T instance) where T : class From d47e414fb142e7aa504814494d496a3d08528a46 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 15 Apr 2020 12:35:43 +0900 Subject: [PATCH 10/21] Apply review feedback (unroll inner loop / xml fixes) --- .../Replays/ManiaReplayFrame.cs | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs index 0059a78a44..da4b0c943c 100644 --- a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs +++ b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs @@ -74,22 +74,20 @@ namespace osu.Game.Rulesets.Mania.Replays // the index in lazer, which doesn't include special keys. int nonSpecialKeyIndex = action - ManiaAction.Key1; + // the index inclusive of special keys. int overallIndex = 0; // iterate to find the index including special keys. - while (true) + for (; overallIndex < maniaBeatmap.TotalColumns; overallIndex++) { - if (!isColumnAtIndexSpecial(maniaBeatmap, overallIndex)) - { - // found a non-special column we could use. - if (nonSpecialKeyIndex == 0) - break; - - // found a non-special column but not ours. - nonSpecialKeyIndex--; - } - - overallIndex++; + // skip over special columns. + if (isColumnAtIndexSpecial(maniaBeatmap, overallIndex)) + continue; + // found a non-special column to use. + if (nonSpecialKeyIndex == 0) + break; + // found a non-special column but not ours. + nonSpecialKeyIndex--; } keys |= 1 << overallIndex; @@ -127,21 +125,20 @@ namespace osu.Game.Rulesets.Mania.Replays /// /// The beatmap. /// The overall index to check. - /// private bool isColumnAtIndexSpecial(ManiaBeatmap beatmap, int index) { foreach (var stage in beatmap.Stages) { - for (int stageIndex = 0; stageIndex < stage.Columns; stageIndex++) + if (index >= stage.Columns) { - if (index == 0) - return stage.IsSpecialColumn(stageIndex); - - index--; + index -= stage.Columns; + continue; } + + return stage.IsSpecialColumn(index); } - return false; + throw new ArgumentException("Column index is too high.", nameof(index)); } } } From e534d59c807ecd1350e2ced30c4595e49fc6af4a Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 15 Apr 2020 13:08:15 +0900 Subject: [PATCH 11/21] Use another argument exception --- osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs index da4b0c943c..dbab54d1d0 100644 --- a/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs +++ b/osu.Game.Rulesets.Mania/Replays/ManiaReplayFrame.cs @@ -117,7 +117,7 @@ namespace osu.Game.Rulesets.Mania.Replays } } - throw new InvalidOperationException("Special key index too high"); + throw new ArgumentException("Special key index is too high.", nameof(specialOffset)); } /// From 72707a9973f41f1961bd030c66d84a7627e4be81 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 15 Apr 2020 13:54:23 +0900 Subject: [PATCH 12/21] Fix OS-dependent substring --- osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index 366437a771..f611f2717e 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs @@ -318,7 +318,7 @@ namespace osu.Game.Tests.Gameplay return new MemoryStream(); } - private void markLookup(string name) => PerformedLookups.Add(name.Substring(name.LastIndexOf('/') + 1)); + private void markLookup(string name) => PerformedLookups.Add(name.Substring(name.LastIndexOf(Path.DirectorySeparatorChar) + 1)); public IEnumerable GetAvailableResources() => Enumerable.Empty(); From 102c1d9095d4189731f6d7ac547abd0c0e5b7527 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 15 Apr 2020 15:50:19 +0900 Subject: [PATCH 13/21] Add disabled state to menu items --- .../Visual/UserInterface/TestSceneOsuMenu.cs | 91 +++++++++++++++++++ .../UserInterface/DrawableOsuMenuItem.cs | 26 +++++- .../Graphics/UserInterface/OsuMenuItem.cs | 6 ++ 3 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs new file mode 100644 index 0000000000..cdda1969ca --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs @@ -0,0 +1,91 @@ +// 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.Linq; +using NUnit.Framework; +using osu.Framework.Graphics; +using osu.Framework.Testing; +using osu.Game.Graphics.UserInterface; +using osuTK.Input; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public class TestSceneOsuMenu : OsuManualInputManagerTestScene + { + public override IReadOnlyList RequiredTypes => new[] + { + typeof(OsuMenu), + typeof(DrawableOsuMenuItem) + }; + + private OsuMenu menu; + private bool actionPeformed; + + [SetUp] + public void Setup() => Schedule(() => + { + actionPeformed = false; + + Child = menu = new OsuMenu(Direction.Vertical, true) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Items = new[] + { + new OsuMenuItem("standard", MenuItemType.Standard, performAction), + new OsuMenuItem("highlighted", MenuItemType.Highlighted, performAction), + new OsuMenuItem("destructive", MenuItemType.Destructive, performAction), + } + }; + }); + + [Test] + public void TestClickEnabledMenuItem() + { + AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("action performed", () => actionPeformed); + } + + [Test] + public void TestDisableMenuItemsAndClick() + { + AddStep("disable menu items", () => + { + foreach (var item in menu.Items) + ((OsuMenuItem)item).Enabled.Value = false; + }); + + AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("action not performed", () => !actionPeformed); + } + + [Test] + public void TestEnableMenuItemsAndClick() + { + AddStep("disable menu items", () => + { + foreach (var item in menu.Items) + ((OsuMenuItem)item).Enabled.Value = false; + }); + + AddStep("enable menu items", () => + { + foreach (var item in menu.Items) + ((OsuMenuItem)item).Enabled.Value = true; + }); + + AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); + AddStep("click", () => InputManager.Click(MouseButton.Left)); + + AddAssert("action performed", () => actionPeformed); + } + + private void performAction() => actionPeformed = true; + } +} diff --git a/osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs b/osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs index a3ca851341..abaae7b43c 100644 --- a/osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs +++ b/osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs @@ -42,6 +42,8 @@ namespace osu.Game.Graphics.UserInterface BackgroundColourHover = Color4Extensions.FromHex(@"172023"); updateTextColour(); + + Item.Action.BindDisabledChanged(_ => updateState(), true); } private void updateTextColour() @@ -65,19 +67,33 @@ namespace osu.Game.Graphics.UserInterface protected override bool OnHover(HoverEvent e) { - sampleHover.Play(); - text.BoldText.FadeIn(transition_length, Easing.OutQuint); - text.NormalText.FadeOut(transition_length, Easing.OutQuint); + updateState(); return base.OnHover(e); } protected override void OnHoverLost(HoverLostEvent e) { - text.BoldText.FadeOut(transition_length, Easing.OutQuint); - text.NormalText.FadeIn(transition_length, Easing.OutQuint); + updateState(); base.OnHoverLost(e); } + private void updateState() + { + Alpha = Item.Action.Disabled ? 0.2f : 1; + + if (IsHovered && !Item.Action.Disabled) + { + sampleHover.Play(); + text.BoldText.FadeIn(transition_length, Easing.OutQuint); + text.NormalText.FadeOut(transition_length, Easing.OutQuint); + } + else + { + text.BoldText.FadeOut(transition_length, Easing.OutQuint); + text.NormalText.FadeIn(transition_length, Easing.OutQuint); + } + } + protected override bool OnClick(ClickEvent e) { sampleClick.Play(); diff --git a/osu.Game/Graphics/UserInterface/OsuMenuItem.cs b/osu.Game/Graphics/UserInterface/OsuMenuItem.cs index 0fe41937ce..36122ca0b2 100644 --- a/osu.Game/Graphics/UserInterface/OsuMenuItem.cs +++ b/osu.Game/Graphics/UserInterface/OsuMenuItem.cs @@ -2,12 +2,15 @@ // See the LICENCE file in the repository root for full licence text. using System; +using osu.Framework.Bindables; using osu.Framework.Graphics.UserInterface; namespace osu.Game.Graphics.UserInterface { public class OsuMenuItem : MenuItem { + public readonly Bindable Enabled = new Bindable(true); + public readonly MenuItemType Type; public OsuMenuItem(string text, MenuItemType type = MenuItemType.Standard) @@ -19,6 +22,9 @@ namespace osu.Game.Graphics.UserInterface : base(text, action) { Type = type; + + Enabled.BindValueChanged(enabled => Action.Disabled = !enabled.NewValue); + Action.BindDisabledChanged(disabled => Enabled.Value = !disabled); } } } From e8c955ed9b0a253ba2bfa6bd1e1af737a4e34440 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 15 Apr 2020 15:50:43 +0900 Subject: [PATCH 14/21] Add CanUndo/CanRedo bindables --- .../Editor/EditorChangeHandlerTest.cs | 25 +++++++++++-------- osu.Game/Screens/Edit/EditorChangeHandler.cs | 17 ++++++++++--- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Editor/EditorChangeHandlerTest.cs b/osu.Game.Tests/Editor/EditorChangeHandlerTest.cs index ef16976130..9613f250c4 100644 --- a/osu.Game.Tests/Editor/EditorChangeHandlerTest.cs +++ b/osu.Game.Tests/Editor/EditorChangeHandlerTest.cs @@ -15,15 +15,18 @@ namespace osu.Game.Tests.Editor { var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap())); - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); + Assert.That(handler.CanRedo.Value, Is.False); handler.SaveState(); - Assert.That(handler.HasUndoState, Is.True); + Assert.That(handler.CanUndo.Value, Is.True); + Assert.That(handler.CanRedo.Value, Is.False); handler.RestoreState(-1); - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); + Assert.That(handler.CanRedo.Value, Is.True); } [Test] @@ -31,20 +34,20 @@ namespace osu.Game.Tests.Editor { var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap())); - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES; i++) handler.SaveState(); - Assert.That(handler.HasUndoState, Is.True); + Assert.That(handler.CanUndo.Value, Is.True); for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES; i++) { - Assert.That(handler.HasUndoState, Is.True); + Assert.That(handler.CanUndo.Value, Is.True); handler.RestoreState(-1); } - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); } [Test] @@ -52,20 +55,20 @@ namespace osu.Game.Tests.Editor { var handler = new EditorChangeHandler(new EditorBeatmap(new Beatmap())); - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES * 2; i++) handler.SaveState(); - Assert.That(handler.HasUndoState, Is.True); + Assert.That(handler.CanUndo.Value, Is.True); for (int i = 0; i < EditorChangeHandler.MAX_SAVED_STATES; i++) { - Assert.That(handler.HasUndoState, Is.True); + Assert.That(handler.CanUndo.Value, Is.True); handler.RestoreState(-1); } - Assert.That(handler.HasUndoState, Is.False); + Assert.That(handler.CanUndo.Value, Is.False); } } } diff --git a/osu.Game/Screens/Edit/EditorChangeHandler.cs b/osu.Game/Screens/Edit/EditorChangeHandler.cs index a8204715cd..1553c2d2ef 100644 --- a/osu.Game/Screens/Edit/EditorChangeHandler.cs +++ b/osu.Game/Screens/Edit/EditorChangeHandler.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Text; +using osu.Framework.Bindables; using osu.Game.Beatmaps.Formats; using osu.Game.Rulesets.Objects; @@ -15,8 +16,10 @@ namespace osu.Game.Screens.Edit /// public class EditorChangeHandler : IEditorChangeHandler { - private readonly LegacyEditorBeatmapPatcher patcher; + public readonly Bindable CanUndo = new Bindable(); + public readonly Bindable CanRedo = new Bindable(); + private readonly LegacyEditorBeatmapPatcher patcher; private readonly List savedStates = new List(); private int currentState = -1; @@ -45,8 +48,6 @@ namespace osu.Game.Screens.Edit SaveState(); } - public bool HasUndoState => currentState > 0; - private void hitObjectAdded(HitObject obj) => SaveState(); private void hitObjectRemoved(HitObject obj) => SaveState(); @@ -90,6 +91,8 @@ namespace osu.Game.Screens.Edit } currentState = savedStates.Count - 1; + + updateBindables(); } /// @@ -114,6 +117,14 @@ namespace osu.Game.Screens.Edit currentState = newState; isRestoring = false; + + updateBindables(); + } + + private void updateBindables() + { + CanUndo.Value = savedStates.Count > 0 && currentState > 0; + CanRedo.Value = currentState < savedStates.Count - 1; } } } From ce21cfbb035b16bb42b473614edbe372b5ace04b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 15 Apr 2020 16:17:34 +0900 Subject: [PATCH 15/21] Use bindables in menu items --- osu.Game/Screens/Edit/Editor.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 14a227eb07..ad17498d93 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -155,8 +155,8 @@ namespace osu.Game.Screens.Edit { Items = new[] { - new EditorMenuItem("Undo", MenuItemType.Standard, undo), - new EditorMenuItem("Redo", MenuItemType.Standard, redo) + new EditorMenuItem("Undo", MenuItemType.Standard, undo) { Enabled = { BindTarget = changeHandler.CanUndo } }, + new EditorMenuItem("Redo", MenuItemType.Standard, redo) { Enabled = { BindTarget = changeHandler.CanRedo } } } } } From e2b28bfe88cc94c685a8ddbe3f496190c39103c1 Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 15 Apr 2020 18:17:12 -0700 Subject: [PATCH 16/21] Hide edit context menu item in multiplayer song select --- .../Select/Carousel/DrawableCarouselBeatmap.cs | 12 +++++++++--- osu.Game/Screens/Select/SongSelect.cs | 10 ---------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 2520c70989..a371c56101 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -41,6 +41,9 @@ namespace osu.Game.Screens.Select.Carousel [Resolved(CanBeNull = true)] private BeatmapSetOverlay beatmapOverlay { get; set; } + [Resolved(CanBeNull = true)] + private SongSelect songSelect { get; set; } + public DrawableCarouselBeatmap(CarouselBeatmap panel) : base(panel) { @@ -49,7 +52,7 @@ namespace osu.Game.Screens.Select.Carousel } [BackgroundDependencyLoader(true)] - private void load(SongSelect songSelect, BeatmapManager manager) + private void load(BeatmapManager manager) { if (songSelect != null) { @@ -190,10 +193,13 @@ namespace osu.Game.Screens.Select.Carousel List items = new List { new OsuMenuItem("Play", MenuItemType.Highlighted, () => startRequested?.Invoke(beatmap)), - new OsuMenuItem("Edit", MenuItemType.Standard, () => editRequested?.Invoke(beatmap)), - new OsuMenuItem("Hide", MenuItemType.Destructive, () => hideRequested?.Invoke(beatmap)), }; + if (songSelect.AllowEditing) + items.Add(new OsuMenuItem("Edit", MenuItemType.Standard, () => editRequested?.Invoke(beatmap))); + + items.Add(new OsuMenuItem("Hide", MenuItemType.Destructive, () => hideRequested?.Invoke(beatmap))); + if (beatmap.OnlineBeatmapID.HasValue) items.Add(new OsuMenuItem("Details", MenuItemType.Standard, () => beatmapOverlay?.FetchAndShowBeatmap(beatmap.OnlineBeatmapID.Value))); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index f164056ede..8967628954 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -34,7 +34,6 @@ using System.Linq; using System.Threading.Tasks; using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Bindings; -using osu.Game.Overlays.Notifications; using osu.Game.Scoring; namespace osu.Game.Screens.Select @@ -71,9 +70,6 @@ namespace osu.Game.Screens.Select /// public virtual bool AllowEditing => true; - [Resolved(canBeNull: true)] - private NotificationOverlay notificationOverlay { get; set; } - [Resolved] private Bindable> selectedMods { get; set; } @@ -328,12 +324,6 @@ namespace osu.Game.Screens.Select public void Edit(BeatmapInfo beatmap = null) { - if (!AllowEditing) - { - notificationOverlay?.Post(new SimpleNotification { Text = "Editing is not available from the current mode." }); - return; - } - Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap ?? beatmapNoDebounce); this.Push(new Editor()); } From 06e25091f666c8b8f2ac4e4e42f1d24d83026c37 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Apr 2020 10:44:08 +0900 Subject: [PATCH 17/21] Fix typo --- .../Visual/UserInterface/TestSceneOsuMenu.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs index cdda1969ca..c171e567ad 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs @@ -21,12 +21,12 @@ namespace osu.Game.Tests.Visual.UserInterface }; private OsuMenu menu; - private bool actionPeformed; + private bool actionPerformed; [SetUp] public void Setup() => Schedule(() => { - actionPeformed = false; + actionPerformed = false; Child = menu = new OsuMenu(Direction.Vertical, true) { @@ -47,7 +47,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); AddStep("click", () => InputManager.Click(MouseButton.Left)); - AddAssert("action performed", () => actionPeformed); + AddAssert("action performed", () => actionPerformed); } [Test] @@ -62,7 +62,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); AddStep("click", () => InputManager.Click(MouseButton.Left)); - AddAssert("action not performed", () => !actionPeformed); + AddAssert("action not performed", () => !actionPerformed); } [Test] @@ -83,9 +83,9 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); AddStep("click", () => InputManager.Click(MouseButton.Left)); - AddAssert("action performed", () => actionPeformed); + AddAssert("action performed", () => actionPerformed); } - private void performAction() => actionPeformed = true; + private void performAction() => actionPerformed = true; } } From c4caf38febbe7862589c2f33d761c005a7e6f0fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Apr 2020 12:10:20 +0900 Subject: [PATCH 18/21] Simplify menu item checks (and add for other items) --- .../Carousel/DrawableCarouselBeatmap.cs | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index a371c56101..3e4798a812 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -41,9 +41,6 @@ namespace osu.Game.Screens.Select.Carousel [Resolved(CanBeNull = true)] private BeatmapSetOverlay beatmapOverlay { get; set; } - [Resolved(CanBeNull = true)] - private SongSelect songSelect { get; set; } - public DrawableCarouselBeatmap(CarouselBeatmap panel) : base(panel) { @@ -52,12 +49,13 @@ namespace osu.Game.Screens.Select.Carousel } [BackgroundDependencyLoader(true)] - private void load(BeatmapManager manager) + private void load(BeatmapManager manager, SongSelect songSelect) { if (songSelect != null) { startRequested = b => songSelect.FinaliseSelection(b); - editRequested = songSelect.Edit; + if (songSelect.AllowEditing) + editRequested = songSelect.Edit; } if (manager != null) @@ -190,18 +188,19 @@ namespace osu.Game.Screens.Select.Carousel { get { - List items = new List - { - new OsuMenuItem("Play", MenuItemType.Highlighted, () => startRequested?.Invoke(beatmap)), - }; + List items = new List(); - if (songSelect.AllowEditing) - items.Add(new OsuMenuItem("Edit", MenuItemType.Standard, () => editRequested?.Invoke(beatmap))); + if (startRequested != null) + items.Add(new OsuMenuItem("Play", MenuItemType.Highlighted, () => startRequested(beatmap))); - items.Add(new OsuMenuItem("Hide", MenuItemType.Destructive, () => hideRequested?.Invoke(beatmap))); + if (editRequested != null) + items.Add(new OsuMenuItem("Edit", MenuItemType.Standard, () => editRequested(beatmap))); - if (beatmap.OnlineBeatmapID.HasValue) - items.Add(new OsuMenuItem("Details", MenuItemType.Standard, () => beatmapOverlay?.FetchAndShowBeatmap(beatmap.OnlineBeatmapID.Value))); + if (hideRequested != null) + items.Add(new OsuMenuItem("Hide", MenuItemType.Destructive, () => hideRequested(beatmap))); + + if (beatmap.OnlineBeatmapID.HasValue && beatmapOverlay != null) + items.Add(new OsuMenuItem("Details", MenuItemType.Standard, () => beatmapOverlay.FetchAndShowBeatmap(beatmap.OnlineBeatmapID.Value))); return items.ToArray(); } From 91b13f91eaaaad87dd221bdb8daf3ed34d7166b2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Apr 2020 12:11:12 +0900 Subject: [PATCH 19/21] Add exception disallowing potential edit when disabled at a property level --- osu.Game/Screens/Select/SongSelect.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 8967628954..5bc2e1aa56 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -324,6 +324,9 @@ namespace osu.Game.Screens.Select public void Edit(BeatmapInfo beatmap = null) { + if (!AllowEditing) + throw new InvalidOperationException($"Attempted to edit when {nameof(AllowEditing)} is disabled"); + Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap ?? beatmapNoDebounce); this.Push(new Editor()); } From 03a74a4320db317e063130161933694d4563ca85 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Apr 2020 12:13:26 +0900 Subject: [PATCH 20/21] Apply same conditional check changes to DrawableCarouselBeatmapSet --- .../Select/Carousel/DrawableCarouselBeatmapSet.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index a53b74c1b8..5acb6d1946 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -46,6 +46,7 @@ namespace osu.Game.Screens.Select.Carousel private void load(BeatmapManager manager, BeatmapSetOverlay beatmapOverlay) { restoreHiddenRequested = s => s.Beatmaps.ForEach(manager.Restore); + if (beatmapOverlay != null) viewDetails = beatmapOverlay.FetchAndShowBeatmapSet; @@ -131,13 +132,14 @@ namespace osu.Game.Screens.Select.Carousel if (Item.State.Value == CarouselItemState.NotSelected) items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => Item.State.Value = CarouselItemState.Selected)); - if (beatmapSet.OnlineBeatmapSetID != null) - items.Add(new OsuMenuItem("Details...", MenuItemType.Standard, () => viewDetails?.Invoke(beatmapSet.OnlineBeatmapSetID.Value))); + if (beatmapSet.OnlineBeatmapSetID != null && viewDetails != null) + items.Add(new OsuMenuItem("Details...", MenuItemType.Standard, () => viewDetails(beatmapSet.OnlineBeatmapSetID.Value))); if (beatmapSet.Beatmaps.Any(b => b.Hidden)) - items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => restoreHiddenRequested?.Invoke(beatmapSet))); + items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => restoreHiddenRequested(beatmapSet))); - items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => dialogOverlay?.Push(new BeatmapDeleteDialog(beatmapSet)))); + if (dialogOverlay != null) + items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => dialogOverlay.Push(new BeatmapDeleteDialog(beatmapSet)))); return items.ToArray(); } From 9e2be6f2f438dcc288bbe711c486a8cc112e310d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 16 Apr 2020 13:25:08 +0900 Subject: [PATCH 21/21] Remove bindable to promote one-way access --- osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs | 6 +++--- osu.Game/Graphics/UserInterface/OsuMenuItem.cs | 6 ------ osu.Game/Screens/Edit/Editor.cs | 9 +++++++-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs index c171e567ad..9ea76c2c7b 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneOsuMenu.cs @@ -56,7 +56,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("disable menu items", () => { foreach (var item in menu.Items) - ((OsuMenuItem)item).Enabled.Value = false; + ((OsuMenuItem)item).Action.Disabled = true; }); AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); @@ -71,13 +71,13 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("disable menu items", () => { foreach (var item in menu.Items) - ((OsuMenuItem)item).Enabled.Value = false; + ((OsuMenuItem)item).Action.Disabled = true; }); AddStep("enable menu items", () => { foreach (var item in menu.Items) - ((OsuMenuItem)item).Enabled.Value = true; + ((OsuMenuItem)item).Action.Disabled = false; }); AddStep("move to first menu item", () => InputManager.MoveMouseTo(menu.ChildrenOfType().First())); diff --git a/osu.Game/Graphics/UserInterface/OsuMenuItem.cs b/osu.Game/Graphics/UserInterface/OsuMenuItem.cs index 36122ca0b2..0fe41937ce 100644 --- a/osu.Game/Graphics/UserInterface/OsuMenuItem.cs +++ b/osu.Game/Graphics/UserInterface/OsuMenuItem.cs @@ -2,15 +2,12 @@ // See the LICENCE file in the repository root for full licence text. using System; -using osu.Framework.Bindables; using osu.Framework.Graphics.UserInterface; namespace osu.Game.Graphics.UserInterface { public class OsuMenuItem : MenuItem { - public readonly Bindable Enabled = new Bindable(true); - public readonly MenuItemType Type; public OsuMenuItem(string text, MenuItemType type = MenuItemType.Standard) @@ -22,9 +19,6 @@ namespace osu.Game.Graphics.UserInterface : base(text, action) { Type = type; - - Enabled.BindValueChanged(enabled => Action.Disabled = !enabled.NewValue); - Action.BindDisabledChanged(disabled => Enabled.Value = !disabled); } } } diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index ad17498d93..9a1f450dc6 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -107,6 +107,8 @@ namespace osu.Game.Screens.Edit dependencies.CacheAs(changeHandler); EditorMenuBar menuBar; + OsuMenuItem undoMenuItem; + OsuMenuItem redoMenuItem; var fileMenuItems = new List { @@ -155,8 +157,8 @@ namespace osu.Game.Screens.Edit { Items = new[] { - new EditorMenuItem("Undo", MenuItemType.Standard, undo) { Enabled = { BindTarget = changeHandler.CanUndo } }, - new EditorMenuItem("Redo", MenuItemType.Standard, redo) { Enabled = { BindTarget = changeHandler.CanRedo } } + undoMenuItem = new EditorMenuItem("Undo", MenuItemType.Standard, undo), + redoMenuItem = new EditorMenuItem("Redo", MenuItemType.Standard, redo) } } } @@ -214,6 +216,9 @@ namespace osu.Game.Screens.Edit } }); + changeHandler.CanUndo.BindValueChanged(v => undoMenuItem.Action.Disabled = !v.NewValue, true); + changeHandler.CanRedo.BindValueChanged(v => redoMenuItem.Action.Disabled = !v.NewValue, true); + menuBar.Mode.ValueChanged += onModeChanged; bottomBackground.Colour = colours.Gray2;