Custom Install Directory: Use Inline if for Safer Default Launcher Selection

[sonic2kk]

Fix #370.

Almost forewent my usual PR format, but that would be no fun, right? 😉

Overview

Changes the check in the Custom Install Dialog that selects the default launcher to use an inline if, which fixes a crash if self.launcher is not a key in self.install_locations_dict. This currently occurs when there are no available launchers installed. We get a KeyError if no launchers are found, because self.launcher is not a valid key in self.install_locations_dict.

Problem

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. For example with an inline if I guess the conditional gets parsed first, like a "full" if, so you can do dictionary[key] if key in dictionary else 'value' safely, as Python won't process the dictionary[key] first. Makes sense having thought it through (you wouldn't want print('hello') if condition else print ('goodbye') to print hello, so condition has 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. On main, 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 😅

Thanks!

[DavidoTek]

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]

Ah damn you're right, self.install_locations_dict.get(self.launcher, 'steam') would probably also work (so it would fall back to 'steam'). Totally didn't occur to me 😅