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

util: Break VDF actions into util functions #407

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Jun 2, 2024

Marking as a draft as it needs a little bit of further testing, and needs #336 as some code in that PR will be migrated over to using this new approach.

Note: The functionality in this PR is specific to text-based VDF files (such as config.vdf or libraryfolders.vdf). It does not cover the binary VDF files (such as shortcuts.vdf).

Overview

This PR implements the VDF splitting logic discussed in #336. This makes interacting with text-based VDF files a lot easier, as we only need to give the path to this file, and a function that will perform some action on this. We accomplish this by defining a function that expects some VDF file dict, and it will manipulate the information in that dict.

This function is then passed to our new util function which loads the VDF file, runs this function to modify the VDF file dict, and then handles writing that updated data out for us.

Doing things this way means:

  • The util function handles checking, opening+loading, and writing to the VDF file data for us, including the details on how the vdf library is used so that we don't have to remember this each time we want to write to the VDF file.
  • The function that wants to write to the VDF file only needs to know how to interact with the data in the VDF file. This means it only has to know the implementation details of the data it wants to modify, and nothing else.
    • It only has to expect to take in some specific VDF file dict, and it knows how to manipulate that. It doesn't have to load this file itself or handle writing this data out. It only has to know, as an example, "I expect to be given a loaded config_vdf_dict and I will handle updating the data I care about in this dict".

Background

The goal of this PR was to avoid duplicated and convoluted calls to vdf.dump, primarily when writing to config.vdf. The problem is that each time we want to do this, we need to know a lot of implementation details and we need to do a bit of boilerplate.

  • We always need to get the config.vdf file handle, check that it exists, and then parse it with vdf.load.
  • We usually need to get, for example, the CompatToolMapping section from this.
  • We perform some action to modify some part of the parsed VDF file dict, for example modifying the CompatToolMapping section inside of it.
  • We have to write this dict out to the VDF file, remembering the implementation on how to use vdf.dump.
  • The variable names don't have great names 😅

Implementation

This PR resolves those issues by creating one util function called update_vdf_text_data that handles all of the details on loading and writing with a text-based VDF file, and accepts a function that takes that loaded VDF file and manipulates it. This util function then handles writing this data out again. This means

  • The caller of update_vdf_text_data doesn't have to know how to load the VDF file, only how it wants to modify the data.
  • The update_vdf_text_data function doesn't have to know how the caller wants to modify the VDF file, it only has to know how to load and write the VDF file.

There is a small convenience wrapper around update_vdf_text_data specific to writing to config.vdf as this is quite common.

