MantisBT - Doomseeker
View Issue Details
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().
TagsNo tags attached.
Relationships
related to 0003260closed Zalewa Translation "en_US" cannot be found, unknown language definition 
child of 0003479closed Zalewa --create-game arg 
Attached Filespng 2018-09-22-171617_524x176_scrot.png (29,700) 2018-09-22 17:31
https://zandronum.com/tracker/file_download.php?file_id=2408&type=bug
png
Issue History
Date ModifiedUsernameFieldChange
2018-09-01 11:22WubTheCaptainNew Issue
2018-09-01 11:23WubTheCaptainRelationship addedrelated to 0003260
2018-09-10 05:29ZalewaRelationship addedrelated to 0003483
2018-09-12 15:03Pol MAssigned To => Pol M
2018-09-12 15:03Pol MStatusnew => assigned
2018-09-13 20:11Pol MNote Added: 0019520
2018-09-13 20:12Pol MStatusassigned => needs review
2018-09-15 07:41ZalewaNote Added: 0019524
2018-09-15 07:41ZalewaStatusneeds review => needs testing
2018-09-15 07:42ZalewaNote Edited: 0019524bug_revision_view_page.php?bugnote_id=19524#r11787
2018-09-18 11:48WubTheCaptainTarget Version => 1.2
2018-09-18 12:14WubTheCaptainNote Added: 0019540
2018-09-18 12:14WubTheCaptainStatusneeds testing => needs review
2018-09-18 12:17WubTheCaptainStatusneeds review => assigned
2018-09-18 12:36WubTheCaptainNote Edited: 0019540bug_revision_view_page.php?bugnote_id=19540#r11807
2018-09-18 12:36WubTheCaptainNote Edited: 0019540bug_revision_view_page.php?bugnote_id=19540#r11808
2018-09-18 16:31Pol MNote Added: 0019543
2018-09-18 16:55Pol MStatusassigned => needs review
2018-09-18 16:55Pol MNote Added: 0019544
2018-09-18 22:13WubTheCaptainNote Added: 0019550
2018-09-18 22:14WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11822
2018-09-18 22:15WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11823
2018-09-18 22:20WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11824
2018-09-18 22:20WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11825
2018-09-18 22:20WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11826
2018-09-18 22:20WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11827
2018-09-18 22:21WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11828
2018-09-18 22:22WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11829
2018-09-18 22:23Pol MNote Added: 0019551
2018-09-18 22:24WubTheCaptainNote Edited: 0019550bug_revision_view_page.php?bugnote_id=19550#r11830
2018-09-19 23:56WubTheCaptainNote Added: 0019577
2018-09-19 23:56WubTheCaptainNote Edited: 0019577bug_revision_view_page.php?bugnote_id=19577#r11859
2018-09-20 05:43Pol MNote Added: 0019578
2018-09-20 16:15ZalewaNote Added: 0019581
2018-09-20 16:41Pol MNote Added: 0019583
2018-09-20 16:41Pol MStatusneeds review => needs testing
2018-09-22 17:04WubTheCaptainNote Added: 0019641
2018-09-22 17:04WubTheCaptainAssigned ToPol M => Zalewa
2018-09-22 17:04WubTheCaptainStatusneeds testing => needs review
2018-09-22 17:06WubTheCaptainNote Added: 0019642
2018-09-22 17:07WubTheCaptainNote Edited: 0019642bug_revision_view_page.php?bugnote_id=19642#r11912
2018-09-22 17:08WubTheCaptainRelationship addedchild of 0003479
2018-09-22 17:16WubTheCaptainNote Added: 0019643
2018-09-22 17:17WubTheCaptainFile Added: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:30WubTheCaptainFile Deleted: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:31WubTheCaptainFile Added: 2018-09-22-171617_524x176_scrot.png
2018-09-22 17:47WubTheCaptainStatusneeds review => assigned
2018-09-22 19:56ZalewaNote Added: 0019653
2018-09-22 20:04WubTheCaptainNote Added: 0019654
2018-09-22 20:05WubTheCaptainNote Edited: 0019654bug_revision_view_page.php?bugnote_id=19654#r11929
2018-09-22 20:59ZalewaNote Added: 0019655
2018-09-22 20:59ZalewaStatusassigned => needs testing
2018-09-22 21:37WubTheCaptainNote Added: 0019658
2018-09-22 21:37WubTheCaptainStatusneeds testing => needs review
2018-09-22 21:38WubTheCaptainNote Edited: 0019658bug_revision_view_page.php?bugnote_id=19658#r11931
2018-09-22 21:38WubTheCaptainNote Edited: 0019658bug_revision_view_page.php?bugnote_id=19658#r11932
2018-09-22 21:43WubTheCaptainNote Added: 0019660
2018-09-22 21:44WubTheCaptainNote Edited: 0019660bug_revision_view_page.php?bugnote_id=19660#r11934
2018-09-22 21:51ZalewaNote Added: 0019662
2018-09-22 21:51ZalewaNote Edited: 0019662bug_revision_view_page.php?bugnote_id=19662#r11938
2018-09-22 21:52ZalewaStatusneeds review => resolved
2018-09-22 21:52ZalewaFixed in Version => 1.2
2018-09-22 21:52ZalewaResolutionopen => fixed
2018-09-22 21:54WubTheCaptainNote Added: 0019664
2018-10-01 04:01WubTheCaptainRelationship deletedrelated to 0003483
2018-10-02 16:58ZalewaNote Added: 0019884
2018-10-02 17:04WubTheCaptainNote Added: 0019885
2018-10-02 17:04WubTheCaptainStatusresolved => needs review
2018-10-02 17:05WubTheCaptainResolutionfixed => reopened
2018-10-02 17:49ZalewaNote Added: 0019886
2018-10-02 19:22WubTheCaptainNote Added: 0019887
2018-10-02 19:22WubTheCaptainStatusneeds review => resolved
2018-10-02 19:22WubTheCaptainResolutionreopened => fixed
2018-10-02 19:26WubTheCaptainNote Added: 0019888
2018-10-02 19:26WubTheCaptainStatusresolved => assigned
2018-10-02 19:26WubTheCaptainResolutionfixed => reopened
2018-10-02 19:32ZalewaNote Added: 0019889
2018-10-02 19:32ZalewaStatusassigned => resolved
2018-10-02 19:32ZalewaResolutionreopened => fixed
2018-10-27 22:54WubTheCaptainStatusresolved => closed

