DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.18k stars 39 forks source link

vkd3d: Make vkd3d-lutris inherit from vkd3d-proton #262

Closed sonic2kk closed 12 months ago

sonic2kk commented 1 year ago

vkd3d-lutris now inherits from the vkd3d-proton ctmod, changing only the CT_URL and CT_INFO_URL to point to the right location. This essentially removes all logic from vkd3d-lutris and the only thing it does is set its own CT_NAME etc, and its own URLs. inside the class, so that other method calls inside this class point to the correct vkd3d-lutris URLs. This is very similar to how Proton-tkg ctmods are implemented.

Implementation

Archive Fetching

I'll be referencing the Proton-tkg a bit here, because it's got a similar approach.

vkd3d-proton has been updated to allow extracting both .tar.zst archives (vkd3d-proton) and .tar.xz archives (vkd3d-lutris). The check in __fetch_github_data also had to be updated, it was checking if the asset name ended with .tar.zst so I changed it to check for both archive formats. If the archive format were ever to change (which I think is unlikely), this check could be changed to check if vkd3d in asset['name'].

This is actually what Proton-tkg does currently (https://github.com/DavidoTek/ProtonUp-Qt/blob/main/pupgui2/resources/ctmods/ctmod_protontkg.py#L130). There could be issues with doing this, such as if a checksum was ever added or anything like that, but the same could be said for Proton-tkg and that implementation is working fine, so either approach would work here too I think (especially since vkd3d releases only have one asset aside from the source code archives: vkd3d-proton releases and vkd3d-lutris releases).

Archive Extraction

Though they use different formats, both vkd3d archives have the same file structure - we just needed to use a different extraction method. Speaking of...

Since #250 was merged it was very straightforward to add the conditional logic for which archive to extract. In fact, I had a little fun with this part. For Proton-tkg, since that ctmod accounts for various different "flavours", to know how to extract each archive we check what it endswith. That way we know which function to call. Both endswith and the extraction methods like extract_tar_zst return booleans, so I was able to do a neat little trick here:

has_extract_tar_zst = vkd3d_archive.endswith('.tar.zst') and extract_tar_zst(vkd3d_archive, vkd3d_dir)
has_extract_tar_xz = vkd3d_archive.endswith('.tar.xz') and extract_tar(vkd3d_archive, vkd3d_dir, mode='xz')

If the endswith check is true on the left, the extract_ on the right will not be executed when anding. If the endswith returns False, it won't try to call the extract function, which should make for a safe extraction check. And since both of these functions return a boolean, we can still do the return False part for failed archive extraction.

Really, I just thought this was a fun way to go about it, but if you'd prefer a more explicit check (along the lines of doing both of these checks in one if, or in a nested if, or two separate ifs) that's fine too :smile:

ctmod filename

I did consider renaming the ctmod filename, but I decided against it. The ctmod name is still vkd3d-proton by default, and also we didn't rename the Proton-tkg ctmod to anything like ctmod_tkg, so I figure keeping the filename the same is fine :-)

Concerns

It isn't impossible that these archive formats would change, but even if the ctmods were kept split, we would still need to update the logic to account for that case anyway, so I don't think that's an issue specific to using an implementation like this. For example if vkd3d-lutris updated to using .tar.gz instead, we'd need to check both formats for backwards compatibility in vkd3d-lutris. The same would be true for this approach (though which one is cleaner could be debated).

This concern would also apply to any ctmod where the archive format could change, or where we deal with multiple archive formats (such as with Proton-tkg).


This change came about mainly because I was looking to add vkd3d to Heroic, and it caught my eye how similar the ctmods were. So I figured an attempt at a refactor here might be in order before looking to add Heroic support (the main change will be adding a get_extract_dir for vkd3d). I also think this approach is slightly cleaner, since it removes the heavily duplicated code. There was a lot of overlap between the ctmods with the main differences being the asset name check, and the extraction logic, both of which were pretty straightforward to add checks for in this ctmod.

Thanks! :-)

DavidoTek commented 12 months ago

Thanks :tada: