-
Notifications
You must be signed in to change notification settings - Fork 124
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
Initial native code for verified mod auto-downloading #545
Conversation
…ady-existing directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks good to me at this point, my previous review comment about maybe making the mods list a convar was more a suggestion than a requirement, so don't consider that blocking
I don't rly know about the hash stuff so dont take my word as gospel about that being good
So this bug happens when using the Now that most of the development of this feature is done, I feel like it might not be required: even if as a developer you wanna bypass checksum comparison, you still have to create an entry for the to-be-downloaded mod in the local verified mods list (to link a mod name to a Thunderstore repository). Thoughts? |
IMO, should fall back to interpreting the mod name as is. |
I feel like this point goes beyond the flag removal discussion, so I fixed in anyway in 7548c47. |
This method does not need a mod name as argument, as mods are downloaded one by one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna give the code one final read again tomorrow but with how many code reviews we have on it already I doubt I'm gonna find anything any more :P So fingers crossed for merge tomorrow 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aight, so in my own review I only found issues that could be considered to be bike-shedding and so I will move them to separate GitHub issues post merge.
Ofc my C++ knowledge is not that great so I could've missed something so I'm mostly relying on previous reviews made by others as well.
Description
This branch allows client to download a mod archive from the Thunderstore API, and extract included mods in the remote mods folder of the current game profile.
Not all mods can be automatically downloaded, as it would cause some security issues, and Thunderstore mod name cannot be deduced from actual mod name: to be eligible to auto-downloading, a mod must appear in the list of verified mods.
Said list and complete mod verification procedure are described here: https://github.com/R2Northstar/VerifiedMods
This branch exposes two commands to test the feature:
fetch_verified_mods
retrieves verified mods list from the GitHub organization, and stores it locally;download_mod
does the actual mod downloading/extraction job.What this PR is about
What this PR is NOT about
reload_mods
invoked)Resources
How to test
fetch_verified_mods
command;download_mod [mod_name] [mod_version]
(onlyFifty.mp_frostbite 0.0.1
is listed at the time of writing);Media
autodl-pr-opening.webm