DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.27k stars 40 forks source link

IndexError: List index out of range #454

Closed vadimen closed 1 month ago

vadimen commented 1 month ago

Please fill out following when reporting a new bug: image

Describe the bug
happens when I try to set any folder as the location for installation

To Reproduce
Steps to reproduce the behavior: try to set a location

Desktop (please complete the following information):

apt-get install libegl1 libxcb-xinerama0 libxcb-cursor0 libxkbcommon-x11-0 libxcb-xinerama0 libxcb-cursor0 libx11-xcb1 libxrender1 libxi6 libxrandr2 libxkbcommon-x11-0 libxcb-xinerama0 libxcb-cursor0 libx11-xcb1 libxrender1 libxi6 libxrandr2 libxkbcommon-x11-0 libxcb-icccm4 libxcb-keysyms1 libxcb-shape0 python3

for ubuntu 23.10 also

apt-get install libglib2.0-0 libgl1 libfontconfig1 libdbus-1-3

./AppRun

Additional context
Add any other context about the problem here.

Terminal output

ProtonUp-Qt 2.10.0 by DavidoTek. Build Info: Official AppImage by DavidoTek.
Python 3.10.4 (main, Apr  2 2022, 09:04:19) [GCC 11.2.0], PySide 6.7.2
Platform: Ubuntu 23.10 Linux-5.15.0-119-generic-x86_64-with-glibc2.38
Loading locale C / C
Loaded ctmod GE-Proton
Loaded ctmod Wine-GE
Loaded ctmod Boxtron
Loaded ctmod Kron4ek Wine-Builds Vanilla
Loaded ctmod Lutris-Wine
Loaded ctmod Luxtorpeda
Loaded ctmod Northstar Proton (Titanfall 2)
Loaded ctmod Proton Tkg
Loaded ctmod Proton Tkg (Wine Master)
Loaded ctmod Roberta
Loaded ctmod Steam-Play-None
Loaded ctmod SteamTinkerLaunch
Loaded ctmod SteamTinkerLaunch-git
Loaded ctmod vkd3d-lutris
Loaded ctmod vkd3d-proton
Loaded ctmod Wine Tkg (Valve Wine Bleeding Edge)
Loaded ctmod Wine Tkg (Wine Master)
Loaded ctmod DXVK
Loaded ctmod DXVK Async
Loaded ctmod DXVK (nightly)
Gamepad error: No gamepad found.
Removed custom install directory
Traceback (most recent call last):
  File "/home/squashfs-root/usr/local/lib/python3.10/dist-packages/pupgui2/pupgui2.py", line 399, in combo_install_location_current_index_changed
    install_dir = install_directory(self.combo_install_location_index_map[self.ui.comboInstallLocation.currentIndex()])
IndexError: list index out of range
DavidoTek commented 1 month ago

Which launchers do you have installed? Did you set a custom folder using the ... button?

I was able to replicate the error by following these steps. Is that what caused the issue for you?

  1. Remove all game launchers
  2. Start ProtonUp-Qt
  3. Press the ... button
  4. Click the Standard button in the custom install folder dialog

Selecting a folder without clicking the Standard button is possible. Default launcher install locations should be detected automatically, where are your launchers installed? Nevertheless, we should investigate what causes the error when clicking the button...

vadimen commented 1 month ago

Hi, the error happens either by pressing "Default" button when setting the folder or by introducing any other folder myself. AppRun is located in /home/squashfs-root. SteamCMD is located in /home/steam/Steam

sonic2kk commented 1 month ago

Haven't tested yet to see if I can reproduce the issue, although I am not sure what the "Standard" button is.

Also, it looks like you're decompressing the AppImage or something to run it? That might be why you need all those extra libraries. You're supposed to just run AppImages with ./, or with AppImageLauncher. If you don't have or don't want AppImage support on your system, you can also use Pacstall to install ProtonUp-Qt.

sonic2kk commented 1 month ago

Okay, I understand the issue now. I can reproduce it in both ways.

The "Default" button resets the install folder back to the default one, so if you set an override to use, for example, /run/media/gaben/Games/MyCustomTools/ as the place where you want compatibility tools to be sourced and saved, using "Default" would reset back to the default folder for that launcher (i.e. /home/gaben/.local/share/Steam/compatibilitytools.d).

However, if a default folder for that launcher was never found, trying to press "Default" will result in the error.

I can reproduce the error the way @DavidoTek mentioned, which can be done by forcing util#available_install_directories to return an empty list. This would simulate ProtonUp-Qt being unable to find any launchers. @vadimen is this what happened for you? Was ProtonUp-Qt unable to find any launchers? Does the UI look something like this for you without a Custom Install Directory?

image

If so, does this also still happen when running the AppImage normally? I have a suspicion a lot of this is caused by running the AppImage in a weird way, as it's possibly trying and failing to use some Python libraries for functionality that it cannot find.

by pressing "Default" button when setting the folder

