Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pupgui2: Disable Remove Button for Global Compatibility Tool #327

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Dec 3, 2023

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:

  • Allow removing in Advanced Mode, though that would be trivial to add
  • Show a warning dialog of any sort

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
Copy link
Owner

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.

@DavidoTek DavidoTek merged commit 367b413 into DavidoTek:main Dec 6, 2023
@sonic2kk
Copy link
Contributor Author

sonic2kk commented Dec 6, 2023

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:

  • If you set GE-Proton8-25 as the compatibility tool for a single game (Properties -> Force the use of a Steam Play compatibility tool -> GE-Proton8-25), close Steam, remove that tool (either manually or with a cool program like ProtonUp-Qt), and re-open Steam, you can't launch the game and Steam removes the Play button, and says: "Available for:" and shows a Windows and/or macOS icon. This is because in config.vdf, that AppID is still under CompatToolMapping with a compatibility tool that does not exist.
  • I imagine, since global compatibility tools are held under the same CompatToolMapping, just with AppID 0, that something similar would happen to all games using the default compatibility tool, unless Steam has some kind of resolution mechanism
  • If a Valve compatibility tool is selected (i.e. Valve Proton versions or the Steam Linux Runtime), if it is not installed, it will be installed prior to a game launch, because the Steam Client can manage these tools, but it cannot manage external tools like the ones in compatibilitytools.d :-)

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
Copy link
Owner

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.

@sonic2kk sonic2kk deleted the disable-remove-global-ctool branch December 26, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants