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

Immediate-store all of view dialog #3987

Merged
merged 16 commits into from
Dec 8, 2024
Merged

Immediate-store all of view dialog #3987

merged 16 commits into from
Dec 8, 2024

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Nov 25, 2024

Description

This continues a series of changes which aims at optionally avoiding the requirement to press "save settings".
All features in the View settings dialog and Configuration should be immediate-store now.

The Language and font settings are probably the only settings that crucially needs to be protected by an explicit store so they are not messed up on a panic exit. All other settings are as easy to reset as they were set.

In the end of this branch, the "save settings" button will be disabled when the corresponding "immediate storage" option in the config dialog has been activated.

It is to be discussed whether "save languages" and "save fonts" buttons should only be available in this new mode. Else they would cause no harm.

Plugins usually have their own "save settings" logic. They may be modified later.

Fixes # (issue)

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Win11
  • Graphics Card: irrelevant

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

- also fix smooth fading of rectangular FOV box
- Final NomenclatureMgr fix
- Move planet visual magnitude selection logic to SolarSystem property
  (This also fixes a missing RemoteSync property not syncing)
- Update URL for JUPOS
- also move selection of single constellations to starlore
  and add isolated setting as well.
  Properties/config keys should be named more fittingly.
@gzotti gzotti self-assigned this Nov 25, 2024
@github-actions github-actions bot requested review from 10110111 and alex-w November 25, 2024 00:28
@gzotti gzotti added the enhancement Improve existing functionality label Nov 25, 2024
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

Files matching guide/**:

  • Did you remember to update screenshots to match new updates?
  • Did you remember to grammar check in changed part of documentation?

This is an automatically generated QA checklist based on modified files.

Copy link

Hello @gzotti!

Thank you for the suggested improvement.

src/core/StelSkyDrawer.cpp Outdated Show resolved Hide resolved
- Config dialog: Info, Extras, Tools
- StelMainView
- View dialog
- Config dialog
- Immediate-store option checkbox to switch modes (Needs one last "save settings action)
- also delete defunct button
@gzotti gzotti marked this pull request as ready for review December 5, 2024 21:53
@gzotti gzotti added this to the 24.4 milestone Dec 5, 2024
@github-actions github-actions bot requested a review from alex-w December 5, 2024 21:53
Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Looks good, but 2 obvious fixes in GUI are needed:
2024-12-06_10-41-12
2024-12-06_10-44-00

@gzotti
Copy link
Member Author

gzotti commented Dec 6, 2024

OK for the SC panel. For the other, it has become a bit convoluted over the last years, historically grown. I understand the top-left options (IIRC the same since ~2010) were options most likely used in an actual planetarium setup (hence the group title): Distortion, disc viewport, gravity labels. For explanations in a planetarium (and actually also regular screens), the "select single constellations" was there as well, but it better fits to the SC panel. The Auto-zoom-out (yes, still open issue...) is also an option apparently (I never used it, so missed its relevance, sorry) invented for and most critically used in a planetarium. The rest of the tab looks like a collection of "I didn't know where to place it, so I added it here".

Decimal degrees and Az. from south could both move to the Extras/Info details tab. (Just have two columns in the top area.)
Nutation/aberration could likewise move there, or move to the view settings/Sky, and Topocentric correction could move to the view settings/SSO (Or also Extras/Info tab). Shall I try how this looks?

Another thing is the font selection which is still hidden by default and needs a manual edit to config.ini. Shall we show it to all at beginners level?

@alex-w
Copy link
Member

alex-w commented Dec 6, 2024

Decimal degrees and Az. from south could both move to the Extras/Info details tab. (Just have two columns in the top area.)

Both options touching not only information section, but yes, them can be moved to the Extras/Info tab.

Nutation/aberration could likewise move there, or move to the view settings/Sky, and Topocentric correction could move to the view settings/SSO (Or also Extras/Info tab).

Well, View/Sky and View/SSO are bloated already, but you’re right - these options can be moved there.

Shall I try how this looks?

Yep.

Another thing is the font selection which is still hidden by default and needs a manual edit to config.ini. Shall we show it to all at beginners level?

I fear it may be very painful for beginners…

@gzotti
Copy link
Member Author

gzotti commented Dec 7, 2024

Decimal degrees and Az. from south could both move to the Extras/Info details tab. (Just have two columns in the top area.)

Both options touching not only information section, but yes, them can be moved to the Extras/Info tab.

Nutation/aberration could likewise move there, or move to the view settings/Sky, and Topocentric correction could move to the view settings/SSO (Or also Extras/Info tab).

Well, View/Sky and View/SSO are bloated already, but you’re right - these options can be moved there.

Indeed Sky and SSO were already large. I left them in the Config dialog, but moved to Info as well. These 3 options involve position refinements, but also of course the provided information. In light of current work on the star catalog, topocentric flag could also switch parallax observance for stars, or this will need another flag later.

I edited the .ui XML directly to also clean up the meanwhile 18 (or so) rows in the Info dialog layout. Maybe the currently only used colums 1 and 3 should also be edited back to 0 and 1?

The mag/squareArcMin and "use symbol" settings are AFAICS used only for DSO. Might be moved there, but the current shortened strings are also OK. (Can they be combined into 1 line?)

Another thing is the font selection which is still hidden by default and needs a manual edit to config.ini. Shall we show it to all at beginners level?

I fear it may be very painful for beginners…

OK, I left the default-hidden font panel as it was.

@alex-w
Copy link
Member

alex-w commented Dec 7, 2024

This is OK:
stellarium-010

Well, View/Sky and View/SSO are bloated already, but you’re right - these options can be moved there.

Indeed Sky and SSO were already large. I left them in the Config dialog, but moved to Info as well. These 3 options involve position refinements, but also of course the provided information. In light of current work on the star catalog, topocentric flag could also switch parallax observance for stars, or this will need another flag later.

This is not OK:
stellarium-011

Topocentric coordinates, nutation and aberration touches not only information box - they touches rendering the sky, so, Extras/Info is bad place for all of them. It may be accepatble for moving to View/SSO tab, but definitely to to Extras/Info.

Topocentric coordinates, nutation and aberration should be moved back to Tools tab, IMHO.

The mag/squareArcMin and "use symbol" settings are AFAICS used only for DSO. Might be moved there, but the current shortened strings are also OK. (Can they be combined into 1 line?)

The current view for these lines are OK for me.

@gzotti
Copy link
Member Author

gzotti commented Dec 8, 2024

OK, next try.

@gzotti gzotti merged commit 59998a7 into master Dec 8, 2024
25 of 26 checks passed
@gzotti gzotti deleted the immediate-store branch December 8, 2024 21:45
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Dec 9, 2024
Copy link

github-actions bot commented Dec 9, 2024

Hello @gzotti!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Dec 22, 2024
Copy link

Hello @gzotti!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality
Development

Successfully merging this pull request may close these issues.

2 participants