-
Notifications
You must be signed in to change notification settings - Fork 127
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: Multiple mod versions #758
Open
Alystrasz
wants to merge
58
commits into
R2Northstar:main
Choose a base branch
from
Alystrasz:fix/multiple-mod-versions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ove NSGetModDescription function
15 tasks
The two PRs are now ready for review, I'll gladly take any feedback. |
ASpoonPlaysGames
added
needs testing
Changes from the PR still need to be tested
needs code review
Changes from PR still need to be reviewed in code
labels
Sep 5, 2024
ASpoonPlaysGames
approved these changes
Sep 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Github is absolutely butchering this diff but I think code looks ok? Really hard to tell which code is actually new or not though.
ASpoonPlaysGames
added
almost ready to merge
Apart from any small remaining other issues addressed by other labels, this would be ready to merge
and removed
needs code review
Changes from PR still need to be reviewed in code
labels
Sep 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
almost ready to merge
Apart from any small remaining other issues addressed by other labels, this would be ready to merge
MAD
Related to mod-auto-download
needs testing
Changes from the PR still need to be tested
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Associated mods PR: R2Northstar/NorthstarMods#824
Problem
Whether through manual installation or through mod auto-downloading, several versions of the same mod can be installed in Northstar, which currently does not handle this:
main_status.webm
This video was recorded using a profile with two Parkour mod versions,
v0.1.2
andv0.2.1
: note how the client does not distinguish between the two versions (version displayed in the UI is the same for both mods), and is not capable of (de)activating any of them.Description
New
enabledmods.json
formatSince the current
enabledmods.json
format does not allow for multiple mod versions, it has been reworked.With this PR, if "old" file format is detected,
enabledmods.json
will automatically be converted to "new" format.Old format
New format
(new
enabledmods.json
format also means mod managers will have to adapt)Squirrel-exposed functions
NSIsModEnabled(string modName)
is nowNSIsModEnabled(string modName, string modVersion)
;NSSetModEnabled(string modName, bool enabled)
is nowNSSetModEnabled(string modName, string modVersion, bool enabled)
;NSIsModRemote(string modName)
is nowNSIsModRemote(string modName, string modVersion)
;NSGetModVersionByModName(string modName)
is nowNSGetModVersions(string modName)
;NSGetModsInformation
method, that returns a list of mods with all associated information at once (and not only names likeNSGetModNames
currently does it);NSGetModsInformation
, the following functions were removed:NSGetModDescriptionByModName
,NSGetModDownloadLinkByModName
,NSGetModLoadPriority
,NSIsModRequiredOnClient
,NSGetModConvarsByModName
Result
multiple_versions_handling.webm
This video was recorded using a profile with two Parkour mod versions,
v0.1.2
andv0.2.1
: the client now differentiate the two versions (they both appear correctly on the UI), and is capable (de)activating any of them.Closes #670.
Closes #757.
Testing (manifesto format)
As this PR modifies your
enabledmods.json
file, you might want to copy it somewhere safe before testing.Test scenarios
Launch game:
with no
enabledmods.json
file at allenabledmods.json
should be created using new formatwith an invalid
enabledmods.json
file (not following JSON format)enabledmods.json
should be renamed intoenabledmods.json.old
enabledmods.json
should be created using new formatwith
enabledmods.json
using old formatenabledmods.json
file should be renamed intoenabledmods.json.old
enabledmods.json
should be recreated using new formatwith
enabledmods.json
using new formatenabledmods.json
configuration (you can check that in the Mods menu)TODOs
UnloadMods
methodGenerateModsConfigurationFile
method (since mod entries are already created ~L667)NSSetModEnabled(name,bool
toNSSetModEnabled(name,ver,bool
)