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

Plugins v2 are out in Northstar v1.13, let's discuss installation standard #288

Closed
GeckoEidechse opened this issue Apr 24, 2023 · 22 comments
Labels
question Further information is requested

Comments

@GeckoEidechse
Copy link
Member

GeckoEidechse commented Apr 24, 2023

With Northstar v1.13.0 plugins v2 released.

Plugins are more powerful than Squirrel mods as they are native code that interacts with Northstar via an interface.
From a file perspective, plugins are essentially just DLLs that go into the R2Northstar/plugins/ (R2Northstar could be replaced with any other profile ofc).

For easier installation of plugin there's discussion about installing plugins via Thunderstore using mod-managers.
(For security concerns in that regard, check #289)

Unlike mods, plugins don't ship a mod.json but rather stores information in the DLL resource.

Now like with mods, we need a way to uniquely map a plugin to a Thunderstore mod post installation.
We want compatibility between mod-managers again, so hence this issue to discuss how to handle the mapping.

Example plugin on Thunderstore right now: https://northstar.thunderstore.io/package/cat_or_not/kyurid/

@GeckoEidechse GeckoEidechse added the question Further information is requested label Apr 24, 2023
@GeckoEidechse
Copy link
Member Author

Poking

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Apr 24, 2023

One idea was simply renaming the plugin to the Thunderstore mod string.

So for example kyurid.dll would become cat_or_not-kyurid-0.1.0.dll which would instantly solve the entire thing for us cause we can just check the filename, except that a Thunderstore could contain multiple plugins and duplicate file names are not allowed x_x

@GeckoEidechse
Copy link
Member Author

Subfolders was another idea, i.e. moving R2Northstar/plugins/kyurid.dll into R2Northstar/plugins/cat_or_not-kyurid-0.1.0.dll/kyurid.dll but currently Northstar does not support loading plugins from subdir so we'd need to fix that upstream first.

@GeckoEidechse
Copy link
Member Author

One thing that would work would be prepending the Thunderstore mod string to the DLL, so like e.g. kyurid.dll -> cat_or_not-kyurid-0.1.0_kyurid.dll

Alternatively we stick an entire .json file into the plugins/ folder and treat that as a sorta database that maps dll names to Thunderstore mod string.

@AnActualEmerald
Copy link
Contributor

AnActualEmerald commented Apr 24, 2023

I think prepending the modstring would be the most compact way to do it as long as there's no risk of illegal characters. We'd have to agree on the delimiter too, and it would have to be something that wouldn't/couldn't appear in the modstring

@0neGal
Copy link
Contributor

0neGal commented Apr 24, 2023

Personally, I think at the very least having the option of subdirs would be nice. But then now you've 2 ways of doing it, largely complicating parsing the installed plugins.

And so, in my opinion it'd be best for folders to be in the Thunderstore mod string format, and everything else being treated as something the user manually installed.

I feel like the JSON database file is over complicating tbh...

@GeckoEidechse
Copy link
Member Author

And so, in my opinion it'd be best for folders to be in the Thunderstore mod string format, and everything else being treated as something the user manually installed.

Agreed. Subdirs named after Thunderstore mod string sounds like the cleanest solution ^^

@catornot
Copy link
Contributor

ok then subdirs should be restored somehow

@GeckoEidechse
Copy link
Member Author

ok then subdirs should be restored somehow

cat said and then made an upstream PR for it R2Northstar/NorthstarLauncher#460 👀

@GeckoEidechse
Copy link
Member Author

Thinking about it, if we stick plugin into separate folder already anyway, we could also add manifest.json to it ^^

@catornot
Copy link
Contributor

already did that

@ASpoonPlaysGames
Copy link
Contributor

Not gonna lie this seems like adding more spaghetti to the thunderstore package loading, we honestly need to rework a large chunk of this stuff

@GeckoEidechse
Copy link
Member Author

Not gonna lie this seems like adding more spaghetti to the thunderstore package loading, we honestly need to rework a large chunk of this stuff

This issue is about plugins specifically btw and we need some way to map a plugin back to a Thunderstore mod to allow for install detection and updating... ^^

@GeckoEidechse
Copy link
Member Author

Unless you can rewrite our whole mod loading system within a week, we will need to implement some way to allow for plugin installation on mod-managers in the meantime :P

@ASpoonPlaysGames
Copy link
Contributor

This issue is about plugins specifically btw

At a surface level, yeah. But the root problem is the gap between thunderstore package format and how launcher reads stuff. Personally I'm a big fan of the idea of just having a separate packages folder instead of doing all this stuff with finding mods/plugins in a package and doing all this extra stuff

@ASpoonPlaysGames
Copy link
Contributor

Unless you can rewrite our whole mod loading system within a week

I could, but at this point I'm not gonna touch it because any PR that gets made will get stuck in PR hell. All the conflicting standards, installation methods, etc. will make any meaningful refactor impossible because of things like existing mod installs.

Imo at this point, rip it all out and start again, with the thunderstore package format being the only format that is accepted. The problem is only going to get worse, so better to do it sooner rather than later

@GeckoEidechse
Copy link
Member Author

Unless you can rewrite our whole mod loading system within a week

All the conflicting standards, installation methods, etc. will make any meaningful refactor impossible because of things like existing mod installs.

Imo at this point, rip it all out and start again, with the thunderstore package format being the only format that is accepted. The problem is only going to get worse, so better to do it sooner rather than later

Agreed, I made an issue now to discuss this: R2Northstar/Northstar#472

However as you kinda realise it will take some time to implement a complete rewrite and we still need some way to install plugins via mod-managers in the meantime. Especially as anything done with mod-managers can be upgraded pretty simply in the future as the mod-managers can just add logic that converts an install from one format to another ^^

 

As a sidenote to PR hell, try to prioritise PRs on what I deem important atm (+ what is faster to review) in my limited time. I'm ofc not the only reviewer/maintainer and others might have other approaches. Overall a mod system refactor would be pretty high up on priority list 👀

@ASpoonPlaysGames
Copy link
Contributor

Agreed, I made an issue now to discuss this: R2Northstar/Northstar#472

Great, thanks

anything done with mod-managers can be upgraded pretty simply in the future as the mod-managers can just add logic that converts an install from one format to another

Yeah this is like our saving grace here tbh, shame that some people love to recommend manual installation over a mod manager

To be clear, I don't disagree with defining a standard for plugin installation. I just see people talking about copying manifests over and stuff and it reminds me how much of a mess this whole system is

@AnActualEmerald
Copy link
Contributor

So this is basically covered by R2Northstar/NorthstarLauncher#505 now right?

@GeckoEidechse
Copy link
Member Author

So this is basically covered by R2Northstar/NorthstarLauncher#505 now right?

Yes. It's technically not implemented in R2Northstar/Northstar#472 but will in time and should be treated accordingly as discussed in R2Northstar/NorthstarLauncher#505 (comment)

@0neGal
Copy link
Contributor

0neGal commented Jul 13, 2023

So this is basically covered by R2Northstar/NorthstarLauncher#505 now right?

Yes.

Of course the security warning should still be implemented, but for a library like libthermite it's a non-issue, since it'd likely be best to have that implemented by whatever is using the library...

@GeckoEidechse
Copy link
Member Author

Closing this as superseded by #415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants