Show player divisions in scoreboard

BlackYps commented 2 months ago

Description of the proposed changes

Show the division of players in the scoreboard instead of rating if available (i.e. in matchmaker games) Closes #4258

Testing done on the proposed changes

Tested them on a matchmaker replay, as well as a normal game to make sure that the rating still shows up when no divisions are available.

The missing space between division and subdivision is because the added space in the game options is not present in current replays. This is just an artefact of the testing process and will be fixed once the release happened.

grafik

Checklist

[x] Changes are annotated, including comments where useful
[x] Changes are documented in the changelog for the next game version

Garanas commented 2 months ago

🤔 , I thought we also merged the images of the badges at some point in the past. We could use those instead of the text

BlackYps commented 2 months ago

According to this https://github.com/FAForever/fa/pull/4263 using the images causes problems with the layout. I have no experience with the ui elements of the game, so I don't feel competent to possibly address these problems. And I would rather have a text implementation than none at all. Seeing that the linked PR is over a year old I don't think someone will show up and implement the images anytime soon.

But even if they do, it doesn't hurt to have the text solution in the meantime.

MrRowey commented 1 month ago

What will this show in custom games tho? and how will it affect Scoreboard UI Mods ?

BlackYps commented 1 month ago

Custom games still show the rating. UI mods are unaffected

MrRowey commented 3 weeks ago

I think it would be better to have it blank rather than it saying unlisted

MrRowey commented 3 weeks ago

@BlackYps How can i test this

BlackYps commented 3 weeks ago

It's a bit tricky to test. You can test the changes in score.lua by simply running replays of custom and matchmaker games. But then you have the old behaviour of autolobby.lua. I haven't found a way to test the autlobby.lua changes. For this you would have to launch an automatch with this branch, I'm not sure if this is possible. Maybe @Garanas knows a way

Garanas commented 2 weeks ago

I do not - I've asked about this before on Zulip but there's no trivial approach to test it at this moment.

BlackYps commented 1 week ago

I don't understand the "testing needed" label here. There is no further testing that can be done. We should merge it in and then see if it behaves correctly in matchmaker games. Which I believe it does.

MrRowey commented 1 week ago

the issue is that this is not going to be a visible change till it deplys in to main faf, then if something is broken it's going to require a hotfix as it can't be tested with out using it to the main.

BlackYps commented 1 week ago

Yes, I think this is an acceptable risk

FAForever / fa