EDCD / EDMarketConnector

Downloads commodity market and other station data from the game Elite: Dangerous for use with all popular online and offline trading tools.
GNU General Public License v2.0
987 stars 155 forks source link

EDSM Preferences Tab Failing to Load on Clean Install #2232

Closed aussig closed 3 weeks ago

aussig commented 1 month ago

Please complete the following information:

Describe the bug After a first install of EDMC on a brand new machine, the EDSM tab in the preferences is failing to load because the core EDSM plugin is throwing an exception.

Note that I have only noticed this while starting to work on my own plugin again on the new machine, so I have run the game a couple of times and set all my controls up. However I have tested that this bug is exhibiting itself with no other plugins installed (I have removed them for testing this bug).

To Reproduce Steps to reproduce the behavior:

  1. Install EDMC on a new machine with no other plugins
  2. Run it. Also (I assume) run the game to get a valid cmdr.
  3. Go to FileSettings
  4. Observe no EDSM tab

Expected behavior The EDSM tab should show to allow configuration.

Screenshots Screenshot 2024-05-12 155907

Additional context I see in the logs that this is being caused by len() being called for edsm_usernames before edsm_usernames has a value.

I had a look at the code - In the code block just above the exception:

    cmdrs = config.get_list('edsm_cmdrs')
    if not cmdrs:
        # Migrate from <= 2.25
        cmdrs = [cmdr]
        config.set('edsm_cmdrs', cmdrs)

    edsm_usernames = config.get_list('edsm_usernames')
    edsm_apikeys = config.get_list('edsm_apikeys')

Shouldn't the other two lists also be initialised if not cmdrs. e.g. something like:

    cmdrs = config.get_list('edsm_cmdrs')
    edsm_usernames = config.get_list('edsm_usernames')
    edsm_apikeys = config.get_list('edsm_apikeys')

    if not cmdrs:
        # Migrate from <= 2.25
        cmdrs = [cmdr]
        edsm_usernames = [""]
        edsm_apikeys = [""]
        config.set('edsm_cmdrs', cmdrs)
        config.set('edsm_usernames ', edsm_usernames )
        config.set('edsm_apikeys ', edsm_apikeys )

The reason I haven't put in a PR for the above is that although I believe this would fix the problem for future clean installs, it doesn't recover my current situation - i.e. on my machine edsm_cmdrs is set correctly but the other two lists are not. I don't know whether you consider that important enough to add code to guard against it too?

EDMarketConnector-debug.log EDMarketConnector.log

Rixxan commented 1 month ago

When in the heck did this bug get introduced???

Bug confirmed, #2233 created to fix. Thanks for reporting!

aussig commented 1 month ago

Erm, well now I'm confused. Launched EDMC today and went to settings (to look at something else) and I see that the EDSM tab is now showing. Checked the logs and the exception is no longer being thrown.

Rixxan commented 1 month ago

It seems this will be fixed anytime something tries to set an EDSM config set. 2233 seems to have fixed it, so will mark issue as staged for next release. Will include in 5.11.