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.17k stars 39 forks source link

Tool List: Mark Tool as Global #314

Closed sonic2kk closed 7 months ago

sonic2kk commented 7 months ago

Addresses at least part of #254.

Overview

This PR adds (global) beside installed compatibility tools which are set as the global tool in the Steam client's Steam Play settings. When no compatibility tool is forced, but when Steam detects that a game/app is a Windows-only application, this is the default tool that will be used to run that app from the Steam Client. This can be any compatibility tool, including things like GE-Proton. In such a case, even though the compatibility tool is technically not explicitly used, it is still the default and so used by all Windows games which do not have an alternative compatibility tool forced in the Properties dialog.

image

Implementation

This PR accomplishes marking the tool as global, in summary, by doing the following:

Future Work

There are some future things we could do for this functionality, and this PR serves as a baseline for implementing them since it allows us to track which tool is the global one:


I actually wrote most of this back in June when I left my comment on #254, but I guess I abandonded it for some reason. Maybe I wanted to try and accomplish a lot more, like some of the things mentioned in Additional Work, but for now I figured this works and is better than not showing anything. I rebased it on main and polished it up a little before making this PR and made a couple of tiny adjustments, and now it's ready for review :-)

Thanks!

sonic2kk commented 7 months ago

Forgot to mention that this PR can be tested both by setting the global compatibility tool within Steam, or (with Steam closed to prevent any overwrites) by temporarily editing the 0 entry under CompatToolMapping section to use a different name. This is how I did most of my testing. For at least GE-Proton versions since the new naming scheme (though possibly further back as well) the internal name is the same as the actual name, so it can simply be set to something like GE-Proton8-25.

sonic2kk commented 7 months ago

Just noticed the "unused" counter still counts the global tool as unused. The counter says "5" but only 4 are actually unused (GE-Proton8-25 is the global tool but appears to still be counted).

I will address this tomorrow :-)

I should also just clarify that the info dialog game list is technically empty for this tool since it isn't forced as a compatibility tool in Steam, it's just the default. We could potentially fetch some os_from and os_to information from somewhere about which tool is using the global compatibility tool, or rather probably just infer it from any Windows game that runs on Linux which doesn't have a compatibility tool forced by the user or selected by Valve. But I'm not sure how to do this, and computing this might have a performance cost depending o where we source the information from.

This list could also be large, in my library this could be close to a thousand games. The overarching installed games list is pretty long but it's a much more general dialog than the compat tool info dialog. I have to wonder just how useful the info list would be at that point for the default tool... Batch update also would not make much sense for the global tool either, maybe in this PR or a follow-up we should disable that if the compat tool is marked as global.

Searching is always an option but searching, to me, is useful on this dialog to see which games I have selected a compatibility tool for, and that doesn't really apply to the default tool. I'm not sure, I figured I'd leave all of thet out of this PR and focus on only marking the tool as global without getting too wrapped up in the finer details just yet 😄

There could be an argument to have the global compat tool at the very top of the games list, with a separate more distinct background color and style, to note that it's the global tool. We could give it a separate dialog or modify the info dialog to not show the games list or game count. Essentially treating the tool differently on the UI to make it more visually clear it's the global tool but also changing how a user can and cannot interact with it.

DavidoTek commented 7 months ago

Thanks!

Code looks good at first glance and what you wrote makes sense. Will do an in-depth review of the code later.


Use badges instead of text, as it may still not be clear which tool is the global one

Ah, that would be nice :smile:

Move the global compatibility tool to the top of the list, to make it visually clearer

Good idea, that makes it more obvious for people with >10 compatibility tools.

All this to say, at the very least a warning is probably necessary when removing the global tool, if not outright disabling the remove button (maybe we could only enable it in Advanced Mode?).

We don't want anyone to break their setup, so a warning + advanced mode seems right.

Add an option to mark a given tool as the global compatibility tool. Steam would need to be closed for this action. [...] but there was not a solid consensus on where to put it

Probably the ct info dialog is the best place to put it. I think the main ui would get to crowded if we put it there.

I should also just clarify that the info dialog game list is technically empty for this tool

I guess that is fine. We could add a text saying "global tool" or something but it should be obvious from the main ui already.

maybe in this PR or a follow-up we should disable that if the compat tool is marked as global.

We could leave it as a way to change the global compatibility tool from ProtonUp-Qt. But maybe there is a cleaner way to do this, maybe from the gamelist, idk.

We could give it a separate dialog or modify the info dialog to not show the games list or game count

We could hide the game list and maybe rename "batch update" to "change global tool" or something

sonic2kk commented 7 months ago

Forgot to reply to this, sorry :sweat_smile:

Probably the ct info dialog is the best place to put it. I think the main ui would get to crowded if we put it there.

I agree.

We could hide the game list and maybe rename "batch update" to "change global tool" or something

I like this, I wonder if it should be a separate widget altogether that we hide by default and show when necessary. That way we don't have to do any checks for a click event :-)

The UI portion of hiding the games list is something I'm thinking about. I think it's a good idea, but to add to it, it might be nice to replace it with a greyed-out bit of text centered relative to where the games list would be, that indicates that the tool is global. Or, if possible, we could put this in the games list.

I'm essentially imagining something similar to what KDE's Dolphin file manager does when a folder is empty, since in this case the games list is also empty.

image

Something like this, although with less empty space since our games list is smaller. The text in our case would say something like "Global Compatibility Tool".

We could even extend this idea to unused compatibility tools, displaying "No Games" in a similar fashion. If we choose to put both labels in the list, it would make for a consistent bit of UX: when a list is empty, we display a label, and for the global tool, we display a different label :-)

sonic2kk commented 7 months ago

On my last point above, on a branch I tested out implementing something like this for "No games", here's how it could look visually. The greyed out text is achieved by "disabling" the label.

image

The exact sizing, font, etc could be figured out later, this is just a visual representation of what we could have for no games and then for the global tool as well. The UX is all wired up here too (refresh will enable/disable elements).

If we want something like this I can get a PR up for the branch I have now :-)

I actually tested with a smaller font, the above was `26` but I lowered it to `18` and think it looks better. I tested out `16` and thought it was a little too small. ![image](https://github.com/DavidoTek/ProtonUp-Qt/assets/7917345/20e53552-d8db-4305-af41-c9a281aeba47)
DavidoTek commented 7 months ago

On my last point above, on a branch I tested out implementing something like this for "No games", here's how it could look visually If we want something like this I can get a PR up for the branch I have now :-)

That looks good. I also like the smaller font (18). If you already have it, feel free do to a PR :)

DavidoTek commented 7 months ago

Thanks. Will be merged.

Changing the Batch update / Ctinfo dialog is probably something for a later PR.

I'm more in favour of following the existing "convention" here (we also do this in other places too, so it's a general convention I mean), so I'll gladly move it outside of init.

I think I need to take a look at all those attributes when I find time and maybe revise it everywhere. Python is a bit strange regarding instance attributes, for example, you also get this: https://stackoverflow.com/questions/472000/usage-of-slots :laughing:

sonic2kk commented 6 months ago

Figured out how to do this for Lutris, still got a few things to iron out though. It stores this information in its cache directory, under versions.json.

Will get a PR up in future for it, no idea when though :-)