-
-
Notifications
You must be signed in to change notification settings - Fork 41
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: Check if Steam configuration files exist before marking as available #356
Merged
DavidoTek
merged 6 commits into
DavidoTek:main
from
sonic2kk:more-robust-valid-install-check
Mar 24, 2024
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
19c3dcd
util: Check if Steam configuration files exist before marking as avai…
sonic2kk d0e05ce
constants: Move valid Steam install check to is_valid_launcher_instal…
sonic2kk 6423f52
constants: Use os.path.join for building Steam install location paths
sonic2kk af05168
constants: Don't use os.path.join when building Steam POSSIBLE_INSTAL…
sonic2kk ed709b9
comment cleanup
sonic2kk 64ec6d0
comment cleanup 2 electric boogaloo
sonic2kk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
from pupgui2.constants import AWACY_GAME_LIST_URL, LOCAL_AWACY_GAME_LIST | ||
from pupgui2.constants import GITHUB_API, GITLAB_API, GITLAB_API_RATELIMIT_TEXT | ||
from pupgui2.datastructures import BasicCompatTool, CTType, Launcher, SteamApp, LutrisGame, HeroicGame | ||
from pupgui2.steamutil import remove_steamtinkerlaunch | ||
from pupgui2.steamutil import remove_steamtinkerlaunch, is_valid_steam_install | ||
|
||
|
||
def create_msgbox( | ||
|
@@ -196,6 +196,25 @@ def create_compatibilitytools_folder() -> None: | |
print(f'Error trying to create compatibility tools folder {str(install_dir)}: {str(e)}') | ||
|
||
|
||
def is_valid_launcher_installation(loc) -> bool: | ||
|
||
""" | ||
Check whether a launcher installation is actually valid based on per-launcher criteria | ||
Return Type: bool | ||
""" | ||
|
||
install_dir = os.path.expanduser(loc['install_dir']) | ||
|
||
# Right now we only check to make sure regular Steam (not Flatpak or Snap) has config.vdf and libraryfolders.vdf | ||
# because Steam can leave behind its directory structure when uninstalled, but not these files. | ||
# | ||
# In future we could expand this to other Steam flavours and other launchers. | ||
if loc['display_name'] == 'Steam': # This seems to get called many times, why? | ||
return is_valid_steam_install(os.path.realpath(os.path.join(install_dir, '..'))) | ||
|
||
return os.path.exists(install_dir) # Default to path check for all other launchers | ||
|
||
|
||
def available_install_directories() -> List[str]: | ||
""" | ||
List available install directories | ||
|
@@ -204,10 +223,12 @@ def available_install_directories() -> List[str]: | |
available_dirs = [] | ||
for loc in POSSIBLE_INSTALL_LOCATIONS: | ||
install_dir = os.path.expanduser(loc['install_dir']) | ||
if os.path.exists(install_dir): | ||
# POSSIBLE_INSTALL_LOCATIONS may contain duplicate paths, so only add unique paths to available_dirs | ||
# This avoids adding symlinked Steam paths | ||
if is_valid_launcher_installation(loc) and not install_dir in available_dirs: | ||
available_dirs.append(install_dir) | ||
install_dir = config_custom_install_location().get('install_dir') | ||
if install_dir and os.path.exists(install_dir): | ||
if install_dir and os.path.exists(install_dir) and not install_dir in available_dirs: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we could use |
||
available_dirs.append(install_dir) | ||
return available_dirs | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering: With this check
not install_dir in available_dirs
in place, do we still needlist(dict.fromkeys(_POSSIBLE_STEAM_ROOTS))
inconstants.py
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In short, no, but it should help catch any case where we might end up with duplicate paths in
POSSIBLE_INSTALL_LOCATIONS
.Most probably not for functionality at least for Steam, but for consistency,
list(dict.fromkeys(_POSSIBLE_STEAM_ROOTS))
(and the earlier checks to remove non-existent paths from_POSSIBLE_STEAM_ROOTS
) should ensure thatPOSSIBLE_INSTALL_LOCATIONS
will only contains Steam install paths that exist on the filesystem. Whether or not they're valid is another question, and that's what our function handles here.The check might also be good in case other install paths in future could have duplicates. This probably won't ever happen, but it just makes sure at this level that we don't list the same path twice. If we really wanted it to be stricter, we could make sure all paths in
available_dirs
andinstall_dir
itself are always expanded to theirrealpath
, and also we could filter out duplicates by returninglist(dict.fromkeys(available_dirs))
in this function, but I think that's overkill.For the changes in this PR, the
not install_dir in available_dirs
condition is not strictly needed, it's just an extra guard.Also, I guess the comment here is wrong, we don't list duplicate paths.