Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show player divisions in scoreboard #6160

Merged
merged 6 commits into from
Jun 30, 2024
Merged

Show player divisions in scoreboard #6160

merged 6 commits into from
Jun 30, 2024

Conversation

BlackYps
Copy link
Contributor

@BlackYps BlackYps commented May 7, 2024

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

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

@BlackYps BlackYps added area: ui Anything to do with the User Interface of the Game ui: scoreboard related to scoreboard ui labels May 7, 2024
@BlackYps BlackYps requested a review from Garanas May 7, 2024 23:06
@Garanas
Copy link
Member

Garanas commented May 8, 2024

🤔 , 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
Copy link
Contributor Author

BlackYps commented May 8, 2024

According to this #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
Copy link
Member

MrRowey commented May 9, 2024

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

@BlackYps
Copy link
Contributor Author

BlackYps commented May 9, 2024

Custom games still show the rating. UI mods are unaffected

@Garanas Garanas changed the base branch from deploy/fafdevelop to develop June 1, 2024 19:30
@MrRowey
Copy link
Member

MrRowey commented Jun 12, 2024

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

@MrRowey
Copy link
Member

MrRowey commented Jun 13, 2024

@BlackYps How can i test this

@BlackYps
Copy link
Contributor Author

BlackYps commented Jun 13, 2024

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
Copy link
Member

Garanas commented Jun 21, 2024

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

@BlackYps
Copy link
Contributor Author

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
Copy link
Member

MrRowey commented Jun 30, 2024

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
Copy link
Contributor Author

Yes, I think this is an acceptable risk

@BlackYps BlackYps merged commit 2e2b822 into develop Jun 30, 2024
5 checks passed
@BlackYps BlackYps deleted the division-scoreboard branch June 30, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ui Anything to do with the User Interface of the Game type: testing needed ui: scoreboard related to scoreboard ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add division to score board for matchmaker games
3 participants