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

Update and fix cnc-ddraw #2199

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Root-Core
Copy link

The current implementation fails, if ddraw.dll is a symlink - like in Proton-GE.

The current implementation fails, if ddraw.dll is a symlink - like in Proton-GE.
src/winetricks Outdated Show resolved Hide resolved
src/winetricks Outdated

w_download https://github.com/FunkyFr3sh/cnc-ddraw/releases/download/v6.2.0.0/cnc-ddraw.zip e5677ba52c31ffa93421a16edacff0c4d1f03e107aea6fc860861b43e3356119 cnc-ddraw-v6.2.0.0.zip
w_try_unzip "${W_SYSTEM32_DLLS}" "${W_CACHE}/${W_PACKAGE}/${file1}"
w_download https://github.com/FunkyFr3sh/cnc-ddraw/releases/download/v6.3.0.0/cnc-ddraw.zip c024d0ea42ec2d9708dc0a19342d037de7307c291dc43c01948a7d8f06b4deca ${file1}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't use ${file1} here it will fail spellcheck that's why I'd not been using it already.

Copy link
Author

Choose a reason for hiding this comment

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

I checked, why it fails and src/linkcheck.sh isn't failing, but tests/shell-checks.
This is fixed in #2204.

@Gcenx
Copy link
Contributor

Gcenx commented Mar 22, 2024

If special changes are required for GE-Proton that should really be handled within umu-protonfixes, I've never seen any complaints of simply unpacking the archive caused any Linux users issues so I'll assume this would be Proton specific.

@Root-Core
Copy link
Author

I've never seen any complaints of simply unpacking the archive caused any Linux users issues so I'll assume this would be Proton specific.

Nah. This is why there is the w_try_cp_dll() function in first place and it is used for other dlls.

winetricks/src/winetricks

Lines 748 to 751 in f87bf9e

# Copy $1 into $2. If $2 is found to be a symbolic link, it will be removed first.
# This solve a problem of dlls being symbolic links on some versions or variants of wine.
# We want to replace the symbolic link and not copy into its target.
w_try_cp_dll()

If special changes are required for GE-Proton that should really be handled within umu-protonfixes,

I don't see a single reason it shouldn't be used here.

@Gcenx
Copy link
Contributor

Gcenx commented Mar 23, 2024

Yes using w_try_cp_dll() would indeed be valid, is wine on Linux using symlinks more heavily now for prefixes?

The Linux specific comment was more aimed at the usage of -u but I see you’ve added thumbs up to both my other comments.

@Gcenx
Copy link
Contributor

Gcenx commented Mar 23, 2024

Something that might be worth noting is if something is platform specific (not project specific like Proton) it could always be placed behind a platform guard like I’ve done for the Steam verb for macOS.

@Root-Core
Copy link
Author

Root-Core commented Mar 23, 2024

Yes using w_try_cp_dll() would indeed be valid, is wine on Linux using symlinks more heavily now for prefixes?

I think so. I'm not using wine directly that much though. I know Proton-GE and Lutris are using it.
I also searched for symlinks in prefixes that use the Proton provided by Steam and they make heavy use out of it.
It makes sense to me, as you don't have to duplicate all files and by replacing a symlink with a file - like in this case - only affects a singular prefix.

The Linux specific comment was more aimed at the usage of -u but I see you’ve added thumbs up to both my other comments.

I will fix it soon. I have only little experience with BSD / MacOS, so I missed that.

I thought an update might be a good thing, but we could just delete files in order to update them.. if necessary.
Afaik there isn't really a mechanism to update a fix to a newer version what so ever, so just forget what I just said. :)

Something that might be worth noting is if something is platform specific (not project specific like Proton) it could always be placed behind a platform guard like I’ve done for the Steam verb for macOS.

Good to know, but I don't think that's necessary here. This fix should work on all platforms.

@Root-Core
Copy link
Author

Updated to v6.8.0.0, could we please merge after #2204 is accepted?

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