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

d8vk: new verb #2045

Closed
wants to merge 9 commits into from
Closed

d8vk: new verb #2045

wants to merge 9 commits into from

Conversation

sonic2kk
Copy link

Adds d8vk verb as requested in #2004.

Tested with Touhou 6 via Lutris, MangoHud showed the renderer changed from WINED3D to DXVK. The selection of D3D8 games I can test with is somewhat limited it seems so further testing is of course welcome.

This PR touches helper_dxvk, I was specifically concerned with the changes to the tar command which now uses --one-top-level, so I tested installing regular dxvk and it seemed to work fine. This extra flag is required because DXVK archives have a root folder in the archive, but D8VK does not.

One outstanding issue is with the URL in helper_dxvk that points to the release notes; this URL is incorrect for D8VK. This is because DXVK release tags are just the version, e.g. v2.1, but D8VK is d8vk-vX.X. The _W_package_version needs to be set differently and I wasn't sure of the best approach to take with this.

First time contribution and first time even looking at Winetricks code so any and all feedback is very welcome :-)

Thanks!

src/winetricks Outdated Show resolved Hide resolved
@sonic2kk
Copy link
Author

sonic2kk commented Apr 5, 2023

I think the checks should pass now

@sonic2kk
Copy link
Author

sonic2kk commented Apr 7, 2023

That was embarrassing, sorry... Guess I changed something since the last time I ran shellcheck or something.

Passes on my machine now, gave the same warning that the CI did so I guess it's fixed 🙂

src/winetricks Outdated
@@ -6912,14 +6912,14 @@ helper_dxvk()
w_try_unzip "${W_TMP}" "${W_CACHE}/${W_PACKAGE}/${_W_package_archive}"
else
w_try_cd "${W_TMP}"
w_try tar -zxf "${W_CACHE}/${W_PACKAGE}/${_W_package_archive}"
w_try tar -zxf "${W_CACHE}/${W_PACKAGE}/${_W_package_archive}" --one-top-level
Copy link
Contributor

Choose a reason for hiding this comment

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

--one-top-level is a GNU extension; this will break on BSD tar.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! After some light research it looks like -C might work for BSD tar but I haven't tested just yet.

Copy link
Author

Choose a reason for hiding this comment

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

Updated to use -C "${_W_package_dir}", Winetricks still fetches and installs d8vk correctly (tested with no cached archive and with a cached archive), only tested Touhou 6 at the moment and it still works too.

@sonic2kk
Copy link
Author

sonic2kk commented Apr 9, 2023

Since merging master the diff seems like it's gotten pretty big? It mentions removing helper_dxvk_nvapi which I saw when resolving the conflicts, but I just chose to accept the incoming changes which evidently removed it. This function is also not present on master it looks like from a Ctrl+F.

Not sure what went wrong there 😦

@austin987
Copy link
Contributor

There was a merge (dxvk_nvapi) that conflicted, so it needs a manual merge.

I did some initial fixes (austin987@01c9188), but didn't tested it with a real application.

Note that the release notes are still broken (should be fixed). Also, this will always point to the latest dxvk release; you probably want to add at least one real release so users have a stable option they can use.

@sonic2kk
Copy link
Author

Thanks, this is my first time resolving a conflict like this so your fork reference was helpful 😄

Attempted to resolve the conflicts, have not yet tested the changes. I will update once I have tested, thanks for the patience on this PR 🙂

@sonic2kk
Copy link
Author

sonic2kk commented Apr 11, 2023

The change with 340d84e seemed to cause issues with D8VK. It would not extract without w_try_mkdir "${_W_package_dir}" (d8vk doesn't have a top-level folder in the archive), and extracted to the wrong directory without the cd (extracted to current working directory).

To get this working for testing, I reverted the change in that commit, but I don't think this is a true solution. So I'm open to feedback on how to best go about resolving this properly.


Shellcheck isn't flagging anything, and grep -n -r [[:blank:]]$ src/winetricks doesn't return anything, so looks good from that angle.

Next going to implement the feedback on always pointing to one fixed release of D8VK. Looking at the Releases page there are currently two, but it seems the first should be avoided as it was a test release. So I will point to v0.10 as the fixed release, which is currently the same as the latest release, but in future when a new version comes out, the current load_d8vk should point to whatever the next latest release is 🙂

@sonic2kk
Copy link
Author

sonic2kk commented Apr 11, 2023

Just thought to test with DXVK_HUD (since I noticed the D8VK release notes showed that) and for some reason it seems to actually be using DXVK. I have no idea how or why, since I can see it's downloading the D8VK archive. Manually installing does indeed show d8vk, so there is an issue with the install.

Marking this PR as draft until I can figure out why this is...

Not sure what happened but it seems fine now... I didn't change anything in the code, I just cleared the cached d8vk files (which were d8vk archives). DXVK_HUD won't show unless D[]VK is being used, and I have verified that the hud doesn't show unless D8VK is installed. So... I am not sure, maybe I just messed something up in my tests.

image


For some reason, D8VK doesn't seem to play nice with the Steam games I am testing (Morrowind and Postal 2). When installed via Winetricks or manually, Morrowind says it encountered a problem, and Postal 2 crashes silently (and replaces with d3d9.dll each time).

Not sure what's up here, but Winetricks is downloading D8VK, copying them into the prefix, and setting the DLL overrides. And it appears to work for the one Non-Steam Game I can test right now, not sure if I have others on GOG that might work just yet.

EDIT: Tested Syberia II from GOG as it's on the D8VK list of games that were reported to work. Game crashed when installing D8VK manually and with Winetricks, so probably nothing is wrong with the code to install D8VK into the prefix here. But still open to further testing and review.

@sonic2kk sonic2kk marked this pull request as draft April 11, 2023 15:53
@sonic2kk sonic2kk marked this pull request as ready for review April 11, 2023 16:19
@sonic2kk
Copy link
Author

Not sure why this is only working for some games and crashing others. Maybe the supported games list only refers to specific versions of games. Touhou 6 works for me, but Syberia II does not. The D3D9 DLL gets overwritten for all the games it looks like, but Touhou 6 works still. I tested different Wine-GE versions and Lutris Wine 7.2 and d8vk consistently installed properly.

I noticed the install script will move the DLLs into the Proton lib folders for Steam games as well as into the games prefix, but as far as I can tell this is not something Winetricks does at all, so I don't think it'a step required here.

Maybe that limits the usefulness of including d8vk in Winetricks. I am not sure what would be going wrong with the code, as it mostly appears to mirror how DXVK is installed, and D8VK works for at least one game.

@sonic2kk sonic2kk closed this Aug 27, 2023
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