Notes
(0019520)
Pol M   
2018-09-13 20:11   
pull request
(0019524)
Zalewa   
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.
(0019540)
WubTheCaptain   
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.
(0019543)
Pol M   
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.
(0019544)
Pol M   
2018-09-18 16:55   
pull request
(0019550)
WubTheCaptain   
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.
(0019551)
Pol M   
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
(0019577)
WubTheCaptain   
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)
(0019578)
Pol M   
2018-09-20 05:43   
No worries XD
(0019581)
Zalewa   
2018-09-20 16:15   
Let's not add the "Restart? Yes/No" buttons for now, the notification is sufficient. PR has been merged.
(0019583)
Pol M   
2018-09-20 16:41   
I agree with Zalewa tbh :P
(0019641)
WubTheCaptain   
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?
(0019642)
WubTheCaptain   
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.
(0019643)
WubTheCaptain   
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.
(0019653)
Zalewa   
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?
(0019654)
WubTheCaptain   
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).
(0019655)
Zalewa   
2018-09-22 20:59   
The discussed label added here:'https://bitbucket.org/Doomseeker/doomseeker/commits/29d58e568aa9e727ebc5ab440dd7ac9e3bdcb90d [^]'
(0019658)
WubTheCaptain   
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.
(0019660)
WubTheCaptain   
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.
(0019662)
Zalewa   
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.
(0019664)
WubTheCaptain   
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.
(0019884)
Zalewa   
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 [^]'
(0019885)
WubTheCaptain   
2018-10-02 17:04   
Quote from Zalewa
https://bitbucket.org/Doomseeker/doomseeker/commits/9334fe8924f1ade53bf1bcd8c7f72ab0fc60ca28


Missing period, please restore.
(0019886)
Zalewa   
2018-10-02 17:49   
'https://bitbucket.org/Doomseeker/doomseeker/commits/f6ac7b72e887bbdae1e758fefe9b3d68909e1fe5 [^]'
(0019887)
WubTheCaptain   
2018-10-02 19:22   
Quote from Zalewa
See if the resize problem is gone now for you.


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


→ "Full retranslation requires a restart."

Sorry, didn't catch this earlier.
(0019889)
Zalewa   
2018-10-02 19:32   
'https://bitbucket.org/Doomseeker/doomseeker/commits/d709267b230bd9196fcb553b41260cda1342caf3 [^]'