ppy / osu-web

the browser-facing portion of osu!
https://osu.ppy.sh
GNU Affero General Public License v3.0
983 stars 386 forks source link

User count on home page doesn't include lazer players #10991

Closed peppy closed 9 months ago

peppy commented 9 months ago

Supersedes and closes https://github.com/ppy/osu-web/pull/10849.

osu-web should add a migration:

alter table osu_banchostats add users_lazer mediumint unsigned default 0 not null after users_osu, algorithm=inplace;

This column will be updated by bancho, to keep things simple. It is using the same logic as osu-server-spectator to aggregate user totals. It will be deployed on monday.

A proposed (untested) osu-web patch would be something like:

diff --git a/app/Libraries/CurrentStats.php b/app/Libraries/CurrentStats.php
index c3b163601c..2eecacd4d8 100644
--- a/app/Libraries/CurrentStats.php
+++ b/app/Libraries/CurrentStats.php
@@ -25,7 +25,7 @@ class CurrentStats
             $latest = array_last($stats);

             return [
-                'currentOnline' => $latest['users_osu'] ?? 0,
+                'currentOnline' => $latest['users_osu'] + $latest['users_lazer'] ?? 0,
                 'currentGames' => $latest['multiplayer_games'] ?? 0,
                 'graphData' => array_to_graph_json($stats, 'users_osu'),
                 'totalUsers' => Count::totalUsers()->count,
diff --git a/app/Models/BanchoStats.php b/app/Models/BanchoStats.php
index c9a3f01f6d..a8460baa1b 100644
--- a/app/Models/BanchoStats.php
+++ b/app/Models/BanchoStats.php
@@ -23,7 +23,7 @@ class BanchoStats extends Model
     {
         return array_reverse(
             static::whereRaw('banchostats_id mod 10 = 0')
-                ->select(['users_irc', 'users_osu', 'multiplayer_games', 'date'])
+                ->select(['users_irc', 'users_osu + users_lazer', 'multiplayer_games', 'date'])
                 ->orderBy('banchostats_id', 'DESC')
                 ->limit(24 * 60 / 10)
                 ->get()
nanaya commented 9 months ago

what 🤔

is it a duplicate of #10816, of which the supposed fix is somewhere else?

peppy commented 9 months ago

Was trying to find that issue, but yeah. This is an alternate solution proposed and generally agreed upon from our end to ensure the graph is also handled correctly. I want to keep historical stats for lazer intact and separate from stable, and this seems the best way, but I'm open to other proposals if there's issue.