================================================================================
SKYCHART - APPLIED FIXES
================================================================================

What is here
------------
  skychart-fixed/        the 14 .pas files that were changed, complete and
                         ready to drop into the skychart/ source directory
  skychart-fixes.diff    unified diff of every change against the originals
  this file              what was changed, what was not, and what to verify

All other .pas files are untouched and are not included.

Every change carries a comment starting with "Bug fix:" explaining what was
wrong. No original comment was removed or reworded anywhere.

These changes have NOT been compiled or run. No Free Pascal compiler was
available, nor the LCL / BGRABitmap / Synapse / SynEdit dependencies. What was
verified mechanically: each edit matched exactly one place in its file;
begin/case/try and end counts stay balanced in every file; byte-level encoding
and line endings are unchanged (u_290.pas keeps its CRLF endings, and the
non-UTF-8 bytes in u_projection.pas and u_util.pas are preserved as-is).
Treat this as a patch to review and compile, not as tested code.


================================================================================
WHAT WAS CHANGED
================================================================================

Section A of skychart-bugs.txt - all 14 items - is implemented.

 ID   File(s)                        Change
 ---  -----------------------------  -----------------------------------------
 A1   cu_plot.pas                    GetBodyImage: the Great Red Spot staleness
                                     test now compares ipla (the body number)
                                     instead of idx (the cache slot).

 A2   u_290.pas                      UCAC4 designation now takes the six digits
                                     after the padding "1" via copy(naamst,2,6)
                                     instead of naamst[1]..naamst[6].

 A3   u_290.pas                      record_size from the file header is
                                     validated against a new valid_record_sizes
                                     table and against SizeOf(buf2) before it is
                                     used as a divisor or as a read length.
                                     The case statement gained an else arm.

 A4   cu_planet.pas                  TPlanet.Asteroid: the elongation and phase
                                     arccos arguments are clamped and the
                                     denominators checked, matching what
                                     TPlanet.Comet already did.

 A5   u_projection.pas               AngularDistance clamps c3 into [-1,1]
                                     rather than returning pi2. The except
                                     fallback returns pi, not pi2.

 A6   cu_planet.pas                  OrbRect saves comelem.Oe and restores it in
                                     a finally block, so the parabolic fallback
                                     no longer corrupts the stored elements.

 A7   cu_tcpserver.pas               inherited Create runs first; a destructor
      pu_main.pas                    now owns cmd; FSock is FreeAndNil'd so the
                                     "FSock <> nil" guard in SendData means
                                     something; ThrdTerminate clears
                                     TCPThrd[i] as well as ThrdActive[i] and
                                     runs on the main thread via a new
                                     SyncTerminate/Synchronize; the daemon's
                                     accept loop tolerates a nil slot.
                                     In pu_main.KillTCPClient the operand order
                                     was fixed so the TCPDaemon <> nil test
                                     happens before TCPDaemon is dereferenced.
                                     (This also covers item B7.)

 A8   fu_chart.pas                   cmd_GetScopeRates wraps srate in
                                     try/finally.

 A9   pu_scriptengine.pas            A newly created script list is attached to
                                     node.Data immediately, so cancelling the
                                     editor no longer orphans it.

 A10  u_CacheBMP.pas, u_orbits.pas   Both destructors now call inherited
                                     Destroy.

 A11  cu_skychart.pas                LoadHorizon trims each line once and tests
                                     the trimmed text for both the blank and the
                                     '#' comment case.

 A12  cu_skychart.pas                LoadHorizon clears horizonlist[0..361]
                                     instead of [1..360].

 A13  u_util.pas                     striphtml reduces the tag to its name,
                                     lower case and without a closing slash,
                                     before testing for p and br.

 A14  fu_config_chart.pas            Deleted the orphaned declarations
      fu_config_system.pas           Panel1: TPanel and Language: TTabSheet.


================================================================================
WHAT WAS NOT CHANGED, AND WHY
================================================================================

