Skip to content

Commit 30412ba

Browse files
committed
Fix song select V2 not preserving selection after an update operation
Because the detached store exists and has a chance to actually semi-reliably intercept a beatmap update operation, I decided to try this. It still uses a bit of a heuristic in that it checks for transactions that delete and insert one beatmap each, but probably the best effort thus far? Notably old song select that was already doing the same thing locally to itself got a bit broken by this, but with some tweaking that *looks* to be more or less harmless I managed to get it unbroken. I'm not too concerned about old song select, mind, mostly just want to keep it *vaguely* working if I can help it.
1 parent 1a522a1 commit 30412ba

File tree

2 files changed

+45
-7
lines changed

2 files changed

+45
-7
lines changed

osu.Game/Database/RealmDetachedBeatmapStore.cs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,34 @@ private void beatmapSetsChanged(IRealmCollection<BeatmapSetInfo> sender, ChangeS
8282
return;
8383
}
8484

85+
if (changes.InsertedIndices.Length == 1 && changes.DeletedIndices.Length == 1)
86+
{
87+
lock (detachedBeatmapSets)
88+
{
89+
var deletedSet = detachedBeatmapSets[changes.DeletedIndices[0]];
90+
var insertedSet = sender[changes.InsertedIndices[0]];
91+
92+
// this handles beatmap updates using a heuristic that a beatmap update will preserve the online ID.
93+
// it relies on the fact that updates are performed by removing the old set and adding a new one, in a single transaction.
94+
// instead of removing the old set and adding a new one to the collection too, which would trigger consumers' logic related to set removals,
95+
// move the deleted set to the index occupied by the new one and then replace it in-place.
96+
// due to this, the operation can be presented to consumer in a manner that permits them to actually handle this as a replace operation
97+
// and not trigger any set removal logic that may result in selections changing or similar undesirable side effects.
98+
if (deletedSet.OnlineID == insertedSet.OnlineID)
99+
{
100+
pendingOperations.Enqueue(new OperationArgs
101+
{
102+
Type = OperationType.MoveAndReplace,
103+
BeatmapSet = insertedSet.Detach(),
104+
Index = changes.DeletedIndices[0],
105+
NewIndex = changes.InsertedIndices[0],
106+
});
107+
108+
return;
109+
}
110+
}
111+
}
112+
85113
foreach (int i in changes.DeletedIndices.OrderDescending())
86114
{
87115
pendingOperations.Enqueue(new OperationArgs
@@ -138,6 +166,11 @@ protected override void Update()
138166
detachedBeatmapSets.ReplaceRange(op.Index, 1, new[] { op.BeatmapSet! });
139167
break;
140168

169+
case OperationType.MoveAndReplace:
170+
detachedBeatmapSets.Move(op.Index, op.NewIndex!.Value);
171+
detachedBeatmapSets.ReplaceRange(op.NewIndex!.Value, 1, [op.BeatmapSet!]);
172+
break;
173+
141174
case OperationType.Remove:
142175
detachedBeatmapSets.RemoveAt(op.Index);
143176
break;
@@ -160,13 +193,15 @@ private record OperationArgs
160193
public OperationType Type;
161194
public BeatmapSetInfo? BeatmapSet;
162195
public int Index;
196+
public int? NewIndex;
163197
}
164198

165199
private enum OperationType
166200
{
167201
Insert,
168202
Update,
169-
Remove
203+
Remove,
204+
MoveAndReplace,
170205
}
171206
}
172207
}

osu.Game/Screens/Select/BeatmapCarousel.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,26 +237,29 @@ private void load(OsuConfigManager config, AudioManager audio, BeatmapStore beat
237237

238238
private void beatmapSetsChanged(object? beatmaps, NotifyCollectionChangedEventArgs changed)
239239
{
240+
IEnumerable<BeatmapSetInfo>? oldBeatmapSets = changed.OldItems?.Cast<BeatmapSetInfo>();
241+
HashSet<Guid> oldBeatmapSetIDs = oldBeatmapSets?.Select(s => s.ID).ToHashSet() ?? [];
242+
240243
IEnumerable<BeatmapSetInfo>? newBeatmapSets = changed.NewItems?.Cast<BeatmapSetInfo>();
244+
HashSet<Guid> newBeatmapSetIDs = newBeatmapSets?.Select(s => s.ID).ToHashSet() ?? [];
241245

242246
switch (changed.Action)
243247
{
244248
case NotifyCollectionChangedAction.Add:
245-
HashSet<Guid> newBeatmapSetIDs = newBeatmapSets!.Select(s => s.ID).ToHashSet();
246-
247249
setsRequiringRemoval.RemoveWhere(s => newBeatmapSetIDs.Contains(s.ID));
248250
setsRequiringUpdate.AddRange(newBeatmapSets!);
249251
break;
250252

251253
case NotifyCollectionChangedAction.Remove:
252-
IEnumerable<BeatmapSetInfo> oldBeatmapSets = changed.OldItems!.Cast<BeatmapSetInfo>();
253-
HashSet<Guid> oldBeatmapSetIDs = oldBeatmapSets.Select(s => s.ID).ToHashSet();
254-
255254
setsRequiringUpdate.RemoveWhere(s => oldBeatmapSetIDs.Contains(s.ID));
256-
setsRequiringRemoval.AddRange(oldBeatmapSets);
255+
setsRequiringRemoval.AddRange(oldBeatmapSets!);
257256
break;
258257

259258
case NotifyCollectionChangedAction.Replace:
259+
setsRequiringUpdate.RemoveWhere(s => oldBeatmapSetIDs.Contains(s.ID));
260+
setsRequiringRemoval.AddRange(oldBeatmapSets!);
261+
262+
setsRequiringRemoval.RemoveWhere(s => newBeatmapSetIDs.Contains(s.ID));
260263
setsRequiringUpdate.AddRange(newBeatmapSets!);
261264
break;
262265

0 commit comments

Comments
 (0)