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

pupgui2: Disable Remove Button for Global Compatibility Tool #327

Closed sonic2kk closed 10 months ago

sonic2kk commented 10 months ago

Another piece for #254.

This PR disables btnRemoveSelected when the currently selected compatibility tool is_global. We already have a loop where we disable based on the compatibility tool type, so this just adds an extra condition :-)

Currently this PR does not:

The approach I took is based on partial discussion in #314: Disable the Remove button for the global tool by default. I figure there's no need to allow remove and then show a warning, we can simply show a warning in advanced mode and users can optionally proceed. We don't want users to risk anything without this enabled, and hopefully it's clear that the global tool shouldn't be removed. The user would have to explicitly set this anyway, so they should also be able to unset it.

Only showing one dialog and only doing it in Advanced Mode probably also makes implementation easier, since we don't have to juggle dialog elements like we do for the SteamTinkerLaunch dialogs :-)

I should be able to add the dialog in this PR as well, but if you'd prefer it go in a separate PR that's fine too.

Thanks!

DavidoTek commented 10 months ago

Thanks!

I should be able to add the dialog in this PR as well, but if you'd prefer it go in a separate PR that's fine too.

I wonder if anyone would actually want to delete their global compatibility tool as it would probably break something. Doing a Batch Update and then deleting it makes more sense. We can merge this for now and if there is actually any use case for being able to just delete it, someone will probably complain. We can then add the Advancedmode+Warning Dialog. Then we could also add a tooltip that tells that advanced mode is required. Feel free to create a PR if you want.

sonic2kk commented 10 months ago

I also am not sure of any reason to remove the global tool. I'm not even sure what happens if you remove it from the Steam Client, I imagine Steam does something to try and resolve it. If you manually remove it though I imagine the same thing happens as when you remove a compatibility tool set for a game. For example:

Even if Steam can resolve this scenario, once again, I don't know why you would do this without first changing the global tool to something else. Even if you remove it while Steam is open, I imagine it'll try to launch with a non-existent compatibility tool and fail. When you change the global compatibility tool you also have to restart the Steam Client anyway. Although if they do this they will also have to restart ProtonUp-Qt. But I think cases like this are very niche anyway.

Once ProtonUp-Qt can actually change the global compatibility tool, I think 99% of cases where someone would want to remove a global compat tool will be resolved. They can switch to a different Proton flavour and then remove the tool.

So I'm not super attached to adding a warning dialog, and if you also don't think it's urgent, we can just wait to see if anyone requests it then.

DavidoTek commented 10 months ago

So I'm not super attached to adding a warning dialog, and if you also don't think it's urgent, we can just wait to see if anyone requests it then.

Yes, I also don't think we need this, so let's wait and see.

Once ProtonUp-Qt can actually change the global compatibility tool, I think 99% of cases where someone would want to remove a global compat tool will be resolved

That makes sense.