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

Fix scores set with unranked mods showing as processing indefinitely (again) #11151

Merged
merged 10 commits into from Apr 22, 2024
4 changes: 2 additions & 2 deletions app/Http/Controllers/BeatmapsController.php
Expand Up @@ -75,7 +75,7 @@ private static function beatmapScores(string $id, ?string $scoreTransformerType,
'type' => $type,
'user' => $currentUser,
]);
$scores = $esFetch->all()->loadMissing(['beatmap', 'user.country']);
$scores = $esFetch->all()->loadMissing(['beatmap', 'user.country', 'processHistory']);
$userScore = $esFetch->userBest();
$scoreTransformer = new ScoreTransformer($scoreTransformerType);

Expand Down Expand Up @@ -494,7 +494,7 @@ public function userScoreAll($beatmapId, $userId)
'sort' => 'score_desc',
'user_id' => (int) $userId,
]);
$scores = (new ScoreSearch($params))->records();
$scores = (new ScoreSearch($params))->records()->loadMissing('processHistory');

return [
'scores' => json_collection($scores, new ScoreTransformer()),
Expand Down
6 changes: 6 additions & 0 deletions app/Models/Solo/Score.php
Expand Up @@ -149,6 +149,11 @@ public function user()
return $this->belongsTo(User::class, 'user_id');
}

public function processHistory()
{
return $this->hasOne(ScoreProcessHistory::class, 'score_id');
}

public function scopeDefault(Builder $query): Builder
{
return $query->whereHas('beatmap.beatmapset');
Expand Down Expand Up @@ -243,6 +248,7 @@ public function getAttribute($key)

'beatmap',
'performance',
'processHistory',
'reportedIn',
'user' => $this->getRelationValue($key),
};
Expand Down
25 changes: 25 additions & 0 deletions app/Models/Solo/ScoreProcessHistory.php
@@ -0,0 +1,25 @@
<?php

// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

declare(strict_types=1);

namespace App\Models\Solo;

use App\Models\Model;

/**
* @property int $score_id
* @property int $processed_version
* @property \Carbon\Carbon $processed_at
*/
class ScoreProcessHistory extends Model
{
protected $table = 'score_process_history';

public function score()
{
return $this->belongsTo(Score::class, 'score_id');
}
}
3 changes: 3 additions & 0 deletions app/Transformers/ScoreTransformer.php
Expand Up @@ -24,6 +24,7 @@ class ScoreTransformer extends TransformerAbstract
// warning: the preload is actually for PlaylistItemUserHighScore, not for Score
const MULTIPLAYER_BASE_PRELOAD = [
'scoreLink.score',
'scoreLink.score.processHistory',
'scoreLink.user.country',
];

Expand All @@ -37,6 +38,7 @@ class ScoreTransformer extends TransformerAbstract
'beatmap.beatmapset',
// it's for user profile so the user is already available
// 'user',
'processHistory',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be above the comment.

];

protected array $availableIncludes = [
Expand Down Expand Up @@ -102,6 +104,7 @@ public function transformSolo(MultiplayerScoreLink|ScoreModel|SoloScore $score)
if ($score instanceof SoloScore) {
$extraAttributes['ranked'] = $score->ranked;
$extraAttributes['preserve'] = $score->preserve;
$extraAttributes['processed'] = $score->legacy_score_id !== null || $score->processHistory !== null;
Copy link
Contributor Author

@bdach bdach Apr 17, 2024

Choose a reason for hiding this comment

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

Uhhh... So earlier I said:

Is every score guaranteed to have a corresponding score_process_history row? Does score_process_history get truncated?

As far as I'm aware, yes.

So as it turns out, I lied here (or if you wanna be charitable, "forgot"). It is not always the case that a score_process_history row will exist for a score that is processed.

The exception is scores that have been initially manually batch-imported from osu_scores_high_* tables. All legacy scores generally are fully processed regardless of the existence of that score_process_history row, because first osu-web-10 processes them synchronously, and then the score watchers import them without doing any further processing relevant to the score - so at the point the scores row exists for a legacy score, it is already fully processed.

I can't exactly double-check production to confirm this but I realised this when testing on my local database wherein I had a local score import from seeded osu_scores_high_* rows into scores.

What this says about this particular PR, and also the complexity in and around the new score table, I'm not entirely sure, but I do acknowledge that it's pretty bad, but also don't really have anything constructive to offer that isn't like a week spent on revisiting the entirety of the table structure again and attempting to somehow beat it into shape better...

}

$hasReplay = $score->has_replay;
Expand Down
1 change: 1 addition & 0 deletions resources/js/interfaces/solo-score-json.ts
Expand Up @@ -41,6 +41,7 @@ type SoloScoreJsonDefaultAttributes = {
passed: boolean;
pp: number | null;
preserve?: boolean;
processed?: boolean;
rank: Rank;
ranked?: boolean;
ruleset_id: number;
Expand Down
6 changes: 3 additions & 3 deletions resources/js/scores/pp-value.tsx
Expand Up @@ -21,12 +21,12 @@ export default function PpValue(props: Props) {
if (!isBest && !isSolo) {
title = trans('scores.status.non_best');
content = '-';
} else if (props.score.ranked === false) {
} else if (props.score.ranked === false || props.score.preserve === false) {
title = trans('scores.status.no_pp');
content = '-';
} else if (props.score.pp == null) {
if (isSolo && !props.score.passed) {
title = trans('scores.status.non_passing');
if (isSolo && props.score.processed === true) {
title = trans('scores.status.no_pp');
content = '-';
} else {
title = trans('scores.status.processing');
Expand Down
1 change: 0 additions & 1 deletion resources/lang/en/scores.php
Expand Up @@ -25,7 +25,6 @@

'status' => [
'non_best' => 'Only personal best scores award pp',
'non_passing' => 'Only passing scores award pp',
'no_pp' => 'pp is not awarded for this score',
'processing' => 'This score is still being calculated and will be displayed soon',
'no_rank' => 'This score has no rank as it is unranked or marked for deletion',
Expand Down