DavidoTek
commented
2 months ago Thanks!
Oh, that fix makes sense. I guess the idea of the custom install dialog is to have the ability to add a launcher manually if it is not detected automatically.
Apparently, in Python, dictionary[key] or 'value' will result in a KeyError if key is not defined. However in other cases, this does not happen
Hmm, a self.install_locations_dict.get(self.launcher) or 'steam' probably would do the same. I guess your solution achieves the same effect.
I did some testing and this prevents the crash, and the crash I was encountering matches the one described in #370 (KeyError: '' and noting line 53, where this fix is implemented). I am very confident this is the cause of the crash.
I didn't explicitly test that scenario, but the current code is definitively defective. Seems reasonable that this is the defect which causes the error.
What I am not sure of is how no Steam installation was found in #370. For some reason this seems to be coming up more in recent months for no clear reason...
Yes, that's a bit strange. I haven't encountered anything similar yet, but I don't exactly have the most exotic setup either.
For example, adding a custom install directory will not immediately find all compatibility tools, you have to restart the launcher If you force no launchers to be found and then add a valid path to a Steam installation's compatibilitytools.d, you have to restart ProtonUp-Qt for any tools to display.
Okay. I agree, this could use some reworking.
Almost forewent my usual PR format, but that would be no fun, right? 😉 Small change, big PR description 😅
I like it. Prevents any confusion about the content of the PR 😉
This one will be a quick merge.
sonic2kk
Fix #370.
Almost forewent my usual PR format, but that would be no fun, right? :wink:
Overview
Changes the check in the Custom Install Dialog that selects the default launcher to use an inline
if, which fixes a crash ifself.launcheris not a key inself.install_locations_dict. This currently occurs when there are no available launchers installed. We get aKeyErrorif no launchers are found, becauseself.launcheris not a valid key inself.install_locations_dict.Problem
Apparently, in Python,
dictionary[key] or 'value'will result in aKeyErrorifkeyis not defined. However in other cases, this does not happen. For example with an inlineifI guess the conditional gets parsed first, like a "full"if, so you can dodictionary[key] if key in dictionary else 'value'safely, as Python won't process thedictionary[key]first. Makes sense having thought it through (you wouldn't wantprint('hello') if condition else print ('goodbye')to printhello, soconditionhas to be processed fiirst), but tripped me up a little.How to Replicate Error
You can test this fix by forcefully returning False at the beginning if
util#is_valid_launcher_installation. This will result in ProtonUp-Qt not finding any launchers. Onmain, opening the Custom Install Directory dialog will give a crash. With this change, it should work correctly.I did some testing and this prevents the crash, and the crash I was encountering matches the one described in #370 (
KeyError: ''and noting line 53, where this fix is implemented). I am very confident this is the cause of the crash. What I am not sure of is how no Steam installation was found in #370. For some reason this seems to be coming up more in recent months for no clear reason...Future Work
There are some UIX issues I'd like to tackle in separate PRs that won't depend on the changes here. For example, adding a custom install directory will not immediately find all compatibility tools, you have to restart the launcher. If you force no launchers to be found and then add a valid path to a Steam installation's
compatibilitytools.d, you have to restart ProtonUp-Qt for any tools to display.Small change, big PR description :sweat_smile:
Thanks!