From 624f9fc774732ae8c7ee795968414d3f7f97926b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 16 Mar 2022 22:41:28 +0100 Subject: [PATCH 01/58] Implement mod settings area component --- .../UserInterface/TestSceneModSettingsArea.cs | 40 ++++ osu.Game/Overlays/Mods/ModSettingsArea.cs | 176 ++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneModSettingsArea.cs create mode 100644 osu.Game/Overlays/Mods/ModSettingsArea.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSettingsArea.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettingsArea.cs new file mode 100644 index 0000000000..ddc1c8c128 --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSettingsArea.cs @@ -0,0 +1,40 @@ +// 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 NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Game.Overlays; +using osu.Game.Overlays.Mods; +using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Osu.Mods; + +namespace osu.Game.Tests.Visual.UserInterface +{ + [TestFixture] + public class TestSceneModSettingsArea : OsuTestScene + { + [Cached] + private OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Green); + + [Test] + public void TestModToggleArea() + { + ModSettingsArea modSettingsArea = null; + + AddStep("create content", () => Child = new Container + { + RelativeSizeAxes = Axes.Both, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Child = modSettingsArea = new ModSettingsArea() + }); + AddStep("set DT", () => modSettingsArea.SelectedMods.Value = new[] { new OsuModDoubleTime() }); + AddStep("set DA", () => modSettingsArea.SelectedMods.Value = new Mod[] { new OsuModDifficultyAdjust() }); + AddStep("set FL+WU+DA+AD", () => modSettingsArea.SelectedMods.Value = new Mod[] { new OsuModFlashlight(), new ModWindUp(), new OsuModDifficultyAdjust(), new OsuModApproachDifferent() }); + AddStep("set empty", () => modSettingsArea.SelectedMods.Value = Array.Empty()); + } + } +} diff --git a/osu.Game/Overlays/Mods/ModSettingsArea.cs b/osu.Game/Overlays/Mods/ModSettingsArea.cs new file mode 100644 index 0000000000..e0a30f60c2 --- /dev/null +++ b/osu.Game/Overlays/Mods/ModSettingsArea.cs @@ -0,0 +1,176 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Input.Events; +using osu.Game.Configuration; +using osu.Game.Graphics; +using osu.Game.Graphics.Containers; +using osu.Game.Graphics.Sprites; +using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.UI; +using osuTK; + +namespace osu.Game.Overlays.Mods +{ + public class ModSettingsArea : CompositeDrawable + { + public Bindable> SelectedMods { get; } = new Bindable>(); + + private readonly Box background; + private readonly FillFlowContainer modSettingsFlow; + + [Resolved] + private OverlayColourProvider colourProvider { get; set; } + + public ModSettingsArea() + { + RelativeSizeAxes = Axes.X; + Height = 250; + + Anchor = Anchor.BottomRight; + Origin = Anchor.BottomRight; + + InternalChild = new Container + { + RelativeSizeAxes = Axes.Both, + Masking = true, + BorderThickness = 2, + Children = new Drawable[] + { + background = new Box + { + RelativeSizeAxes = Axes.Both + }, + new OsuScrollContainer(Direction.Horizontal) + { + RelativeSizeAxes = Axes.Both, + ScrollbarOverlapsContent = false, + Child = modSettingsFlow = new FillFlowContainer + { + AutoSizeAxes = Axes.X, + RelativeSizeAxes = Axes.Y, + Padding = new MarginPadding { Vertical = 7, Horizontal = 70 }, + Spacing = new Vector2(7), + Direction = FillDirection.Horizontal + } + } + } + }; + } + + [BackgroundDependencyLoader] + private void load() + { + background.Colour = colourProvider.Dark3; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + SelectedMods.BindValueChanged(_ => updateMods()); + } + + private void updateMods() + { + modSettingsFlow.Clear(); + + foreach (var mod in SelectedMods.Value.OrderBy(mod => mod.Type).ThenBy(mod => mod.Acronym)) + { + var settings = mod.CreateSettingsControls().ToList(); + + if (settings.Count > 0) + { + if (modSettingsFlow.Any()) + { + modSettingsFlow.Add(new Box + { + RelativeSizeAxes = Axes.Y, + Width = 2, + Colour = colourProvider.Dark4, + }); + } + + modSettingsFlow.Add(new ModSettingsColumn(mod, settings)); + } + } + } + + protected override bool OnMouseDown(MouseDownEvent e) => true; + protected override bool OnHover(HoverEvent e) => true; + + private class ModSettingsColumn : CompositeDrawable + { + public ModSettingsColumn(Mod mod, IEnumerable settingsControls) + { + Width = 250; + RelativeSizeAxes = Axes.Y; + Padding = new MarginPadding { Bottom = 7 }; + + InternalChild = new GridContainer + { + RelativeSizeAxes = Axes.Both, + RowDimensions = new[] + { + new Dimension(GridSizeMode.AutoSize), + new Dimension(GridSizeMode.Absolute, 10), + new Dimension() + }, + Content = new[] + { + new Drawable[] + { + new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Direction = FillDirection.Horizontal, + Spacing = new Vector2(7), + Children = new Drawable[] + { + new ModSwitchTiny(mod) + { + Active = { Value = true }, + Scale = new Vector2(0.6f), + Origin = Anchor.CentreLeft, + Anchor = Anchor.CentreLeft + }, + new OsuSpriteText + { + Text = mod.Name, + Font = OsuFont.Default.With(size: 16, weight: FontWeight.SemiBold), + Origin = Anchor.CentreLeft, + Anchor = Anchor.CentreLeft, + Margin = new MarginPadding { Bottom = 2 } + } + } + } + }, + new[] { Empty() }, + new Drawable[] + { + new OsuScrollContainer(Direction.Vertical) + { + RelativeSizeAxes = Axes.Both, + Child = new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Padding = new MarginPadding { Right = 7 }, + ChildrenEnumerable = settingsControls, + Spacing = new Vector2(0, 7) + } + } + } + } + }; + } + } + } +} From 4adb8c205f7523768f741c2b6d80f0aaa04fdf8a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 07:18:49 +0300 Subject: [PATCH 02/58] Add step to switch hosts randomly in `TestManyUsers` --- .../Multiplayer/TestSceneMultiplayerParticipantsList.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs index 292319171d..bc3bce4b3f 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs @@ -202,9 +202,11 @@ namespace osu.Game.Tests.Visual.Multiplayer [Test] public void TestManyUsers() { + const int users_count = 20; + AddStep("add many users", () => { - for (int i = 0; i < 20; i++) + for (int i = 0; i < users_count; i++) { MultiplayerClient.AddUser(new APIUser { @@ -243,6 +245,8 @@ namespace osu.Game.Tests.Visual.Multiplayer } } }); + + AddRepeatStep("switch hosts", () => MultiplayerClient.TransferHost(RNG.Next(0, users_count)), 10); } [Test] From 1c899d00b92cdea921fcad9ea15cf33fbc839064 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 07:25:03 +0300 Subject: [PATCH 03/58] Pin multiplayer host panel to the top of the list --- .../Multiplayer/Participants/ParticipantsList.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs index afb2111023..6169ad6a9e 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -15,6 +16,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants { private FillFlowContainer panels; + [CanBeNull] + private ParticipantPanel currentHostPanel; + [BackgroundDependencyLoader] private void load() { @@ -55,6 +59,16 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants // Add panels for all users new to the room. foreach (var user in Room.Users.Except(panels.Select(p => p.User))) panels.Add(new ParticipantPanel(user)); + + if (currentHostPanel == null || !currentHostPanel.User.Equals(Room.Host)) + { + // Reset position of previous host back to normal, if one existing. + if (currentHostPanel != null && panels.Contains(currentHostPanel)) + panels.SetLayoutPosition(currentHostPanel, 0); + + // Change position of new host to display above all participants. + panels.SetLayoutPosition(currentHostPanel = panels.Single(u => u.User.Equals(Room.Host)), -1); + } } } } From 98b420ee6f2b256aa6e70847751fe311739b2572 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 07:25:10 +0300 Subject: [PATCH 04/58] Remove no longer correct crown fade animation Since the host is pinned to the top without any animation, it would look jarring for the crown to fade away from the old panel (and at a 50ms duration). --- .../OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs index 96a665f33d..11dba2e52d 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs @@ -206,10 +206,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants else kickButton.FadeOut(fade_time); - if (Room.Host?.Equals(User) == true) - crown.FadeIn(fade_time); - else - crown.FadeOut(fade_time); + crown.Alpha = Room.Host?.Equals(User) == true ? 1 : 0; // If the mods are updated at the end of the frame, the flow container will skip a reflow cycle: https://github.com/ppy/osu-framework/issues/4187 // This looks particularly jarring here, so re-schedule the update to that start of our frame as a fix. From d0cc68bc97ade2891f3e1ee767588d0946b27507 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 07:26:10 +0300 Subject: [PATCH 05/58] Add test coverage --- .../TestSceneMultiplayerParticipantsList.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs index bc3bce4b3f..50faa0a567 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs @@ -163,6 +163,25 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("second user crown visible", () => this.ChildrenOfType().ElementAt(1).ChildrenOfType().First().Alpha == 1); } + [Test] + public void TestHostGetsPinnedToTop() + { + AddStep("add user", () => MultiplayerClient.AddUser(new APIUser + { + Id = 3, + Username = "Second", + CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c3.jpg", + })); + + AddStep("make second user host", () => MultiplayerClient.TransferHost(3)); + AddAssert("second user above first", () => + { + var first = this.ChildrenOfType().ElementAt(0); + var second = this.ChildrenOfType().ElementAt(1); + return second.Y < first.Y; + }); + } + [Test] public void TestKickButtonOnlyPresentWhenHost() { From a7ddfc7f519d2b3583814425071eccf1d4c8d5d2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 08:08:31 +0300 Subject: [PATCH 06/58] Add step for returning host back to local user --- .../Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs index 50faa0a567..8da077cd44 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerParticipantsList.cs @@ -266,6 +266,7 @@ namespace osu.Game.Tests.Visual.Multiplayer }); AddRepeatStep("switch hosts", () => MultiplayerClient.TransferHost(RNG.Next(0, users_count)), 10); + AddStep("give host back", () => MultiplayerClient.TransferHost(API.LocalUser.Value.Id)); } [Test] From 1bd08b4a4b02499a7aed551d338494d3dd0952f6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 18 Mar 2022 08:08:45 +0300 Subject: [PATCH 07/58] Remove kick button fading as well to not look jarring --- .../OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs index 11dba2e52d..e95ca4de32 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs @@ -201,11 +201,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants else userModsDisplay.FadeOut(fade_time); - if (Client.IsHost && !User.Equals(Client.LocalUser)) - kickButton.FadeIn(fade_time); - else - kickButton.FadeOut(fade_time); - + kickButton.Alpha = Client.IsHost && !User.Equals(Client.LocalUser) ? 1 : 0; crown.Alpha = Room.Host?.Equals(User) == true ? 1 : 0; // If the mods are updated at the end of the frame, the flow container will skip a reflow cycle: https://github.com/ppy/osu-framework/issues/4187 From 0adad3a599904f62a9f7908ae85b678c1b83e0c8 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 19 Mar 2022 04:01:35 +0300 Subject: [PATCH 08/58] Handle potential null room hosts --- .../OnlinePlay/Multiplayer/Participants/ParticipantsList.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs index 6169ad6a9e..22c9940cd4 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs @@ -66,8 +66,11 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants if (currentHostPanel != null && panels.Contains(currentHostPanel)) panels.SetLayoutPosition(currentHostPanel, 0); + currentHostPanel = null; + // Change position of new host to display above all participants. - panels.SetLayoutPosition(currentHostPanel = panels.Single(u => u.User.Equals(Room.Host)), -1); + if (Room.Host != null) + panels.SetLayoutPosition(currentHostPanel = panels.SingleOrDefault(u => u.User.Equals(Room.Host)), -1); } } } From 9afe82a0d522adb13b9c58dc40c95fbbd66e9403 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 19 Mar 2022 14:54:58 +0300 Subject: [PATCH 09/58] Fix potentially null drawable call to `SetLayoutPosition` --- .../Multiplayer/Participants/ParticipantsList.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs index 22c9940cd4..14b930f115 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantsList.cs @@ -70,7 +70,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants // Change position of new host to display above all participants. if (Room.Host != null) - panels.SetLayoutPosition(currentHostPanel = panels.SingleOrDefault(u => u.User.Equals(Room.Host)), -1); + { + currentHostPanel = panels.SingleOrDefault(u => u.User.Equals(Room.Host)); + + if (currentHostPanel != null) + panels.SetLayoutPosition(currentHostPanel, -1); + } } } } From 09ec49e6fa68e714730043d9ec723bc352456017 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Mar 2022 18:30:54 +0900 Subject: [PATCH 10/58] Rename realm-backed resource store in preparation for non-legacy usage --- osu.Game/Skinning/DefaultLegacySkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 2 +- ...sedSkinResourceStore.cs => RealmBackedResourceStore.cs} | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) rename osu.Game/Skinning/{LegacyDatabasedSkinResourceStore.cs => RealmBackedResourceStore.cs} (83%) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index c7033d37dc..3a8464879a 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -34,7 +34,7 @@ namespace osu.Game.Skinning resources, // A default legacy skin may still have a skin.ini if it is modified by the user. // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files. - new LegacyDatabasedSkinResourceStore(skin, resources.Files).GetStream("skin.ini") + new RealmBackedResourceStore(skin, resources.Files).GetStream("skin.ini") ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 359d9e5624..9d71fff92f 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -51,7 +51,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, new LegacyDatabasedSkinResourceStore(skin, resources.Files), resources, "skin.ini") + : this(skin, new RealmBackedResourceStore(skin, resources.Files), resources, "skin.ini") { } diff --git a/osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs similarity index 83% rename from osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs rename to osu.Game/Skinning/RealmBackedResourceStore.cs index cd90fea9bb..93ffbe4f44 100644 --- a/osu.Game/Skinning/LegacyDatabasedSkinResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -4,21 +4,22 @@ using System.Collections.Generic; using osu.Framework.Extensions; using osu.Framework.IO.Stores; +using osu.Game.Database; using osu.Game.Extensions; namespace osu.Game.Skinning { - public class LegacyDatabasedSkinResourceStore : ResourceStore + public class RealmBackedResourceStore : ResourceStore { private readonly Dictionary fileToStoragePathMapping = new Dictionary(); - public LegacyDatabasedSkinResourceStore(SkinInfo source, IResourceStore underlyingStore) + public RealmBackedResourceStore(IHasRealmFiles source, IResourceStore underlyingStore) : base(underlyingStore) { initialiseFileCache(source); } - private void initialiseFileCache(SkinInfo source) + private void initialiseFileCache(IHasRealmFiles source) { fileToStoragePathMapping.Clear(); foreach (var f in source.Files) From 5c4a74378dec7f2b64f5de49abde8779c6793486 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Mar 2022 18:36:42 +0900 Subject: [PATCH 11/58] Remove `Textures` and `Samples` initialisation to `Skin` --- osu.Game/Skinning/LegacySkin.cs | 25 --------- osu.Game/Skinning/Skin.cs | 99 ++++++++++++++++++--------------- 2 files changed, 53 insertions(+), 71 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 9d71fff92f..5533615568 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -27,12 +27,6 @@ namespace osu.Game.Skinning { public class LegacySkin : Skin { - [CanBeNull] - protected TextureStore Textures; - - [CanBeNull] - protected ISampleStore Samples; - /// /// Whether texture for the keys exists. /// Used to determine if the mania ruleset is skinned. @@ -77,18 +71,6 @@ namespace osu.Game.Skinning protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) : base(skin, resources, configurationStream) { - if (storage != null) - { - var samples = resources?.AudioManager?.GetSampleStore(storage); - if (samples != null) - samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - - Samples = samples; - Textures = new TextureStore(resources?.CreateTextureLoaderStore(storage)); - - (storage as ResourceStore)?.AddExtension("ogg"); - } - // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. hasKeyTexture = new Lazy(() => this.GetAnimation( lookupForMania(new LegacyManiaSkinConfigurationLookup(4, LegacyManiaSkinConfigurationLookups.KeyImage, 0))?.Value ?? "mania-key1", true, @@ -551,12 +533,5 @@ namespace osu.Game.Skinning // Fall back to using the last piece for components coming from lazer (e.g. "Gameplay/osu/approachcircle" -> "approachcircle"). yield return componentName.Split('/').Last(); } - - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - Textures?.Dispose(); - Samples?.Dispose(); - } } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 931bdfed48..d1ea7aa300 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -16,7 +16,6 @@ using osu.Framework.Graphics.Textures; using osu.Framework.Logging; using osu.Game.Audio; using osu.Game.Database; -using osu.Game.Extensions; using osu.Game.IO; using osu.Game.Screens.Play.HUD; @@ -24,8 +23,19 @@ namespace osu.Game.Skinning { public abstract class Skin : IDisposable, ISkin { + /// + /// A texture store which can be used to perform user file loops for this skin. + /// + [CanBeNull] + protected TextureStore Textures { get; set; } + + /// + /// A sample store which can be used to perform user file loops for this skin. + /// + [CanBeNull] + protected ISampleStore Samples { get; set; } + public readonly Live SkinInfo; - private readonly IStorageResourceProvider resources; public SkinConfiguration Configuration { get; set; } @@ -41,16 +51,30 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(TLookup lookup); + private readonly RealmBackedResourceStore skinStorage; + protected Skin(SkinInfo skin, IStorageResourceProvider resources, [CanBeNull] Stream configurationStream = null) { - SkinInfo = resources?.RealmAccess != null - ? skin.ToLive(resources.RealmAccess) - // This path should only be used in some tests. - : skin.ToLiveUnmanaged(); + if (resources.RealmAccess != null) + { + SkinInfo = skin.ToLive(resources.RealmAccess); + skinStorage = new RealmBackedResourceStore(skin, resources.Files); - this.resources = resources; + var samples = resources?.AudioManager?.GetSampleStore(skinStorage); + if (samples != null) + samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - configurationStream ??= getConfigurationStream(); + Samples = samples; + Textures = new TextureStore(resources?.CreateTextureLoaderStore(skinStorage)); + + skinStorage.AddExtension("ogg"); + } + else + { + SkinInfo = skin.ToLiveUnmanaged(); + } + + configurationStream ??= skinStorage?.GetStream(@"skin.ini"); if (configurationStream != null) // stream will be closed after use by LineBufferedReader. @@ -59,40 +83,30 @@ namespace osu.Game.Skinning Configuration = new SkinConfiguration(); // skininfo files may be null for default skin. - SkinInfo.PerformRead(s => + foreach (SkinnableTarget skinnableTarget in Enum.GetValues(typeof(SkinnableTarget))) { - // we may want to move this to some kind of async operation in the future. - foreach (SkinnableTarget skinnableTarget in Enum.GetValues(typeof(SkinnableTarget))) + string filename = $"{skinnableTarget}.json"; + + byte[] bytes = skinStorage?.Get(filename); + + if (bytes == null) + continue; + + try { - string filename = $"{skinnableTarget}.json"; + string jsonContent = Encoding.UTF8.GetString(bytes); + var deserializedContent = JsonConvert.DeserializeObject>(jsonContent); - // skininfo files may be null for default skin. - var fileInfo = s.Files.FirstOrDefault(f => f.Filename == filename); - - if (fileInfo == null) + if (deserializedContent == null) continue; - byte[] bytes = resources?.Files.Get(fileInfo.File.GetStoragePath()); - - if (bytes == null) - continue; - - try - { - string jsonContent = Encoding.UTF8.GetString(bytes); - var deserializedContent = JsonConvert.DeserializeObject>(jsonContent); - - if (deserializedContent == null) - continue; - - DrawableComponentInfo[skinnableTarget] = deserializedContent.ToArray(); - } - catch (Exception ex) - { - Logger.Error(ex, "Failed to load skin configuration."); - } + DrawableComponentInfo[skinnableTarget] = deserializedContent.ToArray(); } - }); + catch (Exception ex) + { + Logger.Error(ex, "Failed to load skin configuration."); + } + } } protected virtual void ParseConfigurationStream(Stream stream) @@ -101,16 +115,6 @@ namespace osu.Game.Skinning Configuration = new LegacySkinDecoder().Decode(reader); } - private Stream getConfigurationStream() - { - string path = SkinInfo.PerformRead(s => s.Files.SingleOrDefault(f => f.Filename.Equals(@"skin.ini", StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath()); - - if (string.IsNullOrEmpty(path)) - return null; - - return resources?.Files.GetStream(path); - } - /// /// Remove all stored customisations for the provided target. /// @@ -168,6 +172,9 @@ namespace osu.Game.Skinning return; isDisposed = true; + + Textures?.Dispose(); + Samples?.Dispose(); } #endregion From b4d89b4e313863e6dec8ce85a4948ef4d9321b63 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Mar 2022 18:42:45 +0900 Subject: [PATCH 12/58] Replace duplicate `LegacySkinResourceStore` class with `RealmBackedResourceStore` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- osu.Game/Skinning/LegacySkinResourceStore.cs | 39 -------------------- 2 files changed, 1 insertion(+), 40 deletions(-) delete mode 100644 osu.Game/Skinning/LegacySkinResourceStore.cs diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index f80a980351..2ddf79ed5a 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -21,7 +21,7 @@ namespace osu.Game.Skinning protected override bool UseCustomSampleBanks => true; public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IResourceStore storage, IStorageResourceProvider resources) - : base(createSkinInfo(beatmapInfo), new LegacySkinResourceStore(beatmapInfo.BeatmapSet, storage), resources, beatmapInfo.Path) + : base(createSkinInfo(beatmapInfo), new RealmBackedResourceStore(beatmapInfo.BeatmapSet, storage), resources, beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; diff --git a/osu.Game/Skinning/LegacySkinResourceStore.cs b/osu.Game/Skinning/LegacySkinResourceStore.cs deleted file mode 100644 index 2487a469c8..0000000000 --- a/osu.Game/Skinning/LegacySkinResourceStore.cs +++ /dev/null @@ -1,39 +0,0 @@ -// 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.Framework.Extensions; -using osu.Framework.IO.Stores; -using osu.Game.Database; -using osu.Game.Extensions; - -namespace osu.Game.Skinning -{ - public class LegacySkinResourceStore : ResourceStore - { - private readonly IHasNamedFiles source; - - public LegacySkinResourceStore(IHasNamedFiles source, IResourceStore underlyingStore) - : base(underlyingStore) - { - this.source = source; - } - - protected override IEnumerable GetFilenames(string name) - { - foreach (string filename in base.GetFilenames(name)) - { - string path = getPathForFile(filename.ToStandardisedPath()); - if (path != null) - yield return path; - } - } - - private string getPathForFile(string filename) => - source.Files.FirstOrDefault(f => string.Equals(f.Filename, filename, StringComparison.OrdinalIgnoreCase))?.File.GetStoragePath(); - - public override IEnumerable GetAvailableResources() => source.Files.Select(f => f.Filename); - } -} From 35d2f973a39b533ed1cb7392d31d1d8299e9b0c9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Mar 2022 19:07:05 +0900 Subject: [PATCH 13/58] Prefer provided resource store over realm backed to keep tests working --- osu.Game/Skinning/LegacySkin.cs | 2 +- osu.Game/Skinning/Skin.cs | 39 +++++++++++++++++++-------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 5533615568..84844c4502 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -69,7 +69,7 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// An optional stream containing the contents of a skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) - : base(skin, resources, configurationStream) + : base(skin, resources, storage, configurationStream) { // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. hasKeyTexture = new Lazy(() => this.GetAnimation( diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index d1ea7aa300..a47da70cd6 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -13,6 +13,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.OpenGL.Textures; using osu.Framework.Graphics.Textures; +using osu.Framework.IO.Stores; using osu.Framework.Logging; using osu.Game.Audio; using osu.Game.Database; @@ -51,30 +52,36 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(TLookup lookup); - private readonly RealmBackedResourceStore skinStorage; - - protected Skin(SkinInfo skin, IStorageResourceProvider resources, [CanBeNull] Stream configurationStream = null) + protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] Stream configurationStream = null) { - if (resources.RealmAccess != null) + if (resources != null) { - SkinInfo = skin.ToLive(resources.RealmAccess); - skinStorage = new RealmBackedResourceStore(skin, resources.Files); + if (resources.RealmAccess != null) + { + SkinInfo = skin.ToLive(resources.RealmAccess); - var samples = resources?.AudioManager?.GetSampleStore(skinStorage); + if (storage == null) + { + var realmStorage = new RealmBackedResourceStore(skin, resources.Files); + realmStorage.AddExtension("ogg"); + + storage = realmStorage; + } + } + else + { + SkinInfo = skin.ToLiveUnmanaged(); + } + + var samples = resources.AudioManager?.GetSampleStore(storage); if (samples != null) samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; Samples = samples; - Textures = new TextureStore(resources?.CreateTextureLoaderStore(skinStorage)); - - skinStorage.AddExtension("ogg"); - } - else - { - SkinInfo = skin.ToLiveUnmanaged(); + Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); } - configurationStream ??= skinStorage?.GetStream(@"skin.ini"); + configurationStream ??= storage?.GetStream(@"skin.ini"); if (configurationStream != null) // stream will be closed after use by LineBufferedReader. @@ -87,7 +94,7 @@ namespace osu.Game.Skinning { string filename = $"{skinnableTarget}.json"; - byte[] bytes = skinStorage?.Get(filename); + byte[] bytes = storage?.Get(filename); if (bytes == null) continue; From a7f63fb034d5174dc7e51e46fdfce88a06a4a89c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Mar 2022 19:23:22 +0900 Subject: [PATCH 14/58] Make providing a custom `ResourceStore` to `LegacyBeatmapSkin` optional (for tests only) --- osu.Game.Tests/Skins/TestSceneBeatmapSkinLookupDisables.cs | 2 +- osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs | 2 +- .../Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs | 2 +- osu.Game/Beatmaps/WorkingBeatmapCache.cs | 2 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 +++-- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 2 +- osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Skins/TestSceneBeatmapSkinLookupDisables.cs b/osu.Game.Tests/Skins/TestSceneBeatmapSkinLookupDisables.cs index 71544e94f3..0c1981b35d 100644 --- a/osu.Game.Tests/Skins/TestSceneBeatmapSkinLookupDisables.cs +++ b/osu.Game.Tests/Skins/TestSceneBeatmapSkinLookupDisables.cs @@ -77,7 +77,7 @@ namespace osu.Game.Tests.Skins public class BeatmapSkinSource : LegacyBeatmapSkin { public BeatmapSkinSource() - : base(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo, null, null) + : base(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo, null) { } diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index 870d6d8f57..d3cacaa88c 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs @@ -202,7 +202,7 @@ namespace osu.Game.Tests.Skins public class BeatmapSkinSource : LegacyBeatmapSkin { public BeatmapSkinSource() - : base(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo, null, null) + : base(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo, null) { } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs index eb1695b3df..53364b6d89 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapSkinFallbacks.cs @@ -50,7 +50,7 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestEmptyLegacyBeatmapSkinFallsBack() { - CreateSkinTest(DefaultSkin.CreateInfo(), () => new LegacyBeatmapSkin(new BeatmapInfo(), null, null)); + CreateSkinTest(DefaultSkin.CreateInfo(), () => new LegacyBeatmapSkin(new BeatmapInfo(), null)); AddUntilStep("wait for hud load", () => Player.ChildrenOfType().All(c => c.ComponentsLoaded)); AddAssert("hud from default skin", () => AssertComponentsFromExpectedSource(SkinnableTarget.MainHUDComponents, skinManager.CurrentSkin.Value)); } diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index d3f356bb24..7d28208157 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -225,7 +225,7 @@ namespace osu.Game.Beatmaps { try { - return new LegacyBeatmapSkin(BeatmapInfo, resources.Files, resources); + return new LegacyBeatmapSkin(BeatmapInfo, resources); } catch (Exception e) { diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 2ddf79ed5a..9b3548912c 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.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 JetBrains.Annotations; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -20,8 +21,8 @@ namespace osu.Game.Skinning protected override bool AllowManiaSkin => false; protected override bool UseCustomSampleBanks => true; - public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IResourceStore storage, IStorageResourceProvider resources) - : base(createSkinInfo(beatmapInfo), new RealmBackedResourceStore(beatmapInfo.BeatmapSet, storage), resources, beatmapInfo.Path) + public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null) + : base(createSkinInfo(beatmapInfo), storage ?? new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files), resources, beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 2a3e51b4f5..c13504c3de 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -206,7 +206,7 @@ namespace osu.Game.Tests.Beatmaps this.resources = resources; } - protected internal override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resourceStore, resources); + protected internal override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resources, resourceStore); } } } diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 5c522058d9..fee650ded0 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -112,7 +112,7 @@ namespace osu.Game.Tests.Beatmaps public static readonly Color4 HYPER_DASH_FRUIT_COLOUR = Color4.DarkGoldenrod; public TestBeatmapSkin(BeatmapInfo beatmapInfo, bool hasColours) - : base(beatmapInfo, new ResourceStore(), null) + : base(beatmapInfo, null, new ResourceStore()) { if (hasColours) { From e1236e07add705b02c145518169152e7c4e2fcd9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 13:36:33 +0900 Subject: [PATCH 15/58] Fix extensions not being specified in time for realm file caching --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 5 +++-- osu.Game/Skinning/RealmBackedResourceStore.cs | 9 +++++++- osu.Game/Skinning/Skin.cs | 21 ++++++++----------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 9b3548912c..d2428b0ccf 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -21,8 +21,9 @@ namespace osu.Game.Skinning protected override bool AllowManiaSkin => false; protected override bool UseCustomSampleBanks => true; - public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null) - : base(createSkinInfo(beatmapInfo), storage ?? new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files), resources, beatmapInfo.Path) + public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null) + : base(createSkinInfo(beatmapInfo), storage ?? (resources != null ? new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }) : null), resources, + beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; diff --git a/osu.Game/Skinning/RealmBackedResourceStore.cs b/osu.Game/Skinning/RealmBackedResourceStore.cs index 93ffbe4f44..fc9036727f 100644 --- a/osu.Game/Skinning/RealmBackedResourceStore.cs +++ b/osu.Game/Skinning/RealmBackedResourceStore.cs @@ -13,9 +13,16 @@ namespace osu.Game.Skinning { private readonly Dictionary fileToStoragePathMapping = new Dictionary(); - public RealmBackedResourceStore(IHasRealmFiles source, IResourceStore underlyingStore) + public RealmBackedResourceStore(IHasRealmFiles source, IResourceStore underlyingStore, string[] extensions = null) : base(underlyingStore) { + // Must be initialised before the file cache. + if (extensions != null) + { + foreach (string extension in extensions) + AddExtension(extension); + } + initialiseFileCache(source); } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index a47da70cd6..654c4e75f7 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -60,25 +60,22 @@ namespace osu.Game.Skinning { SkinInfo = skin.ToLive(resources.RealmAccess); - if (storage == null) - { - var realmStorage = new RealmBackedResourceStore(skin, resources.Files); - realmStorage.AddExtension("ogg"); - - storage = realmStorage; - } + storage ??= new RealmBackedResourceStore(skin, resources.Files, new[] { @"ogg" }); } else { SkinInfo = skin.ToLiveUnmanaged(); } - var samples = resources.AudioManager?.GetSampleStore(storage); - if (samples != null) - samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; + if (storage != null) + { + var samples = resources.AudioManager?.GetSampleStore(storage); + if (samples != null) + samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - Samples = samples; - Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); + Samples = samples; + Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); + } } configurationStream ??= storage?.GetStream(@"skin.ini"); From 32e2cfb8ee710feabc140f1920c6b4e4c7150062 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 13:38:32 +0900 Subject: [PATCH 16/58] Leave realm resource store construction to base class --- osu.Game/Skinning/LegacySkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 84844c4502..238cda1151 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -45,7 +45,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, new RealmBackedResourceStore(skin, resources.Files), resources, "skin.ini") + : this(skin, null, resources, "skin.ini") { } From 3c38b142281e2bb90023be88b96ce0ac38a8b745 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 13:14:56 +0900 Subject: [PATCH 17/58] Documentation improvements --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 23 +++++++++++++++++++++-- osu.Game/Skinning/LegacySkin.cs | 5 +++-- osu.Game/Skinning/Skin.cs | 7 +++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index d2428b0ccf..7b8b659473 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -9,6 +9,7 @@ using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; +using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; @@ -21,14 +22,28 @@ namespace osu.Game.Skinning protected override bool AllowManiaSkin => false; protected override bool UseCustomSampleBanks => true; + /// + /// Construct a new legacy beatmap skin instance. + /// + /// The model for this beatmap. + /// Access to raw game resources. + /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null) - : base(createSkinInfo(beatmapInfo), storage ?? (resources != null ? new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }) : null), resources, + : base(createSkinInfo(beatmapInfo), storage ?? createRealmBackedStore(beatmapInfo, resources), resources, beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; } + private static IResourceStore createRealmBackedStore(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) + { + if (resources == null) + return null; + + return new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }); + } + public override Drawable GetDrawableComponent(ISkinComponent component) { if (component is SkinnableTargetComponent targetComponent) @@ -79,6 +94,10 @@ namespace osu.Game.Skinning } private static SkinInfo createSkinInfo(BeatmapInfo beatmapInfo) => - new SkinInfo { Name = beatmapInfo.ToString(), Creator = beatmapInfo.Metadata.Author.Username ?? string.Empty }; + new SkinInfo + { + Name = beatmapInfo.ToString(), + Creator = beatmapInfo.Metadata.Author.Username ?? string.Empty + }; } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 238cda1151..1559b9af53 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -15,6 +15,7 @@ using osu.Framework.Graphics.Textures; using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps.Formats; +using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Scoring; @@ -53,7 +54,7 @@ namespace osu.Game.Skinning /// Construct a new legacy skin instance. /// /// The model for this skin. - /// A storage for looking up files within this skin using user-facing filenames. + /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// Access to raw game resources. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) @@ -65,7 +66,7 @@ namespace osu.Game.Skinning /// Construct a new legacy skin instance. /// /// The model for this skin. - /// A storage for looking up files within this skin using user-facing filenames. + /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// Access to raw game resources. /// An optional stream containing the contents of a skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 654c4e75f7..84041deb1a 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -52,6 +52,13 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(TLookup lookup); + /// + /// Construct a new skin. + /// + /// The skin's metadata. Usually a live realm object. + /// Access to game-wide resources. + /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. + /// An optional stream which will be used to read the skin configuration. If null, the configuration will be retrieved from the storage using "skin.ini". protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] Stream configurationStream = null) { if (resources != null) From 00aea9bef02f559210bef8ef898a07191c03f96e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 13:46:19 +0900 Subject: [PATCH 18/58] Only use legacy resources lookup for protected (aka default) skin --- osu.Game/Skinning/DefaultLegacySkin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 3a8464879a..7d7d2db7f3 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -30,7 +30,7 @@ namespace osu.Game.Skinning public DefaultLegacySkin(SkinInfo skin, IStorageResourceProvider resources) : base( skin, - new NamespacedResourceStore(resources.Resources, "Skins/Legacy"), + skin.Protected ? new NamespacedResourceStore(resources.Resources, "Skins/Legacy") : null, resources, // A default legacy skin may still have a skin.ini if it is modified by the user. // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files. From a5acd38fd5b68a9e137a1852aa3683b94a60bd12 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:14:55 +0900 Subject: [PATCH 19/58] Fix `HitObjectSampleTest` adding null files to realm models --- osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index c13504c3de..630fbda688 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -96,12 +96,14 @@ namespace osu.Game.Tests.Beatmaps AddStep("setup skins", () => { userSkinInfo.Files.Clear(); - userSkinInfo.Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = userFile }, userFile)); + if (!string.IsNullOrEmpty(userFile)) + userSkinInfo.Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = userFile }, userFile)); Debug.Assert(beatmapInfo.BeatmapSet != null); beatmapInfo.BeatmapSet.Files.Clear(); - beatmapInfo.BeatmapSet.Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = beatmapFile }, beatmapFile)); + if (!string.IsNullOrEmpty(beatmapFile)) + beatmapInfo.BeatmapSet.Files.Add(new RealmNamedFileUsage(new RealmFile { Hash = beatmapFile }, beatmapFile)); // Need to refresh the cached skin source to refresh the skin resource store. dependencies.SkinSource = new SkinProvidingContainer(Skin = new LegacySkin(userSkinInfo, this)); From e56d13d8be0ac9774aecf477cb43c24987295dd0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:21:35 +0900 Subject: [PATCH 20/58] Fix realm backed store not being initialised for some tests --- osu.Game/Skinning/Skin.cs | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 84041deb1a..4c2b58bbc2 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -63,26 +63,18 @@ namespace osu.Game.Skinning { if (resources != null) { - if (resources.RealmAccess != null) - { - SkinInfo = skin.ToLive(resources.RealmAccess); + SkinInfo = resources.RealmAccess != null + ? skin.ToLive(resources.RealmAccess) + : skin.ToLiveUnmanaged(); - storage ??= new RealmBackedResourceStore(skin, resources.Files, new[] { @"ogg" }); - } - else - { - SkinInfo = skin.ToLiveUnmanaged(); - } + storage ??= new RealmBackedResourceStore(skin, resources.Files, new[] { @"ogg" }); - if (storage != null) - { - var samples = resources.AudioManager?.GetSampleStore(storage); - if (samples != null) - samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; + var samples = resources.AudioManager?.GetSampleStore(storage); + if (samples != null) + samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY; - Samples = samples; - Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); - } + Samples = samples; + Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); } configurationStream ??= storage?.GetStream(@"skin.ini"); From b48aa1d8fa934eaedb0c03f65e630d3217c94826 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:23:07 +0900 Subject: [PATCH 21/58] Ensure `HitObjectSampleTest`'s `TestWorkingBeatmap` provides the marking resource store correctly --- .../Tests/Beatmaps/HitObjectSampleTest.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs index 630fbda688..4667a385b3 100644 --- a/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs +++ b/osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs @@ -193,22 +193,32 @@ namespace osu.Game.Tests.Beatmaps } } - private class TestWorkingBeatmap : ClockBackedTestWorkingBeatmap + private class TestWorkingBeatmap : ClockBackedTestWorkingBeatmap, IStorageResourceProvider { private readonly BeatmapInfo skinBeatmapInfo; - private readonly IResourceStore resourceStore; private readonly IStorageResourceProvider resources; - public TestWorkingBeatmap(BeatmapInfo skinBeatmapInfo, IResourceStore resourceStore, IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock referenceClock, IStorageResourceProvider resources) + public TestWorkingBeatmap(BeatmapInfo skinBeatmapInfo, IResourceStore accessMarkingResourceStore, IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock referenceClock, + IStorageResourceProvider resources) : base(beatmap, storyboard, referenceClock, resources.AudioManager) { this.skinBeatmapInfo = skinBeatmapInfo; - this.resourceStore = resourceStore; + Files = accessMarkingResourceStore; this.resources = resources; } - protected internal override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resources, resourceStore); + protected internal override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, this); + + public AudioManager AudioManager => resources.AudioManager; + + public IResourceStore Files { get; } + + public IResourceStore Resources => resources.Resources; + + public RealmAccess RealmAccess => resources.RealmAccess; + + public IResourceStore CreateTextureLoaderStore(IResourceStore underlyingStore) => resources.CreateTextureLoaderStore(underlyingStore); } } } From 6c405f1dee7242d5fc7dbdff300658d4693e3221 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:31:24 +0900 Subject: [PATCH 22/58] Remove `storage` override from `LegacyBeatmapSkin` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 10 ++++------ osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 7b8b659473..6dc68b23d3 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -9,7 +9,6 @@ using osu.Framework.IO.Stores; using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; -using osu.Game.Database; using osu.Game.IO; using osu.Game.Rulesets.Objects.Legacy; using osu.Game.Rulesets.Objects.Types; @@ -27,10 +26,8 @@ namespace osu.Game.Skinning /// /// The model for this beatmap. /// Access to raw game resources. - /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. - public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null) - : base(createSkinInfo(beatmapInfo), storage ?? createRealmBackedStore(beatmapInfo, resources), resources, - beatmapInfo.Path) + public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) + : base(createSkinInfo(beatmapInfo), createRealmBackedStore(beatmapInfo, resources), resources, beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; @@ -39,7 +36,8 @@ namespace osu.Game.Skinning private static IResourceStore createRealmBackedStore(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) { if (resources == null) - return null; + // should only ever be used in tests. + return new ResourceStore(); return new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }); } diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index fee650ded0..b2b348d869 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -112,7 +112,7 @@ namespace osu.Game.Tests.Beatmaps public static readonly Color4 HYPER_DASH_FRUIT_COLOUR = Color4.DarkGoldenrod; public TestBeatmapSkin(BeatmapInfo beatmapInfo, bool hasColours) - : base(beatmapInfo, null, new ResourceStore()) + : base(beatmapInfo, null) { if (hasColours) { From 9d3c6ade6237c347bef8ad060ea755f2f2cf9d15 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:38:40 +0900 Subject: [PATCH 23/58] Remove unnecessary skin reading hack in `DefaultLegacySkin` --- osu.Game/Skinning/DefaultLegacySkin.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 7d7d2db7f3..e7d0ed793f 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -30,11 +30,10 @@ namespace osu.Game.Skinning public DefaultLegacySkin(SkinInfo skin, IStorageResourceProvider resources) : base( skin, + // In the case of the actual default legacy skin (ie. the fallback one, which a user hasn't applied any modifications to) we want to use the game provided resources. skin.Protected ? new NamespacedResourceStore(resources.Resources, "Skins/Legacy") : null, resources, - // A default legacy skin may still have a skin.ini if it is modified by the user. - // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files. - new RealmBackedResourceStore(skin, resources.Files).GetStream("skin.ini") + "skin.ini" ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); From d0ea1739b4767427c6ccd1b8de425a5fa4059be3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 14:53:00 +0900 Subject: [PATCH 24/58] Remove skin configuration stream logic as it is no longer required --- osu.Game/Skinning/LegacySkin.cs | 14 +------------- osu.Game/Skinning/Skin.cs | 6 +++--- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 1559b9af53..6752851ca6 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -58,19 +58,7 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) - : this(skin, storage, resources, string.IsNullOrEmpty(configurationFilename) ? null : storage?.GetStream(configurationFilename)) - { - } - - /// - /// Construct a new legacy skin instance. - /// - /// The model for this skin. - /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. - /// Access to raw game resources. - /// An optional stream containing the contents of a skin.ini file. - protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) - : base(skin, resources, storage, configurationStream) + : base(skin, resources, storage, configurationFilename) { // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. hasKeyTexture = new Lazy(() => this.GetAnimation( diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 4c2b58bbc2..72bbca4903 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -58,8 +58,8 @@ namespace osu.Game.Skinning /// The skin's metadata. Usually a live realm object. /// Access to game-wide resources. /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. - /// An optional stream which will be used to read the skin configuration. If null, the configuration will be retrieved from the storage using "skin.ini". - protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] Stream configurationStream = null) + /// An optional filename to read the skin configuration from. If not provided, the configuration will be retrieved from the storage using "skin.ini". + protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] string configurationFilename = "skin.ini") { if (resources != null) { @@ -77,7 +77,7 @@ namespace osu.Game.Skinning Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); } - configurationStream ??= storage?.GetStream(@"skin.ini"); + var configurationStream = storage?.GetStream(configurationFilename); if (configurationStream != null) // stream will be closed after use by LineBufferedReader. From 7a1909bf970acd9f5a2ebcce6387a644b0ec3002 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 15:02:01 +0900 Subject: [PATCH 25/58] Change parameter order of `LegacySkin` to put `IStorageResourceProvider` first The optional resource store should not be before the (basically) required resource provider. --- .../CatchSkinColourDecodingTest.cs | 2 +- osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 2 +- osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs | 2 +- osu.Game/Skinning/DefaultLegacySkin.cs | 2 +- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 6 +++--- osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs | 2 +- osu.Game/Tests/Visual/SkinnableTestScene.cs | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs b/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs index e70def7f8b..ea3c0f3f4a 100644 --- a/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs +++ b/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs @@ -30,7 +30,7 @@ namespace osu.Game.Rulesets.Catch.Tests { public TestLegacySkin(SkinInfo skin, IResourceStore storage) // Bypass LegacySkinResourceStore to avoid returning null for retrieving files due to bad skin info (SkinInfo.Files = null). - : base(skin, storage, null, "skin.ini") + : base(skin, null, storage, "skin.ini") { } } diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index d19b3c71f1..0d436c1ef7 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -175,7 +175,7 @@ namespace osu.Game.Tests.Beatmaps.Formats private class TestLegacySkin : LegacySkin { public TestLegacySkin(IResourceStore storage, string fileName) - : base(new SkinInfo { Name = "Test Skin", Creator = "Craftplacer" }, storage, null, fileName) + : base(new SkinInfo { Name = "Test Skin", Creator = "Craftplacer" }, null, storage, fileName) { } } diff --git a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs index 6457a23a1b..cfb158cc5a 100644 --- a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs @@ -148,7 +148,7 @@ namespace osu.Game.Tests.Gameplay private class TestSkin : LegacySkin { public TestSkin(string resourceName, IStorageResourceProvider resources) - : base(DefaultLegacySkin.CreateInfo(), new TestResourceStore(resourceName), resources, "skin.ini") + : base(DefaultLegacySkin.CreateInfo(), resources, new TestResourceStore(resourceName), "skin.ini") { } } diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index e7d0ed793f..be1b7b8f25 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -30,9 +30,9 @@ namespace osu.Game.Skinning public DefaultLegacySkin(SkinInfo skin, IStorageResourceProvider resources) : base( skin, + resources, // In the case of the actual default legacy skin (ie. the fallback one, which a user hasn't applied any modifications to) we want to use the game provided resources. skin.Protected ? new NamespacedResourceStore(resources.Resources, "Skins/Legacy") : null, - resources, "skin.ini" ) { diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 6dc68b23d3..e3685c986b 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -27,7 +27,7 @@ namespace osu.Game.Skinning /// The model for this beatmap. /// Access to raw game resources. public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) - : base(createSkinInfo(beatmapInfo), createRealmBackedStore(beatmapInfo, resources), resources, beatmapInfo.Path) + : base(createSkinInfo(beatmapInfo), resources, createRealmBackedStore(beatmapInfo, resources), beatmapInfo.Path) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 6752851ca6..fa581f98e0 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -46,7 +46,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, null, resources, "skin.ini") + : this(skin, resources, null, "skin.ini") { } @@ -54,10 +54,10 @@ namespace osu.Game.Skinning /// Construct a new legacy skin instance. /// /// The model for this skin. - /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// Access to raw game resources. + /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. - protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) + protected LegacySkin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage, string configurationFilename) : base(skin, resources, storage, configurationFilename) { // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index b2b348d869..531c38b655 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -141,7 +141,7 @@ namespace osu.Game.Tests.Beatmaps public static readonly Color4 HYPER_DASH_FRUIT_COLOUR = Color4.LightCyan; public TestSkin(bool hasCustomColours) - : base(new SkinInfo(), new ResourceStore(), null, string.Empty) + : base(new SkinInfo(), null, new ResourceStore(), string.Empty) { if (hasCustomColours) { diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index 1107089a46..c8fc988ffd 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -187,7 +187,7 @@ namespace osu.Game.Tests.Visual private readonly bool extrapolateAnimations; public TestLegacySkin(SkinInfo skin, IResourceStore storage, IStorageResourceProvider resources, bool extrapolateAnimations) - : base(skin, storage, resources, "skin.ini") + : base(skin, resources, storage, "skin.ini") { this.extrapolateAnimations = extrapolateAnimations; } From 078288a616319eed8e5a9a3d653a22d92ae3c88a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 15:05:01 +0900 Subject: [PATCH 26/58] Make "skin.ini" the default skin filename and remove redundant parameters --- osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs | 2 +- osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs | 2 +- osu.Game/Skinning/DefaultLegacySkin.cs | 3 +-- osu.Game/Skinning/LegacySkin.cs | 4 ++-- osu.Game/Skinning/Skin.cs | 2 +- osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs | 3 +-- osu.Game/Tests/Visual/SkinnableTestScene.cs | 2 +- 7 files changed, 8 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs b/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs index ea3c0f3f4a..bb3a724b91 100644 --- a/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs +++ b/osu.Game.Rulesets.Catch.Tests/CatchSkinColourDecodingTest.cs @@ -30,7 +30,7 @@ namespace osu.Game.Rulesets.Catch.Tests { public TestLegacySkin(SkinInfo skin, IResourceStore storage) // Bypass LegacySkinResourceStore to avoid returning null for retrieving files due to bad skin info (SkinInfo.Files = null). - : base(skin, null, storage, "skin.ini") + : base(skin, null, storage) { } } diff --git a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs index cfb158cc5a..76ec35d87d 100644 --- a/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs +++ b/osu.Game.Tests/Gameplay/TestSceneStoryboardSamples.cs @@ -148,7 +148,7 @@ namespace osu.Game.Tests.Gameplay private class TestSkin : LegacySkin { public TestSkin(string resourceName, IStorageResourceProvider resources) - : base(DefaultLegacySkin.CreateInfo(), resources, new TestResourceStore(resourceName), "skin.ini") + : base(DefaultLegacySkin.CreateInfo(), resources, new TestResourceStore(resourceName)) { } } diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index be1b7b8f25..f7b415e886 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/osu.Game/Skinning/DefaultLegacySkin.cs @@ -32,8 +32,7 @@ namespace osu.Game.Skinning skin, resources, // In the case of the actual default legacy skin (ie. the fallback one, which a user hasn't applied any modifications to) we want to use the game provided resources. - skin.Protected ? new NamespacedResourceStore(resources.Resources, "Skins/Legacy") : null, - "skin.ini" + skin.Protected ? new NamespacedResourceStore(resources.Resources, "Skins/Legacy") : null ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index fa581f98e0..1c2ca797c6 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -46,7 +46,7 @@ namespace osu.Game.Skinning [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] public LegacySkin(SkinInfo skin, IStorageResourceProvider resources) - : this(skin, resources, null, "skin.ini") + : this(skin, resources, null) { } @@ -57,7 +57,7 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. - protected LegacySkin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage, string configurationFilename) + protected LegacySkin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage, string configurationFilename = @"skin.ini") : base(skin, resources, storage, configurationFilename) { // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 72bbca4903..e28c35937d 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -59,7 +59,7 @@ namespace osu.Game.Skinning /// Access to game-wide resources. /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// An optional filename to read the skin configuration from. If not provided, the configuration will be retrieved from the storage using "skin.ini". - protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] string configurationFilename = "skin.ini") + protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] string configurationFilename = @"skin.ini") { if (resources != null) { diff --git a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs index 531c38b655..597c5e9a2b 100644 --- a/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs +++ b/osu.Game/Tests/Beatmaps/LegacyBeatmapSkinColourTest.cs @@ -7,7 +7,6 @@ using System.Linq; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; -using osu.Framework.IO.Stores; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Skinning; @@ -141,7 +140,7 @@ namespace osu.Game.Tests.Beatmaps public static readonly Color4 HYPER_DASH_FRUIT_COLOUR = Color4.LightCyan; public TestSkin(bool hasCustomColours) - : base(new SkinInfo(), null, new ResourceStore(), string.Empty) + : base(new SkinInfo(), null, null) { if (hasCustomColours) { diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index c8fc988ffd..b4da91a97a 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -187,7 +187,7 @@ namespace osu.Game.Tests.Visual private readonly bool extrapolateAnimations; public TestLegacySkin(SkinInfo skin, IResourceStore storage, IStorageResourceProvider resources, bool extrapolateAnimations) - : base(skin, resources, storage, "skin.ini") + : base(skin, resources, storage) { this.extrapolateAnimations = extrapolateAnimations; } From 05c7e09d79fa1f667c8bdfe7472eaa3a7677fa2f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Mar 2022 23:57:14 +0900 Subject: [PATCH 27/58] Make `Textures` and `Samples` read-only --- .../Skinning/LegacySkinTextureFallbackTest.cs | 54 +++++++++++++++---- osu.Game/Skinning/Skin.cs | 4 +- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs b/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs index 69e66942ab..7516e7500b 100644 --- a/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs +++ b/osu.Game.Tests/NonVisual/Skinning/LegacySkinTextureFallbackTest.cs @@ -1,12 +1,21 @@ // 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; +using System.Threading.Tasks; using NUnit.Framework; -using osu.Framework.Graphics.OpenGL.Textures; +using osu.Framework.Audio; using osu.Framework.Graphics.Textures; +using osu.Framework.IO.Stores; +using osu.Game.Database; +using osu.Game.IO; using osu.Game.Skinning; +using SixLabors.ImageSharp; +using SixLabors.ImageSharp.PixelFormats; namespace osu.Game.Tests.NonVisual.Skinning { @@ -71,7 +80,7 @@ namespace osu.Game.Tests.NonVisual.Skinning var texture = legacySkin.GetTexture(requestedComponent); Assert.IsNotNull(texture); - Assert.AreEqual(textureStore.Textures[expectedTexture], texture); + Assert.AreEqual(textureStore.Textures[expectedTexture].Width, texture.Width); Assert.AreEqual(expectedScale, texture.ScaleAdjust); } @@ -88,23 +97,50 @@ namespace osu.Game.Tests.NonVisual.Skinning private class TestLegacySkin : LegacySkin { - public TestLegacySkin(TextureStore textureStore) - : base(new SkinInfo(), null, null, string.Empty) + public TestLegacySkin(IResourceStore textureStore) + : base(new SkinInfo(), new TestResourceProvider(textureStore), null, string.Empty) { - Textures = textureStore; + } + + private class TestResourceProvider : IStorageResourceProvider + { + private readonly IResourceStore textureStore; + + public TestResourceProvider(IResourceStore textureStore) + { + this.textureStore = textureStore; + } + + public AudioManager AudioManager => null; + public IResourceStore Files => null; + public IResourceStore Resources => null; + public RealmAccess RealmAccess => null; + public IResourceStore CreateTextureLoaderStore(IResourceStore underlyingStore) => textureStore; } } - private class TestTextureStore : TextureStore + private class TestTextureStore : IResourceStore { - public readonly Dictionary Textures; + public readonly Dictionary Textures; public TestTextureStore(params string[] fileNames) { - Textures = fileNames.ToDictionary(fileName => fileName, fileName => new Texture(1, 1)); + // use an incrementing width to allow assertion matching on correct textures as they turn from uploads into actual textures. + int width = 1; + Textures = fileNames.ToDictionary(fileName => fileName, fileName => new TextureUpload(new Image(width, width++))); } - public override Texture Get(string name, WrapMode wrapModeS, WrapMode wrapModeT) => Textures.GetValueOrDefault(name); + public TextureUpload Get(string name) => Textures.GetValueOrDefault(name); + + public Task GetAsync(string name, CancellationToken cancellationToken = new CancellationToken()) => Task.FromResult(Get(name)); + + public Stream GetStream(string name) => throw new NotImplementedException(); + + public IEnumerable GetAvailableResources() => throw new NotImplementedException(); + + public void Dispose() + { + } } } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e28c35937d..b6f46a0d81 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -28,13 +28,13 @@ namespace osu.Game.Skinning /// A texture store which can be used to perform user file loops for this skin. /// [CanBeNull] - protected TextureStore Textures { get; set; } + protected TextureStore Textures { get; } /// /// A sample store which can be used to perform user file loops for this skin. /// [CanBeNull] - protected ISampleStore Samples { get; set; } + protected ISampleStore Samples { get; } public readonly Live SkinInfo; From 3e020073c50d325f9c19ded1515d0653e19f5fb6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 00:08:01 +0900 Subject: [PATCH 28/58] Convert `Skin` to use `nullable` --- osu.Game/Skinning/ISkin.cs | 18 +++++++----------- osu.Game/Skinning/Skin.cs | 36 +++++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 73f7cf6d39..4b14dcfd62 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -1,7 +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 JetBrains.Annotations; +#nullable enable + using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -21,16 +22,14 @@ namespace osu.Game.Skinning /// /// The requested component. /// A drawable representation for the requested component, or null if unavailable. - [CanBeNull] - Drawable GetDrawableComponent(ISkinComponent component); + Drawable? GetDrawableComponent(ISkinComponent component); /// /// Retrieve a . /// /// The requested texture. /// A matching texture, or null if unavailable. - [CanBeNull] - Texture GetTexture(string componentName) => GetTexture(componentName, default, default); + Texture? GetTexture(string componentName) => GetTexture(componentName, default, default); /// /// Retrieve a . @@ -39,23 +38,20 @@ namespace osu.Game.Skinning /// The texture wrap mode in horizontal direction. /// The texture wrap mode in vertical direction. /// A matching texture, or null if unavailable. - [CanBeNull] - Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT); + Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT); /// /// Retrieve a . /// /// The requested sample. /// A matching sample channel, or null if unavailable. - [CanBeNull] - ISample GetSample(ISampleInfo sampleInfo); + ISample? GetSample(ISampleInfo sampleInfo); /// /// Retrieve a configuration value. /// /// The requested configuration value. /// A matching value boxed in an , or null if unavailable. - [CanBeNull] - IBindable GetConfig(TLookup lookup); + IBindable? GetConfig(TLookup lookup); } } diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index b6f46a0d81..643e0c38d4 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -1,12 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Text; -using JetBrains.Annotations; using Newtonsoft.Json; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; @@ -27,14 +29,12 @@ namespace osu.Game.Skinning /// /// A texture store which can be used to perform user file loops for this skin. /// - [CanBeNull] - protected TextureStore Textures { get; } + protected TextureStore? Textures { get; } /// /// A sample store which can be used to perform user file loops for this skin. /// - [CanBeNull] - protected ISampleStore Samples { get; } + protected ISampleStore? Samples { get; } public readonly Live SkinInfo; @@ -44,13 +44,13 @@ namespace osu.Game.Skinning private readonly Dictionary drawableComponentInfo = new Dictionary(); - public abstract ISample GetSample(ISampleInfo sampleInfo); + public abstract ISample? GetSample(ISampleInfo sampleInfo); - public Texture GetTexture(string componentName) => GetTexture(componentName, default, default); + public Texture? GetTexture(string componentName) => GetTexture(componentName, default, default); - public abstract Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT); + public abstract Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT); - public abstract IBindable GetConfig(TLookup lookup); + public abstract IBindable? GetConfig(TLookup lookup); /// /// Construct a new skin. @@ -59,13 +59,11 @@ namespace osu.Game.Skinning /// Access to game-wide resources. /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// An optional filename to read the skin configuration from. If not provided, the configuration will be retrieved from the storage using "skin.ini". - protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] string configurationFilename = @"skin.ini") + protected Skin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStore? storage = null, string configurationFilename = @"skin.ini") { if (resources != null) { - SkinInfo = resources.RealmAccess != null - ? skin.ToLive(resources.RealmAccess) - : skin.ToLiveUnmanaged(); + SkinInfo = skin.ToLive(resources.RealmAccess); storage ??= new RealmBackedResourceStore(skin, resources.Files, new[] { @"ogg" }); @@ -76,12 +74,20 @@ namespace osu.Game.Skinning Samples = samples; Textures = new TextureStore(resources.CreateTextureLoaderStore(storage)); } + else + { + // Generally only used for tests. + SkinInfo = skin.ToLiveUnmanaged(); + } var configurationStream = storage?.GetStream(configurationFilename); if (configurationStream != null) + { // stream will be closed after use by LineBufferedReader. ParseConfigurationStream(configurationStream); + Debug.Assert(Configuration != null); + } else Configuration = new SkinConfiguration(); @@ -90,7 +96,7 @@ namespace osu.Game.Skinning { string filename = $"{skinnableTarget}.json"; - byte[] bytes = storage?.Get(filename); + byte[]? bytes = storage?.Get(filename); if (bytes == null) continue; @@ -136,7 +142,7 @@ namespace osu.Game.Skinning DrawableComponentInfo[targetContainer.Target] = targetContainer.CreateSkinnableInfo().ToArray(); } - public virtual Drawable GetDrawableComponent(ISkinComponent component) + public virtual Drawable? GetDrawableComponent(ISkinComponent component) { switch (component) { From 194bf4fb057e60eff9781c2d85896e3b79075202 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 00:21:19 +0900 Subject: [PATCH 29/58] Convert `LegacySkin` to use `nullable` --- osu.Game/Skinning/LegacySkin.cs | 35 ++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 1c2ca797c6..20cac52d21 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Collections.Generic; using System.Diagnostics; @@ -57,7 +59,7 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. - protected LegacySkin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage, string configurationFilename = @"skin.ini") + protected LegacySkin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStore? storage, string configurationFilename = @"skin.ini") : base(skin, resources, storage, configurationFilename) { // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. @@ -81,7 +83,7 @@ namespace osu.Game.Skinning } } - public override IBindable GetConfig(TLookup lookup) + public override IBindable? GetConfig(TLookup lookup) { switch (lookup) { @@ -127,7 +129,7 @@ namespace osu.Game.Skinning return null; } - private IBindable lookupForMania(LegacyManiaSkinConfigurationLookup maniaLookup) + private IBindable? lookupForMania(LegacyManiaSkinConfigurationLookup maniaLookup) { if (!maniaConfigurations.TryGetValue(maniaLookup.Keys, out var existing)) maniaConfigurations[maniaLookup.Keys] = existing = new LegacyManiaSkinConfiguration(maniaLookup.Keys); @@ -267,20 +269,19 @@ namespace osu.Game.Skinning /// The source to retrieve the combo colours from. /// The preferred index for retrieving the combo colour with. /// Information on the combo whose using the returned colour. - protected virtual IBindable GetComboColour(IHasComboColours source, int colourIndex, IHasComboInformation combo) + protected virtual IBindable? GetComboColour(IHasComboColours source, int colourIndex, IHasComboInformation combo) { var colour = source.ComboColours?[colourIndex % source.ComboColours.Count]; return colour.HasValue ? new Bindable(colour.Value) : null; } - private IBindable getCustomColour(IHasCustomColours source, string lookup) + private IBindable? getCustomColour(IHasCustomColours source, string lookup) => source.CustomColours.TryGetValue(lookup, out var col) ? new Bindable(col) : null; - private IBindable getManiaImage(LegacyManiaSkinConfiguration source, string lookup) + private IBindable? getManiaImage(LegacyManiaSkinConfiguration source, string lookup) => source.ImageLookups.TryGetValue(lookup, out string image) ? new Bindable(image) : null; - [CanBeNull] - private IBindable legacySettingLookup(SkinConfiguration.LegacySetting legacySetting) + private IBindable? legacySettingLookup(SkinConfiguration.LegacySetting legacySetting) { switch (legacySetting) { @@ -292,9 +293,11 @@ namespace osu.Game.Skinning } } - [CanBeNull] - private IBindable genericLookup(TLookup lookup) + private IBindable? genericLookup(TLookup lookup) { + if (lookup == null) + return null; + try { if (Configuration.ConfigDictionary.TryGetValue(lookup.ToString(), out string val)) @@ -316,7 +319,7 @@ namespace osu.Game.Skinning return null; } - public override Drawable GetDrawableComponent(ISkinComponent component) + public override Drawable? GetDrawableComponent(ISkinComponent component) { if (base.GetDrawableComponent(component) is Drawable c) return c; @@ -385,7 +388,7 @@ namespace osu.Game.Skinning case GameplaySkinComponent resultComponent: // TODO: this should be inside the judgement pieces. - Func createDrawable = () => getJudgementAnimation(resultComponent.Component); + Func createDrawable = () => getJudgementAnimation(resultComponent.Component); // kind of wasteful that we throw this away, but should do for now. if (createDrawable() != null) @@ -404,7 +407,7 @@ namespace osu.Game.Skinning return this.GetAnimation(component.LookupName, false, false); } - private Texture getParticleTexture(HitResult result) + private Texture? getParticleTexture(HitResult result) { switch (result) { @@ -421,7 +424,7 @@ namespace osu.Game.Skinning return null; } - private Drawable getJudgementAnimation(HitResult result) + private Drawable? getJudgementAnimation(HitResult result) { switch (result) { @@ -441,7 +444,7 @@ namespace osu.Game.Skinning return null; } - public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) + public override Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) { foreach (string name in getFallbackNames(componentName)) { @@ -469,7 +472,7 @@ namespace osu.Game.Skinning return null; } - public override ISample GetSample(ISampleInfo sampleInfo) + public override ISample? GetSample(ISampleInfo sampleInfo) { IEnumerable lookupNames; From 7296bad294c93d7e088f52a103a0f395dae1581b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 00:24:06 +0900 Subject: [PATCH 30/58] Convert `LegacyBeatmapSkin` to use `nullable` --- osu.Game/Skinning/LegacyBeatmapSkin.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index e3685c986b..16a05f4197 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -1,9 +1,11 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using JetBrains.Annotations; +#nullable enable + using osu.Framework.Audio.Sample; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.IO.Stores; using osu.Game.Audio; @@ -26,14 +28,14 @@ namespace osu.Game.Skinning /// /// The model for this beatmap. /// Access to raw game resources. - public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) - : base(createSkinInfo(beatmapInfo), resources, createRealmBackedStore(beatmapInfo, resources), beatmapInfo.Path) + public LegacyBeatmapSkin(BeatmapInfo beatmapInfo, IStorageResourceProvider? resources) + : base(createSkinInfo(beatmapInfo), resources, createRealmBackedStore(beatmapInfo, resources), beatmapInfo.Path.AsNonNull()) { // Disallow default colours fallback on beatmap skins to allow using parent skin combo colours. (via SkinProvidingContainer) Configuration.AllowDefaultComboColoursFallback = false; } - private static IResourceStore createRealmBackedStore(BeatmapInfo beatmapInfo, [CanBeNull] IStorageResourceProvider resources) + private static IResourceStore createRealmBackedStore(BeatmapInfo beatmapInfo, IStorageResourceProvider? resources) { if (resources == null) // should only ever be used in tests. @@ -42,7 +44,7 @@ namespace osu.Game.Skinning return new RealmBackedResourceStore(beatmapInfo.BeatmapSet, resources.Files, new[] { @"ogg" }); } - public override Drawable GetDrawableComponent(ISkinComponent component) + public override Drawable? GetDrawableComponent(ISkinComponent component) { if (component is SkinnableTargetComponent targetComponent) { @@ -61,7 +63,7 @@ namespace osu.Game.Skinning return base.GetDrawableComponent(component); } - public override IBindable GetConfig(TLookup lookup) + public override IBindable? GetConfig(TLookup lookup) { switch (lookup) { @@ -77,10 +79,10 @@ namespace osu.Game.Skinning return base.GetConfig(lookup); } - protected override IBindable GetComboColour(IHasComboColours source, int comboIndex, IHasComboInformation combo) + protected override IBindable? GetComboColour(IHasComboColours source, int comboIndex, IHasComboInformation combo) => base.GetComboColour(source, combo.ComboIndexWithOffsets, combo); - public override ISample GetSample(ISampleInfo sampleInfo) + public override ISample? GetSample(ISampleInfo sampleInfo) { if (sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacy && legacy.CustomSampleBank == 0) { From a4d17a915f96c505e518dd261f9b4d1d5d967b7f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 12:36:16 +0900 Subject: [PATCH 31/58] Fix incorrect HUD component fallback Legacy skins should now always show the legacy hud components. The conditional here is no longer valid as fallback lookups happen at a *skin*-fallback level rather than internal *source*-fallback. Put another way, `LegacyDefaultSkin` (with user customisations) should still display the classic HUD components even if a font is not provided, as that font will be available via the skin lookup hierarchy. The TODO removed in this commit has been already resolved so this code is no longer required. --- osu.Game/Skinning/LegacySkin.cs | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 1c2ca797c6..244774fd4c 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -356,26 +356,15 @@ namespace osu.Game.Skinning } }) { - Children = this.HasFont(LegacyFont.Score) - ? new Drawable[] - { - new LegacyComboCounter(), - new LegacyScoreCounter(), - new LegacyAccuracyCounter(), - new LegacyHealthDisplay(), - new SongProgress(), - new BarHitErrorMeter(), - } - : new Drawable[] - { - // TODO: these should fallback to using osu!classic skin textures, rather than doing this. - new DefaultComboCounter(), - new DefaultScoreCounter(), - new DefaultAccuracyCounter(), - new DefaultHealthDisplay(), - new SongProgress(), - new BarHitErrorMeter(), - } + Children = new Drawable[] + { + new LegacyComboCounter(), + new LegacyScoreCounter(), + new LegacyAccuracyCounter(), + new LegacyHealthDisplay(), + new SongProgress(), + new BarHitErrorMeter(), + } }; return skinnableTargetWrapper; From 0cd29a73b929b007c02619d96fa6b71f0834aa24 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 12:39:47 +0900 Subject: [PATCH 32/58] Fix typo in xmldocs --- osu.Game/Skinning/Skin.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index b6f46a0d81..e1fcf196f3 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -25,13 +25,13 @@ namespace osu.Game.Skinning public abstract class Skin : IDisposable, ISkin { /// - /// A texture store which can be used to perform user file loops for this skin. + /// A texture store which can be used to perform user file lookups for this skin. /// [CanBeNull] protected TextureStore Textures { get; } /// - /// A sample store which can be used to perform user file loops for this skin. + /// A sample store which can be used to perform user file lookups for this skin. /// [CanBeNull] protected ISampleStore Samples { get; } From e243a7c55d349788f85c34a1b573b88547ace35b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 12:45:11 +0900 Subject: [PATCH 33/58] Reword `storage` param xmldoc to use stronger and better defined langauge --- osu.Game/Skinning/Skin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e1fcf196f3..e00dd950a7 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -57,7 +57,7 @@ namespace osu.Game.Skinning /// /// The skin's metadata. Usually a live realm object. /// Access to game-wide resources. - /// An optional store which will be used for looking up skin resources. If null, one will be created from realm pattern. + /// An optional store which will *replace* all file lookups that are usually sourced from . /// An optional filename to read the skin configuration from. If not provided, the configuration will be retrieved from the storage using "skin.ini". protected Skin(SkinInfo skin, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] IResourceStore storage = null, [CanBeNull] string configurationFilename = @"skin.ini") { From 4d0b4c2541823d083c235c6b390a8d17d2f73ac9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 12:53:50 +0900 Subject: [PATCH 34/58] Fix realm potentially not being refreshed in time for test asserts in `BeatmapImporterTests` As seen at https://github.com/ppy/osu/runs/5659368512?check_suite_focus=true Went through every usage of `.Import` and added either an `EnsureLoaded`, or where that provides too many checks, an explicit `realm.Refresh()`. --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 9abd78039a..f9c13a8169 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -147,7 +147,10 @@ namespace osu.Game.Tests.Database Live? imported; using (var reader = new ZipArchiveReader(TestResources.GetTestBeatmapStream())) + { imported = await importer.Import(reader); + EnsureLoaded(realm.Realm); + } Assert.AreEqual(1, realm.Realm.All().Count()); @@ -510,6 +513,8 @@ namespace osu.Game.Tests.Database new ImportTask(zipStream, string.Empty) ); + realm.Run(r => r.Refresh()); + checkBeatmapSetCount(realm.Realm, 0); checkBeatmapCount(realm.Realm, 0); @@ -565,6 +570,8 @@ namespace osu.Game.Tests.Database { } + EnsureLoaded(realm.Realm); + checkBeatmapSetCount(realm.Realm, 1); checkBeatmapCount(realm.Realm, 12); @@ -726,6 +733,8 @@ namespace osu.Game.Tests.Database var imported = importer.Import(toImport); + realm.Run(r => r.Refresh()); + Assert.NotNull(imported); Debug.Assert(imported != null); @@ -891,6 +900,8 @@ namespace osu.Game.Tests.Database string? temp = TestResources.GetTestBeatmapForImport(); await importer.Import(temp); + EnsureLoaded(realm.Realm); + // Update via the beatmap, not the beatmap info, to ensure correct linking BeatmapSetInfo setToUpdate = realm.Realm.All().First(); From a7d5f2281ce59c004991070865a6c4cf640bc637 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 15:49:13 +0900 Subject: [PATCH 35/58] Apply beatmap offsets to legacy replay frame handling --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 8 +++++++- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index fefee370b9..9885fe5528 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -23,6 +23,8 @@ namespace osu.Game.Scoring.Legacy private IBeatmap currentBeatmap; private Ruleset currentRuleset; + private float beatmapOffset; + public Score Parse(Stream stream) { var score = new Score @@ -72,6 +74,10 @@ namespace osu.Game.Scoring.Legacy currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); scoreInfo.BeatmapInfo = currentBeatmap.BeatmapInfo; + // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) + // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. + beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? 24 : 0; + /* score.HpGraphString = */ sr.ReadString(); @@ -229,7 +235,7 @@ namespace osu.Game.Scoring.Legacy private void readLegacyReplay(Replay replay, StreamReader reader) { - float lastTime = 0; + float lastTime = beatmapOffset; ReplayFrame currentFrame = null; string[] frames = reader.ReadToEnd().Split(','); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index f0ead05280..6a321bed7a 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -111,6 +111,10 @@ namespace osu.Game.Scoring.Legacy { StringBuilder replayData = new StringBuilder(); + // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) + // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. + double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -24 : 0; + if (score.Replay != null) { int lastTime = 0; @@ -120,7 +124,7 @@ namespace osu.Game.Scoring.Legacy var legacyFrame = getLegacyFrame(f); // Rounding because stable could only parse integral values - int time = (int)Math.Round(legacyFrame.Time); + int time = (int)Math.Round(legacyFrame.Time + offset); replayData.Append(FormattableString.Invariant($"{time - lastTime}|{legacyFrame.MouseX ?? 0}|{legacyFrame.MouseY ?? 0}|{(int)legacyFrame.ButtonState},")); lastTime = time; } From 59a7eb532203629e8dbd6dbc8f44cbc83c03fbde Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:34:21 +0900 Subject: [PATCH 36/58] Add test coverage ensuring offsets are correct before and after legacy replay encode --- .../Formats/LegacyScoreDecoderTest.cs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 2ba8c51a10..a50cef238a 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -8,6 +8,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Formats; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; @@ -64,6 +65,55 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [TestCase(3)] + [TestCase(6)] + [TestCase(LegacyBeatmapDecoder.LATEST_VERSION)] + public void TestLegacyBeatmapReplayOffsets(int beatmapVersion) + { + const double first_frame_time = 2000; + const double second_frame_time = 3000; + + var ruleset = new OsuRuleset().RulesetInfo; + var scoreInfo = TestResources.CreateTestScoreInfo(ruleset); + var beatmap = new TestBeatmap(ruleset) + { + BeatmapInfo = + { + BeatmapVersion = beatmapVersion + } + }; + + var score = new Score + { + ScoreInfo = scoreInfo, + Replay = new Replay + { + Frames = new List + { + new OsuReplayFrame(first_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton), + new OsuReplayFrame(second_frame_time, OsuPlayfield.BASE_SIZE / 2, OsuAction.LeftButton) + } + } + }; + + // the "se" culture is used here, as it encodes the negative number sign as U+2212 MINUS SIGN, + // rather than the classic ASCII U+002D HYPHEN-MINUS. + CultureInfo.CurrentCulture = new CultureInfo("se"); + + var encodeStream = new MemoryStream(); + + var encoder = new LegacyScoreEncoder(score, beatmap); + encoder.Encode(encodeStream); + + var decodeStream = new MemoryStream(encodeStream.GetBuffer()); + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + var decodedAfterEncode = decoder.Parse(decodeStream); + + Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(first_frame_time)); + Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time)); + } + [Test] public void TestCultureInvariance() { @@ -118,6 +168,8 @@ namespace osu.Game.Tests.Beatmaps.Formats private class TestLegacyScoreDecoder : LegacyScoreDecoder { + private readonly int beatmapVersion; + private static readonly Dictionary rulesets = new Ruleset[] { new OsuRuleset(), @@ -126,6 +178,11 @@ namespace osu.Game.Tests.Beatmaps.Formats new ManiaRuleset() }.ToDictionary(ruleset => ((ILegacyRuleset)ruleset).LegacyID); + public TestLegacyScoreDecoder(int beatmapVersion = LegacyBeatmapDecoder.LATEST_VERSION) + { + this.beatmapVersion = beatmapVersion; + } + protected override Ruleset GetRuleset(int rulesetId) => rulesets[rulesetId]; protected override WorkingBeatmap GetBeatmap(string md5Hash) => new TestWorkingBeatmap(new Beatmap @@ -134,7 +191,8 @@ namespace osu.Game.Tests.Beatmaps.Formats { MD5Hash = md5Hash, Ruleset = new OsuRuleset().RulesetInfo, - Difficulty = new BeatmapDifficulty() + Difficulty = new BeatmapDifficulty(), + BeatmapVersion = beatmapVersion, } }); } From 2efae031c97a904c7876f6ee89093725b70f1bc5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:39:56 +0900 Subject: [PATCH 37/58] Add test coverage of decode specifically --- .../Formats/LegacyScoreDecoderTest.cs | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index a50cef238a..39586bcd8c 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -65,10 +65,29 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [TestCase(3, true)] + [TestCase(6, false)] + [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)] + public void TestLegacyBeatmapReplayOffsetsDecode(int beatmapVersion, bool offsetApplied) + { + const double first_frame_time = 48; + const double second_frame_time = 65; + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + + using (var resourceStream = TestResources.OpenResource("Replays/mania-replay.osr")) + { + var score = decoder.Parse(resourceStream); + + Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? 24 : 0))); + Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? 24 : 0))); + } + } + [TestCase(3)] [TestCase(6)] [TestCase(LegacyBeatmapDecoder.LATEST_VERSION)] - public void TestLegacyBeatmapReplayOffsets(int beatmapVersion) + public void TestLegacyBeatmapReplayOffsetsEncodeDecode(int beatmapVersion) { const double first_frame_time = 2000; const double second_frame_time = 3000; From a7554dcdf77637543960f712cbbcb521fc3f410b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 16:43:41 +0900 Subject: [PATCH 38/58] Use a constant for the early version timing offset --- osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs | 4 ++-- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 8 ++++++-- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 3 +-- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 8 ++++---- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 39586bcd8c..1cd910789f 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -79,8 +79,8 @@ namespace osu.Game.Tests.Beatmaps.Formats { var score = decoder.Parse(resourceStream); - Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? 24 : 0))); - Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? 24 : 0))); + Assert.That(score.Replay.Frames[0].Time, Is.EqualTo(first_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0))); + Assert.That(score.Replay.Frames[1].Time, Is.EqualTo(second_frame_time + (offsetApplied ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0))); } } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index e2a043490f..79d8bd3bb3 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -19,6 +19,11 @@ namespace osu.Game.Beatmaps.Formats { public class LegacyBeatmapDecoder : LegacyDecoder { + /// + /// An offset which needs to be applied to old beatmaps (v4 and lower) to correct timing changes that were applied at a game client level. + /// + public const int EARLY_VERSION_TIMING_OFFSET = 24; + internal static RulesetStore RulesetStore; private Beatmap beatmap; @@ -50,8 +55,7 @@ namespace osu.Game.Beatmaps.Formats RulesetStore = new AssemblyRulesetStore(); } - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) - offset = FormatVersion < 5 ? 24 : 0; + offset = FormatVersion < 5 ? EARLY_VERSION_TIMING_OFFSET : 0; } protected override Beatmap CreateTemplateObject() diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 9885fe5528..754ace82c5 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -74,9 +74,8 @@ namespace osu.Game.Scoring.Legacy currentBeatmap = workingBeatmap.GetPlayableBeatmap(currentRuleset.RulesetInfo, scoreInfo.Mods); scoreInfo.BeatmapInfo = currentBeatmap.BeatmapInfo; - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. - beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? 24 : 0; + beatmapOffset = currentBeatmap.BeatmapInfo.BeatmapVersion < 5 ? LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0; /* score.HpGraphString = */ sr.ReadString(); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index 6a321bed7a..ae9afbf32e 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -1,12 +1,15 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.IO; using System.Linq; using System.Text; using osu.Framework.Extensions; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Formats; using osu.Game.Extensions; using osu.Game.IO.Legacy; using osu.Game.Replays.Legacy; @@ -14,8 +17,6 @@ using osu.Game.Rulesets.Replays; using osu.Game.Rulesets.Replays.Types; using SharpCompress.Compressors.LZMA; -#nullable enable - namespace osu.Game.Scoring.Legacy { public class LegacyScoreEncoder @@ -111,9 +112,8 @@ namespace osu.Game.Scoring.Legacy { StringBuilder replayData = new StringBuilder(); - // BeatmapVersion 4 and lower had an incorrect offset (stable has this set as 24ms off) // As this is baked into hitobject timing (see `LegacyBeatmapDecoder`) we also need to apply this to replay frame timing. - double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -24 : 0; + double offset = beatmap?.BeatmapInfo.BeatmapVersion < 5 ? -LegacyBeatmapDecoder.EARLY_VERSION_TIMING_OFFSET : 0; if (score.Replay != null) { From d6fc53579eb0d7d5d3eed86575006b8820d9135e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 17:00:49 +0900 Subject: [PATCH 39/58] Split out shared code for encode-decode cycle (and remove unrelated culture set) --- .../Formats/LegacyScoreDecoderTest.cs | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 1cd910789f..1474f2d277 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -115,19 +115,7 @@ namespace osu.Game.Tests.Beatmaps.Formats } }; - // the "se" culture is used here, as it encodes the negative number sign as U+2212 MINUS SIGN, - // rather than the classic ASCII U+002D HYPHEN-MINUS. - CultureInfo.CurrentCulture = new CultureInfo("se"); - - var encodeStream = new MemoryStream(); - - var encoder = new LegacyScoreEncoder(score, beatmap); - encoder.Encode(encodeStream); - - var decodeStream = new MemoryStream(encodeStream.GetBuffer()); - - var decoder = new TestLegacyScoreDecoder(beatmapVersion); - var decodedAfterEncode = decoder.Parse(decodeStream); + var decodedAfterEncode = encodeThenDecode(beatmapVersion, score, beatmap); Assert.That(decodedAfterEncode.Replay.Frames[0].Time, Is.EqualTo(first_frame_time)); Assert.That(decodedAfterEncode.Replay.Frames[1].Time, Is.EqualTo(second_frame_time)); @@ -155,15 +143,7 @@ namespace osu.Game.Tests.Beatmaps.Formats // rather than the classic ASCII U+002D HYPHEN-MINUS. CultureInfo.CurrentCulture = new CultureInfo("se"); - var encodeStream = new MemoryStream(); - - var encoder = new LegacyScoreEncoder(score, beatmap); - encoder.Encode(encodeStream); - - var decodeStream = new MemoryStream(encodeStream.GetBuffer()); - - var decoder = new TestLegacyScoreDecoder(); - var decodedAfterEncode = decoder.Parse(decodeStream); + var decodedAfterEncode = encodeThenDecode(LegacyBeatmapDecoder.LATEST_VERSION, score, beatmap); Assert.Multiple(() => { @@ -179,6 +159,20 @@ namespace osu.Game.Tests.Beatmaps.Formats }); } + private static Score encodeThenDecode(int beatmapVersion, Score score, TestBeatmap beatmap) + { + var encodeStream = new MemoryStream(); + + var encoder = new LegacyScoreEncoder(score, beatmap); + encoder.Encode(encodeStream); + + var decodeStream = new MemoryStream(encodeStream.GetBuffer()); + + var decoder = new TestLegacyScoreDecoder(beatmapVersion); + var decodedAfterEncode = decoder.Parse(decodeStream); + return decodedAfterEncode; + } + [TearDown] public void TearDown() { From e889d9344159d6c38641d6eaae56c4ab957a34d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 17:47:58 +0900 Subject: [PATCH 40/58] Add asserts of playlist being non-empty after client operations --- osu.Game/Online/Multiplayer/MultiplayerClient.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Online/Multiplayer/MultiplayerClient.cs b/osu.Game/Online/Multiplayer/MultiplayerClient.cs index 2d5496c5c1..faa995ed19 100644 --- a/osu.Game/Online/Multiplayer/MultiplayerClient.cs +++ b/osu.Game/Online/Multiplayer/MultiplayerClient.cs @@ -171,6 +171,8 @@ namespace osu.Game.Online.Multiplayer Room = joinedRoom; APIRoom = room; + Debug.Assert(joinedRoom.Playlist.Count > 0); + APIRoom.Playlist.Clear(); APIRoom.Playlist.AddRange(joinedRoom.Playlist.Select(createPlaylistItem)); @@ -683,6 +685,8 @@ namespace osu.Game.Online.Multiplayer Room.Playlist.Remove(Room.Playlist.Single(existing => existing.ID == playlistItemId)); APIRoom.Playlist.RemoveAll(existing => existing.ID == playlistItemId); + Debug.Assert(Room.Playlist.Count > 0); + ItemRemoved?.Invoke(playlistItemId); RoomUpdated?.Invoke(); }); From 2d58feebb1ff4050f522caeb053ce4ca819cdfb7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 17:54:21 +0900 Subject: [PATCH 41/58] Guard against potential null `CurrentItem` in `ParticipantPanel` --- .../OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs index 96a665f33d..128b9a1640 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs @@ -189,7 +189,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants var currentItem = Playlist.GetCurrentItem(); Debug.Assert(currentItem != null); - var ruleset = rulesets.GetRuleset(currentItem.RulesetID)?.CreateInstance(); + var ruleset = currentItem != null ? rulesets.GetRuleset(currentItem.RulesetID)?.CreateInstance() : null; int? currentModeRank = ruleset != null ? User.User?.RulesetsStatistics?.GetValueOrDefault(ruleset.ShortName)?.GlobalRank : null; userRankText.Text = currentModeRank != null ? $"#{currentModeRank.Value:N0}" : string.Empty; From 2938f44e6c0307293b4b2f32c26d7f352a5dfe4b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 23:40:46 +0900 Subject: [PATCH 42/58] Update `PresentExternally` usages in line with framework changes --- osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs | 2 +- osu.Game/IO/WrappedStorage.cs | 4 ++-- osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs b/osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs index 93cfa9634e..f0aa857769 100644 --- a/osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs +++ b/osu.Game.Tournament/Screens/Setup/TournamentSwitcher.cs @@ -26,7 +26,7 @@ namespace osu.Game.Tournament.Screens.Setup dropdown.Current.BindValueChanged(v => Button.Enabled.Value = v.NewValue != startupTournament, true); Action = () => game.GracefullyExit(); - folderButton.Action = storage.PresentExternally; + folderButton.Action = () => storage.PresentExternally(); ButtonText = "Close osu!"; } diff --git a/osu.Game/IO/WrappedStorage.cs b/osu.Game/IO/WrappedStorage.cs index 6f0f898de3..a6605de1d2 100644 --- a/osu.Game/IO/WrappedStorage.cs +++ b/osu.Game/IO/WrappedStorage.cs @@ -70,9 +70,9 @@ namespace osu.Game.IO public override Stream GetStream(string path, FileAccess access = FileAccess.Read, FileMode mode = FileMode.OpenOrCreate) => UnderlyingStorage.GetStream(MutatePath(path), access, mode); - public override void OpenFileExternally(string filename) => UnderlyingStorage.OpenFileExternally(MutatePath(filename)); + public override bool OpenFileExternally(string filename) => UnderlyingStorage.OpenFileExternally(MutatePath(filename)); - public override void PresentFileExternally(string filename) => UnderlyingStorage.PresentFileExternally(MutatePath(filename)); + public override bool PresentFileExternally(string filename) => UnderlyingStorage.PresentFileExternally(MutatePath(filename)); public override Storage GetStorageForDirectory(string path) { diff --git a/osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs b/osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs index 158d8811b5..0b4eca6379 100644 --- a/osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs @@ -68,7 +68,7 @@ namespace osu.Game.Overlays.Settings.Sections.General Add(new SettingsButton { Text = GeneralSettingsStrings.OpenOsuFolder, - Action = storage.PresentExternally, + Action = () => storage.PresentExternally(), }); Add(new SettingsButton From b04ca111c6b4f526dc38f172e81275fc7641db3b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Mar 2022 22:28:26 +0900 Subject: [PATCH 43/58] Allow realm subscriptions to be initiated from a non-update thread --- osu.Game/Database/RealmAccess.cs | 30 +++++++++++++++++------------- osu.Game/OsuGameBase.cs | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index f0d4011ab8..8574002436 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Collections.Generic; using System.ComponentModel; @@ -17,6 +19,7 @@ using osu.Framework.Input.Bindings; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Statistics; +using osu.Framework.Threading; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Input.Bindings; @@ -28,8 +31,6 @@ using osu.Game.Stores; using Realms; using Realms.Exceptions; -#nullable enable - namespace osu.Game.Database { /// @@ -46,6 +47,8 @@ namespace osu.Game.Database private readonly IDatabaseContextFactory? efContextFactory; + private readonly SynchronizationContext? updateThreadSyncContext; + /// /// Version history: /// 6 ~2021-10-18 First tracked version. @@ -143,12 +146,15 @@ namespace osu.Game.Database /// /// The game storage which will be used to create the realm backing file. /// The filename to use for the realm backing file. A ".realm" extension will be added automatically if not specified. + /// The game update thread, used to post realm operations into a thread-safe context. /// An EF factory used only for migration purposes. - public RealmAccess(Storage storage, string filename, IDatabaseContextFactory? efContextFactory = null) + public RealmAccess(Storage storage, string filename, GameThread? updateThread = null, IDatabaseContextFactory? efContextFactory = null) { this.storage = storage; this.efContextFactory = efContextFactory; + updateThreadSyncContext = updateThread?.SynchronizationContext ?? SynchronizationContext.Current; + Filename = filename; if (!Filename.EndsWith(realm_extension, StringComparison.Ordinal)) @@ -379,9 +385,6 @@ namespace osu.Game.Database public IDisposable RegisterForNotifications(Func> query, NotificationCallbackDelegate callback) where T : RealmObjectBase { - if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException(@$"{nameof(RegisterForNotifications)} must be called from the update thread."); - lock (realmLock) { Func action = realm => query(realm).QueryAsyncWithNotifications(callback); @@ -459,23 +462,24 @@ namespace osu.Game.Database /// An which should be disposed to unsubscribe any inner subscription. public IDisposable RegisterCustomSubscription(Func action) { - if (!ThreadSafety.IsUpdateThread) - throw new InvalidOperationException(@$"{nameof(RegisterForNotifications)} must be called from the update thread."); - - var syncContext = SynchronizationContext.Current; + if (updateThreadSyncContext == null) + throw new InvalidOperationException("Attempted to register a realm subscription before update thread registration."); total_subscriptions.Value++; - registerSubscription(action); + if (ThreadSafety.IsUpdateThread) + updateThreadSyncContext.Send(_ => registerSubscription(action), null); + else + updateThreadSyncContext.Post(_ => registerSubscription(action), null); // This token is returned to the consumer. // When disposed, it will cause the registration to be permanently ceased (unsubscribed with realm and unregistered by this class). return new InvokeOnDisposal(() => { if (ThreadSafety.IsUpdateThread) - syncContext.Send(_ => unsubscribe(), null); + updateThreadSyncContext.Send(_ => unsubscribe(), null); else - syncContext.Post(_ => unsubscribe(), null); + updateThreadSyncContext.Post(_ => unsubscribe(), null); void unsubscribe() { diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 5468db348e..7b9aca4086 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -200,7 +200,7 @@ namespace osu.Game if (Storage.Exists(DatabaseContextFactory.DATABASE_NAME)) dependencies.Cache(EFContextFactory = new DatabaseContextFactory(Storage)); - dependencies.Cache(realm = new RealmAccess(Storage, "client", EFContextFactory)); + dependencies.Cache(realm = new RealmAccess(Storage, "client", Host.UpdateThread, EFContextFactory)); dependencies.CacheAs(RulesetStore = new RealmRulesetStore(realm, Storage)); dependencies.CacheAs(RulesetStore); From 878e8d21a3925bd4fc1cf4339ca6b346643bbac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 24 Mar 2022 21:51:10 +0100 Subject: [PATCH 44/58] Remove assertion to fix "expression always true" inspection --- .../OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs index 128b9a1640..70f4030b79 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Participants/ParticipantPanel.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; @@ -187,8 +186,6 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Participants const double fade_time = 50; var currentItem = Playlist.GetCurrentItem(); - Debug.Assert(currentItem != null); - var ruleset = currentItem != null ? rulesets.GetRuleset(currentItem.RulesetID)?.CreateInstance() : null; int? currentModeRank = ruleset != null ? User.User?.RulesetsStatistics?.GetValueOrDefault(ruleset.ShortName)?.GlobalRank : null; From b4c0155b3d22f2115fc250ef3451db9e1b2fe273 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 13:07:21 +0900 Subject: [PATCH 45/58] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 1b5461959a..182495cd56 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,7 +52,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 1c1deaae8e..4193dc8fa0 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -36,7 +36,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index 23101c5af6..f97f15d091 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -61,7 +61,7 @@ - + @@ -84,7 +84,7 @@ - + From 09c5325b08bfa8bb9c993c3428b5d79e65c91ed2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 13:18:49 +0900 Subject: [PATCH 46/58] Update resources --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 182495cd56..6a3b113fa2 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -51,7 +51,7 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 4193dc8fa0..3c01f29671 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -37,7 +37,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index f97f15d091..c8f170497d 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -62,7 +62,7 @@ - + From 816fcae3a182647781490aa3c817e93a1c223f47 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 15:12:39 +0900 Subject: [PATCH 47/58] Rename `Button` to `MainMenuButton` to differentiate better --- osu.Game/Screens/Menu/ButtonSystem.cs | 28 +++++++++---------- .../Menu/{Button.cs => MainMenuButton.cs} | 4 +-- 2 files changed, 16 insertions(+), 16 deletions(-) rename osu.Game/Screens/Menu/{Button.cs => MainMenuButton.cs} (97%) diff --git a/osu.Game/Screens/Menu/ButtonSystem.cs b/osu.Game/Screens/Menu/ButtonSystem.cs index b03425fef4..a1f0d22efc 100644 --- a/osu.Game/Screens/Menu/ButtonSystem.cs +++ b/osu.Game/Screens/Menu/ButtonSystem.cs @@ -79,10 +79,10 @@ namespace osu.Game.Screens.Menu private readonly ButtonArea buttonArea; - private readonly Button backButton; + private readonly MainMenuButton backButton; - private readonly List - public class Button : BeatSyncedContainer, IStateful + public class MainMenuButton : BeatSyncedContainer, IStateful { public event Action StateChanged; @@ -51,7 +51,7 @@ namespace osu.Game.Screens.Menu public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => box.ReceivePositionalInputAt(screenSpacePos); - public Button(LocalisableString text, string sampleName, IconUsage symbol, Color4 colour, Action clickAction = null, float extraWidth = 0, Key triggerKey = Key.Unknown) + public MainMenuButton(LocalisableString text, string sampleName, IconUsage symbol, Color4 colour, Action clickAction = null, float extraWidth = 0, Key triggerKey = Key.Unknown) { this.sampleName = sampleName; this.clickAction = clickAction; From 416b57ea69ac8c6bf43c7d0b52338a65246dc2a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 15:13:22 +0900 Subject: [PATCH 48/58] Fix main menu buttons handling keys when super (cmd) is held --- osu.Game/Screens/Menu/MainMenuButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Menu/MainMenuButton.cs b/osu.Game/Screens/Menu/MainMenuButton.cs index 1014249531..88bea43b23 100644 --- a/osu.Game/Screens/Menu/MainMenuButton.cs +++ b/osu.Game/Screens/Menu/MainMenuButton.cs @@ -209,7 +209,7 @@ namespace osu.Game.Screens.Menu protected override bool OnKeyDown(KeyDownEvent e) { - if (e.Repeat || e.ControlPressed || e.ShiftPressed || e.AltPressed) + if (e.Repeat || e.ControlPressed || e.ShiftPressed || e.AltPressed || e.SuperPressed) return false; if (TriggerKey == e.Key && TriggerKey != Key.Unknown) From 23c4f9910e5bba1c0b556bb3838d9eea87cdd6dd Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 25 Mar 2022 15:53:55 +0900 Subject: [PATCH 49/58] Apply notnull constraint --- osu.Game/Skinning/ISkin.cs | 4 +++- osu.Game/Skinning/LegacySkin.cs | 6 +++--- osu.Game/Skinning/ResourceStoreBackedSkin.cs | 5 ++++- osu.Game/Skinning/Skin.cs | 4 +++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/osu.Game/Skinning/ISkin.cs b/osu.Game/Skinning/ISkin.cs index 4b14dcfd62..414a316dec 100644 --- a/osu.Game/Skinning/ISkin.cs +++ b/osu.Game/Skinning/ISkin.cs @@ -52,6 +52,8 @@ namespace osu.Game.Skinning /// /// The requested configuration value. /// A matching value boxed in an , or null if unavailable. - IBindable? GetConfig(TLookup lookup); + IBindable? GetConfig(TLookup lookup) + where TLookup : notnull + where TValue : notnull; } } diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index a3c08f4ba2..92713023f4 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -282,6 +282,7 @@ namespace osu.Game.Skinning => source.ImageLookups.TryGetValue(lookup, out string image) ? new Bindable(image) : null; private IBindable? legacySettingLookup(SkinConfiguration.LegacySetting legacySetting) + where TValue : notnull { switch (legacySetting) { @@ -294,10 +295,9 @@ namespace osu.Game.Skinning } private IBindable? genericLookup(TLookup lookup) + where TLookup : notnull + where TValue : notnull { - if (lookup == null) - return null; - try { if (Configuration.ConfigDictionary.TryGetValue(lookup.ToString(), out string val)) diff --git a/osu.Game/Skinning/ResourceStoreBackedSkin.cs b/osu.Game/Skinning/ResourceStoreBackedSkin.cs index 4787b5a4e9..48286bff59 100644 --- a/osu.Game/Skinning/ResourceStoreBackedSkin.cs +++ b/osu.Game/Skinning/ResourceStoreBackedSkin.cs @@ -46,7 +46,10 @@ namespace osu.Game.Skinning return null; } - public IBindable? GetConfig(TLookup lookup) => null; + public IBindable? GetConfig(TLookup lookup) + where TLookup : notnull + where TValue : notnull + => null; public void Dispose() { diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 2766e58c96..324ed90a67 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -50,7 +50,9 @@ namespace osu.Game.Skinning public abstract Texture? GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT); - public abstract IBindable? GetConfig(TLookup lookup); + public abstract IBindable? GetConfig(TLookup lookup) + where TLookup : notnull + where TValue : notnull; /// /// Construct a new skin. From 4a30b6ef565f320074116cb7ea4a9b447818e471 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 16:22:53 +0900 Subject: [PATCH 50/58] Update multiplayer countdown button text more often At once a second, it regularly skips whole seconds (because scheduler isn't guaranteed to run exactly as often as specified). 10 updates a second seems amicable and less noticeable to my eye. --- .../OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs index 746e4257f1..f0049565c9 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerReadyButton.cs @@ -47,7 +47,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match countdown = room?.Countdown; if (room?.Countdown != null) - countdownUpdateDelegate ??= Scheduler.AddDelayed(updateButtonText, 1000, true); + countdownUpdateDelegate ??= Scheduler.AddDelayed(updateButtonText, 100, true); else { countdownUpdateDelegate?.Cancel(); From 5fcd3b07f1352601ec8970a891f431f9d70433f6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 16:35:32 +0900 Subject: [PATCH 51/58] Fix visual test crashes due to local realm not having update thread --- osu.Game/Tests/Visual/OsuTestScene.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index f287a04d71..6c332c2408 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/osu.Game/Tests/Visual/OsuTestScene.cs @@ -115,11 +115,13 @@ namespace osu.Game.Tests.Visual protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) { - headlessHostStorage = (parent.Get() as HeadlessGameHost)?.Storage; + var host = parent.Get(); + + headlessHostStorage = (host as HeadlessGameHost)?.Storage; Resources = parent.Get().Resources; - realm = new Lazy(() => new RealmAccess(LocalStorage, "client")); + realm = new Lazy(() => new RealmAccess(LocalStorage, "client", host.UpdateThread)); RecycleLocalStorage(false); From 6b22e5774f79d7e7b19e0ff24aee61b77ddb8b9e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 16:42:35 +0900 Subject: [PATCH 52/58] Remove conditional access on known non-null --- osu.Game/Tests/Visual/SkinnableTestScene.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Tests/Visual/SkinnableTestScene.cs b/osu.Game/Tests/Visual/SkinnableTestScene.cs index b4da91a97a..2e1ca09fe4 100644 --- a/osu.Game/Tests/Visual/SkinnableTestScene.cs +++ b/osu.Game/Tests/Visual/SkinnableTestScene.cs @@ -96,7 +96,7 @@ namespace osu.Game.Tests.Visual }, new OsuSpriteText { - Text = skin?.SkinInfo?.Value.Name ?? "none", + Text = skin?.SkinInfo.Value.Name ?? "none", Scale = new Vector2(1.5f), Padding = new MarginPadding(5), }, From 463091bde25e9d97353c6ea3bcb978b1b0523f79 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 18:25:32 +0900 Subject: [PATCH 53/58] Use more appropriate icon on countdown button (and give tooltip) --- .../Multiplayer/Match/MultiplayerCountdownButton.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerCountdownButton.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerCountdownButton.cs index 3bf7e91a55..d6a09e0b31 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerCountdownButton.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerCountdownButton.cs @@ -34,8 +34,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match public MultiplayerCountdownButton() { - Icon = FontAwesome.Solid.CaretDown; - IconScale = new Vector2(0.6f); + Icon = FontAwesome.Regular.Clock; Add(background = new Box { @@ -44,6 +43,8 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match }); base.Action = this.ShowPopover; + + TooltipText = "Countdown settings"; } [BackgroundDependencyLoader] From a0a3bba46e82f954c2264e9553a24c160d9fa813 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 18:31:23 +0900 Subject: [PATCH 54/58] Avoid crashing if a skin component cannot be instantiated correctly --- osu.Game/Skinning/Skin.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index e00dd950a7..7e93a74b7e 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -144,9 +144,23 @@ namespace osu.Game.Skinning if (!DrawableComponentInfo.TryGetValue(target.Target, out var skinnableInfo)) return null; + var components = new List(); + + foreach (var i in skinnableInfo) + { + try + { + components.Add(i.CreateInstance()); + } + catch (Exception e) + { + Logger.Error(e, $"Unable to create skin component {i.Type.Name}"); + } + } + return new SkinnableTargetComponentsContainer { - ChildrenEnumerable = skinnableInfo.Select(i => i.CreateInstance()) + Children = components, }; } From b13408aed0b07708ffdb0c54bb4545da38074638 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 18:37:20 +0900 Subject: [PATCH 55/58] Add back "room visibility" control commented out for now --- .../Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs index aef04c106d..a103d71120 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Match/MultiplayerMatchSettingsOverlay.cs @@ -165,6 +165,14 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer.Match LengthLimit = 100, }, }, + // new Section("Room visibility") + // { + // Alpha = disabled_alpha, + // Child = AvailabilityPicker = new RoomAvailabilityPicker + // { + // Enabled = { Value = false } + // }, + // }, new Section("Game type") { Child = new FillFlowContainer From 320110f179efd7f7e4b5e565d5e85156852c04ba Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 18:37:46 +0900 Subject: [PATCH 56/58] Remove "room visibility" from playlists settings to match --- .../Playlists/PlaylistsRoomSettingsOverlay.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs index 6674a37c3c..1bd227fa4d 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs @@ -152,14 +152,14 @@ namespace osu.Game.Screens.OnlinePlay.Playlists PlaceholderText = "Unlimited", }, }, - new Section("Room visibility") - { - Alpha = disabled_alpha, - Child = AvailabilityPicker = new RoomAvailabilityPicker - { - Enabled = { Value = false } - }, - }, + // new Section("Room visibility") + // { + // Alpha = disabled_alpha, + // Child = AvailabilityPicker = new RoomAvailabilityPicker + // { + // Enabled = { Value = false } + // }, + // }, new Section("Max participants") { Alpha = disabled_alpha, From 76abce486766094622979c2b4be641f8feec118b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 18:38:26 +0900 Subject: [PATCH 57/58] Add missing wait calls on `async` test steps --- .../Visual/Multiplayer/TestSceneMatchStartControl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchStartControl.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchStartControl.cs index e58b9893ce..52854db235 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchStartControl.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchStartControl.cs @@ -192,7 +192,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("local user became ready", () => MultiplayerClient.LocalUser?.State == MultiplayerUserState.Ready); AddUntilStep("countdown button visible", () => this.ChildrenOfType().Single().IsPresent); - AddStep("enable auto start", () => MultiplayerClient.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) })); + AddStep("enable auto start", () => MultiplayerClient.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) }).WaitSafely()); ClickButtonWhenEnabled(); AddUntilStep("local user became ready", () => MultiplayerClient.LocalUser?.State == MultiplayerUserState.Ready); @@ -202,7 +202,7 @@ namespace osu.Game.Tests.Visual.Multiplayer [Test] public void TestClickingReadyButtonUnReadiesDuringAutoStart() { - AddStep("enable auto start", () => MultiplayerClient.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) })); + AddStep("enable auto start", () => MultiplayerClient.ChangeSettings(new MultiplayerRoomSettings { AutoStartDuration = TimeSpan.FromMinutes(1) }).WaitSafely()); ClickButtonWhenEnabled(); AddUntilStep("local user became ready", () => MultiplayerClient.LocalUser?.State == MultiplayerUserState.Ready); From f989158a31e8c8c5a2e9301075c8b1fa4da0cae2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Mar 2022 19:20:16 +0900 Subject: [PATCH 58/58] Add back playlist availability control (because it's hooked up half way?) --- .../Playlists/PlaylistsRoomSettingsOverlay.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs index 1bd227fa4d..6674a37c3c 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSettingsOverlay.cs @@ -152,14 +152,14 @@ namespace osu.Game.Screens.OnlinePlay.Playlists PlaceholderText = "Unlimited", }, }, - // new Section("Room visibility") - // { - // Alpha = disabled_alpha, - // Child = AvailabilityPicker = new RoomAvailabilityPicker - // { - // Enabled = { Value = false } - // }, - // }, + new Section("Room visibility") + { + Alpha = disabled_alpha, + Child = AvailabilityPicker = new RoomAvailabilityPicker + { + Enabled = { Value = false } + }, + }, new Section("Max participants") { Alpha = disabled_alpha,