Compute the top local rank directly without an expensive detach call

[AkiSakurai]

The detach call can be slow because it has to access every properties of ScoreInfo and its descendants.

[bdach]

First of all, if this is a helper now for whatever reason, then it cannot return ScoreInfo. Removing the detach means that the ScoreInfo instance is now unsafe for off-thread accesses and for writes. It should return Live<ScoreInfo> instead. Read Realm usage rules for more. (You wouldn't have to bother with Live<T> if you just removed the detach in the place it previously was.)

Secondly, why's an optimisation PR changing order, too? What are these compare rules, why is OnlineID a sort parameter? Also how does this sorting by anonymous tuple even work?

[bdach]

I guess the answer to that last question is here. TIL.

[AkiSakurai]

First of all, if this is a helper now for whatever reason, then it cannot return ScoreInfo. Removing the detach means that the ScoreInfo instance is now unsafe for off-thread accesses and for writes. It should return Live<ScoreInfo> instead. Read Realm usage rules for more. (You wouldn't have to bother with Live<T> if you just removed the detach in the place it previously was.)

Note that when subscribing to changes, it is recommended to not locally use Live as the subscription handling ensures that all (update thread) access will be serviced by valid realm instances. That said, if passing the results of a subscription to another class, Live should still be employed.

I think the threading should be handled at the call site instead of being returned by a helper function?
I have updated the code to inline the function since it is only called once and is simple enough.

Secondly, why's an optimisation PR changing order, too? What are these compare rules, why is OnlineID a sort parameter? Also how does this sorting by anonymous tuple even work?

The order is following the original code, but OnlineID is not needed in this case since the data is already filtered.I have updated the code to remove it.

osu/osu.Game/Scoring/ScoreInfoExtensions.cs

Lines 23 to 27 in 1868826

	public static IEnumerable<ScoreInfo> OrderByTotalScore(this IEnumerable<ScoreInfo> scores)
	=> scores.OrderByDescending(s => s.TotalScore)
	.ThenBy(s => s.OnlineID)
	// Local scores may not have an online ID. Fall back to date in these cases.
	.ThenBy(s => s.Date);

https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.thenby?view=net-6.0

Performs a subsequent ordering of the elements in a sequence in ascending order

[bdach]

I think the threading should be handled at the call site instead of being returned by a helper function

What you had initially was a general purpose helper that could be called by anything and anyone. Using Live<T> is a must in such scenarios.

[peppy]

Probably fine, since the helper method was only used in one location. Biggest concern might be that "this may be required in other places in the future and get reimplemented because we missed the local implementation", but not sure how valid that concern is.