Custom Install Directory Dialog Overhaul

I had some time so I decided to take a look at overhauling the custom install directory dialog, making several small tweaks that turned out to be quite an overhaul. This is not yet complete but I made enough progress that I'm ready to open it and get some feedback :-)

Overview

This PR overhauls the Custom Install Directory dialog in a number of ways:

Convert the dialog to use a .ui file - This doesn't reduce the size of the file in the end but allows for consistency with the other dialogs.
Add a short description on the purpose of this dialog - Phrasing improvements welcome!
Add a "Close" button, which allows for the dialog to be closed when running in GameScope (i.e. on Steam Deck).
Add a "Default" button, which takes the place of the "Reset" functionality previously coupled to the "Save" button.
- This emits custom_id_set when clicked, so the launcher dropdown on the main menu will update.
Add some logic to the "Save" button which only enables it if a valid directory path is selected and if we can write to this directory.
- This is only checked when the path is being entered, unless there is an existing check for custom install directories, there is no logic added in this PR (currently anyway) which validates that a given custom install directory still exists on boot of ProtonUp-Qt. So if a path disappeared I am not sure what would happen.
Rework the launcher names in the dropdown, so instead of using something like heroicwine, it now says Heroic (Wine) - Very minor change I wanted to make :sweat_smile:
Select the current launcher in the combobox - So if the dialog is opened when we're on Lutris, it will select "Lutris".

Implementation

I have a couple of reservations about the way I went about some things in this PR specifically that I wanted to comment on.

set_selected_launcher is identical to the logic in the install dialog for checking the exact same thing -- Finding the index of a dropdown element based on the text and selecting that item. Maybe this could be broken out into a utility function?
I created a method get_dict_key_from_value, which is used to get the "internal" name key (i.e. heroicwine) from the display text (i.e. Heroic (Wine)). This feels... not-pythonic at best. If there is a better way to go about this please do correct me :-)
There's an explicit call to emit the index changed signal when removing the selected custom install directory. This is because when the custom dir is selected in the main menu and then defaulted in the dialog, the launcher combobox value on the main menu updates (defaults to the first element) but it doesn't emit the signal. So when we send an empty string to the custom install directory signal (i.e. when the default button is pressed) we check for that and manually emit the index changed signal. I'd be happy to take any feedback to improve this!

And of course feedback is welcome on the rest of the code, those are just items I specifically wanted to call out.

Considerations

I don't have Bottles installed so I haven't tested any of these changes against it. However, for Steam (steam package from Arch repos), Lutris (lutris-git from AUR), and Heroic (Flatpak), things seem to work as expected.

When selecting "Bottles" and adding a custom install directory through, I noticed it just adds the path and does not set a display name. This happens in main as well. For example adding /home/gaben/PressurizedBottles/, the dropdown in the main menu only displays the path and not the launcher display name, nor any icon. This isn't something that broke in this PR but it is something that could be fixed here or in future.

I'm not sure how often this feature is used (to be honest I haven't actually used it...), but I wanted to try my hand at improving it, so all feedback to better reach that end goal is welcome :-)

Thanks!

P.S. - Looks like the combined downloads and Flathub installs has surpassed 1 million, congrats! :-)

TODO

[ ] Potentially improve how the custom install directory is written out (icon, display name)
[x] Select the new directory in the main menu on "Save"
[x] Refresh compat tool list when removing currently selected custom dialog (dropdown value updates but compat tool list does not)

There are a couple of areas that I noticed could be improved while working on this PR, but which may not really fit this PR:

The remove logic in install_directory could be split out into a separate function. It appears this logic is only used by the custom install directory dialog
A base class dialog could be created, since each dialog now uses a UI file. This would take the path to the UI file and automatically call load_ui based on the given path to the UI file and such. This would reduce the duplicated code between dialog and streamline creating new dialogs, which could end up being useful for #196 if any additional dialogs are required for this :-)

I broke the logic for finding the combobox text in set_selected_launcher out into a util function, and applied it for this custom install dialog as well as the install dialog. I felt it would be cleaner to go ahead and do so :-)

Great!

Having three buttons, instead of just one seems like a good idea :)

