[qt] update time toolbar pause button to pause/resume

[dave-kaye]

[dave-kaye]

Within the QT interface is a toolbar for the control of the timing of the simulation. It contains a pause button that when pressed pauses the simulation and when pressed again restarts the simulation. The icon for the button is "||". In many programs, especially the multimedia players, the pause button is "||" and the play button is ">". The Celestia pause button keeps the same icon and tool tip "pause time" even if it is functioning as a restart button. The user does not have any visual cues to indicate the purpose of the button has changed. Moreover, alongside the pause button is a button with the icon ">" and tool tip "real time" which sets the simulation speed to normal time. This can be confusing to users, who anticipate thia button will function as a play button.

This code submission converts the pause button to a 2-way button with icon "||" and tooltip "pause time" for pausing the simulation; and icon ">" and tooltip "resume time" for restarting the simulation. The single button cycles through the two functions, similar to a play/pause button in multimedia. The submission also creates a new icon for the button that sets simulation speed to real time.

The original pause button is created by a Qt action. Attempts to directly add the pause/resume function to the slot code caused invalid scope compile errors. Attempts to pass the button identification through the slot parameters caused the button to fail to work or crash the program. Attempts to use sender or widget calling functions to identify the button in the slot code also caused the button to fail to work or crash the program. It appears the issue is in the use of the action method of creating the button. The action method for button creation was replaced with creating the button, labeling it, adding a click event to signal/slot and adding it to the toolbar. The name of the button was added to the class information which overcame variable scope issues in the slot code. While not elegant, it is functional. However, not using the action method to create the button means the button does not conform to the toolbuttonstyle setting. If an icon is set the button style is icon, if only a text is set the button style is text. A boolean variable was added to the class information to indicate if the button style was text.

If this code submission is accepted, the addition of "resume time" to the user interface must be integrated with language translations. Also, there are 2 issues that still can be addressed in this submission.

The first issue is a compiler directive #if/#else which is permanently set to not compile the text versions of the toolbar buttons. The text version was not maintained, and was out-of-sync with the icon button version. The code submission updates the text buttons so both styles are the same. But if the text buttons are never going to be used, they could be removed from the code and the added coding to track the button style type could also be removed. This would simplify the code. Let me know via comments to this submission if you want this to be done.

The second issue is in the fast/slow time button icons. For slowing time they are "<<|" and "<|", a comparison symbol followed by a vertical line. For faster time they are ">>>" and ">>", comparison symbols and no vertical line. It would be simple to create uniform icons either with or without the vertical line. If you wish this to be done, indicate the desired style via comments to this submission.

[dave-kaye]

From prior pull request @375gnu requested following modifications:

Change buttons for slowing time speed from style "<|" to style "<<". - DONE in both icon and text button modes.

Update IF statement to conform to Celestia coding style - DONE (hopefully)

Prior pull request deleted to allow single merge commit.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

[375gnu]

@dave-kaye pleas fix the commit message.

@levinli303 what do you think about having toolbar buttons without icons? I'm surprised that such mode is supported at all. I would like to remove it.

[dave-kaye]

@levinli303 what do you think about having toolbar buttons without icons? I'm surprised that such mode is supported at all. I would like to remove it.

FYI: buttons in text mode use the alphabet letters found in the text variable. They are not blank if text is entered. They just do not use pictures that the icon mode uses. You decide if keeping them is worth maintaining the coding that is setup not to compile them

[levinli303]

do we have screenshots of before/after so the visual change can be more clear. also im worried that as the paused/resumed state can be triggered by script or other sources, which might make the content of the button inaccurate.

[dave-kaye]

Attached are 6 screenshots of the original/new code time toolbar. Note the tool tips displayed. The red "paused" word in the upper right corner shows the status of the simulation. The buttons for speed and pause are grouped together similar to multimedia players. The new icon for real time is shown, replacing the icon ">" which almost universally indicates on media players the play function. New icons for slow time, slow-slow time are displayed, going from "<|" style to "<<" style to match the faster icons (as previously discussed).

One command in the cel and celx scripts that could create a problem with the toolbar pause/resume button is the WAIT command. The wait command appears to run a separate timer loop, it does not affect the paused state of Celestia. Putting Celestia into/outof paused state while a script is running has no affect on running the script - other than celestial movement is paused during the script; but the wait timer continues to run and the script continues to perform commands as expected. This was tested using a modified GitHub supplied script tour-system.celx.

The documentation states the WAIT command operates differently in cel versus celx scripts. The command returns control during the waiting period to Celestia main loop in celx scripts, but not in cel scripts. I used the same test techniques on a modified demo.cel script from Celestia 1.5.0. This script has several time changes and wait periods. The pause/resume button operated on the time flow as expected, it did not affect the script nor did the script affect it. On one part the script sped up the time flow but the pause/resume button paused and restarted the flow as expected, independent of the script.

A second instance of possible interference is the command sequence GETTIME and SETTIME. These commands will get the Julian date/time and set the Julian date/time. The commands to get and set the time appear to be unaffected by the pause state. The only affect is celestial movement is paused during the time the pause time button is active, but the script jumping to the correct time/date state is not affected. Tested using a modified GitHub supplied script annum.celx.

There is a boolean variable ISPAUSED in the celx documentation. It returns the present state of the pause function. There is an undocumented method PAUSE() in the celx code. It can set the pause state based upon a boolean variable. Utilizing this method, the pause/resume button functions normally while the script is running. However, if the script executes a pause() method, the pause/resume button face is not updated.

I have also found that there is a keypress that toggles the pause/resume function, the key. If the changes the pause state, the pause/resume button face is not updated.

If a code sequence causes the pause/resume button to go out-of-sync with the simulation, it heals itself after the first click of the button. This is because the button click code will check the simulation pause state, then invert the state and set the buttons to the inverted state. Therefore, after the first click the buttons and simulation are back in-sync.

Celx scripts and keypress events do affect other parameters on the interface. These methods use a watcher function sequence to update the interface when the parameters change. It may be possible to add time functions to the existing watchers, or generate a new series for time functions that will update the pause/resume button automatically in the Qt interface.

In summary, the pause/resume button does not affect the operation of scripts, they operate the same as original code. Some scripts and keypress events can cause the button face to go out-of-sync with the simulation pause state. There are examples within the code where similar interface sync issues are handled successfully.

If the submission is held until this code is written, then: 1) whether the pause/resume concept is acceptable or will be denied, 2) whether the icon time toolbar as drawn and pictured is acceptable, and 3) whether the text button only code can be removed should be stated. This will direct any future efforts.

It is also possible to do this as a two part submission. The first part would put the new icons on the buttons but keep the original pause button and the original pause button code. At this time the text button code could also be removed.

One effect that I saw was if Celestia was in a paused state and a script was started, the script failed to operate. This was seen on both the original and new code versions. It is part of Celestia itself.

Original code - time running Original code - time paused New code - time running New code - time paused Original code - real time button New code - real time button

[levinli303]

keeping text only buttons for these controls are bad in my perspective, font could mess it up. I think it should be removed.