For the two examples in this PR, I updated steam_update_ctools (used to update the compatibility tools in bulk from the Games List) and steam_update_ctool (used to update games' ctools with Batch Update). These two util functions now have nested functions inside of them that manipulate the config_vdf_dict. These nested functions can even use the passed data to the parent function! For example, steam_update_ctools gets passed a games list. But in order to properly update config_vdf_dict we need data about this game list. A nested function inside of steam_update_ctools can use the games list parameter as well as its own parameter that will get passed to this nested function when it is called by update_vdf_text_data.

This is really cool, because it means the nested function can still act the same way as the old code and still have access to the games list parameter, but doesn't have to be handle the VDF dict itself, it can just get that from whoever calls it! How this works is an absolute mystery to me, it is quite magic.


This is the solution I came up with. Right now it seems like a pretty clean way to break out this functionality , there's a (hopefully) clear illustration on how the data interacts. We have the util function that writes to the VDF files, and can take some Callable that can manipulate the data. That callable and the caller of the util function doesn't have to care about how the VDF files are loaded, it just has to expect the data and care how it wants to manipulate it for its own implementation detail.

As always, all feedback is welcome. Once I have tested some more and once #336 is merged, I will pull this out of draft. But I wanted to work on it while the idea was fresh in my head, and get a PR writeup done now while the rationale and implementation was also fresh 😄

Thanks!

@sonic2kk sonic2kk marked this pull request as draft June 2, 2024 16:47
@sonic2kk
Copy link
Contributor Author

sonic2kk commented Jun 2, 2024

I should've given an example in the OP, but to illustrate more benefits to using this approach, here's how we might use this function in future.

Update localconfig.vdf

In this example we will update localconfig.vdf (located around ~/.local/share/Steam/userdata/{SteamUserID}/config/localconfig.vdf. This file has information on Steam Shortcuts, including a section that controls if a Steam Shortcut should use the Steam Overlay. We may not actually want to implement this in ProtonUp-Qt, this is just one example of how we can use the functionality added by this PR to more cleanly update text-based VDF files.

Before

Here we have to do a bunch of checks on the VDF file path, and manage loading it ourselves. We have to remember how to use the VDF library to load the write to the file, and repeat this throughout the codebase. And we have to wrap everything in a try/except.

def update_steam_shortcut_localconfig(game: SteamApp, updated_game_props: dict, steam_userdata_config_folder: str):
    localconfig_vdf_file = os.path.join(os.path.expanduser(steam_userdata_config_folder), 'localconfig.vdf')
    if not localconfig_vdf_file:
        return False

    try:
        localconfig_vdf_dict = vdf.load(open(localconfig_vdf_file))
        localconfig_ulcs_apps = localconfig_vdf_dict.get('UserLocalConfigStore', {}).get('Apps', {})

        game_id = str(game.app_id)

        if updated_game_props is None:
            return False  # Exit early if no props given to update
        
        # Update existing game props with updated game props
        # Or create new entry if it doesn't exist
        localconfig_ulcs_apps[game_id] = updated_game_props

        vdf.dump(localconfig_vdf_dict, open(localconfig_vdf_file, 'w'), pretty=True)
    except Exception as e:
        print('Error, could not update Steam Shortcuts localconfig:', e, ', vdf:', localconfig_vdf_file)
        return False

    return True

# Example call
userdata_config_dir = '~/.local/share/Steam/userdata/12345678/config/localconfig.vdf'
updated_shortcut_props = { "OverlayAppEnable": "0" }

# Update shortcut in `localconfig.vdf` under "UserLocalConfigStore/Apps" section to have the Steam Overlay disabled
update_steam_shortcut_localconfig(imaginary_SteamApp_object, { "OverlayAppEnable": "0"  }, userdata_config_dir)

After (This PR)

The usage of the VDF library is abstracted away behind update_vdf_text_data, and we only interact with the VDF file using _update_shortcut. This function only needs to expect a dictionary and can interact with it normally as a "regular" Python dictionary, all the details of how the file is used and written out are handled separately.

def update_steam_shortcut_localconfig(game: SteamApp, updated_game_props: dict, steam_userdata_config_folder: str):
    def _update_shortcut(localconfig_vdf_dict: dict):
        localconfig_ulcs_apps = localconfig_vdf_dict.get('UserLocalConfigStore', {}).get('Apps', {})
        game_id = str(game.app_id)

        if updated_game_props is None:
            return False  # Exit early if no props given to update
        
        # Update existing game props with updated game props
        # Or create new entry if it doesn't exist
        localconfig_ulcs_apps[game_id] = updated_game_props

    # Note: We could have a function to fetch this, defining it here is just for illustration purposes
    steam_userdata_localconfig_vdf_path = os.path.join(os.path.expanduser(steam_userdata_config_folder), 'localconfig.vdf')
    
    # Call update_vdf_text_data to load the VDF file at the path.
    # It will use _update_shortcut to modify the VDF file, and then write the result for us
    success = update_vdf_text_data(steam_userdata_localconfig_vdf_path, _update_shortcut)
    if not success:
        print('Error, could not update localconfig.vdf')
    return success

Using the vdf_file_action parameter for update_vdf_text_data makes the function flexible as well, as we aren't limited to just nested functions in whatever parent function is calling update_vdf_text_data. We could make separate util functions in future if we wanted that we can just pass. For example:

def some_common_vdf_action:
    # ...

def do_vdf_action:
    update_vdf_text_data(vdf_file_path, some_common_vdf_action)

At least in my opinion this approach is flexible and more maintainable in future, it makes interacting with text VDF files a bit more straightforward. When writing a function that will touch one, you just have to know how you want to interact with it, and then update_vdf_text_data will take care of the rest.

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.

1 participant