Country flags: Use QLocale::matchingLocales in place of hardcoded table #2409
Conversation
The country-code-to-flag-image handling has traditionally used a rather large switch/case statement to retain support for Qt4 (according to the comments). We no longer support building with Qt4. A Qt-native approach had already been developed by @corrados, but it was disabled. This commit removes the switch-based lookup and enables the QLocale::matchingLocales-based lookup instead. It has been verified that the new approach handles all of the previously supported codes. The new code adds support for all codes which are available in the current Qt version and where flag icons are available. When building on Qt 5.15.2 or 6.2.3 this results in support for 53 new countries/flags compared to the old logic. Those countries become selectable in clients' user profile settings and in servers' country settings. Clients which are based on the old logic will show a placeholder if they can't resolve an unknown country code. Co-authored-by: Volker Fischer <corrados@users.noreply.github.com>
|
Current status of this PR: I assume the code is done, but I still have to re-run some tests and aggregate the results. That's why I've left it in Draft status for now. |
|
Another update: I have added the before/after test to the PR description and everything looks good, so I'd say the PR is ready for further reviews. |
|
I've checked all newly supported flags and compared them with https://en.wikipedia.org/wiki/Country_code_top-level_domain |
|
All further tests (old client vs. new client vs. old server vs. new server) were successful and did not show any issues. Therefore removing Draft status. Can be merged after another review. |
softins
left a comment
There was a problem hiding this comment.
Well the code looks ok, so I'll approve it. I've built it on my Pi with Qt 5.15.2, and can see the extra territories.
But reading the comments around #2299 (comment), I'm still wondering about the compatibility between Qt5 and Qt6 versions of Jamulus with respect to the country value on the wire in the Jamulus protocol. Do we need a compatibility layer that defines the country codes on the wire and in --serverinfo independently of Qt? (Although they would in fact be the same as the Qt5 numbers).
Thanks!
Yes, we do need that and that's part of #2299. This PR (#2409) only splits out one part (replacement of the flag handling) as it's rather large (code-wise) and simplifies the work in #2299. |
Short description of changes
The country-code-to-flag-image handling has traditionally used a rather large switch/case statement to retain support for Qt4 (according to the comments). We no longer support building with Qt4.
A Qt-native approach had already been developed by @corrados, but it was disabled. This commit removes the switch-based lookup and enables the
QLocale::matchingLocales-based lookup instead.It has been verified that the new approach handles all of the previously supported codes.
The new code adds support for all codes which are available in the current Qt version and where flag icons are available. When building on Qt 5.15.2 or 6.2.3 this results in support for 53 new countries/flags compared to the old logic. IOW, we already have flag icons for 53 countries which are unusable with the current approach.
With this change, those countries become selectable in clients' user profile settings and in servers' country settings. Clients which enounter one of the new countries, but are based on the old logic will show a placeholder if they can't resolve the unknown country code.
This stems from #2299 (comment)
Context: Fixes an issue?
Related: #2299
Does this change need documentation? What needs to be documented and how?
No.
CHANGELOG: GUI: Improve Country selection handling to work with Qt6 and cover 53 previously unsupported territories.
Status of this Pull Request
Ready for review.
What is missing until this pull request can be merged?
Done.
Checklist
Comparison of the results of the old logic vs. the new logic
Setup
Patch main.cpp as follows to make Jamulus output a list of all country codes along with their assigned flag, human-readable name and flag load status
Generate lists for both builds
Patch, run build on master and this PR and capture the outputs
Analysis
Full list