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

refactor: Mod information #899

Merged
merged 15 commits into from
Nov 21, 2024

Conversation

Alystrasz
Copy link
Contributor

Mod counterpart of R2Northstar/NorthstarLauncher#826.

@github-actions github-actions bot 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 Nov 1, 2024
@Alystrasz Alystrasz added the depends on another PR Blocked until the PR it depends on is merged label Nov 12, 2024
Copy link
Contributor Author

@Alystrasz Alystrasz left a comment

Choose a reason for hiding this comment

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

On vc yesterday, Blue told NSGetModVersions and NSIsModRequiredOnClient methods are now useless, as all mod information should be accessed through NSGetModsInformation.
While I agree about NSIsModRequiredOnClient, I still think NSGetModVersions is required, notably in the main menu (we don't want to fetch info from all mods to display Northstar current version).
@RoyalBlue1 wdyt?

@RoyalBlue1
Copy link
Contributor

RoyalBlue1 commented Nov 19, 2024

I still think NSGetModVersions is required, notably in the main menu (we don't want to fetch info from all mods to display Northstar current version).

Maybe instead NSGetModVersions creating a function that can get all info for a singular mod might be more usefull while still minimizing the Api between Native and squirrel

@Alystrasz
Copy link
Contributor Author

I still think NSGetModVersions is required, notably in the main menu (we don't want to fetch info from all mods to display Northstar current version).

Maybe instead NSGetModVersions creating a function that can get all info for a singular mod might be more usefull while still minimizing the Api between Native and squirrel

NSGetModInformation was added in d0286a9; NS mod getters/setters were removed in the following commits (with NSSetModEnabled as the only exception).

Copy link
Contributor

@RoyalBlue1 RoyalBlue1 left a comment

Choose a reason for hiding this comment

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

LGTM changes all the uses of the singular functions to get mod info to the struct

@GeckoEidechse GeckoEidechse merged commit e7aa1c2 into R2Northstar:main Nov 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depends on another PR Blocked until the PR it depends on is merged needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants