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

Clean up ThunderstoreDownloaderProvider #1422

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

anttimaki
Copy link
Collaborator

Remove references to settings and active game from the download provider, as these were mostly used to only access the setting telling whether to ignore the cached mod downloads. Instead pass the settings as a boolean argument.

Implement related changes to provider implementation and their callsites.

In downloadAndSave, switch to checking the variable for ignoring the cache first, before checking if the mod actually exists on the disk, as this might reduce the number of unnecessary file operations.

Copy link
Collaborator

@VilppeRiskidev VilppeRiskidev left a comment

Choose a reason for hiding this comment

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

LGTM.

Remove references to settings and active game from the download
provider, as these were mostly used to only access the setting telling
whether to ignore the cached mod downloads. Instead pass the settings
as a boolean argument.

Implement related changes to provider implementation and their
callsites.

In downloadAndSave, switch to checking the variable for ignoring the
cache first, before checking if the mod actually exists on the disk,
as this might reduce the number of unnecessary file operations.
@anttimaki anttimaki merged commit 431df54 into develop Sep 11, 2024
5 of 7 checks passed
@anttimaki anttimaki deleted the download-provider-cleanup branch September 11, 2024 06:19
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