Show changes to user statistics in-game

[peppy]

Currently there is no replacement for stable's scrollable results screen, which shows changes to online statistics after the gameplay loop. We need to add something similar, and potentially consider cases where statistics change outside the results screen.

[peppy]

@arflyte I brought this up with you a while ago, curious if you have anything yet?

[arflyte]

Unfortunately no, I haven't got to this yet.

[peppy]

@arflyte any updates?

[peppy]

@arflyte any update?

[bdach]

The lack of design aside, the thing I'm wondering here is: how should the data for the display be retrieved?

Now I haven't exactly been keeping up with infra, but on a five minute look the current flow looks to be:

• Client submits score to osu-web
• osu-web receives score, pushes it to redis
• Score is picked up from redis by osu-queue-score-statistics in order to update user stats

Do I have this right? If so, then the immediate issue is that while this system does have eventual consistency, there is no guarantee that the stats update is received before the results screen is shown. The obvious option is to poll periodically and see if user stats change - is this fine, or should the solution be smarter than this somehow?

[peppy]

Everything there is correct thinking, yes.

I do believe we'd eventually want a push flow for this, potentially via osu-server-spectator (see how beatmap metadata is being pushed to the client, as an example), and it may actually fit quite well with the changes smoogi is working on for replay saving (worth checking that out). It might actually be simpler to implement this up-front in some manner rather than figuring out a polling setup.

Or maybe polling will work for now, up to you really. I think getting a flow in place even without a design would be a great starting point.

[peppy]

One thing I should probably mention is that for replay saving, it's only relying on the score being fulfilled (ie. inserted into solo_scores). For stats updates you care more about whether the score has been processed. There's actually a table that tracks that which you may be able to make use of server-side, if looking at the push path.

https://github.com/ppy/osu-queue-score-statistics/blob/6653566bea9510469126d3d97257ccfc15d8f774/osu.Server.Queues.ScoreStatisticsProcessor/Models/ProcessHistory.cs#L11-L18

An entry will only be inserted once processing has completed.

[bdach]

Puttng the pieces together, I think I might have a vague vision in mind of how the push flow could work, but I'd probably be looking to base it on solo_scores_process_history.processed_at? The thought process is that I could use it as a temporal delimiter, like the metadata broadcaster does with queueId, and just not consider any rows older than the last time that score processing state was checked. While the field is commented out in the link above, I am seeing this on my local docker db:

+-------------------+-----------+------+-----+-------------------+-----------------------------------------------+
| Field             | Type      | Null | Key | Default           | Extra                                         |
+-------------------+-----------+------+-----+-------------------+-----------------------------------------------+
| processed_at      | timestamp | NO   |     | CURRENT_TIMESTAMP | DEFAULT_GENERATED on update CURRENT_TIMESTAMP |
+-------------------+-----------+------+-----+-------------------+-----------------------------------------------+

so I presume that the DBMS is going to fill in valid data there because of the default and the trigger. I guess timezones/clock sync may be an issue (but one that can be circumvented rather easily by querying latest processed_at when the spectator server launches, and just always comparing DB timestamps against each other).

Without this field it's really difficult to not have to look up the entire table to scan for new changes. While score_id is still a foreign key to an autoincrement column and thus it could be used as a delimiter, processed_version ruins that a bit, since as I understand that may change when scores are being reprocessed using new score/pp/whatever, so it doesn't look like it's a given that I can entirely ignore it.

The other thing that worries me is that if this does become based on processed_at, then that field may need an index at some point. There isn't one right now on my docker instance (there's only the primary key on score_id).

[peppy]

So there's two ways I can imagine this working.

