Skip to content

Allow elongation dependent cultural names #4371

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

Merged
merged 3 commits into from
Jun 15, 2025
Merged

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Jun 15, 2025

Name the Morning Star and Evening Star only if visible as such.

Description

This little branch provides an extension of the CulturalName, introducing the first "special" constants. More might come as needed.

The index.json NAME entry for planets can now be extended with an "elongation" tag which can be eastern or western.
In gthe code, this is translated to the Special constants Morning or Evening. I am not sure this is meaningful, maybe they should bear equal names. But also not sure which of the two is the better. opinions?

Fixes #4326

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

@gzotti gzotti added this to the 25.2 milestone Jun 15, 2025
@gzotti gzotti self-assigned this Jun 15, 2025
@gzotti gzotti added feature Entirely new feature importance: medium A bit annoying, minor miscalculation, but no crash subsystem: skycultures The issue is related to skycultures of planetarium... purpose: cultural astronomy Issues, pull requests and proposals with cultural astronomy purposes labels Jun 15, 2025
@github-actions github-actions bot requested review from alex-w, 10110111 and sushoff June 15, 2025 00:10
@gzotti gzotti moved this from Backlog to In progress in Skycultures 2.0 Jun 15, 2025
Copy link

github-actions bot commented Jun 15, 2025

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?

Files matching skycultures/**:

  • Did you remember to update skycultures/CMakeLists.txt file respectively to changes in sky cultures?
  • Did you remember to define classification parameter in sky cultures (see index.json file)?
  • Did you remember to define license parameter in sky cultures (see description.md file)?
  • Did you remember to define region parameter in sky culture (see index.json file)?

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

Copy link

Hello @gzotti!

Thank you for proposing of the feature.

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.

Good idea, but I don't like using identical code in 2 places without re-use (computation the elongation along ecliptic) - IMHO this code should be moved in separate method and use in getInfoStringEloPhase() and getCultureLabels() methods.

The second problem - what will see observer on the Moon or on the Mars?

@gzotti
Copy link
Member Author

gzotti commented Jun 15, 2025

+1 on code reuse. (There are many other places to do that...)
Name will be limited to Earth use (as are all human cultures)
What about the key words? In the json, "elongation":"western"|"eastern" reads familiar. Or "visible":"morning"|"evening"? Or even "special", to match the internal name?

@alex-w
Copy link
Member

alex-w commented Jun 15, 2025

"visible":"morning"|"evening" is best IMHO

@gzotti gzotti marked this pull request as ready for review June 15, 2025 09:24
@gzotti gzotti moved this from In progress to In review in Skycultures 2.0 Jun 15, 2025
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.

Thanks! It’s OK for me, but please check list of names for inner planets in SC

@gzotti
Copy link
Member Author

gzotti commented Jun 15, 2025

I am just about to change/add a few entries, but some are incomplete/inconsistent.

@gzotti
Copy link
Member Author

gzotti commented Jun 15, 2025

And grmbl, Notepad++ fails to delete parts of Arab (RTL) strings properly.

- name Morning Star and Evening Star
@gzotti gzotti force-pushed the sc/MorningEveningStar branch from b1008ff to 8128443 Compare June 15, 2025 11:21
@gzotti gzotti force-pushed the sc/MorningEveningStar branch from 8128443 to 1347031 Compare June 15, 2025 11:56
@gzotti gzotti merged commit 95444a0 into master Jun 15, 2025
27 of 28 checks passed
@gzotti gzotti deleted the sc/MorningEveningStar branch June 15, 2025 11:57
@github-project-automation github-project-automation bot moved this from In review to Done in Skycultures 2.0 Jun 15, 2025
@sushoff
Copy link
Contributor

sushoff commented Jun 16, 2025

I know, you merged it already, I like the new feature

Question:

  • does this name only appear to Venus?
  • elongation also may apply to Mercury but indeed Jupiter is sometimes mistaken as "evening star" ... did you have this in mind, too?

I'm just asking - either way, it should be mentioned/ explained in the SUG (to be reworked later as you state above).

@gzotti
Copy link
Member Author

gzotti commented Jun 16, 2025

The actual names are written in index.json. Venus is clear. For Mercury, I am not aware about a dedicated name currently usable for the modern SC, but I can imagine other SCs could have it, so the technology is here. Elongation-dependent (actually, only separating east/west) names can actually be added to any planet, but won't make much sense. Not sure about other criteria, e.g. is a bright Mars in opposition, rivaling Jupiter, named different from a dimmer Mars anywhere? Such things could still be added later.

People pointing at Jupiter and naming it Evening Star should be instructed to not doing so. "The Evening Star" is Venus, and nothing else. A balloon is no airplane either, despite being "up there".

The SUG instructs SC authors what to specify in the index.json. Maybe, in total, some more words can be provided about SC2.0 in the "Astronomical Concepts" chapter. But as you frequently say, who reads the dedicated Guide?

@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Jun 16, 2025
Copy link

Hello @gzotti!

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

@sushoff
Copy link
Contributor

sushoff commented Jun 17, 2025

The actual names are written in index.json. Venus is clear. For Mercury, I am not aware about a dedicated name currently usable for the modern SC, but I can imagine other SCs could have it, so the technology is here. Elongation-dependent (actually, only separating east/west) names can actually be added to any planet, but won't make much sense. Not sure about other criteria, e.g. is a bright Mars in opposition, rivaling Jupiter, named different from a dimmer Mars anywhere? Such things could still be added later.

People pointing at Jupiter and naming it Evening Star should be instructed to not doing so. "The Evening Star" is Venus, and nothing else. A balloon is no airplane either, despite being "up there".

LOL aaha, good to know ... of course, I did that several times in astronomy lectures.

Here, I referred to historical observations. If an ancient Babylonian wrote sth like this on a clay tablet, modern Assyriologists would think "ah, the term xxx can refer to both Venus and/or Jupiter".

The SUG instructs SC authors what to specify in the index.json. Maybe, in total, some more words can be provided about SC2.0 in the "Astronomical Concepts" chapter. But as you frequently say, who reads the dedicated Guide?

@gzotti
Copy link
Member Author

gzotti commented Jun 17, 2025

Here, I referred to historical observations. If an ancient Babylonian wrote sth like this on a clay tablet, modern Assyriologists would think "ah, the term xxx can refer to both Venus and/or Jupiter".

You could just add some byname for "Jupiter in the Evening Sky" with "visible": "evening" to the "NAME Jupiter" entry in your Babylonian SCs. It will be displayed when eastern elongation is less than 180°. If we need other limits, these must be defined and implemented.

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Jun 22, 2025
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
feature Entirely new feature importance: medium A bit annoying, minor miscalculation, but no crash purpose: cultural astronomy Issues, pull requests and proposals with cultural astronomy purposes subsystem: skycultures The issue is related to skycultures of planetarium...
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Separate handling for Mercury and Venus as Evening/Morning objects
3 participants