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

Add support for loading Thunderstore mods natively #503

Merged
merged 10 commits into from
Jul 16, 2023

Conversation

GeckoEidechse
Copy link
Member

@GeckoEidechse GeckoEidechse commented Jul 5, 2023

This PR allows for loading Thunderstore mods directly from a separate directory (called thunderstore-mods rn, I'm happy to change was changed to packages).

It expects a folder that represents the unpacked zip from the mod from Thunderstore. This heavily simplifies mod management as now there's a direct link between Thunderstore mod and Northstar (Thunderstore folder name).

image

Loading user installed Northstar mods from R2Northstar/mods/ is still supported for now as mod-managers need to be updated first and as a fallback for mods not on Thunderstore.

Mod-managers could then also take care of moving existing Thunderstore mods from mods/ to thunderstore-mods/ packages.

I will take care of notifying mod-manager authors once a decision about merging (or not merging) this PR has been made.

This PR is an initial implementation and could be adapted based on feedback. As a reminder I barely know how to read C++ and simply took existing code as an inspiration. If something's done in a peculiar way, it's most likely not cause I was trying to be "clever" but simply used what worked so please point out anything odd you might see in the code.

Related: R2Northstar/Northstar#472

Renamed to `packages` cause it's essentially mod packages.
to match `AUTHOR-MOD-VERSION` pattern.
@GeckoEidechse GeckoEidechse added the feedback wanted Feedback is wanted whether the changes by this PR are desired label Jul 6, 2023
@GeckoEidechse GeckoEidechse requested a review from F1F7Y July 6, 2023 13:15
@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Jul 6, 2023

I put you under review request specifically @F1F7Y as you had some ideas for a v2 rewrite of mods (which I'm all in favour). I see this PR as a good way to prepare for a v2 and as such wanna make sure that it's set up in a way that helps us to get to a v2 ^^

So feedback would be super appreciated when you get the chance to <3

@F1F7Y
Copy link
Member

F1F7Y commented Jul 6, 2023

I put you under review request specifically @F1F7Y as you had some ideas for a v2 rewrite of mods (which I'm all in favour). I see this PR as a good way to prepare for a v2 and as such wanna make sure that it's set up in a way that helps us to get to a v2 ^^

So feedback would be super appreciated when you get the chance to <3

Going through it it looks good, will review tomorrow when i get home.

Ignoring core mods would this fully replace the mods/ folder or would we have two folders for for manual install and Thunderstore packages?

Also considering 1 thunderstore package can have multiple mods it might be a good idea to display the thunderstore package name to the user in some way. ( Id keep this for a future PR, just worth mentioning here imo )

@GeckoEidechse
Copy link
Member Author

Going through it it looks good, will review tomorrow when i get home.

Awesome, super appreciated <3

Ignoring core mods would this fully replace the mods/ folder or would we have two folders for for manual install and Thunderstore packages?

That's what we gotta figure out next. Right now all documentation for manually install explains how to extract the folder into the mods/ folder. If we wanna change it to packages/ we would first have to update the documentation, then add some deprecation notices when loading non-core mods, then stop loading mods from mods/ dir.

However that doesn't handle the case of what to do with non-Thunderstore mods, e.g. a modder testing a current build of their mod that isn't published to Thunderstore yet. Maybe have another directory to differentiate between core, Thunderstore, and manual (non-thunderstore)? At that point things are gonna start getting chaotic though, especially when you also factor in auto-downloaded mods.

So yeah, don't really have a good solution for that yet. My suggestion would be to keep manually installed non-Thunderstore mods in mods/ and for manually installing Thunderstore mods, update the documentation so that user can install it in a way that it passes the regex and in turn can then also be picked up by mod-managers ^^

Also considering 1 thunderstore package can have multiple mods it might be a good idea to display the thunderstore package name to the user in some way. ( Id keep this for a future PR, just worth mentioning here imo )

100% agreed. Thunderstore packages contain a manifest.json So we could use that and/or the folder name to see which Thunderstore mod a Northstar mod is part of and then display that within the Northstar mod menu.

@Alystrasz Alystrasz self-requested a review July 6, 2023 22:32
Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

Works in testing

Code looks okay, but the whone ModManager class should probably be refactored for auto-download

@GeckoEidechse
Copy link
Member Author

Code looks okay, but the whole ModManager class should probably be refactored for auto-download

Would we do that in #362 or in a separate PR before/after? ^^

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Jul 10, 2023

Created #505 to discuss timeline of implementation with mod-manager authors under the assumption that apart from technical aspects this PR will be merged.

@F1F7Y
Copy link
Member

F1F7Y commented Jul 10, 2023

Would we do that in #362 or in a separate PR before/after? ^^

Unsure, we'll see how launcher PRs progress as I'd like to implement bobs changes, but making multiple large PRs is just asking for trouble.

Regarding this PR i'm fine with this getting merged.

@F1F7Y F1F7Y added almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge and removed feedback wanted Feedback is wanted whether the changes by this PR are desired labels Jul 14, 2023
@GeckoEidechse GeckoEidechse merged commit 0309af1 into main Jul 16, 2023
@GeckoEidechse GeckoEidechse deleted the add-thunderstore-mods-folder branch July 16, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants