From 695f156f5f405c6c729c26e73572a1523f922944 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 14:12:13 +0900 Subject: [PATCH 1/6] Change collection management dialog to handle changes more correctly --- .../Collections/DrawableCollectionList.cs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/osu.Game/Collections/DrawableCollectionList.cs b/osu.Game/Collections/DrawableCollectionList.cs index 6fe38a3229..96013314c7 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -43,8 +43,25 @@ namespace osu.Game.Collections private void collectionsChanged(IRealmCollection collections, ChangeSet? changes) { - Items.Clear(); - Items.AddRange(collections.AsEnumerable().Select(c => c.ToLive(realm))); + if (changes == null) + { + Items.AddRange(collections.AsEnumerable().Select(c => c.ToLive(realm))); + return; + } + + foreach (int i in changes.DeletedIndices.OrderDescending()) + Items.RemoveAt(i); + + foreach (int i in changes.InsertedIndices) + Items.Insert(i, collections[i].ToLive(realm)); + + foreach (int i in changes.NewModifiedIndices) + { + var updatedItem = collections[i]; + + Items.RemoveAt(i); + Items.Insert(i, updatedItem.ToLive(realm)); + } } protected override OsuRearrangeableListItem> CreateOsuDrawable(Live item) From ebe9a9f0837cd4e039069b46371b7ff3e6339182 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 14:12:31 +0900 Subject: [PATCH 2/6] Avoid writing changes to realm when collection name wasn't actually changed --- .../Collections/DrawableCollectionList.cs | 25 ++++++++++++++++- .../Collections/DrawableCollectionListItem.cs | 27 +++++++++---------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/osu.Game/Collections/DrawableCollectionList.cs b/osu.Game/Collections/DrawableCollectionList.cs index 96013314c7..e87a79f2e8 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -140,12 +140,35 @@ namespace osu.Game.Collections var previous = PlaceholderItem; placeholderContainer.Clear(false); - placeholderContainer.Add(PlaceholderItem = new DrawableCollectionListItem(new BeatmapCollection().ToLiveUnmanaged(), false)); + placeholderContainer.Add(PlaceholderItem = new NewCollectionEntryItem()); return previous; } } + private class NewCollectionEntryItem : DrawableCollectionListItem + { + [Resolved] + private RealmAccess realm { get; set; } = null!; + + public NewCollectionEntryItem() + : base(new BeatmapCollection().ToLiveUnmanaged(), false) + { + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + TextBox.OnCommit += (sender, newText) => + { + if (newText && !string.IsNullOrEmpty(TextBox.Current.Value)) + realm.Write(r => r.Add(new BeatmapCollection(TextBox.Current.Value))); + TextBox.Text = string.Empty; + }; + } + } + /// /// The flow of . Disables layout easing unless a drag is in progress. /// diff --git a/osu.Game/Collections/DrawableCollectionListItem.cs b/osu.Game/Collections/DrawableCollectionListItem.cs index e71368c079..a17a50e232 100644 --- a/osu.Game/Collections/DrawableCollectionListItem.cs +++ b/osu.Game/Collections/DrawableCollectionListItem.cs @@ -28,6 +28,10 @@ namespace osu.Game.Collections private const float item_height = 35; private const float button_width = item_height * 0.75f; + protected TextBox TextBox => content.TextBox; + + private ItemContent content = null!; + /// /// Creates a new . /// @@ -48,7 +52,7 @@ namespace osu.Game.Collections CornerRadius = item_height / 2; } - protected override Drawable CreateContent() => new ItemContent(Model); + protected override Drawable CreateContent() => content = new ItemContent(Model); /// /// The main content of the . @@ -57,10 +61,7 @@ namespace osu.Game.Collections { private readonly Live collection; - private ItemTextBox textBox = null!; - - [Resolved] - private RealmAccess realm { get; set; } = null!; + public ItemTextBox TextBox { get; private set; } = null!; public ItemContent(Live collection) { @@ -80,7 +81,7 @@ namespace osu.Game.Collections { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, - IsTextBoxHovered = v => textBox.ReceivePositionalInputAt(v) + IsTextBoxHovered = v => TextBox.ReceivePositionalInputAt(v) } : Empty(), new Container @@ -89,7 +90,7 @@ namespace osu.Game.Collections Padding = new MarginPadding { Right = collection.IsManaged ? button_width : 0 }, Children = new Drawable[] { - textBox = new ItemTextBox + TextBox = new ItemTextBox { RelativeSizeAxes = Axes.Both, Size = Vector2.One, @@ -107,18 +108,14 @@ namespace osu.Game.Collections base.LoadComplete(); // Bind late, as the collection name may change externally while still loading. - textBox.Current.Value = collection.PerformRead(c => c.IsValid ? c.Name : string.Empty); - textBox.OnCommit += onCommit; + TextBox.Current.Value = collection.PerformRead(c => c.IsValid ? c.Name : string.Empty); + TextBox.OnCommit += onCommit; } private void onCommit(TextBox sender, bool newText) { - if (collection.IsManaged) - collection.PerformWrite(c => c.Name = textBox.Current.Value); - else if (!string.IsNullOrEmpty(textBox.Current.Value)) - realm.Write(r => r.Add(new BeatmapCollection(textBox.Current.Value))); - - textBox.Text = string.Empty; + if (collection.IsManaged && newText) + collection.PerformWrite(c => c.Name = TextBox.Current.Value); } } From 17b1888c597434326db9db5da21f022be5b26fe3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 14:23:43 +0900 Subject: [PATCH 3/6] Avoid using `newTexst` as it doesn't work well with tests --- osu.Game/Collections/DrawableCollectionList.cs | 6 ++++-- osu.Game/Collections/DrawableCollectionListItem.cs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Collections/DrawableCollectionList.cs b/osu.Game/Collections/DrawableCollectionList.cs index e87a79f2e8..aaef41eee1 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -162,8 +162,10 @@ namespace osu.Game.Collections TextBox.OnCommit += (sender, newText) => { - if (newText && !string.IsNullOrEmpty(TextBox.Current.Value)) - realm.Write(r => r.Add(new BeatmapCollection(TextBox.Current.Value))); + if (string.IsNullOrEmpty(TextBox.Text)) + return; + + realm.Write(r => r.Add(new BeatmapCollection(TextBox.Text))); TextBox.Text = string.Empty; }; } diff --git a/osu.Game/Collections/DrawableCollectionListItem.cs b/osu.Game/Collections/DrawableCollectionListItem.cs index a17a50e232..f07ec87353 100644 --- a/osu.Game/Collections/DrawableCollectionListItem.cs +++ b/osu.Game/Collections/DrawableCollectionListItem.cs @@ -114,7 +114,7 @@ namespace osu.Game.Collections private void onCommit(TextBox sender, bool newText) { - if (collection.IsManaged && newText) + if (collection.IsManaged && collection.Value.Name != TextBox.Current.Value) collection.PerformWrite(c => c.Name = TextBox.Current.Value); } } From d66daf15a56579b1bf6ae905555df944fd846fdf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 14:36:58 +0900 Subject: [PATCH 4/6] Fix tests clicking wrong delete buttons due to internal drawable ordering --- .../Collections/TestSceneManageCollectionsDialog.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs index 747cf73baf..46bf6dfafd 100644 --- a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs +++ b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs @@ -205,7 +205,9 @@ namespace osu.Game.Tests.Visual.Collections AddStep("click first delete button", () => { - InputManager.MoveMouseTo(dialog.ChildrenOfType().First(), new Vector2(5, 0)); + InputManager.MoveMouseTo(dialog + .ChildrenOfType().Single(i => i.Model.Value.Name == "1") + .ChildrenOfType().Single(), new Vector2(5, 0)); InputManager.Click(MouseButton.Left); }); @@ -213,9 +215,11 @@ namespace osu.Game.Tests.Visual.Collections assertCollectionCount(1); assertCollectionName(0, "2"); - AddStep("click first delete button", () => + AddStep("click second delete button", () => { - InputManager.MoveMouseTo(dialog.ChildrenOfType().First(), new Vector2(5, 0)); + InputManager.MoveMouseTo(dialog + .ChildrenOfType().Single(i => i.Model.Value.Name == "2") + .ChildrenOfType().Single(), new Vector2(5, 0)); InputManager.Click(MouseButton.Left); }); @@ -310,7 +314,7 @@ namespace osu.Game.Tests.Visual.Collections AddStep("focus first collection", () => { - InputManager.MoveMouseTo(firstItem = dialog.ChildrenOfType().First()); + InputManager.MoveMouseTo(firstItem = dialog.ChildrenOfType().Single(i => i.Model.Value.Name == "1")); InputManager.Click(MouseButton.Left); }); From fea6a544325854877637d13e587864afeb4ca1ab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 14:46:58 +0900 Subject: [PATCH 5/6] Fix more tests reading in wrong order --- .../Visual/Collections/TestSceneManageCollectionsDialog.cs | 3 ++- osu.Game/Collections/DrawableCollectionList.cs | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs index 46bf6dfafd..56e7b4d39f 100644 --- a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs +++ b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs @@ -265,6 +265,7 @@ namespace osu.Game.Tests.Visual.Collections } [Test] + [Solo] public void TestCollectionRenamedExternal() { BeatmapCollection first = null!; @@ -341,6 +342,6 @@ namespace osu.Game.Tests.Visual.Collections => AddUntilStep($"{count} collections shown", () => dialog.ChildrenOfType().Count() == count + 1); // +1 for placeholder private void assertCollectionName(int index, string name) - => AddUntilStep($"item {index + 1} has correct name", () => dialog.ChildrenOfType().ElementAt(index).ChildrenOfType().First().Text == name); + => AddUntilStep($"item {index + 1} has correct name", () => dialog.ChildrenOfType().Single().OrderedItems.ElementAt(index).ChildrenOfType().First().Text == name); } } diff --git a/osu.Game/Collections/DrawableCollectionList.cs b/osu.Game/Collections/DrawableCollectionList.cs index aaef41eee1..9f7494b556 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; @@ -29,7 +30,11 @@ namespace osu.Game.Collections private IDisposable? realmSubscription; - protected override FillFlowContainer>> CreateListFillFlowContainer() => new Flow + private Flow flow = null!; + + public IEnumerable OrderedItems => flow.FlowingChildren; + + protected override FillFlowContainer>> CreateListFillFlowContainer() => flow = new Flow { DragActive = { BindTarget = DragActive } }; From f9489690cb64bfc30ed1b2c96c8cb1b011364efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 14 Nov 2024 11:58:57 +0100 Subject: [PATCH 6/6] Fix code inspection --- osu.Game/Collections/DrawableCollectionList.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Collections/DrawableCollectionList.cs b/osu.Game/Collections/DrawableCollectionList.cs index 9f7494b556..164ec558a4 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -151,7 +151,7 @@ namespace osu.Game.Collections } } - private class NewCollectionEntryItem : DrawableCollectionListItem + private partial class NewCollectionEntryItem : DrawableCollectionListItem { [Resolved] private RealmAccess realm { get; set; } = null!;