From 773ec469898c9d616c2211723ebb3a9742dc76c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 11 Sep 2023 23:59:44 +0900 Subject: [PATCH 01/19] Expose some storyboard pieces to allow better testability --- osu.Game/Storyboards/Drawables/DrawableStoryboard.cs | 4 +++- osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.cs | 5 ++++- osu.Game/Storyboards/Storyboard.cs | 2 +- osu.Game/Storyboards/StoryboardVideo.cs | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs index 6931cea81e..a11251ed22 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs @@ -23,7 +23,9 @@ namespace osu.Game.Storyboards.Drawables { public partial class DrawableStoryboard : Container { - [Cached] + public Vector2 AppliedScale { get; private set; } + + [Cached(typeof(Storyboard))] public Storyboard Storyboard { get; } /// diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.cs index 38e7ff1c70..40842fe7ed 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.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 System.Collections.Generic; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Graphics; @@ -30,10 +31,12 @@ namespace osu.Game.Storyboards.Drawables InternalChild = ElementContainer = new LayerElementContainer(layer); } - protected partial class LayerElementContainer : LifetimeManagementContainer + public partial class LayerElementContainer : LifetimeManagementContainer { private readonly StoryboardLayer storyboardLayer; + public IEnumerable Elements => InternalChildren; + public LayerElementContainer(StoryboardLayer layer) { storyboardLayer = layer; diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index 1892855d3d..03e30d6272 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -86,7 +86,7 @@ namespace osu.Game.Storyboards } } - public DrawableStoryboard CreateDrawable(IReadOnlyList? mods = null) => + public virtual DrawableStoryboard CreateDrawable(IReadOnlyList? mods = null) => new DrawableStoryboard(this, mods); private static readonly string[] image_extensions = { @".png", @".jpg" }; diff --git a/osu.Game/Storyboards/StoryboardVideo.cs b/osu.Game/Storyboards/StoryboardVideo.cs index 4652e45852..8c11e19a06 100644 --- a/osu.Game/Storyboards/StoryboardVideo.cs +++ b/osu.Game/Storyboards/StoryboardVideo.cs @@ -14,7 +14,7 @@ namespace osu.Game.Storyboards public double StartTime { get; } - public StoryboardVideo(string path, int offset) + public StoryboardVideo(string path, double offset) { Path = path; StartTime = offset; From 2f020f8682aece5fcf4b54f879c4492149485ff6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Sep 2023 19:44:41 +0900 Subject: [PATCH 02/19] Add test coverage of storyboard preferring skin when specified --- .../TestSceneDrawableStoryboardSprite.cs | 176 +++++++++++++++--- 1 file changed, 150 insertions(+), 26 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs index ec4bb1a86b..d20c7c2f7a 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs @@ -2,17 +2,23 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; +using osu.Framework.IO.Stores; using osu.Framework.Testing; using osu.Game.Rulesets; +using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Storyboards; using osu.Game.Storyboards.Drawables; +using osu.Game.Tests.Resources; using osuTK; namespace osu.Game.Tests.Visual.Gameplay @@ -21,17 +27,21 @@ namespace osu.Game.Tests.Visual.Gameplay { protected override Ruleset CreateRulesetForSkinProvider() => new OsuRuleset(); - [Cached] - private Storyboard storyboard { get; set; } = new Storyboard(); + [Cached(typeof(Storyboard))] + private TestStoryboard storyboard { get; set; } = new TestStoryboard(); private IEnumerable sprites => this.ChildrenOfType(); + private const string lookup_name = "hitcircleoverlay"; + [Test] public void TestSkinSpriteDisallowedByDefault() { - const string lookup_name = "hitcircleoverlay"; - - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = false); + AddStep("disallow all lookups", () => + { + storyboard.UseSkinSprites = false; + storyboard.AlwaysProvideTexture = false; + }); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); @@ -40,11 +50,13 @@ namespace osu.Game.Tests.Visual.Gameplay } [Test] - public void TestAllowLookupFromSkin() + public void TestLookupFromStoryboard() { - const string lookup_name = "hitcircleoverlay"; - - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = true); + AddStep("allow storyboard lookup", () => + { + storyboard.UseSkinSprites = false; + storyboard.AlwaysProvideTexture = true; + }); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); @@ -52,16 +64,54 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("sprite found texture", () => sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Texture != null))); - AddAssert("skinnable sprite has correct size", () => - sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Size == new Vector2(128)))); + assertStoryboardSourced(); + } + + [Test] + public void TestSkinLookupPreferredOverStoryboard() + { + AddStep("allow all lookups", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = true; + }); + + AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); + + // Only checking for at least one sprite that succeeded, as not all skins in this test provide the hitcircleoverlay texture. + AddAssert("sprite found texture", () => + sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Texture != null))); + + assertSkinSourced(); + } + + [Test] + public void TestAllowLookupFromSkin() + { + AddStep("allow skin lookup", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = false; + }); + + AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); + + // Only checking for at least one sprite that succeeded, as not all skins in this test provide the hitcircleoverlay texture. + AddAssert("sprite found texture", () => + sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Texture != null))); + + assertSkinSourced(); } [Test] public void TestFlippedSprite() { - const string lookup_name = "hitcircleoverlay"; + AddStep("allow all lookups", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = true; + }); - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = true); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); AddStep("flip sprites", () => sprites.ForEach(s => { @@ -74,9 +124,12 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestZeroScale() { - const string lookup_name = "hitcircleoverlay"; + AddStep("allow all lookups", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = true; + }); - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = true); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); AddAssert("sprites present", () => sprites.All(s => s.IsPresent)); AddStep("scale sprite", () => sprites.ForEach(s => s.VectorScale = new Vector2(0, 1))); @@ -86,9 +139,12 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestNegativeScale() { - const string lookup_name = "hitcircleoverlay"; + AddStep("allow all lookups", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = true; + }); - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = true); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); AddStep("scale sprite", () => sprites.ForEach(s => s.VectorScale = new Vector2(-1))); AddAssert("origin flipped", () => sprites.All(s => s.Origin == Anchor.BottomRight)); @@ -97,9 +153,12 @@ namespace osu.Game.Tests.Visual.Gameplay [Test] public void TestNegativeScaleWithFlippedSprite() { - const string lookup_name = "hitcircleoverlay"; + AddStep("allow all lookups", () => + { + storyboard.UseSkinSprites = true; + storyboard.AlwaysProvideTexture = true; + }); - AddStep("allow skin lookup", () => storyboard.UseSkinSprites = true); AddStep("create sprites", () => SetContents(_ => createSprite(lookup_name, Anchor.TopLeft, Vector2.Zero))); AddStep("scale sprite", () => sprites.ForEach(s => s.VectorScale = new Vector2(-1))); AddAssert("origin flipped", () => sprites.All(s => s.Origin == Anchor.BottomRight)); @@ -111,13 +170,78 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("origin back", () => sprites.All(s => s.Origin == Anchor.TopLeft)); } - private DrawableStoryboardSprite createSprite(string lookupName, Anchor origin, Vector2 initialPosition) - => new DrawableStoryboardSprite( - new StoryboardSprite(lookupName, origin, initialPosition) - ).With(s => + private DrawableStoryboard createSprite(string lookupName, Anchor origin, Vector2 initialPosition) + { + var layer = storyboard.GetLayer("Background"); + + var sprite = new StoryboardSprite(lookupName, origin, initialPosition); + sprite.AddLoop(Time.Current, 100).Alpha.Add(Easing.None, 0, 10000, 1, 1); + + layer.Elements.Clear(); + layer.Add(sprite); + + return storyboard.CreateDrawable(); + } + + private void assertStoryboardSourced() + { + AddAssert("sprite came from storyboard", () => + sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Size == new Vector2(200)))); + } + + private void assertSkinSourced() + { + AddAssert("sprite came from skin", () => + sprites.Any(sprite => sprite.ChildrenOfType().All(s => s.Size == new Vector2(128)))); + } + + private partial class TestStoryboard : Storyboard + { + public override DrawableStoryboard CreateDrawable(IReadOnlyList? mods = null) { - s.LifetimeStart = double.MinValue; - s.LifetimeEnd = double.MaxValue; - }); + return new TestDrawableStoryboard(this, mods); + } + + public bool AlwaysProvideTexture { get; set; } + + public override string GetStoragePathFromStoryboardPath(string path) => AlwaysProvideTexture ? path : string.Empty; + + private partial class TestDrawableStoryboard : DrawableStoryboard + { + private readonly bool alwaysProvideTexture; + + public TestDrawableStoryboard(TestStoryboard storyboard, IReadOnlyList? mods) + : base(storyboard, mods) + { + alwaysProvideTexture = storyboard.AlwaysProvideTexture; + } + + protected override IResourceStore CreateResourceLookupStore() => alwaysProvideTexture + ? new AlwaysReturnsTextureStore() + : new ResourceStore(); + + internal class AlwaysReturnsTextureStore : IResourceStore + { + private const string test_image = "Resources/Textures/test-image.png"; + + private readonly DllResourceStore store; + + public AlwaysReturnsTextureStore() + { + store = TestResources.GetStore(); + } + + public void Dispose() => store.Dispose(); + + public byte[] Get(string name) => store.Get(test_image); + + public Task GetAsync(string name, CancellationToken cancellationToken = new CancellationToken()) => store.GetAsync(test_image, cancellationToken); + + public Stream GetStream(string name) => store.GetStream(test_image); + + public IEnumerable GetAvailableResources() => store.GetAvailableResources(); + } + } + } } } From 4a7dc4d7927aaba2b1f527e6b2939fa179189535 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Sep 2023 19:13:58 +0900 Subject: [PATCH 03/19] Fix storyboard `UseSkinSprites` being implemented incorrectly This was implemented as a "fallback", but it's actually intended to be an "override". As in it allows storyboarders to *prefer* a skin sprite before falling back to a local version contained within the storyboard. Can be tested with https://osu.ppy.sh/beatmapsets/832364#osu/1743837. Closes https://github.com/ppy/osu/issues/24813. --- .../Drawables/DrawableStoryboardSprite.cs | 23 ++++++++++++------- osu.Game/Storyboards/Storyboard.cs | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs index 379de1a497..14132654d1 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.ComponentModel; using osu.Framework.Allocation; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; @@ -74,6 +75,15 @@ namespace osu.Game.Storyboards.Drawables public override bool IsPresent => !float.IsNaN(DrawPosition.X) && !float.IsNaN(DrawPosition.Y) && base.IsPresent; + [Resolved] + private ISkinSource skin { get; set; } = null!; + + [Resolved] + private Storyboard storyboard { get; set; } = null!; + + [Resolved] + private TextureStore textureStore { get; set; } = null!; + public DrawableStoryboardSprite(StoryboardSprite sprite) { Sprite = sprite; @@ -84,24 +94,21 @@ namespace osu.Game.Storyboards.Drawables LifetimeEnd = sprite.EndTimeForDisplay; } - [Resolved] - private ISkinSource skin { get; set; } = null!; - [BackgroundDependencyLoader] - private void load(TextureStore textureStore, Storyboard storyboard) + private void load() { - Texture = textureStore.Get(Sprite.Path); - - if (Texture == null && storyboard.UseSkinSprites) + if (storyboard.UseSkinSprites) { skin.SourceChanged += skinSourceChanged; skinSourceChanged(); } + else + Texture = textureStore.Get(Sprite.Path); Sprite.ApplyTransforms(this); } - private void skinSourceChanged() => Texture = skin.GetTexture(Sprite.Path); + private void skinSourceChanged() => Texture = skin.GetTexture(Sprite.Path) ?? textureStore.Get(Sprite.Path); protected override void Dispose(bool isDisposing) { diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index 03e30d6272..21342831b0 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -18,7 +18,7 @@ namespace osu.Game.Storyboards public BeatmapInfo BeatmapInfo = new BeatmapInfo(); /// - /// Whether the storyboard can fall back to skin sprites in case no matching storyboard sprites are found. + /// Whether the storyboard should prefer textures from the current skin before using local storyboard textures. /// public bool UseSkinSprites { get; set; } From 50adb5f7a7991dcfd84eba98bb28155b65e32175 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 12:54:28 +0900 Subject: [PATCH 04/19] Remove incorrectly merge conflict resolved --- osu.Game/Storyboards/Drawables/DrawableStoryboard.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs index a11251ed22..352246c533 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs @@ -23,8 +23,6 @@ namespace osu.Game.Storyboards.Drawables { public partial class DrawableStoryboard : Container { - public Vector2 AppliedScale { get; private set; } - [Cached(typeof(Storyboard))] public Storyboard Storyboard { get; } From b5e64d933c9d09196c0553d548e5dde0bbec8ed2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 12:54:36 +0900 Subject: [PATCH 05/19] Apply same fix to `DrawableStoryboardAnimation` --- .../Drawables/DrawableStoryboardAnimation.cs | 43 ++++++++++++------- .../Drawables/DrawableStoryboardSprite.cs | 6 +-- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardAnimation.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardAnimation.cs index 054a50456b..33f7a3c6f2 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardAnimation.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardAnimation.cs @@ -94,25 +94,19 @@ namespace osu.Game.Storyboards.Drawables [Resolved] private IBeatSyncProvider beatSyncProvider { get; set; } + [Resolved] + private TextureStore textureStore { get; set; } + [BackgroundDependencyLoader] - private void load(TextureStore textureStore, Storyboard storyboard) + private void load(Storyboard storyboard) { - int frameIndex = 0; - - Texture frameTexture = textureStore.Get(getFramePath(frameIndex)); - - if (frameTexture != null) + if (storyboard.UseSkinSprites) { - // sourcing from storyboard. - for (frameIndex = 0; frameIndex < Animation.FrameCount; frameIndex++) - AddFrame(textureStore.Get(getFramePath(frameIndex)), Animation.FrameDelay); - } - else if (storyboard.UseSkinSprites) - { - // fallback to skin if required. skin.SourceChanged += skinSourceChanged; skinSourceChanged(); } + else + addFramesFromStoryboardSource(); Animation.ApplyTransforms(this); } @@ -135,11 +129,28 @@ namespace osu.Game.Storyboards.Drawables // When reading from a skin, we match stables weird behaviour where `FrameCount` is ignored // and resources are retrieved until the end of the animation. - foreach (var texture in skin.GetTextures(Path.GetFileNameWithoutExtension(Animation.Path)!, default, default, true, string.Empty, out _)) - AddFrame(texture, Animation.FrameDelay); + var skinTextures = skin.GetTextures(Path.GetFileNameWithoutExtension(Animation.Path)!, default, default, true, string.Empty, out _); + + if (skinTextures.Length > 0) + { + foreach (var texture in skinTextures) + AddFrame(texture, Animation.FrameDelay); + } + else + { + addFramesFromStoryboardSource(); + } } - private string getFramePath(int i) => Animation.Path.Replace(".", $"{i}."); + private void addFramesFromStoryboardSource() + { + int frameIndex; + // sourcing from storyboard. + for (frameIndex = 0; frameIndex < Animation.FrameCount; frameIndex++) + AddFrame(textureStore.Get(getFramePath(frameIndex)), Animation.FrameDelay); + + string getFramePath(int i) => Animation.Path.Replace(".", $"{i}."); + } protected override void Dispose(bool isDisposing) { diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs index 14132654d1..ad344b6bd4 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.ComponentModel; using osu.Framework.Allocation; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; @@ -78,9 +77,6 @@ namespace osu.Game.Storyboards.Drawables [Resolved] private ISkinSource skin { get; set; } = null!; - [Resolved] - private Storyboard storyboard { get; set; } = null!; - [Resolved] private TextureStore textureStore { get; set; } = null!; @@ -95,7 +91,7 @@ namespace osu.Game.Storyboards.Drawables } [BackgroundDependencyLoader] - private void load() + private void load(Storyboard storyboard) { if (storyboard.UseSkinSprites) { From d7129da8ea708f5485e023a18012940b1d5d3dfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 20 Sep 2023 12:05:23 +0200 Subject: [PATCH 06/19] Fix `TestSceneDrawableStoryboardSprite` not displaying anything --- .../Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs index d20c7c2f7a..32693c2bb2 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableStoryboardSprite.cs @@ -180,7 +180,7 @@ namespace osu.Game.Tests.Visual.Gameplay layer.Elements.Clear(); layer.Add(sprite); - return storyboard.CreateDrawable(); + return storyboard.CreateDrawable().With(s => s.RelativeSizeAxes = Axes.Both); } private void assertStoryboardSourced() From 59d3143645ce5af731bd52fc4a08fc12cc0268be Mon Sep 17 00:00:00 2001 From: Fabian van Oeffelt Date: Mon, 25 Sep 2023 16:53:40 +0200 Subject: [PATCH 07/19] Fix SR/BPM Display in Playlist rooms --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 8d08de4168..cfae890323 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -32,6 +32,7 @@ using osu.Game.Rulesets.Mods; using osu.Game.Screens.Menu; using osu.Game.Screens.OnlinePlay.Match.Components; using osu.Game.Screens.OnlinePlay.Multiplayer; +using osuTK; namespace osu.Game.Screens.OnlinePlay.Match { @@ -437,6 +438,8 @@ namespace osu.Game.Screens.OnlinePlay.Match } else { + var localBeatmap = selected.Beatmap == null ? null : beatmapManager.QueryBeatmap(b => b.OnlineID == selected.Beatmap.OnlineID); + UserModsSelectOverlay.Beatmap = beatmapManager.GetWorkingBeatmap(localBeatmap); UserModsSection?.Show(); UserModsSelectOverlay.IsValidMod = m => allowedMods.Any(a => a.GetType() == m.GetType()); } From 2a3391d83b0e1832a8127d2b8a9b7b6f3e940efb Mon Sep 17 00:00:00 2001 From: Fabian van Oeffelt Date: Mon, 25 Sep 2023 16:56:34 +0200 Subject: [PATCH 08/19] Remove unused namepace --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index cfae890323..f9482b810a 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -32,7 +32,6 @@ using osu.Game.Rulesets.Mods; using osu.Game.Screens.Menu; using osu.Game.Screens.OnlinePlay.Match.Components; using osu.Game.Screens.OnlinePlay.Multiplayer; -using osuTK; namespace osu.Game.Screens.OnlinePlay.Match { From 2040dcabe067a9daa54f6d6cab490d3c97f4ef45 Mon Sep 17 00:00:00 2001 From: Fabian van Oeffelt Date: Mon, 25 Sep 2023 17:51:39 +0200 Subject: [PATCH 09/19] Improve Code Quality --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index f9482b810a..ab29ac6d06 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -437,7 +437,7 @@ namespace osu.Game.Screens.OnlinePlay.Match } else { - var localBeatmap = selected.Beatmap == null ? null : beatmapManager.QueryBeatmap(b => b.OnlineID == selected.Beatmap.OnlineID); + var localBeatmap = beatmapManager.QueryBeatmap(b => b.OnlineID == selected.Beatmap.OnlineID); UserModsSelectOverlay.Beatmap = beatmapManager.GetWorkingBeatmap(localBeatmap); UserModsSection?.Show(); UserModsSelectOverlay.IsValidMod = m => allowedMods.Any(a => a.GetType() == m.GetType()); From 567bc8fcd346573f1b9086f6636c2020624bc04d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Sep 2023 14:13:51 +0900 Subject: [PATCH 10/19] Ensure `DrawableStoryboardSprite`'s texture size propagates corectly on skin change --- .../Storyboards/Drawables/DrawableStoryboardSprite.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs index ad344b6bd4..87c09c3e5e 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs @@ -104,7 +104,14 @@ namespace osu.Game.Storyboards.Drawables Sprite.ApplyTransforms(this); } - private void skinSourceChanged() => Texture = skin.GetTexture(Sprite.Path) ?? textureStore.Get(Sprite.Path); + private void skinSourceChanged() + { + // Setting texture will only update the size if it's zero. + // So let's force an update by setting to zero. + Size = Vector2.Zero; + + Texture = skin.GetTexture(Sprite.Path) ?? textureStore.Get(Sprite.Path); + } protected override void Dispose(bool isDisposing) { From d1d82d2b4952a179b5d55ac6f1ed144d07770993 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Sep 2023 15:00:56 +0900 Subject: [PATCH 11/19] Improve notification display when score import fails --- osu.Game/Database/RealmArchiveModelImporter.cs | 2 +- osu.Game/Scoring/ScoreImporter.cs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index 9d06c14b4b..d2de00d476 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -149,7 +149,7 @@ namespace osu.Game.Database return imported; } - notification.Text = $"{HumanisedModelName.Humanize(LetterCasing.Title)} import failed!"; + notification.Text = $"{HumanisedModelName.Humanize(LetterCasing.Title)} import failed! Check logs for more information."; notification.State = ProgressNotificationState.Cancelled; } else diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index b85b6a066e..5af53c6f7b 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -62,6 +62,11 @@ namespace osu.Game.Scoring api.Queue(req); return null; } + catch (Exception) + { + Logger.Log($@"Score '{archive.Name}' failed to import: failed to parse replay headers.", LoggingTarget.Database); + return null; + } } } From 3708e79577472e2c5284fc635eb65a9dd022545f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Sep 2023 16:55:03 +0900 Subject: [PATCH 12/19] Adjust logging to still expose the underlying exception --- osu.Game/Scoring/ScoreImporter.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index 5af53c6f7b..2314dbb63b 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -52,19 +52,19 @@ namespace osu.Game.Scoring { return new DatabasedLegacyScoreDecoder(rulesets, beatmaps()).Parse(stream).ScoreInfo; } - catch (LegacyScoreDecoder.BeatmapNotFoundException e) + catch (LegacyScoreDecoder.BeatmapNotFoundException notFound) { - Logger.Log($@"Score '{archive.Name}' failed to import: no corresponding beatmap with the hash '{e.Hash}' could be found.", LoggingTarget.Database); + Logger.Log($@"Score '{archive.Name}' failed to import: no corresponding beatmap with the hash '{notFound.Hash}' could be found.", LoggingTarget.Database); // In the case of a missing beatmap, let's attempt to resolve it and show a prompt to the user to download the required beatmap. - var req = new GetBeatmapRequest(new BeatmapInfo { MD5Hash = e.Hash }); - req.Success += res => PostNotification?.Invoke(new MissingBeatmapNotification(res, archive, e.Hash)); + var req = new GetBeatmapRequest(new BeatmapInfo { MD5Hash = notFound.Hash }); + req.Success += res => PostNotification?.Invoke(new MissingBeatmapNotification(res, archive, notFound.Hash)); api.Queue(req); return null; } - catch (Exception) + catch (Exception e) { - Logger.Log($@"Score '{archive.Name}' failed to import: failed to parse replay headers.", LoggingTarget.Database); + Logger.Log($@"Failed to parse headers of score '{archive.Name}': {e}.", LoggingTarget.Database); return null; } } From 2481c0b64bc2158d710f8cdc57d6efcc4b05f5f3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Sep 2023 17:02:47 +0900 Subject: [PATCH 13/19] Don't show replay import "missing beatmap" notifications during stable import Closes https://github.com/ppy/osu/issues/24926. --- osu.Game/Beatmaps/BeatmapImporter.cs | 2 +- osu.Game/Database/RealmArchiveModelImporter.cs | 5 +++-- osu.Game/Scoring/ScoreImporter.cs | 14 +++++++++----- osu.Game/Skinning/SkinImporter.cs | 2 +- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index b497b8e8dc..e89e5339e1 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -280,7 +280,7 @@ namespace osu.Game.Beatmaps public override string HumanisedModelName => "beatmap"; - protected override BeatmapSetInfo? CreateModel(ArchiveReader reader) + protected override BeatmapSetInfo? CreateModel(ArchiveReader reader, ImportParameters parameters) { // let's make sure there are actually .osu files to import. string? mapName = reader.Filenames.FirstOrDefault(f => f.EndsWith(".osu", StringComparison.OrdinalIgnoreCase)); diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index 9d06c14b4b..581ed41f75 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -229,7 +229,7 @@ namespace osu.Game.Database try { - model = CreateModel(archive); + model = CreateModel(archive, parameters); if (model == null) return null; @@ -474,8 +474,9 @@ namespace osu.Game.Database /// Actual expensive population should be done in ; this should just prepare for duplicate checking. /// /// The archive to create the model for. + /// Parameters to further configure the import process. /// A model populated with minimal information. Returning a null will abort importing silently. - protected abstract TModel? CreateModel(ArchiveReader archive); + protected abstract TModel? CreateModel(ArchiveReader archive, ImportParameters parameters); /// /// Populate the provided model completely from the given archive. diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index b85b6a066e..28e70cff4f 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -42,7 +42,7 @@ namespace osu.Game.Scoring this.api = api; } - protected override ScoreInfo? CreateModel(ArchiveReader archive) + protected override ScoreInfo? CreateModel(ArchiveReader archive, ImportParameters parameters) { string name = archive.Filenames.First(f => f.EndsWith(".osr", StringComparison.OrdinalIgnoreCase)); @@ -56,10 +56,14 @@ namespace osu.Game.Scoring { Logger.Log($@"Score '{archive.Name}' failed to import: no corresponding beatmap with the hash '{e.Hash}' could be found.", LoggingTarget.Database); - // In the case of a missing beatmap, let's attempt to resolve it and show a prompt to the user to download the required beatmap. - var req = new GetBeatmapRequest(new BeatmapInfo { MD5Hash = e.Hash }); - req.Success += res => PostNotification?.Invoke(new MissingBeatmapNotification(res, archive, e.Hash)); - api.Queue(req); + if (!parameters.Batch) + { + // In the case of a missing beatmap, let's attempt to resolve it and show a prompt to the user to download the required beatmap. + var req = new GetBeatmapRequest(new BeatmapInfo { MD5Hash = e.Hash }); + req.Success += res => PostNotification?.Invoke(new MissingBeatmapNotification(res, archive, e.Hash)); + api.Queue(req); + } + return null; } } diff --git a/osu.Game/Skinning/SkinImporter.cs b/osu.Game/Skinning/SkinImporter.cs index f2103a45c4..b543197296 100644 --- a/osu.Game/Skinning/SkinImporter.cs +++ b/osu.Game/Skinning/SkinImporter.cs @@ -39,7 +39,7 @@ namespace osu.Game.Skinning protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == @".osk"; - protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name ?? @"No name" }; + protected override SkinInfo? CreateModel(ArchiveReader archive, ImportParameters parameters) => new SkinInfo { Name = archive.Name ?? @"No name" }; private const string unknown_creator_string = @"Unknown"; From 3705c4c8d5b9cac2422ab1cb8e44244c1f5de085 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Sep 2023 17:16:45 +0900 Subject: [PATCH 14/19] Avoid potentially incorrect size adjustment when texture doesn't change --- .../Storyboards/Drawables/DrawableStoryboardSprite.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs index 87c09c3e5e..68892a6fc1 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs @@ -106,11 +106,11 @@ namespace osu.Game.Storyboards.Drawables private void skinSourceChanged() { - // Setting texture will only update the size if it's zero. - // So let's force an update by setting to zero. - Size = Vector2.Zero; - Texture = skin.GetTexture(Sprite.Path) ?? textureStore.Get(Sprite.Path); + + // Setting texture will only update the size if it's zero. + // So let's force an explicit update. + Size = new Vector2(Texture.DisplayWidth, Texture.DisplayHeight); } protected override void Dispose(bool isDisposing) From 175cf68bd6003e61210929642ebf501725ad4b4c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Sep 2023 18:13:07 +0900 Subject: [PATCH 15/19] Avoid re-fetching `WorkingBeatmap` --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index ab29ac6d06..2974035127 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -437,8 +437,7 @@ namespace osu.Game.Screens.OnlinePlay.Match } else { - var localBeatmap = beatmapManager.QueryBeatmap(b => b.OnlineID == selected.Beatmap.OnlineID); - UserModsSelectOverlay.Beatmap = beatmapManager.GetWorkingBeatmap(localBeatmap); + UserModsSelectOverlay.Beatmap = Beatmap.Value; UserModsSection?.Show(); UserModsSelectOverlay.IsValidMod = m => allowedMods.Any(a => a.GetType() == m.GetType()); } From a639e51ddb8c0de208ea5a6f7b54ffff5487370e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Sep 2023 18:17:25 +0900 Subject: [PATCH 16/19] Fix value not updating after missing beatmap is imported --- osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs index 2974035127..b8887b3161 100644 --- a/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Match/RoomSubScreen.cs @@ -437,7 +437,6 @@ namespace osu.Game.Screens.OnlinePlay.Match } else { - UserModsSelectOverlay.Beatmap = Beatmap.Value; UserModsSection?.Show(); UserModsSelectOverlay.IsValidMod = m => allowedMods.Any(a => a.GetType() == m.GetType()); } @@ -453,7 +452,7 @@ namespace osu.Game.Screens.OnlinePlay.Match // Retrieve the corresponding local beatmap, since we can't directly use the playlist's beatmap info var localBeatmap = beatmap == null ? null : beatmapManager.QueryBeatmap(b => b.OnlineID == beatmap.OnlineID); - Beatmap.Value = beatmapManager.GetWorkingBeatmap(localBeatmap); + UserModsSelectOverlay.Beatmap = Beatmap.Value = beatmapManager.GetWorkingBeatmap(localBeatmap); } protected virtual void UpdateMods() From e314913aeb0f509b526198af5871ea49269c5952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 27 Sep 2023 11:18:38 +0200 Subject: [PATCH 17/19] Handle null texture case --- osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs index 68892a6fc1..ec875219b6 100644 --- a/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs +++ b/osu.Game/Storyboards/Drawables/DrawableStoryboardSprite.cs @@ -110,7 +110,7 @@ namespace osu.Game.Storyboards.Drawables // Setting texture will only update the size if it's zero. // So let's force an explicit update. - Size = new Vector2(Texture.DisplayWidth, Texture.DisplayHeight); + Size = new Vector2(Texture?.DisplayWidth ?? 0, Texture?.DisplayHeight ?? 0); } protected override void Dispose(bool isDisposing) From 83bf1efc1cff73e82356915e5963f454ff0342af Mon Sep 17 00:00:00 2001 From: NiceAesth Date: Wed, 27 Sep 2023 16:29:29 +0300 Subject: [PATCH 18/19] Add `color` search alias for `colour` settings --- osu.Game.Rulesets.Mania/ManiaSettingsSubsection.cs | 1 + .../Overlays/Settings/Sections/Gameplay/BeatmapSettings.cs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/ManiaSettingsSubsection.cs b/osu.Game.Rulesets.Mania/ManiaSettingsSubsection.cs index 065534eec4..30eca0636c 100644 --- a/osu.Game.Rulesets.Mania/ManiaSettingsSubsection.cs +++ b/osu.Game.Rulesets.Mania/ManiaSettingsSubsection.cs @@ -41,6 +41,7 @@ namespace osu.Game.Rulesets.Mania }, new SettingsCheckbox { + Keywords = new[] { "color" }, LabelText = RulesetSettingsStrings.TimingBasedColouring, Current = config.GetBindable(ManiaRulesetSetting.TimingBasedNoteColouring), } diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/BeatmapSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/BeatmapSettings.cs index 9efdfa9955..69566d85f4 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/BeatmapSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/BeatmapSettings.cs @@ -30,7 +30,7 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay }, new SettingsCheckbox { - Keywords = new[] { "combo", "override" }, + Keywords = new[] { "combo", "override", "color" }, LabelText = SkinSettingsStrings.BeatmapColours, Current = config.GetBindable(OsuSetting.BeatmapColours) }, @@ -47,6 +47,7 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay }, new SettingsSlider { + Keywords = new[] { "color" }, LabelText = GraphicsSettingsStrings.ComboColourNormalisation, Current = comboColourNormalisation, DisplayAsPercentage = true, From edf40cc091191d3e4ffdac906992afcfdd1bbfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 27 Sep 2023 17:09:42 +0200 Subject: [PATCH 19/19] Fix nullability inspection --- osu.Game/Skinning/SkinImporter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinImporter.cs b/osu.Game/Skinning/SkinImporter.cs index b543197296..40585a3fd2 100644 --- a/osu.Game/Skinning/SkinImporter.cs +++ b/osu.Game/Skinning/SkinImporter.cs @@ -39,7 +39,7 @@ namespace osu.Game.Skinning protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path).ToLowerInvariant() == @".osk"; - protected override SkinInfo? CreateModel(ArchiveReader archive, ImportParameters parameters) => new SkinInfo { Name = archive.Name ?? @"No name" }; + protected override SkinInfo CreateModel(ArchiveReader archive, ImportParameters parameters) => new SkinInfo { Name = archive.Name ?? @"No name" }; private const string unknown_creator_string = @"Unknown";