search

mtgatracker / python-mtga

MTGA set data & tooling for python
MIT License
26 stars 23 forks source link

Slightly smarter logic to find the MTGA install location. #18

Closed pak21 closed 4 years ago

pak21 commented 4 years ago

First of all, big thanks for the dynamic card finder, I've been missing my ELD cards :-)

That said, I can think of two scenarios in which the hardcoded C:\Program Files (x86)\Wizards of the Coast\MTGA\MTGA_Data\Downloads\Data isn't correct:

  1. (Untested) It is possible to change the default install location in Windows, often done to install things other than on the C: drive. I have no idea if the MTGA installer honours this, but it's in theory possible.
  2. There's no guarantee that the Python is being run under Windows. This is actually my use case in that I do my Python dev work in either Windows Subsystem for Linux or dual booted. The MTGA files are accessible in both cases but just not at C:\....

This change (tested under Windows 10, WSL/Ubuntu and native boot Debian) allows at least a small amount of configuration for the location of the MTGA files. It does "the right thing" on Windows when the ProgramFiles(x86) environment variable exists, falling back to ProgramFiles if that doesn't exist, and allows bodging it a bit by explicitly setting ProgramFiles in the other environments.

Technically, I think the fallback from ProgramFiles(x86) to ProgramFiles isn't necessary here as MTGA requires a 64-bit Windows install and that means that ProgramFiles(x86) will always be set... but it's impossible to set an environment variable with parentheses in the name on a Unix-ish OS so it's handy to have anyway.

If you think there's a better way of allowing the MTGA install location to be specified, feel free to ignore this patch and do that instead - in the longer term, there will probably be a need to support the upcoming macOS version as well.

Spencatro commented 4 years ago

@pak21 thanks so much for the PR! You're definitely right, hardcoding the path was shortsighted, and totally screws anyone who installs to a different directory. I have an idea of how to fix this longer term, but in the meantime I'm totally fine with this going in. I will do my best to get this published to pypi sometime this week!