Other in Educational
Move the coordinate conversion functions out of SkyPoint.
This commit is rather more monstrous than anticipated, since in
order to do this, we have to touch every part of the KStars code that
uses the conversion functions of SkyPoint. This illustrates just
how screwed up this class is.
The main problem that the SkyPoint has is that it's really three
points bundled together into one, with no synchronization between the
coordinates. The three coordinates (catalog, equatorial, horizontal)
are supposed to represent the same point in some cases, but not in others.
We can't just add synchronization, for example, because it would break
all of the things that rely on being able to update things independently.
But not having synchronization causes all kinds of position bugs, and
makes the code incredibly difficult to reason about as well.
I suspect that there are, amongst the various parts of the codebase,
incompatible expectations about how the class should behave (i.e., there
is no correct behaviour).
The other problem with the SkyPoint, of course, is that it confuses the
relationship between "has a coordinate" and "is a coordinate". This may
be related to the problems above.
When we reimplement the functions now in the KSEngine::Old* namespaces,
we will not repeat these mistakes. I intend to keep the coordinates to
just a POD quaternion, with one type for each system, and have all of the
functions be *functions*, not methods, whose output depends only on their
input and are side-effect free. This also means that we can actually
unit test them, since they will both a) have "correct" behaviour
and b) not require the entire rest of KStars to be running to operate.