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.

Sign up for GitHub

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

Jump to bottom

CtInfo: Add Button to Mark Compat Tool as Global #336

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

CtInfo: Add Button to Mark Compat Tool as Global #336

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
83537b1
CtInfo: Add button to make compat tool global default
sonic2kk Dec 16, 2023
6e8cbdf
CtInfo: Only show Mark Global button for Steam launcher
sonic2kk Dec 16, 2023
5236d50
CtInfo: Only show Mark Global for CTType.CUSTOM
sonic2kk Dec 16, 2023
98dc0ec
CtInfo: Initial implementation of set_global_compat_tool
sonic2kk Dec 16, 2023
cbd2318
CtInfo: Don't show Mark Global button for Global Tool
sonic2kk Dec 17, 2023
03cd5c3
CtInfo: Remove print
sonic2kk Jan 30, 2024
b2ded73
steamutil: Enable writing to config.vdf
sonic2kk Jan 30, 2024
5aad7b8
Merge branch 'main' into global-compat-tool-button
DavidoTek Mar 24, 2024
618083e
steamutil: `return 1` when adding shortcut fails
sonic2kk Apr 20, 2024
5b0793a
CtInfo: Break out is_mark_global_available out into a util function
sonic2kk Apr 20, 2024
6fd4d26
CtInfo: Correctly bind btn_mark_global_clicked to btnMarkGlobal
sonic2kk Apr 25, 2024
9f7c1c2
Refactor to use generic set_launcher_global_tool util function
sonic2kk Jun 2, 2024
02fd807
Set correct tab focus order
sonic2kk Jun 2, 2024
24061ce
remove unnecessary property from CtInfo designer file
sonic2kk Jun 2, 2024
05fb631
Toggle Mark Global button visibility after pressing
sonic2kk Jun 16, 2024
08849b7
Use return value of `set_steam_global_compat_tool` in `set_launcher_g…
sonic2kk Jun 16, 2024
922f9cf
Don't allow marking already global compat tools as global
sonic2kk Jul 6, 2024
c1fa092
steamutil: Simplify set_steam_global_compat_tool
sonic2kk Jul 6, 2024
6394f9b
Move self.is_mark_global_available assignment to update_game_list_ui
sonic2kk Jul 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion pupgui2/pupgui2ctinfodialog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pupgui2.datastructures import BasicCompatTool, CTType, SteamApp, LutrisGame, HeroicGame
from pupgui2.lutrisutil import get_lutris_game_list
from pupgui2.pupgui2ctbatchupdatedialog import PupguiCtBatchUpdateDialog
from pupgui2.steamutil import get_steam_game_list
from pupgui2.steamutil import get_steam_game_list, set_global_compat_tool
from pupgui2.util import open_webbrowser_thread, get_random_game_name
from pupgui2.heroicutil import get_heroic_game_list, is_heroic_launcher

Expand All @@ -29,6 +29,7 @@ def __init__(self, parent=None, ctool: BasicCompatTool = None, install_loc=None)
self.games: List[Union[SteamApp, LutrisGame, HeroicGame]] = []
self.install_loc = install_loc
self.is_batch_update_available = False
self.is_mark_global_available = (self.ctool.ct_type == CTType.CUSTOM and not self.ctool.is_global) and self.install_loc.get('launcher') == 'steam' and 'vdf_dir' in self.install_loc
sonic2kk marked this conversation as resolved.
Show resolved Hide resolved

self.load_ui()
self.setup_ui()
Expand All @@ -46,6 +47,7 @@ def setup_ui(self):
self.ui.txtInstallDirectory.setText(self.ctool.get_install_dir())
self.ui.btnBatchUpdate.setVisible(False)
self.ui.searchBox.setVisible(False)
self.ui.btnMarkGlobal.setVisible(self.is_mark_global_available)

self.update_game_list()

Expand All @@ -64,6 +66,7 @@ def update_game_list(self, cached=True):
self.is_batch_update_available = True
self.ui.btnBatchUpdate.setVisible(not self.ui.searchBox.isVisible())
self.ui.btnBatchUpdate.clicked.connect(self.btn_batch_update_clicked)
self.ui.btnMarkGlobal.clicked.connect(self.btn_mark_global_clicked)
elif self.install_loc.get('launcher') == 'lutris':
self.update_game_list_lutris()
elif is_heroic_launcher(self.install_loc.get('launcher')):
Expand Down Expand Up @@ -161,3 +164,7 @@ def search_ctinfo_games(self, text):
for row in range(self.ui.listGames.rowCount()):
should_hide: bool = not text.lower() in self.ui.listGames.item(row, 1).text().lower()
self.ui.listGames.setRowHidden(row, should_hide)

def btn_mark_global_clicked(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should disable the button after clicking because there currently is no visual feedback of the event.

That could be as simple as self.btnMarkGlobal.setEnabled(false). We would need to add another check in the constructor so that the user won't click the button if it is already global.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's a good idea!

There's no reason to click this button again after marking a tool as global, at least not in the context of the currently open dialog.

An edge case could be opening two ctinfo dialogs at once, say GE-Proton9-7 and Luxtorpeda (just to have distinct examples). This is extremely unlikely to ever happen and could be alleviated with other behaviour, it's not a reason against implementing this button disabling but an outline of a potential problem scenario (although again, I really don't think anyone would do this if they weren't trying to break something)

  1. User opens both dialogs at once.
  2. User marks GE-Proton9-7 as Global.
  3. With this suggestion, the button is disabled.
  4. User then marks Luxtorpeda as Global without closing the prior dialog.
  5. Perhaps a user did this in error and now they want GE-Proton9-7 to be the global tool again.
  6. But since the global button is disabled on the GE-Proton9-7 dialog, they cannot.
  7. Re-opening the dialog would fix this but it may not be very visually obvious.

A big way to alleviate this being a problem is the confirmation dialog noted earlier, because it would vastly reduce the likelihood of accidentally setting a tool as global.

There's also the possibility of closing the dialog which could be seen as expected since it's on the bottom row of the dialog buttons, but perhaps also not.

Disabling the button makes sense to me, I'll add it when I get home. I'm not sure if we want to close dialogs after a tool is marked as global, it might be a little jarring.

I also had a thought, I don't remember what happens if a tool is marked as global and then the game list is refreshed. If we get the "tool is global" text like if the user closed and re-opened the dialog, maybe the button also gets hidden, which I think is good too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, actually it seems like we don't hide the Mark Global button when the games list is refreshed.

We check if we should display the Mark Global button when loading the ctinfo dialog with self.ui.btnMarkGlobal.setVisible(self.is_mark_global_available), perhaps we should also call this in update_game_list_ui?

I'm thinking we should rework the logic like this:

  1. Remove the initial btnMarkGlobal.setVisible call from setup_ui.
  2. setup_ui calls update_game_list (updates launcher-specific UI elements) which in turn calls update_game_list_ui (handles generic non-launcher-specific UI elements)
  3. In update_game_list_ui we can call check if the Mark Global button should be displayed using self.ui.btnMarkGlobal.setVisible(self.is_mark_global_available) (since we may want to display this for other launchers, and is_mark_global_available will handle checking if Mark Global is available for the current launcher anyway)
    a. Right now the logic is only set if the current launcher is Steam, but with the recent refactor, we can call this generically and call this regardless of our current launcher, because is_mark_global_available is responsible for checking if the current launcher supports this action
  4. We also move the self.ui.btnMarkGlobal.clicked.connect action to connect the button logic from update_game_list to update_game_list_ui
    a. Similar to above, this is also nested in a Steam-specific launcher check, but calling it from update_game_list_ui means we can connect this action regardless of launcher. Then the only thing we need to care about is if is_mark_global_available is True or False, and we don't need to care about knowing whether or not the launcher should support it, that's taken care of by the call to is_mark_global_available.
  5. Update set_launcher_global_tool to return a success boolean
  6. Change the call to set_launcher_global_tool to actually update is_mark_global_available, like this: self.is_mark_global_available = set_launcher_global_tool(self.install_loc, self.ctool). We could I guess hardcode self.is_mark_global_available = False but that would assume set_launcher_global_tool will always succeed. If set_launcher_global_tool fails, this approach means we will update the value correctly.
    a. We could simply leave set_launcher_global_tool unchanged and instead make another call like this: self.is_mark_global_available = is_mark_global_available(self.install_loc, self.ctool) to achieve the same thing. The approach here is more stylistic preference, although I think personally making set_launcher_global_tool return a success boolean is a bit cleaner.
  7. This would mean we wouldn't need to disable the button. Instead, we will update is_mark_global_available and our existing call to btn_refresh_games_clicked will handle running all the checks again for whether or not the Mark Global button should be displayed, using the same refactored logic described above that we would use when the dialog opens.

Basically we move the logic to set whether the button is visible or not into update_game_list_ui, meaning we will update it regardless of launcher, because is_mark_global_available performs a launcher check for us. Then when we mark a tool as global, we update the value of self.is_mark_global_available. Finally our existing call to refresh the game list elements means we will pick up any changes to is_mark_global_available and show/hide the button.

The reason showing and hiding may be better is because we show/hide the button when the dialog opens.

set_global_compat_tool(self.ctool, self.install_loc.get('vdf_dir'))
self.btn_refresh_games_clicked()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to self.ctool is updated when the refresh button is clicked. This means self.ctool.is_global is still False. So when the tool is marked as global, or when the refresh button is generally clicked (i.e. if the global Steam compat tool was updated externally while the CtInfo dialog was open, and the refresh button was pressed after that) the dialog does not updated to reflect that the tool is global.

So if GE-Proton9-7's CtInfo dialog is open, and the global compat tool in config.vdf is updated to be GE-Proton9-7, pressing the refresh button will not show/hide the Mark Global button, and it will not update the game list to display "Tool is Global".

However, it does update the compat tool list on the Main Menu to display (global) because the compat tool name!

And re-opening the dialog of course works as expected.

I'm not sure how to fix this one, it is a bit of an edge-case and at worst something we can update in a later PR.

7 changes: 7 additions & 0 deletions pupgui2/resources/ui/pupgui2_ctinfodialog.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,13 @@
</property>
</widget>
</item>
<item>
<widget class="QPushButton" name="btnMarkGlobal">
<property name="text">
<string>Set as Global</string>
</property>
</widget>
</item>
<item>
<spacer name="horizontalSpacer">
<property name="orientation">
Expand Down
27 changes: 26 additions & 1 deletion pupgui2/steamutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,4 +791,29 @@ def determine_most_recent_steam_user(steam_users: List[SteamUser]) -> SteamUser:
return steam_users[0]

print('Warning: No Steam users found. Returning None')
return None
return None

def set_global_compat_tool(ctool: BasicCompatTool, steam_config_folder):
""" Update the global compatibility tool to the ctool parameter defined in config.vdf CompatToolMapping (AppID '0') """

if ctool.ct_type == CTType.CUSTOM:
config_vdf_file = os.path.join(os.path.expanduser(steam_config_folder), 'config.vdf')
sonic2kk marked this conversation as resolved.
Show resolved Hide resolved
if not os.path.exists(config_vdf_file):
return False

try:
d = vdf.load(open(config_vdf_file))
c = get_steam_vdf_compat_tool_mapping(d)

# Create or update the '0' CompatToolMapping entry
if not c.get('0', {}):
c['0'] = { 'name': ctool.get_internal_name(), 'config': '', 'priority': '75' }
else:
c['0']['name'] = ctool.get_internal_name()

vdf.dump(d, open(config_vdf_file, 'w'), pretty=True)
sonic2kk marked this conversation as resolved.
Show resolved Hide resolved
except Exception as e:
print(f'Error, could not update Global Steam compatibility tool to {ctool.displayname}: {e}, vdf: {config_vdf_file}')
return False

return True
sonic2kk marked this conversation as resolved.
Show resolved Hide resolved