Section B of skychart-bugs.txt is systemic rather than local, so none of it was
applied except B7, which came along with A7. Specifically:

  B1  77 empty except blocks. Mechanical but large, and the handlers are not
      all equivalent: several in cu_ascommount.pas / cu_ascomrestmount.pas are
      deliberately probing optional ASCOM interface members and should stay
      quiet, while the ones around the config writers in pu_main.pas should
      arguably become hard failures. Applying one blanket treatment to all 77
      would be wrong. Happy to do this as a separate pass if you want it.

  B2  Writable typed constants and globals in u_290.pas. Fixing this properly
      means the T290Reader class from improvements item I8, not a patch.

  B3  The out parameter mag2 read before assignment. Changing `out` to `var`
      in a public function signature affects callers; worth doing, but it is a
      deliberate API change rather than a bug fix.

  B4  The two `h := h;` no-ops in u_projection.pas. Harmless, and removing them
      changes nothing at runtime.

  B5  Runtime-check settings and ShowHints. These live in cdc.lpi, not in any
      .pas file.

  B6, B8, B9, B10  All design changes (float comparison policy, threading model,
      cache API, format settings), covered in skychart-improvements.txt.


================================================================================
POINTS TO REVIEW BEFORE TRUSTING THIS PATCH
================================================================================

1. A3 and record size 7
   The .290 header comment lists 7 as a legal record size, but the case
   statement has never had an arm for it. The patch therefore marks 7 as not
   decodable and rejects such a file, where the old code silently emitted
   duplicated phantom stars. If you have a real 7-byte .290 file, this changes
   behaviour from "wrong data" to "no data", which is a deliberate choice on my
   part and you may want the opposite. The valid_record_sizes table is a single
   line to change, with a comment pointing at what else would need adding.

2. A7 and Synchronize at shutdown
   SyncTerminate is called through Synchronize from the end of Execute. This is
   safe here because the shutdown path in pu_main.pas uses ISleep, which calls
   Application.ProcessMessages in a loop and therefore keeps servicing the
   synchronise queue for the 800 ms it runs. If that shutdown sequence is ever
   changed to stop pumping messages, a worker thread could block in Synchronize.
   Worth a comment in whatever replaces it.

3. A7 residual race
   Clearing TCPThrd[i] on the main thread closes the window where the main
   thread dereferences a pointer it has just seen as non-nil. The daemon thread
   still reads the same slot in its accept loop, so a narrow race remains
   between the daemon's nil test and its use of the pointer. Closing that
   properly means the ownership change described in the improvements file
   (daemon owns the threads, no FreeOnTerminate), which is a redesign rather
   than a fix.

4. A6 and the nested solvers
   eliptique / parabolique / hyperbolique read comelem.Oe directly, so the
   substituted value still has to be in the field while they run. The patch
   therefore saves and restores rather than introducing a local, which is the
   smaller change but does mean comelem.Oe is briefly not the real eccentricity.
   If another thread ever reads comelem during an ephemeris calculation it would
   see the substituted value - not possible today, since TPlanet is used from
   one thread.

5. A9 and empty script lists
   Attaching the list to node.Data at creation means a cancelled edit leaves an
   empty TStringList on the node where there was previously nil. Every consumer
   in pu_scriptengine.pas guards with "is TStringList" and then Assigns, so an
   empty list behaves the same as no list, and the cleanup at the end of the
   unit frees it. Worth a glance to confirm you agree.

6. A5 changes numeric output
   Clamping instead of returning pi2 means AngularDistance now returns a small
   value where it used to return 2*pi for near-coincident positions. Any code
   that was inadvertently relying on the old sentinel to mean "no match" will
   behave differently. Call sites checked: cu_catalog.pas:4913, pu_info.pas:223,
   cu_skychart.pas:2212/2618/8896, fu_chart.pas:3507/7358, u_satellite.pas:213/373,
   cu_planet.pas:2615, cu_catalog.pas:6280 - all treat the result as a distance
   and compare it against a radius, so all of them get better answers. Still the
   change most worth eyeballing in a running build.
