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

Restore lost lunar "previous" and "next" full moon information #2025

Merged
merged 2 commits into from
Nov 11, 2021

Conversation

A-j-K
Copy link
Contributor

@A-j-K A-j-K commented Nov 11, 2021

Restores the previously displayed "previous" and "next" full moon information (when the Moon is selected) which was removed when the show_today = false (#1852) was implemented.

Description

The "calculateSolarSystemEvents()" function is called when the Moon is selected to display the "previous" and "next" Full Moon information.

The current Stellarium Guide states that this Lunar information is displayed so #1852 introduced a minor regression.

Fixes #1929

Screenshots (if appropriate):

See issue, screenshot there.

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

How Has This Been Tested?

  1. Original issue poster agrees screenshot is as expected.
  2. Now matches the Stellarium Guide.

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
  • n/a I have made corresponding changes to the documentation
  • 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

@alex-w alex-w added this to the 0.21.3 milestone Nov 11, 2021
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.

Something strange happening when settings in the plugin are changed. Plus now other 3 lines of data are lost for the Moon.

@A-j-K
Copy link
Contributor Author

A-j-K commented Nov 11, 2021

Something strange happening when settings in the plugin are changed.

I wil investigate this.

Plus now other 3 lines of data are lost for the Moon.

In order to figure out what the 3 missing lines were I hacked it so that the previous show_today = true would work soI could see what those three lines were actually meant to be showing, here is what I found:-

  1. lineObservableRange :: Was a blank line with no information
  2. lineAcroCos :: Continued to display the data of the previously selected object and nothing to do with the Moon
  3. lineHeli :: Continued to display the data of the previously selected object and nothing to do with the Moon

This is why I set them to ".clear()" as those lines apparently didn't do anything useful when the Moon was selected.

I tried to read the Stellarium Guide to figure out what was meant to be displayed for the Moon and only one line (previous full and next full) was mentioned.

The guide states:

Full Moon When the Moon is selected, the program can compute the exact closest dates of the Moon’s opposition to the Sun.

I guess I'm missing something here. I maybe have to rollback my build to a previous release to see what should actually be displayed.

@A-j-K
Copy link
Contributor Author

A-j-K commented Nov 11, 2021

@alex-w I have installed 0.21.1 as well as my build env. Here is what 0.21.1 does when show_today = true It can be seen that the Rise/Set/Culminate is displayed which has now moved to the extrainfo panel. That just leaves one line of display with the other 3 blank. So, I think my patch is correct to meet the OPs original issue. Comments?

issue-2025

@alex-w
Copy link
Member

alex-w commented Nov 11, 2021

@alex-w I have installed 0.21.1 as well as my build env. Here is what 0.21.1 does when show_today = true It can be seen that the Rise/Set/Culminate is displayed which has now moved to the extrainfo panel. That just leaves one line of display with the other 3 blank. So, I think my patch is correct to meet the OPs original issue. Comments?

issue-2025

OK, you are right for "THIS YEAR" block!

@alex-w
Copy link
Member

alex-w commented Nov 11, 2021

To reproduce the strange behaviour please select any star and try change the settings in the plugin.

@gzotti
Copy link
Member

gzotti commented Nov 11, 2021

Why don't you simply revert that micro commit where I disabled the redundant info?
873b392
I would like to rewrite most of what this plugin does in the long run. (Before about 2023...)

@A-j-K
Copy link
Contributor Author

A-j-K commented Nov 11, 2021

To reproduce the strange behaviour please select any star and try change the settings in the plugin.

I already committed a fix for the settings issue.
0d6a21d

@A-j-K
Copy link
Contributor Author

A-j-K commented Nov 11, 2021

Why don't you simply revert that micro commit where I disabled the redundant info? 873b392 I would like to rewrite most of what this plugin does in the long run. (Before about 2023...)

@gzotti When the micro commit is reverse and show_today = true in ini file this is the result:-

2025-pr

@A-j-K
Copy link
Contributor Author

A-j-K commented Nov 11, 2021

@gzotti

I would like to rewrite most of what this plugin does in the long run. (Before about 2023...)

Understood. This is why my fix is tiny, despite seeing there can be improvements made in the plugin I resisted the temptation to "have a go" and just restrained myself to just fixing the OPs original issue.

@alex-w alex-w merged commit 75bfce2 into Stellarium:master Nov 11, 2021
@A-j-K A-j-K deleted the observability-issue-1929 branch November 11, 2021 19:40
@github-actions
Copy link

Hello @A-j-K! Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@github-actions
Copy link

Hello @A-j-K! 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
None yet
Development

Successfully merging this pull request may close these issues.

The OBSERVABILITY function doesn't work with the Moon
3 participants