arviceblot/eso-addons

Clean up config handling

Closed this issue · 7 comments

Let's be honest, the config code is a bit of a mess. Let's clean it up by:

  • Separating out the os-specific code for the addon_dir from the initial config file creation
  • Move the config dir creation check out of the addon service
  • Consider creating traits for the config struct to house most of the config functions
  • Remove or consolidate other config functionality

Would it be possible to streamline the GNU/Linux ESO locations during this clean-up? Currently there are two different specifications, one for the Steam Deck (which is just the generic way of Steam, but with the deck username hard-coded) and one for GNU/Linux which I assume references a generic WINE location. This leaves out people who use another distro that also use Steam for ESO, and those who use different locations for generic WINE installs of ESO.

To resolve the Steam issue, you'd have it call the current user's username in the file-path - as an example:
/home/$USER/.local/share/Steam/steamapps/compatdata/306130/pfx/drive_c/users/steamuser/Documents/Elder Scrolls Online/live/AddOns

That will handle basically all the normal Steam set-ups, regardless of distro (aside from a few edge-cases due to their unique goals of the distro, but the average end-user won't be using one of those distros to begin with).

Honestly, if I were to suggest a design for handling this, it'd be:

  1. Two options for handling the client for the user to choose between in the settings; Steam Install, Non-Steam Install (drop-down for this is probably ideal)
  2. Steam Install would reference the Steam location with dynamic username handling for the home folder
  3. Non-Steam Install would prompt the user to manually specify the explicit location of the Add-Ons directory just like Trojan's original config did, but you can use the file-picker to prompt via GUI - this prompt would dynamically appear/hide based upon value of the drop-down previously suggested

Yeah, basing the path off the current user should be easy enough. Ultimately on the first run I'd like it to auto check several paths and pick one if it exists. Otherwise be able to manually set the path through the UI like option 3 with a basic setup/onboarding screen. TBH the existing config handling is a bit of a mess because I just wanted to get it working.

Also, thank you for the interest in this project. Let me know if you have any other feedback or contributions and I'll gladly consider it.

@MonospacedFonts I pushed a new release last night with some updates to the config and AddOn directory selection (among other things). It's kind of basic for now, but if you get a chance to install it, let me know if it works for you and I'll close this issue.

I selected the folder, and it detects the correct folder by default - so that part works. It seems to detect the add-ons from the log that was being filled upon first run of the new build, but it doesn't actually display them even after restarting the program. It just says no add-ons are installed.

I tried seeing if installing an add-on works after that, but when I click on the 'Install' button, the entire add-on list disappears and reappears instantly with no action occurring; button doesn't change from 'Install' and nothing written to the add-on directory either when I go check.

Edit: Forgot to mention, I tested this with the native build and the AppImage, both present the same issues.

Edit 2: Upon further testing, it appears that the download issue happens only on the 'Browse' section (and when you click on an entry there to open the details). You can still successfully download items using the 'Search' view. I also saw some error, so I ran a backtrace and here is what is the result:

2023-05-27T08:20:55.881424Z  INFO eso_addons_core::api: Requesting: https://api.mmoui.com/v3/game/ESO/filedetails/3505.json
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: ApiParseResponse { source: reqwest::Error { kind: Decode, source: Error("invalid type: map, expected a sequence", line: 1, column: 0) }, url: "https://api.mmoui.com/v3/game/ESO/filedetails/3505.json" }', core/src/api.rs:76:68
stack backtrace:
   0: rust_begin_unwind
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
   1: core::panicking::panic_fmt
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
   2: core::result::unwrap_failed
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1750:5
   3: eso_addons_core::service::AddonService::p_update_addon_details::{{closure}}
   4: eso_addons_core::service::AddonService::p_install::{{closure}}
   5: lazy_async_promise::immediatevalue::ImmediateValuePromise<T>::new::{{closure}}
   6: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
   7: tokio::runtime::task::core::Core<T,S>::poll
   8: tokio::runtime::task::harness::Harness<T,S>::poll
   9: tokio::runtime::scheduler::multi_thread::worker::Context::run_task
  10: tokio::runtime::scheduler::multi_thread::worker::Context::run
  11: tokio::macros::scoped_tls::ScopedKey<T>::set
  12: tokio::runtime::scheduler::multi_thread::worker::run
  13: tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut
  14: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  15: tokio::runtime::task::harness::Harness<T,S>::poll
  16: tokio::runtime::blocking::pool::Inner::run
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I checked the add-on that error referenced ("I Was Blocking") and I'm not using that add-on, so that leads me to believe it's related to the 'Browse' issue, but I could be wrong. Let me know if there's anything else you need and I'll try to do what I can to help.

Unfortunately I haven't hooked up the "Install" button from the browse section yet. So I'm a bit surprised an addon is attempting to install. I looked up "I was blocking" and apparently that's a deleted addon so it won't be possible to install. Would you mind sending a zip of your .config/eso-addons dir so I can check it out?

Apologies for the delay, I sent it to you via E-Mail.

I didn't see anything too strange in the files but thanks for sending them. I would recommend updating to the latest version and see if the problem persists. Since the topic as deviated from the original issue I'm going to close this. Any remaining issues can be discussed separately.