Code review comment for lp:~georg-zotti/stellarium/gz_fix-ecliptic-obliquity

Revision history for this message
gzotti (georg-zotti) wrote :

> OK, I think ecliptic obliquity and XYZ should display for Earth only and she
> should not mixed with other data for the chosen planets. What think about
> adding "For Earth:" line after info (after the one empty line) and put
> ecliptic obliquity and XYZ data after it.

Given that nobody outside our development team thinks that "Ecliptic" does not refer to the Earth only, I see no need to annotate "Ecliptic" with "Earth".

By the way, the "ecliptical coordinates" in the infostring are totally nonsense for observer locations outside earth. You would have to first find orbital plane for the planet, and then the "spring point" (ascending node between orbital plane and planet equator). The situation found here were in principle terrestrial ecliptical coordinates with different obliquity, that's nonsense. I now have changed this, for non-terrestrial observers, to

*) show "ecliptical coordinates J2000" referring to earth's orbital plane of J2000.
   The displayed coordinates fit to the displayed ecliptical grid of J2000.
*) Disable display of ecliptical coordinates of date because they were wrong.
*) Disable display of "Ecliptic obliquity" for non-earth observers to avoid confusion.

If we want to display "axis inclination to planet's orbit" or (90-Planet.getRotObliquity(JDE)), this may be a nice extra information.

Also XYZ is mostly here for testing, but it should show selected object's position in "parentocentric" XYZ coordinates (i.e., for moons we have the respective planets as zero). I have commented it away for your comfort. Please do not delete the lines, though, I think it can be helpful when integrating DE431 which have to be rotated into VSOP87 reference frame.

> The question for info with the planetocentric distance is more difficult here,
> because this infobox should not mixed with data about chosen planet too. But
> this data available for all planets in general. I think I can propose
> compromise for visualization of this data - add the planetocentric distance
> info as tooltip for height data on bottom toolbar.

Also here I just wanted a quick view while I move latitude with the location panel open, and not just qDebug() lines in the logfile. The current location was admittedly not good, but this is an intermediate state, not polished. If you can make a tooltip from that, it's fine.

r7621 has the latest changes. I now move to Linux for testing the unit test...

« Back to merge proposal