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

CtInfo: Hide Game List When Tool is Global #329

Merged
merged 4 commits into from
Dec 12, 2023

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Dec 9, 2023

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:

  • Hiding the games list of the current ctool is global.
  • Hiding the batch update button if the current ctool is global.
  • Disabling the search button if the current ctool is global.
  • Showing the lblGamesList (by default says "No games", introduced in CtInfo: Display 'No games' text in place of empty games list #319) if the current ctool is global.
  • Changing the lblGamesList text to say "Tool is Global" instead of the default "No Games" text if the current ctool is global.
    • This means even though the global ctool should normally have 0 games, we prioritize displaying the "Tool is Global" message, which makes a bit more sense since it isn't quite right to say "No games". It also means we can re-use the lblGamesList.

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

DavidoTek commented Dec 9, 2023

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 #314 , right?


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

@sonic2kk
Copy link
Contributor Author

sonic2kk commented Dec 9, 2023

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

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

@DavidoTek DavidoTek merged commit 8d352c4 into DavidoTek:main Dec 12, 2023
@sonic2kk
Copy link
Contributor Author

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? 😄

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? 😮


Thanks for the fixes :-)

@DavidoTek
Copy link
Owner

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.

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