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

Display server region in serverbrowser #479

Merged
merged 15 commits into from
Dec 4, 2022

Conversation

GeckoEidechse
Copy link
Member

@GeckoEidechse GeckoEidechse commented Aug 8, 2022

mp_lobby0015

This replaces the existing and currently still unused latency column in the serverbrowser with a new column called "Region". Note that I have replaced most of the references to "*latency*" with "*region*".

Still missing are:

  • Filtering serverbrowser list by region using the search field
  • Localisations
    • English
    • French
    • German
    • Italian
    • Japanese
    • Mexican Spanish
    • Portuguese
    • Russian
    • Spanish
    • Traditional Chinese

Depends on R2Northstar/NorthstarMasterServer#84 and R2Northstar/NorthstarLauncher#232 (merged)

@GeckoEidechse GeckoEidechse added feedback wanted Feedback is wanted whether the changes by this PR are desired depends on another PR Blocked until the PR it depends on is merged labels Aug 8, 2022
@GeckoEidechse GeckoEidechse marked this pull request as draft August 8, 2022 12:15
@GeckoEidechse
Copy link
Member Author

Set to draft as the PR it depends on is not yet merged.

@uniboi
Copy link
Contributor

uniboi commented Aug 18, 2022

What's blocking this?

@GeckoEidechse
Copy link
Member Author

What's blocking this?

It needs R2Northstar/NorthstarMasterServer#84 and R2Northstar/NorthstarLauncher#232 to be merged first.

@GeckoEidechse
Copy link
Member Author

Alignment is scuffed but still works otherwise :D

image

@uniboi
Copy link
Contributor

uniboi commented Nov 20, 2022

isn't that how the alignment is supposed to be? centered?

@GeckoEidechse
Copy link
Member Author

isn't that how the alignment is supposed to be? centered?

Yes but becuase most names max out the column it looks a bit weird IMO. Doesn't matter though cause R2Northstar/Atlas#13 should shorten things a bit ^^

@pg9182
Copy link
Member

pg9182 commented Nov 26, 2022

@GeckoEidechse

@GeckoEidechse
Copy link
Member Author

Still in draft cause I'm missing translations, pinging translators for it ^^

@uniboi
Copy link
Contributor

uniboi commented Nov 26, 2022

Translations still default to english so it shouldn't matter

@GeckoEidechse GeckoEidechse marked this pull request as ready for review November 26, 2022 12:35
@GeckoEidechse
Copy link
Member Author

Translations are done now and PR is ready for review :>

@GeckoEidechse GeckoEidechse added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code and removed depends on another PR Blocked until the PR it depends on is merged feedback wanted Feedback is wanted whether the changes by this PR are desired labels Nov 26, 2022
Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, didnt test ingame

@uniboi
Copy link
Contributor

uniboi commented Dec 2, 2022

@pg9182 can you say what you reviewed

@GeckoEidechse GeckoEidechse removed the needs code review Changes from PR still need to be reviewed in code label Dec 3, 2022
@GeckoEidechse
Copy link
Member Author

What @uniboi said tbh...

Also @uniboi wanna test the PR? :>

Copy link
Contributor

@uniboi uniboi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
code looks good at a first glance and works in testing

@GeckoEidechse GeckoEidechse merged commit a4606ba into main Dec 4, 2022
@GeckoEidechse GeckoEidechse deleted the display-region-in-serverbrowser branch December 4, 2022 22:04
@ASpoonPlaysGames ASpoonPlaysGames removed the needs testing Changes from the PR still need to be tested label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants