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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lutris: Add some util functions for conditional game list parsing #364

Merged
merged 1 commit into from
Mar 24, 2024

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Mar 9, 2024

Some readability improvements that may help extending these checks later for future work, hopefully not too premature! 馃槃

When I was digging into #363 I noticed that some of the conditional logic was a bit long and seemingly arbitrary. To help with readability I made some generic utility functions that should explain better than the checks we were doing before.

For #363 we may need to extend the conditional logic i.e. in pupgui2ctinfodialog.py, we may need to add a check for whether a given runtime is in use. This would get pretty long, but with this format, we could make is_lutris_game_using_runtime('dxvk', game.dxvk_version) (or something along these lines, idk how we would want to do this just yet, and not too important for this PR :-) ).

The first function added is to check what runner a LutrisGame is using, with not None and length sanity checks. We were already doing this for the Games List

(

self.games = list(filter(lambda lutris_game: (lutris_game.runner is not None and lutris_game.runner != 'steam' and len(lutris_game.runner) > 0), get_lutris_game_list(self.install_loc)))
)

But this function now means anytime we check the runner name, we perform these checks.

The 2nd function is more specific, and it checks if a LutrisGame is using Wine as its runner, with an optional parameter to check if it's using a specific Wine version. I figured there's no harm in making this function a bit more flexible as it was pretty straightforward to extend.

This could make our checks a lot cleaner should they be extended further. At least in my opinion, if is_lutris_game_using_wine(game, self.ctool.displayname) or is_lutris_game_using_runtime(game, ...) is a lot cleaner than extending our existing check.

@sonic2kk sonic2kk force-pushed the lutris-util-funcs-readability branch from 6d09a41 to da5113c Compare March 9, 2024 03:46
@sonic2kk
Copy link
Contributor Author

sonic2kk commented Mar 9, 2024

There are some typing issues in the ctinfo dialog file, but I think we can clean this up as part of a general sweep of typing changes, once we have Python 3.10 (#359).

@sonic2kk
Copy link
Contributor Author

sonic2kk commented Mar 9, 2024

I'm wondering if these would be better as methods on the LutrisGame class itself, since we have to pass it a LutrisGame anyway...

@DavidoTek
Copy link
Owner

Thanks! Looks good.

Makes sense to create utility functions for that, especially if we also need is_lutris_game_using_runtime('dxvk', game.dxvk_version) later.

I noticed that the sanity checks also catch situations where the game.runner is None. Can't remember if that is the case anywhere. If yes, we might want to add a type hint like runner: Union[str, None] to datastructures.py#LutrisGame in the future.

The 2nd function is more specific, and it checks if a LutrisGame is using Wine as its runner, with an optional parameter to check if it's using a specific Wine version. I figured there's no harm in making this function a bit more flexible as it was pretty straightforward to extend.
This could make our checks a lot cleaner should they be extended further. At least in my opinion, if is_lutris_game_using_wine(game, self.ctool.displayname) or is_lutris_game_using_runtime(game, ...) is a lot cleaner than extending our existing check.

Yes, that's definitively worth adding the utility functions for making the other code cleaner.

(I just noticed that these functions are a great example for something that could use unit tests #347. Haven't gotten around to look into that yet...)

I'm wondering if these would be better as methods on the LutrisGame class itself, since we have to pass it a LutrisGame anyway...

I feel like that it is fine as you did for now. I wonder if at some point we might actually want to move the data structures into the launcher specific files (i.e., move LutrisGame to lutrisutil.py). We could then move the functions into LutrisGame and just rename the game parameter into self.

@DavidoTek DavidoTek merged commit 059c4e0 into DavidoTek:main Mar 24, 2024
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.

None yet

2 participants