-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: support thunderstore ror2mm protocol for installing mods #248
base: main
Are you sure you want to change the base?
Conversation
src/modules/protocol.js
Outdated
if (version.northstar() == "unknown") | ||
return; | ||
|
||
const args = process.argv.slice(app.isPackaged ? 1 : 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this for the sake of sanity
when packaged the first argument is the bundled electron app
when not packaged the first argument is electron and the second the script.
Not really needed, can remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ternary operators are the devil and not as readable imo, something like this seems better:
let args = process.argv;
if (app.isPackaged) {args.shift()}
This is achieved by sending an IPC event to the renderer and waiting for a reply once Since this is async we return from the function after sending the event and recursively invoke it once the reply arrives The package data is returned a a JSON String because Electron is unable to copy the Object over IPC
the only thing I think is missing here is preventing multiple instances of viper from being open using requestSingleInstanceLock and forwarding any information to the already running process |
Viper now requests an instance lock, preventing multiple instances of running at once and forwarding process info to the main process. made Would it be good to add a toast message when a mod is installed via the ror2mm protocol? |
It may be worth also either forwarding regular CLI commands so that they get run, or displaying a message akin to "Existing Viper process found, exiting...", as from what I can tell (not able to test at this very moment), new instances simply exit without forwarding this or printing an error, personally I think the error would be better, as we'd otherwise also have to have the output of the CLI command be forwarded etc, and it would overall be a pain.
I would say so, there's no other hints it's been installed without that, right? If not then it would be a good idea |
The lock is acquired after the CLI checks, so they shouldn't be affected.
Alternatively, the active Viper window could be put into focus
There is a Toast after a mod was installed, which is good when the user presses the "Install" button in Viper, but when using the protocol there is nothing that states what its doing. |
I see, yeah I think it should perhaps just do as it's doing now with the CLI args, but if it's being launched normally/with a GUI, then show a message that it's exiting and why, then focus the existing window.
I think adding a toast for when the download is started would suffice just fine, on top of the normal "mod installed" toasts |
okay uhh |
By "launched normally" I mostly meant when launching Viper by itself, and not through the scheme handler, so yeah, handle CLI args if there are any, then if no instance exists simply start normally and do what needs to be done, otherwise pass over the URLs to the existing instance and exit with some message. |
Changes:
|
Video of this in action 2024-08-14_20-09-10.mp4 |
Currently there's no preferred way, mostly because the only place we use It may however be an idea to entirely omit the "Installing" part of the description i.e the toast would be:
Instead of:
But that's open for discussion... |
fine by me, less strings that needs translating |
its a bit redundant since the title already states that we are installing
Needed for when 0neGal/viper#248 is merged will allow mods to be installed through the browser
Implements #124 (as the first launcher, afaik)
details are up for discussion, this is a minimal implementation into the existing system.
2024-08-04_18-41-12.mp4