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

Only fetch MAD manifesto on server join #751

Merged

Conversation

Alystrasz
Copy link
Contributor

@Alystrasz Alystrasz commented Jul 14, 2024

Previously, the verified mods manifesto was fetched on game start without checking if the verified mod feature is enabled Squirrel
-side; with this, the manifesto is only fetched when the user wants to download a mod (meaning they enabled the feature beforehand).

(must be merged with the mods PR!)

Launcher changes

  • Expose FetchModsListFromAPI method to Squirrel VM
  • Do not invoke FetchModsListFromAPI method on client start
  • Add ModInstallState.MANIFESTO_FETCHING enum value to better track mod install progress

TODOs

Testing

Without PRs:

  1. Launch game;
  2. Check the logs: they should display verified mods manifesto was fetched close to game launch;

With PRs:

  1. Install both launcher and mods PRs;
  2. Launch game, reach MP lobby and enable MAD (allow_mod_auto_download 1);
  3. Try and connect to a server requiring verified mods (Space battle or Parkour server should be up at the time of writing);
  4. Check the logs: manifesto should be fetched when trying to join the server.
Media
delayed_manifesto_fetching.webm

(if you look closely, you can see verified mods manifesto is fetched WHEN joining the server, around 0:12 in the video)

@Alystrasz Alystrasz marked this pull request as ready for review July 15, 2024 22:40
Copy link
Member

@EladNLG EladNLG left a comment

Choose a reason for hiding this comment

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

Reviewed it all, don't see anything needing changing, not tested.

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

As far as I understand the code changes they seem correct to me ^^

primedev/mods/autodownload/moddownloader.cpp Show resolved Hide resolved
@GeckoEidechse GeckoEidechse added needs testing Changes from the PR still need to be tested almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Jul 26, 2024
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Confirmed working in testing together with R2Northstar/NorthstarMods#821

Enabled MAD and joined a server. Observed the delayed manifesto fetching as intended.

@GeckoEidechse GeckoEidechse added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Jul 28, 2024
@GeckoEidechse GeckoEidechse changed the title refactor: MAD manifesto VM fetching Only fetch MAD manifesto on server join Jul 29, 2024
@GeckoEidechse GeckoEidechse merged commit 6551632 into R2Northstar:main Jul 29, 2024
4 checks passed
@Alystrasz Alystrasz deleted the refactor/mad-manifesto-vm-fetching branch July 29, 2024 23:17
wolf109909 pushed a commit to R2NorthstarCN/NorthstarLauncher that referenced this pull request Jul 30, 2024
Previously, the verified mods manifesto was fetched on game start without checking if the verified mod feature is enabled Squirrel-side; with this, the manifesto is only fetched when the user wants to download a mod (meaning they enabled the feature beforehand).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants