NorthstarLauncher
NorthstarLauncher copied to clipboard
R2Northstar
feat: Bad mod formatting warnings
While loading mods, this checks if a mod.json file is present in mods directories; if not, this will display a warning message, telling user mod hasn't been installed properly.
This does not analyze hidden directories (with a filename beginning with a .).
Depends on (cyclic dependency):
- [ ] https://github.com/R2Northstar/NorthstarMods/pull/597
@GeckoEidechse if you want to keep doing your shenanigans, this will ignore your git folders grin
Depending on how fast VTOL switches to using enabledmods.json we might also have to ignore the disabled folder it uses to hide the mod.json.
@BigSpice is there an ETA on the fix in VTOL? ^^
Actually I just realised that this might also cause issues for players where there is a leftover disabled folder in the mods dir. Not sure if Viper ever cleaned that up after switching to enabledmods.json.
It might make more sense to check folder recursively for mod.json and only warn if there is actually one in a subdirectory.
It might make more sense to check folder recursively for mod.json and only warn if there is actually one in a subdirectory.
What about mod folders with no mod.json file? (quite unlikely, I know)
What about mod folders with no
mod.jsonfile? (quite unlikely, I know)
For that to occur, the player must have manually deleted that file, given that a mod author wouldn't release a mod with missing mod.json cause that mod would have never loaded on their end in the first place. I'd argue that this would be a rarer occurrence then a player accidentally creating an empty folder. ^^
It might make more sense to check folder recursively for mod.json and only warn if there is actually one in a subdirectory.
Addressed in c0e44c9.
concept is good, but would be nicer if this was exposed to script and shown in ui, rather than than shown as a messagebox? messagebox would be annoying in cases where mod formatting isn't an issue since it'd block execution
concept is good, but would be nicer if this was exposed to script and shown in ui, rather than than shown as a messagebox?
What method would you use to show error message in UI? All cool UI methods I know are callable from Squirrel VM.
messagebox would be annoying in cases where mod formatting isn't an issue since it'd block execution
Yeah that's also cool, because this way we're sure user knows some of their mods are fucked (in comparison with a notification that closes automatically without user noticing it)
What method would you use to show error message in UI? All cool UI methods I know are callable from Squirrel VM.
something on the title screen to display warnings would work, would need to expose all messages to squirrel on reload and do some ui stuff in script to display them
Yeah that's also cool, because this way we're sure user knows some of their mods are fucked (in comparison with a notification that closes automatically without user noticing it)
it'd be very annoying for developers in situations where they know they'll get the warning and don't want to wait for a blocking textbox every startup, is what i'm thinking
What method would you use to show error message in UI? All cool UI methods I know are callable from Squirrel VM.
Define a global function in the UI that displays a dialog / menu / submenu that explains what's going on and call that function from native after the UI VM is initialized and mods that are formatted wrong are detected
Reviews have been addressed, @uniboi implemented the UI part in https://github.com/R2Northstar/NorthstarMods/pull/597. I'm marking this as ready for reviews.
Do note that this changes the function signatures of some script methods as they are currently incorrect. https://github.com/R2Northstar/NorthstarMods/pull/597 fixes those issues as well.
Wait, how does this depend on https://github.com/R2Northstar/NorthstarMods/pull/597 ?
Isn't it the other way round or do both depend on each other?
As in can this launcher PR not be added alone without breaking something in mods?
As in can this launcher PR not be added alone without breaking something in mods?
It definitely can, but you won't see any displayed errors, which is the point of both PRs 😃
As in can this launcher PR not be added alone without breaking something in mods?
It definitely can, but you won't see any displayed errors, which is the point of both PRs 😃
Oh yeah, that's totally fine and expected :P The "Depends on" just threw me off, updating PR description accordingly ^^
Wait, how does this depend on R2Northstar/NorthstarMods#597 ?
Isn't it the other way round or do both depend on each other?
As in can this launcher PR not be added alone without breaking something in mods?
No, this introduces compile errors (without the mods pr) because I corrected some signatures of functions exposed to squirrel.
What's your issue with smart pointers?
The fact we don't use them anywhere else ( apart from logging, but that dont count ). Also the fact that a few lines up there is a std::vector<Mod>.
My personal opinion is that this can be merged as is since ill probably get to cleaning this up ( and possibly including some of bobs changes from mod loading refactor ) sooner or later.
@Alystrasz just a heads-up that this PR sadly has merge conflicts now ^^"
@Alystrasz just a heads-up that this PR sadly has merge conflicts now ^^"
Done :)
@Alystrasz would this need a rework to check packages as well? We already have basic format requirements for packages already so maybe not but want your opinion
@Alystrasz would this need a rework to check
packagesas well? We already have basic format requirements forpackagesalready so maybe not but want your opinion
About packages, currently we only check if directory names match the AUTHOR-MOD-VERSION pattern, not checking if the mod manifest is at the right place; this has been added in e398b5cb5a40ed08b0fff20ed78df9dddf51170e and 0d6b62491326bbf764bbb5ad32fad5dc8772cb9d.
I removed the "READY TO MERGE" label since the diff shows this is deleting functions such as NSGetModNames (probably some merge operation failed), I need to address that.
Waiting for @uniboi to re-add the NSGetInvalidMods function with new syntax since they've done it in the first place :)