Add "argon" variant of song progress display #22144
Conversation
…oning, font and colour
|
Looks like a pretty amazing start! I do think that the density should be visible even from time in the past, rather than having it fully opaque white. Maybe have a play with that and see if you can dim it slightly and still have the density view above the progress. |
|
cc @arflyte please view the video in the opening post and provide any feedback (also see my comment above). |
|
I think this whole test class makes no sense, since pretty much everything is tested in the tests for |
With additive blending applied on the graph I get this.
|
|
I think that looks fine, let's go with that. You'll need to look into test and code quality failures on CI as well. |
|
I love what I'm looking at so far :D |
|
I've fixed one issue, but will leave the rest to you. Also, it looks like masking isn't being applied correctly:
|
|
Oh you were faster than me 😅 |
…visible for past time
|
I did used the |
| private DrawableRuleset? drawableRuleset { get; set; } | ||
|
|
||
| // Even though `FrameStabilityContainer` caches as a `GameplayClock`, we need to check it directly via `drawableRuleset` | ||
| // as this calculator is not contained within the `FrameStabilityContainer` and won't see the dependency. |
There was a problem hiding this comment.
| // as this calculator is not contained within the `FrameStabilityContainer` and won't see the dependency. | |
| // as this component is not contained within the `FrameStabilityContainer` and won't see the dependency. |
There was a problem hiding this comment.
It's fine that code is now gone with further refactoring :D.
Allows directly referencing rather than going through `DrawableRuleset`. Helps with testing and implementation of the new song progress display (ppy#22144).
|
I've finished my pass on this PR. Will request one more team review as I've changed a lot. There's probably still work that can be done on this, but I'd like to get this in sooner rather than nitpicking, so I'd recommend anyone re-reviewing this applies changes themselves and goes ahead with the merge, bar anything major being wrong. |
peppy
changed the title
There was a problem hiding this comment.
I got interrupted as I was reviewing this, but it looked mostly fine to me (minus one issue I noticed when hiding HUD where the song progress bar doesn't hide at all, which is unrelated to this PR and more focused on SegmentedGraph drawnodes instead). Need a second review to ensure everything is good.
Related:
Depends on:
SegmentedGraphgo there)IFrameStableClockinPlayerfor easier access #22273Also addresses #21814 in some way, as it will no longer distract the player during gameplay.
I tried to implement the argon variant of the song progress
osu._RJ6xXSiZSx.mp4
I made some assumptions regarding UX since I don't know what exactly are the directions about this, any suggestion is welcomed.