Zandronum Chat on our Discord Server Get the latest version: 3.1
Source Code
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003482Doomseeker[All Projects] Suggestionpublic2018-09-01 11:222018-10-27 22:54
ReporterWubTheCaptain 
Assigned ToZalewa 
PrioritynormalSeveritytweakReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.1 
Target Version1.2Fixed in Version1.2 
Summary0003482: Add visual text indication to Appearance configuration if/when the program needs to be restarted for changes to apply (locales)
DescriptionSince commit acb43c63c19086f8d592380b66cbb43d77f76366 in Doomseeker 1.2~beta, there's a line printed to log (stdout) telling the program needs to be restarted. In regular use, this line is unfortunately not often to be seen. The user could be left slightly clueless, deprived of any hint that the program needs to be restarted for changes to apply in entirety.

I figured there's also no visual text indication when a program needs to be restarted in Doomseeker 1.1 for language changes to take effect, if this issue even applies there.

Display a warning message in "Appearance" configuration when the appropriate conditions exist for program restart required.
Steps To Reproduce
  1. Start Doomseeker.
  2. Press the F5 key to open the configuration or find Options → Configuration from the top crate navigation.
  3. Navigate to "Appearance" settings.
  4. Select another language from the dropdown menu.
  5. Press "Ok" or "Apply".
  6. Notice many parts of the GUI are not translated, such as server filter.
  7. Optionally, revert back to the language prior (e.g., English) and apply changes. Some parts will still be or won't be translated.
  8. A program restart is required, but there is no warning message in the "Appearance" configuration window to say this in a foolproof way.
Additional Informationsrc/core/gui/configuration/cfgappearance.cpp CFGAppearance::saveSettings().
Attached Filespng file icon 2018-09-22-171617_524x176_scrot.png [^] (29,700 bytes) 2018-09-22 17:31

- Relationships
related to 0003260closedZalewa Translation "en_US" cannot be found, unknown language definition 
child of 0003479closedZalewa --create-game arg 

-  Notes
User avatar (0019520)
Pol M (developer)
2018-09-13 20:11

 pull request
User avatar (0019524)
Zalewa (developer)
2018-09-15 07:41
edited on: 2018-09-15 07:42

 I think this works as intended now, with the message box appearing after the config box is closed.

There's a slight misdemeanor when you switch to a different language, apply and then go back to the original one and apply again. Even though you haven't changed the language effectively, the popup still appears, as the partial translation already happens immediately after you press the apply button. It could be delayed to occur after you close the config box and only if the user has effectively changed the locale. Alternative idea is to completely drop the dynamic retranslation code, but we cannot do that as the message box that tells the user to restart the program should appear in the language the user has chosen.
User avatar (0019540)
WubTheCaptain (reporter)
2018-09-18 12:14
edited on: 2018-09-18 12:36
Quote
Doomseeker needs to restart to be able to apply some changes


→ "Doomseeker needs to be restarted for some changes to be applied. [Would you like to restart Doomseeker now? [Yes/No]]"

Notice missing period and the copyedit.

The choice is optional, but would have increased usability. Alternatively, see below.

Quote
QMessageBox::warning(NULL, tr("Restart needed"), warningRestartNeeded);


Use of QMessageBox is somewhat inconsistent with other warnings in the Configuration menu, and griefing like Zalewa mentioned. I'd take a look at how Wadseeker's General configuration with missing/invalid paths creates that QPixmap(":/icons/exclamation_16.png") string thingy and add it to the Appearance menu instead when the language option is changed (before pressing "OK" or "Apply"), replacing the popup box.

I think this is more of an enhancement than required functionality for this ticket, though. I would create a new ticket for this.
User avatar (0019543)
Pol M (developer)
2018-09-18 16:31

 hey, I've been reading the code and I have noticed the existence of importantMessagesWidget which helps with the creation of small messages, just like this one... I'm quite sure that Zalewa was referring to this, my bad, I had to go to sleep :/
I'm gonna change the QMessage to importantMessagesWidget and change the text to:
Quote from WubTheCaptain

"Doomseeker needs to be restarted for some changes to be applied."


In case that the yes/no buttons are really mandatory, I'd have to add a custom entry in mainwindow.ui, like the server filter text. I personally find it unnecessary.

Also, I tried to extend this to make the message callable in case any future configuration required to restart, instead of having to make other messages pop up in all of those options, but since right now the only plausible one is the locale, I would understand if you prefer the second recommendation.
User avatar (0019544)
Pol M (developer)
2018-09-18 16:55

 pull request
