Add in-gameplay rank display indicating minimum/maximum achievable rank #28614
Conversation
|
Would you mind explaining what is going on with #28583 getting closed and then this getting opened? |
I'm sorry for that, my repo master branch get out of sync and I don't know how to fix it. So that I deleted my repo, but I don't know it will cause the pr getting close, so I copy all the change and make a new pr |
It seems that my master branch is currently ahead of the osu! branch. But I accidentally merged my feature branch into the master branch, and it appears that the action to undo was not explicitly outlined in the documentation. sorry again for causing confusion. |
+ private Bindable<ScoreRank> rankBinding = new Bindable<ScoreRank>();this binding was added to the rank display component, as they handle the rank display update. |
…ithub.com/jkh675/osu into features/minimum-and-maximum-rank-display
|
I get notifications for any change you make, there's no need to request review, it's adding more noise to my end. |
sorry for that, I didn't know the changes can trigger the notification |
It's made to work for a single specific case, it shouldn't be scattered all over the class.
| // this arbitrarily only fills HitResult.Miss because it's the only hit result that can affect the state of RankFromScore in the main rulesets (see OsuScoreProcessor). | ||
| // eventually (whenever it's needed), other miss result types should be correctly filled and this dictionary should be updated regularly next to ScoreResultCounts, | ||
| // but this is sufficient for now. | ||
| minimumAccuracyResultCounts[HitResult.Miss] = MaxHits - JudgedHits + ScoreResultCounts.GetValueOrDefault(HitResult.Miss); |
There was a problem hiding this comment.
@ppy/team-client requesting review for the addition of this part in particular to resolve #28614 (comment)
There was a problem hiding this comment.
Too hacky to live imo. The remaining results should be populated so that this is in a sane state.
Also I'm not even convinced this is right - what if some of the remaining judgements are IgnoreMiss, for example?
|
Please do not merge master branch, it adds unnecessary noise to our notification inbox. |
| // this arbitrarily only fills HitResult.Miss because it's the only hit result that can affect the state of RankFromScore in the main rulesets (see OsuScoreProcessor). | ||
| // eventually (whenever it's needed), other miss result types should be correctly filled and this dictionary should be updated regularly next to ScoreResultCounts, | ||
| // but this is sufficient for now. | ||
| minimumAccuracyResultCounts[HitResult.Miss] = MaxHits - JudgedHits + ScoreResultCounts.GetValueOrDefault(HitResult.Miss); |
There was a problem hiding this comment.
Too hacky to live imo. The remaining results should be populated so that this is in a sane state.
Also I'm not even convinced this is right - what if some of the remaining judgements are IgnoreMiss, for example?
| public partial class TestSceneOsuScoreProcessor : OsuTestScene | ||
| { | ||
| [Test] | ||
| public void TestMinimumRankNeverReachesSRank() |
There was a problem hiding this comment.
This should read "never reaches S rank before completion". That should have test coverage too.
There was a problem hiding this comment.
Maybe should make the minimum rank calculations separate into a virtual method, and can be overwritten when needed, (e.g. the situation that misses or others can affect the rank)
…kShouldNeverReachSRankBeforeComplete`
I've added a feature to the rank display component that allows displaying the maximum and minimum ranks. It works similar to the accuracy display, where we have a maximum and minimum value shown for accuracy. I implemented this feature in the rank display, so now it can show both the minimum-achievable and maximum-achievable ranks.
In order to enable the rank display component to utilize this functionality, I have added two new properties,
MinimumRankandMaximumRank, to theScoreProcessorclass.sorry for the bad English :(
Preview:
2024-06-25.22-47-35.mp4
2024-06-25.21-57-35.1.mp4
Even though the new Osu! update has been implementing the rank display update but I think it's good to implement the max. and min. achievable rank display. Just like how the accuracy did.It allows players to actually see what rank they can if they mess up at the following or what rank they can achieve if they get all perfectly.