which validates that a given custom install directory still exists on boot of ProtonUp-Qt. So if a path disappeared I am not sure what would happen.

I think it will just not display anything. Not much of a problem.

I created a method get_dict_key_from_value, which is used to get the "internal" name key (i.e. heroicwine) from the display text (i.e. Heroic (Wine)). This feels... not-pythonic at best. If there is a better way to go about this please do correct me :-)

There are more-pythonic ways, see e.g. https://www.geeksforgeeks.org/python-get-key-from-value-in-dictionary/ But I think the get_dict_key_from_value method works fine and is easy to comprehend while at least some pythonic methods are quite messy in my opinion.

There's an explicit call to emit the index changed signal when removing the selected custom install directory

Intersting. Might be caused by https://github.com/DavidoTek/ProtonUp-Qt/blob/3c81fc754604619f7e7a759ba3da2f5a5941e488/pupgui2/pupgui2.py#L363-L364

When selecting "Bottles" and adding a custom install directory through, I noticed it just adds the path and does not set a display name

You're right. Currently, it just ignores the display name and launcher. See https://github.com/DavidoTek/ProtonUp-Qt/blob/3c81fc754604619f7e7a759ba3da2f5a5941e488/pupgui2/util.py#L200

The remove logic in install_directory could be split out into a separate function. It appears this logic is only used by the custom install directory dialog

Not sure, do you mean calling config_custom_install_location(install_dir='remove')?

A base class dialog could be created, since each dialog now uses a UI file [...] which could end up being useful for Add support for CryoByte's Steam Deck Utilities #196

Yeah, that could be a useful addition to make development easier.

Thanks!

P.S. - Looks like the combined downloads and Flathub installs has surpassed 1 million, congrats! :-)

Nice, I have seen that too. I think we're doing a great job of improving user experience for Linux and Steam Deck gaming! :tada: :tada: :tada:

[...] some pythonic methods are quite messy in my opinion.

Yeah, some of the ways in that link are shorter for sure but not quite as clean.

Intersting. Might be caused by [code here]

I was messing around here but I just tried removing the check in combo_install_location_current_index_changed and yup, now the method is called properly.

Just out of curiosity, what does the updating_combo_install_location do? Just so I can get a better idea of how to potentially fix the logic so that we don't have to make an explicit call to the combobox signal :slightly_smiling_face:

I wonder if it might be worth creating another signal for custom install directory removed. Not sure for now.

You're right. Currently, it just ignores the display name and launcher

Ah good point, and also here (sorry, I don't know how to do the actual line embed :sweat:)

https://github.com/DavidoTek/ProtonUp-Qt/blob/main/pupgui2/util.py#L271

config_custom_install_location also notes that display_name is always ''. That could be changed in this PR, or at a later date, or not at all if it's going to be a breaking change :sweat_smile:

Not sure, do you mean calling config_custom_install_location(install_dir='remove')?

Yeah, I was just thinking that it might be slightly cleaner to have it as a separate function, or perhaps now that I think about it, it could be a separate argument. Of course it isn't a massive issue, it was just a small observation :smile:

I pushed a couple of small changes related to fixing some incorrect behaviour with and when using get_dict_key_from_value, and some small comment cleanup.

Just out of curiosity, what does the updating_combo_install_location do?

It will clear the install location combo box in the main dialog and re-populate it with all available install locations. It is called from custom install dialog so the new launcher gets added.

I wonder if it might be worth creating another signal for custom install directory removed.

updating_combo_install_location should handle that fine.

I don't know how to do the actual line embed sweat

Pasting a permalink to a line should do that automatically. Probably it is not working here because it's a normal link, and not a permalink.

it might be slightly cleaner to have it as a separate function, or perhaps now that I think about it, it could be a separate argument

Yeah, a separate argument would be a clean option. EDIT: Implemented with https://github.com/DavidoTek/ProtonUp-Qt/pull/225/commits/e5e25e29d616a5d483f0eaba464bd90e48b6039d

One thing I noticed is that the dialog doesn't show the current custom path when opened (but neither did the old version of the dialog). I'll take a look and add it quick.

Thanks

DavidoTek / ProtonUp-Qt