User avatar (0019550)
WubTheCaptain (reporter)
2018-09-18 22:13
edited on: 2018-09-18 22:24

 Pol M: As a sidenote, I think it'd be easier to test your things if you used a single repository with multiple named Mercurial branches. See'https://www.mercurial-scm.org/wiki/NamedBranches [^]' for more info. Right now you're creating and uploading multiple copies of the Doomseeker repository to Bitbucket with unusual project identifiers, wasting some bandwidth in the process – and causing me a little bit of griefance, because I can't notice/fetch/pull/update your changes from a single repository URL for new branches. In other words: You have 7 different "forks" of Doomseeker, not one; all with a single "default" branch, not seperated branches from upstream.

In other words: I would prefer to pull all your changes for testing from'https://bitbucket.org/Pol_M/doomseeker [^]' with "default" branch being the upstream, and "blah-whatever-feature-or-fix-you-name" branch being your change for that thing – and you can continue pushing to "blah-whatever-feature-or-fix-you-name" for further changes of the same ticket.
User avatar (0019551)
Pol M (developer)
2018-09-18 22:23

 Sorry about that, it just is the quickest way to get my things done. I'm quite new to version control systems where there is more than just me making changes. I'll try to learn about branches for the next ticket, and use them instead of having multiple forks.

Thanks for the link :D
User avatar (0019577)
WubTheCaptain (reporter)
2018-09-19 23:56
edited on: 2018-09-19 23:56
Quote from WubTheCaptain
Pol M: As a sidenote, I think it'd be easier to test your things if you used a single repository with multiple named Mercurial branches.


