Skip to content

Add API endpoints for ranked play rankings #12940

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

Open
LiquidPL wants to merge 6 commits into
base: master
Choose a base branch
from
+150 −23
Open

Add API endpoints for ranked play rankings #12940

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b92bc7
Add API endpoints for ranked play rankings
LiquidPL Apr 23, 2026
23dad9d
Use compact user transformer
LiquidPL Apr 23, 2026
fac2db3
update routes to match non-api
LiquidPL May 21, 2026
3664080
remove unnecessary preload
LiquidPL May 21, 2026
954c042
use sigma value as tiebreaker for equal ratings
LiquidPL May 21, 2026
ff724a2
update api_routes.json
LiquidPL May 21, 2026
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
105 changes: 81 additions & 24 deletions app/Http/Controllers/Ranking/MatchmakingController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
use App\Http\Controllers\Controller;
use App\Models\Beatmap;
use App\Models\MatchmakingPool;
use App\Models\Model;
use App\Transformers\MatchmakingPoolTransformer;
use App\Transformers\MatchmakingUserStatsTransformer;
use Illuminate\Pagination\LengthAwarePaginator;

class MatchmakingController extends Controller
{
Expand All @@ -18,38 +22,91 @@ class MatchmakingController extends Controller
'rating' => [['rating', 'DESC'], ['total_points', 'DESC']],
];

public function __construct()
{
parent::__construct();

$this->middleware('require-scopes:public');
}

public function index(string $rulesetName)
{
$pools = $this->getPoolsQuery($rulesetName)->get();

return json_collection($pools, new MatchmakingPoolTransformer());
}

public function show(?string $rulesetName = null, ?string $poolId = null)
{
$rulesetName ??= default_mode();
$rulesetId = Beatmap::MODES[$rulesetName] ?? abort(422, 'invalid ruleset parameter');
$poolsQuery = $this->getPoolsQuery($rulesetName);

$poolsQuery = MatchmakingPool::where([
'ruleset_id' => $rulesetId,
'type' => 'ranked_play',
])->orderByDesc('active')->orderByDesc('id');
if (is_api_request()) {
// we can just query directly, since the api route
// requires the pool id to be specified
$pool = $poolsQuery->findOrFail($poolId);
} else {
if ($poolId === null) {
$pool = $poolsQuery->firstOrFail();

if ($poolId === null) {
$pool = $poolsQuery->firstOrFail();
return ujs_redirect(route('rankings.matchmaking', ['mode' => $rulesetName, 'pool' => $pool->getKey()]));
}

return ujs_redirect(route('rankings.matchmaking', ['mode' => $rulesetName, 'pool' => $pool->getKey()]));
$pools = $poolsQuery->get();
Comment thread
notbakaneko marked this conversation as resolved.
$pool = $pools->findOrFail($poolId);
}

$pools = $poolsQuery->get();

$pool = $pools->findOrFail($poolId);
$scores = $pool
$scoresQuery = $pool
->allUserStats()
->with('user.team')
->with(['user', 'user.country', 'user.team'])

@nanaya nanaya Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

country shouldn't be preloaded (the relation isn't directly used anymore)

->default()
->orderByDesc('rating')
->paginate()
->withQueryString();

return ext_view('rankings.matchmaking', compact(
'pool',
'pools',
'rulesetName',
'scores',
));
->orderByDesc('rating');

$maxResults = $scoresQuery->count();
$maxPages = ceil($maxResults / Model::PER_PAGE);
$page = get_int(cursor_from_params(\Request::input())['page'] ?? null)
?? get_int(\Request::input('page'))
?? 1;
$page = \Number::clamp($page, 1, $maxPages);

$scores = $scoresQuery
->limit(Model::PER_PAGE)
->offset(Model::PER_PAGE * ($page - 1))
->get();

if (is_api_request()) {
return [
...cursor_for_response(
empty($scores) || $page >= $maxPages ? null : ['page' => $page + 1],
),
'ranking' => json_collection($scores, new MatchmakingUserStatsTransformer(), MatchmakingUserStatsTransformer::RANKING_INCLUDES),
'total' => $maxResults,
];
Comment on lines +77 to +84

@notbakaneko notbakaneko Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

API should stop using offset-limit and use cursors for pagination; see the stuff with WithDbCursorHelper for reference

@LiquidPL LiquidPL Apr 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looking at how the cursor helper works, I'm not sure if it's going to work here. If I understand things correctly, it needs some index that's strictly monotonic across all rows so that the where clause can properly sort rows and do its filtering.

The problem is that several users can have the same rating, which will cause the cursor to skip any records with the same rating that happen to be on the next "page". withRank probably won't work either since it gives the same rank for equal ratings.

Also would this allow fetching an arbitrary page of the rankings? Currently lazer only shows the first page, and it'd be great to bring it to parity with web in that regard.

@notbakaneko notbakaneko Apr 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you can use a tiebreaker column like

osu-web/app/Models/Multiplayer/UserScoreAggregate.php

Lines 32 to 37 in 23dad9d

const SORTS = [
'score_asc' => [
['column' => 'total_score', 'order' => 'ASC'],
['column' => 'last_score_id', 'order' => 'DESC'],
],
];

Although, if it's the same format as web's rankings page listing, then it should probably stick with offset-limit pagination, but it still needs a tie-breakers or it'll show equal values in whatever arbitrary order the db decides on when querying.

@LiquidPL LiquidPL Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah to my understanding the order of those is still undetermined in the current rankings as they are on prod

i'll need to think it through

i initially wanted to order by username but realized it's a pain to pull in other tables for this purpose

@LiquidPL LiquidPL May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wonder if elo_data['approximate_posterior']['sig'] (as seen in #12979) would be an appropriate tiebreaker here, given that it's meant to automatically decay the longer a user has not played. This way we could effectively order people with the same rating so that the most active ones are at the top, which seems instinctively correct.

Mainly I'm wondering if using a field pulled out of JSON for ordering would impact performance negatively. just realized it's already doing that for rating anyway

@LiquidPL LiquidPL May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've made it use the sigma value as a tiebreaker, hopefully it's going to work fine. I'm guessing it also needs the indexes adjusted but I'm not confident in that myslelf.

} else {
$scores = new LengthAwarePaginator(
$scores,
$maxResults,
Model::PER_PAGE,
$page,
['path' => route('rankings.matchmaking', ['mode' => $rulesetName, 'pool' => $pool->getKey()])],
);

return ext_view('rankings.matchmaking', compact(
'pool',
'pools',
'rulesetName',
'scores',
));
}
}

private function getPoolsQuery(string $rulesetName)
{
$rulesetName ??= default_mode();
$rulesetId = Beatmap::MODES[$rulesetName] ?? abort(422, 'invalid ruleset parameter');

return MatchmakingPool::where([
'ruleset_id' => $rulesetId,
'type' => 'ranked_play',
])->orderByDesc('active')->orderByDesc('id');
}
}
8 changes: 8 additions & 0 deletions app/Transformers/MatchmakingUserStatsTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@

