Enable NRT for multiplayer and playlists #30807
Conversation
This one is super duper annoying to test, because we already have e.g. `TestScenePlaylistsScreen`. The only way to test it would be to use an `OsuGameTestScene`. Maybe this is okay?
Yeah that one is a stinker. To call it overloaded with functionality is underselling it. If you find yourself taking a hammer to that atrocity I'd be overjoyed. |
|
It'll happen at some point for sure... |
| public RoomStatusFilter Status; | ||
| public string Category; | ||
| public RulesetInfo Ruleset; | ||
| public string Category = string.Empty; |
There was a problem hiding this comment.
Would also accept string? here, but either works
| [Resolved(CanBeNull = true)] | ||
| private IdleTracker? idleTracker { get; set; } |
There was a problem hiding this comment.
I wonder a bit, at this point the CanBeNull thing on ResolvedAttribute is redundant right? Like the NRT annotation on the field itself is enough to convey the same meaning? Is there any reason to keep specifying it then?
There was a problem hiding this comment.
It is redundant, yes. I kept it for conformity. We have an analyser to automatically clean up or warn, but it was disabled due to iOS/Mono not supporting NRT completely back in the day.
These days we compile with source gen'd activators as well so it should be a non-issue in any case.
| @@ -84,7 +83,7 @@ public GameTypePickerItem(MatchType value) | |||
| }; | |||
| } | |||
|
|
|||
| private Sample selectSample; | |||
| private Sample selectSample = null!; | |||
There was a problem hiding this comment.
A little funny, most places will declare samples as nullable, I guess for safety against the audio store lookup failing. Not something that matters either way, just noticed in passing.
There was a problem hiding this comment.
Yeah this is a very weird one and I was a bit conflicted in what changes to make. I decided to keep existing code as much as possible at the end of the day such that if something was used in a may-be-null way then I tended towards assuming null is still a valid value, and suspending my own analysis. Except things in load() and similar.
We can probably do a pass through the rest of the game and remove this pattern later.
Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
Most relevant changes:
RoomSubScreen.CreateGameplayScreen()-PlaylistItemparameter added with non-null precondition inRoomSubScreen.StartPlay().DrawableRoomPlaylistItem- this class is totally boned, with a virtual method call that's never called in the base class. Fields made nullable because there is at least one usage that doesn't call said virtual method at all.OnlinePlayScreen- moved screen stack construction to class init because it's referenced externally viaCursorVisible.