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

Add support for computing performance of non-legacy scores #195

Merged
merged 7 commits into from
Feb 11, 2024

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Jan 30, 2024

Shouldn't conflict (apart from usings) with #194 .

LegacyHelper implementation taken from https://github.com/ppy/osu-queue-score-statistics/blob/master/osu.Server.Queues.ScoreStatisticsProcessor/Helpers/LegacyModsHelper.cs Not sure if this needs to be moved into osu.Game at this point.

Commands:

performance score <score-id> <client-id> <client-secret> [-a|--online-attributes]
performance legacy-score <score-id> <ruleset-id> <client-id> <client-secret> [-a|--online-attributes]

Note that the order of arguments in the legacy-score command differs from master in which it is currently <ruleset-id> <score-id>.

This also adds support for the -a|--online-attributes option, which allows database attributes to be used instead by querying the beatmap/{id}/attributes endpoint.

Can be tested with:

diff --git a/PerformanceCalculator/Program.cs b/PerformanceCalculator/Program.cs
index 42f4b4b..3d662db 100644
--- a/PerformanceCalculator/Program.cs
+++ b/PerformanceCalculator/Program.cs
@@ -26,7 +26,7 @@ namespace PerformanceCalculator
     [HelpOption("-?|-h|--help")]
     public class Program
     {
-        public static readonly EndpointConfiguration ENDPOINT_CONFIGURATION = new ProductionEndpointConfiguration();
+        public static readonly EndpointConfiguration ENDPOINT_CONFIGURATION = new ExperimentalEndpointConfiguration();

         public static void Main(string[] args)
         {

| LegacyMods.Key9 | LegacyMods.KeyCoop;

// See: https://github.com/ppy/osu-performance/blob/83c02f50315a4ef7feea80acb84c66ee437d7210/include/pp/Common.h#L109-L129
public static LegacyMods MaskRelevantMods(LegacyMods mods, bool isConvertedBeatmap, int rulesetId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could at least link back to the osu-queue-score-statistics version of this.

@bdach
Copy link
Contributor

bdach commented Feb 6, 2024

I tested this some and there's a bit of an issue with legacy scores. They can be queried through both of these commands, but if they are queried through the non-legacy one, then the score gets all sorts of weird things wrong like two classic mods applied and an incorrect legacy total score.

Compare output of performance legacy-score 4580689900 0 and performance 2298040790, for instance. The latter will have something like:

Basic score info
beatmap             : 4284475 - -LostFairy- - El'na dia (Nevertary)
total score         : 226333
legacy total score  : 290461
accuracy            : 86.17
combo               : 156
mods                : CL, CL

Hit statistics
ok                  : 170
meh                 : 8
miss                : 88
great               : 1238

Performance attributes
aim                 : 0.00
speed               : 0.03
accuracy            : 0.28
flashlight          : 0.00
effective miss count: 88.00
pp                  : 0.34

Difficulty attributes
star rating         : 7.46
max combo           : 2,218.00
aim difficulty      : 3.85
speed difficulty    : 3.22
speed note count    : 635.37
slider factor       : 1.00
approach rate       : 9.60
overall difficulty  : 9.20

That is because this block:

if (apiScore.BuildID == null)
{
score.Mods = score.Mods.Append(ruleset.CreateMod<ModClassic>()).ToArray();
score.IsLegacyScore = true;
score.LegacyTotalScore = (int)score.TotalScore;
LegacyScoreDecoder.PopulateMaximumStatistics(score, workingBeatmap);
StandardisedScoreMigrationTools.UpdateFromLegacy(
score,
ruleset,
LegacyBeatmapConversionDifficultyInfo.FromAPIBeatmap(apiBeatmap),
((ILegacyRuleset)ruleset).CreateLegacyScoreSimulator().Simulate(workingBeatmap, workingBeatmap.GetPlayableBeatmap(ruleset.RulesetInfo, score.Mods)));
}

is basically invalid for new scores, since the import process would have done all that already server-side. A fix to this could look something like so:

diff --git a/PerformanceCalculator/Performance/LegacyScorePerformanceCommand.cs b/PerformanceCalculator/Performance/LegacyScorePerformanceCommand.cs
index a125c96..846a283 100644
--- a/PerformanceCalculator/Performance/LegacyScorePerformanceCommand.cs
+++ b/PerformanceCalculator/Performance/LegacyScorePerformanceCommand.cs
@@ -1,8 +1,16 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.Linq;
 using McMaster.Extensions.CommandLineUtils;
+using osu.Game.Beatmaps;
+using osu.Game.Database;
 using osu.Game.Online.API.Requests.Responses;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Mods;
+using osu.Game.Rulesets.Scoring.Legacy;
+using osu.Game.Scoring;
+using osu.Game.Scoring.Legacy;
 
 namespace PerformanceCalculator.Performance
 {
@@ -13,5 +21,22 @@ public class LegacyScorePerformanceCommand : ScorePerformanceCommand
         public int RulesetId { get; set; }
 
         protected override SoloScoreInfo QueryScore() => GetJsonFromApi<SoloScoreInfo>($"scores/{LegacyHelper.GetRulesetShortNameFromId(RulesetId)}/{ScoreId}");
+
+        protected override ScoreInfo CreateScore(SoloScoreInfo apiScore, Ruleset ruleset, APIBeatmap apiBeatmap, WorkingBeatmap workingBeatmap)
+        {
+            var score = base.CreateScore(apiScore, ruleset, apiBeatmap, workingBeatmap);
+
+            score.Mods = score.Mods.Append(ruleset.CreateMod<ModClassic>()).ToArray();
+            score.IsLegacyScore = true;
+            score.LegacyTotalScore = (int)score.TotalScore;
+            LegacyScoreDecoder.PopulateMaximumStatistics(score, workingBeatmap);
+            StandardisedScoreMigrationTools.UpdateFromLegacy(
+                score,
+                ruleset,
+                LegacyBeatmapConversionDifficultyInfo.FromAPIBeatmap(apiBeatmap),
+                ((ILegacyRuleset)ruleset).CreateLegacyScoreSimulator().Simulate(workingBeatmap, workingBeatmap.GetPlayableBeatmap(ruleset.RulesetInfo, score.Mods)));
+
+            return score;
+        }
     }
 }
diff --git a/PerformanceCalculator/Performance/ScorePerformanceCommand.cs b/PerformanceCalculator/Performance/ScorePerformanceCommand.cs
index 7385497..7ee184b 100644
--- a/PerformanceCalculator/Performance/ScorePerformanceCommand.cs
+++ b/PerformanceCalculator/Performance/ScorePerformanceCommand.cs
@@ -10,18 +10,15 @@
 using Newtonsoft.Json;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.Legacy;
-using osu.Game.Database;
 using osu.Game.Models;
 using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Catch.Difficulty;
 using osu.Game.Rulesets.Difficulty;
 using osu.Game.Rulesets.Mania.Difficulty;
-using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu.Difficulty;
-using osu.Game.Rulesets.Scoring.Legacy;
 using osu.Game.Rulesets.Taiko.Difficulty;
-using osu.Game.Scoring.Legacy;
+using osu.Game.Scoring;
 
 namespace PerformanceCalculator.Performance
 {
@@ -42,29 +39,8 @@ public override void Execute()
             APIBeatmap apiBeatmap = GetJsonFromApi<APIBeatmap>($"beatmaps/lookup?id={apiScore.BeatmapID}");
 
             var ruleset = LegacyHelper.GetRulesetFromLegacyID(apiScore.RulesetID);
-            var score = apiScore.ToScoreInfo(apiScore.Mods.Select(m => m.ToMod(ruleset)).ToArray(), apiBeatmap);
-            score.Ruleset = ruleset.RulesetInfo;
-            score.BeatmapInfo!.Metadata = new BeatmapMetadata
-            {
-                Title = apiBeatmap.Metadata.Title,
-                Artist = apiBeatmap.Metadata.Artist,
-                Author = new RealmUser { Username = apiBeatmap.Metadata.Author.Username },
-            };
-
-            var workingBeatmap = ProcessorWorkingBeatmap.FromFileOrId(score.BeatmapInfo!.OnlineID.ToString());
-
-            if (apiScore.BuildID == null)
-            {
-                score.Mods = score.Mods.Append(ruleset.CreateMod<ModClassic>()).ToArray();
-                score.IsLegacyScore = true;
-                score.LegacyTotalScore = (int)score.TotalScore;
-                LegacyScoreDecoder.PopulateMaximumStatistics(score, workingBeatmap);
-                StandardisedScoreMigrationTools.UpdateFromLegacy(
-                    score,
-                    ruleset,
-                    LegacyBeatmapConversionDifficultyInfo.FromAPIBeatmap(apiBeatmap),
-                    ((ILegacyRuleset)ruleset).CreateLegacyScoreSimulator().Simulate(workingBeatmap, workingBeatmap.GetPlayableBeatmap(ruleset.RulesetInfo, score.Mods)));
-            }
+            var workingBeatmap = ProcessorWorkingBeatmap.FromFileOrId(apiScore.BeatmapID.ToString());
+            var score = CreateScore(apiScore, ruleset, apiBeatmap, workingBeatmap);
 
             DifficultyAttributes attributes;
 
@@ -85,6 +61,20 @@ public override void Execute()
             OutputPerformance(score, performanceAttributes, attributes);
         }
 
+        protected virtual ScoreInfo CreateScore(SoloScoreInfo apiScore, Ruleset ruleset, APIBeatmap apiBeatmap, WorkingBeatmap workingBeatmap)
+        {
+            var score = apiScore.ToScoreInfo(apiScore.Mods.Select(m => m.ToMod(ruleset)).ToArray(), apiBeatmap);
+            score.Ruleset = ruleset.RulesetInfo;
+            score.BeatmapInfo!.Metadata = new BeatmapMetadata
+            {
+                Title = apiBeatmap.Metadata.Title,
+                Artist = apiBeatmap.Metadata.Artist,
+                Author = new RealmUser { Username = apiBeatmap.Metadata.Author.Username },
+            };
+
+            return score;
+        }
+
         protected virtual SoloScoreInfo QueryScore() => GetJsonFromApi<SoloScoreInfo>($"scores/{ScoreId}");
 
         private DifficultyAttributes queryApiAttributes(int beatmapId, int rulesetId, LegacyMods mods)

This resolves all that, except for a remaining small niggle in SoloScoreInfo.LegacyTotalScore not getting written across which is addressed in ppy/osu#27069.

Copy link
Contributor

@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.

as above

Copy link
Contributor

@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.

think this can be merged now, ppy/osu#27069 can bubble down here whenever most convenient

@peppy peppy merged commit bfe1d34 into ppy:master Feb 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants