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.

Sign up for GitHub

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

Show difficulty rating with mods applied on score pages #8599

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Show difficulty rating with mods applied on score pages #8599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/Http/Controllers/ScoresController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public function show($mode, $id)
$scoreJson = json_item($score, 'Score', array_merge([
'beatmap.max_combo',
'beatmapset',
'difficulty_rating',
'rank_global',
], $userIncludes));

Expand Down
12 changes: 12 additions & 0 deletions app/Models/Score/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ public static function getMode(): string
return snake_case(get_class_basename(static::class));
}

public function difficultyRating(): ?float
Copy link
Member

Choose a reason for hiding this comment

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

If this is to actually exist here, it at very least needs a better name. A score can't have a "difficulty rating", so it would need a beatmap prefix.

Optimally I don't think it should be in here at all, but will defer to the web team to ascertain whether a structure to make that happen is possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the closest thing I can think of is making it a relation called beatmapDifficulty instead.

Similarly for the transformer although considering difficulty rating is the only relevant value beatmap_difficulty_rating could be sufficient...

Copy link
Collaborator

Choose a reason for hiding this comment

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

if the difficulty difference is only when mods are applied, then this shouldn't run extra queries if there're no mods

Copy link
Collaborator

@nanaya nanaya Feb 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

looking again, relation is quite difficult because the difficulty table uses composite primary key. Short of adding virtual column and indexing it, it's probably better being an extra select column with subquery similar to this (although with simpler query).

But then again for single request it probably doesn't matter much.

Copy link
Collaborator

@nanaya nanaya Feb 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

And if adding skipping difficulty query, then both mode and mods should be checked in case of convert maps (which would have different difficulty rating even when no other mods).

(checking beatmap model would be required in that case, otherwise the table can be checked directly)

{
$value = $this->beatmap
->difficulty()
->where('mode', Beatmap::modeInt(static::getMode()))
->where('mods', ModsHelper::toBitset($this->enabled_mods))
->first()
?->diff_unified;

return $value === null ? null : round($value, 2);
}

public function scopeDefault($query)
{
return $query
Expand Down
6 changes: 6 additions & 0 deletions app/Transformers/ScoreTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class ScoreTransformer extends TransformerAbstract
'beatmap',
'beatmapset',
'current_user_attributes',
'difficulty_rating',
'rank_country',
'rank_global',
'weight',
Expand Down Expand Up @@ -125,6 +126,11 @@ public function includeBeatmapset($score)
return $this->item($score->beatmap->beatmapset, new BeatmapsetCompactTransformer());
}

public function includeDifficultyRating($score)
{
return $this->primitive($score->difficultyRating());
}

public function includeWeight($score)
{
if ($score instanceof ScoreBest && $score->weight !== null) {
Expand Down
1 change: 1 addition & 0 deletions resources/assets/lib/interfaces/score-json.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default interface ScoreJson {
current_user_attributes: {
pin?: ScoreCurrentUserPinJson;
};
difficulty_rating?: number | null;
id: number;
max_combo: number;
mode?: GameMode;
Expand Down
4 changes: 4 additions & 0 deletions resources/assets/lib/scores-show/main.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export default function Main(props: Props) {
throw new Error('score json is missing beatmap or beatmapset details');
}

if (props.score.difficulty_rating != null) {
beatmap.difficulty_rating = props.score.difficulty_rating;
}
Comment on lines +23 to +25
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should be indicator that it's a modified difficulty rating instead of simply overwriting it.


return (
<>
<HeaderV4 />
Expand Down