archaeolines plugin: increase location accuracy

[axd1967]

Description

The custom location lat/long input boxes accepted 2 digits after the comma, which lead to considerable azimuth deviations for custom locations less than a few km distant.
Precision has been increased to 6 digits (as used in current location precision).

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

How Has This Been Tested?

Test Configuration:

• Operating system: KUbuntu 20.04
• Graphics Card: <Manufacturer (likely Intel, NVidia, AMD?), Model (HD, Geforce, Radeon..., with model number), driver version?>

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
• 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

Note

There is a strange behaviour when entering values by keyboard (this used to happen in Ocular plugin if I'm not mistaken): the GUI prevents entering several digits at a time, so one has to enter digits one by one. Maybe due to too early event handling?

edit added: this is an issue that existed before this PR.

Workaround: after entering a digit, select the remaining digits (SHIFT+END) and type the next digit, until all digits have been entered.

[alex-w]

Please fix CodeFactor warning

[gzotti]

It may then be worth using the same coordinate input boxes that are used in the Location panel. They also automatically react to decimal degrees setting.

[alex-w]

It may then be worth using the same coordinate input boxes that are used in the Location panel. They also automatically react to decimal degrees setting.

Good idea! Plus following “Use decimal degrees” option from main GUI.

[axd1967]

It may then be worth using the same coordinate input boxes that are used in the Location panel. They also automatically react to decimal degrees setting.

Good idea! Plus following “Use decimal degrees” option from main GUI.

I've looked into the code of both classes and this is far from trivial. To start, both modules were written very differently (especially event handling). Also, additional work would be nice, eg introduce additional UI classes derived from AngleSpinBox (such as LongitudeSpinBox) to avoid too much duplication of code (such as setting properties such as limits, format, and prefixtype and handling events such as decimal degree mode).

I'd see this in a separate change request that should no longer block the current change.

[alex-w]

Why you added weird StelLocation method? Why you added const int variables with strange names and why you don’t use it completely? Why AngleSpinBox is too hard to you and why you want to get a separate classes?

[axd1967]

Why you added weird StelLocation method? Why you added const int variables with strange names and why you don’t use it completely? Why AngleSpinBox is too hard to you and why you want to get a separate classes?

I won't take such aggressive remarks, sorry. This is NOT the way to treat contributions. You should be ashamed of your attitude!

[alex-w]

Really?! Where do you see "aggressive remarks"? 

Maybe here:

Why did you add the weird StelLocation method?

But I really do not understand why you add a new method in StelLocation class which is unusable in any part of Stellarium except for one place - ArcheoLines plugin. Why you not use simple struct here?

Maybe here: 

Why you added const int variables with strange names and why you don’t use it completely?

You are added the LOCATION_DEGREE_PRECISION_DIGITS and LOCATION_RAD_PRECISION_DIGITS variables, but... where you see radians for latitude and longitude? Right now by the fact both variables are added just because you may add them. Why didn't you use LOCATION_RAD_PRECISION_DIGITS within the AngleSpinBox class (Hint: see constructor)?

Or maybe here:

Why AngleSpinBox is too hard for you and why you want to get separate classes?

I don't understand why AngleSpinBox is not usable for you and you need 2 separate classes for coordinates.

Why I should be ashamed if you writing something strange and unobvious?

[github-actions]

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

Files matching src/**/*.cpp:

• Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files

[axd1967]

As a result of not accepting this PR, the current implementation (0.20.4) shows a horrible angular precision issue, just because the input boxes do not accept a higher precision.

here is the proposed change: master...axd1967:contrib/plugin/archeo/20201229-precision

One will notice that the proposed changes are very simple and basic (and removes DRY and meant to be more readable, with the price of an added dependency...). And I explain why I refuse to perform further refactoring. In exchange, I am being scolded. Profusingly.

[gzotti]

What is KBC, what kind of line have you set, and what is DRY? You can set a line to 1/100 degrees accuracy, exceeding application of naked-eye resolution this plugin's use was originally intended for.

In a process of further developing this plugin it may make sense to replace Qt's default DoubleSpinBox by whatever multi-angle-detail GUI input element has been developed for use in the Location panel. A cheap solution would be to increase field width to 7 post-comma places. What for?

[alex-w]

@axd1967 you are added new method in StelLocation class where is not used in this class, you are defined 2 variables in AngleSpinBox class and both variables are not used in this class - I asked why you doing it and what I got in answer?

[alex-w]

n a process of further developing this plugin it may make sense to replace Qt's default DoubleSpinBox by whatever multi-angle-detail GUI input element has been developed for use in the Location panel.

In the Location panel and in the AstroCalc tool we use standard Qt's spinboxes, but with DMS style - no need some special for it.

To follow DD/DMS styles we need add additional 5 lines in the code - no need any strange work as @axd1967 proposed.

[gzotti]

In the Location panel and in the AstroCalc tool we use standard Qt's spinboxes, but with DMS style - no need some special for it.

To follow DD/DMS styles we need add additional 5 lines in the code - no need any strange work as @axd1967 proposed.

OK, even better.

[axd1967]

KBC doesn't matter. it is a custom geographic location that I defined in the archeo plugin, and the corresponding azimuth towards that same location was separately computed in the landscape.

because of the limited input precision of the archeo plugin, it is currently not capable to compute accurate azimuths for nearby locations: the result in this case is a 7-degree difference.

For DRY, see https://en.wikipedia.org/wiki/Don%27t_repeat_yourself . that's the "strange work" and the seemingly "unneeded" constructor (and it's not because a method is not used in a class, that it makes no sense. a class can provide services for its clients without using those services). I believe in readability over efficiency, especially in an open source product: better readable code will attract more developers, who will then be even more productive.

[alex-w]

According to DRY principle you should obtain coordinates for 2 locations from existing database of locations through existing methods in the class - not adding messing through adding "empty" methods with doubling data and unobvious dependencies. Please read about KISS principle. Same problem for introduced by you 2 variables in one class to using exclusively in the other class (really it need one variable!).

[gzotti]

currently not capable to compute accurate azimuths for nearby locations: the result in this case is a 7-degree difference.

Finally you expressed the problem case clear enough to understand. Nearby locations.

[axd1967]

Good point, @gzotti , but no cigar, as I already mention it in the PR :-)
("considerable azimuth deviations for custom locations less than a few km distant")

[gzotti]

I don't smoke... ;-)

[gzotti]

#1573 should provide what you need.

[github-actions]

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