From b75437ee1303dfef70cd548b15a9fcc20eea8032 Mon Sep 17 00:00:00 2001 From: Darius Wattimena Date: Tue, 15 Oct 2024 21:13:04 +0200 Subject: [PATCH 01/23] Fix an issue where changing the CircleSize wouldn't adjust the catcher size and represent hyperdashes incorrectly --- .../Edit/CatchEditorPlayfield.cs | 6 ++++- .../Edit/DrawableCatchEditorRuleset.cs | 23 +++++++++++++++++++ osu.Game.Rulesets.Catch/UI/Catcher.cs | 18 +++++++++++---- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs b/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs index c9481c2757..78f5c3e9ed 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs @@ -8,7 +8,6 @@ namespace osu.Game.Rulesets.Catch.Edit { public partial class CatchEditorPlayfield : CatchPlayfield { - // TODO fixme: the size of the catcher is not changed when circle size is changed in setup screen. public CatchEditorPlayfield(IBeatmapDifficultyInfo difficulty) : base(difficulty) { @@ -23,5 +22,10 @@ namespace osu.Game.Rulesets.Catch.Edit // TODO: disable hit lighting as well } + + public void ApplyCircleSizeToCatcher(IBeatmapDifficultyInfo difficulty) + { + Catcher.SetScaleAndCatchWidth(difficulty); + } } } diff --git a/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs b/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs index 7ad2106ab9..cd6d490a7f 100644 --- a/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs +++ b/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs @@ -2,16 +2,22 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Extensions.ObjectExtensions; using osu.Game.Beatmaps; using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.UI; +using osu.Game.Screens.Edit; namespace osu.Game.Rulesets.Catch.Edit { public partial class DrawableCatchEditorRuleset : DrawableCatchRuleset { + [Resolved] + private EditorBeatmap editorBeatmap { get; set; } = null!; + public readonly BindableDouble TimeRangeMultiplier = new BindableDouble(1); public DrawableCatchEditorRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList? mods = null) @@ -28,6 +34,23 @@ namespace osu.Game.Rulesets.Catch.Edit TimeRange.Value = gamePlayTimeRange * TimeRangeMultiplier.Value * playfieldStretch; } + protected override void LoadComplete() + { + base.LoadComplete(); + + editorBeatmap.BeatmapReprocessed += onBeatmapReprocessed; + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (editorBeatmap.IsNotNull()) + editorBeatmap.BeatmapReprocessed -= onBeatmapReprocessed; + } + + private void onBeatmapReprocessed() => (Playfield as CatchEditorPlayfield)?.ApplyCircleSizeToCatcher(editorBeatmap.Difficulty); + protected override Playfield CreatePlayfield() => new CatchEditorPlayfield(Beatmap.Difficulty); public override PlayfieldAdjustmentContainer CreatePlayfieldAdjustmentContainer() => new CatchEditorPlayfieldAdjustmentContainer(); diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 6a1b251d60..3a0863473a 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -116,7 +116,7 @@ namespace osu.Game.Rulesets.Catch.UI /// /// Width of the area that can be used to attempt catches during gameplay. /// - public readonly float CatchWidth; + public float CatchWidth; private readonly SkinnableCatcher body; @@ -142,10 +142,7 @@ namespace osu.Game.Rulesets.Catch.UI Size = new Vector2(BASE_SIZE); - if (difficulty != null) - Scale = calculateScale(difficulty); - - CatchWidth = CalculateCatchWidth(Scale); + SetScaleAndCatchWidth(difficulty); InternalChildren = new Drawable[] { @@ -312,6 +309,17 @@ namespace osu.Game.Rulesets.Catch.UI } } + /// + /// Set the scale and catch width. + /// + public void SetScaleAndCatchWidth(IBeatmapDifficultyInfo? difficulty) + { + if (difficulty != null) + Scale = calculateScale(difficulty); + + CatchWidth = CalculateCatchWidth(Scale); + } + /// /// Drop any fruit off the plate. /// From 1e2c1323ff861386a3272f7db3ee1fefc45da1fb Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 17:05:49 +0900 Subject: [PATCH 02/23] Show star difficulty attribute inclusive of mods --- .../Components/BeatmapAttributeText.cs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index 63ba6d1581..d6246b4bcf 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading; using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -33,7 +34,12 @@ namespace osu.Game.Skinning.Components [Resolved] private IBindable beatmap { get; set; } = null!; + [Resolved] + private BeatmapDifficultyCache difficultyCache { get; set; } = null!; + private readonly OsuSpriteText text; + private IBindable? difficultyBindable; + private CancellationTokenSource? difficultyCancellationSource; public BeatmapAttributeText() { @@ -55,7 +61,18 @@ namespace osu.Game.Skinning.Components Attribute.BindValueChanged(_ => updateText()); Template.BindValueChanged(_ => updateText()); - beatmap.BindValueChanged(b => updateText()); + + beatmap.BindValueChanged(b => + { + difficultyCancellationSource?.Cancel(); + difficultyCancellationSource = new CancellationTokenSource(); + + difficultyBindable?.UnbindAll(); + difficultyBindable = difficultyCache.GetBindableDifficulty(b.NewValue.BeatmapInfo, difficultyCancellationSource.Token); + difficultyBindable.BindValueChanged(_ => updateText()); + + updateText(); + }, true); updateText(); } @@ -172,7 +189,9 @@ namespace osu.Game.Skinning.Components return ((double)beatmap.Value.BeatmapInfo.Difficulty.ApproachRate).ToLocalisableString(@"F2"); case BeatmapAttribute.StarRating: - return beatmap.Value.BeatmapInfo.StarRating.ToLocalisableString(@"F2"); + return difficultyBindable?.Value is StarDifficulty starDifficulty + ? starDifficulty.Stars.ToLocalisableString(@"F2") + : @"..."; default: throw new ArgumentOutOfRangeException(); @@ -182,6 +201,15 @@ namespace osu.Game.Skinning.Components protected override void SetFont(FontUsage font) => text.Font = font.With(size: 40); protected override void SetTextColour(Colour4 textColour) => text.Colour = textColour; + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + difficultyCancellationSource?.Cancel(); + difficultyCancellationSource?.Dispose(); + difficultyCancellationSource = null; + } } // WARNING: DO NOT ADD ANY VALUES TO THIS ENUM ANYWHERE ELSE THAN AT THE END. From 400142545df498fa57f375a3b67e5150ee9f6c07 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 17:26:02 +0900 Subject: [PATCH 03/23] Show difficulty values inclusive of mods --- .../Components/BeatmapAttributeText.cs | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index d6246b4bcf..ed44d4b44c 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using JetBrains.Annotations; using osu.Framework.Allocation; @@ -19,6 +20,9 @@ using osu.Game.Graphics.Sprites; using osu.Game.Localisation; using osu.Game.Localisation.SkinComponents; using osu.Game.Resources.Localisation.Web; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Mods; +using osu.Game.Utils; namespace osu.Game.Skinning.Components { @@ -34,6 +38,12 @@ namespace osu.Game.Skinning.Components [Resolved] private IBindable beatmap { get; set; } = null!; + [Resolved] + private IBindable> mods { get; set; } = null!; + + [Resolved] + private IBindable ruleset { get; set; } = null!; + [Resolved] private BeatmapDifficultyCache difficultyCache { get; set; } = null!; @@ -74,6 +84,9 @@ namespace osu.Game.Skinning.Components updateText(); }, true); + mods.BindValueChanged(_ => updateText()); + ruleset.BindValueChanged(_ => updateText()); + updateText(); } @@ -177,16 +190,16 @@ namespace osu.Game.Skinning.Components return beatmap.Value.BeatmapInfo.BPM.ToLocalisableString(@"F2"); case BeatmapAttribute.CircleSize: - return ((double)beatmap.Value.BeatmapInfo.Difficulty.CircleSize).ToLocalisableString(@"F2"); + return computeDifficulty().CircleSize.ToLocalisableString(@"F2"); case BeatmapAttribute.HPDrain: - return ((double)beatmap.Value.BeatmapInfo.Difficulty.DrainRate).ToLocalisableString(@"F2"); + return computeDifficulty().DrainRate.ToLocalisableString(@"F2"); case BeatmapAttribute.Accuracy: - return ((double)beatmap.Value.BeatmapInfo.Difficulty.OverallDifficulty).ToLocalisableString(@"F2"); + return computeDifficulty().OverallDifficulty.ToLocalisableString(@"F2"); case BeatmapAttribute.ApproachRate: - return ((double)beatmap.Value.BeatmapInfo.Difficulty.ApproachRate).ToLocalisableString(@"F2"); + return computeDifficulty().ApproachRate.ToLocalisableString(@"F2"); case BeatmapAttribute.StarRating: return difficultyBindable?.Value is StarDifficulty starDifficulty @@ -196,6 +209,22 @@ namespace osu.Game.Skinning.Components default: throw new ArgumentOutOfRangeException(); } + + BeatmapDifficulty computeDifficulty() + { + BeatmapDifficulty difficulty = new BeatmapDifficulty(beatmap.Value.BeatmapInfo.Difficulty); + + foreach (var mod in mods.Value.OfType()) + mod.ApplyToDifficulty(difficulty); + + if (ruleset.Value is RulesetInfo rulesetInfo) + { + double rate = ModUtils.CalculateRateWithMods(mods.Value); + difficulty = rulesetInfo.CreateInstance().GetRateAdjustedDisplayDifficulty(difficulty, rate); + } + + return difficulty; + } } protected override void SetFont(FontUsage font) => text.Font = font.With(size: 40); From 9ea15a39619068deed002b50d794b9eacaef8b64 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 17:32:32 +0900 Subject: [PATCH 04/23] Show bpm and length inclusive of mods --- osu.Game/Skinning/Components/BeatmapAttributeText.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index ed44d4b44c..6cfed8b945 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -181,13 +181,13 @@ namespace osu.Game.Skinning.Components return beatmap.Value.BeatmapInfo.Metadata.Source; case BeatmapAttribute.Length: - return TimeSpan.FromMilliseconds(beatmap.Value.BeatmapInfo.Length).ToFormattedDuration(); + return Math.Round(beatmap.Value.BeatmapInfo.Length / ModUtils.CalculateRateWithMods(mods.Value)).ToFormattedDuration(); case BeatmapAttribute.RankedStatus: return beatmap.Value.BeatmapInfo.Status.GetLocalisableDescription(); case BeatmapAttribute.BPM: - return beatmap.Value.BeatmapInfo.BPM.ToLocalisableString(@"F2"); + return FormatUtils.RoundBPM(beatmap.Value.BeatmapInfo.BPM, ModUtils.CalculateRateWithMods(mods.Value)).ToLocalisableString(@"F2"); case BeatmapAttribute.CircleSize: return computeDifficulty().CircleSize.ToLocalisableString(@"F2"); From f9a9ceb41c76fd67b90a129cc97d0b4018afa2d9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 17:42:40 +0900 Subject: [PATCH 05/23] Also bind to mod setting changes --- .../Skinning/Components/BeatmapAttributeText.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index 6cfed8b945..ccee90410e 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -50,6 +50,7 @@ namespace osu.Game.Skinning.Components private readonly OsuSpriteText text; private IBindable? difficultyBindable; private CancellationTokenSource? difficultyCancellationSource; + private ModSettingChangeTracker? modSettingTracker; public BeatmapAttributeText() { @@ -84,7 +85,17 @@ namespace osu.Game.Skinning.Components updateText(); }, true); - mods.BindValueChanged(_ => updateText()); + mods.BindValueChanged(m => + { + modSettingTracker?.Dispose(); + modSettingTracker = new ModSettingChangeTracker(m.NewValue) + { + SettingChanged = _ => updateText() + }; + + updateText(); + }, true); + ruleset.BindValueChanged(_ => updateText()); updateText(); From b40c31af3adc922d2c47ecd0f810e179aece085f Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 20:36:15 +0900 Subject: [PATCH 06/23] Store difficulty to reduce flickering The computation usually finishes in a few milliseconds anyway. --- osu.Game/Skinning/Components/BeatmapAttributeText.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index ccee90410e..316b20035c 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -51,6 +51,7 @@ namespace osu.Game.Skinning.Components private IBindable? difficultyBindable; private CancellationTokenSource? difficultyCancellationSource; private ModSettingChangeTracker? modSettingTracker; + private StarDifficulty? starDifficulty; public BeatmapAttributeText() { @@ -80,7 +81,11 @@ namespace osu.Game.Skinning.Components difficultyBindable?.UnbindAll(); difficultyBindable = difficultyCache.GetBindableDifficulty(b.NewValue.BeatmapInfo, difficultyCancellationSource.Token); - difficultyBindable.BindValueChanged(_ => updateText()); + difficultyBindable.BindValueChanged(d => + { + starDifficulty = d.NewValue; + updateText(); + }); updateText(); }, true); @@ -213,9 +218,7 @@ namespace osu.Game.Skinning.Components return computeDifficulty().ApproachRate.ToLocalisableString(@"F2"); case BeatmapAttribute.StarRating: - return difficultyBindable?.Value is StarDifficulty starDifficulty - ? starDifficulty.Stars.ToLocalisableString(@"F2") - : @"..."; + return (starDifficulty?.Stars ?? 0).ToLocalisableString(@"F2"); default: throw new ArgumentOutOfRangeException(); From c0eda3606cb6be03938f3df155e3648e8f46f7bc Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Oct 2024 22:01:55 +0900 Subject: [PATCH 07/23] Add mod-related tests --- .../TestSceneBeatmapAttributeText.cs | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs index bf959d9862..01659a2654 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs @@ -1,14 +1,28 @@ // 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 Newtonsoft.Json; using NUnit.Framework; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; +using osu.Framework.Localisation; using osu.Framework.Testing; using osu.Game.Beatmaps; +using osu.Game.Configuration; using osu.Game.Models; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Difficulty; +using osu.Game.Rulesets.Difficulty.Preprocessing; +using osu.Game.Rulesets.Difficulty.Skills; +using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Rulesets.UI; +using osu.Game.Scoring; using osu.Game.Skinning.Components; using osu.Game.Tests.Beatmaps; @@ -30,6 +44,8 @@ namespace osu.Game.Tests.Visual.UserInterface [SetUp] public void Setup() => Schedule(() => { + SelectedMods.SetDefault(); + Ruleset.SetDefault(); Beatmap.Value = CreateWorkingBeatmap(new TestBeatmap(new OsuRuleset().RulesetInfo) { BeatmapInfo = @@ -93,6 +109,126 @@ namespace osu.Game.Tests.Visual.UserInterface AddAssert("check new title", getText, () => Is.EqualTo("Title: Another")); } + [Test] + public void TestWithMods() + { + AddStep("set beatmap", () => Beatmap.Value = CreateWorkingBeatmap(new TestBeatmap(new OsuRuleset().RulesetInfo) + { + BeatmapInfo = + { + BPM = 100, + Length = 30000, + Difficulty = + { + ApproachRate = 10, + CircleSize = 9 + } + } + })); + + test(BeatmapAttribute.BPM, new OsuModDoubleTime(), "BPM: 100.00", "BPM: 150.00"); + test(BeatmapAttribute.Length, new OsuModDoubleTime(), "Length: 00:30", "Length: 00:20"); + test(BeatmapAttribute.ApproachRate, new OsuModDoubleTime(), "Approach Rate: 10.00", "Approach Rate: 11.00"); + test(BeatmapAttribute.CircleSize, new OsuModHardRock(), "Circle Size: 9.00", "Circle Size: 10.00"); + + void test(BeatmapAttribute attribute, Mod mod, string before, string after) + { + AddStep($"set attribute: {attribute}", () => text.Attribute.Value = attribute); + AddAssert("check text is correct", getText, () => Is.EqualTo(before)); + + AddStep("add DT mod", () => SelectedMods.Value = new[] { mod }); + AddAssert("check text is correct", getText, () => Is.EqualTo(after)); + AddStep("clear mods", () => SelectedMods.SetDefault()); + } + } + + [Test] + public void TestStarRating() + { + AddStep("set test ruleset", () => Ruleset.Value = new TestRuleset().RulesetInfo); + AddStep("set star rating attribute", () => text.Attribute.Value = BeatmapAttribute.StarRating); + AddAssert("check star rating is 0", getText, () => Is.EqualTo("Star Rating: 0.00")); + + // Adding mod + TestMod mod = null!; + AddStep("add mod with difficulty 1", () => SelectedMods.Value = new[] { mod = new TestMod { Difficulty = { Value = 1 } } }); + AddUntilStep("check star rating is 1", getText, () => Is.EqualTo("Star Rating: 1.00")); + + // Changing mod setting + AddStep("change mod difficulty to 2", () => mod.Difficulty.Value = 2); + AddUntilStep("check star rating is 2", getText, () => Is.EqualTo("Star Rating: 2.00")); + } + private string getText() => text.ChildrenOfType().Single().Text.ToString(); + + private class TestRuleset : Ruleset + { + public override IEnumerable GetModsFor(ModType type) => new[] + { + new TestMod() + }; + + public override IBeatmapConverter CreateBeatmapConverter(IBeatmap beatmap) + => new OsuRuleset().CreateBeatmapConverter(beatmap); + + public override DifficultyCalculator CreateDifficultyCalculator(IWorkingBeatmap beatmap) + => new TestDifficultyCalculator(new TestRuleset().RulesetInfo, beatmap); + + public override PerformanceCalculator CreatePerformanceCalculator() + => new TestPerformanceCalculator(new TestRuleset()); + + public override DrawableRuleset CreateDrawableRulesetWith(IBeatmap beatmap, IReadOnlyList? mods = null) + => null!; + + public override string Description => string.Empty; + public override string ShortName => string.Empty; + } + + private class TestDifficultyCalculator : DifficultyCalculator + { + public TestDifficultyCalculator(IRulesetInfo ruleset, IWorkingBeatmap beatmap) + : base(ruleset, beatmap) + { + } + + protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beatmap, Mod[] mods, Skill[] skills, double clockRate) + => new DifficultyAttributes(mods, mods.OfType().SingleOrDefault()?.Difficulty.Value ?? 0); + + protected override IEnumerable CreateDifficultyHitObjects(IBeatmap beatmap, double clockRate) + => Array.Empty(); + + protected override Skill[] CreateSkills(IBeatmap beatmap, Mod[] mods, double clockRate) + => Array.Empty(); + } + + private class TestPerformanceCalculator : PerformanceCalculator + { + public TestPerformanceCalculator(Ruleset ruleset) + : base(ruleset) + { + } + + protected override PerformanceAttributes CreatePerformanceAttributes(ScoreInfo score, DifficultyAttributes attributes) + => new PerformanceAttributes { Total = score.Mods.OfType().SingleOrDefault()?.Performance.Value ?? 0 }; + } + + private class TestMod : Mod + { + [SettingSource("difficulty")] + public BindableDouble Difficulty { get; } = new BindableDouble(0); + + [SettingSource("performance")] + public BindableDouble Performance { get; } = new BindableDouble(0); + + [JsonConstructor] + public TestMod() + { + } + + public override string Name => string.Empty; + public override LocalisableString Description => string.Empty; + public override double ScoreMultiplier => 1.0; + public override string Acronym => "Test"; + } } } From 17702ead0bcde62988b2146cfd8db671b64da09d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 22 Oct 2024 14:20:10 +0900 Subject: [PATCH 08/23] Fix ruleset not being reset correctly in tests --- .../Visual/UserInterface/TestSceneBeatmapAttributeText.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs index 01659a2654..91525e02c1 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneBeatmapAttributeText.cs @@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.UserInterface public void Setup() => Schedule(() => { SelectedMods.SetDefault(); - Ruleset.SetDefault(); + Ruleset.Value = new OsuRuleset().RulesetInfo; Beatmap.Value = CreateWorkingBeatmap(new TestBeatmap(new OsuRuleset().RulesetInfo) { BeatmapInfo = From 5e642cbce72862b29b117829e7ec90d6d6b0ce1e Mon Sep 17 00:00:00 2001 From: Darius Wattimena Date: Thu, 24 Oct 2024 23:17:47 +0200 Subject: [PATCH 09/23] Apply code feedback and also resize catcher trails when any is shown --- .../Edit/CatchEditorPlayfield.cs | 5 ----- .../Edit/DrawableCatchEditorRuleset.cs | 9 ++++++++- osu.Game.Rulesets.Catch/UI/Catcher.cs | 6 +++--- osu.Game.Rulesets.Catch/UI/CatcherArea.cs | 8 ++++---- .../UI/CatcherTrailDisplay.cs | 20 +++++++++++++++++++ 5 files changed, 35 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs b/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs index 78f5c3e9ed..3967c54f4b 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchEditorPlayfield.cs @@ -22,10 +22,5 @@ namespace osu.Game.Rulesets.Catch.Edit // TODO: disable hit lighting as well } - - public void ApplyCircleSizeToCatcher(IBeatmapDifficultyInfo difficulty) - { - Catcher.SetScaleAndCatchWidth(difficulty); - } } } diff --git a/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs b/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs index cd6d490a7f..86cdc2df5c 100644 --- a/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs +++ b/osu.Game.Rulesets.Catch/Edit/DrawableCatchEditorRuleset.cs @@ -49,7 +49,14 @@ namespace osu.Game.Rulesets.Catch.Edit editorBeatmap.BeatmapReprocessed -= onBeatmapReprocessed; } - private void onBeatmapReprocessed() => (Playfield as CatchEditorPlayfield)?.ApplyCircleSizeToCatcher(editorBeatmap.Difficulty); + private void onBeatmapReprocessed() + { + if (Playfield is CatchEditorPlayfield catchPlayfield) + { + catchPlayfield.Catcher.ApplyDifficulty(editorBeatmap.Difficulty); + catchPlayfield.CatcherArea.CatcherTrails.UpdateCatcherTrailsScale(catchPlayfield.Catcher.BodyScale); + } + } protected override Playfield CreatePlayfield() => new CatchEditorPlayfield(Beatmap.Difficulty); diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 3a0863473a..ebf3e47fd7 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -116,7 +116,7 @@ namespace osu.Game.Rulesets.Catch.UI /// /// Width of the area that can be used to attempt catches during gameplay. /// - public float CatchWidth; + public float CatchWidth { get; private set; } private readonly SkinnableCatcher body; @@ -142,7 +142,7 @@ namespace osu.Game.Rulesets.Catch.UI Size = new Vector2(BASE_SIZE); - SetScaleAndCatchWidth(difficulty); + ApplyDifficulty(difficulty); InternalChildren = new Drawable[] { @@ -312,7 +312,7 @@ namespace osu.Game.Rulesets.Catch.UI /// /// Set the scale and catch width. /// - public void SetScaleAndCatchWidth(IBeatmapDifficultyInfo? difficulty) + public void ApplyDifficulty(IBeatmapDifficultyInfo? difficulty) { if (difficulty != null) Scale = calculateScale(difficulty); diff --git a/osu.Game.Rulesets.Catch/UI/CatcherArea.cs b/osu.Game.Rulesets.Catch/UI/CatcherArea.cs index 338e1364a9..7f0a79ef49 100644 --- a/osu.Game.Rulesets.Catch/UI/CatcherArea.cs +++ b/osu.Game.Rulesets.Catch/UI/CatcherArea.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Catch.UI private readonly CatchComboDisplay comboDisplay; - private readonly CatcherTrailDisplay catcherTrails; + public readonly CatcherTrailDisplay CatcherTrails; private Catcher catcher = null!; @@ -55,7 +55,7 @@ namespace osu.Game.Rulesets.Catch.UI Children = new Drawable[] { catcherContainer = new Container { RelativeSizeAxes = Axes.Both }, - catcherTrails = new CatcherTrailDisplay(), + CatcherTrails = new CatcherTrailDisplay(), comboDisplay = new CatchComboDisplay { RelativeSizeAxes = Axes.None, @@ -112,7 +112,7 @@ namespace osu.Game.Rulesets.Catch.UI { const double trail_generation_interval = 16; - if (Time.Current - catcherTrails.LastDashTrailTime >= trail_generation_interval) + if (Time.Current - CatcherTrails.LastDashTrailTime >= trail_generation_interval) displayCatcherTrail(Catcher.HyperDashing ? CatcherTrailAnimation.HyperDashing : CatcherTrailAnimation.Dashing); } @@ -170,6 +170,6 @@ namespace osu.Game.Rulesets.Catch.UI } } - private void displayCatcherTrail(CatcherTrailAnimation animation) => catcherTrails.Add(new CatcherTrailEntry(Time.Current, Catcher.CurrentState, Catcher.X, Catcher.BodyScale, animation)); + private void displayCatcherTrail(CatcherTrailAnimation animation) => CatcherTrails.Add(new CatcherTrailEntry(Time.Current, Catcher.CurrentState, Catcher.X, Catcher.BodyScale, animation)); } } diff --git a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs index e3e01c1b39..6a9162da91 100644 --- a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs +++ b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs @@ -10,6 +10,7 @@ using osu.Framework.Graphics.Pooling; using osu.Game.Rulesets.Catch.Skinning; using osu.Game.Rulesets.Objects.Pooling; using osu.Game.Skinning; +using osuTK; using osuTK.Graphics; namespace osu.Game.Rulesets.Catch.UI @@ -55,6 +56,25 @@ namespace osu.Game.Rulesets.Catch.UI }; } + /// + /// Update the scale of all trails. + /// + /// The new body scale of the Catcher + public void UpdateCatcherTrailsScale(Vector2 scale) + { + applyScaleChange(scale, dashTrails); + applyScaleChange(scale, hyperDashTrails); + applyScaleChange(scale, hyperDashAfterImages); + } + + private void applyScaleChange(Vector2 scale, Container trails) + { + foreach (var trail in trails) + { + trail.Scale = scale; + } + } + protected override void LoadComplete() { base.LoadComplete(); From 2b14d78f04083afb3f8e067ce00554fdfdc5e41f Mon Sep 17 00:00:00 2001 From: Darius Wattimena Date: Thu, 24 Oct 2024 23:40:23 +0200 Subject: [PATCH 10/23] Hide the after image instead as the effect can't be resized easily --- osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs index 6a9162da91..ad2ae53f48 100644 --- a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs +++ b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs @@ -64,7 +64,12 @@ namespace osu.Game.Rulesets.Catch.UI { applyScaleChange(scale, dashTrails); applyScaleChange(scale, hyperDashTrails); - applyScaleChange(scale, hyperDashAfterImages); + + foreach (var afterImage in hyperDashAfterImages) + { + afterImage.Hide(); + afterImage.Expire(); + } } private void applyScaleChange(Vector2 scale, Container trails) From 74dc0dc4806c22fdf353f8297f3bbd874247c7a9 Mon Sep 17 00:00:00 2001 From: Joseph Madamba Date: Tue, 29 Oct 2024 20:21:26 -0700 Subject: [PATCH 11/23] Fix editor sidebar resizing on hover repeatedly when polygon popover is opened --- osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs b/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs index 6325de5851..a2ee4a888d 100644 --- a/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs +++ b/osu.Game.Rulesets.Osu/Edit/PolygonGenerationPopover.cs @@ -53,6 +53,8 @@ namespace osu.Game.Rulesets.Osu.Edit [BackgroundDependencyLoader] private void load() { + AllowableAnchors = new[] { Anchor.CentreLeft, Anchor.CentreRight }; + Child = new FillFlowContainer { Width = 220, From 40c2d4e942453b583ff3dc41368992b347a474dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 07:35:00 +0100 Subject: [PATCH 12/23] Adjust test to match desired reality --- osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs index 11c4c54ea6..33d1e5c91e 100644 --- a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs +++ b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs @@ -241,8 +241,8 @@ namespace osu.Game.Tests.Beatmaps metadataLookup.Update(beatmapSet, preferOnlineFetch); - Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.None)); - Assert.That(beatmap.OnlineID, Is.EqualTo(-1)); + Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Ranked)); + Assert.That(beatmap.OnlineID, Is.EqualTo(654321)); } [Test] From 1a2e323c11158aab0433f144d1595e06f7a1fe20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 07:40:08 +0100 Subject: [PATCH 13/23] Remove problematic online ID check --- osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs index 034ec31ee4..42d8e07432 100644 --- a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs +++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs @@ -85,11 +85,11 @@ namespace osu.Game.Beatmaps private bool shouldDiscardLookupResult(OnlineBeatmapMetadata result, BeatmapInfo beatmapInfo) { - if (beatmapInfo.OnlineID > 0 && result.BeatmapID != beatmapInfo.OnlineID) - { - Logger.Log($"Discarding metadata lookup result due to mismatching online ID (expected: {beatmapInfo.OnlineID} actual: {result.BeatmapID})", LoggingTarget.Database); - return true; - } + // previously this used to check whether the `OnlineID` of the looked-up beatmap matches the local `OnlineID`. + // unfortunately it appears that historically stable mappers would apply crude hacks to fix unspecified "issues" with submission + // which would amount to reusing online IDs of other beatmaps. + // this means that the online ID in the `.osu` file is not reliable, and this cannot be fixed server-side + // because updating the maps retroactively would break stable (by losing all of users' local scores). if (beatmapInfo.OnlineID == -1 && result.MD5Hash != beatmapInfo.MD5Hash) { From 776fabd77c5a6225e69a0b14e79b9e097692320c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 08:00:57 +0100 Subject: [PATCH 14/23] Only use MD5 when performing metadata lookups Both online and offline using the cache. The rationale behind this change is that in the current state of affairs, `TestPartiallyMaliciousSet()` fails in a way that cannot be reconciled without this sort of change. The test exercises a scenario where the beatmap being imported has an online ID in the `.osu` file, but its hash does not match the online hash of the beatmap. This turns out to be a more frequent scenario than envisioned because of users doing stupid things with manual file editing rather than reporting issues properly. The scenario is realistic only because the behaviour of the endpoint responsible for looking up beatmaps is such that if multiple parameters are given (e.g. all three of beatmap MD5, online ID, and filename), it will try the three in succession: https://github.com/ppy/osu-web/blob/f6b341813be270de59ec8f9698428aa6422bc8ae/app/Http/Controllers/BeatmapsController.php#L260-L266 and the local metadata cache implementation reflected this implementation. Because online ID and filename are inherently unreliable in this scenario due to being directly manipulable by clueless or malicious users, neither should not be used as a fallback. --- osu.Game/Beatmaps/APIBeatmapMetadataSource.cs | 2 +- .../LocalCachedBeatmapMetadataSource.cs | 12 +++------- .../Online/API/Requests/GetBeatmapRequest.cs | 22 +++++++++++++------ .../OnlinePlay/TestRoomRequestsHandler.cs | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs b/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs index a2eebe6161..9c292a4cfb 100644 --- a/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs +++ b/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs @@ -33,7 +33,7 @@ namespace osu.Game.Beatmaps Debug.Assert(beatmapInfo.BeatmapSet != null); - var req = new GetBeatmapRequest(beatmapInfo); + var req = new GetBeatmapRequest(beatmapInfo.MD5Hash); try { diff --git a/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs b/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs index eaa4d8ebfb..39afe5f519 100644 --- a/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs +++ b/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs @@ -89,9 +89,7 @@ namespace osu.Game.Beatmaps return false; } - if (string.IsNullOrEmpty(beatmapInfo.MD5Hash) - && string.IsNullOrEmpty(beatmapInfo.Path) - && beatmapInfo.OnlineID <= 0) + if (string.IsNullOrEmpty(beatmapInfo.MD5Hash)) { onlineMetadata = null; return false; @@ -240,11 +238,9 @@ namespace osu.Game.Beatmaps using var cmd = db.CreateCommand(); cmd.CommandText = - @"SELECT beatmapset_id, beatmap_id, approved, user_id, checksum, last_update FROM osu_beatmaps WHERE checksum = @MD5Hash OR beatmap_id = @OnlineID OR filename = @Path"; + @"SELECT beatmapset_id, beatmap_id, approved, user_id, checksum, last_update FROM osu_beatmaps WHERE checksum = @MD5Hash"; cmd.Parameters.Add(new SqliteParameter(@"@MD5Hash", beatmapInfo.MD5Hash)); - cmd.Parameters.Add(new SqliteParameter(@"@OnlineID", beatmapInfo.OnlineID)); - cmd.Parameters.Add(new SqliteParameter(@"@Path", beatmapInfo.Path)); using var reader = cmd.ExecuteReader(); @@ -281,12 +277,10 @@ namespace osu.Game.Beatmaps SELECT `b`.`beatmapset_id`, `b`.`beatmap_id`, `b`.`approved`, `b`.`user_id`, `b`.`checksum`, `b`.`last_update`, `s`.`submit_date`, `s`.`approved_date` FROM `osu_beatmaps` AS `b` JOIN `osu_beatmapsets` AS `s` ON `s`.`beatmapset_id` = `b`.`beatmapset_id` - WHERE `b`.`checksum` = @MD5Hash OR `b`.`beatmap_id` = @OnlineID OR `b`.`filename` = @Path + WHERE `b`.`checksum` = @MD5Hash """; cmd.Parameters.Add(new SqliteParameter(@"@MD5Hash", beatmapInfo.MD5Hash)); - cmd.Parameters.Add(new SqliteParameter(@"@OnlineID", beatmapInfo.OnlineID)); - cmd.Parameters.Add(new SqliteParameter(@"@Path", beatmapInfo.Path)); using var reader = cmd.ExecuteReader(); diff --git a/osu.Game/Online/API/Requests/GetBeatmapRequest.cs b/osu.Game/Online/API/Requests/GetBeatmapRequest.cs index 3383d21dfc..091f336b71 100644 --- a/osu.Game/Online/API/Requests/GetBeatmapRequest.cs +++ b/osu.Game/Online/API/Requests/GetBeatmapRequest.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.Globalization; using osu.Framework.IO.Network; using osu.Game.Beatmaps; using osu.Game.Online.API.Requests.Responses; @@ -9,23 +10,30 @@ namespace osu.Game.Online.API.Requests { public class GetBeatmapRequest : APIRequest { - public readonly IBeatmapInfo BeatmapInfo; - public readonly string Filename; + public readonly int OnlineID = -1; + public readonly string? MD5Hash; + public readonly string? Filename; public GetBeatmapRequest(IBeatmapInfo beatmapInfo) { - BeatmapInfo = beatmapInfo; + OnlineID = beatmapInfo.OnlineID; + MD5Hash = beatmapInfo.MD5Hash; Filename = (beatmapInfo as BeatmapInfo)?.Path ?? string.Empty; } + public GetBeatmapRequest(string md5Hash) + { + MD5Hash = md5Hash; + } + protected override WebRequest CreateWebRequest() { var request = base.CreateWebRequest(); - if (BeatmapInfo.OnlineID > 0) - request.AddParameter(@"id", BeatmapInfo.OnlineID.ToString()); - if (!string.IsNullOrEmpty(BeatmapInfo.MD5Hash)) - request.AddParameter(@"checksum", BeatmapInfo.MD5Hash); + if (OnlineID > 0) + request.AddParameter(@"id", OnlineID.ToString(CultureInfo.InvariantCulture)); + if (!string.IsNullOrEmpty(MD5Hash)) + request.AddParameter(@"checksum", MD5Hash); if (!string.IsNullOrEmpty(Filename)) request.AddParameter(@"filename", Filename); diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs index 592e4c6eee..ec89f81597 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs @@ -188,7 +188,7 @@ namespace osu.Game.Tests.Visual.OnlinePlay case GetBeatmapRequest getBeatmapRequest: { - getBeatmapRequest.TriggerSuccess(createResponseBeatmaps(getBeatmapRequest.BeatmapInfo.OnlineID).Single()); + getBeatmapRequest.TriggerSuccess(createResponseBeatmaps(getBeatmapRequest.OnlineID).Single()); return true; } From 2c2f307a63c1c4eee8ccbc0bc5d339ccaaf308af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 08:01:57 +0100 Subject: [PATCH 15/23] Remove no longer applicable test After dd06dd0e699311494412e36bc3f37bb055a01477 the behaviour set up on the mock in the test in question is no longer realistic. Online metadata lookups will no longer fall back to online ID or filename. --- .../BeatmapUpdaterMetadataLookupTest.cs | 53 ------------------- 1 file changed, 53 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs index 33d1e5c91e..2e1bb3a1c6 100644 --- a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs +++ b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs @@ -383,58 +383,5 @@ namespace osu.Game.Tests.Beatmaps Assert.That(beatmapSet.Status, Is.EqualTo(BeatmapOnlineStatus.None)); } - - [Test] - public void TestPartiallyMaliciousSet([Values] bool preferOnlineFetch) - { - var firstResult = new OnlineBeatmapMetadata - { - BeatmapID = 654321, - BeatmapStatus = BeatmapOnlineStatus.Ranked, - BeatmapSetStatus = BeatmapOnlineStatus.Ranked, - MD5Hash = @"cafebabe" - }; - var secondResult = new OnlineBeatmapMetadata - { - BeatmapStatus = BeatmapOnlineStatus.Ranked, - BeatmapSetStatus = BeatmapOnlineStatus.Ranked, - MD5Hash = @"dededede" - }; - - var targetMock = preferOnlineFetch ? apiMetadataSourceMock : localCachedMetadataSourceMock; - targetMock.Setup(src => src.Available).Returns(true); - targetMock.Setup(src => src.TryLookup(It.Is(bi => bi.OnlineID == 654321), out firstResult)) - .Returns(true); - targetMock.Setup(src => src.TryLookup(It.Is(bi => bi.OnlineID == 666666), out secondResult)) - .Returns(true); - - var firstBeatmap = new BeatmapInfo - { - OnlineID = 654321, - MD5Hash = @"cafebabe", - }; - var secondBeatmap = new BeatmapInfo - { - OnlineID = 666666, - MD5Hash = @"deadbeef" - }; - var beatmapSet = new BeatmapSetInfo(new[] - { - firstBeatmap, - secondBeatmap - }); - firstBeatmap.BeatmapSet = beatmapSet; - secondBeatmap.BeatmapSet = beatmapSet; - - metadataLookup.Update(beatmapSet, preferOnlineFetch); - - Assert.That(firstBeatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Ranked)); - Assert.That(firstBeatmap.OnlineID, Is.EqualTo(654321)); - - Assert.That(secondBeatmap.Status, Is.EqualTo(BeatmapOnlineStatus.None)); - Assert.That(secondBeatmap.OnlineID, Is.EqualTo(-1)); - - Assert.That(beatmapSet.Status, Is.EqualTo(BeatmapOnlineStatus.None)); - } } } From 0e52797f2948aa60c6b375f80239c091d1a3fb7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 08:07:19 +0100 Subject: [PATCH 16/23] Prefer not deleted models when picking model instances for reuse when importing This fell out while investigating why the issue with online IDs mismatching in the `.osu` could be worked around by importing the map three times in total when starting from it not being available locally. Here follows an explanation of why that "helped". Import 1: - The beatmap set is imported normally. - Online metadata population sees the online ID mismatch and resets it on the problematic beatmap. Import 2: - The existing beatmap set is found, but deemed not reusable because of the single beatmap having its ID reset to -1. - The existing beatmap set is marked deleted, and all the IDs of its beatmaps are reset to -1. - The beatmap set is reimported afresh. - Online metadata population still sees the online ID mismatch and resets it on the problematic beatmap. Note that at this point the first import *is still physically present in the database* but marked deleted. Import 3: - When trying to find the existing beatmap set to see if it can be reused, *the one pending deletion and with its IDs reset - - the remnant from import 1 - is returned*. - Because of this, `validateOnlineIds()` resets online IDs *on the model representing the current reimport*. - The beatmap set is reimported yet again. - With the online ID reset, the online metadata population check for online ID mismatch does not run because *the IDs were reset to -1* earlier. Preferring undeleted models when picking the model instance for reuse prevents this scenario. --- osu.Game/Database/RealmArchiveModelImporter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index cf0625c51c..75462797f8 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -528,7 +528,7 @@ namespace osu.Game.Database /// The new model proposed for import. /// The current realm context. /// An existing model which matches the criteria to skip importing, else null. - protected TModel? CheckForExisting(TModel model, Realm realm) => string.IsNullOrEmpty(model.Hash) ? null : realm.All().FirstOrDefault(b => b.Hash == model.Hash); + protected TModel? CheckForExisting(TModel model, Realm realm) => string.IsNullOrEmpty(model.Hash) ? null : realm.All().OrderBy(b => b.DeletePending).FirstOrDefault(b => b.Hash == model.Hash); /// /// Whether import can be skipped after finding an existing import early in the process. From 2b0fd3558f2f7f1c6a79dbba62ab80f40d656ca9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 08:44:23 +0100 Subject: [PATCH 17/23] Remove more no-longer-required checks The scenario that remaining guard was trying to protect against is obviated by and no longer possible after 776fabd77c5a6225e69a0b14e79b9e097692320c. --- .../BeatmapUpdaterMetadataLookupTest.cs | 28 ------------------- .../Beatmaps/BeatmapUpdaterMetadataLookup.cs | 24 ++++------------ 2 files changed, 5 insertions(+), 47 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs index 2e1bb3a1c6..82e54875ef 100644 --- a/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs +++ b/osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs @@ -273,34 +273,6 @@ namespace osu.Game.Tests.Beatmaps Assert.That(beatmap.OnlineID, Is.EqualTo(654321)); } - [Test] - public void TestMetadataLookupForBeatmapWithoutPopulatedIDAndIncorrectHash([Values] bool preferOnlineFetch) - { - var lookupResult = new OnlineBeatmapMetadata - { - BeatmapID = 654321, - BeatmapStatus = BeatmapOnlineStatus.Ranked, - MD5Hash = @"cafebabe", - }; - - var targetMock = preferOnlineFetch ? apiMetadataSourceMock : localCachedMetadataSourceMock; - targetMock.Setup(src => src.Available).Returns(true); - targetMock.Setup(src => src.TryLookup(It.IsAny(), out lookupResult)) - .Returns(true); - - var beatmap = new BeatmapInfo - { - MD5Hash = @"deadbeef" - }; - var beatmapSet = new BeatmapSetInfo(beatmap.Yield()); - beatmap.BeatmapSet = beatmapSet; - - metadataLookup.Update(beatmapSet, preferOnlineFetch); - - Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.None)); - Assert.That(beatmap.OnlineID, Is.EqualTo(-1)); - } - [Test] public void TestReturnedMetadataHasDifferentHash([Values] bool preferOnlineFetch) { diff --git a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs index 42d8e07432..dd17ee784b 100644 --- a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs +++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using osu.Framework.Logging; using osu.Framework.Platform; using osu.Game.Online.API; @@ -44,10 +43,14 @@ namespace osu.Game.Beatmaps foreach (var beatmapInfo in beatmapSet.Beatmaps) { + // note that it is KEY that the lookup here only uses MD5 inside + // (see implementations of underlying `IOnlineBeatmapMetadataSource`s). + // anything else like online ID or filename can be manipulated, thus being inherently unreliable, + // thus being unusable here for any purpose. if (!tryLookup(beatmapInfo, preferOnlineFetch, out var res)) continue; - if (res == null || shouldDiscardLookupResult(res, beatmapInfo)) + if (res == null) { beatmapInfo.ResetOnlineInfo(); lookupResults.Add(null); // mark lookup failure @@ -83,23 +86,6 @@ namespace osu.Game.Beatmaps } } - private bool shouldDiscardLookupResult(OnlineBeatmapMetadata result, BeatmapInfo beatmapInfo) - { - // previously this used to check whether the `OnlineID` of the looked-up beatmap matches the local `OnlineID`. - // unfortunately it appears that historically stable mappers would apply crude hacks to fix unspecified "issues" with submission - // which would amount to reusing online IDs of other beatmaps. - // this means that the online ID in the `.osu` file is not reliable, and this cannot be fixed server-side - // because updating the maps retroactively would break stable (by losing all of users' local scores). - - if (beatmapInfo.OnlineID == -1 && result.MD5Hash != beatmapInfo.MD5Hash) - { - Logger.Log($"Discarding metadata lookup result due to mismatching hash (expected: {beatmapInfo.MD5Hash} actual: {result.MD5Hash})", LoggingTarget.Database); - return true; - } - - return false; - } - /// /// Attempts to retrieve the for the given . /// From 7e3564cb4aa92c6206e7840dcd13f6d2e73e8b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 30 Oct 2024 10:23:57 +0100 Subject: [PATCH 18/23] Bring back matching by filename when performing online metadata lookups --- osu.Game/Beatmaps/APIBeatmapMetadataSource.cs | 2 +- osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs | 13 +++++++++---- .../Beatmaps/LocalCachedBeatmapMetadataSource.cs | 9 ++++++--- osu.Game/Online/API/Requests/GetBeatmapRequest.cs | 10 +++++----- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs b/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs index 9c292a4cfb..34eedfb474 100644 --- a/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs +++ b/osu.Game/Beatmaps/APIBeatmapMetadataSource.cs @@ -33,7 +33,7 @@ namespace osu.Game.Beatmaps Debug.Assert(beatmapInfo.BeatmapSet != null); - var req = new GetBeatmapRequest(beatmapInfo.MD5Hash); + var req = new GetBeatmapRequest(md5Hash: beatmapInfo.MD5Hash, filename: beatmapInfo.Path); try { diff --git a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs index dd17ee784b..364a0f9b4b 100644 --- a/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs +++ b/osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs @@ -43,10 +43,15 @@ namespace osu.Game.Beatmaps foreach (var beatmapInfo in beatmapSet.Beatmaps) { - // note that it is KEY that the lookup here only uses MD5 inside - // (see implementations of underlying `IOnlineBeatmapMetadataSource`s). - // anything else like online ID or filename can be manipulated, thus being inherently unreliable, - // thus being unusable here for any purpose. + // note that these lookups DO NOT ACTUALLY FULLY GUARANTEE that the beatmap is what it claims it is, + // i.e. the correctness of this lookup should be treated as APPROXIMATE AT WORST. + // this is because the beatmap filename is used as a fallback in some scenarios where the MD5 of the beatmap may mismatch. + // this is considered to be an acceptable casualty so that things can continue to work as expected for users in some rare scenarios + // (stale beatmap files in beatmap packs, beatmap mirror desyncs). + // however, all this means that other places such as score submission ARE EXPECTED TO VERIFY THE MD5 OF THE BEATMAP AGAINST THE ONLINE ONE EXPLICITLY AGAIN. + // + // additionally note that the online ID stored to the map is EXPLICITLY NOT USED because some users in a silly attempt to "fix" things for themselves on stable + // would reuse online IDs of already submitted beatmaps, which means that information is pretty much expected to be bogus in a nonzero number of beatmapsets. if (!tryLookup(beatmapInfo, preferOnlineFetch, out var res)) continue; diff --git a/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs b/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs index 39afe5f519..66fad6c8d8 100644 --- a/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs +++ b/osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs @@ -89,7 +89,8 @@ namespace osu.Game.Beatmaps return false; } - if (string.IsNullOrEmpty(beatmapInfo.MD5Hash)) + if (string.IsNullOrEmpty(beatmapInfo.MD5Hash) + && string.IsNullOrEmpty(beatmapInfo.Path)) { onlineMetadata = null; return false; @@ -238,9 +239,10 @@ namespace osu.Game.Beatmaps using var cmd = db.CreateCommand(); cmd.CommandText = - @"SELECT beatmapset_id, beatmap_id, approved, user_id, checksum, last_update FROM osu_beatmaps WHERE checksum = @MD5Hash"; + @"SELECT beatmapset_id, beatmap_id, approved, user_id, checksum, last_update FROM osu_beatmaps WHERE checksum = @MD5Hash OR filename = @Path"; cmd.Parameters.Add(new SqliteParameter(@"@MD5Hash", beatmapInfo.MD5Hash)); + cmd.Parameters.Add(new SqliteParameter(@"@Path", beatmapInfo.Path)); using var reader = cmd.ExecuteReader(); @@ -277,10 +279,11 @@ namespace osu.Game.Beatmaps SELECT `b`.`beatmapset_id`, `b`.`beatmap_id`, `b`.`approved`, `b`.`user_id`, `b`.`checksum`, `b`.`last_update`, `s`.`submit_date`, `s`.`approved_date` FROM `osu_beatmaps` AS `b` JOIN `osu_beatmapsets` AS `s` ON `s`.`beatmapset_id` = `b`.`beatmapset_id` - WHERE `b`.`checksum` = @MD5Hash + WHERE `b`.`checksum` = @MD5Hash OR `b`.`filename` = @Path """; cmd.Parameters.Add(new SqliteParameter(@"@MD5Hash", beatmapInfo.MD5Hash)); + cmd.Parameters.Add(new SqliteParameter(@"@Path", beatmapInfo.Path)); using var reader = cmd.ExecuteReader(); diff --git a/osu.Game/Online/API/Requests/GetBeatmapRequest.cs b/osu.Game/Online/API/Requests/GetBeatmapRequest.cs index 091f336b71..14bb0d3122 100644 --- a/osu.Game/Online/API/Requests/GetBeatmapRequest.cs +++ b/osu.Game/Online/API/Requests/GetBeatmapRequest.cs @@ -10,20 +10,20 @@ namespace osu.Game.Online.API.Requests { public class GetBeatmapRequest : APIRequest { - public readonly int OnlineID = -1; + public readonly int OnlineID; public readonly string? MD5Hash; public readonly string? Filename; public GetBeatmapRequest(IBeatmapInfo beatmapInfo) + : this(onlineId: beatmapInfo.OnlineID, md5Hash: beatmapInfo.MD5Hash, filename: (beatmapInfo as BeatmapInfo)?.Path) { - OnlineID = beatmapInfo.OnlineID; - MD5Hash = beatmapInfo.MD5Hash; - Filename = (beatmapInfo as BeatmapInfo)?.Path ?? string.Empty; } - public GetBeatmapRequest(string md5Hash) + public GetBeatmapRequest(int onlineId = -1, string? md5Hash = null, string? filename = null) { + OnlineID = onlineId; MD5Hash = md5Hash; + Filename = filename; } protected override WebRequest CreateWebRequest() From c1a40388ff3cff1aef4d8ea054cc88a94144b8a0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 30 Oct 2024 23:47:56 +0900 Subject: [PATCH 19/23] Cap effective miss count to total hits --- osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs b/osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs index ddabf866ff..43ae95c75e 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/OsuPerformanceCalculator.cs @@ -87,6 +87,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty } effectiveMissCount = Math.Max(countMiss, effectiveMissCount); + effectiveMissCount = Math.Min(totalHits, effectiveMissCount); double multiplier = PERFORMANCE_BASE_MULTIPLIER; From 372162de5dbdaea57697bfb5cfdff039efcb5c71 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 31 Oct 2024 08:55:40 +0900 Subject: [PATCH 20/23] Ignore casing when matching mods acronyms --- osu.Game/Rulesets/Ruleset.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Ruleset.cs b/osu.Game/Rulesets/Ruleset.cs index d4989642c2..bd1f273b49 100644 --- a/osu.Game/Rulesets/Ruleset.cs +++ b/osu.Game/Rulesets/Ruleset.cs @@ -101,7 +101,7 @@ namespace osu.Game.Rulesets /// The acronym to query for . public Mod? CreateModFromAcronym(string acronym) { - return AllMods.FirstOrDefault(m => m.Acronym == acronym)?.CreateInstance(); + return AllMods.FirstOrDefault(m => string.Equals(m.Acronym, acronym, StringComparison.OrdinalIgnoreCase))?.CreateInstance(); } /// From 4a26989084cfdb0634c2029225f116878d6e8074 Mon Sep 17 00:00:00 2001 From: Joseph Madamba Date: Wed, 30 Oct 2024 17:43:39 -0700 Subject: [PATCH 21/23] Fix android screen orientation locking in portrait mode during gameplay when exiting/re-entering app --- osu.Android/GameplayScreenRotationLocker.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Android/GameplayScreenRotationLocker.cs b/osu.Android/GameplayScreenRotationLocker.cs index 26234545ef..42583b5dc2 100644 --- a/osu.Android/GameplayScreenRotationLocker.cs +++ b/osu.Android/GameplayScreenRotationLocker.cs @@ -5,7 +5,6 @@ using Android.Content.PM; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Game; using osu.Game.Screens.Play; namespace osu.Android @@ -28,7 +27,7 @@ namespace osu.Android { gameActivity.RunOnUiThread(() => { - gameActivity.RequestedOrientation = userPlaying.NewValue != LocalUserPlayingState.NotPlaying ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; + gameActivity.RequestedOrientation = userPlaying.NewValue == LocalUserPlayingState.Playing ? ScreenOrientation.Locked : gameActivity.DefaultOrientation; }); } } From 5f6395059807f2c0eca9c354dd30e03952bce876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 1 Nov 2024 11:26:59 +0100 Subject: [PATCH 22/23] Add missing disposal --- osu.Game/Skinning/Components/BeatmapAttributeText.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Skinning/Components/BeatmapAttributeText.cs b/osu.Game/Skinning/Components/BeatmapAttributeText.cs index 9176c42da8..4e35c90ba6 100644 --- a/osu.Game/Skinning/Components/BeatmapAttributeText.cs +++ b/osu.Game/Skinning/Components/BeatmapAttributeText.cs @@ -257,6 +257,8 @@ namespace osu.Game.Skinning.Components difficultyCancellationSource?.Cancel(); difficultyCancellationSource?.Dispose(); difficultyCancellationSource = null; + + modSettingTracker?.Dispose(); } } From 488fabcd5479ac4cdd43889ed71202eb00b75278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 1 Nov 2024 13:07:32 +0100 Subject: [PATCH 23/23] Use alternative method of resizing the catcher trails This one preserves the catcher afterimage, which I'd count as a win. --- .../UI/CatcherTrailDisplay.cs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs index ad2ae53f48..6b888be961 100644 --- a/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs +++ b/osu.Game.Rulesets.Catch/UI/CatcherTrailDisplay.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; @@ -62,21 +63,16 @@ namespace osu.Game.Rulesets.Catch.UI /// The new body scale of the Catcher public void UpdateCatcherTrailsScale(Vector2 scale) { - applyScaleChange(scale, dashTrails); - applyScaleChange(scale, hyperDashTrails); + var oldEntries = Entries.ToList(); - foreach (var afterImage in hyperDashAfterImages) - { - afterImage.Hide(); - afterImage.Expire(); - } - } + Clear(); - private void applyScaleChange(Vector2 scale, Container trails) - { - foreach (var trail in trails) + foreach (var oldEntry in oldEntries) { - trail.Scale = scale; + // use magnitude of the new scale while preserving the sign of the old one in the X direction. + // the end effect is preserving the direction in which the trail sprites face, which is important. + var targetScale = new Vector2(Math.Abs(scale.X) * Math.Sign(oldEntry.Scale.X), Math.Abs(scale.Y)); + Add(new CatcherTrailEntry(oldEntry.LifetimeStart, oldEntry.CatcherState, oldEntry.Position, targetScale, oldEntry.Animation)); } }