Add WebSocket messages for beatmap data and user activity#38071
Add WebSocket messages for beatmap data and user activity#38071tsunyoku wants to merge 23 commits into
Conversation
| // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence. | ||
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| using Newtonsoft.Json; |
There was a problem hiding this comment.
One thing I'm not super sure about is continuing to use Newtonsoft.Json. I know I used it for the PoC message, but I really did not think much about things that should be thought about going forward with a proper implementation of this as it was a singular, low-frequency, small-size message.
The primary reason to go with System.Text.Json instead would be performance. I have not measured myself, but most everyone on the Internet appears to indicate that System.Text.Json is significantly faster at the expense of supporting less bells and whistles.
Another semi-related thing to consider is that maybe the overhead of performing the serialisation itself should be dumped off to a thread to minimise delays on the update thread. Binding to things like OsuGame.Beatmap means that the serialisation code runs on every change, and in particular on the thread that triggered the bindable change, which may degrade performance of something like song select noticeably, what with between the frequency of changes there.
I'm not super sure what to suggest at this stage yet, as I'd have to allocate time to investigate these things myself. Maybe @ppy/team-client wants to chip in here.
There was a problem hiding this comment.
I'm all for avoiding using Newtonsoft for new usages, unless there's good reason to do so. I trust microsoft's implementation more as far as security goes.
There was a problem hiding this comment.
I'd also prefer STJ, just didn't know that was an option here.
bdach
left a comment
There was a problem hiding this comment.
Source read pass before I attempt to run and evaluate further based on the results of running.
There was a problem hiding this comment.
General details of this are mostly fine by me at this point. Review pass below is mostly haggling over minor design / code quality concerns.
At some point this should go through a performance check but I'm not sure when to invest that time. Maybe I'd like one more @ppy/team-client review to go over the general shape of this, and maybe some of the nits mentioned in this fixed, and then I'd do a final "does this work and how fast" check.
Warning
To any onlookers, this API is still rapidly changing. Please expect further breaking changes and do not use this PR to flood requests for what data should be available.
RFC.
Example player state message
{ "beatmap": { "beatmap_id": 974423, "beatmapset_id": 239330, "beatmap_hash": "583aa402376a240ae57f0326c62f2452", "metadata": { "artist": "Rameses B", "artist_unicode": "Rameses B", "title": "We Love", "title_unicode": "We Love", "author": "Milan-", "source": "", "tags": "dnb drum and bass liquid uplifting melodic chopping chill", "user_tags": [] }, "difficulty": { "approach_rate": 8.23, "circle_size": 3, "overall_difficulty": 8.03, "drain_rate": 4 }, "difficulty_name": "Normal", "ruleset_id": 0, "bpm": 306, "star_rating": 3.06, "max_combo": 240, "status": "Ranked" }, "ruleset_id": 0, "mods": [ { "acronym": "DT", "settings": { "speed_change": 1.76 } } ], "type": "PlayerStateWebSocketMessage" }Example user activity message
{"status":"InSoloGame","data":{"beatmap_id":259,"ruleset_id":0},"type":"UserActivityWebSocketMessage"}