s/branches/bookmarks/, as pointed out by Blutz3. (https://www.mercurial-scm.org/wiki/Bookmarks)
User avatar (0019578)
Pol M (developer)
2018-09-20 05:43

 No worries XD
User avatar (0019581)
Zalewa (developer)
2018-09-20 16:15

 Let's not add the "Restart? Yes/No" buttons for now, the notification is sufficient. PR has been merged.
User avatar (0019583)
Pol M (developer)
2018-09-20 16:41

 I agree with Zalewa tbh :P
User avatar (0019641)
WubTheCaptain (reporter)
2018-09-22 17:04

 Polish translation needs to be updated. Zalewa? (Assigned)

The solution is not exactly what I expected, but it's good and reasonable. The inconsistencies with other notifications/warnings in the appearance config I've already mentioned here.

It would be even better if there was that QPixmap(":/icons/exclamation_16.png") in the message. Should this be added before closing this ticket?
User avatar (0019642)
WubTheCaptain (reporter)
2018-09-22 17:06
edited on: 2018-09-22 17:07

 Actually, I take some of what I just said back. There's a bug.

Since 0003479 was implemented, there is no information about a restart being required with --create-game option. If at all possible, do the QPixmap(":/icons/exclamation_16.png") inside appearance configuration – much like Wadseeker settings and such.
User avatar (0019643)
WubTheCaptain (reporter)
2018-09-22 17:16
Quote from WubTheCaptain
If at all possible, do the QPixmap(":/icons/exclamation_16.png") inside appearance configuration – much like Wadseeker settings and such.


Because I've probably been misunderstood few times in this ticket, I'll attach a screenshot showing what I mean.
User avatar (0019653)
Zalewa (developer)
2018-09-22 19:56

 There are two reasons why the exclamation mark notification appearing below the language settings is not used in this case:

1. This notification is used for erroreneous configurations to explicitly show the user that something is wrong. User can then see the bad settings and fix them until all exclamation marks disappear. If we used the same stylization for the restart notification, which is not an error, would then be stylized in the same way as an error. However, this problem is solvable by using a different icon than the exclamation mark.

2. The notification will not appear in the language that the user has chosen, but in the currently loaded language. Do you think this will be a problem?
User avatar (0019654)
WubTheCaptain (reporter)
2018-09-22 20:04
edited on: 2018-09-22 20:05
Quote from Zalewa
However, this problem is solvable by using a different icon than the exclamation mark.


Agreed with the issue and solution. Blue is a neutral color for information exclamation. 🛈 / ⃝ℹ

Quote from Zalewa
The notification will not appear in the language that the user has chosen, but in the currently loaded language. Do you think this will be a problem?


This is currently experienced with the "clear" button in importantMessagesWidget.

I don't think it will be too big of a problem, if the user changes the language before hitting "OK" or "Apply" to close the dialog. It's "not fun" to introduce more dynamic translation bugs – which already plague the code and make it more difficult to solve later, if attempted – but I think the benefits of doing this outweight the negatives, but only because 0003479 was implemented. If 0003479 wasn't implemented, then I would've said the opposite and left it as is (with the importantMessagesWidget message).
User avatar (0019655)
Zalewa (developer)
2018-09-22 20:59

 The discussed label added here:'https://bitbucket.org/Doomseeker/doomseeker/commits/29d58e568aa9e727ebc5ab440dd7ac9e3bdcb90d [^]'
User avatar (0019658)
WubTheCaptain (reporter)
2018-09-22 21:37
edited on: 2018-09-22 21:38
Quote from Zalewa
The discussed label added here:'https://bitbucket.org/Doomseeker/doomseeker/commits/29d58e568aa9e727ebc5ab440dd7ac9e3bdcb90d
[^]'

Selecting another language, which displays the "Restart will be required to apply the translation fully." message (is this professional English?), causes the configuration window to resize (becomes wider). It's a paper cut bug.

Otherwise, this is good for me to close as "resolved". Icon source and copyright seems fine at glance.
User avatar (0019660)
WubTheCaptain (reporter)
2018-09-22 21:43
edited on: 2018-09-22 21:44

 Actually, one more thing Zalewa.

The red exclamation warnings display the icon on the left in the "tree view" of the configuration (for lack of better word), but that's not the case for this informational exclamation.
User avatar (0019662)
Zalewa (developer)
2018-09-22 21:51
edited on: 2018-09-22 21:51
Quote from Wub
the configuration becomes wider

On Windows 7 it doesn't. I can't cover for all possible font sizes, UI styles and string length variations depending on the current translation. UI will inevitably shift around as new elements are added to it.


Quote from Wub
The red exclamation warnings display the icon on the left in the "tree view" of the configuration (for lack of better word), but that's not the case for this informational exclamation.

No it doesn't. That's on purpose. Again, the exclamations inform the user that something is wrong and they do that immediately after the config box is open. The retranslation information shows up only on user's direct modification of the combo box above it. I didn't expect I'd have to explain that.
User avatar (0019664)
WubTheCaptain (reporter)
2018-09-22 21:54
Quote from Zalewa
I didn't expect I'd have to explain that.


Okay, thank you the clarification to your intentions. And thanks for your contributions.

Polish translation will probably come later.
User avatar (0019884)
Zalewa (developer)
2018-10-02 16:58
Quote from Wub

Selecting another language, which displays the "Restart will be required to apply the translation fully." message (is this professional English?), causes the configuration window to resize (becomes wider). It's a paper cut bug.


Being more creative provides better results: I shortened the notification text. See if the resize problem is gone now for you.

'https://bitbucket.org/Doomseeker/doomseeker/commits/9334fe8924f1ade53bf1bcd8c7f72ab0fc60ca28 [^]'
User avatar (0019885)
WubTheCaptain (reporter)
2018-10-02 17:04
Quote from Zalewa
https://bitbucket.org/Doomseeker/doomseeker/commits/9334fe8924f1ade53bf1bcd8c7f72ab0fc60ca28


Missing period, please restore.
User avatar (0019886)
Zalewa (developer)
2018-10-02 17:49

 'https://bitbucket.org/Doomseeker/doomseeker/commits/f6ac7b72e887bbdae1e758fefe9b3d68909e1fe5 [^]'
User avatar (0019887)
WubTheCaptain (reporter)
2018-10-02 19:22
Quote from Zalewa
See if the resize problem is gone now for you.


That's also fixed.
User avatar (0019888)
WubTheCaptain (reporter)
2018-10-02 19:26
Quote
Full retranslation requires restart.


→ "Full retranslation requires a restart."

Sorry, didn't catch this earlier.
User avatar (0019889)
Zalewa (developer)
2018-10-02 19:32

 'https://bitbucket.org/Doomseeker/doomseeker/commits/d709267b230bd9196fcb553b41260cda1342caf3 [^]'

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2018-09-01 11:22 WubTheCaptain New Issue
2018-09-01 11:23 WubTheCaptain Relationship added related to 0003260
2018-09-10 05:29 Zalewa Relationship added related to 0003483
2018-09-12 15:03 Pol M Assigned To => Pol M
2018-09-12 15:03 Pol M Status new => assigned
2018-09-13 20:11 Pol M Note Added: 0019520
2018-09-13 20:12 Pol M Status assigned => needs review
2018-09-15 07:41 Zalewa Note Added: 0019524
2018-09-15 07:41 Zalewa Status needs review => needs testing
2018-09-15 07:42 Zalewa Note Edited: 0019524 View Revisions
2018-09-18 11:48 WubTheCaptain Target Version => 1.2
2018-09-18 12:14 WubTheCaptain Note Added: 0019540
2018-09-18 12:14 WubTheCaptain Status needs testing => needs review
2018-09-18 12:17 WubTheCaptain Status needs review => assigned
2018-09-18 12:36 WubTheCaptain Note Edited: 0019540 View Revisions
2018-09-18 12:36 WubTheCaptain Note Edited: 0019540 View Revisions
2018-09-18 16:31 Pol M Note Added: 0019543
2018-09-18 16:55 Pol M Status assigned => needs review
2018-09-18 16:55 Pol M Note Added: 0019544
2018-09-18 22:13 WubTheCaptain Note Added: 0019550
2018-09-18 22:14 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:15 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:20 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:21 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:22 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-18 22:23 Pol M Note Added: 0019551
2018-09-18 22:24 WubTheCaptain Note Edited: 0019550 View Revisions
2018-09-19 23:56 WubTheCaptain Note Added: 0019577
2018-09-19 23:56 WubTheCaptain Note Edited: 0019577 View Revisions
2018-09-20 05:43 Pol M Note Added: 0019578
2018-09-20 16:15 Zalewa Note Added: 0019581
2018-09-20 16:41 Pol M Note Added: 0019583
2018-09-20 16:41 Pol M Status needs review => needs testing
2018-09-22 17:04 WubTheCaptain Note Added: 0019641
2018-09-22 17:04 WubTheCaptain Assigned To Pol M => Zalewa
2018-09-22 17:04 WubTheCaptain Status needs testing => needs review
2018-09-22 17:06 WubTheCaptain Note Added: 0019642
2018-09-22 17:07 WubTheCaptain Note Edited: 0019642 View Revisions
2018-09-22 17:08 WubTheCaptain Relationship added child of 0003479
2018-09-22 17:16 WubTheCaptain Note Added: 0019643
2018-09-22 17:17 WubTheCaptain File Added: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:30 WubTheCaptain File Deleted: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:31 WubTheCaptain File Added: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:47 WubTheCaptain Status needs review => assigned
2018-09-22 19:56 Zalewa Note Added: 0019653
2018-09-22 20:04 WubTheCaptain Note Added: 0019654
2018-09-22 20:05 WubTheCaptain Note Edited: 0019654 View Revisions
2018-09-22 20:59 Zalewa Note Added: 0019655
2018-09-22 20:59 Zalewa Status assigned => needs testing
2018-09-22 21:37 WubTheCaptain Note Added: 0019658
2018-09-22 21:37 WubTheCaptain Status needs testing => needs review
2018-09-22 21:38 WubTheCaptain Note Edited: 0019658 View Revisions
2018-09-22 21:38 WubTheCaptain Note Edited: 0019658 View Revisions
2018-09-22 21:43 WubTheCaptain Note Added: 0019660
2018-09-22 21:44 WubTheCaptain Note Edited: 0019660 View Revisions
2018-09-22 21:51 Zalewa Note Added: 0019662
2018-09-22 21:51 Zalewa Note Edited: 0019662 View Revisions
2018-09-22 21:52 Zalewa Status needs review => resolved
2018-09-22 21:52 Zalewa Fixed in Version => 1.2
2018-09-22 21:52 Zalewa Resolution open => fixed
2018-09-22 21:54 WubTheCaptain Note Added: 0019664
2018-10-01 04:01 WubTheCaptain Relationship deleted related to 0003483
2018-10-02 16:58 Zalewa Note Added: 0019884
2018-10-02 17:04 WubTheCaptain Note Added: 0019885
2018-10-02 17:04 WubTheCaptain Status resolved => needs review
2018-10-02 17:05 WubTheCaptain Resolution fixed => reopened
2018-10-02 17:49 Zalewa Note Added: 0019886
2018-10-02 19:22 WubTheCaptain Note Added: 0019887
2018-10-02 19:22 WubTheCaptain Status needs review => resolved
2018-10-02 19:22 WubTheCaptain Resolution reopened => fixed
2018-10-02 19:26 WubTheCaptain Note Added: 0019888
2018-10-02 19:26 WubTheCaptain Status resolved => assigned
2018-10-02 19:26 WubTheCaptain Resolution fixed => reopened
2018-10-02 19:32 Zalewa Note Added: 0019889
2018-10-02 19:32 Zalewa Status assigned => resolved
2018-10-02 19:32 Zalewa Resolution reopened => fixed
2018-10-27 22:54 WubTheCaptain Status resolved => closed




Questions or other issues? Contact Us.

Links

Copyright © 2000 - 2024 MantisBT Team
Powered by Mantis Bugtracker