frozenpandaman / s3s

Successor to splatnet2statink. Takes battle data from the SplatNet 3 app and uploads it to stat.ink!
https://github.com/frozenpandaman/s3s/wiki
GNU General Public License v3.0
394 stars 72 forks source link

Add appdirs to store config data in platform specific user directory #144

Closed Lokno closed 2 months ago

Lokno commented 11 months ago

Currently the script stores user data in a generated file called config.txt To stay up-to-date on changes to s3s, users are asked to clone the repo. There are plans to release an executable in the near future. I tested using pyinstaller on the script myself and I was able to generate an exe without issue.

However, if the user were to download a new executable, or even now if they redownload the code in a different location, they might forget to keep the config.txt file in the same folder, or might accidentally delete it.

The appdirs module stores user data in a platform specific location on the client machine.

Example:

import appdirs

...

user_dir = appdirs.user_data_dir("s3s","frozenpandaman")
config_file = os.path.join(user_dir , "config.txt")

The script should still look for and prioritize a config.txt file if it exists in the current directory. That way it won't break existing user setups when they upgrade.

If you like the idea, I could put together a pull request.

frozenpandaman commented 11 months ago

@Lokno Hey! I like the idea – thanks for the suggestion. The only thing I'm worried about is that, if this is the default setup going forward, it raises the overall complexity of configuring s3s a bit. Some people run s3s just to get the gtoken in config.txt, say, which they then plug into their browser to access SplatNet that way, or regularly look at it to see their stat.ink API key, or even swap between two config files for different users, and other similar use cases. As a result, I don't want it to be hidden out of the way in (in my case) ~/Library/Application Support/… especially because ~/Library/ is hidden on macOS by default now, so I'd need add even more instructions telling people how to navigate to it via Terminal or via Go > Go to Folder in Finder and have them type out the path manually… etc. It's a lot easier to just be able to say "it gets generated wherever s3s.py is", you know?

I really do like the idea quite a bit but I'm just worried that it complicates things (especially for users without lots of tech literacy). The config.txt file, in my mind, should always be very accessible and easy to find (and edit) – by everyone. It'd be nice if this could be opt-in… but doing that would probably require changing something in config.txt itself, so… 😆

I'm traveling for the next few weeks and can reply more when I return later this month – sorry for the delay! But let me know what you think.

Lokno commented 11 months ago

@frozenpandaman

This is a valid concern. These locations are necessarily obfuscated from the user. But there are benefits to an class of user who will likely never touch config.txt.

Here are some options to mitigate user confusion and interference with existing workflows:

Once again, happy to take this on as a pull request if we're in agreement that its worthwhile. No rush, have a safe trip.

frozenpandaman commented 8 months ago

@Lokno Very sorry for the late reply. I want to respond to this before the next update (which I need to do soon because it fixes some breaking SR stuff, and a version bump is the only way for users to get prompted for the automatic update, and it's already been a week...)

But there are benefits to an class of user who will likely never touch config.txt.

This is true! But it's also... kind of easy to just ignore it, and not ever touch it even if it's right there in front of them. However, editing a text file to change settings is something I'd like anyone to be able to do if they ever need/want, and for users who aren't super comfortable doing even this (yes, I have gotten messages asking me how to open and edit a text file) I want them to not be dissuaded even further but instead get more comfortable doing this sort of stuff on a computer. So having the .txt right there for most users seems ideal to me, philosophically and practically.

  • prompt the user during initialization if they would like config.txt to be generated in the same directory

I like this idea, but then how would s3s know where to look for/find it in the future? All those settings are stored inside config.txt itself, so the logic becomes a bit circular...


I think this suggestion is what would be best:

  • a config.txt in the same directory as s3s.py takes precedence over the adddirs location

So how about s3s still creates config.txt by default in the normal place, and looks for it there... but if it can't find it, instead of automatically creating a new one, it does a fallback check to look in the user data dir? That way you (or anyone else) can move it there manually as a one-time operation and nothing should break.

The only thing is, appdirs would require an addition to requirements.txt since it doesn't come with Python by default, and at this point in the project's lifetime, I don't think I want to modify that file to introduce another requirement, especially one that won't be used by the vast majority of people and is only super-optional for a fallback case... etc. I'm interested in other solutions but (as the existence of the module suggests) it's pretty hard/annoying to be able to find this directory reliably cross-platform, so I'm not sure how to do this.

As a result I don't think I'll implement this as part of the imminent v0.6.0 but definitely happy to keep discussing and get this added in the future, if/when we find a solution.

Open to your thoughts here!

Lokno commented 8 months ago

Your suggested change in the saving behavior sounds reasonable. It would still address the scenario where a user is running a version of s3s.py from a new location, and it would just work without needing to have saved config.txt manually. When reading the configuration, it would look for config.txt in the local directory first, and than in the appdirs location. When writing the configuration, it would write it to both locations. So whenever the file needs to be updated, it would again create config.txt in the local directory. In this way, upgrading to a new version wouldn't require manually moving config.txt or re-authenticating.

As for the appdirs requirement, I agree that requiring the script to depend on this module for a QoL feature isn't ideal. Instead, I would suggest attempting to use the module, if installed, and quietly failing:

def get_appdir_path(file_name,app_name="s3s",app_author="frozenpandaman"):
    if not 'appdirs' in sys.modules:
        try:
            global appdirs
            import appdirs
        except ModuleNotFoundError:
            return None
    file_dir = appdirs.user_data_dir(app_name, app_author)
    return os.path.join(file_dir, file_name) 

This function encapsulates the appdirs dependency. If the path is None than this option is unavailable. Note, the directories might not exist, so os.makedirs() will need to be called before this location can be written to.

Happy to continue to discuss the exact behavior for a potential later release. Thank you for your feedback and consideration.

frozenpandaman commented 2 months ago

Hey! Sorry to disappoint; I honestly never really got around to this since it's a pretty big change (conceptually, at least) in terms of script behavior/function, and I don't play the game much anymore (at all, really). If other people want to try implementing something like this and submit a PR, I'm open! I think this should not be surfaced to users unless perhaps they update the code themselves to flip a boolean, e.g. like the USE_OLD_NSOAPP_VER key in iksm.py. Just for the sake of cleanliness & lack of activity/discussion (my fault, I know!) I'll close this issue for now.