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

Fix some Modrinth mrpack import issues #771

Merged
merged 6 commits into from
Jun 12, 2022

Conversation

flowln
Copy link
Contributor

@flowln flowln commented Jun 9, 2022

Closes #766 (but CC @DioEgizio test this to be sure, please :p)

Actually, most modpacks weren't working because we were considering the env field to be required, but it's optional x.x

The download array is now properly parsed as well, though trying to use the secondary downloads in case the first fails is :pofat: tech ™️, since we don't yet have the proper task mechanisms to do it right (those are being worked on ;) )

This removes the whitelisted hosts check, since people didn't like it and it was causing some issues trying to integrate with multiple download urls anyways

Lastly, this fixes an issue where client overrides where not taken into consideration when importing mrpacks!

It's optional, so some files may not have it (like most of FO).
Kinda, it's ugly and hackish, since we don't have the facilities to
do this properly (yet!)
people didn't seem to like it, and its not required
@flowln flowln added the bug Something isn't working label Jun 9, 2022
@flowln flowln added this to the 1.3.2 milestone Jun 9, 2022
@lgtm-com
Copy link

lgtm-com bot commented Jun 10, 2022

This pull request introduces 1 alert when merging b3c8f9d into 309013e - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@txtsd txtsd self-requested a review June 10, 2022 00:52
@DioEgizio
Copy link
Contributor

seems to work now!

launcher/InstanceImportTask.cpp Show resolved Hide resolved
launcher/InstanceImportTask.cpp Outdated Show resolved Hide resolved
Co-authored-by: Sefa Eyeoglu <contact@scrumplex.net>
Co-authored-by: Sefa Eyeoglu <contact@scrumplex.net>
@lgtm-com
Copy link

lgtm-com bot commented Jun 11, 2022

This pull request introduces 1 alert when merging 37160f9 into 9bbf50e - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@Scrumplex Scrumplex merged commit c4f2e3a into PolyMC:develop Jun 12, 2022
@flowln flowln deleted the modrinth_multiple_downloads branch July 22, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mrpacks with more than one download url in the array don't import correctly
5 participants