class MatchmakingUserStatsTransformer extends TransformerAbstract
{
const array RANKING_INCLUDES = UserStatisticsTransformer::RANKING_INCLUDES;

protected array $availableIncludes = [
'pool',
'user',
];

public function transform(MatchmakingUserStats $stats): array
Expand All @@ -34,4 +37,9 @@ public function includePool(MatchmakingUserStats $stats): ResourceInterface
{
return $this->item($stats->pool, new MatchmakingPoolTransformer());
}

public function includeUser(MatchmakingUserStats $stats): ResourceInterface
{
return $this->item($stats->user, new UserCompactTransformer());
}
}
2 changes: 2 additions & 0 deletions routes/web.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,8 @@
Route::post('notifications/mark-read', 'NotificationsController@markRead')->name('notifications.mark-read');

Route::get('rankings/kudosu', 'RankingController@kudosu');
Route::get('rankings/{mode}/ranked-play', 'Ranking\MatchmakingController@index')->name('rankings.matchmaking.index');
Route::get('rankings/{mode}/ranked-play/{pool}', 'Ranking\MatchmakingController@show')->name('rankings.matchmaking.show');

@nanaya nanaya Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this don't match the web routes

@LiquidPL LiquidPL Apr 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

client expects ranking routes to be in a rankings/<ruleset>/<type> format, just like the already existing RankingController route a line below

i could override that on a case by case basis if that's how it should be, but in my view it is the web routes that are inconsistent (some are <ruleset>/<type> while others are <type>/<ruleset>)

@nanaya nanaya Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

those are from before other type of rankings are added (and to minimise url change during refactor), and some of the newer ones don't even have ruleset so the scheme already fell apart

// GET /api/v2/rankings/:mode/:type
Route::get('rankings/{mode}/{type}', 'RankingController@index')->name('rankings');
Route::resource('spotlights', 'SpotlightsController', ['only' => ['index']]);
Expand Down
32 changes: 32 additions & 0 deletions tests/api_routes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1642,6 +1642,38 @@
"public"
]
},
{
"uri": "api/v2/rankings/{mode}/ranked-play",
"methods": [
"GET",
"HEAD"
],
"controller": "App\\Http\\Controllers\\Ranking\\MatchmakingController@index",
"middlewares": [
"App\\Http\\Middleware\\ThrottleRequests:1200,1,api:",
"App\\Http\\Middleware\\RequireScopes",
"App\\Http\\Middleware\\RequireScopes:public"
],
"scopes": [
"public"
]
},
{
"uri": "api/v2/rankings/{mode}/ranked-play/{pool}",
"methods": [
"GET",
"HEAD"
],
"controller": "App\\Http\\Controllers\\Ranking\\MatchmakingController@show",
"middlewares": [
"App\\Http\\Middleware\\ThrottleRequests:1200,1,api:",
"App\\Http\\Middleware\\RequireScopes",
"App\\Http\\Middleware\\RequireScopes:public"
],
"scopes": [
"public"
]
},
{
"uri": "api/v2/rankings/{mode}/{type}",
"methods": [
Expand Down