aussig / BGS-Tally

A tool to track and report your Background Simulation (BGS) and Thargoid War (TW) activity in Elite Dangerous, implemented as an EDMC plugin. BGS Tally counts all the work you do for any faction, in any system.
https://discord.gg/YDNVtjPnnm
MIT License
31 stars 9 forks source link

Fails on Update / Restart #165

Closed combivanCoder closed 7 months ago

combivanCoder commented 7 months ago

Describe the bug On loading EDMC today, BGS-Tally notes that an update will be applied on restart. On restart the plugin fails to load

To Reproduce Steps to reproduce the behavior:

  1. Await autoupdate
  2. When promoted to exit to update / update on restart do so
  3. Start EDCM again
  4. See error

Expected behavior Update applied with no apparent ill effect / normal operation of EDMC / BGS-Tally

Screenshots

bgstally-bug-1

Desktop (please complete the following information):

Additional context EDMC logs show the issue is potentially related to a datetime parsing issue (paths truncated)

2024-02-10 16:02:03.271 UTC - ERROR - 43044:48716:48716 plug._load_found_plugins:203: Failure loading found Plugin "BGS-Tally-Vanilla"
Traceback (most recent call last):
  File "plug.pyc", line 200, in _load_found_plugins
  File "plug.pyc", line 74, in __init__
  File "EDMarketConnector\plugins\BGS-Tally-Vanilla\load.py", line 21, in plugin_start3
    this.plugin_start(plugin_dir)
  File "EDMarketConnector\plugins\BGS-Tally-Vanilla\bgstally\bgstally.py", line 76, in plugin_start
    self.tick:Tick = Tick(self, True)
                     ^^^^^^^^^^^^^^^^
  File "EDMarketConnector\plugins\BGS-Tally-Vanilla\bgstally\tick.py", line 25, in __init__
    if load: self.load()
             ^^^^^^^^^^^
  File "EDMarketConnector\plugins\BGS-Tally-Vanilla\bgstally\tick.py", line 66, in load
    self.tick_time = datetime.strptime(config.get_str("XTickTime", default=self.tick_time.strftime(DATETIME_FORMAT_ELITEBGS)), DATETIME_FORMAT_ELITEBGS)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "_strptime.pyc", line 568, in _strptime_datetime
  File "_strptime.pyc", line 349, in _strptime
ValueError: time data '2024-02-10T07:13:12' does not match format '%Y-%m-%dT%H:%M:%S.%fZ'
combivanCoder commented 7 months ago

Btw: Reverting to 3.3.0 works but immediately tries to update on restart

aussig commented 7 months ago

Huh... That's an internal state variable that hasn't changed in this version (or for years). Somehow it has been set to a different format.

Do you have any other plugins installed in EDMC?

combivanCoder commented 7 months ago

I'll double check in the morning but I haven't added or removed any plugins recently.

Rest are disabled I think. When next at pc I'll disable all and test bgstally clean with no others

aussig commented 7 months ago

Do you have download links for those two plugins? I'll take a look and see whether either could be using the same XTickTime state.

combivanCoder commented 7 months ago

Huh. Put in a bit of logging and discovered that the tick ID being retreived from the config is 2024-02-10T07:13:12. So not being parsed from another format into this.

To test a theory I put a temporary try-catch around the tick::load method to get the plugin loading with some logging and then forced the tick... now working fine. So overwriting the problematic last time time in config sorted the problem.

I'm not familiar enough with EDMC or Python really but it looks like these settings were persisted somewhere despite me removing all plugin files and deploying latest from the repo's releases list. Can't see any of these settings in config/config.ini though.

Other Plugins For Reference

aussig commented 7 months ago

Yes the XTickTime value is retrieved from EDMC config. On Windows, EDMC stores this in the Windows Registry, so you won't see it in any files on disk.

These Registry keys are unfortunately shared between all EDMC plugins, hence my question about other plugins, because if something else uses the key XTickTime it will overwrite it. I inherited that key name from Tez's original plugin code, and for more recent config values I've used a more distinct BGST_ prefix instead, to make the BGST keys more globally unique.

I've taken a look through the code for those other three plugins and can't see any references to XTickTime in them, so am still scratching my head as to how that could have got in there with the wrong date format.

I see you've already fixed this yourself by catching and handling the exception. Two other solutions spring to mind:

  1. Manually remove that entry from the Windows Registry
  2. As you are clearly experienced with code, write a tiny EDMC plugin that deletes that config entry using config.delete().

Would you like to submit a PR to handle the exception, or if not I'll get a fix done under this issue. Though I think this is a one-off as I haven't had any other problem reports (yet) with 3.4.0.

combivanCoder commented 7 months ago

Would you like to submit a PR to handle the exception, or if not I'll get a fix done under this issue. Though I think this is a one-off as I haven't had any other problem reports (yet) with 3.4.0.

After a bit of thought... I wonder if this was related to the experimental PR for change of tick detection? Noone else in the sqdn has yet reported the same issue. The tick datetime format was different on the EDCD variant I think which would explain this adjustment between versions.

aussig commented 7 months ago

Ah, gosh yes I forgot you were involved with that PR! That will definitely be the cause of this problem - BGST assumes a set format for the tick date/time and if it was different then that would explain it.

I've had 500 installs of the new version and no other reports of this problem so yes, I think we can be certain this is the cause.

combivanCoder commented 7 months ago

Yeh its looking that way - sorry for the wild goose chase there.PEBKAK at my end :) ---- On Mon, 12 Feb 2024 13:19:53 +0100 @.*** wrote ---- Ah, gosh yes I forgot you were involved with that PR! That will definitely be the cause of this problem - BGST assumes a set format for the tick date/time and if it was different then that would explain it. I've had 500 installs of the new version and no other reports of this problem so yes, I think we can be certain this is the cause.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

aussig commented 7 months ago

Heh, no problem sir! Closing.