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

WIP: Add option to install ReShade shaders #3

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

cozyGalvinism
Copy link
Owner

Installing ReShade also installs a ReShade.ini now

Installing ReShade also installs a ReShade.ini now
@cozyGalvinism cozyGalvinism linked an issue Feb 15, 2023 that may be closed by this pull request
@cozyGalvinism cozyGalvinism merged commit 445ab29 into develop Feb 15, 2023
@cozyGalvinism cozyGalvinism deleted the install-reshade-shaders branch February 15, 2023 13:03
Copy link

@ultimatespirit ultimatespirit left a comment

Choose a reason for hiding this comment

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

I think this will end up as a review on this merged pull rather than an Issue, but really what I've brought up are Issues at this point... I'll copy them over to an issue if needed.


if repo_texture_directory.exists() {
let builder = CopyBuilder::new(&repo_texture_directory, &texture_directory);
builder.overwrite(true).run()?;

Choose a reason for hiding this comment

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

We run into an ordering issue here between crosire/reshade-shaders and CeeJayDK/SweetFX, where if one repo had overlapping files that were not exactly the same then the order in which we add them matters. It stands to reason that it's likely for crosire/reshade-shaders to be the base directory over which things get overwritten, if needed. Currently I believe the ordering of repos in SHADER_REPOSITORIES makes it so SweetFX would get overwritten.

And indeed, if you run the following (assuming all pulled repositories are in the CWD at start)....

$ cd SweetFX
$ shasum Shaders/* > ../SweetFX-Shaders-SHAsums.txt
$ cd ../reshade-shaders
$ shasum -qc ../SweetFX-Shaders-SHAsums.txt --ignore-missing
Shaders/FXAA.fx: FAILED
Shaders/Nostalgia.fx: FAILED
Shaders/SMAA.fx: FAILED
shasum: WARNING: 3 computed checksums did NOT match

So at time of writing three files in SweetFX that are also in crosire/reshade-shaders on the master branch do not match.

I checked the SHAs versus a copy of reshade-shaders from a fresh install on Windows that a friend provided me and SweetFX shaders match perfectly, whereas crosire/reshade-shaders did not match for 6 files (the 3 noted above that conflict between SweetFX and itself, as well as 3 additional files that are newer in the fresh install than what appears to be currently on master in crosire/reshade-shaders. These files appear to be from the slim branch.).

For what it's worth, there is no overlap currently between SweetFX and the slim branch, and it seems none of the files on master that are not in slim are in the fresh install. So I think this may be a case of the infrastructure having changed since the time of the shell script my original suggestion was based off of. Indeed that script actually didn't even make the subdirectories currently seen, it just shoved everything into one flat Shaders directory, though it did not overwrite anything.

Tl;dr it seems we need to use the slim branch to conform most closely to what a pristine installation does, and regardless we want crosire/reshade-shaders to go first / get overwritten when a conflict emerges. If we did want to ship the extra shaders master has over slim it would require some finangling since slim is definitely newer than master is currently.

Also having said all that... it seems this file is probably our canonical information sheet on what to install and how: https://github.com/crosire/reshade-shaders/blob/list/EffectPackages.ini

From the list branch of crosire/reshade-shaders.

So... that's an oops on my part sorry.

ForceWindowed=0

[GENERAL]
EffectSearchPaths=.\reshade-shaders\Shaders,.\reshade-shaders\Shaders\qUINT,.\reshade-shaders\Shaders\PD80,.\reshade-shaders\Shaders\AstrayFX,.\gshade-shaders\Shaders,.\gshade-shaders\ComputeShaders

Choose a reason for hiding this comment

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

We may not want to by default allow both reshade-shaders and gshade-shaders. If someone ends up with both directories in their area for whatever reason shaders will double load which is bad. Same story for textures.

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.

Add option to install ReShade shaders
2 participants