-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement component for displaying running totals in daily challenge #29099
Conversation
Total pass count and cumulative total score, to be more precise.
{ | ||
Id = 2, | ||
Username = "peppy", | ||
CoverUrl = "https://osu.ppy.sh/images/headers/profile-covers/c3.jpg", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just passing by and very unrelated but this file doesn't exist anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a whole bunch of this in other tests too so I'd rather fix separately in one fell swoop.
Is there even a static URL for a cover that we can use for a test like this anymore? I was probably going to yoink something like https://assets.ppy.sh/user-cover-presets/1/df28696b58541a9e67f6755918951d542d93bdf1da41720fcca2fd2c1ea8cf51.jpeg randomly, is that gonna break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that one should be fine yeah. alternatively can put some static assets like osu-web's placeholder I guess (opening that link is not recommended)
/// <summary> | ||
/// The cumulative total of all passing scores (across all users) in the playlist so far. | ||
/// </summary> | ||
[Key(2)] | ||
public long TotalPlaylistScore { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peppy Slight naming nit but the class is named MultiplayerPlaylistItemStats
because it was kinda designed to potentially be usable for standard playlists. So being hypercorrect this could probably be better named as TotalPlaylistItemScore
, but not sure it's a huge deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, are you okay with CumulativeScore
instead to better clarify and remove redundant item part (see class name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, works for me.
Also document a bit more.
Total pass count and cumulative total score, to be more precise.
2024-07-26.09-50-34.mp4