-
Notifications
You must be signed in to change notification settings - Fork 423
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
1b92bc7
23dad9d
fac2db3
3664080
954c042
ff724a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||||||||||
| { | ||||||||||||||
|
|
@@ -18,38 +22,92 @@ 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(); | ||||||||||||||
| $pool = $pools->findOrFail($poolId); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| $pools = $poolsQuery->get(); | ||||||||||||||
|
|
||||||||||||||
| $pool = $pools->findOrFail($poolId); | ||||||||||||||
| $scores = $pool | ||||||||||||||
| $scoresQuery = $pool | ||||||||||||||
| ->allUserStats() | ||||||||||||||
| ->with('user.team') | ||||||||||||||
| ->with(['user', 'user.team']) | ||||||||||||||
| ->default() | ||||||||||||||
| ->orderByDesc('rating') | ||||||||||||||
| ->paginate() | ||||||||||||||
| ->withQueryString(); | ||||||||||||||
|
|
||||||||||||||
| return ext_view('rankings.matchmaking', compact( | ||||||||||||||
| 'pool', | ||||||||||||||
| 'pools', | ||||||||||||||
| 'rulesetName', | ||||||||||||||
| 'scores', | ||||||||||||||
| )); | ||||||||||||||
| ->orderBy('sigma'); | ||||||||||||||
|
|
||||||||||||||
| $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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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". 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?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); | ||
|
|
||
| use Illuminate\Database\Migrations\Migration; | ||
| use Illuminate\Database\Schema\Blueprint; | ||
| use Illuminate\Support\Facades\Schema; | ||
|
|
||
| return new class extends Migration | ||
| { | ||
| public function up(): void | ||
| { | ||
| Schema::table('matchmaking_user_stats', function (Blueprint $table) { | ||
| $table->double('sigma')->storedAs("JSON_EXTRACT(elo_data, '$.approximate_posterior.sig')"); | ||
| }); | ||
| } | ||
|
|
||
| public function down(): void | ||
| { | ||
| Schema::table('matchmaking_user_stats', function (Blueprint $table) { | ||
| $table->dropColumn('sigma'); | ||
| }); | ||
| } | ||
| }; |
Uh oh!
There was an error while loading. Please reload this page.