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

Copy found blocked 3rd party mods to the new instance (ftb and curseforge) #304

Merged
merged 10 commits into from Nov 11, 2022

Conversation

Ryex
Copy link
Contributor

@Ryex Ryex commented Oct 25, 2022

This PR enhances the user experience when dealing with mods blocked from 3rd party launchers.

Fixes #222

When the BlockedMods dialog is created it check the configured global mods folder and the users' downloads folder for the missing .jar names and if found verifies them by hash.
A filesystem watch is then established on both folders in case the user needs to download the missing mods.

The UI updates appropriately to indicate the blocked mods have been found.
Once the dialog has been accepted the mods are copied to the instance's mods folder.

Of note:

  • UI is not necessarily final.
  • There are a few flow questions to answer Namely,
    • should the dialog's OK button be disabled until all blocked mods are detected?
    • If blocked mod fails to copy for any reason should the whole instance creation be aborted?

Code review is needed.

Copy link
Contributor

@flowln flowln left a comment

Choose a reason for hiding this comment

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

just some preliminary stuff :)

launcher/modplatform/modpacksch/FTBPackInstallTask.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
@Ryex Ryex requested a review from flowln October 30, 2022 00:54
launcher/FileSystem.cpp Outdated Show resolved Hide resolved
launcher/modplatform/flame/FlameInstanceCreationTask.cpp Outdated Show resolved Hide resolved
launcher/modplatform/flame/FlameInstanceCreationTask.cpp Outdated Show resolved Hide resolved
launcher/modplatform/helpers/HashUtils.cpp Show resolved Hide resolved
launcher/modplatform/modpacksch/FTBPackInstallTask.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.h Outdated Show resolved Hide resolved
@flowln flowln added the enhancement New feature or request label Oct 30, 2022
@flowln flowln added this to the 6.0 milestone Oct 30, 2022
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Now that PrismLauncher#333 is merged and FS::copy works on non directory copyFile can be removed.

Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
@Ryex
Copy link
Contributor Author

Ryex commented Nov 1, 2022

Rebased and removed FS::copyFile now that #333 is merged with it's fix for FS::copy

launcher/modplatform/flame/FlameInstanceCreationTask.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
launcher/ui/dialogs/BlockedModsDialog.cpp Outdated Show resolved Hide resolved
Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
Copy link
Contributor

@flowln flowln left a comment

Choose a reason for hiding this comment

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

looks good to me, thanks!

Copy link
Member

@Scrumplex Scrumplex left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just a few smaller things.

Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com>
@Ryex
Copy link
Contributor Author

Ryex commented Nov 11, 2022

We all good now 👍

@DioEgizio
Copy link
Member

for people with a custom download location, would be cool if you could just drag and drop or browse and insert the mods

@Ryex
Copy link
Contributor Author

Ryex commented Nov 11, 2022

for people with a custom download location, would be cool if you could just drag and drop or browse and insert the mods

These are good suggestions and relatively easy to add, would you prefer them in this PR or a separate one?

@flowln
Copy link
Contributor

flowln commented Nov 11, 2022

These are good suggestions and relatively easy to add, would you prefer them in this PR or a separate one?

Probably another PR would be better, this one already has a good amount of changes, and will soon be merged :>

@flowln flowln merged commit 64576f4 into PrismLauncher:develop Nov 11, 2022
@Ryex Ryex mentioned this pull request Nov 12, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Curseforge Blocked mods automatically into mods folder
5 participants