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 #826

Merged
merged 11 commits into from
Nov 21, 2024

Conversation

Alystrasz
Copy link
Contributor

@Alystrasz Alystrasz commented Nov 1, 2024

Currently, to retrieve mod information from launcher, Squirrel VM must summon methods such as NSGetModDescriptionByModName, NSGetModDownloadLinkByModName, NSGetModLoadPriority etc, which all loop over the list of mods until they find the correct one (using mod name as key), which is highly ineffective: this means we need as many iterations over mods list as we need information about the mod.

To solve this problem, this PR introduces a NSGetModsInformation, which exposes all mod information to Squirrel VM at once, using a new ModInfo structure.

global struct ModInfo
{
    string name = ""
    string description = ""
    string version = ""
    string downloadLink = ""
    int loadPriority = 0
    bool enabled = false
    bool requiredOnClient = false
    bool isRemote
    array<string> conVars = []
}

This is meant to make reviewing #758 and R2Northstar/NorthstarMods#824 easier.

(I highly discourage you to look at the github diff, which is fucked up, and suggest rather looking at the changes commit per commit!)

Don't merge without mods counterpart: R2Northstar/NorthstarMods#899

Testing

  1. Install both launcher and mods PRs;
  2. Start the game;
  3. Try to break the mod menu;
  4. Connect to Northstar;
  5. Try to break the server browser.

@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
primedev/scripts/client/scriptmodmenu.cpp Outdated Show resolved Hide resolved
primedev/scripts/client/scriptmodmenu.cpp Outdated Show resolved Hide resolved
primedev/scripts/client/scriptmodmenu.cpp Outdated Show resolved Hide resolved
@Alystrasz Alystrasz requested a review from RoyalBlue1 November 20, 2024 20:37
@Alystrasz Alystrasz requested a review from RoyalBlue1 November 20, 2024 21:37
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.

Requested changes have been implemented and code looks good

@Alystrasz Alystrasz removed the needs code review Changes from PR still need to be reviewed in code label Nov 21, 2024
@RoyalBlue1 RoyalBlue1 merged commit 2d97883 into R2Northstar:main Nov 21, 2024
4 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 testing Changes from the PR still need to be tested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants