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

Big rpak loading refactor #766

Merged
merged 53 commits into from
Sep 7, 2024

Conversation

ASpoonPlaysGames
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames commented Aug 18, 2024

Related docs PR: R2Northstar/ModdingDocs#251

Well this branch got out of hand.

Anyway, this PR reworks how rpaks are loaded, unloaded and tracked.

It allows for rpak reloading between map loads, meaning that skins and map overhauls could be enabled and disabled on the fly.

Previous methods of loading rpaks still work, but the new cool way is to format the rpak.json file like this:

{
	"mp_colony_snow.rpak": "mp_colony02",
	"multiplayer_only.rpak": "mp_.*",
	"singleplayer_only.rpak": "sp_.*"
}

An rpak will be loaded if the map that is being loaded matches the regex given in the json.

How to test:

Ensuring old behaviour

  1. acquire an rpak mod that uses Preload
  2. acquire an rpak mod that uses Postload
  3. acquire an rpak mod that uses Aliases
  4. make sure that those three work as before. textures load etc.

Testing new behaviour

  1. take an rpak mod that is like a skin or something
  2. edit the rpak.json to load the rpak on a map of your choice with regex
  3. make sure that the rpak loads as intended
  4. disable the mod or remove the entry from the rpak.json and reload_mods
  5. go back into the same map
  6. the skin should no longer be showing

Update for testing new behaviour:

Here is an example mod that uses the new rpak loading system. It's a skin for the satchel and with the rpak.json setup it will only load on mp_angel_city

To further test this example mod, change the mp_angel_city to some regex that will match some maps, all maps, or whatever and test it. You shouldn't need to restart the game entirely for these changes to be respected, simply reload_mods and then load a level. For example mp_ should match any multiplayer map and . should match every map.

@ASpoonPlaysGames 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 Aug 18, 2024
Copy link
Contributor Author

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

The new method of loading/unloading rpaks seems to have been working fine in the ctf playtest we did. Didn't run any mods that load the old method though.

edit: turns out I was also running a mod that used Aliases so that is also confirmed to be working.
second edit: just tested with a mod that used Preload and that also works fine
epic third edit: went ahead and tested Postload mods as well, also seem to work fine. Notably they got unloaded when disabling the mod but didn't appear to get loaded again afterwards when re-enabling the mod.

@ASpoonPlaysGames
Copy link
Contributor Author

Ok this commit should make Postload and Preload rpaks get reloaded nicely when doing reload_mods and changing map. Tested this locally but ofc a second tester would be appreciated.

Copy link
Contributor

@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.

From quick testing:

  • troll face did not appear with main build;
  • with PR, troll face did appear on angel city;
  • with PR, updating rpak.json file (to "trol.rpak": "mp_drydock") and then running reload_mods, then map mp_angel_city made the troll face disappear, as expected.

Could you also provide more detailed instructions regarding Preload, Postload and Aliases testing? Maybe by updating the test mod?
(I still have to take a look at the code.)

@Alystrasz Alystrasz self-requested a review August 30, 2024 23:39
@ASpoonPlaysGames
Copy link
Contributor Author

Could you also provide more detailed instructions regarding Preload, Postload and Aliases testing? Maybe by updating the test mod?

That stuff is just maintaining old behaviour. Best way to test would be to grab some mods and give them a go, make sure they act like normal.

Any mod made with advocate should be using Preload. Most model mods (that arent using advocate) will probably be using Postload, and camo mods tend to use Aliases.

All of these should work as usual, with Preload and Postload also supporting hot reloading now (though no model reloading so model textures will be reloadrd but not the models themselves)

Copy link
Contributor

@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.

Code looks good to me afaik.


I tested the PR with these 3 mods:

All 3 mods behave the same on this PR than on main, and I confirm Preload and Postload support hot reloading (with reload_mods + reload commands ran successively).

primedev/core/filesystem/rpakfilesystem.cpp Show resolved Hide resolved
primedev/core/filesystem/rpakfilesystem.cpp Outdated Show resolved Hide resolved
Co-authored-by: Rémy Raes <contact@remyraes.com>
@ASpoonPlaysGames 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 testing Changes from the PR still need to be tested labels Sep 2, 2024
@GeckoEidechse
Copy link
Member

The fact that this is even fully backwards compatible with current rpak.json just blows my mind and is gonna make this so much nicer to join <3

Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

Gotta say this is very much northstar code. I'd say any improvements should just be separate PRs or this gets stuck we know where :)

@F1F7Y F1F7Y added READY TO MERGE This mergeable right now and removed needs code review Changes from PR still need to be reviewed in code almost ready to merge Apart from any small remaining other issues addressed by other labels, this would be ready to merge labels Sep 3, 2024
ASpoonPlaysGames added a commit to ASpoonPlaysGames/Advocate that referenced this pull request Sep 6, 2024
@GeckoEidechse GeckoEidechse merged commit 160f503 into R2Northstar:main Sep 7, 2024
4 checks passed
wolf109909 pushed a commit to R2NorthstarCN/NorthstarLauncher that referenced this pull request Sep 8, 2024
This reworks how rpaks are loaded, unloaded and tracked.
It allows for rpak reloading between map loads, meaning that skins and map overhauls could be enabled and disabled on the fly.
Previous methods of loading rpaks still work.
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.

4 participants