Add vocals censorship chart feature (core)#405
Conversation
- plus some stuff i forgot in the previous commit
- and remove it from the engine parameters
|
I have not done a full review, but a quick read-through of the diff did not show any obvious glaring architectural issues, so I don't think any massive redesign is going to be required before merging this. Thanks for handling this, I know there are some team members who are quite interested. |
wyrdough
left a comment
There was a problem hiding this comment.
This is what I can see from looking at the diff alone. Feel free to push back, because I'm definitely missing context.
|
|
||
| public int PlaylistTrack => _metadata.PlaylistTrack; | ||
|
|
||
| public SongRating SongRating => _metadata.SongRating; |
There was a problem hiding this comment.
Shouldn't this have changed to public SongRating SongRating => GetSongRating(); instead of being deleted outright?
There was a problem hiding this comment.
I wasn't sure if I should enforce an explicit contract with any code that gets SongRating - essentially, should they be required to consider explicitly whether to return that with censorship enabled or not?
As described here: https://docs.google.com/document/d/1L5yDejAOpbSnDvicgh6Id0aIeMewORVBQRStErAnogw/edit?usp=sharing
This encompasses three parts, each of which are technically independent from each other, but all of which depend on the "Censor Mature Content" setting being enabled in YARG:
clean_vocals = 1is found in the ini, drop the rating to "Supervision Required".[CENSORSHIP_MARKERS]text event is found in that vocals part, anywhere where MIDI note 104 is covering a vocal note, remove the note, and replace the entire word with a dash. The state of this setting is also stored in replays, to ensure correct playback.While I am relatively happy with the stem and content rating implementations, I am unsure about the chart stuff. The way I have it implemented here is as a public post-processing step in SongChart, which applies the transformation. Not sure if this is better, or if passing it further up the chain to MoonSongLoader (or handling it in GameManager) is a better idea. Furthermore, because of the way this works, it requires looking at vocal notes in MoonSongLoader's lyric parsing which doesn't deal with flags at all, making it a bit less clean.
Also, sorry about the random whitespace changes, I have no idea how to make Rider stop doing that.
Depends on YARC-Official/YARG#1529