• We could lean on the redis pubsub implementation (https://redis.io/docs/manual/pubsub/) which is already used for notifications / chat internally, and create a channel exposed by osu-queue-score-statistics which osu-server-spectator can listen to. Benefits are less faffing around with database, but comes at the cost of requiring changes in that project too.
• osu-server-spectator could keep up with all new rows in the processing table and handle any relevant ones (similar to MetadataBroadcaster). Maybe store to a local bucket and then pull out/handle as required.

Also I don't think you need to worry about the processed_at date. For new score submissions entries in the table will appear once and we don't need to worry about which version is there or anything. Just checking for presence of the row.

I'd probably also see a similar process to replay saving, where EndPlaySession triggers a component to begin watching for processing to complete, rather than the client requiring to "subscribe" for the event.

[bdach]

The prospect of having a full flow without polling using redis pubsub is tempting. I guess I'll try that first and dumb down as required if too many complications arise.

[peppy]

Tend to agree. Polling regardless of how it's done is going to end up being ugly due to parallel processing considerations server-side (scores may not be processed in perfect sequential order).

[bdach]

So after a weekend of experimenting I have a... semblance? of a working flow with no polling. In full, at a high level, it looks like this:

sequenceDiagram
    critical Set up redis pubsub channel
        Note over osu-server-spectator,redis: Happens at osu-server-spectator startup.
        osu-server-spectator ->> redis: SUBSCRIBE osu-channel:score:processed
        redis -->> osu-server-spectator: ACK
    end

    critical Set up signalr channel
        Note over osu,osu-server-spectator: Happens at osu startup.
        osu ->> osu-server-spectator: SpectatorClient.OnUserScoreProcessed +=<br>(int userId, long scoreId) => { ... }
        osu-server-spectator -->> osu: ACK
    end

    critical Set up statistics watcher
        Note over osu,osu-web: Happens at osu startup.
        osu ->> osu-web: GET /users/?ids[]={localUserId}
        osu-web -->> osu: UserCompact<br>(with user stats for all rulesets)
    end

    Note over osu-server-spectator,osu-queue-score-statistics: Score submission flow begins here.

    osu ->> osu-web: PUT /beatmaps/{beatmapId}/solo/scores/{tokenId}
    osu-web -->> osu: MultiplayerScore (with populated online ID)

    par
        osu ->>+ osu-server-spectator: SpectatorClient.EndPlaying(SpectatorState)
        Note over osu-server-spectator: osu client is implicitly subscribed<br>for notifications about score<br>with ID from spectator state.
        osu-server-spectator -->>- osu: ACK
    and
        osu-web ->>+ redis: LPUSH osu-queue:score-statistics, Score { id: ... }
        redis -->>- osu-web: ACK

        Note over osu-queue-score-statistics,redis: At some indeterminate point later on...

        osu-queue-score-statistics ->>+ redis: RPOP osu-queue:score-statistics
        redis -->>- osu-queue-score-statistics: Score { id: ... }
        Note over osu-queue-score-statistics: Score is processed.
        osu-queue-score-statistics ->> redis: PUBLISH osu-channel:score:processed<br>ScoreProcessed { scoreId: ..., processedVersion: ... }
        redis -->> osu-queue-score-statistics: ACK
        redis -) osu-server-spectator: ScoreProcessed { scoreId: ..., processedVersion: ... }
        osu-server-spectator -) osu: SpectatorClient.UserScoreProcessed(userId, scoreId)
        osu ->>+ osu-web: GET /me/{rulesetName}
        osu-web -->>- osu: UserCompact<br>(with stats for last play ruleset)
        Note over osu: Compute differences in stats<br>and display to user.
    end

Loading

Questions at this point are:

1. Is the above even remotely decipherable?
2. If yes, then does it look remotely correct?
3. Thoughts on data flow here? The big point is that the client is carrying the brunt of the work here in keeping track of the user's stats and computing the differences in stats to display to the user. I figured approaching it this way and only bubbling up a generic "score was processed" message with an ID can be helpful for Something Else at some point.

The code I have is working but quite dirty, so I need to do cleanup before it can be PRed. But there's something there.

[peppy]

Let's mark this as closed. Thanks to @bdach for the work on the initial implementation (finalised with ppy/osu-server-spectator#157 and #21778).

[bdach]

Building on the initial implementation that only provides the equivalent of stable's "overall ranking", I also attempted to add "beatmap ranking" back, and found that it may be feasible to add that back with relatively low effort and just via client-side changes. Structure would be very similar to what's in place (i.e. all of the work would be done client-side, leveraging web via the API).

Would that be something we'd be interested in getting in place short term, or should I wait and see how the current flow holds up for now?