Improved solo high scores, basic band high scores #883
Conversation
Solo high scores are now displayed when there is exactly one non-bot player present. High scores are now retrieved per player, song and instrument. Added a new cache for the above when retrieving high scores for the same player + instrument for many songs in a row. Removed instrument icon for high scores on Music Library song list, replaced with a FC indicator sprite (using a placeholder sprite for now). Moved retrieval of solo high scores per song to MusicLibraryMenu. Marked several methods as Obsolete
…eep track of the best percentage and best score (matches existing behaviour) Added basic implementation for Band High Scores Removed magic number in RecommandedSongs
|
Not yet ready to review. Still needs testing. |
thomeval
changed the title
Fixed Best percentage scores not being loaded. Fixed Best percentage score not being displayed if its different to the best score.
Fixed best percentage not loading when multiple profiles have played the same song+instrument combination.
|
Ready for review. |
Added comments for GetBestPercentageScore()
There was a problem hiding this comment.
Hi, sorry for the very late review, I've been very busy recently.
I've listed some styling changes below, however, there's one big thing that I want changed which is how bots are handled at the moment with band scoring.
Bots should never under any circumstances have their scores saved. This wasn't really a problem before, as we only displayed individual scores, but with this change, bands with bot scores in there show up, which obviously doesn't make sense. Once we introduce band multipliers, it will make even less sense, as individual scores will be effected too.
Because of this, all band scores with bots should be completely invalidated and not saved. From the looks of it we currently do, however, I'd classify this as a bug as it really shouldn't be.
Once this change is made, this PR looks pretty much good to me, however I haven't tested it in-game yet so I'll do that once this is fixed.
Thanks!
Edit: if this is changed then also the PlayerContainer.Players.Count(e => !e.Profile.IsBot) == 1 wouldn't be needed and could be simplified as if a bot is in your band, you're still technically in a band and not playing solo.
|
I've updated the branch to fix those style issues. I also absolutely agree that we should not save band high scores (GameRecords) for sessions that have bot involvement, but it would probably make more sense to create a separate PR for that. I do think that we should keep the ShouldDisplaySoloHighScores property as-is though (show solo high scores if there is exactly one non-bot player). Suppose that we have a player that's playing alone, doesn't have any real friends around, and decides to add a couple of bots for a band experience. I may or may not be speaking from experience. In that case, any band scores earned would be invalid, so you'd never see any of the band scores added or updated. To the uninformed player, it would look like the game simply doesn't support high scores at all. On the other hand, that player's solo score would be valid, so I suggest that we display solo scores instead in this case. |
|
#898 has been merged, so if you can update this PR to account for this, I'll review it and merge it. Thanks! |
|
@EliteAsian123 Okay, this PR has been updated with the latest dev branch and is ready to review. |
This PR improves the way solo high scores are handled on the Music Library screen (and elsewhere), bringing it more in-line with RB and CH.
Main changes include:
This PR also introduces a basic version of band high scores, shown when there are no players, or more than one player connected:
These are pulled from the
GameRecordtable. Known limitations: