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

CtInfo: Hide Game List When Tool is Global #329

Closed sonic2kk closed 6 months ago

sonic2kk commented 6 months ago

UX tweak related to #254. Follow up from #319.

Overview

This PR hides the games list for the global compatibility tool. Currently this only applies to the Steam global compatibility tool. This is because this tool is the default for any game not using a custom compatibility tool, so it doesn't really make sense to show a games list, and any games list we show would be incomplete, and there's no need to show the list when a given tool is the global one.

I also made a minor change to set the focus to be the "Close" button when the games list is blank or when the tool is global.

image

(The number of games using the compatibility tool is 5 because I modified my config.vdf to set this tool as global instead of actually setting it as global from Steam, to avoid messing up any game configurations, since I don't actually want GE-Proton8-25 to be my global compatibility tool. Normally, this would probably be 0, so hope that clears up the discrepancy.)

Since the only defaults Valve set are Valve Proton versions (which are not displayed in ProtonUp-Qt), if ProtonUp-Qt is displaying a given tool as the global one, that means the user went out of their way to specifically set this tool as global in the Steam Settings, so they should be aware anyway that this tool is the global one and what that means :-)

This PR also makes a couple of layout changes, fixing some spacers and reducing the overall perceived padding introduced to the games list in #319. I have included a comparison screenshot of main to illustrate the UI difference.

image image

Background

When a tool is marked as global for Steam, it basically means any Windows game (or Steam app!) will use that tool by default unless a custom compatibility tool is forced from the Steam Properties menu for a specific game ("Force the use of a custom Steam Play compatibility tool"), or if Valve have specified a specific compatibility tool for that game instead.

The global compatibility tool, as far as I know, cannot be selected from the custom Steam Play Compatibility Tool dropdown on the Game Properties menu (unless you're forcing a native game to use Proton), though it could be set manually from config.vdf. However there is no reason to do this since it will be used by default if no custom compatibility tool is forced.

As a result of this behaviour, all installed games which are not using a custom compatibility tool, will be using the global compatibility tool. Therefore it is a little misleading to show a games list, as it would either be incomplete, or we would have to end up creating logic to list all the Windows games not using a compatibility tool, and there is no real need to do this since we have the Games List for that purpose. We'd have to do some kind of filtering on the from_os value for a SteamApp if we store that, and it could end up getting complicated.

Implementation

The biggest part of the diff here is the UI file edited using Qt Designer. The actual code changes were pretty small. Essentially just:

We re-use the setVisible and setEnabled calls from update_game_list_ui, and just update their logic to have an extra check for self.ctool.is_global, so we just piggyback off of these existing checks. This means the UIX from #319 when we have 0 games directly and identically applies to when we have a global compatibility tool.

I chose to add an additional check for self.ctool.is_global to these checks to be more explicit. In most cases, the global ctool should have zero games, but in cases where config.vdf is manually updated, or potentially some other weird corner cases, having this explicit check means we don't have to rely on "expected" behaviour, we can use our boolean for when we know a tool should be the global one, and work with that.

Future Work

Games List Sizing Regression

A sizing regression was introduced in #319, and has been slightly mitigated in this PR but is still problematic.

When resizing the CtInfo dialog, because of the spacers that position the lblGamesList element, the games list does not vertically fill the CtInfo dialog. The vertical spacers

We could name and hide the spacers like we do for other UI elements, but it feels a bit messy. We also can't use layouts to solve this problem, as putting the spacers, games list, and label in one layout just creates the same problem. Putting just the spacers and the label in a layout may solve the problem, but layouts cannot be hidden in Qt as far as I understand (Qt forums link, likely also applies to PySide6 as there is no setVisible for layouts since that is a property of QWidgets). So we would have to first make a parent QWidget which -> contains VBoxLayout which -> contains a Spacer + QLabel + Spacer.

This seems a bit unclean though, perhaps there is a better way to organize this layout.

Other Launchers

Right now we only mark a tool as global for Steam, but this change is not Steam-specific in case there is a time in future when we want to do this for other launchers. The game list UI changes here are not tied solely to Steam, but currently, they only impact Steam.


Just some follow-up UI tweaks to take another step towards fully implementing what was discussed for #254. Even if that issue is closed there were a couple of other bits of functionality discussed and with this PR we should be another step closer!

DavidoTek commented 6 months ago

Thanks!

reducing the overall perceived padding introduced to the games list in #319

I already was wondering what that was for when looking through the diffs, but actually didn't notice it when just running it.

The global compatibility tool, as far as I know, cannot be selected from the custom Steam Play Compatibility Tool dropdown on the Game Properties menu

I think you can set it in the Steam settings under Compatibility and then enable Enable Steam Play for all other titles. Then, you get the option Run other titles with.

Therefore it is a little misleading to show a games list [...] there is no real need to do this since we have the Games List

I agree.

Games List Sizing Regression

I wasn't able to reproduce that[1]. Is that only the case when the game list overflows?

Qt has the https://doc.qt.io/qt-6/qstackedlayout.html which allows to switch between two layouts, a bit like QTabWidget but without navigation buttons.

Other Launchers

I see. I think we only need to implement the logic for determining whether a compatibility tools is a global one, as you did for Steam in https://github.com/DavidoTek/ProtonUp-Qt/pull/314 , right?


[1] Screenshot CtInfoDialog (Branch: main @ 1a1d9ac) grafik

sonic2kk commented 6 months ago

I already was wondering what that was for when looking through the diffs, but actually didn't notice it when just running it.

Yeah, sorry about that :sweat: I took another look just now in this PR to confirm and I don't think there is any stray UI elements, but if there are I'm happy to address them here.

Qt has the https://doc.qt.io/qt-6/qstackedlayout.html which allows to switch between two layouts, a bit like QTabWidget but without navigation buttons.

Hmm, this might be what we're looking for then!

I see. I think we only need to implement the logic for determining whether a compatibility tools is a global one, as you did for Steam in https://github.com/DavidoTek/ProtonUp-Qt/pull/314 , right?

I believe so, since the only thing we're hooking into for this is checking if the ctool object has is_global. If we can set this for other launchers, the logic here should come for free :-)

I wasn't able to reproduce that[1]. Is that only the case when the game list overflows?

Ah sorry, I meant vertically.

Here's how it looks on main:

image

Here's how it looks in this PR, after some Qt Designer changes:

image

DavidoTek commented 6 months ago

QStackedLayout Hmm, this might be what we're looking for then!

I implemented it, works as expected.

Here's how it looks on main: Here's how it looks in this PR, after some Qt Designer changes:

Oh, okay. Yeah, ideally the QTableWidget would expand vertically.

That seems to be caused by two things:

  1. The "label spacers" will push the QTableWidget up. I removed them as they aren't necessary with the QStackedLayout
  2. The row indexing of the QFormLayout (for "Compatibility tool", "Game Launcher", ...) is off (starting with 1 instead of 0). I found this to be the case also for the about dialog though. Maybe this is a leftover from the Qt5 -> Qt6 conversion? Strange nevertheless. I will push a commit that fixes this.

Thanks, I will merge this now

sonic2kk commented 6 months ago

The "label spacers" will push the QTableWidget up. I removed them as they aren't necessary with the QStackedLayout

Awesome, so the QStackedLayout was the fix in the end it seems? :smile:

The row indexing of the QFormLayout (for "Compatibility tool", "Game Launcher", ...) is off (starting with 1 instead of 0). I found this to be the case also for the about dialog though. Maybe this is a leftover from the Qt5 -> Qt6 conversion? Strange nevertheless. I will push a commit that fixes this.

Oooh, that's right, that was the case with the unset labels in #186. Random aside: Did ProtonUp-Qt originally start as a Qt5 / PySide2 project? :open_mouth:


Thanks for the fixes :-)

DavidoTek commented 6 months ago

Awesome, so the QStackedLayout was the fix in the end it seems? 😄

Yes, everything related to the label is now inside a separate page.

Did ProtonUp-Qt originally start as a Qt5 / PySide2 project? 😮

I checked, it's apparently not in this repository, but I did some development for it using PySide2 before that. So the index misalignment probably comes from some minor version change.

It actually started with version 1.4.0. I think I called it this way because it was using protonup 0.1.4 for the backend code, but later switched to 2.* when going away from protonup as the API wasn't really build to use with a gui. I also just noticed that the project is around for almost 2.5 years, nice.