cozyGalvinism / reshader

A tool and library for installing ReShade on Linux!
GNU Lesser General Public License v3.0
8 stars 1 forks source link

Great work, small issues to track or ignore #1

Closed ultimatespirit closed 1 year ago

ultimatespirit commented 1 year ago

Hey,

Great work on this and thanks for making it. Though I have not myself used it as I swapped over manually unzipping the exe and what not few days back, I did read through the code and liked it + I always appreciate people willing to make helper tools for Linux / share their knowledge.

With that in mind, wanted to bring up some small suggestions for the todo list, though I'm ignoring more code cleanup style things I'm sure you're already handling (like the repeated error handling logic areas that could probably be refactored etc.). Obviously feel free to ignore any or all of them, I've ordered them roughly from smallest to largest changes.

That's more or less all that comes to mind at the moment, thanks for reading.

cozyGalvinism commented 1 year ago

Heya, thank you for your input c: The program is still work in progress so there is a bunch of unhandled errors (pretty much 90% of it is still unhandled). With that said, let me touch on your feedback:

You probably don't want to assume xdg-open exists / fail if it doesn't exist outright in the presets install section.

Yeah, I don't feel comfortable depending on other software to be present, though unfortunately there isn't much to be done about it... As far as I'm aware, Linux doesn't open the default browser when trying to execute an URL as opposed to that being the case on Windows, which pretty much only leaves XDG as a source. Obviously this fails for any distro that doesn't conform to XDG or doesn't have xdg-utils installed.

Though displaying the URL would work fine here, I agree.

Since the presets are being symlink'd, perhaps also symlink dxgi.dll and d3dcompiler_47.dll after storing them in the data folder. Saves on re-downloading them all across multiple installs, not to mention deduplication between multiple installs. By the way, thank you for respecting the XDG directory specification / using libraries that respect it.

I didn't think of that, good catch. Symlinking the libraries is how GShade did it too, not sure why it didn't come to mind...

It would also be nice to be able to process an already provided ReShade installer / alternative resource acquisition paths.

Definitely agreed. I think a CLI parameter would work best here, especially with the next idea.

More advanced though similarly helpful version of the previous suggestion would be proper command line arguments to be able to process in one go instead of relying on multiple input sections.

Yup. Not much to add here, eventually you should be able to run this with just parameters to skip all the user inputs. This will be interesting for the GShade presets and shader downloads, as the license of those state that you can't download those automatically. So it basically comes down to the user to either provide both zips (in one way or another) and then run reshader to symlink away.

Input sections using readline could be cool too, or similar modern equivalents if Rust has any, not too familiar with readline-type stuff for Rust.

The input sections at the moment are powered by inquire, which makes the CLI pretty organized and straightforward in my opinion. I also wouldn't be opposed to using something else, inquire just seemed like a pretty modern library and looked pretty good as well c:


So to summarize, I will get some TODOs here:

These shouldn't be too hard to implement either, I might get around to fix error handling and implement these features this weekend.

Thank you again for your input! Greatly appreciated!

ultimatespirit commented 1 year ago

Thanks! And re: inquire, that seems cool! I honestly missed that entirely when reading the code just figured they were straight reads of stdin for now (I know that's exactly the sort of thing I'd do for a quick hacky first draft to get things working haha).

With regards to the license, while I agree with the spirit of not touching that mess, I would add that.. it's highly unlikely that license is enforceable, like, at all. I'm not sure I've ever seen a license of "no automatically hitting this public endpoint" manage to hold any water, not to mention the legal mess of proving what is or is not automatic (It definitely feels like you could argue any computer action is not entirely manual and therefore automatic access...). In the context of what led to the GShade debacle it really feels like those additions were solely to screw with the original helper tool. Guess we'll find out what gposers do with that, or if better community resources come about for the shaders/presets.

I'll be sure to swing any Linux folks I know who play over to this tool, rather than my very jank quick and dirty shell script I threw together day of haha. Anyway, thanks for working on this and all the best!