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 6fe38a322927..96013314c786 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -43,8 +43,25 @@ protected override void LoadComplete() 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 96013314c786..e87a79f2e8f8 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -140,12 +140,35 @@ public DrawableCollectionListItem ReplacePlaceholder() 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 e71368c079b4..a17a50e232c8 100644 --- a/osu.Game/Collections/DrawableCollectionListItem.cs +++ b/osu.Game/Collections/DrawableCollectionListItem.cs @@ -28,6 +28,10 @@ public partial class DrawableCollectionListItem : OsuRearrangeableListItem content.TextBox; + + private ItemContent content = null!; + /// /// Creates a new . /// @@ -48,7 +52,7 @@ public DrawableCollectionListItem(Live item, bool isCreated) 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 @@ private partial class ItemContent : CompositeDrawable { 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 @@ private void load() { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, - IsTextBoxHovered = v => textBox.ReceivePositionalInputAt(v) + IsTextBoxHovered = v => TextBox.ReceivePositionalInputAt(v) } : Empty(), new Container @@ -89,7 +90,7 @@ private void load() 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 @@ protected override void LoadComplete() 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 e87a79f2e8f8..aaef41eee137 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -162,8 +162,10 @@ protected override void LoadComplete() 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 a17a50e232c8..f07ec87353fe 100644 --- a/osu.Game/Collections/DrawableCollectionListItem.cs +++ b/osu.Game/Collections/DrawableCollectionListItem.cs @@ -114,7 +114,7 @@ protected override void LoadComplete() 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 747cf73bafe7..46bf6dfafd58 100644 --- a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs +++ b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs @@ -205,7 +205,9 @@ public void TestRemoveCollectionViaButton() 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 @@ public void TestRemoveCollectionViaButton() 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 @@ public void TestCollectionRenamedOnTextChange(bool commitWithEnter) 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 46bf6dfafd58..56e7b4d39fa4 100644 --- a/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs +++ b/osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs @@ -265,6 +265,7 @@ public void TestCollectionNotRemovedWhenDialogCancelled() } [Test] + [Solo] public void TestCollectionRenamedExternal() { BeatmapCollection first = null!; @@ -341,6 +342,6 @@ private void assertCollectionCount(int count) => 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 aaef41eee137..9f7494b5561d 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 @@ public partial class DrawableCollectionList : OsuRearrangeableListContainer>> 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 9f7494b5561d..164ec558a480 100644 --- a/osu.Game/Collections/DrawableCollectionList.cs +++ b/osu.Game/Collections/DrawableCollectionList.cs @@ -151,7 +151,7 @@ public DrawableCollectionListItem ReplacePlaceholder() } } - private class NewCollectionEntryItem : DrawableCollectionListItem + private partial class NewCollectionEntryItem : DrawableCollectionListItem { [Resolved] private RealmAccess realm { get; set; } = null!;