-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
New leaderboard score card design implementation #22237
Conversation
Co-authored-by: Joseph Madamba <madamba.joehu@outlook.com>
…aynames. Make adjustments to statistics to allow them to work with autosizing
So what happens for a mania score? Can you add one to the test scene please? I don't think information is going to fit due to more judgements. When designing tests, please make sure to not just consider the scenario which works well, but intentionally look for the edge cases that need to be supported. Without this, the tests are not that useful. |
added to the test, fwiw i had already guarded against this by making the container for the judgements autosize to the size of the judgement names line . although i feel perhaps it would look better if the judgements were centered underneath? it doesnt look good side by side with osu ruleset but thats not a case that can happen outside of the test scene |
@arflyte are you sure this is the best way to display this? it feels a bit.. half-assed? especially when there's more judgements, it's very hard to visually parse the mapping of judgement names to counts. |
I'm for moving this to the tooltip. It's not information you really need at a first glance ( at least in my experience) and that way the tooltip isn't just an expanded mod panel? As well as displaying them in a much more comprehensible way |
Keep in mind that we already have this in the tooltip on |
just to clarify im kind of unsure how i should take this, am i meant to pull something out of somewhere and see if it works or await a design from @arflyte |
I've been looking into this for the past few weeks; it's part of the song select revision and mod display fix. I still have no final solution to this yet, but it's one of the higher priorities I'm working on right now. |
That is also a viable option as long as we agree we don't need to show more than 4 mods, I thought the point of going up to 7 mods is to resolve some recurring issues we faced with regards to overflow of mods, but I don't know if that's really the case or just an arbitrary change in this PR. |
Upon taking a peek at some of the leaderboards, I think we should at least support showing up to 5 mods for the initial push of the screen, to handle the famous mod combination DT FL HR HD combined with the Classic mod. This is definitely going to show up in a lot of scores and just displaying a text saying "5 mods" feels like it misrepresents the score (as these combinations are quite common), let alone showing them in a contracted state. Final proposal for the time being: Keeps design consistent and avoids mods overlapping each other with the "contracted" state (I think contraction is best suited for icon-based mods, which we have seemingly moved away from). diffdiff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneLeaderboardScoreV2.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneLeaderboardScoreV2.cs
index c5f96d1568..321f443a97 100644
--- a/osu.Game.Tests/Visual/SongSelect/TestSceneLeaderboardScoreV2.cs
+++ b/osu.Game.Tests/Visual/SongSelect/TestSceneLeaderboardScoreV2.cs
@@ -154,19 +154,11 @@ private static ScoreInfo[] getTestScores()
}
};
- for (int i = 0; i < LeaderboardScoreV2.MAX_MODS_EXPANDED; i++)
- scores[0].Mods = scores[0].Mods.Concat(new Mod[] { i % 2 == 0 ? new OsuModHidden() : halfTime }).ToArray();
-
- for (int i = 0; i < LeaderboardScoreV2.MAX_MODS_EXPANDED + 1; i++)
- scores[1].Mods = scores[1].Mods.Concat(new Mod[] { i % 2 == 0 ? new OsuModHidden() : new OsuModHalfTime() }).ToArray();
-
- for (int i = 0; i < LeaderboardScoreV2.MAX_MODS_CONTRACTED; i++)
- scores[2].Mods = scores[2].Mods.Concat(new Mod[] { i % 2 == 0 ? new OsuModHidden() : halfTime }).ToArray();
-
- for (int i = 0; i < LeaderboardScoreV2.MAX_MODS_CONTRACTED + 1; i++)
- scores[3].Mods = scores[3].Mods.Concat(new Mod[] { i % 2 == 0 ? new OsuModHidden() : new OsuModHalfTime() }).ToArray();
-
- scores[4].Mods = scores[4].BeatmapInfo!.Ruleset.CreateInstance().CreateAllMods().ToArray();
+ scores[0].Mods = new Mod[] { new OsuModDoubleTime(), new OsuModHidden() };
+ scores[1].Mods = new Mod[] { new OsuModDoubleTime(), new OsuModHidden(), new OsuModFlashlight() };
+ scores[2].Mods = new Mod[] { new OsuModDoubleTime(), new OsuModHidden(), new OsuModFlashlight(), new OsuModHardRock() };
+ scores[3].Mods = new Mod[] { new OsuModDoubleTime(), new OsuModHidden(), new OsuModFlashlight(), new OsuModHardRock(), new OsuModClassic() };
+ scores[4].Mods = new Mod[] { new OsuModDoubleTime(), new OsuModHidden(), new OsuModFlashlight(), new OsuModHardRock(), new OsuModClassic(), new OsuModDifficultyAdjust() };
return scores;
}
diff --git a/osu.Game/Online/Leaderboards/LeaderboardScoreV2.cs b/osu.Game/Online/Leaderboards/LeaderboardScoreV2.cs
index b9ae3bb20e..862eae10e7 100644
--- a/osu.Game/Online/Leaderboards/LeaderboardScoreV2.cs
+++ b/osu.Game/Online/Leaderboards/LeaderboardScoreV2.cs
@@ -41,16 +41,11 @@ namespace osu.Game.Online.Leaderboards
public partial class LeaderboardScoreV2 : OsuClickableContainer, IHasContextMenu, IHasCustomTooltip<ScoreInfo>
{
/// <summary>
- /// The maximum number of mods when contracted until the mods display width exceeds the <see cref="right_content_width"/>.
+ /// The maximum number of mods until the mods display width exceeds the <see cref="right_content_width"/>.
/// </summary>
- public const int MAX_MODS_CONTRACTED = 13;
+ public const int MAX_MODS = 5;
- /// <summary>
- /// The maximum number of mods when expanded until the mods display width exceeds the <see cref="right_content_width"/>.
- /// </summary>
- public const int MAX_MODS_EXPANDED = 4;
-
- private const float right_content_width = 180;
+ private const float right_content_width = 210;
private const float grade_width = 40;
private const float username_min_width = 125;
private const float statistics_regular_min_width = 175;
@@ -182,8 +177,6 @@ private void load()
};
innerAvatar.OnLoadComplete += d => d.FadeInFromZero(200);
-
- modsContainer.Spacing = new Vector2(modsContainer.Children.Count > MAX_MODS_EXPANDED ? -20 : 2, 0);
modsContainer.Padding = new MarginPadding { Top = modsContainer.Children.Count > 0 ? 4 : 0 };
}
@@ -439,6 +432,7 @@ private void load()
Shear = -shear,
AutoSizeAxes = Axes.Both,
Direction = FillDirection.Horizontal,
+ Spacing = new Vector2(2f, 0f),
ChildrenEnumerable = score.Mods.AsOrdered().Select(mod => new ColouredModSwitchTiny(mod) { Scale = new Vector2(0.375f) })
},
modsCounter = new OsuSpriteText
@@ -492,7 +486,7 @@ public override void Show()
using (BeginDelayedSequence(50))
{
- Drawable modsDrawable = score.Mods.Length > MAX_MODS_CONTRACTED ? modsCounter : modsContainer;
+ Drawable modsDrawable = score.Mods.Length > MAX_MODS ? modsCounter : modsContainer;
var drawables = new[] { flagBadgeAndDateContainer, modsDrawable }.Concat(statisticsLabels).ToArray();
for (int i = 0; i < drawables.Length; i++)
drawables[i].FadeIn(100 + i * 50); |
modsCounter = new OsuSpriteText | ||
{ | ||
Anchor = Anchor.TopRight, | ||
Origin = Anchor.TopRight, | ||
Shear = -shear, | ||
Text = $"{score.Mods.Length} mods", | ||
Alpha = 0, | ||
} |
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.
I have stirred up some discussion on whether this can be replaced by an indicator next to an arbitrary subset of mods, and it was concluded that it's better. For example (design is arbitrary):
Currently waiting for a response from @arflyte for a better design of this indicator, unless it's fine to proceed with the current one (I definitely need to rewrite the code because I faked up a Mod
to achieve this).
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.
Go with what you have, I think it's fine to start with.
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.
Added in 5955378
I think that it's much more readable to use an icons instead of acronyms |
Also adjusts right content width to contain those 5 mods. Not sure how to handle the extra space in the score though...to be dealt with later.
I've changed the mod display as proposed in #22237 (comment) & #22237 (comment). I've also made the width of the right content change based on scoring mode, to handle marathon beatmaps where scores can go up to 9 digits (e.g. https://osu.ppy.sh/beatmapsets/972#osu/9007). Preview: CleanShot.2024-05-25.at.17.24.00.mp4This looks to be in a good state to me. |
See `ModSelectOverlay` components.
I've pushed a commit that follows precedent to the slanted flow logic (mod overlay components when initializing use When restarting the test (to see the pop in animation), I thought the right content width logic was broken, but it was just changing the scoring mode initially. Not sure how to prevent that step from happening initially. Recent changes look good to me otherwise. |
This comment was marked as duplicate.
This comment was marked as duplicate.
🎉 |
- No more border gradient (probably same as ppy#22237 (comment)) - No more radius 50 which is maybe a performance issue
Figma reference: https://www.figma.com/file/EjS27IBaZhc2qX97lFBUxN/Song-Select-5?type=design&node-id=1-1010&mode=design&t=WQyZkwmDC4JWz6An-0
More song select stuff.
CleanShot.2024-05-25.at.17.24.00.mp4
old
Couple of things to note when reviewing this:
Something to note while reviewing this, due to the amount of restructuring that will be required to fit these into song select they have been PRed in isolation, a follow up PR, most likely by me if im allowed will go through the necessary refactor to fit these into a new leaderboard, with the appropriate height, shear, etc.
I do not recommend comparing the implementation to the old one, since ive split the commits to be reviewed as if this was an entirely new component, since i found it to be probably the easiest way to understand it.
As for the overlapping mods when the count exceeds the amount that can be housed inside the right side of the component they will collapse and overlap as per @peppy 's suggestion.
And final major thing to note, i opted to specify the widths of the statistics manually, as to avoid them differing in length across different scores, which looked horrible when placed in a leaderboard together.
Header should be coming some time in the next few days as well as the new tooltip for this component.