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

Profile import summary #1430

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Profile import summary #1430

merged 2 commits into from
Sep 11, 2024

Conversation

ebkr
Copy link
Owner

@ebkr ebkr commented Sep 7, 2024

This PR aims to allow users to see which mods are going to be installed when importing a profile

image

image

@ebkr ebkr changed the base branch from develop to prevent-closing-profile-import-progress-modal September 7, 2024 16:21
Copy link
Collaborator

@anttimaki anttimaki left a comment

Choose a reason for hiding this comment

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

Overall the code changes look fine. Couple of possible issues:

  • Any local mods in the imported profile are not shown on the preview
  • https://github.com/ebkr/r2modmanPlus/blob/develop/src/r2mm/downloading/BetterThunderstoreDownloader.ts#L209 has this checkup for importing profiles that contains no mods found in Thunderstore's API. Usually this is due to importing the profile to a wrong game, but could be due to mod takedowns as well. Currently the preview is empty and if user proceeds the error is shown as before. It would be better UX to handle the "empty profile" case already on the preview phase and not let the user proceed.

I think these are somewhat niche cases so IDK if they should be considered blockers. The feature itself is probably valuable for most of the users as it is now.

@ebkr
Copy link
Owner Author

ebkr commented Sep 9, 2024

Any local mods in the imported profile are not shown on the preview

Locally imported mods aren't included anyway. We could potentially indicate which ones but I don't think it's a concern and is honestly probably better to inform the original exporter rather than the person importing the profile.

https://github.com/ebkr/r2modmanPlus/blob/develop/src/r2mm/downloading/BetterThunderstoreDownloader.ts#L209 has this checkup for importing profiles that contains no mods found in Thunderstore's API

That's a good spot that I didn't consider - I'll add a new path to say that there are no resolvable mods to import for that code.

@ebkr
Copy link
Owner Author

ebkr commented Sep 9, 2024

If running locally, this can be tested using: 0191d789-b860-5ed9-8a64-f3778f0f760e

image

@anttimaki
Copy link
Collaborator

Locally imported mods aren't included anyway. We could potentially indicate which ones but I don't think it's a concern and is honestly probably better to inform the original exporter rather than the person importing the profile.

Okay so what tripped me up on this is that the locally imported mods are included in the mods.yml and shown on local mod list, although the actual files for the mod aren't installed. Since locally importing the mod added it into the cache, the icon even worked for me on the mod list. But that's probably a niche issue and not related to changes in this PR, so not a blocker IMO.

That's a good spot that I didn't consider - I'll add a new path to say that there are no resolvable mods to import for that code.

Yeah, this is now a lot nicer experience for the user 👍

Base automatically changed from prevent-closing-profile-import-progress-modal to develop September 10, 2024 14:55
@anttimaki
Copy link
Collaborator

Hope I'm not jumping the gun when I merge this to get the changes to TSMM release.

@anttimaki anttimaki merged commit 1ffee55 into develop Sep 11, 2024
5 of 7 checks passed
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