Merge branch 'stable' into releases

.github/workflows/windows-build.yml

@@ -3,7 +3,7 @@ name: Build EDMC for Windows
 on:
   push:
     tags:
-      - "v*"
+      - "Release/*"
   workflow_dispatch:
 
 jobs:
@@ -42,3 +42,29 @@ jobs:
         with:
           name: Built files
           path: EDMarketConnector_win*.msi
+
+  release:
+    name: Release new version
+    runs-on: ubuntu-latest
+    needs: test
+    if: "${{ github.event_name != 'workflow_dispatch' }}"
+
+    steps:
+      - name: Download binary
+        uses: actions/download-artifact@v2
+        with:
+          name: Built files
+          path: ./
+          
+      - name: Hash files
+        run: sha256sum EDMarketConnector_win*.msi > ./hashes.sum
+
+      - name: Create Draft Release
+        uses: "softprops/action-gh-release@v1"
+        with:
+          token: "${{secrets.GITHUB_TOKEN}}"
+          draft: true
+          prerelease: true
+          files: |
+            ./EDMarketConnector_win*.msi
+            ./hashes.sum

ChangeLog.md

@@ -17,6 +17,53 @@ This is the master changelog for Elite Dangerous Market Connector.  Entries are
   in the source (it's not distributed with the Windows installer) for the
   currently used version in a given branch.
 
+Release 5.1.3
+===
+
+* Attempt to flush any pending EDSM API data when a Journal `Shutdown` or 
+  `Fileheader` event is seen.  After this, the data is dropped.  This ensures
+  that, if the user next logs in to a different commander, the data isn't then
+  sent to the wrong EDSM account.
+
+* Ensure a previous Journal file is fully read/drained before starting 
+  processing of a new one.  In particular, this ensures properly seeing the end
+  of a continued Journal file when opening the continuation file.
+
+* New config options, in a new `Privacy` tab, to hide the current Private 
+  Group, or captain of a ship you're multi-crewing on.  These usually appear
+  on the `Commander` line of the main UI, appended after your commander name,
+  with a `/` between.
+
+* EDO dockable settlement names with `+` characters appended will no longer 
+  cause 'server lagging' reports.
+  
+* Don't force DEBUG level logging to the
+  [plain log file](https://github.com/EDCD/EDMarketConnector/wiki/Troubleshooting#plain-log-file)
+  if `--trace` isn't used to force TRACE level logging.  This means logging
+  *to the plain log file* will once more respect the user-set Log Level, as in
+  the Configuration tab of Settings.
+
+  As its name implies, the [debug log file](https://github.com/EDCD/EDMarketConnector/wiki/Troubleshooting#debug-log-files)
+  will always contain at least DEBUG level logging, or TRACE if forced.
+
+(Plugin) Developers
+---
+
+* New EDMarketConnector option `--trace-on ...` to control if certain TRACE
+  level logging is used or not.  This helps keep the noise down whilst being
+  able to have users activate choice bits of logging to help track down bugs.
+
+  See [Contributing.md](Contributing.md#use-the-appropriate-logging-level) for
+  details.
+
+* Loading of `ShipLocker.json` content is now tried up to 5 times, 10ms apart,
+  if there is a file loading, or JSON decoding, failure.  This should 
+  hopefully result in the data being loaded correctly if a race condition with
+  the game client actually writing to and closing the file is encountered.
+
+* `config.get_bool('some_str', default=SomeDefault)` will now actually honour
+  that specified default.
+
 Release 5.1.2
 ===
 
@@ -45,7 +92,7 @@ Plugin Developers
 
 * Various suit data, i.e. class and mods, is now stored from relevant
   Journal events, rather than only being available from CAPI data.  In
-  general we now consider the Journal to be the canonical source of suit
+  general, we now consider the Journal to be the canonical source of suit
   data, with CAPI only as a backup.
 
 * Backpack contents should now track correctly if using the 'Resupply' option

Contributing.md

@@ -225,7 +225,39 @@ Adding `--trace` to a `pytest` invocation causes it to drop into a
 [`pdb`](https://docs.python.org/3/library/pdb.html) prompt for each test,
 handy if you want to step through the testing code to be sure of anything.
 
-Otherwise, see the [pytest documentation](https://docs.pytest.org/en/stable/contents.html).
+Otherwise, see the [pytest documentation](https://docs.pytest.org/en/stable/contents.html). 
+
+---
+## Debugging network sends
+
+Rather than risk sending bad data to a remote service, even if only through
+repeatedly sending the same data you can cause such code to instead send 
+through a local web server and thence to a log file.
+
+1. This utilises the `--debug-sender ...` command-line argument.  The argument
+  to this is free-form, so there's nothing to edit in EDMarketConnector.py 
+  in order to support a new target for this.
+2. The debug web server is set up globally in EDMarketConnector.py.
+3. In code where you want to utilise this you will need at least something 
+  like this (taken from some plugins/edsm.py code):
+
+```python
+from config import debug_senders
+from edmc_data import DEBUG_WEBSERVER_HOST, DEBUG_WEBSERVER_PORT
+
+TARGET_URL = 'https://www.edsm.net/api-journal-v1'
+if 'edsm' in debug_senders:
+    TARGET_URL = f'http://{DEBUG_WEBSERVER_HOST}:{DEBUG_WEBSERVER_PORT}/edsm'
+
+...
+    r = this.session.post(TARGET_URL, data=data, timeout=_TIMEOUT)
+```
+
+   Be sure to set a URL path in the `TARGET_URL` that denotes where the data
+   would normally be sent to.
+4. The output will go into a file in `%TEMP%\EDMarketConnector\http_debug` 
+  whose name is based on the path component of the URL.  In the code example 
+  above it will come out as `edsm.log` due to how `TARGET_URL` is set.
 
 ---
 
@@ -376,6 +408,22 @@ In addition to that we utilise one of the user-defined levels as:
   command-line argument and `.bat` file for users to enable it.  It cannot be
   selected from Settings in the UI.
 
+  As well as just using bare `logger.trace(...)` you can also gate it to only
+  log if asked to at invocation time by utilising the `--trace-on ...` 
+  command-line argument.  e.g.
+ `EDMarketConnector.py --trace --trace-on edsm-cmdr-events`.  Note how you
+  still need to include `--trace`. The code to check and log would be like:
+
+    ```python
+    from config import trace_on
+  
+    if 'edsm-cmdr-events' in trace_on:
+        logger.trace(f'De-queued ({cmdr=}, {entry["event"]=})')
+  ```
+  
+  This way you can set up TRACE logging that won't spam just because of 
+  `--trace` being used.
+
 ---
 
 ## Use fstrings, not modulo-formatting or .format

EDMarketConnector.py

@@ -99,6 +99,12 @@ if __name__ == '__main__':  # noqa: C901
         action='append',
     )
 
+    parser.add_argument(
+        '--trace-on',
+        help='Mark the selected trace logging as active.',
+        action='append',
+    )
+
     auth_options = parser.add_mutually_exclusive_group(required=False)
     auth_options.add_argument('--force-localserver-for-auth',
                               help='Force EDMC to use a localhost webserver for Frontier Auth callback',
@@ -120,8 +126,6 @@ if __name__ == '__main__':  # noqa: C901
     if args.trace:
         logger.setLevel(logging.TRACE)
         edmclogger.set_channels_loglevel(logging.TRACE)
-    else:
-        edmclogger.set_channels_loglevel(logging.DEBUG)
 
     if args.force_localserver_for_auth:
         config.set_auth_force_localserver()
@@ -146,6 +150,13 @@ if __name__ == '__main__':  # noqa: C901
 
         debug_webserver.run_listener(DEBUG_WEBSERVER_HOST, DEBUG_WEBSERVER_PORT)
 
+    if args.trace_on and len(args.trace_on) > 0:
+        import config as conf_module
+
+        conf_module.trace_on = [x.casefold() for x in args.trace_on]  # duplicate the list just in case
+        for d in conf_module.trace_on:
+            logger.info(f'marked {d} for TRACE')
+
     def handle_edmc_callback_or_foregrounding() -> None:  # noqa: CCR001
         """Handle any edmc:// auth callback, else foreground existing window."""
         logger.trace('Begin...')
@@ -1085,12 +1096,17 @@ class AppWindow(object):
             # Update main window
             self.cooldown()
             if monitor.cmdr and monitor.state['Captain']:
-                self.cmdr['text'] = f'{monitor.cmdr} / {monitor.state["Captain"]}'
+                if not config.get_bool('hide_multicrew_captain', default=False):
+                    self.cmdr['text'] = f'{monitor.cmdr} / {monitor.state["Captain"]}'
+
+                else:
+                    self.cmdr['text'] = f'{monitor.cmdr}'
+
                 self.ship_label['text'] = _('Role') + ':'  # LANG: Multicrew role label in main window
                 self.ship.configure(state=tk.NORMAL, text=crewroletext(monitor.state['Role']), url=None)
 
             elif monitor.cmdr:
-                if monitor.group:
+                if monitor.group and not config.get_bool("hide_private_group", default=False):
                     self.cmdr['text'] = f'{monitor.cmdr} / {monitor.group}'
 
                 else:

L10n/en.template

@@ -630,3 +630,15 @@
 
 /* stats.py: Status dialog title; In files: stats.py:376; */
 "Ships" = "Ships";
+
+/* prefs.py: UI elements privacy section header in privacy tab of preferences; In files: prefs.py:682; */
+"Main UI privacy options" = "Main UI privacy options";
+
+/* prefs.py: Hide private group owner name from UI checkbox; In files: prefs.py:687; */
+"Hide private group name in UI" = "Hide private group name in UI";
+
+/* prefs.py: Hide multicrew captain name from main UI checkbox; In files: prefs.py:691; */
+"Hide multi-crew captain name" = "Hide multi-crew captain name";
+
+/* prefs.py: Preferences privacy tab title; In files: prefs.py:695; */
+"Privacy" = "Privacy";

companion.py

@@ -437,7 +437,7 @@ class Auth(object):
             cmdrs = config.get_list('cmdrs', default=[])
             idx = cmdrs.index(cmdr)
             to_set = config.get_list('fdev_apikeys', default=[])
-            to_set = to_set + [''] * (len(cmdrs) - len(to_set))
+            to_set = to_set + [''] * (len(cmdrs) - len(to_set))  # type: ignore
             to_set[idx] = ''
 
         if to_set is None:
@@ -670,6 +670,10 @@ class Session(object):
             logger.warning("No lastStarport name!")
             return data
 
+        # WORKAROUND: n/a | 06-08-2021: Issue 1198 and https://issues.frontierstore.net/issue-detail/40706
+        # -- strip "+" chars off star port names returned by the CAPI
+        last_starport_name = last_starport["name"] = last_starport_name.rstrip(" +")
+
         services = last_starport.get('services', {})
         if not isinstance(services, dict):
             # Odyssey Alpha Phase 3 4.0.0.20 has been observed having
@@ -687,23 +691,24 @@ class Session(object):
 
         if services.get('commodities'):
             marketdata = self.query(URL_MARKET)
-            if last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id']):
-                logger.warning(f"{last_starport_name!r} != {marketdata['name']!r}"
-                               f" or {last_starport_id!r} != {int(marketdata['id'])!r}")
+            if last_starport_id != int(marketdata['id']):
+                logger.warning(f"{last_starport_id!r} != {int(marketdata['id'])!r}")
                 raise ServerLagging()
 
             else:
+                marketdata['name'] = last_starport_name
                 data['lastStarport'].update(marketdata)
 
         if services.get('outfitting') or services.get('shipyard'):
             shipdata = self.query(URL_SHIPYARD)
-            if last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id']):
-                logger.warning(f"{last_starport_name!r} != {shipdata['name']!r} or "
-                               f"{last_starport_id!r} != {int(shipdata['id'])!r}")
+            if last_starport_id != int(shipdata['id']):
+                logger.warning(f"{last_starport_id!r} != {int(shipdata['id'])!r}")
                 raise ServerLagging()
 
             else:
+                shipdata['name'] = last_starport_name
                 data['lastStarport'].update(shipdata)
+# WORKAROUND END
 
         return data
 

config.py

@@ -33,7 +33,7 @@ appcmdname = 'EDMC'
 # <https://semver.org/#semantic-versioning-specification-semver>
 # Major.Minor.Patch(-prerelease)(+buildmetadata)
 # NB: Do *not* import this, use the functions appversion() and appversion_nobuild()
-_static_appversion = '5.1.2'
+_static_appversion = '5.1.3'
 _cached_version: Optional[semantic_version.Version] = None
 copyright = '© 2015-2019 Jonathan Harris, 2020-2021 EDCD'
 
@@ -41,6 +41,9 @@ update_feed = 'https://raw.githubusercontent.com/EDCD/EDMarketConnector/releases
 update_interval = 8*60*60
 # Providers marked to be in debug mode. Generally this is expected to switch to sending data to a log file
 debug_senders: List[str] = []
+# TRACE logging code that should actually be used.  Means not spamming it
+# *all* if only interested in some things.
+trace_on: List[str] = []
 
 # This must be done here in order to avoid an import cycle with EDMCLogging.
 # Other code should use EDMCLogging.get_main_logger
@@ -585,7 +588,7 @@ class WinConfig(AbstractConfig):
 
         Implements :meth:`AbstractConfig.get_bool`.
         """
-        res = self.get_int(key)
+        res = self.get_int(key, default=default)  # type: ignore
         if res is None:
             return default  # type: ignore # Yes it could be None, but we're _assuming_ that people gave us a default
 

docs/Automatic Builds.md

@@ -26,3 +26,12 @@ When the workflow is (successfully) completed, it will upload the msi file it bu
 Within the `Built Files` zip file is the installer msi
 
 **Please ensure you test the built msi before creating a release.**
+
+## Automatic release creation
+
+Github Actions can automatically create a release after finishing a build (as mentioned above). To make this happen,
+simply push a tag to the repo with the format `v1.2.3` where 1.2.3 is the semver for the version (Note that this is
+**DISTINCT** from the normal `Release/1.2.3` format for release tags).
+
+Once the push is completed, a build will start, and once that is complete, a draft release will be created. Edit the
+release as needed and publish it. **Note that you should still test the built msi before publishing the release**

monitor.py

@@ -11,7 +11,7 @@ from os import SEEK_END, SEEK_SET, listdir
 from os.path import basename, expanduser, isdir, join
 from sys import platform
 from time import gmtime, localtime, sleep, strftime, strptime, time
-from typing import TYPE_CHECKING, Any, Dict, List, MutableMapping, Optional
+from typing import TYPE_CHECKING, Any, BinaryIO, Dict, List, MutableMapping, Optional
 from typing import OrderedDict as OrderedDictT
 from typing import Tuple
 
@@ -19,7 +19,7 @@ if TYPE_CHECKING:
     import tkinter
 
 import util_ships
-from config import config
+from config import config, trace_on
 from edmc_data import edmc_suit_shortnames, edmc_suit_symbol_localised
 from EDMCLogging import get_main_logger
 
@@ -203,7 +203,7 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
                 key=lambda x: x.split('.')[1:]
             )
 
-            self.logfile = join(self.currentdir, logfiles[-1]) if logfiles else None
+            self.logfile = join(self.currentdir, logfiles[-1]) if logfiles else None  # type: ignore
 
         except Exception:
             logger.exception('Failed to find latest logfile')
@@ -326,9 +326,10 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
 
         logger.debug(f'Starting on logfile "{self.logfile}"')
         # Seek to the end of the latest log file
+        log_pos = -1  # make this bound, but with something that should go bang if its misused
         logfile = self.logfile
         if logfile:
-            loghandle = open(logfile, 'rb', 0)  # unbuffered
+            loghandle: BinaryIO = open(logfile, 'rb', 0)  # unbuffered
             if platform == 'darwin':
                 fcntl(loghandle, F_GLOBAL_NOCACHE, -1)  # required to avoid corruption on macOS over SMB
 
@@ -404,7 +405,33 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
                     logger.exception('Failed to find latest logfile')
                     newlogfile = None
 
+            if logfile:
+                loghandle.seek(0, SEEK_END)		  # required to make macOS notice log change over SMB
+                loghandle.seek(log_pos, SEEK_SET)  # reset EOF flag # TODO: log_pos reported as possibly unbound
+                for line in loghandle:
+                    # Paranoia check to see if we're shutting down
+                    if threading.current_thread() != self.thread:
+                        logger.info("We're not meant to be running, exiting...")
+                        return  # Terminate
+
+                    if b'"event":"Continue"' in line:
+                        for _ in range(10):
+                            logger.trace("****")
+                        logger.trace('Found a Continue event, its being added to the list, we will finish this file up'
+                                     ' and then continue with the next')
+
+                    self.event_queue.put(line)
+
+                if not self.event_queue.empty():
+                    if not config.shutting_down:
+                        # logger.trace('Sending <<JournalEvent>>')
+                        self.root.event_generate('<<JournalEvent>>', when="tail")
+
+                log_pos = loghandle.tell()
+
             if logfile != newlogfile:
+                for _ in range(10):
+                    logger.trace("****")
                 logger.info(f'New Journal File. Was "{logfile}", now "{newlogfile}"')
                 logfile = newlogfile
                 if loghandle:
@@ -417,27 +444,6 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
 
                     log_pos = 0
 
-            if logfile:
-                loghandle.seek(0, SEEK_END)		  # required to make macOS notice log change over SMB
-                loghandle.seek(log_pos, SEEK_SET)  # reset EOF flag # TODO: log_pos reported as possibly unbound
-                for line in loghandle:
-                    # Paranoia check to see if we're shutting down
-                    if threading.current_thread() != self.thread:
-                        logger.info("We're not meant to be running, exiting...")
-                        return  # Terminate
-
-                    # if b'"event":"Location"' in line:
-                    #     logger.trace('Found "Location" event, adding to event_queue')
-
-                    self.event_queue.put(line)
-
-                if not self.event_queue.empty():
-                    if not config.shutting_down:
-                        # logger.trace('Sending <<JournalEvent>>')
-                        self.root.event_generate('<<JournalEvent>>', when="tail")
-
-                log_pos = loghandle.tell()
-
             sleep(self._POLL)
 
             # Check whether we're still supposed to be running
@@ -510,6 +516,10 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
 
             elif event_type == 'commander':
                 self.live = True  # First event in 3.0
+                self.cmdr = entry['Name']
+                self.state['FID'] = entry['FID']
+                if 'startup' in trace_on:
+                    logger.trace(f'"Commander" event, {monitor.cmdr=}, {monitor.state["FID"]=}')
 
             elif event_type == 'loadgame':
                 # Odyssey Release Update 5 -- This contains data that doesn't match the format used in FileHeader above
@@ -551,6 +561,9 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
                 if entry.get('Ship') is not None and self._RE_SHIP_ONFOOT.search(entry['Ship']):
                     self.state['OnFoot'] = True
 
+                if 'startup' in trace_on:
+                    logger.trace(f'"LoadGame" event, {monitor.cmdr=}, {monitor.state["FID"]=}')
+
             elif event_type == 'newcommander':
                 self.cmdr = entry['Name']
                 self.group = None
@@ -853,28 +866,42 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
 
             elif event_type == 'shiplocker':
                 # As of 4.0.0.400 (2021-06-10)
-                # "ShipLocker" will be a full list written to the journal at startup/boarding/disembarking, and also
+                # "ShipLocker" will be a full list written to the journal at startup/boarding, and also
                 # written to a separate shiplocker.json file - other updates will just update that file and mention it
                 # has changed with an empty shiplocker event in the main journal.
 
-                # Always attempt loading of this.
-                # Confirmed filename for 4.0.0.400
-                try:
-                    currentdir_path = pathlib.Path(str(self.currentdir))
-                    with open(currentdir_path / 'ShipLocker.json', 'rb') as h:  # type: ignore
-                        entry = json.load(h, object_pairs_hook=OrderedDict)
-                        self.state['ShipLockerJSON'] = entry
+                # Always attempt loading of this, but if it fails we'll hope this was
+                # a startup/boarding version and thus `entry` contains
+                # the data anyway.
+                currentdir_path = pathlib.Path(str(self.currentdir))
+                shiplocker_filename = currentdir_path / 'ShipLocker.json'
+                shiplocker_max_attempts = 5
+                shiplocker_fail_sleep = 0.01
+                attempts = 0
+                while attempts < shiplocker_max_attempts:
+                    attempts += 1
+                    try:
+                        with open(shiplocker_filename, 'rb') as h:  # type: ignore
+                            entry = json.load(h, object_pairs_hook=OrderedDict)
+                            self.state['ShipLockerJSON'] = entry
+                            break
 
-                except FileNotFoundError:
-                    logger.warning('ShipLocker event but no ShipLocker.json file')
-                    pass
+                    except FileNotFoundError:
+                        logger.warning('ShipLocker event but no ShipLocker.json file')
+                        sleep(shiplocker_fail_sleep)
+                        pass
 
-                except json.JSONDecodeError as e:
-                    logger.warning(f'ShipLocker.json failed to decode:\n{e!r}\n')
-                    pass
+                    except json.JSONDecodeError as e:
+                        logger.warning(f'ShipLocker.json failed to decode:\n{e!r}\n')
+                        sleep(shiplocker_fail_sleep)
+                        pass
+
+                else:
+                    logger.warning(f'Failed to load & decode shiplocker after {shiplocker_max_attempts} tries. '
+                                   'Giving up.')
 
                 if not all(t in entry for t in ('Components', 'Consumables', 'Data', 'Items')):
-                    logger.trace('ShipLocker event is an empty one (missing at least one data type)')
+                    logger.warning('ShipLocker event is missing at least one category')
 
                 # This event has the current totals, so drop any current data
                 self.state['Component'] = defaultdict(int)
@@ -882,10 +909,6 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
                 self.state['Item'] = defaultdict(int)
                 self.state['Data'] = defaultdict(int)
 
-                # 4.0.0.400 - No longer zeroing out the BackPack in this event,
-                # as we should now always get either `Backpack` event/file or
-                # `BackpackChange` as needed.
-
                 clean_components = self.coalesce_cargo(entry['Components'])
                 self.state['Component'].update(
                     {self.canonicalise(x['Name']): x['Count'] for x in clean_components}
@@ -1588,7 +1611,7 @@ class EDLogs(FileSystemEventHandler):  # type: ignore # See below
             self.state['GameVersion'] = entry['gameversion']
             self.state['GameBuild'] = entry['build']
             self.version = self.state['GameVersion']
-            self.is_beta = any(v in self.version.lower() for v in ('alpha', 'beta'))
+            self.is_beta = any(v in self.version.lower() for v in ('alpha', 'beta'))  # type: ignore
         except KeyError:
             if not suppress:
                 raise

plugins/edsm.py

@@ -10,11 +10,12 @@
 #    text is always fired.  i.e. CAPI cmdr_data() processing.
 
 import json
-import sys
+import threading
 import tkinter as tk
 from queue import Queue
 from threading import Thread
-from typing import TYPE_CHECKING, Any, List, Mapping, MutableMapping, Optional, Tuple
+from time import sleep
+from typing import TYPE_CHECKING, Any, List, Literal, Mapping, MutableMapping, Optional, Set, Tuple, Union
 
 import requests
 
@@ -22,7 +23,7 @@ import killswitch
 import myNotebook as nb  # noqa: N813
 import plug
 from companion import CAPIData
-from config import applongname, appversion, config, debug_senders
+from config import applongname, appversion, config, debug_senders, trace_on
 from edmc_data import DEBUG_WEBSERVER_HOST, DEBUG_WEBSERVER_PORT
 from EDMCLogging import get_main_logger
 from ttkHyperlinkLabel import HyperlinkLabel
@@ -35,28 +36,59 @@ logger = get_main_logger()
 
 EDSM_POLL = 0.1
 _TIMEOUT = 20
+DISCARDED_EVENTS_SLEEP = 10
 
 
-this: Any = sys.modules[__name__]  # For holding module globals
-this.session: requests.Session = requests.Session()
-this.queue: Queue = Queue()		# Items to be sent to EDSM by worker thread
-this.discardedEvents: List[str] = []  # List discarded events from EDSM
-this.lastlookup: bool = False		# whether the last lookup succeeded
+class This:
+    """Holds module globals."""
+
+    def __init__(self):
+        self.shutting_down = False  # Plugin is shutting down.
+
+        self.session: requests.Session = requests.Session()
+        self.queue: Queue = Queue()		# Items to be sent to EDSM by worker thread
+        self.discarded_events: Set[str] = []  # List discarded events from EDSM
+        self.lastlookup: requests.Response  # Result of last system lookup
+
+        # Game state
+        self.multicrew: bool = False  # don't send captain's ship info to EDSM while on a crew
+        self.coordinates: Optional[Tuple[int, int, int]] = None
+        self.newgame: bool = False  # starting up - batch initial burst of events
+        self.newgame_docked: bool = False  # starting up while docked
+        self.navbeaconscan: int = 0		# batch up burst of Scan events after NavBeaconScan
+        self.system_link: tk.Widget = None
+        self.system: tk.Tk = None
+        self.system_address: Optional[int] = None  # Frontier SystemAddress
+        self.system_population: Optional[int] = None
+        self.station_link: tk.Widget = None
+        self.station: Optional[str] = None
+        self.station_marketid: Optional[int] = None  # Frontier MarketID
+        self.on_foot = False
+
+        self._IMG_KNOWN = None
+        self._IMG_UNKNOWN = None
+        self._IMG_NEW = None
+        self._IMG_ERROR = None
+
+        self.thread: Optional[threading.Thread] = None
+
+        self.log = None
+        self.log_button = None
+
+        self.label = None
+
+        self.cmdr_label = None
+        self.cmdr_text = None
+
+        self.user_label = None
+        self.user = None
+
+        self.apikey_label = None
+        self.apikey = None
+
+
+this = This()
 
-# Game state
-this.multicrew: bool = False  # don't send captain's ship info to EDSM while on a crew
-this.coordinates: Optional[Tuple[int, int, int]] = None
-this.newgame: bool = False  # starting up - batch initial burst of events
-this.newgame_docked: bool = False  # starting up while docked
-this.navbeaconscan: int = 0		# batch up burst of Scan events after NavBeaconScan
-this.system_link: tk.Tk = None
-this.system: tk.Tk = None
-this.system_address: Optional[int] = None  # Frontier SystemAddress
-this.system_population: Optional[int] = None
-this.station_link: tk.Tk = None
-this.station: Optional[str] = None
-this.station_marketid: Optional[int] = None  # Frontier MarketID
-this.on_foot = False
 STATION_UNDOCKED: str = '×'  # "Station" name to display when not docked = U+00D7
 __cleanup = str.maketrans({' ': None, '\n': None})
 IMG_KNOWN_B64 = """
@@ -128,7 +160,8 @@ def plugin_start3(plugin_dir: str) -> str:
 
     # Migrate old settings
     if not config.get_list('edsm_cmdrs'):
-        if isinstance(config.get_list('cmdrs'), list) and config.get_list('edsm_usernames') and config.get_list('edsm_apikeys'):
+        if isinstance(config.get_list('cmdrs'), list) and \
+                config.get_list('edsm_usernames') and config.get_list('edsm_apikeys'):
             # Migrate <= 2.34 settings
             config.set('edsm_cmdrs', config.get_list('cmdrs'))
 
@@ -167,8 +200,9 @@ def plugin_stop() -> None:
     """Stop this plugin."""
     logger.debug('Signalling queue to close...')
     # Signal thread to close and wait for it
-    this.queue.put(None)
-    this.thread.join()
+    this.shutting_down = True
+    this.queue.put(None)  # Still necessary to get `this.queue.get()` to unblock
+    this.thread.join()  # type: ignore
     this.thread = None
     this.session.close()
     # Suppress 'Exception ignored in: <function Image.__del__ at ...>' errors # TODO: this is bad.
@@ -262,7 +296,7 @@ def prefs_cmdr_changed(cmdr: str, is_beta: bool) -> None:
         # LANG: We have no data on the current commander
         this.cmdr_text['text'] = _('None')
 
-    to_set = tk.DISABLED
+    to_set: Union[Literal['normal'], Literal['disabled']] = tk.DISABLED
     if cmdr and not is_beta and this.log.get():
         to_set = tk.NORMAL
 
@@ -326,6 +360,9 @@ def credentials(cmdr: str) -> Optional[Tuple[str, str]]:
     :param cmdr: The commander to get credentials for
     :return: The credentials, or None
     """
+    if 'edsm-cmdr-events' in trace_on:
+        logger.trace(f'{cmdr=}')
+
     # Credentials for cmdr
     if not cmdr:
         return None
@@ -343,13 +380,19 @@ def credentials(cmdr: str) -> Optional[Tuple[str, str]]:
         if idx >= len(edsm_usernames) or idx >= len(edsm_apikeys):
             return None
 
+        if 'edsm-cmdr-events' in trace_on:
+            logger.trace(f'{cmdr=}: returning ({edsm_usernames[idx]=}, {edsm_apikeys[idx]=})')
+
         return (edsm_usernames[idx], edsm_apikeys[idx])
 
     else:
+        if 'edsm-cmdr-events' in trace_on:
+            logger.trace(f'{cmdr=}: returning None')
+
         return None
 
 
-def journal_entry(
+def journal_entry(  # noqa: C901, CCR001
     cmdr: str, is_beta: bool, system: str, station: str, entry: MutableMapping[str, Any], state: Mapping[str, Any]
 ) -> None:
     """Journal Entry hook."""
@@ -411,7 +454,7 @@ def journal_entry(
                 to_set = ''
 
         this.station_link['text'] = to_set
-        this.station_link['url'] = station_url(this.system, str(this.station))
+        this.station_link['url'] = station_url(str(this.system), str(this.station))
         this.station_link.update_idletasks()
 
     # Update display of 'EDSM Status' image
@@ -450,11 +493,8 @@ def journal_entry(
         if state['BackpackJSON']:
             entry = state['BackpackJSON']
 
-    # Send interesting events to EDSM
-    if (
-        config.get_int('edsm_out') and not is_beta and not this.multicrew and credentials(cmdr) and
-        entry['event'] not in this.discardedEvents
-    ):
+    # Queue all events to send to EDSM.  worker() will take care of dropping EDSM discarded events
+    if config.get_int('edsm_out') and not is_beta and not this.multicrew and credentials(cmdr):
         # Introduce transient states into the event
         transient = {
             '_systemName': system,
@@ -475,12 +515,18 @@ def journal_entry(
                 'Encoded':      [{'Name': k, 'Count': v} for k, v in state['Encoded'].items()],
             }
             materials.update(transient)
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'"LoadGame" event, queueing Materials: {cmdr=}')
+
             this.queue.put((cmdr, materials))
 
         # if entry['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked'):
         #     logger.trace(f'''{entry["event"]}
 # Queueing: {entry!r}'''
 #                          )
+        if 'edsm-cmdr-events' in trace_on:
+            logger.trace(f'"{entry["event"]=}" event, queueing: {cmdr=}')
+
         this.queue.put((cmdr, entry))
 
 
@@ -533,27 +579,66 @@ TARGET_URL = 'https://www.edsm.net/api-journal-v1'
 if 'edsm' in debug_senders:
     TARGET_URL = f'http://{DEBUG_WEBSERVER_HOST}:{DEBUG_WEBSERVER_PORT}/edsm'
 
-# Worker thread
+
+def get_discarded_events_list() -> None:
+    """Retrieve the list of to-discard events from EDSM."""
+    try:
+        r = this.session.get('https://www.edsm.net/api-journal-v1/discard', timeout=_TIMEOUT)
+        r.raise_for_status()
+        this.discarded_events = set(r.json())
+
+        this.discarded_events.discard('Docked')  # should_send() assumes that we send 'Docked' events
+        if not this.discarded_events:
+            logger.warning(
+                'Unexpected empty discarded events list from EDSM: '
+                f'{type(this.discarded_events)} -- {this.discarded_events}'
+            )
+
+    except Exception as e:
+        logger.warning('Exception whilst trying to set this.discarded_events:', exc_info=e)
 
 
 def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
     """
     Handle uploading events to EDSM API.
 
+    Target function of a thread.
+
     Processes `this.queue` until the queued item is None.
     """
     logger.debug('Starting...')
-    pending = []  # Unsent events
+    pending: List[Mapping[str, Any]] = []  # Unsent events
     closing = False
+    cmdr: str = ""
+    entry: Mapping[str, Any] = {}
 
+    while not this.discarded_events:
+        if this.shutting_down:
+            logger.debug(f'returning from discarded_events loop due to {this.shutting_down=}')
+            return
+
+        get_discarded_events_list()
+        if this.discarded_events:
+            break
+
+        sleep(DISCARDED_EVENTS_SLEEP)
+
+    logger.debug('Got "events to discard" list, commencing queue consumption...')
     while True:
+        if this.shutting_down:
+            logger.debug(f'{this.shutting_down=}, so setting closing = True')
+            closing = True
+
         item: Optional[Tuple[str, Mapping[str, Any]]] = this.queue.get()
         if item:
             (cmdr, entry) = item
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'De-queued ({cmdr=}, {entry["event"]=})')
 
         else:
             logger.debug('Empty queue message, setting closing = True')
             closing = True  # Try to send any unsent events before we close
+            entry = {'event': 'ShutDown'}  # Dummy to allow for `entry['event']` below
 
         retrying = 0
         while retrying < 3:
@@ -563,48 +648,36 @@ def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
                 )
                 break
             try:
-                if TYPE_CHECKING:
-                    # Tell the type checker that these two are bound.
-                    # TODO: While this works because of the item check below, these names are still technically unbound
-                    # TODO: in some cases, therefore this should be refactored.
-                    cmdr: str = ""
-                    entry: Mapping[str, Any] = {}
+                if item and entry['event'] not in this.discarded_events:
+                    if 'edsm-cmdr-events' in trace_on:
+                        logger.trace(f'({cmdr=}, {entry["event"]=}): not in discarded_events, appending to pending')
 
-                if item and entry['event'] not in this.discardedEvents:  # TODO: Technically entry can be unbound here.
                     pending.append(entry)
 
-                # Get list of events to discard
-                if not this.discardedEvents:
-                    r = this.session.get('https://www.edsm.net/api-journal-v1/discard', timeout=_TIMEOUT)
-                    r.raise_for_status()
-                    this.discardedEvents = set(r.json())
+                if pending and should_send(pending, entry['event']):
+                    if 'edsm-cmdr-events' in trace_on:
+                        logger.trace(f'({cmdr=}, {entry["event"]=}): should_send() said True')
+                        pendings = [f"{p}\n" for p in pending]
+                        logger.trace(f'pending contains:\n{pendings}')
 
-                    this.discardedEvents.discard('Docked')  # should_send() assumes that we send 'Docked' events
-                    if not this.discardedEvents:
-                        logger.error(
-                            'Unexpected empty discarded events list from EDSM. Bailing out of send: '
-                            f'{type(this.discardedEvents)} -- {this.discardedEvents}'
-                        )
-                        continue
-
-                    # Filter out unwanted events
-                    pending = list(filter(lambda x: x['event'] not in this.discardedEvents, pending))
-
-                if should_send(pending):
-                    # if any(p for p in pending if p['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked')):
-                    #     logger.trace("pending has at least one of "
-                    #                  "('CarrierJump', 'FSDJump', 'Location', 'Docked')"
-                    #                  " and it passed should_send()")
-                    #     for p in pending:
-                    #         if p['event'] in ('Location'):
-                    #             logger.trace('"Location" event in pending passed should_send(), '
-                    #                          f'timestamp: {p["timestamp"]}')
+                    if 'edsm-locations' in trace_on and \
+                            any(p for p in pending if p['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked')):
+                        logger.trace("pending has at least one of "
+                                     "('CarrierJump', 'FSDJump', 'Location', 'Docked')"
+                                     " and it passed should_send()")
+                        for p in pending:
+                            if p['event'] in ('Location'):
+                                logger.trace('"Location" event in pending passed should_send(), '
+                                             f'timestamp: {p["timestamp"]}')
 
                     creds = credentials(cmdr)  # TODO: possibly unbound
                     if creds is None:
                         raise ValueError("Unexpected lack of credentials")
 
                     username, apikey = creds
+                    if 'edsm-cmdr-events' in trace_on:
+                        logger.trace(f'({cmdr=}, {entry["event"]=}): Using {username=} from credentials()')
+
                     data = {
                         'commanderName': username.encode('utf-8'),
                         'apiKey': apikey,
@@ -613,22 +686,23 @@ def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
                         'message': json.dumps(pending, ensure_ascii=False).encode('utf-8'),
                     }
 
-                    # if any(p for p in pending if p['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked')):
-                    #     data_elided = data.copy()
-                    #     data_elided['apiKey'] = '<elided>'
-                    #     logger.trace(
-                    #         "pending has at least one of "
-                    #         "('CarrierJump', 'FSDJump', 'Location', 'Docked')"
-                    #         " Attempting API call with the following events:"
-                    #     )
+                    if 'edsm-locations' in trace_on and \
+                            any(p for p in pending if p['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked')):
+                        data_elided = data.copy()
+                        data_elided['apiKey'] = '<elided>'
+                        logger.trace(
+                            "pending has at least one of "
+                            "('CarrierJump', 'FSDJump', 'Location', 'Docked')"
+                            " Attempting API call with the following events:"
+                        )
 
-                    #     for p in pending:
-                    #         logger.trace(f"Event: {p!r}")
-                    #         if p['event'] in ('Location'):
-                    #             logger.trace('Attempting API call for "Location" event with timestamp: '
-                    #                          f'{p["timestamp"]}')
+                        for p in pending:
+                            logger.trace(f"Event: {p!r}")
+                            if p['event'] in ('Location'):
+                                logger.trace('Attempting API call for "Location" event with timestamp: '
+                                             f'{p["timestamp"]}')
 
-                    #     logger.trace(f'Overall POST data (elided) is:\n{data_elided}')
+                        logger.trace(f'Overall POST data (elided) is:\n{data_elided}')
 
                     r = this.session.post(TARGET_URL, data=data, timeout=_TIMEOUT)
                     # logger.trace(f'API response content: {r.content}')
@@ -668,9 +742,9 @@ def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
                                 if not config.shutting_down:
                                     this.system_link.event_generate('<<EDSMStatus>>', when="tail")
 
-                            if r['msgnum'] // 100 != 1:
-                                logger.warning(f'EDSM event with not-1xx status:\n{r["msgnum"]}\n{r["msg"]}\n'
-                                               f'{json.dumps(e, separators = (",", ": "))}')
+                            if r['msgnum'] // 100 != 1:  # type: ignore
+                                logger.warning(f'EDSM event with not-1xx status:\n{r["msgnum"]}\n'  # type: ignore
+                                               f'{r["msg"]}\n{json.dumps(e, separators = (",", ": "))}')
 
                         pending = []
 
@@ -684,6 +758,12 @@ def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
             # LANG: EDSM Plugin - Error connecting to EDSM API
             plug.show_error(_("Error: Can't connect to EDSM"))
 
+        if entry['event'].lower() in ('shutdown', 'commander', 'fileheader'):
+            # Game shutdown or new login so we MUST not hang on to pending
+            pending = []
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'Blanked pending because of event: {entry["event"]}')
+
         if closing:
             logger.debug('closing, so returning.')
             return
@@ -691,18 +771,28 @@ def worker() -> None:  # noqa: CCR001 C901 # Cant be broken up currently
     logger.debug('Done.')
 
 
-def should_send(entries: List[Mapping[str, Any]]) -> bool:
+def should_send(entries: List[Mapping[str, Any]], event: str) -> bool:  # noqa: CCR001
     """
     Whether or not any of the given entries should be sent to EDSM.
 
     :param entries: The entries to check
     :return: bool indicating whether or not to send said entries
     """
+    # We MUST flush pending on logout, in case new login is a different Commander
+    if event.lower() in ('shutdown', 'fileheader'):
+        if 'edsm-cmdr-events' in trace_on:
+            logger.trace(f'True because {event=}')
+
+        return True
+
     # batch up burst of Scan events after NavBeaconScan
     if this.navbeaconscan:
         if entries and entries[-1]['event'] == 'Scan':
             this.navbeaconscan -= 1
             if this.navbeaconscan:
+                if 'edsm-cmdr-events' in trace_on:
+                    logger.trace(f'False because {this.navbeaconscan=}')
+
                 return False
 
         else:
@@ -717,6 +807,9 @@ def should_send(entries: List[Mapping[str, Any]]) -> bool:
             # Cargo is the last event on startup, unless starting when docked in which case Docked is the last event
             this.newgame = False
             this.newgame_docked = False
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'True because {entry["event"]=}')
+
             return True
 
         elif this.newgame:
@@ -726,8 +819,18 @@ def should_send(entries: List[Mapping[str, Any]]) -> bool:
                 'CommunityGoal',  # Spammed periodically
                 'ModuleBuy', 'ModuleSell', 'ModuleSwap',		# will be shortly followed by "Loadout"
                 'ShipyardBuy', 'ShipyardNew', 'ShipyardSwap'):  # "
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'True because {entry["event"]=}')
+
             return True
 
+        else:
+            if 'edsm-cmdr-events' in trace_on:
+                logger.trace(f'{entry["event"]=}, {this.newgame_docked=}')
+
+    if 'edsm-cmdr-events' in trace_on:
+        logger.trace(f'False as default: {this.newgame_docked=}')
+
     return False
 
 

prefs.py

@@ -289,6 +289,7 @@ class PreferencesDialog(tk.Toplevel):
         self.__setup_output_tab(notebook)
         self.__setup_plugin_tabs(notebook)
         self.__setup_config_tab(notebook)
+        self.__setup_privacy_tab(notebook)
         self.__setup_appearance_tab(notebook)
         self.__setup_plugin_tab(notebook)
 
@@ -671,6 +672,28 @@ class PreferencesDialog(tk.Toplevel):
         # LANG: Label for 'Configuration' tab in Settings
         notebook.add(config_frame, text=_('Configuration'))
 
+    def __setup_privacy_tab(self, notebook: Notebook) -> None:
+        frame = nb.Frame(notebook)
+        self.hide_multicrew_captain = tk.BooleanVar(value=config.get_bool('hide_multicrew_captain', default=False))
+        self.hide_private_group = tk.BooleanVar(value=config.get_bool('hide_private_group', default=False))
+        row = AutoInc()
+
+        # LANG: UI elements privacy section header in privacy tab of preferences
+        nb.Label(frame, text=_('Main UI privacy options')).grid(
+            row=row.get(), column=0, sticky=tk.W, padx=self.PADX, pady=self.PADY
+        )
+
+        nb.Checkbutton(
+            frame, text=_('Hide private group name in UI'),  # LANG: Hide private group owner name from UI checkbox
+            variable=self.hide_private_group
+        ).grid(row=row.get(), column=0, padx=self.PADX, pady=self.PADY)
+        nb.Checkbutton(
+            frame, text=_('Hide multi-crew captain name'),  # LANG: Hide multicrew captain name from main UI checkbox
+            variable=self.hide_multicrew_captain
+        ).grid(row=row.get(), column=0, padx=self.PADX, pady=self.PADY)
+
+        notebook.add(frame, text=_('Privacy'))  # LANG: Preferences privacy tab title
+
     def __setup_appearance_tab(self, notebook: Notebook) -> None:
         self.languages = Translations.available_names()
         # Appearance theme and language setting
@@ -1234,6 +1257,10 @@ class PreferencesDialog(tk.Toplevel):
         config.set('language', lang_codes.get(self.lang.get()) or '')  # or '' used here due to Default being None above
         Translations.install(config.get_str('language', default=None))  # type: ignore # This sets self in weird ways.
 
+        # Privacy options
+        config.set('hide_private_group', self.hide_private_group.get())
+        config.set('hide_multicrew_captain', self.hide_multicrew_captain.get())
+
         config.set('ui_scale', self.ui_scale.get())
         config.set('ui_transparency', self.transparency.get())
         config.set('always_ontop', self.always_ontop.get())

requirements-dev.txt

@@ -8,7 +8,7 @@ flake8-annotations-coverage==0.0.5
 flake8-cognitive-complexity==0.1.0
 flake8-comprehensions==3.5.0
 flake8-docstrings==1.6.0
-isort==5.9.2
+isort==5.9.3
 flake8-isort==4.0.0
 flake8-json==21.7.0
 flake8-noqa==1.1.0
@@ -18,7 +18,7 @@ flake8-use-fstring==1.1
 mypy==0.910
 pep8-naming==0.12.0
 safety==1.10.3
-types-requests==2.25.0
+types-requests==2.25.2
 
 # Code formatting tools
 autopep8==1.5.7