From df04dd21de6a16da70031ee4fac020ae0f923211 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 07:45:29 +0300 Subject: [PATCH 01/19] Add failing test case --- .../Beatmaps/IO/ImportBeatmapTest.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index c32e359de6..041df3e6ff 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -852,6 +852,39 @@ namespace osu.Game.Tests.Beatmaps.IO } } + [Test] + public async Task TestItemRemovedShouldPassConsumableBeatmapSet() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportBeatmapTest))) + { + try + { + var osu = LoadOsuIntoHost(host); + var manager = osu.Dependencies.Get(); + + var removedQueue = new Queue(); + manager.ItemRemoved.BindValueChanged(evt => + { + if (evt.NewValue.TryGetTarget(out var target)) + removedQueue.Enqueue(target); + }); + + var imported = await LoadOszIntoOsu(osu); + deleteBeatmapSet(imported, osu); + + Assert.That(removedQueue.Count, Is.EqualTo(1)); + + var removedItem = removedQueue.Single(); + Assert.That(removedItem.Metadata, Is.EqualTo(imported.Metadata)); + Assert.That(removedItem.Beatmaps, Is.EquivalentTo(imported.Beatmaps)); + } + finally + { + host.Exit(); + } + } + } + public static async Task LoadOszIntoOsu(OsuGameBase osu, string path = null, bool virtualTrack = false) { var temp = path ?? TestResources.GetTestBeatmapForImport(virtualTrack); From 738c94d1938f99ff5fb638528cff02bf80f297df Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 07:46:51 +0300 Subject: [PATCH 02/19] Update soft-deletion logic to use model store's consumable items instead --- osu.Game/Database/ArchiveModelManager.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 36cc4cce39..61b2f7668e 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -76,7 +76,7 @@ namespace osu.Game.Database protected readonly IDatabaseContextFactory ContextFactory; - protected readonly MutableDatabaseBackedStore ModelStore; + protected readonly MutableDatabaseBackedStoreWithFileIncludes ModelStore; // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private ArchiveImportIPCChannel ipc; @@ -492,7 +492,7 @@ namespace osu.Game.Database using (ContextFactory.GetForWrite()) { // re-fetch the model on the import context. - var foundModel = queryModel().Include(s => s.Files).ThenInclude(f => f.FileInfo).FirstOrDefault(s => s.ID == item.ID); + var foundModel = ModelStore.ConsumableItems.SingleOrDefault(i => i.ID == item.ID); if (foundModel == null || foundModel.DeletePending) return false; @@ -731,8 +731,6 @@ namespace osu.Game.Database yield return f.Filename; } - private DbSet queryModel() => ContextFactory.Get().Set(); - protected virtual string HumanisedModelName => $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; #region Event handling / delaying From 1463ff288629232333880a0f18888c4f1a65e5d6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 08:12:31 +0300 Subject: [PATCH 03/19] Remove unnecessary using directive --- osu.Game/Database/ArchiveModelManager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 61b2f7668e..b9f805ae31 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -9,7 +9,6 @@ using System.Threading; using System.Threading.Tasks; using Humanizer; using JetBrains.Annotations; -using Microsoft.EntityFrameworkCore; using osu.Framework; using osu.Framework.Bindables; using osu.Framework.Extensions; From ca5f2bcd4ca8395698a5267d6a8fb2a6cb214e45 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 08:49:03 +0300 Subject: [PATCH 04/19] Revert database-side changes --- .../Beatmaps/IO/ImportBeatmapTest.cs | 33 ------------------- osu.Game/Database/ArchiveModelManager.cs | 6 ++-- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 041df3e6ff..c32e359de6 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -852,39 +852,6 @@ namespace osu.Game.Tests.Beatmaps.IO } } - [Test] - public async Task TestItemRemovedShouldPassConsumableBeatmapSet() - { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportBeatmapTest))) - { - try - { - var osu = LoadOsuIntoHost(host); - var manager = osu.Dependencies.Get(); - - var removedQueue = new Queue(); - manager.ItemRemoved.BindValueChanged(evt => - { - if (evt.NewValue.TryGetTarget(out var target)) - removedQueue.Enqueue(target); - }); - - var imported = await LoadOszIntoOsu(osu); - deleteBeatmapSet(imported, osu); - - Assert.That(removedQueue.Count, Is.EqualTo(1)); - - var removedItem = removedQueue.Single(); - Assert.That(removedItem.Metadata, Is.EqualTo(imported.Metadata)); - Assert.That(removedItem.Beatmaps, Is.EquivalentTo(imported.Beatmaps)); - } - finally - { - host.Exit(); - } - } - } - public static async Task LoadOszIntoOsu(OsuGameBase osu, string path = null, bool virtualTrack = false) { var temp = path ?? TestResources.GetTestBeatmapForImport(virtualTrack); diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index b9f805ae31..81d8668196 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -75,7 +75,7 @@ namespace osu.Game.Database protected readonly IDatabaseContextFactory ContextFactory; - protected readonly MutableDatabaseBackedStoreWithFileIncludes ModelStore; + protected readonly MutableDatabaseBackedStore ModelStore; // ReSharper disable once NotAccessedField.Local (we should keep a reference to this so it is not finalised) private ArchiveImportIPCChannel ipc; @@ -491,7 +491,7 @@ namespace osu.Game.Database using (ContextFactory.GetForWrite()) { // re-fetch the model on the import context. - var foundModel = ModelStore.ConsumableItems.SingleOrDefault(i => i.ID == item.ID); + var foundModel = queryModel().Include(s => s.Files).ThenInclude(f => f.FileInfo).FirstOrDefault(s => s.ID == item.ID); if (foundModel == null || foundModel.DeletePending) return false; @@ -730,6 +730,8 @@ namespace osu.Game.Database yield return f.Filename; } + private DbSet queryModel() => ContextFactory.Get().Set(); + protected virtual string HumanisedModelName => $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; #region Event handling / delaying From 445a4bd01c70428f33bc36b603a52e80e92d0aeb Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 08:51:37 +0300 Subject: [PATCH 05/19] Re-query beatmap info on database changes --- osu.Game/Database/ArchiveModelManager.cs | 1 + .../OnlinePlay/Components/ReadyButton.cs | 34 ++++--------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 81d8668196..36cc4cce39 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using Humanizer; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore; using osu.Framework; using osu.Framework.Bindables; using osu.Framework.Extensions; diff --git a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs index 08f89d8ed8..144535ed4c 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs @@ -2,8 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Linq; -using System.Linq.Expressions; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Game.Beatmaps; @@ -41,38 +39,20 @@ namespace osu.Game.Screens.OnlinePlay.Components SelectedItem.BindValueChanged(item => updateSelectedItem(item.NewValue), true); } - private void updateSelectedItem(PlaylistItem item) - { - hasBeatmap = findBeatmap(expr => beatmaps.QueryBeatmap(expr)); - } + private void updateSelectedItem(PlaylistItem _) => updateBeatmapState(); + private void beatmapUpdated(ValueChangedEvent> _) => updateBeatmapState(); + private void beatmapRemoved(ValueChangedEvent> _) => updateBeatmapState(); - private void beatmapUpdated(ValueChangedEvent> weakSet) - { - if (weakSet.NewValue.TryGetTarget(out var set)) - { - if (findBeatmap(expr => set.Beatmaps.AsQueryable().FirstOrDefault(expr))) - Schedule(() => hasBeatmap = true); - } - } - - private void beatmapRemoved(ValueChangedEvent> weakSet) - { - if (weakSet.NewValue.TryGetTarget(out var set)) - { - if (findBeatmap(expr => set.Beatmaps.AsQueryable().FirstOrDefault(expr))) - Schedule(() => hasBeatmap = false); - } - } - - private bool findBeatmap(Func>, BeatmapInfo> expression) + private void updateBeatmapState() { int? beatmapId = SelectedItem.Value?.Beatmap.Value?.OnlineBeatmapID; string checksum = SelectedItem.Value?.Beatmap.Value?.MD5Hash; if (beatmapId == null || checksum == null) - return false; + return; - return expression(b => b.OnlineBeatmapID == beatmapId && b.MD5Hash == checksum) != null; + var databasedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == beatmapId && b.MD5Hash == checksum); + hasBeatmap = databasedBeatmap != null && !databasedBeatmap.BeatmapSet.DeletePending; } protected override void Update() From cb7df0fe1172935581909b938cab6fe987fcc5ff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Jan 2021 15:14:39 +0900 Subject: [PATCH 06/19] Add failing test for storyboard start time ordering --- .../Formats/LegacyStoryboardDecoderTest.cs | 20 +++++++++++++++++++ .../Resources/out-of-order-starttimes.osb | 6 ++++++ 2 files changed, 26 insertions(+) create mode 100644 osu.Game.Tests/Resources/out-of-order-starttimes.osb diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs index 9ebedb3c80..b36597a949 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs @@ -95,6 +95,26 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [Test] + public void TestOutOfOrderStartTimes() + { + var decoder = new LegacyStoryboardDecoder(); + + using (var resStream = TestResources.OpenResource("out-of-order-starttimes.osb")) + using (var stream = new LineBufferedReader(resStream)) + { + var storyboard = decoder.Decode(stream); + + StoryboardLayer background = storyboard.Layers.Single(l => l.Depth == 3); + Assert.AreEqual(2, background.Elements.Count); + + Assert.AreEqual(1500, background.Elements[0].StartTime); + Assert.AreEqual(1000, background.Elements[1].StartTime); + + Assert.AreEqual(1000, storyboard.FirstEventTime); + } + } + [Test] public void TestDecodeVariableWithSuffix() { diff --git a/osu.Game.Tests/Resources/out-of-order-starttimes.osb b/osu.Game.Tests/Resources/out-of-order-starttimes.osb new file mode 100644 index 0000000000..09988ff64e --- /dev/null +++ b/osu.Game.Tests/Resources/out-of-order-starttimes.osb @@ -0,0 +1,6 @@ +[Events] +//Storyboard Layer 0 (Background) +Sprite,Background,TopCentre,"img.jpg",320,240 + F,0,1500,1600,0,1 +Sprite,Background,TopCentre,"img.jpg",320,240 + F,0,1000,1100,0,1 From 20d04d69332f18b7f14eedc44fed5bdf0ae9e9a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Jan 2021 15:16:01 +0900 Subject: [PATCH 07/19] Fix Storyboard's FirstEventTime not finding the true earliest event --- .../Beatmaps/Formats/LegacyStoryboardDecoderTest.cs | 2 +- osu.Game/Screens/Play/GameplayClockContainer.cs | 4 +++- osu.Game/Storyboards/Storyboard.cs | 10 +++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs index b36597a949..7bee580863 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs @@ -111,7 +111,7 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.AreEqual(1500, background.Elements[0].StartTime); Assert.AreEqual(1000, background.Elements[1].StartTime); - Assert.AreEqual(1000, storyboard.FirstEventTime); + Assert.AreEqual(1000, storyboard.EarliestEventTime); } } diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 0248432917..ddbb087962 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -131,7 +131,9 @@ namespace osu.Game.Screens.Play // if a storyboard is present, it may dictate the appropriate start time by having events in negative time space. // this is commonly used to display an intro before the audio track start. - startTime = Math.Min(startTime, beatmap.Storyboard.FirstEventTime); + double? firstStoryboardEvent = beatmap.Storyboard.EarliestEventTime; + if (firstStoryboardEvent != null) + startTime = Math.Min(startTime, firstStoryboardEvent.Value); // some beatmaps specify a current lead-in time which should be used instead of the ruleset-provided value when available. // this is not available as an option in the live editor but can still be applied via .osu editing. diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index e0d18eab00..d4ba18d394 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.EntityFrameworkCore.Internal; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; @@ -27,7 +28,14 @@ namespace osu.Game.Storyboards public bool HasDrawable => Layers.Any(l => l.Elements.Any(e => e.IsDrawable)); - public double FirstEventTime => Layers.Min(l => l.Elements.FirstOrDefault()?.StartTime ?? 0); + /// + /// Across all layers, find the earliest point in time that a storyboard element exists at. + /// Will return null if there are no elements. + /// + /// + /// This iterates all elements and as such should be used sparingly or stored locally. + /// + public double? EarliestEventTime => Layers.SelectMany(l => l.Elements).OrderBy(e => e.StartTime).FirstOrDefault()?.StartTime; /// /// Depth of the currently front-most storyboard layer, excluding the overlay layer. From 9e0c490141e4e85ecde7c96d7a6b3f37f0b3b4fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Jan 2021 15:40:22 +0900 Subject: [PATCH 08/19] Remove unused using --- osu.Game/Storyboards/Storyboard.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Storyboards/Storyboard.cs b/osu.Game/Storyboards/Storyboard.cs index d4ba18d394..1ba25cc11e 100644 --- a/osu.Game/Storyboards/Storyboard.cs +++ b/osu.Game/Storyboards/Storyboard.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Microsoft.EntityFrameworkCore.Internal; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.Textures; From ea38b00b29adabf0ec91b0b085f2af0b11829b23 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 10:27:08 +0300 Subject: [PATCH 09/19] Schedule all calls to `updateBeatmapState()` --- osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs index 144535ed4c..6f86f2b879 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs @@ -39,9 +39,9 @@ namespace osu.Game.Screens.OnlinePlay.Components SelectedItem.BindValueChanged(item => updateSelectedItem(item.NewValue), true); } - private void updateSelectedItem(PlaylistItem _) => updateBeatmapState(); - private void beatmapUpdated(ValueChangedEvent> _) => updateBeatmapState(); - private void beatmapRemoved(ValueChangedEvent> _) => updateBeatmapState(); + private void updateSelectedItem(PlaylistItem _) => Scheduler.AddOnce(updateBeatmapState); + private void beatmapUpdated(ValueChangedEvent> _) => Scheduler.AddOnce(updateBeatmapState); + private void beatmapRemoved(ValueChangedEvent> _) => Scheduler.AddOnce(updateBeatmapState); private void updateBeatmapState() { From 485a57776b205581528925011a1d2ec77d8fc26d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Jan 2021 10:28:41 +0300 Subject: [PATCH 10/19] Fix `hasBeatmap` potentially checking on outdated `DeletePending` value --- osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs index 6f86f2b879..b782df75df 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs @@ -52,7 +52,15 @@ namespace osu.Game.Screens.OnlinePlay.Components return; var databasedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == beatmapId && b.MD5Hash == checksum); - hasBeatmap = databasedBeatmap != null && !databasedBeatmap.BeatmapSet.DeletePending; + + if (databasedBeatmap == null) + hasBeatmap = false; + else + { + // DeletePending isn't updated in the beatmap info query above, need to directly query the beatmap set from database as well. + var databasedSet = beatmaps.QueryBeatmapSet(s => s.Equals(databasedBeatmap.BeatmapSet)); + hasBeatmap = databasedSet?.DeletePending == false; + } } protected override void Update() From 2d1b52be0d8e02ffc4c4fbbd6a9f8f621d54f211 Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 17:21:31 +0000 Subject: [PATCH 11/19] Moved "ToolbarSocialButton" This will remove it from coming off the screen. --- osu.Game/Overlays/Toolbar/Toolbar.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/Toolbar.cs b/osu.Game/Overlays/Toolbar/Toolbar.cs index 393e349bd0..88a2518f84 100644 --- a/osu.Game/Overlays/Toolbar/Toolbar.cs +++ b/osu.Game/Overlays/Toolbar/Toolbar.cs @@ -69,12 +69,13 @@ namespace osu.Game.Overlays.Toolbar AutoSizeAxes = Axes.X, Children = new Drawable[] { + + new ToolbarSocialButton(), new ToolbarNewsButton(), new ToolbarChangelogButton(), new ToolbarRankingsButton(), new ToolbarBeatmapListingButton(), new ToolbarChatButton(), - new ToolbarSocialButton(), new ToolbarMusicButton(), //new ToolbarButton //{ From 2e2b3ab5d4f66cd0295cd939a209c51485d59b41 Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 17:26:42 +0000 Subject: [PATCH 12/19] Should remove codeFactor error --- osu.Game/Overlays/Toolbar/Toolbar.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/Toolbar.cs b/osu.Game/Overlays/Toolbar/Toolbar.cs index 88a2518f84..c95d02cea4 100644 --- a/osu.Game/Overlays/Toolbar/Toolbar.cs +++ b/osu.Game/Overlays/Toolbar/Toolbar.cs @@ -69,7 +69,6 @@ namespace osu.Game.Overlays.Toolbar AutoSizeAxes = Axes.X, Children = new Drawable[] { - new ToolbarSocialButton(), new ToolbarNewsButton(), new ToolbarChangelogButton(), From 73f5e5aaf9fb94726b6906f98b6493202fedb680 Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 21:03:51 +0000 Subject: [PATCH 13/19] Moved "ToolbarSocialButton" back --- osu.Game/Overlays/Toolbar/Toolbar.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/Toolbar.cs b/osu.Game/Overlays/Toolbar/Toolbar.cs index c95d02cea4..393e349bd0 100644 --- a/osu.Game/Overlays/Toolbar/Toolbar.cs +++ b/osu.Game/Overlays/Toolbar/Toolbar.cs @@ -69,12 +69,12 @@ namespace osu.Game.Overlays.Toolbar AutoSizeAxes = Axes.X, Children = new Drawable[] { - new ToolbarSocialButton(), new ToolbarNewsButton(), new ToolbarChangelogButton(), new ToolbarRankingsButton(), new ToolbarBeatmapListingButton(), new ToolbarChatButton(), + new ToolbarSocialButton(), new ToolbarMusicButton(), //new ToolbarButton //{ From 3468df840b37291dfdae94af3fb0d29a8e7493fa Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 21:04:30 +0000 Subject: [PATCH 14/19] Moved tooltip to the left to stop the overflow --- osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs b/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs index e62c7bc807..ca334702ce 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs @@ -2,12 +2,16 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Game.Input.Bindings; namespace osu.Game.Overlays.Toolbar { public class ToolbarSocialButton : ToolbarOverlayToggleButton { + + protected override Anchor TooltipAnchor => Anchor.TopRight; + public ToolbarSocialButton() { Hotkey = GlobalAction.ToggleSocial; From 0e42d415c18888192358e4f7e2a41074ebd0530f Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 21:05:28 +0000 Subject: [PATCH 15/19] Hit another oopie --- osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs b/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs index ca334702ce..1e00afc5fd 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarSocialButton.cs @@ -9,7 +9,6 @@ namespace osu.Game.Overlays.Toolbar { public class ToolbarSocialButton : ToolbarOverlayToggleButton { - protected override Anchor TooltipAnchor => Anchor.TopRight; public ToolbarSocialButton() From 1234d0fa0409c786c4e4ec4e2a63b5a5da406292 Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 22:01:12 +0000 Subject: [PATCH 16/19] Applied all tooltips to the right --- osu.Game/Overlays/Toolbar/ToolbarBeatmapListingButton.cs | 3 +++ osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs | 5 ++++- osu.Game/Overlays/Toolbar/ToolbarChatButton.cs | 3 +++ osu.Game/Overlays/Toolbar/ToolbarNewsButton.cs | 3 +++ osu.Game/Overlays/Toolbar/ToolbarRankingsButton.cs | 3 +++ 5 files changed, 16 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/ToolbarBeatmapListingButton.cs b/osu.Game/Overlays/Toolbar/ToolbarBeatmapListingButton.cs index 0363873326..c495d673ce 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarBeatmapListingButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarBeatmapListingButton.cs @@ -2,12 +2,15 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Game.Input.Bindings; namespace osu.Game.Overlays.Toolbar { public class ToolbarBeatmapListingButton : ToolbarOverlayToggleButton { + protected override Anchor TooltipAnchor => Anchor.TopRight; + public ToolbarBeatmapListingButton() { Hotkey = GlobalAction.ToggleDirect; diff --git a/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs b/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs index 23f8b141b2..28112d178f 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs @@ -2,11 +2,14 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; namespace osu.Game.Overlays.Toolbar { public class ToolbarChangelogButton : ToolbarOverlayToggleButton - { + { + protected override Anchor TooltipAnchor => Anchor.TopRight; + [BackgroundDependencyLoader(true)] private void load(ChangelogOverlay changelog) { diff --git a/osu.Game/Overlays/Toolbar/ToolbarChatButton.cs b/osu.Game/Overlays/Toolbar/ToolbarChatButton.cs index f9a66ae7bb..2d3b33e9bc 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarChatButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarChatButton.cs @@ -2,12 +2,15 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Game.Input.Bindings; namespace osu.Game.Overlays.Toolbar { public class ToolbarChatButton : ToolbarOverlayToggleButton { + protected override Anchor TooltipAnchor => Anchor.TopRight; + public ToolbarChatButton() { Hotkey = GlobalAction.ToggleChat; diff --git a/osu.Game/Overlays/Toolbar/ToolbarNewsButton.cs b/osu.Game/Overlays/Toolbar/ToolbarNewsButton.cs index 0ba2935c80..9b2573ad07 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarNewsButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarNewsButton.cs @@ -2,11 +2,14 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; namespace osu.Game.Overlays.Toolbar { public class ToolbarNewsButton : ToolbarOverlayToggleButton { + protected override Anchor TooltipAnchor => Anchor.TopRight; + [BackgroundDependencyLoader(true)] private void load(NewsOverlay news) { diff --git a/osu.Game/Overlays/Toolbar/ToolbarRankingsButton.cs b/osu.Game/Overlays/Toolbar/ToolbarRankingsButton.cs index 22a01bcdb5..312fc41aab 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarRankingsButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarRankingsButton.cs @@ -2,11 +2,14 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Graphics; namespace osu.Game.Overlays.Toolbar { public class ToolbarRankingsButton : ToolbarOverlayToggleButton { + protected override Anchor TooltipAnchor => Anchor.TopRight; + [BackgroundDependencyLoader(true)] private void load(RankingsOverlay rankings) { From 77e660e42677eee4d2407b3b59a649f836b35f34 Mon Sep 17 00:00:00 2001 From: KyeKiller Date: Mon, 4 Jan 2021 22:11:52 +0000 Subject: [PATCH 17/19] Should pass all checks again now. --- osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs b/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs index 28112d178f..86bc73361a 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarChangelogButton.cs @@ -7,7 +7,7 @@ using osu.Framework.Graphics; namespace osu.Game.Overlays.Toolbar { public class ToolbarChangelogButton : ToolbarOverlayToggleButton - { + { protected override Anchor TooltipAnchor => Anchor.TopRight; [BackgroundDependencyLoader(true)] From 81355652fa8601d4537cb6998afe6208da1b925d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Jan 2021 06:00:15 +0300 Subject: [PATCH 18/19] Add simple test coverage --- .../TestSceneMultiplayerReadyButton.cs | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs index 6b11613f1c..03ba73d35b 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerReadyButton.cs @@ -7,8 +7,10 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.Platform; +using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Rulesets; @@ -23,6 +25,7 @@ namespace osu.Game.Tests.Visual.Multiplayer public class TestSceneMultiplayerReadyButton : MultiplayerTestScene { private MultiplayerReadyButton button; + private BeatmapSetInfo importedSet; private BeatmapManager beatmaps; private RulesetStore rulesets; @@ -38,9 +41,8 @@ namespace osu.Game.Tests.Visual.Multiplayer [SetUp] public new void Setup() => Schedule(() => { - var beatmap = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First().Beatmaps.First(); - - Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap); + importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First(); + Beatmap.Value = beatmaps.GetWorkingBeatmap(importedSet.Beatmaps.First()); Child = button = new MultiplayerReadyButton { @@ -51,13 +53,30 @@ namespace osu.Game.Tests.Visual.Multiplayer { Value = new PlaylistItem { - Beatmap = { Value = beatmap }, - Ruleset = { Value = beatmap.Ruleset } + Beatmap = { Value = Beatmap.Value.BeatmapInfo }, + Ruleset = { Value = Beatmap.Value.BeatmapInfo.Ruleset } } } }; }); + [Test] + public void TestDeletedBeatmapDisableReady() + { + OsuButton readyButton = null; + + AddAssert("ensure ready button enabled", () => + { + readyButton = button.ChildrenOfType().Single(); + return readyButton.Enabled.Value; + }); + + AddStep("delete beatmap", () => beatmaps.Delete(importedSet)); + AddAssert("ready button disabled", () => !readyButton.Enabled.Value); + AddStep("undelete beatmap", () => beatmaps.Undelete(importedSet)); + AddAssert("ready button enabled back", () => readyButton.Enabled.Value); + } + [Test] public void TestToggleStateWhenNotHost() { From ed6ffe2ef1b55d957851391d4635d0e93585dcf2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 5 Jan 2021 14:54:59 +0900 Subject: [PATCH 19/19] Remove hacky code --- osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs index b782df75df..64ddba669d 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs @@ -53,14 +53,7 @@ namespace osu.Game.Screens.OnlinePlay.Components var databasedBeatmap = beatmaps.QueryBeatmap(b => b.OnlineBeatmapID == beatmapId && b.MD5Hash == checksum); - if (databasedBeatmap == null) - hasBeatmap = false; - else - { - // DeletePending isn't updated in the beatmap info query above, need to directly query the beatmap set from database as well. - var databasedSet = beatmaps.QueryBeatmapSet(s => s.Equals(databasedBeatmap.BeatmapSet)); - hasBeatmap = databasedSet?.DeletePending == false; - } + hasBeatmap = databasedBeatmap?.BeatmapSet?.DeletePending == false; } protected override void Update()