search

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.22k stars 40 forks source link

Show a message box when a compatibility tool failed to load #184

Closed teppyboy closed 1 year ago

teppyboy commented 1 year ago

Is your feature request related to a problem? Please describe. Without reading the log, I didn't notice that there were some CTs failed to load.

Describe the solution you'd like Show a message box about CTs failed to load, and optionally the reason too

Describe alternatives you've considered Read the logs, but since ProtonUp-Qt is designed for user-friendly experience I don't think we need to read the log for that thing.

teppyboy commented 1 year ago

The message box can be like this:

image (yes, this is a real message box implementation in my fork and these CTs failed to load because of missing 'zstandard' dependency)

DavidoTek commented 1 year ago

Having a way to inform the user if something really really goes wrong can make sense. I'm not sure whether it should/can also contain the error message to the dialog. Maybe a expandable section which contains the precise error and a text Please report a bug on GitHub <url>.

because of missing 'zstandard' dependency

In the first place we should focus on resolving this kind of issues. Getting an issue due to a missing dependency (or missing internet connection, etc.) could confuse many users.

Maybe it would be better if the error dialog first will be displayed when the user actually tries to select/install the broken compatibility tool as (currently at least) the ctmod is only used for installing and not for detecting/removing compatibility tools. Though I'm open on this.

in my fork

Fell free to open a PR :)

teppyboy commented 1 year ago

Maybe it would be better if the error dialog first will be displayed when the user actually tries to select/install the broken compatibility tool

The current implementation doesn't show the CT when it failed to load, maybe we can show all the CTs in the dropdown and only load them if the user click on them

Fell free to open a PR :)

Done :smile:, also implemented the detailed section (which is hidden by default) show the precise error. image

sonic2kk commented 1 year ago

Looks really nice! Though I am interested in how this error got through in the first place.

Flatpak should manage all necessary dependencies, not sure about AppImage but I would guess so too right? It should have all the Python libs. Though this would be useful in cases like #169, where Flatpak was missing the Zstandard library (though cases like this should be few and far between).

So this would impact running from source, where there could be an instance that a new dependency is added to the requirements.txt but the installed dependencies are not. The requirements.txt does have zstandard, so nothing should be missing there.

Regardless of this specific instance, it is good in general to show this dialog. Ctmods failing to load should really never happen but even for developing it would be handy to have :+1:

teppyboy commented 1 year ago

Though I am interested in how this error got through in the first place.

I installed it from 3rd party Arch repos (chaotic-aur) so lol, my fault

DavidoTek commented 1 year ago

Same here, looks very good!

The current implementation doesn't show the CT when it failed to load, maybe we can show all the CTs in the dropdown and only load them if the user click on them

Interesting idea, we should keep it in mind. The name of the ctmod should be available in CT_NAME without having to load the whole module.

I installed it from 3rd party Arch repos (chaotic-aur)

Maybe someone can create an issue here: https://github.com/chaotic-aur/packages/issues