From ad9504160539750de0944df0cb1e9cc395b09a86 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 28 Nov 2022 10:51:25 +0000 Subject: [PATCH 1/4] edsm: Send gameversion/build in all messages * Record the 'state' version of these in `this`. * Use those when constructing the message. * NB: Need to check if messages can be retained in the queue across client changes. Coming up .... --- plugins/edsm.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/edsm.py b/plugins/edsm.py index 8bd5fd30..2c826f58 100644 --- a/plugins/edsm.py +++ b/plugins/edsm.py @@ -71,6 +71,9 @@ class This: def __init__(self): self.shutting_down = False # Plugin is shutting down. + self.game_version = "" + self.game_build = "" + self.session: requests.Session = requests.Session() self.session.headers['User-Agent'] = user_agent self.queue: Queue = Queue() # Items to be sent to EDSM by worker thread @@ -432,6 +435,9 @@ def journal_entry( # noqa: C901, CCR001 if should_return: return + this.game_version = state['GameVersion'] + this.game_build = state['GameBuild'] + entry = new_entry this.on_foot = state['OnFoot'] @@ -726,6 +732,8 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently 'apiKey': apikey, 'fromSoftware': applongname, 'fromSoftwareVersion': str(appversion()), + 'fromGameVersion': this.game_version, + 'fromGameBuild': this.game_build, 'message': json.dumps(pending, ensure_ascii=False).encode('utf-8'), } From 5743fd38034a224a5f35f5687264460c62adc65f Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 28 Nov 2022 11:04:51 +0000 Subject: [PATCH 2/4] edsm: Push gameversion/build into the queue to ensure correctness 1. Due to the _TIMEOUT on the actual `post()` of a message it would be possible for new entries to get queued in the meantime. These queued entries could be 'in session' and end up going through pending and thus sent before one of the 'new session' events is detected so as to clear pending. The `this.gameversion/build` could have changed in the meantime, so are no longer correct if game client changed. 2. So, pass in the current gameversion/build when a message is pushed into the queue, and parse those back out when they're pulled out of the queue. 3. Use those versions in the message, not `this.` versions. --- plugins/edsm.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/edsm.py b/plugins/edsm.py index 2c826f58..bf82b815 100644 --- a/plugins/edsm.py +++ b/plugins/edsm.py @@ -552,7 +552,7 @@ entry: {entry!r}''' materials.update(transient) logger.trace_if(CMDR_EVENTS, f'"LoadGame" event, queueing Materials: {cmdr=}') - this.queue.put((cmdr, materials)) + this.queue.put((cmdr, this.game_version, this.game_build, materials)) if entry['event'] in ('CarrierJump', 'FSDJump', 'Location', 'Docked'): logger.trace_if( @@ -561,7 +561,7 @@ Queueing: {entry!r}''' ) logger.trace_if(CMDR_EVENTS, f'"{entry["event"]=}" event, queueing: {cmdr=}') - this.queue.put((cmdr, entry)) + this.queue.put((cmdr, this.game_version, this.game_build, entry)) # Update system data @@ -663,10 +663,10 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently logger.debug(f'{this.shutting_down=}, so setting closing = True') closing = True - item: Optional[Tuple[str, Mapping[str, Any]]] = this.queue.get() + item: Optional[Tuple[str, str, str, Mapping[str, Any]]] = this.queue.get() if item: - (cmdr, entry) = item - logger.trace_if(CMDR_EVENTS, f'De-queued ({cmdr=}, {entry["event"]=})') + (cmdr, game_version, game_build, entry) = item + logger.trace_if(CMDR_EVENTS, f'De-queued ({cmdr=}, {game_version=}, {game_build=}, {entry["event"]=})') else: logger.debug('Empty queue message, setting closing = True') @@ -732,8 +732,8 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently 'apiKey': apikey, 'fromSoftware': applongname, 'fromSoftwareVersion': str(appversion()), - 'fromGameVersion': this.game_version, - 'fromGameBuild': this.game_build, + 'fromGameVersion': game_version, + 'fromGameBuild': game_build, 'message': json.dumps(pending, ensure_ascii=False).encode('utf-8'), } @@ -815,7 +815,7 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently 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 + # Game shutdown or new login, so we MUST not hang on to pending pending = [] logger.trace_if(CMDR_EVENTS, f'Blanked pending because of event: {entry["event"]}') From 1ec1253b486937cd0cf2f430c687f1b010ecde53 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 28 Nov 2022 11:08:48 +0000 Subject: [PATCH 3/4] appversion: Change to 5.6.0-alpha2 to be distinct * This is alpha, not beta. * We have an -alpha0 and a -beta1 already, so use -alpha2 so even that digit is distinct. --- config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/__init__.py b/config/__init__.py index 70d0603e..bd8c1418 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -52,7 +52,7 @@ appcmdname = 'EDMC' # # Major.Minor.Patch(-prerelease)(+buildmetadata) # NB: Do *not* import this, use the functions appversion() and appversion_nobuild() -_static_appversion = '5.6.0-alpha0' +_static_appversion = '5.6.0-alpha2' _cached_version: Optional[semantic_version.Version] = None copyright = '© 2015-2019 Jonathan Harris, 2020-2022 EDCD' From a581d889fece98b053a7ec076db6cade490fc9bb Mon Sep 17 00:00:00 2001 From: Athanasius Date: Mon, 28 Nov 2022 12:18:10 +0000 Subject: [PATCH 4/4] edsm: Add a paranoia check for changed gameversion * In theory we would always see `Fileheader` and clear `pending[]`, but let's be extra paranoid and also clear it if there's a gameversion/build difference between the prior event and the current one. --- plugins/edsm.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/edsm.py b/plugins/edsm.py index bf82b815..8ddb7661 100644 --- a/plugins/edsm.py +++ b/plugins/edsm.py @@ -644,6 +644,8 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently pending: List[Mapping[str, Any]] = [] # Unsent events closing = False cmdr: str = "" + last_game_version = "" + last_game_build = "" entry: Mapping[str, Any] = {} while not this.discarded_events: @@ -692,6 +694,20 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently logger.trace_if( CMDR_EVENTS, f'({cmdr=}, {entry["event"]=}): not in discarded_events, appending to pending') + # Discard the pending list if it's a new Journal file OR + # if the gameversion has changed. We claim a single + # gameversion for an entire batch of events so can't mix + # them. + # The specific gameversion check caters for scenarios where + # we took some time in the last POST, had new events queued + # in the meantime *and* the game client crashed *and* was + # changed to a different gameversion. + if ( + entry['event'].lower() == 'fileheader' + or last_game_version != game_version or last_game_build != game_build + ): + pending = [] + pending.append(entry) # drop events if required by killswitch @@ -823,6 +839,9 @@ def worker() -> None: # noqa: CCR001 C901 # Cant be broken up currently logger.debug('closing, so returning.') return + last_game_version = game_version + last_game_build = game_build + logger.debug('Done.')