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

NorthStar Proton: Simplify by inheriting GE-Proton Ctmod #491

Merged
merged 2 commits into from
Mar 3, 2025

Conversation

sonic2kk
Copy link
Contributor

@sonic2kk sonic2kk commented Mar 2, 2025

This PR simplifies the NorthStar Proton Ctmod by subclassing the GE-Proton ctmod, removing a lot of overlapping code.

As part of some cleanup to achieve this, the GE-Proton Ctmod was refactored slightly to hold a release_format. NorthStar Proton and GE-Proton both use .tar.gz formats, but in future I hope to allow more Ctmods to inherit from GE-Proton. Since I was here I figured it would be good housekeeping to implement it now that we have a couple of Ctmods taking from GE-Proton :-)

I tested both GE-Proton and NorthStar Proton and they should still be downloading correctly.

image


Let me know if there are any concerns with this, or if I have missed something. Thanks!

sonic2kk added 2 commits March 2, 2025 23:33
Allows us to specify the archive format for tools that subclass GE-Proton.
Could be replaced in future with a specific check that then chooses
which extract util function to use.
@sonic2kk sonic2kk changed the title GE-Proton: Use release_format NorthStar Proton: Simplify by inheriting GE-Proton Ctmod Mar 2, 2025
@sonic2kk
Copy link
Contributor Author

sonic2kk commented Mar 3, 2025

I wonder if instead of specifying release_format, would it be better to have an self.__extract_tool method that child classes would be expected to override.

@@ -159,7 +161,7 @@ def get_tool(self, version, install_dir, temp_dir):
if source_checksum and (download_checksum not in source_checksum):
return False

if not extract_tar(proton_tar, install_dir, mode='gz'):
if not extract_tar(proton_tar, install_dir, mode=self.release_format.split('.')[-1]):
Copy link
Owner

Choose a reason for hiding this comment

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

This may need some modifications in case we get more compatibility tools that use .zip like Proton-Tkg or some other non-tar archive.
We can change that once we inherit a tool that needs it.

@DavidoTek
Copy link
Owner

I wonder if instead of specifying release_format, would it be better to have an self.__extract_tool method that child classes would be expected to override.

I think the release_format variable is fine. The variable makes it obvious what format the tool uses. Adding a reference to some function wouldn't be that clean.

@DavidoTek DavidoTek merged commit f3cde0e into DavidoTek:main Mar 3, 2025
1 check passed
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.

2 participants