Fwiw I think there is a bit of confusion here that the above should clarify, perhaps you thought "Default" would set the currently entered path or selected folder as some kind of default, which is not the case. There is no reason to press "Default" when adding a folder.


The issue here is caused by, as the error, suggests, this part of the code in pupgui2#combo_install_location_current_index_changed.

    def combo_install_location_current_index_changed(self):
        if not self.updating_combo_install_location:
            install_dir = install_directory(self.combo_install_location_index_map[self.ui.comboInstallLocation.currentIndex()])
            self.ui.statusBar().showMessage(self.tr('Changed install directory to {install_dir}.').format(install_dir=install_dir), timeout=3000)
            self.update_ui()

This gets called whenever the combobox value changes, which happens whenever the Custom Install Directory gets updated - a new one added, or reset to the Default for the launcher (i.e. pressing "Default" for Steam to go back to the default Steam folder). Note that assigning to install_dir appears to be only for displaying in the statusBar.

To update the value we try to set the install directory to choose to the currently selected one based on combo_install_location_index_map. But this is not safe, because it possibly makes too many assumptions about the structure of combo_install_location_index_map. The main one here is that it assumes self.combo_install_location_index_map is always going to be greater than zero, (possibly assuming self.ui.comboInstallLocation.currentIndex() is always going to return an integer is somewhat unsafe, but this comes from Qt and is a method attached to QComboBoxes, so it's the safer assumption of the two here).

Fixing this could be tricky. I am playing around locally and will update when I have a fix.

DavidoTek commented 1 month ago

Haven't tested yet to see if I can reproduce the issue, although I am not sure what the "Standard" button is.

It's the "Default" button. I forgot to set the LANG=en environmental variable for ProtonUp-Qt.

Also, it looks like you're decompressing the AppImage or something to run it? That might be why you need all those extra libraries. You're supposed to just run AppImages with ./, or with AppImageLauncher. If you don't have or don't want AppImage support on your system, you can also use Pacstall to install ProtonUp-Qt.

Yeah, would be interesting to know. That isn't related to the problem.

DavidoTek commented 1 month ago

perhaps you thought "Default" would set the currently entered path or selected folder as some kind of default Fixing this could be tricky. I am playing around locally and will update when I have a fix.

I think it would make sense to disable the button when there isn't a custom launcher currently.

sonic2kk commented 1 month ago

I think it would make sense to disable the button when there isn't a custom launcher currently.

This probably makes sense, since there is nothing to default back to. Although it wouldn't fix the issue, as trying to default back (i.e. remove the Custom Install Directory) would still result in this crash. But still a good idea I think :+1:

We need to also skip over calling install_directory(self.combo_install_location_index_map[self.ui.comboInstallLocation.currentIndex()]) if it is called when attempting to default back. I initially thought that maybe we could skip this if len(self.combo_install_location_index_map) <= 0, but this I think prevents Custom Install Directory from setting anything. Still playing around locally :smile:

sonic2kk commented 1 month ago

This gets a bit stranger...

In PupguiCustomInstallDirectoryDialog#btn_default_clicked, if we emit with a non-empty string, i.e. even something like 'b', then this issue goes away. There is no longer a crash.

In MainWindow, we call combo_install_location_current_index_changed when the index changes. We can accept this as an argument, since when this is called the new index is passed, and if this less than zero (which it is if we default with no launchers, it will be -1) we can skip calling install_directory(self.combo_install_location_index_map[self.ui.comboInstallLocation.currentIndex()]). This means in every case where combo_install_location_current_index_changed is triggered but where there is no valid index to switch to, we will avoid trying to index combo_install_location_index_map and so can avoid the index out of range error.

My hesitation around this is that I don't understand why emitting custom_id_set.emit with a non-empty string fixes the issue. Although overall I do think that validating the index we want to change to in combo_install_location_current_index_changed is correct. This function should validate that it has a valid index and ignore invalid ones, irrespective of where or how it gets called.

sonic2kk commented 1 month ago

Hm, it seems like we may not need the index after all. It seems like we can just check if len(self.combo_install_location_index_map) <= 0 and skip setting install_directory if so. I will keep testing and will try to open a PR soon with a tentative fix.

DavidoTek commented 1 month ago

My hesitation around this is that I don't understand why emitting custom_id_set.emit with a non-empty string fixes the issue

When custom_id_set is emitted with an empty string, update_combo_install_location is called with custom_install_dir='' . If there isn't any launcher detected, install_directory() will return '' (not None!).

This means, current_install_dir will equal '':

https://github.com/DavidoTek/ProtonUp-Qt/blob/cca78466954ce01d15fe4d21cbb25657264bc2db/pupgui2/pupgui2.py#L212

The condition is the following lines will be True because custom_install_dir = '' and len(custom_install_dir) = 0. This means, currentIndexChange is emitted and combo_install_location_current_index_changed is called.

https://github.com/DavidoTek/ProtonUp-Qt/blob/cca78466954ce01d15fe4d21cbb25657264bc2db/pupgui2/pupgui2.py#L227-L229