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

Stable scores should be using updated grade definitions #26595

Closed
peppy opened this issue Jan 17, 2024 · 5 comments · Fixed by ppy/osu-queue-score-statistics#203
Closed

Stable scores should be using updated grade definitions #26595

peppy opened this issue Jan 17, 2024 · 5 comments · Fixed by ppy/osu-queue-score-statistics#203
Assignees
Labels
area:scoring priority:1 Very important. Feels bad without fix. Affects the majority of users.

Comments

@peppy
Copy link
Member

peppy commented Jan 17, 2024

As touched on in #26579, both server side and client should be recalculating grades of converted scores, else things are going to get very weird on global leaderboards. Hopefully this is not a contentious issue.

This is going to require a reprocess of all scores server-side, and some client side migration too.

@peppy peppy added area:scoring priority:1 Very important. Feels bad without fix. Affects the majority of users. labels Jan 17, 2024
@peppy peppy moved this from Needs discussion to Needs implementation in Path to osu!(lazer) ranked play Jan 17, 2024
@bdach
Copy link
Collaborator

bdach commented Jan 17, 2024

I'll prepare a diff that does this, but one thing I'm not entirely clear on is what such a change is going to mean for users' rank counts. Are we going to recalculate users' rank count totals to use new grades? I imagine some people may not be very happy about that, and I think this'd be a change that would immediately affect users even on the "stable" site instance...

@bdach
Copy link
Collaborator

bdach commented Jan 17, 2024

In theory something like the following should work:

https://github.com/ppy/osu/compare/master...bdach:osu:use-new-ranks-everywhere?expand=1
https://github.com/ppy/osu-queue-score-statistics/compare/master...bdach:osu-queue-score-statistics:use-new-ranks-everywhere?expand=1

Initial tests look promising, although I'll need to apply some extra care to make sure the simplifications I applied there are 100% correct. This would be done by running some sheets.

Also holding off from PRing this until we figure out what to do with rank counts as per comment above.

@bdach bdach self-assigned this Jan 17, 2024
@peppy
Copy link
Member Author

peppy commented Jan 18, 2024

Are we going to recalculate users' rank count totals to use new grades?

I think this will be an afterthought, depends on how people receive the new scoring and everything else. For now it wouldn't change. If/when it changes, it would be a matter of enabling the correct processor on score statistics processor and disabling the rank count increase on web-10, then reprocessing.

@peppy
Copy link
Member Author

peppy commented Jan 22, 2024

@peppy
Copy link
Member Author

peppy commented Jan 22, 2024

@bdach i wrote a test which may help with this (in conjunction with #26630)

diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs
index e56dff74c3..e345a46192 100644
--- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs
+++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs
@@ -252,7 +252,7 @@ public void TestSoloScoreData()
         }
 
         [Test]
-        public void AccuracyAndRankOfStableScorePreserved()
+        public void AccuracyOfStableScorePreserved()
         {
             var memoryStream = new MemoryStream();
 
@@ -292,6 +292,47 @@ public void AccuracyAndRankOfStableScorePreserved()
             });
         }
 
+        [Test]
+        public void RankOfStableScoreUsesLazerDefinitions()
+        {
+            var memoryStream = new MemoryStream();
+
+            // local partial implementation of legacy score encoder
+            // this is done half for readability, half because `LegacyScoreEncoder` forces `LATEST_VERSION`
+            // and we want to emulate a stable score here
+            using (var sw = new SerializationWriter(memoryStream, true))
+            {
+                sw.Write((byte)0); // ruleset id (osu!)
+                sw.Write(20240116); // version (anything below `LegacyScoreEncoder.FIRST_LAZER_VERSION` is stable)
+                sw.Write(string.Empty.ComputeMD5Hash()); // beatmap hash, irrelevant to this test
+                sw.Write("username"); // irrelevant to this test
+                sw.Write(string.Empty.ComputeMD5Hash()); // score hash, irrelevant to this test
+                sw.Write((ushort)195); // count300
+                sw.Write((ushort)1); // count100
+                sw.Write((ushort)4); // count50
+                sw.Write((ushort)0); // countGeki
+                sw.Write((ushort)0); // countKatu
+                sw.Write((ushort)0); // countMiss
+                sw.Write(12345678); // total score, irrelevant to this test
+                sw.Write((ushort)1000); // max combo, irrelevant to this test
+                sw.Write(false); // full combo, irrelevant to this test
+                sw.Write((int)LegacyMods.Hidden); // mods
+                sw.Write(string.Empty); // hp graph, irrelevant
+                sw.Write(DateTime.Now); // date, irrelevant
+                sw.Write(Array.Empty<byte>()); // replay data, irrelevant
+                sw.Write((long)1234); // legacy online ID, irrelevant
+            }
+
+            memoryStream.Seek(0, SeekOrigin.Begin);
+            var decoded = new TestLegacyScoreDecoder().Parse(memoryStream);
+
+            Assert.Multiple(() =>
+            {
+                // In stable this would be an A because there are over 1% 50s. But that's not a thing in lazer.
+                Assert.That(decoded.ScoreInfo.Rank, Is.EqualTo(ScoreRank.S));
+            });
+        }
+
         [Test]
         public void AccuracyOfLazerScorePreserved()
         {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:scoring priority:1 Very important. Feels bad without fix. Affects the majority of users.
Projects
No open projects
2 participants