SteamTinkerLaunch: Type hinting and refactoring improvements #448
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.
(Note: This PR is opened as a draft because I have not yet had a chance to extensively test the changes.)
I've been meaning to take a look at the SteamTinkerLaunch ctmod for a while, as it could be brought up-to-date with the other ctmods to use the new utility functions like the fetching project data functions and even the new
download_file
utility function. Also, the ctmod was written a while ago and not very many changes have been made, and I think there are some logic improvements that could be made (for example,get_tool
is huge and could really be split out into separate methods). However before we get that far there are some higher level improvements that I wanted to make, mainly around better type hinting and refactoring to make the ctmod a bit nicer to work with.This PR adds various type hints around the ctmod (this will help when other functions and methods are more thoroughly type-hinted) and does a bit of refactoring to reduce indentation and make code paths cleaner. Namely:
is_system_compatible
if we're on SteamOS, as we don't check dependencies at all on SteamOS.get_tool
from the tarfile context manager, as we don't need to be indented so much in that block.tarfile.open
entirely in favour of our helper methods to handle extraction, but I didn't do that in this PRcontinue
in at least one loop to continue if a condition is met instead of keeping the logic in an indented block.Generally in this PR I only made surface-level comfort changes to make the ctmod a bit less "scary". In my head I have a mental image of this ctmod being a bit on the "unclean" side and getting back into this part of the code, it isn't nearly as much of a disaster as I pictured it in my head, but I see a lot of potential for improvement and I want to split it across multiple PRs sequentially. I want to get some basic cleanup in place with this PR, and then move onto splitting parts of the code up a bit better, and then moving onto using our helper methods such as `download_file. All of these steps should help "modernize" this ctmod and make it a lot less "dated". I know a lot more than I did 2 years ago when I initially stepped up to work on this and want to give back by bringing this ctmod up to speed with the other improvements made in the codebase since that time.
If you have any other suggestions for changes I should include in this PR, please let me know and I'll be happy to include them! But again this PR is mainly for those "higher level" changes, like removing unnecessary indentation, returning early, and improving readibility. I understand bigger changes are probably desired but I didn't want to make some monster PR. I think this PR might already be a bit difficult to review with the number of changes despite my best efforts to keep it on the smaller side 😅
All feedback is welcome on this. Thanks!