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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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
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
Status: No status
Development

Successfully merging this pull request may close these issues.

1 participant