Skip to content
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

Fixed User Ruleset from Leaderboard #31720

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Ianlucht
Copy link
Contributor

@Ianlucht Ianlucht commented Jan 29, 2025

Made a user profile open to the desired ruleset from a beatmap rather than the profile ruleset at all times.

Resolves #31628

@Ianlucht
Copy link
Contributor Author

Alright, I made the changes, but now there is a dependency issue in , and I have no idea how to fix this issue. osu.Framework.Allocation.DependencyNotRegisteredException: The type LeaderboardScore has a dependency on OsuGame, but the dependency is not registered. I had to add the private OsuGame = game there to be able to get the correct user profile up with the ruleset, but in adding that it seems to have broken the dependencies there. I don't know how to properly register it. I thought I just had to add it the way it was added to ClickableAvatar, but that seems incorrect.

Ianlucht and others added 3 commits January 30, 2025 09:33
Formatting error might be because of the preprocessor directive being inside the class.
@Ianlucht
Copy link
Contributor Author

Trying to figure out a way to have the nullable OsuGame reference where the other references are explicitly un-nullable

@Ianlucht Ianlucht requested a review from bdach January 31, 2025 19:46
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash and force-push all of the extra commits out of here, it's noise in history that doesn't need to be there.

Upon closer inspection this should probably also be applied to clickable avatars on the results screen:

image

which is going to be a bit annoying to achieve and also makes me unsure about whether this change is worth it in general.

Comment on lines +90 to +94
// using nullable enable here because canBeNull is not respected for OsuGame
#nullable enable

[Resolved]
private OsuGame? game { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how this should be done. Old classes where NRT is still not on are a bit of an edge case.

diff --git a/osu.Game/Online/Leaderboards/LeaderboardScore.cs b/osu.Game/Online/Leaderboards/LeaderboardScore.cs
index 2f8e7bb7b9..438a0e2268 100644
--- a/osu.Game/Online/Leaderboards/LeaderboardScore.cs
+++ b/osu.Game/Online/Leaderboards/LeaderboardScore.cs
@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
@@ -87,11 +88,9 @@ public partial class LeaderboardScore : OsuClickableContainer, IHasContextMenu,
         [Resolved]
         private ScoreManager scoreManager { get; set; } = null!;
 
-        // using nullable enable here because canBeNull is not respected for OsuGame
-#nullable enable
-
-        [Resolved]
-        private OsuGame? game { get; set; }
+        [Resolved(CanBeNull = true)]
+        [CanBeNull]
+        private OsuGame game { get; set; }
 
         public LeaderboardScore(ScoreInfo score, int? rank, bool isOnlineScope = true, bool highlightFriend = true)
         {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opening a profile from the leaderboard of a specific ruleset still shows the default ruleset.
3 participants